[Bug c++/105726] New: spurious warning with -Warray-bounds

[Bug c++/105726] New: spurious warning with -Warray-bounds

[cuzdav at gmail dot com]

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

            Bug ID: 105726
           Summary: spurious warning with -Warray-bounds
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: cuzdav at gmail dot com
  Target Milestone: ---

Created attachment 53030
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53030&action=edit
preprocessed version

I just ran into an issue that seems to have appeared in Gcc 10.1 and is still
there in 11, 12, and in the trunk.

The following code produces a spurious warning about reading out of bounds,
but if I remove the "pad" field, or change from std::array to a "raw" array,
the warning goes away.  

//===============================
#include <array>
#include <cstring>

struct X {
    char pad[4];
    std::array<char, 1> mField;
};

void encode(char* aBuffer, const X& aMessage) {
    strncpy(aBuffer, aMessage.mField.data(), 1);
}
//===============================

LIVE: https://godbolt.org/z/zEsx5P8eP

<source>: In function 'void encode(char*, const X&)':
<source>:10:16: warning: 'char* strncpy(char*, const char*, size_t)' offset 4
from the object at 'aMessage' is out of the bounds of referenced subobject
'std::array<char, 1>::_M_elems' with type 'char [1]' at offset 4
[-Warray-bounds]
   10 |         strncpy(aBuffer, aMessage.mField.data(), 1);
      |         ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from <source>:1:
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/array:115:56: note:
subobject 'std::array<char, 1>::_M_elems' declared here
  115 |       typename _AT_Type::_Type                         _M_elems;
      |                                                        ^~~~~~~~
Compiler returned: 0


OUTPUT of g++ -v

Using built-in specs.
COLLECT_GCC=/opt/imc/gcc-12.1.0/bin/g++
COLLECT_LTO_WRAPPER=/opt/imc/gcc-12.1.0/libexec/gcc/x86_64-pc-linux-gnu/12.1.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../gcc-12.1.0/configure --prefix=/opt/imc/gcc-12.1.0
--enable-languages=c,c++,fortran,lto --disable-multilib
--with-build-time-tools=/build/INSTALLDIR//opt/imc/gcc-12.1.0/bin
--enable-libstdcxx-time=rt
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 12.1.0 (GCC)

[Bug c++/105726] spurious warning with -Warray-bounds

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
             Blocks|                            |56456
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2022-05-25
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.  At -O1 emitted by the warn_restrict pass on

<bb 2> [local count: 1073741824]:
_3 = &MEM[(const struct array *)aMessage_1(D) + 4B]._M_elems;
strncpy (aBuffer_4(D), _3, 1);
return;

 <addr_expr 0x7ffff5d83c00
    type <pointer_type 0x7ffff5d86e70
        type <array_type 0x7ffff5d86dc8 _Type type <integer_type 0x7ffff6544540
char>
            type_6 QI
            size <integer_cst 0x7ffff653a048 constant 8>
            unit-size <integer_cst 0x7ffff653a060 constant 1>
            align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7ffff5d86d20 domain <integer_type 0x7ffff6542c78>
            pointer_to_this <pointer_type 0x7ffff5d86e70>>
        unsigned DI
        size <integer_cst 0x7ffff6517f48 constant 64>
        unit-size <integer_cst 0x7ffff6517f60 constant 8>
        align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7ffff5d86f18>
    readonly
    arg:0 <component_ref 0x7ffff5d50bd0
        type <array_type 0x7ffff5d862a0 type <integer_type 0x7ffff65383f0 char>
            readonly type_6 QI size <integer_cst 0x7ffff653a048 8> unit-size
<integer_cst 0x7ffff653a060 1>
            align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7ffff5d862a0 domain <integer_type 0x7ffff6542c78>>
        readonly
        arg:0 <mem_ref 0x7ffff5d891e0 type <record_type 0x7ffff5d7f888 array>

            arg:0 <ssa_name 0x7ffff5cf84c8 type <reference_type 0x7ffff5d86888>
                visited var <parm_decl 0x7ffff5d85400 aMessage>
                def_stmt GIMPLE_NOP
                version:1
                ptr-info 0x7ffff5d7b8e8>
            arg:1 <integer_cst 0x7ffff5d7b7c8 constant 4>>
        arg:1 <field_decl 0x7ffff5d7c720 _M_elems type <array_type
