Bug 99673 - [11/12 Regression] bogus -Wstringop-overread warning with address sanitizer due to member address substitution
Summary: [11/12 Regression] bogus -Wstringop-overread warning with address sanitizer d...
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: sanitizer (show other bugs)
Version: 11.0
: P3 normal
Target Milestone: 11.3
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks: Wstringop-overread
  Show dependency treegraph
 
Reported: 2021-03-19 16:59 UTC by Arnd Bergmann
Modified: 2021-07-28 07:06 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 11.0
Last reconfirmed: 2021-03-19 00:00:00


Attachments
manually reduced test case (641 bytes, text/plain)
2021-03-19 16:59 UTC, Arnd Bergmann
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Arnd Bergmann 2021-03-19 16:59:10 UTC
Created attachment 50435 [details]
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.
Comment 1 Martin Sebor 2021-03-19 21:44:07 UTC
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;
 
 }
Comment 2 Arnd Bergmann 2021-03-20 12:22:00 UTC
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?
Comment 3 Martin Sebor 2021-03-21 19:06:13 UTC
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.)
Comment 4 Arnd Bergmann 2021-03-22 16:07:39 UTC
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
Comment 5 Richard Biener 2021-04-08 13:09:16 UTC
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).
Comment 6 Jakub Jelinek 2021-04-27 11:40:37 UTC
GCC 11.1 has been released, retargeting bugs to GCC 11.2.
Comment 7 Richard Biener 2021-07-28 07:06:11 UTC
GCC 11.2 is being released, retargeting bugs to GCC 11.3