Bug 95353 - [10 Regression] spurious -Wstringop-overflow writing to a trailing array plus offset
Summary: [10 Regression] spurious -Wstringop-overflow writing to a trailing array plus...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 10.1.1
: P2 normal
Target Milestone: 10.3
Assignee: Martin Sebor
URL:
Keywords: diagnostic, patch
: 95490 95533 96406 98561 (view as bug list)
Depends on:
Blocks: Wstringop-overflow
  Show dependency treegraph
 
Reported: 2020-05-27 02:03 UTC by H.J. Lu
Modified: 2021-01-21 22:51 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work: 11.0
Known to fail: 10.1.0
Last reconfirmed: 2020-05-27 00:00:00


Attachments
a testcase (103.51 KB, application/octet-stream)
2020-05-27 03:15 UTC, H.J. Lu
Details

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2020-05-27 02:03:54 UTC
Assembler in binutils has

struct frag {
  ...
  /* Data begins here.  */
  char fr_literal[1];
};

and fr_literal is accessed as

char *buf = fragp->fr_fix + fragp->fr_literal;

GCC 10 gave

gas/config/tc-csky.c: In function ‘md_convert_frag’:
gas/config/tc-csky.c:4507:9: error: writing 1 byte into a region of size 0 [-Wer
ror=stringop-overflow=]
 4507 |  buf[1] = BYTE_1 (CSKYV1_INST_SUBI | (7 << 4));
      |         ^

I checked in this:

char *buf = fragp->fr_fix + &fragp->fr_literal[0];

as a workaround.  But it doesn't solve the problem in existing binutils
sources and some future version of gcc might see through the obfuscation
of the source, rendering this work-around ineffective.  is there a solution
which is future proof?
Comment 1 Andrew Pinski 2020-05-27 02:21:55 UTC
Can you provide the full preprocessed source?
Comment 2 H.J. Lu 2020-05-27 03:15:26 UTC
Created attachment 48612 [details]
a testcase

$ gcc -O2 x.i -S
/export/gnu/import/git/sources/binutils-gdb-release/gas/config/tc-csky.c: In function ‘md_convert_frag’:
/export/gnu/import/git/sources/binutils-gdb-release/gas/config/tc-csky.c:4507:9: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
 4507 |  buf[1] = BYTE_1 (CSKYV1_INST_SUBI | (7 << 4));
      |  ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                          
/export/gnu/import/git/sources/binutils-gdb-release/gas/config/tc-csky.c:4508:9: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
 4508 |  buf[2] = BYTE_0 (CSKYV1_INST_STW  | (15 << 8));    /* stw r15, r0.  */
      |  ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                    
/export/gnu/import/git/sources/binutils-gdb-release/gas/config/tc-csky.c:4509:9: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
 4509 |  buf[3] = BYTE_1 (CSKYV1_INST_STW  | (15 << 8));
      |  ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                     ...
