Bug 93200 - [10 Regression] spurious -Wstringop-overflow due to assignment vectorization to multiple members
Summary: [10 Regression] spurious -Wstringop-overflow due to assignment vectorization ...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 10.0
: P3 normal
Target Milestone: 10.0
Assignee: Martin Sebor
URL:
Keywords: diagnostic, patch
Depends on:
Blocks: Wstringop-overflow
  Show dependency treegraph
 
Reported: 2020-01-08 11:32 UTC by Martin Sebor
Modified: 2020-09-07 23:15 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-01-08 00:00:00


Attachments
Translation unit to reproduce the warning. (19.91 KB, text/plain)
2020-01-08 11:37 UTC, Martin Sebor
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2020-01-08 11:32:14 UTC
This bug tracks the language-independent part of the Fortran-specific bug 92956 based on the test case in bug 92956 comment #10.  The transformation of assignments to multiple consecutive members into a single "vectorized" MEM_REF assignment starting at the address of the first member can cause spurious -Wstringop-overflow warnings, depending on the object layout.  This has been reported to cause false positives in builds of cjdns-v20.4.

The small test case below reproduces the problem:

$ cat t.c && gcc -O3 -S -Wall -fdump-tree-strlen=/dev/stdout t.c 
struct A { char b, c; };
struct B { char i; struct A a; };

void f (struct B *p)
{
  p->a.b = 1;   // no warning here
  p->a.c = 2;
}

struct C { int i; struct A a; };

void g (struct C *p)
{
  p->a.b = 1;   // spurious -Wstringop-overflow
  p->a.c = 2;
}

;; Function f (f, funcdef_no=0, decl_uid=1936, 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 }
f (struct B * p)
{
  <bb 2> [local count: 1073741824]:
  p_2(D)->a.b = 1;
  p_2(D)->a.c = 2;
  return;

}



;; Function g (g, funcdef_no=1, decl_uid=1942, cgraph_uid=2, symbol_order=1)

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2
;; 2 succs { 1 }
t.c: In function ‘g’:
t.c:14:10: warning: writing 2 bytes into a region of size 1 [-Wstringop-overflow=]
   14 |   p->a.b = 1;   // spurious -Wstringop-overflow
      |   ~~~~~~~^~~
t.c:1:17: note: at offset 0 to object ‘b’ with size 1 declared here
    1 | struct A { char b, c; };
      |                 ^
g (struct C * p)
{
  vector(2) char * vectp.8;
  vector(2) char * vectp_p.7;

  <bb 2> [local count: 1073741824]:
  vectp.8_6 = &p_2(D)->a.b;
  MEM <vector(2) char> [(char *)vectp.8_6] = { 1, 2 };
  return;

}
Comment 1 Martin Sebor 2020-01-08 11:37:50 UTC
Created attachment 47612 [details]
Translation unit to reproduce the warning.

Attached is a cjdns-v20.4 translation unit that reproduces the warning with -O3 -Wall. 

For the record, here's what I replied to Jeff's report of the cjdns-v20.4 warning.
 
The code that triggers the warning is these two assignments to two consecutive members of type unsigned char:

    header->unused = 0;
    header->flags = 1;

GCC turns them into this single vector assignment to the first of them, overwriting both:

  vectp.336_252 = &header_139->flags;
  MEM <vector(2) unsigned char> [(unsigned char *)vectp.336_252] = { 1, 0 };

The warning is working correctly, it's just not prepared for GCC to do the sorts of transformations that are indistinguishable from invalid C code it is designed to warn for.

I think this is also behind the Fortran PR 92956 (see the test case in comment 10).

A possible workaround, if this is causing lots of false positives, is to detect this vectorization pattern and suppress the warning for it.  It seems unlikely that it would the result of an accidental bug in the source code.  Then again, bugs are often in tricky code that tries to do something "clever" and gets it wrong.

Longer term, I think the robust solution is to distinguish these GCC transformations from user code.  It could be done by simply setting some otherwise unused bit on the MEM_REF tree node used to represent them (this is my comment #13 on PR 92956).  But that's probably GCC 11 material.
Comment 2 Martin Sebor 2020-01-08 16:26:20 UTC
Testing a temporary workaround.
Comment 3 Martin Sebor 2020-01-08 17:23:50 UTC
Patch: https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00381.html
Comment 4 Richard Biener 2020-01-09 11:21:20 UTC
(In reply to Martin Sebor from comment #1)
> Created attachment 47612 [details]
> Translation unit to reproduce the warning.
> 
> Attached is a cjdns-v20.4 translation unit that reproduces the warning with
> -O3 -Wall. 
> 
> For the record, here's what I replied to Jeff's report of the cjdns-v20.4
> warning.
>  
> The code that triggers the warning is these two assignments to two
> consecutive members of type unsigned char:
> 
>     header->unused = 0;
>     header->flags = 1;
> 
> GCC turns them into this single vector assignment to the first of them,
> overwriting both:
> 
>   vectp.336_252 = &header_139->flags;
>   MEM <vector(2) unsigned char> [(unsigned char *)vectp.336_252] = { 1, 0 };
> 
> The warning is working correctly, it's just not prepared for GCC to do the
> sorts of transformations that are indistinguishable from invalid C code it
> is designed to warn for.
> 
> I think this is also behind the Fortran PR 92956 (see the test case in
> comment 10).
> 
> A possible workaround, if this is causing lots of false positives, is to
> detect this vectorization pattern and suppress the warning for it.  It seems
> unlikely that it would the result of an accidental bug in the source code. 
> Then again, bugs are often in tricky code that tries to do something
> "clever" and gets it wrong.
> 
> Longer term, I think the robust solution is to distinguish these GCC
> transformations from user code.  It could be done by simply setting some
> otherwise unused bit on the MEM_REF tree node used to represent them (this
> is my comment #13 on PR 92956).  But that's probably GCC 11 material.

Another solution is to make the vectorizer not use &header_139->flags
but instead &MEM[header_139 + offsetof(flags)].
Comment 5 Martin Sebor 2020-01-09 12:00:21 UTC
Author: msebor
Date: Thu Jan  9 11:59:41 2020
New Revision: 280041

URL: https://gcc.gnu.org/viewcvs?rev=280041&root=gcc&view=rev
Log:
PR middle-end/93200 - spurious -Wstringop-overflow due to assignment vectorization to multiple members
PR fortran/92956 - 'libgomp.fortran/examples-4/async_target-2.f90' fails with offloading due to bogus -Wstringop-overflow warning

gcc/testsuite/ChangeLog:

	PR middle-end/93200
	* gcc.dg/Wstringop-overflow-30.c: New test.

gcc/ChangeLog:

	PR middle-end/93200
	PR fortran/92956
	* builtins.c (compute_objsize): Avoid handling MEM_REFs of vector type.


Added:
    trunk/gcc/testsuite/gcc.dg/Wstringop-overflow-30.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/builtins.c
    trunk/gcc/testsuite/ChangeLog
Comment 6 Martin Sebor 2020-01-09 12:01:52 UTC
The warning has been temporarily suppressed for these cases, until a longer term solution is put in place.