[Bug tree-optimization/43270] New: array-bounds false negative

[Bug tree-optimization/43270] New: array-bounds false negative

[matt at use dot net]

Compiling the attached code with -O3 -Wall or -O2 -Wall doesn't elicit an array
bounds warning, when it should in the constructor for FixedString.


-- 
           Summary: array-bounds false negative
           Product: gcc
           Version: 4.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: matt at use dot net
 GCC build triplet: x86_64-unknown-linux-gnu
  GCC host triplet: x86_64-unknown-linux-gnu
GCC target triplet: x86_64-unknown-linux-gnu


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[matt at use dot net]

------- Comment #1 from matt at use dot net  2010-03-05 20:22 -------
Created an attachment (id=20031)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20031&action=view)
compilation unit that reproduces the bug


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[matt at use dot net]

------- Comment #2 from matt at use dot net  2010-03-05 20:33 -------
This occurs with both gcc 4.4.1 and 4.5.0.20100304.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[pinskia at gcc dot gnu dot org]

------- Comment #3 from pinskia at gcc dot gnu dot org  2010-03-05 20:34 -------
Well this is semi on purpose.  Though we are should reject the zero sized
arrays anyways.

The problem is here we have:
struct f
{
  char a[0];
};

Which is common in GNU C/C++ to say f::a is a flexible array member.  Also we
don't warn about the last array in a struct for that reason.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[matt at use dot net]

------- Comment #4 from matt at use dot net  2010-03-05 22:17 -------
It's not the fact that it's zero-sized in and of itself, but rather the
assignment to contents[0] in the ctor should trigger the warning. Oddly,
PC-Lint warns of the zero-sized array, but not the actual overflow.

As a test, I tried changing the ctor assignment to contents[1], and the warning
still isn't triggered in either GCC or PC-Lint. 

Whatever this blind spot is, it's in both tools.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[pinskia at gcc dot gnu dot org]

------- Comment #5 from pinskia at gcc dot gnu dot org  2010-03-05 22:22 -------
Well:
struct f
{
  int t[0];
};

is invalid c/C++ :). But we accept it as an extension.  Anyways the thing when
you do:
f *t = ..;
t->t[1] = 1;

We don't warn there on purpose as f::t might used as flexible array.  We do the
same thing for any  array that ends the struct, it does not matter.  This is
very common in C and C++ code so turning this warning on for this one false
negative case is going to be hard not to get that many false positive warnings.

Sorry.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |WONTFIX


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[matt at use dot net]

------- Comment #6 from matt at use dot net  2010-03-05 23:24 -------
I see your point about supporting existing code that uses this feature in the
way you describe.

I modified the example to not rely upon zero-length array and have attached it.
(The bug in the original code didn't use it that way either, I was just trying
to make the reproducuble test case simpler.) GCC 4.4.1 and 4.5.0.20100304 still
do not warn about the array-bounds issue. PC-Lint now does warn about it.


-- 

matt at use dot net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |UNCONFIRMED
         Resolution|WONTFIX                     |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[matt at use dot net]

------- Comment #7 from matt at use dot net  2010-03-05 23:25 -------
Created an attachment (id=20032)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20032&action=view)
updated example that doesn't rely on zero-length arrays


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[pinskia at gcc dot gnu dot org]

------- Comment #8 from pinskia at gcc dot gnu dot org  2010-03-05 23:26 -------
As I mentioned, it is the array at the end of the struct which is where we
don't warn.  


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |WONTFIX


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[matt at use dot net]

------- Comment #9 from matt at use dot net  2010-03-06 00:18 -------
Alright. Even though PC-Lint now correctly warns, and GCC still does not, I
have updated the attached example yet again to avoid the next constraint you
mention.

GCC still does not detect the array-bounds issue, even when the array whose
bound is being violated it is neither the first nor the last field in the
struct/class. I tried using primitive types, complex types, references/pointer,
and arrays for the fields in question. GCC still never detects the issue.

Even for existing code that uses this pattern for flexible array members, a
private array that has not been otherwise initialized in the ctor, which is
accessed out of bounds in said ctor, seems unlikely. Perhaps I will enter that
as a separate bug, but I hope that this latest code example (which still
decently matches the real code I had the bug with) is worthy of the bug being
detected.


-- 

matt at use dot net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |UNCONFIRMED
         Resolution|WONTFIX                     |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[matt at use dot net]

------- Comment #10 from matt at use dot net  2010-03-06 00:19 -------
Created an attachment (id=20033)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20033&action=view)
yet another example, that does not rely on zero-length arrays or on the array
being the 'last' field in the struct/class


-- 

matt at use dot net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #20031|0                           |1
        is obsolete|                            |
  Attachment #20032|0                           |1
        is obsolete|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[pinskia at gcc dot gnu dot org]

------- Comment #11 from pinskia at gcc dot gnu dot org  2010-03-06 00:23 -------
Oh we ignore off by one errors in some cases too.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[matt at use dot net]

