Bug 45978 - [4.6 Regression] bogus "array subscript is above array bounds" warning in extremely simple code with no loops
[4.6 Regression] bogus "array subscript is above array bounds" warning in ext...
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: tree-optimization
4.6.0
: P2 normal
: 4.7.0
Assigned To: Not yet assigned to anyone
: diagnostic
Depends on:
Blocks: 56456
  Show dependency treegraph
 
Reported: 2010-10-12 07:37 UTC by miles
Modified: 2013-04-12 16:17 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work: 4.7.0
Known to fail:
Last reconfirmed: 2010-10-12 09:44:10


Attachments
C++ source file illustrating bug (243 bytes, text/plain)
2010-10-12 07:37 UTC, miles
Details
An obviously correct example that triggers the bug. (140 bytes, text/x-csrc)
2011-08-19 22:54 UTC, torsten
Details
testcase (1.08 KB, text/x-csrc)
2012-07-16 06:48 UTC, Petr.Salinger
Details

Note You need to log in before you can comment on or make changes to this bug.
Description miles 2010-10-12 07:37:22 UTC
Created attachment 22021 [details]
C++ source file illustrating bug

[I don't know if the "component:" field is correct, but there doesn't seem to be a "I don't know" option]

There are many bugs in gcc's bugzilla related to bogus array-bounds warnings, but most of them seem to involve loops and other situations that may need non-trivial analysis by the compiler.

However the attached C++ source file ("tt.cc") seems to be almost trivial; it contains no loops at all, and all array references use a constant (and valid) index, but it nonetheless elicits an "array subscript is above array bounds" warning from the compiler.  As far as I can figure, the warning is bogus.

Compilation looks like:

   $ g++-snapshot -O3 -S -Wall -Wextra tt.cc
   tt.cc: In function 'Z test()':
   tt.cc:15:21: warning: array subscript is above array bounds [-Warray-bounds]

Version info:

   $ g++-snapshot --version
   g++ (Debian 20101009-1) 4.6.0 20101009 (experimental) [trunk revision 165234]
   Copyright (C) 2010 Free Software Foundation, Inc.
   This is free software; see the source for copying conditions.  There is NO
   warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

"g++-snapshot" just invokes g++ from Debian's gcc-snapshot package, which is a somewhat recent trunk snapshot.

["tt.cc" is as minimal as I can get it -- moving, eliminating, or changing any field in any of the structures seems to silence the warning.]

Thanks,