0x7ffff5d7ddc8 _Type>

I think the pass mis-analyzes the reference.  Let me have a look.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56456
[Bug 56456] [meta-bug] bogus/missing -Warray-bounds

[Bug c++/105726] spurious warning with -Warray-bounds

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |msebor at gcc dot gnu.org

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Oh, my - this is mightly complicated code.  We get to

void
builtin_memref::set_base_and_offset (tree expr)
{
...
  if (TREE_CODE (base) == MEM_REF)
    {
      tree memrefoff = fold_convert (ptrdiff_type_node, TREE_OPERAND (base,
1));
      extend_offset_range (memrefoff);
      base = TREE_OPERAND (base, 0);

      if (refoff != HOST_WIDE_INT_MIN
          && TREE_CODE (expr) == COMPONENT_REF)
        {
          /* Bump up the offset of the referenced subobject to reflect
             the offset to the enclosing object.  For example, so that
             in
               struct S { char a, b[3]; } s[2];
               strcpy (s[1].b, "1234");
             REFOFF is set to s[1].b - (char*)s.  */
          offset_int off = tree_to_shwi (memrefoff);
          refoff += off;
        }

which looks somewhat suspicious and then

tree 
builtin_memref::offset_out_of_bounds (int strict, offset_int ooboff[3]) const
{
...
      /* When the referenced subobject is known, the end offset must be
         within its bounds.  Otherwise there is nothing to do.  */
      if (strict
          && !decl_p
          && ref
          && refsize >= 0
          && TREE_CODE (ref) == COMPONENT_REF)
        {
          /* If REFOFF is negative, SIZE will become negative here.  */
          size = refoff + refsize;
          obj = ref;

triggers with ref being a COMPONENT_REF to the array, refoff being 4 (from the
MEM_REF) and refsize == 0.

The idea is probably that the '4' is for the pointer adjustment (base
is &aMessage here), but then the object shouldn't be 'ref'?

But I'm confused by all the complexity and the abstraction and I'm sure
starting to deleting code that confuses me will cause all sorts of
regressions in the diagnostic parts of the testsuite.

Martin - can you have a look here and maybe give directions?

In particular how 'refoff' and 'offrange' depend (or if they stand on its
own).  I think that &ref - &base == refoff?

So for &MEM[(const struct array *)aMessage_1(D) + 4B]._M_elems
having ref = MEM[(const struct array *)aMessage_1(D) + 4B]._M_elems,
base = aMessage_1(D), refoff = 4 and offrange[] = {4, 4} looks OK.
But then I'm confused as to why the diagnostic triggers.  Maybe that's
because refsize == 0 which is a special case for "unknown"?  That's
because ref analysis does

      if (!integer_zerop (memrefoff))
        /* A non-zero offset into an array of struct with flexible array
           members implies that the array is empty because there is no
           way to initialize such a member when it belongs to an array.
           This must be some sort of a bug.  */
        refsize = 0;

on the MEM_REF path.  But I don't understand how the comment applies
here - there is no flexibel array member involved, in particular
the MEM_REF doesn't offset an array it offsets struct X.  So I'm
inclined to simply kill the above ...?

[Bug c++/105726] spurious warning with -Warray-bounds

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
The following fixes the bogus diagnostic:

diff --git a/gcc/gimple-ssa-warn-restrict.cc b/gcc/gimple-ssa-warn-restrict.cc
index b678e806da3..25c63f99c61 100644
--- a/gcc/gimple-ssa-warn-restrict.cc
+++ b/gcc/gimple-ssa-warn-restrict.cc
@@ -539,13 +559,6 @@ builtin_memref::set_base_and_offset (tree expr)
          offset_int off = tree_to_shwi (memrefoff);
          refoff += off;
        }
-
-      if (!integer_zerop (memrefoff))
-       /* A non-zero offset into an array of struct with flexible array
-          members implies that the array is empty because there is no
-          way to initialize such a member when it belongs to an array.
-          This must be some sort of a bug.  */
-       refsize = 0;
     }

   if (TREE_CODE (ref) == COMPONENT_REF)

[Bug c++/105726] spurious warning with -Warray-bounds

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #3)
> The following fixes the bogus diagnostic:
> 
> diff --git a/gcc/gimple-ssa-warn-restrict.cc
> b/gcc/gimple-ssa-warn-restrict.cc
> index b678e806da3..25c63f99c61 100644
> --- a/gcc/gimple-ssa-warn-restrict.cc
> +++ b/gcc/gimple-ssa-warn-restrict.cc
> @@ -539,13 +559,6 @@ builtin_memref::set_base_and_offset (tree expr)
>           offset_int off = tree_to_shwi (memrefoff);
>           refoff += off;
>         }
> -
> -      if (!integer_zerop (memrefoff))
> -       /* A non-zero offset into an array of struct with flexible array
> -          members implies that the array is empty because there is no
> -          way to initialize such a member when it belongs to an array.
> -          This must be some sort of a bug.  */
> -       refsize = 0;
>      }
>  
>    if (TREE_CODE (ref) == COMPONENT_REF)