------- Comment #12 from matt at use dot net  2010-03-06 01:31 -------
Changing contents[size] to contents[size + 10] or to contents[size+10000] is
still not triggering the array-bounds warning in any of the compilers I tested
(previously mentioned). In my real code, it was an OB1 bug, so that's what I
would have like to have been detected.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[rguenth at gcc dot gnu dot org]

------- Comment #13 from rguenth at gcc dot gnu dot org  2010-03-06 14:27 -------
Err - it's just because the code is broken:

  tree low_bound, up_bound = array_ref_up_bound (ref);

  low_sub = up_sub = TREE_OPERAND (ref, 1);

  if (!up_bound || TREE_NO_WARNING (ref)
      || TREE_CODE (up_bound) != INTEGER_CST
      /* Can not check flexible arrays.  */
      || (TYPE_SIZE (TREE_TYPE (ref)) == NULL_TREE
          && TYPE_DOMAIN (TREE_TYPE (ref)) != NULL_TREE
          && TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (ref))) == NULL_TREE)

well - this checks TYPE_SIZE/DOMAIN on the element type ...

and the struct hack check is too strict:

      /* Accesses after the end of arrays of size 0 (gcc
         extension) and 1 are likely intentional ("struct
         hack").  */
      || compare_tree_int (up_bound, 1) <= 0)

Replacing that with a more proper (but still too strict) check like

  /* Accesses after the end of arrays at the end of structures
     are likely intentional ("struct hack").  */
  if (TREE_CODE (TREE_OPERAND (ref, 0)) == COMPONENT_REF
      && !TREE_CHAIN (TREE_OPERAND (TREE_OPERAND (ref, 0), 1)))
    return;

gets you

t.C: In function 'int main()':
t.C:27:45: warning: array subscript is above array bounds

it doesn't print that this is from an inlined constructor though.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |rguenth at gcc dot gnu dot
                   |dot org                     |org
           Severity|normal                      |enhancement
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2010-03-06 14:27:49
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[rguenth at gcc dot gnu dot org]

------- Comment #14 from rguenth at gcc dot gnu dot org  2010-03-06 14:28 -------
Created an attachment (id=20036)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20036&action=view)
untested patch


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[rguenth at gcc dot gnu dot org]

------- Comment #15 from rguenth at gcc dot gnu dot org  2010-04-07 12:31 -------
Subject: Bug 43270

Author: rguenth
Date: Wed Apr  7 12:31:32 2010
New Revision: 158058

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=158058
Log:
2010-04-07  Richard Guenther  <rguenther@suse.de>

        PR tree-optimization/43270
        * tree-vrp.c (check_array_ref): Fix flexible array member
        detection.
        * tree-ssa-sccvn.h (fully_constant_vn_reference_p): Declare.
        * tree-ssa-pre.c (phi_translate_1): Adjust.
        (fully_constant_expression): Split out vn_reference handling to ...
        * tree-ssa-sccvn.c (fully_constant_vn_reference_p): ... here.
        Fold reads from constant strings.
        (vn_reference_lookup): Handle fully constant references.
        (vn_reference_lookup_pieces): Likewise.
        * Makefile.in (expmed.o-warn): Add -Wno-error.

        * g++.dg/warn/Warray-bounds-4.C: New testcase.
        * gcc.dg/Warray-bounds-7.c: Likewise.

Added:
    trunk/gcc/testsuite/g++.dg/warn/Warray-bounds-4.C
    trunk/gcc/testsuite/gcc.dg/Warray-bounds-7.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/Makefile.in
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-pre.c
    trunk/gcc/tree-ssa-sccvn.c
    trunk/gcc/tree-ssa-sccvn.h
    trunk/gcc/tree-vrp.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[rguenth at gcc dot gnu dot org]

------- Comment #16 from rguenth at gcc dot gnu dot org  2010-04-07 12:34 -------
Fixed for 4.6.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.6.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[pinskia at gcc dot gnu dot org]

------- Comment #17 from pinskia at gcc dot gnu dot org  2010-07-05 00:11 -------
>        * Makefile.in (expmed.o-warn): Add -Wno-error.

There is no comment in Makefile.in why this is there, can you add one.  Also is
this a false warning from gcc or a true one, I cannot tell.

Note I also get a warning for:
/home/apinski/src/gcc-fsf/local//gcc/libgcc/../gcc/crtstuff.c:372:19: warning:
array subscript is above array bounds [-Warray-bounds]

Where the code does:
  if (__JCR_LIST__[0])

__JCR_LIST__ is defined as:
STATIC void *__JCR_LIST__[]
  __attribute__ ((used, section(JCR_SECTION_NAME), aligned(sizeof(void*))))
  = { };

Which means it is not above the array bounds after all since there is no real
array bounds for that array :).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270

[Bug tree-optimization/43270] array-bounds false negative

[rguenth at gcc dot gnu dot org]

------- Comment #18 from rguenth at gcc dot gnu dot org  2010-07-07 10:08 -------
*** Bug 44848 has been marked as a duplicate of this bug. ***


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |eli dot friedman at gmail
                   |                            |dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43270