-Miles
Comment 1 Richard Biener 2010-10-12 09:44:10 UTC
Confirmed.  The vectorizer uses &MEM[(struct Y *)&<retval>].ar[4]; as the
base address for the store to .c.x, .c.y, .c.z, .d.
Comment 2 Richard Biener 2010-11-15 12:00:20 UTC
Hmm, I'm not sure how to address this - we warn about the address-taking
operation which only at VRP time get's folded to &MEM[(struct Y *)&<retval>].ar[4]
via maybe_fold_stmt_addition (which I only preserved to avoid some regressions
I don't remember anymore during mem-ref development).

Clearly even the first vectorized access spans more than the array (but we
could set TREE_NO_WARNING on the memory reference itself).
Comment 3 Richard Biener 2011-02-08 13:37:48 UTC
Something like

Index: gimple-fold.c
===================================================================
--- gimple-fold.c       (revision 169917)
+++ gimple-fold.c       (working copy)
@@ -212,6 +212,7 @@ maybe_fold_offset_to_array_ref (location
   tree min_idx, idx, idx_type, elt_offset = integer_zero_node;
   tree array_type, elt_type, elt_size;
   tree domain_type;
+  tree no_warning = false;
 
   /* If BASE is an ARRAY_REF, we can pick up another offset (this time
      measured in units of the size of elements type) from that ARRAY_REF).
@@ -308,26 +309,34 @@ maybe_fold_offset_to_array_ref (location
        char *(c[4]);
        c[3][2];
      should not be simplified into (*c)[14] or tree-vrp will
-     give false warnings.
-     This is only an issue for multi-dimensional arrays.  */
-  if (TREE_CODE (elt_type) == ARRAY_TYPE
-      && domain_type)
+     give false warnings.  For multi-dimensional arrays
+     avoid this transformation, for one-dimensional arrays
+     set TREE_NO_WARNING on out-of-bound references.  */
+  if (domain_type)
     {
+      bool oob = false;
       if (TYPE_MAX_VALUE (domain_type)
          && TREE_CODE (TYPE_MAX_VALUE (domain_type)) == INTEGER_CST
          && tree_int_cst_lt (TYPE_MAX_VALUE (domain_type), idx))
-       return NULL_TREE;
+       oob = true;
       else if (TYPE_MIN_VALUE (domain_type)
               && TREE_CODE (TYPE_MIN_VALUE (domain_type)) == INTEGER_CST
               && tree_int_cst_lt (idx, TYPE_MIN_VALUE (domain_type)))
-       return NULL_TREE;
+       oob = true;
       else if (compare_tree_int (idx, 0) < 0)
+       oob = true;
+      if (oob)
+       {
+       if (TREE_CODE (elt_type) == ARRAY_TYPE)
        return NULL_TREE;
+       no_warning = true;
+       }
     }
 
   {
     tree t = build4 (ARRAY_REF, elt_type, base, idx, NULL_TREE, NULL_TREE);
     SET_EXPR_LOCATION (t, loc);
+    TREE_NO_WARNING (t) = no_warning;
     return t;
   }
 }


fixes this but will cause us to omit all warnings for C array accesses
that are out-of-bounds:

FAIL: gcc.dg/Warray-bounds.c  (test for warnings, line 59)
FAIL: gcc.dg/Warray-bounds.c  (test for warnings, line 60)
FAIL: gcc.dg/Warray-bounds.c  (test for warnings, line 65)
FAIL: gcc.dg/Warray-bounds.c  (test for warnings, line 66)
FAIL: gcc.dg/Warray-bounds.c  (test for warnings, line 72)
FAIL: gcc.dg/Warray-bounds.c  (test for warnings, line 73)


In general I'd say we should not warn from VRP after loop opts, and
for address-taking operations we should have a distinct warning,
eventually looking at object sizes, not only type bounds.

This bug is a regression only because we now vectorize the testcase to

        xorps   %xmm0, %xmm0
        movq    %rdi, %rax
        movlps  %xmm0, (%rdi)
        movhps  %xmm0, 8(%rdi)
        movlps  %xmm0, 16(%rdi)
        movlps  %xmm0, 24(%rdi)

compared to

        xorl    %edx, %edx
        movq    %rdi, %rax
        movl    %edx, (%rdi)
        movl    %edx, 4(%rdi)
        movl    %edx, 8(%rdi)
        movl    %edx, 12(%rdi)
        movl    %edx, 16(%rdi)
        movl    %edx, 20(%rdi)
        movl    %edx, 24(%rdi)
        movl    %edx, 28(%rdi)

which is a good thing.

Eventually we can mitigate the problem from inside the vectorizer by
not using

  vect_p.7_13 = &MEM[(struct Y *)&<retval>].ar[0];

but instead

  vect_p.7_13 = &<retval> + off

style initial addresses.  Unfortunately that isn't very easy to do.
Comment 4 Jakub Jelinek 2011-03-25 19:52:47 UTC
GCC 4.6.0 is being released, adjusting target milestone.
Comment 5 miles 2011-05-20 04:09:01 UTC
Hmm, I'm not getting this warning anymore ... is the bug fixed?

[version "g++ (Debian 20110519-1) 4.7.0 20110519 (experimental) [trunk revision 173903]"]

Thanks,

-Miles
Comment 6 Richard Biener 2011-05-20 08:19:59 UTC
The bug was indeed worked around in VRP by making it use the CCP engine.
It now sees

  MEM[(struct Y *)&<retval> + 16B] = vect_cst_.6_11;

instead of &MEM[(struct Y *)&<retval>].ar[4] as a base which makes it not warn.

The vectorizer still uses that address as base though.

So I suppose we can say it's fixed in 4.7 by adjusting the pass that
emits the warning.
Comment 7 Jakub Jelinek 2011-06-27 12:33:02 UTC
GCC 4.6.1 is being released.
Comment 8 torsten 2011-08-19 22:54:11 UTC
Created attachment 25059 [details]
An obviously correct example that triggers the bug.

(In reply to comment #7)
> GCC 4.6.1 is being released.

I just searched for this problem due to a bug report that SWIG causes out of bounds array accesses: https://sourceforge.net/tracker/?func=detail&atid=101645&aid=3394790&group_id=1645

It turns out that the generated code is similar to the trivial example that I attached. Perhaps this makes it easier to track down the bug.
Comment 9 Jakub Jelinek 2011-10-26 17:13:44 UTC
GCC 4.6.2 is being released.
Comment 10 Jakub Jelinek 2012-03-01 14:38:47 UTC
GCC 4.6.3 is being released.
Comment 11 Petr.Salinger 2012-07-16 06:48:07 UTC
Created attachment 27800 [details]
testcase

Another failing testcase - reduced from xserver-xorg-input-mouse
Comment 12 Jakub Jelinek 2013-04-12 16:17:49 UTC
The 4.6 branch has been closed, fixed in GCC 4.7.0.