Bug 113214 - false-positive -Wstringop-overflow warning with thread sanitizer
Summary: false-positive -Wstringop-overflow warning with thread sanitizer
Status: UNCONFIRMED
Alias: None
Product: gcc
Classification: Unclassified
Component: sanitizer (show other bugs)
Version: 13.2.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks: Wstringop-overflow
  Show dependency treegraph
 
Reported: 2024-01-03 11:46 UTC by Arnd Bergmann
Modified: 2024-02-05 18:38 UTC (History)
8 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Arnd Bergmann 2024-01-03 11:46:14 UTC
I came across another -Wstringop-overflow warning while building the kernel in a newly added device driver, when the thread sanitizer is enabled. Reduced my test case to

void _dev_warn(const void *dev, ...);

struct xe_uc {
        int guc;
};

struct xe_gt { 
        struct xe_tile *tile;
        struct pf_queue {
                unsigned int data[128];
                unsigned int tail;
        } pf_queue[4];
        struct xe_uc uc;
};
#define container_of(ptr, type, member) ({                              \
        void *__mptr = (void *)(ptr);                                   \
        ((type *)(__mptr - __builtin_offsetof(type, member))); })


void xe_guc_pagefault_handler(struct xe_uc *uc, int asid, void *msg, int len)
{
        struct xe_gt *gt = container_of(uc, struct xe_gt, uc);
        void *xe = gt->tile;
        struct pf_queue *pf_queue;
        if (len != 4)
                return;
        pf_queue = &gt->pf_queue[asid % 4];
        __builtin_memcpy(pf_queue->data + pf_queue->tail,
                         msg, len * sizeof(unsigned int));
                         
        _dev_warn(xe);
}

Original source code at
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/xe/xe_gt_pagefault.c?h=next-20240103#n322

Reproducer at https://godbolt.org/z/MMaz8rqcj

aarch64-linux-gcc-13.2 -Wall -O2 -fsanitize=thread -Werror=stringop-overflow -Wall -c xe_gt_pagefault.c 
xe_gt_pagefault.c: In function 'xe_guc_pagefault_handler':
xe_gt_pagefault.c:26:9: error: writing 16 bytes into a region of size 0 [-Werror=stringop-overflow=]
   26 |         __builtin_memcpy(pf_queue->data + pf_queue->tail,
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   27 |                          msg, len * sizeof(unsigned int));
      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
xe_gt_pagefault.c:6:25: note: at offset 8 into destination object 'tile' of size 8
    6 |         struct xe_tile *tile;
      |                         ^~~~
cc1: some warnings being treated as errors

Currently I see this with gcc-13.x and gcc-14.0 but not gcc-12.
Comment 1 Andrew Pinski 2024-01-03 21:51:11 UTC
Note the GCC manual does say:
```
Note that sanitizers tend to increase the rate of false positive warnings, most notably those around -Wmaybe-uninitialized. We recommend against combining -Werror and [the use of] sanitizers.
```

So I suspect this should be closed as won't fix ...
Comment 2 Arnd Bergmann 2024-02-05 16:49:32 UTC
The warning is now turned off in the kernel as a workaround:

https://lore.kernel.org/all/CAHk-=whzBDLC024NXgJEsFOOpJ9BO2BkuxHXr4h5wOSYK9AwbQ@mail.gmail.com/

Also, my local one-line workaround is applied for this driver, but this
workaound is clearly not useful as a general solution:

https://lore.kernel.org/lkml/sbbfz5zzdjj7hjcmyqvof3roe6zb43kflgmweopfu65hllxdep@m4pxjiuqxood/#t
Comment 3 Jakub Jelinek 2024-02-05 18:38:30 UTC
I think the reason for the warning is fre5 optimizing
   _21 = &MEM[(struct xe_gt *)uc_8(D) + -2072B].tile;
...
-  _20 = uc_8(D) + 18446744073709549544;
-  _2 = _20 + _19;
+  _2 = _21 + _19;
...
   _5 = _4 * 4;
   _6 = _2 + _5;
...
   MEM <uint128_t> [(char * {ref-all})_6] = _13;
and the -Wstringop-overflow warning stuff (done during the strlen pass) considering it then to be access into the tile member rather than anywhere into the structure.

Sure, if one writes:
void foo (struct xe_gt *p, int i) { uint128_t *q = (uint128_t *) &p->tile; q += i; *q = 0; }
in the source, then it will be UB not just because of the most likely aliasing violation, but also because the pointer in some kind of Martin's strict reading is just to the particular element rather than whole structure.
But 
void baz (struct xe_tile **);
void bar (struct xe_gt *p, int i) { baz (&p->tile); uint128_t *q = (uint128_t *) p; q += i; *q = 0; }
should be fine.
The reason it doesn't trigger without -fsanitize=thread is that then nothing takes address of the &(uc + cst)->tile in that case, it is just read, so there is nothing to CSE.
Before IPA we try to maintain what the address taking refers to exactly for builtin {,dynamic} object size 1/3 modes, but afterwards such distinctions are lost.