Bug 85788 - False positive of -Wstringop-overflow= warning
Summary: False positive of -Wstringop-overflow= warning
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 8.1.1
: P3 normal
Target Milestone: ---
Assignee: qinzhao
URL:
Keywords: diagnostic
Depends on:
Blocks: Wstringop-overflow
  Show dependency treegraph
 
Reported: 2018-05-15 08:04 UTC by Martin Liška
Modified: 2024-11-04 21:28 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 7.3.0, 8.1.0
Last reconfirmed: 2018-05-15 00:00:00


Attachments
original test-case (42.22 KB, text/plain)
2018-05-16 07:20 UTC, Martin Liška
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Liška 2018-05-15 08:04:26 UTC
Probably a know scenario, but I'll report that anyway. It's reduced from cuneiform package:

$ cat tc2.i
int b;
int *d = 0, *e;
void a(void *k, long l) {
  long f = __builtin_object_size(k, 0);
  __builtin___memset_chk(k, b, l, f);
}
typedef struct {
  int g;
  int h;
  char i[8000 * 8];
} j;
static int make_str_raster(j *k) {
  int *c = d;
  for (; c; c = e)
    k->g = k->h = 32767;
  
  a(k->i, k->g / 8 * k->h);
  for (; d;)
    ;
}
j m;
void n() { make_str_raster(&m); }

$ gcc tc2.i -O2
In function ‘a’,
    inlined from ‘make_str_raster.constprop’ at tc2.i:17:3,
    inlined from ‘n’ at tc2.i:22:12:
tc2.i:5:3: warning: ‘__builtin___memset_chk’ writing 134180865 bytes into a region of size 64000 overflows the destination [-Wstringop-overflow=]
   __builtin___memset_chk(k, b, l, f);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

As seen, d == 0, thus 'for (; c; c = e)' never executes. It's combination of jump-threading and VRP that triggers the warning.
Comment 1 Martin Sebor 2018-05-15 20:27:42 UTC
Confirmed at -O2 with GCC 8.1 but only at -O3 on trunk where the warning sees:

n ()
{
  int * c;
  int b.2_13;
  int b.2_16;
  int pretmp_25;
  int pretmp_26;
  int _28;
  int _31;
  long unsigned int _33;

  <bb 2> [local count: 1073741826]:
  c_3 = d;
  if (c_3 != 0B)
    goto <bb 4>; [89.00%]
  else
    goto <bb 3>; [11.00%]

  <bb 3> [local count: 1073741817]:
  pretmp_25 = m.g;
  pretmp_26 = m.h;
  _28 = pretmp_25 / 8;
  _31 = pretmp_26 * _28;
  _33 = (long unsigned int) _31;
  b.2_16 = b;
  __builtin___memset_chk (&m.i, b.2_16, _33, 64000); [tail call]
  return;

  <bb 4> [local count: 955630226]:
  c_4 = e;
  if (c_4 != 0B)
    goto <bb 6>; [89.00%]
  else
    goto <bb 5>; [11.00%]

  <bb 5> [local count: 955630226]:
  MEM[(int *)&m] = 140733193420799;
  b.2_13 = b;
  __builtin___memset_chk (&m.i, b.2_13, 134180865, 64000);
  goto <bb 7>; [100.00%]

  <bb 6> [local count: 7731917263]:
  goto <bb 6>; [100.00%]

  <bb 7> [local count: 8687547448]:
  goto <bb 7>; [100.00%]

}


In function ‘a’,
    inlined from ‘make_str_raster.constprop’ at t.c:17:3,
    inlined from ‘n’ at t.c:22:12:
t.c:5:3: warning: ‘__builtin___memset_chk’ writing 134180865 bytes into a region of size 64000 overflows the destination [-Wstringop-overflow=]
   __builtin___memset_chk(k, b, l, f);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Comment 2 Martin Liška 2018-05-16 07:20:24 UTC
Created attachment 44137 [details]
original test-case

On trunk disappeared in r259672. I'm attaching original test-case which reproduces on trunk as well.
Comment 3 qinzhao 2024-10-03 18:22:27 UTC
adding -fno-thread-jumps makes the warning disappear. 
Looks like another duplication of PR109071.
Comment 4 qinzhao 2024-10-10 16:33:32 UTC
(In reply to Martin Liška from comment #0)
> As seen, d == 0, thus 'for (; c; c = e)' never executes. It's combination of
> jump-threading and VRP that triggers the warning.

Yes, in this specific testing case, d == 0, the loop never executes. This is a very special situation, and under such situation, the reported warning is a false positive.

However, Whenever we slightly modify the testing case to make d not being 0, the warning still there, and the loop executes, Then this warning catches a REAL bug in the source code. 

So, I think that the warning is good to have in general. similar as PR109071, providing more context to the user will be beneficial in general.
Comment 5 qinzhao 2024-10-10 16:42:20 UTC
with my current patch to PR109071, adding -fdiagnostics-explain-harder,  with an additional option -fno-tree-dominator-opts, the diagnostic becomes:

t_85788.c:5:3: warning: ‘__builtin___memset_chk’ forming offset [64008, 134180872] is out of the bounds [0, 64008] of object ‘m’ with type ‘j’ [-Warray-bounds=]
    5 |   __builtin___memset_chk(k, b, l, f);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ‘n’: events 1-2
    5 |   __builtin___memset_chk(k, b, l, f);
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |   |
      |   (2) t_85788.c:20:1: warning: out of array bounds here
......
   14 |   for (; c; c = e)
      |          ^
      |          |
      |          (1) t_85788.c:20:1: warning: when the condition is evaluated to true
t_85788.c: In function ‘n’:
t_85788.c:21:3: note: ‘m’ declared here

I think that the above more details will help the user to locate the issue in their code.

I will improve my current patch to record the code duplication during "tree-dominator-opts" after "thread" in order to remove the "-fno-tree-dominator-opts"