It for example regresses gcc.dg/Warray-bounds-46.c which essentially does

struct X { char pad[4]; char ax[]; };

void foo (struct X *p, char *a)
{
  __builtin_strcpy (p[1].ax, a);
}

and the intent was to detect that p[1] implies a zero-size 'ax'.  That's
going to be interesting to distinguish from our case here with just
the MEM_REF - possibly the easiest heuristic is to trigger this only
for incomplete TREE_TYPE of the MEM_REF at least.  That will still
trigger in too many cases, requiring the MEM_REF offset to be a multiple
of the size of the struct will trigger in not enough cases (if there's
also an embedded offset into the array).  But maybe it's a good compromise:

diff --git a/gcc/gimple-ssa-warn-restrict.cc b/gcc/gimple-ssa-warn-restrict.cc
index b678e806da3..734cdd7f5b4 100644
--- a/gcc/gimple-ssa-warn-restrict.cc
+++ b/gcc/gimple-ssa-warn-restrict.cc
@@ -525,7 +525,6 @@ builtin_memref::set_base_and_offset (tree expr)
     {
       tree memrefoff = fold_convert (ptrdiff_type_node, TREE_OPERAND (base,
1));
       extend_offset_range (memrefoff);
-      base = TREE_OPERAND (base, 0);

       if (refoff != HOST_WIDE_INT_MIN
          && TREE_CODE (expr) == COMPONENT_REF)
@@ -538,14 +537,19 @@ builtin_memref::set_base_and_offset (tree expr)
             REFOFF is set to s[1].b - (char*)s.  */
          offset_int off = tree_to_shwi (memrefoff);
          refoff += off;
-       }
-
-      if (!integer_zerop (memrefoff))
-       /* A non-zero offset into an array of struct with flexible array
-          members implies that the array is empty because there is no
-          way to initialize such a member when it belongs to an array.
-          This must be some sort of a bug.  */
-       refsize = 0;
+
+         if (!integer_zerop (memrefoff)
+             && !COMPLETE_TYPE_P (TREE_TYPE (expr))
+             && multiple_of_p (sizetype, memrefoff,
+                               TYPE_SIZE_UNIT (TREE_TYPE (base)), true))
+           /* A non-zero offset into an array of struct with flexible array
+              members implies that the array is empty because there is no
+              way to initialize such a member when it belongs to an array.
+              This must be some sort of a bug.  */
+           refsize = 0;
+       }
+
+      base = TREE_OPERAND (base, 0);
     }

   if (TREE_CODE (ref) == COMPONENT_REF)

[Bug tree-optimization/105726] [10/11/12/13 Regression] spurious warning with -Warray-bounds

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|spurious warning with       |[10/11/12/13 Regression]
                   |-Warray-bounds              |spurious warning with
                   |                            |-Warray-bounds
      Known to fail|                            |10.3.1, 11.3.0, 12.1.0
   Target Milestone|---                         |10.4
           Priority|P3                          |P2
          Component|c++                         |tree-optimization
      Known to work|                            |9.4.1

[Bug tree-optimization/105726] [10/11/12/13 Regression] spurious warning with -Warray-bounds

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 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:e7c482b08076bb299742883c4ffd65b31e33200c