Comment 3 Marc Glisse 2020-05-27 06:42:26 UTC
Do you need fr_literal to have size at least 1 (say, when creating an object on the stack), or can you use the official flexible array member (drop the 1, just [] in the declaration)?
Comment 4 Richard Biener 2020-05-27 08:07:27 UTC
Another case of bogus warning...
Comment 5 Martin Sebor 2020-05-27 15:15:54 UTC
The warning is due to a limitation of the compute_objsize() function.  A small "supported" test case (one that doesn't depend on a trailing array of non-zero size being treated as a flexible array member) that I think reproduces the Binutils warning is below.  In this case the function doesn't work hard enough to determine that the pointer points to a trailing array member and instead uses the the array's actual size.  It needs to be improved or preferably rewritten as discussed in pr94335 comment 7.

As suggested, using a flexible array member instead of the one-element (or zero-length) array avoids the warning.

$ cat z.c && gcc -O2 -S -Wall -fdump-tree-strlen=/dev/stdout z.c
struct S {
  char n, a[0];
};


void f (struct S *p)
{
  char *q = p->a;
  q[1] = 1;    // no warning
}

void g (struct S *p, int i)
{
  char *q = p->a + i;
  q[1] = 1;    // spurious -Wstringop-overflow
}

;; Function f (f, funcdef_no=0, decl_uid=1933, 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 S * p)
{
  <bb 2> [local count: 1073741824]:
  MEM[(char *)p_1(D) + 2B] = 1;
  return;

}



;; Function g (g, funcdef_no=1, decl_uid=1938, 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 }
z.c: In function ‘g’:
z.c:15:8: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
   15 |   q[1] = 1;    // spurious -Wstringop-overflow
      |   ~~~~~^~~
g (struct S * p, int i)
{
  char * q;
  char[0:] * _1;
  sizetype _2;

  <bb 2> [local count: 1073741824]:
  _1 = &p_3(D)->a;             <<< doesn't consider that a is a trailing array
  _2 = (sizetype) i_4(D);
  q_5 = _1 + _2;              
  MEM[(char *)q_5 + 1B] = 1;   <<< warning here
  return;

}
Comment 6 Alan Modra 2020-05-28 04:41:48 UTC
binutils code was written originally in K&R C, with quite a lot of the code base still having K&R flavour.  We no doubt could move to C99 to use flexible array members and other nice features.  For now, binutils is a useful "old" code base to test new gcc warnings..
Comment 7 Martin Sebor 2020-05-28 15:21:34 UTC
I test my warning changes with binutils and --enable-targets=all.  But that apparently doesn't compile all source files, and I don't have a cross-build setup in place (or the resources to do it).  If someone who already does cross-builds could periodically post results to gcc-testresults (or report bugs), or if we could get these going in Jeff's buildbot, that would help catch the problems in those before either package is released.
Comment 8 Martin Sebor 2020-05-28 20:10:14 UTC
Adjust Summary to more accurately reflect the problem.
Comment 10 Martin Sebor 2020-06-03 16:10:49 UTC
*** Bug 95490 has been marked as a duplicate of this bug. ***
Comment 11 Martin Sebor 2020-06-04 16:18:40 UTC
*** Bug 95533 has been marked as a duplicate of this bug. ***
Comment 12 GCC Commits 2020-06-10 18:02:25 UTC
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:

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

commit r11-1183-ga2c2cee92e5defff9bf23d3b1184ee96e57e5fdd
Author: Martin Sebor <msebor@redhat.com>
Date:   Wed Jun 10 12:00:08 2020 -0600

    PR middle-end/95353 - spurious -Wstringop-overflow writing to a trailing array plus offset
    
    Also resolves:
    PR middle-end/92939 - missing -Wstringop-overflow on negative index from the end of array
    
    gcc/ChangeLog:
    
            PR middle-end/95353
            PR middle-end/92939
            * builtins.c (inform_access): New function.
            (check_access): Call it.  Add argument.
            (addr_decl_size): Remove.
            (get_range): New function.
            (compute_objsize): New overload.  Only use compute_builtin_object_size
            with raw memory function.
            (check_memop_access): Pass new argument to compute_objsize and
            check_access.
            (expand_builtin_memchr, expand_builtin_strcat): Same.
            (expand_builtin_strcpy, expand_builtin_stpcpy_1): Same.
            (expand_builtin_stpncpy, check_strncat_sizes): Same.
            (expand_builtin_strncat, expand_builtin_strncpy): Same.
            (expand_builtin_memcmp): Same.
            * builtins.h (check_nul_terminated_array): Declare extern.
            (check_access): Add argument.
            (struct access_ref, struct access_data): New structs.
            * gimple-ssa-warn-restrict.c (clamp_offset): New helper.
            (builtin_access::overlap): Call it.
            * tree-object-size.c (decl_init_size): Declare extern.
            (addr_object_size): Correct offset computation.
            * tree-object-size.h (decl_init_size): Declare.
            * tree-ssa-strlen.c (handle_integral_assign): Remove a call
            to maybe_warn_overflow when assigning to an SSA_NAME.
    
    gcc/testsuite/ChangeLog:
    
            PR middle-end/95353
            PR middle-end/92939
            * c-c++-common/Wstringop-truncation.c: Remove an xfail.
            * gcc.dg/Warray-bounds-46.c: Remove a bogus warning.
            * gcc.dg/Wrestrict-9.c: Disable -Wstringop-overflow.
            * gcc.dg/Wstringop-overflow-12.c: Remove xfails.
            * gcc.dg/Wstringop-overflow-28.c: Same.
            * gcc.dg/builtin-stringop-chk-4.c: Same.
            * gcc.dg/builtin-stringop-chk-5.c: Same.
            * gcc.dg/builtin-stringop-chk-8.c: Same.
            * gcc.dg/strlenopt-74.c: Avoid buffer overflow.
            * gcc.dg/Wstringop-overflow-34.c: New test.
            * gcc.dg/Wstringop-overflow-35.c: New test.
            * gcc.dg/Wstringop-overflow-36.c: New test.
            * gcc.dg/Wstringop-overflow-37.c: New test.
            * gcc.dg/Wstringop-overflow-38.c: New test.
Comment 13 Martin Sebor 2020-06-10 18:03:12 UTC
Fixed on trunk.
Comment 14 GCC Commits 2020-06-13 17:32:01 UTC
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:

https://gcc.gnu.org/g:678178cbf3642f272459b2be675cc518b3121d09

commit r11-1292-g678178cbf3642f272459b2be675cc518b3121d09
Author: Martin Sebor <msebor@redhat.com>
Date:   Sat Jun 13 11:29:01 2020 -0600

    Correct test to avoid failures in ILP32.
    
    Amends: PR middle-end/95353 - spurious -Wstringop-overflow writing to a trailing array plus offset.
    
    gcc/testsuite/ChangeLog:
            * gcc.dg/builtin-stringop-chk-5.c: Make assertions independent of
            data model.
Comment 15 Richard Biener 2020-07-23 06:51:56 UTC
GCC 10.2 is released, adjusting target milestone.
Comment 16 Martin Sebor 2020-08-03 14:41:50 UTC
*** Bug 96406 has been marked as a duplicate of this bug. ***
Comment 17 Martin Sebor 2021-01-06 16:37:29 UTC
*** Bug 98561 has been marked as a duplicate of this bug. ***
Comment 18 Martin Sebor 2021-01-21 22:51:06 UTC
Fixed for GCC 11.  The patch seems too intrusive to me to backport.