[Bug sanitizer/99673] New: [11 Regression] bogus -Wstringop-overread warning with address sanitizer

[Bug sanitizer/99673] New: [11 Regression] bogus -Wstringop-overread warning with address sanitizer

[arnd at linaro dot org]

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

            Bug ID: 99673
           Summary: [11 Regression] bogus -Wstringop-overread warning with
                    address sanitizer
           Product: gcc
           Version: 11.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: ---

Created attachment 50435
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50435&action=edit
manually reduced test case

gcc-11 warns about one file in the linux kernel, in which it fails to find the
size of an object:

$ arm-linux-gnueabi-gcc -Os -fno-inline-functions-called-once 
-fsanitize=address
In function ‘ath11k_peer_assoc_h_vht’,
    inlined from ‘ath11k_peer_assoc_prepare’ at
drivers/net/wireless/ath/ath11k/mac.c:92:2:
drivers/net/wireless/ath/ath11k/mac.c:66:13: warning:
‘ath11k_peer_assoc_h_vht_masked’ reading 16 bytes from a region of size 4
[-Wstringop-overread]
   66 |         if (ath11k_peer_assoc_h_vht_masked(vht_mcs_mask))
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/wireless/ath/ath11k/mac.c: In function ‘ath11k_peer_assoc_prepare’:
drivers/net/wireless/ath/ath11k/mac.c:66:13: note: referencing argument 1 of
type ‘const u16 *’ {aka ‘const short unsigned int *’}
drivers/net/wireless/ath/ath11k/mac.c:49:1: note: in a call to function
‘ath11k_peer_assoc_h_vht_masked’
   49 | ath11k_peer_assoc_h_vht_masked(const u16
vht_mcs_mask[NL80211_VHT_NSS_MAX])
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I can't see where the '4' even comes from here, both in the original test case
and the reduced version

https://godbolt.org/z/79GE8M

$ arm-linux-gnueabi-gcc --version
arm-linux-gnueabi-gcc (GCC) 11.0.1 20210315 (experimental)

The behavior seems to be target independent, I can reproduce it on arm and x86.

