[Bug sanitizer/113214] New: false-positive -Wstringop-overflow warning with thread sanitizer

[Bug sanitizer/113214] New: false-positive -Wstringop-overflow warning with thread sanitizer

[arnd at linaro dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113214

            Bug ID: 113214
           Summary: false-positive -Wstringop-overflow warning with thread
                    sanitizer
           Product: gcc
           Version: 13.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: arnd at linaro dot org
                CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org,
                    jakub at gcc dot gnu.org, kcc at gcc dot gnu.org, marxin at gcc dot gnu.org
  Target Milestone: ---

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.

[Bug sanitizer/113214] false-positive -Wstringop-overflow warning with thread sanitizer

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113214

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
             Blocks|                            |88443

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
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 ...


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88443
[Bug 88443] [meta-bug] bogus/missing -Wstringop-overflow warnings

[Bug sanitizer/113214] false-positive -Wstringop-overflow warning with thread sanitizer

[arnd at linaro dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113214

--- Comment #2 from Arnd Bergmann <arnd at linaro dot org> ---
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

[Bug sanitizer/113214] false-positive -Wstringop-overflow warning with thread sanitizer

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113214

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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.