commit r13-796-ge7c482b08076bb299742883c4ffd65b31e33200c
Author: Richard Biener <rguenther@suse.de>
Date:   Wed May 25 11:49:03 2022 +0200

    tree-optimization/105726 - adjust array bound heuristic

    There's heuristic to detect ptr[1].a[...] out of bound accesses
    reasoning that if ptr points to an array of aggregates a trailing
    incomplete array has to have size zero.  The following more
    thoroughly constrains the cases this applies to avoid false
    positive diagnostics.

    2022-05-25  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/105726
            * gimple-ssa-warn-restrict.cc
(builtin_memref::set_base_and_offset):
            Constrain array-of-flexarray case more.

            * g++.dg/warn/Warray-bounds-27.C: New testcase.

[Bug tree-optimization/105726] [10/11/12 Regression] spurious warning with -Warray-bounds

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |13.0
            Summary|[10/11/12/13 Regression]    |[10/11/12 Regression]
                   |spurious warning with       |spurious warning with
                   |-Warray-bounds              |-Warray-bounds

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk sofar.

[Bug tree-optimization/105726] [10/11/12 Regression] spurious warning with -Warray-bounds

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:9bc27ee76d510360637c9578ffdefeab5c336e86

commit r12-8452-g9bc27ee76d510360637c9578ffdefeab5c336e86
Author: Richard Biener <rguenther@suse.de>
Date:   Wed May 25 11:49:03 2022 +0200

    tree-optimization/105726 - adjust array bound heuristic

    There's heuristic to detect ptr[1].a[...] out of bound accesses
    reasoning that if ptr points to an array of aggregates a trailing
    incomplete array has to have size zero.  The following more
    thoroughly constrains the cases this applies to avoid false
    positive diagnostics.

    2022-05-25  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/105726
            * gimple-ssa-warn-restrict.cc
(builtin_memref::set_base_and_offset):
            Constrain array-of-flexarray case more.

            * g++.dg/warn/Warray-bounds-27.C: New testcase.

    (cherry picked from commit e7c482b08076bb299742883c4ffd65b31e33200c)

[Bug tree-optimization/105726] [10/11 Regression] spurious warning with -Warray-bounds

[cvs-commit at gcc dot gnu.org]

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

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

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

commit r11-10069-gd76c15430f74024f1658be55cfcd2ed9f581f1bd
Author: Richard Biener <rguenther@suse.de>
Date:   Wed May 25 11:49:03 2022 +0200

    tree-optimization/105726 - adjust array bound heuristic

    There's heuristic to detect ptr[1].a[...] out of bound accesses
    reasoning that if ptr points to an array of aggregates a trailing
    incomplete array has to have size zero.  The following more
    thoroughly constrains the cases this applies to avoid false
    positive diagnostics.

    2022-05-25  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/105726
            * gimple-ssa-warn-restrict.c (builtin_memref::set_base_and_offset):
            Constrain array-of-flexarray case more.

            * g++.dg/warn/Warray-bounds-27.C: New testcase.

    (cherry picked from commit e7c482b08076bb299742883c4ffd65b31e33200c)

[Bug tree-optimization/105726] [10/11 Regression] spurious warning with -Warray-bounds

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:55ea76600843498048158ec08a661fe7f0c7bcb0

commit r10-10844-g55ea76600843498048158ec08a661fe7f0c7bcb0
Author: Richard Biener <rguenther@suse.de>
Date:   Wed May 25 11:49:03 2022 +0200

    tree-optimization/105726 - adjust array bound heuristic

    There's heuristic to detect ptr[1].a[...] out of bound accesses
    reasoning that if ptr points to an array of aggregates a trailing
    incomplete array has to have size zero.  The following more
    thoroughly constrains the cases this applies to avoid false
    positive diagnostics.

    2022-05-25  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/105726
            * gimple-ssa-warn-restrict.c (builtin_memref::set_base_and_offset):
            Constrain array-of-flexarray case more.

            * g++.dg/warn/Warray-bounds-27.C: New testcase.

    (cherry picked from commit e7c482b08076bb299742883c4ffd65b31e33200c)

[Bug tree-optimization/105726] [10/11 Regression] spurious warning with -Warray-bounds

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |10.3.1, 11.3.1
      Known to fail|10.3.1                      |10.3.0
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.