[Bug sanitizer/99673] [11 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |11.0
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
                 CC|                            |msebor at gcc dot gnu.org
   Last reconfirmed|                            |2021-03-19
            Summary|[11 Regression] bogus       |[11 Regression] bogus
                   |-Wstringop-overread warning |-Wstringop-overread warning
                   |with address sanitizer      |with address sanitizer due
                   |                            |to member address
                   |                            |substitution

--- Comment #1 from Martin Sebor <msebor at gcc dot gnu.org> ---
A further reduced/simplified test case is below.  The root cause is similar to
the one discussed in pr99578 comment 8.

$ cat pr99673.c && gcc -Os -S -Wall -fsanitize=address
-fno-inline-functions-called-once -m32 pr99673.c
struct B {
  int i;
  struct A {
    short sa[8];
  } a[2];
};

struct C {
  char n, ax[];
};

struct D { int i, j, k; };

int f (const short[8]);

void g (struct C *pc, struct D *pd, int i)
{
  struct B *pb = (void *)pc->ax;
  pd->i = pb->i;

  const short *psa = pb->a[i].sa;
  if (f (psa))
    return;
}
pr99673.c: In function ‘g’:
pr99673.c:22:7: warning: ‘f’ reading 16 bytes from a region of size 4
[-Wstringop-overread]
   22 |   if (f (psa))
      |       ^~~~~~~
pr99673.c:22:7: note: referencing argument 1 of type ‘const short int *’
pr99673.c:14:5: note: in a call to function ‘f’
   14 | int f (const short[8]);
      |     ^

The output of -fdump-tree-optimized shows what triggers the warning.  The
difference between the case discussed in pr99578 comment 8 and this is here
that it's the sanopt pass that replaces the pb->a[i].sa as the argument to f()
with the address of pc->i plus some offset.  pc->i is a 4-byte int, which is
where the 4 bytes in the warning comes from.

;; Function g (g, funcdef_no=0, decl_uid=2314, cgraph_uid=1, symbol_order=0)

void g (struct C * pc, struct D * pd, int i)
{
  const short int * psa;
  int _1;
  sizetype _9;
  sizetype _10;
  sizetype _11;
  unsigned int _12;
  int * _13;
  int * _14;
  unsigned int _17;
  unsigned int _18;
  signed char * _19;
  signed char _20;
  _Bool _21;
  unsigned int _22;
  signed char _23;
  signed char _24;
  _Bool _25;
  _Bool _26;
  unsigned int _27;
  unsigned int _28;
  unsigned int _29;
  signed char * _30;
  signed char _31;
  _Bool _32;
  unsigned int _33;
  signed char _34;
  signed char _35;
  _Bool _36;
  _Bool _37;

  <bb 2> [local count: 1073741824]:
  _13 = &MEM[(struct B *)pc_2(D) + 1B].i;   <<< address of pc.i
  _12 = (unsigned int) _13;
  _17 = _12 >> 3;
  _18 = _17 + 536870912;
  _19 = (signed char *) _18;
  _20 = *_19;
  _21 = _20 != 0;
  _22 = _12 & 7;
  _23 = (signed char) _22;
  _24 = _23 + 3;
  _25 = _24 >= _20;
  _26 = _21 & _25;
  if (_26 != 0)
    goto <bb 3>; [0.05%]
  else
    goto <bb 4>; [99.95%]

  <bb 3> [local count: 536864]:
  __builtin___asan_report_load4 (_12);

  <bb 4> [local count: 1073741824]:
  _1 = MEM[(struct B *)pc_2(D) + 1B].i;
  _14 = &pd_4(D)->i;
  _27 = (unsigned int) _14;
  _28 = _27 >> 3;
  _29 = _28 + 536870912;
  _30 = (signed char *) _29;
  _31 = *_30;
  _32 = _31 != 0;
  _33 = _27 & 7;
  _34 = (signed char) _33;
  _35 = _34 + 3;
  _36 = _35 >= _31;
  _37 = _32 & _36;
  if (_37 != 0)
    goto <bb 5>; [0.05%]
  else
    goto <bb 6>; [99.95%]

  <bb 5> [local count: 536864]:
  __builtin___asan_report_store4 (_27);

  <bb 6> [local count: 1073741824]:
  pd_4(D)->i = _1;
  _9 = (sizetype) i_6(D);
  _10 = _9 * 16;
  _11 = _10 + 4;
  psa_7 = _13 + _11;                        <<< &pc.i + (i * 16) + 4
  f (psa_7); [tail call]
  return;

}

The output below shows the difference between the last "good" IL and the "bad"
IL introduced by the sanopt transformation.  To avoid the warning either the
sanopt pass will need to be changed to avoid this substitution or to annotate
it somehow so the warning knows not to trigger.

--- pr99673.c.184t.slsr 2021-03-19 15:21:31.134229177 -0600
+++ pr99673.c.239t.sanopt       2021-03-19 15:21:31.135229188 -0600
@@ -1,13 +1,14 @@

 ;; Function g (g, funcdef_no=0, decl_uid=2314, cgraph_uid=1, symbol_order=0)

-;; 1 loops found
-;;
-;; Loop 0
-;;  header 0, latch 1
-;;  depth 0, outer -1
-;;  nodes: 0 1 2
-;; 2 succs { 1 }
+
+Symbols to be put in SSA form
+{ D.2322 }
+Incremental SSA update started at block: 0
+Number of blocks in CFG: 7
+Number of blocks to update: 6 ( 86%)
+
+
 void g (struct C * pc, struct D * pd, int i)
 {
   const short int * psa;
@@ -15,23 +16,81 @@
   sizetype _9;
   sizetype _10;
   sizetype _11;
-  struct B * _12;
+  unsigned int _12;
   int * _13;
   int * _14;
+  unsigned int _17;
+  unsigned int _18;
+  signed char * _19;
+  signed char _20;
+  _Bool _21;
+  unsigned int _22;
+  signed char _23;
+  signed char _24;
+  _Bool _25;
+  _Bool _26;
+  unsigned int _27;
+  unsigned int _28;
+  unsigned int _29;
+  signed char * _30;
+  signed char _31;
+  _Bool _32;
+  unsigned int _33;
+  signed char _34;
+  signed char _35;
+  _Bool _36;
+  _Bool _37;

   <bb 2> [local count: 1073741824]:
   _13 = &MEM[(struct B *)pc_2(D) + 1B].i;
-  .ASAN_CHECK (6, _13, 4, 4);
+  _12 = (unsigned int) _13;
+  _17 = _12 >> 3;
+  _18 = _17 + 536870912;
+  _19 = (signed char *) _18;
+  _20 = *_19;
+  _21 = _20 != 0;
+  _22 = _12 & 7;
+  _23 = (signed char) _22;
+  _24 = _23 + 3;
+  _25 = _24 >= _20;
+  _26 = _21 & _25;
+  if (_26 != 0)
+    goto <bb 4>; [0.05%]
+  else
+    goto <bb 3>; [99.95%]
+
+  <bb 4> [local count: 536864]:
+  __builtin___asan_report_load4 (_12);
+
+  <bb 3> [local count: 1073741824]:
   _1 = MEM[(struct B *)pc_2(D) + 1B].i;
   _14 = &pd_4(D)->i;
-  .ASAN_CHECK (7, _14, 4, 4);
+  _27 = (unsigned int) _14;
+  _28 = _27 >> 3;
+  _29 = _28 + 536870912;
+  _30 = (signed char *) _29;
+  _31 = *_30;
+  _32 = _31 != 0;
+  _33 = _27 & 7;
+  _34 = (signed char) _33;
+  _35 = _34 + 3;
+  _36 = _35 >= _31;
+  _37 = _32 & _36;
+  if (_37 != 0)
+    goto <bb 6>; [0.05%]
+  else
+    goto <bb 5>; [99.95%]
+
+  <bb 6> [local count: 536864]:
+  __builtin___asan_report_store4 (_27);
+
+  <bb 5> [local count: 1073741824]:
   pd_4(D)->i = _1;
   _9 = (sizetype) i_6(D);
   _10 = _9 * 16;
   _11 = _10 + 4;
-  _12 = &MEM[(struct B *)pc_2(D) + 1B];
-  psa_7 = _12 + _11;
-  f (psa_7);
+  psa_7 = _13 + _11;
+  f (psa_7); [tail call]
   return;

 }

[Bug sanitizer/99673] [11 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution

[arnd at linaro dot org]

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

--- Comment #2 from Arnd Bergmann <arnd at linaro dot org> ---
Thank you for the detailed analysis. This was the last such warning I get with
linux kernel randconfig build that I could not explain based on the earlier
discussion, so now I can submit the local workarounds and reference the bug
reports. Among the ten -Wstringop-overread warnings I got for this codebase,
around half should not have been a warning, the others are mostly harmless,
though the warning seems reasonable, while one or two seem to be actual bugs
but need to be confirmed.

Based on your explanation, is it safe to assume this can only affect the
diagnostic output and not lead to incorrect or misoptimized code being
generated?

[Bug sanitizer/99673] [11 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution

[msebor at gcc dot gnu.org]

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

--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
None of the -Wstringop-xxx warnings (either true or false positives) affects
code generation.

Thanks for the true vs false positive breakdown.  If you remember and could let
me know the details of the real bugs that'd be great.  (In addition to
reporting any false positives we don't yet know about as you have been.)

[Bug sanitizer/99673] [11 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0

[Bug sanitizer/99673] [11 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution

[arnd at linaro dot org]

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

--- Comment #4 from Arnd Bergmann <arnd at linaro dot org> ---
I posted a set of kernel patches to address all the warnings I found at

https://lore.kernel.org/lkml/20210322160253.4032422-1-arnd@kernel.org/T/#t

[Bug sanitizer/99673] [11 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
I guess sanopt could be more "careful" here but fact is the warning builds upon
wrong assumptions about the GIMPLE IL (see the examples involving CSE of
addresses of different structure).

[Bug sanitizer/99673] [11/12 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.0                        |11.2

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 11.1 has been released, retargeting bugs to GCC 11.2.

[Bug sanitizer/99673] [11/12 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.2                        |11.3

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 11.2 is being released, retargeting bugs to GCC 11.3

[Bug sanitizer/99673] [11 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11/12 Regression] bogus    |[11 Regression] bogus
                   |-Wstringop-overread warning |-Wstringop-overread warning
                   |with address sanitizer due  |with address sanitizer due
                   |to member address           |to member address
                   |substitution                |substitution
           Keywords|                            |needs-bisection
      Known to work|                            |12.0
      Known to fail|                            |11.2.1
           Priority|P3                          |P2

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
On trunk the testcase no longer reproduces the issue, it's still present on the
GCC 11 branch though.  I wonder what fixed it.

Note before .asan we have

  <bb 2> [local count: 1073741824]:
  _1 = MEM[(struct B *)pc_2(D) + 1B].i;
  pd_4(D)->i = _1;
  _9 = (sizetype) i_6(D);
  _10 = _9 * 16;
  _11 = _10 + 4;
  _12 = &MEM[(struct B *)pc_2(D) + 1B];
  psa_7 = _12 + _11;
  f (psa_7);

and after

  <bb 2> [local count: 1073741824]:
  _13 = &MEM[(struct B *)pc_2(D) + 1B].i;
  .ASAN_CHECK (6, _13, 4, 4);
  _1 = MEM[(struct B *)pc_2(D) + 1B].i;
  _14 = &pd_4(D)->i;
  .ASAN_CHECK (7, _14, 4, 4);
  pd_4(D)->i = _1;
  _9 = (sizetype) i_6(D);
  _10 = _9 * 16;
  _11 = _10 + 4;
  _12 = &MEM[(struct B *)pc_2(D) + 1B];
  psa_7 = _12 + _11;
  f (psa_7);

so I see nothing wrong.  Later FRE determines that _13 and _12 are equal
(again correct but this confuses the diagnostics).

On trunk we seem to have lowered _12 to a pointer-plus and VN fails to CSE
the addresses.  That's an optimization regression.

[Bug sanitizer/99673] [11 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:caca8eddd9356777f5fe28454eb083b1779adc26

commit r12-6793-gcaca8eddd9356777f5fe28454eb083b1779adc26
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Jan 21 13:57:12 2022 +0100

    sanitizer/99673 - bad -Wstringop-overread diagnostic with asan

    The testcase got fixed by lowering of &MEM[ptr + CST] to ptr + CST.

    2022-01-21  Richard Biener  <rguenther@suse.de>

            PR sanitizer/99673
            * gcc.dg/asan/pr99673.c: New testcase.

[Bug sanitizer/99673] [11 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|needs-bisection             |

--- Comment #10 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Martin Sebor from comment #1)
> A further reduced/simplified test case is below.  The root cause is similar
> to the one discussed in pr99578 comment 8.
> 
> $ cat pr99673.c && gcc -Os -S -Wall -fsanitize=address
> -fno-inline-functions-called-once -m32 pr99673.c

This was fixed with r12-5465-g911b633803dcbb29 and started with
r11-3305-gbaad4c48a85a354d.

[Bug sanitizer/99673] [11 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.3                        |11.4

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 11.3 is being released, retargeting bugs to GCC 11.4.

[Bug sanitizer/99673] [11/13 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11 Regression] bogus       |[11/13 Regression] bogus
                   |-Wstringop-overread warning |-Wstringop-overread warning
                   |with address sanitizer due  |with address sanitizer due
                   |to member address           |to member address
                   |substitution                |substitution
      Known to fail|                            |13.0

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
Broken again for GCC 13 by r13-137-gee1cb43bc76de8 which restores the CSE.

[Bug sanitizer/99673] [11/13/14 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.4                        |11.5

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 11.4 is being released, retargeting bugs to GCC 11.5.

[Bug sanitizer/99673] [11/13/14 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |akihiko.odaki at daynix dot com

--- Comment #14 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 114494 has been marked as a duplicate of this bug. ***