[Bug tree-optimization/51721] New: -Warray-bounds false positives and inconsistencies

[Bug tree-optimization/51721] New: -Warray-bounds false positives and inconsistencies

[vincent-gcc at vinc17 dot net]

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

             Bug #: 51721
           Summary: -Warray-bounds false positives and inconsistencies
    Classification: Unclassified
           Product: gcc
           Version: 4.6.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: vincent-gcc@vinc17.net


I get strange -Warray-bounds false positives on the following code:

static int a[10], b[10], c[10], d[10];

unsigned int f (unsigned int v)
{
  return v == 17 ? 11 : v;
}

unsigned int g (unsigned int v)
{
  return v == 17 ? 17 : v;
}

void t (unsigned int s)
{
  if (s >> 1 == 0)
    {
      a[f(s)] = 0;
      a[f(s)] = 0;
      b[f(s)] = 0;
      c[g(s)] = 0;
      c[g(s)] = 0;
      d[H(s)] = 0;
    }
}

With gcc (Debian 4.6.2-9) 4.6.2 on a Debian/x86_64 machine:

$ gcc-snapshot -O2 -Warray-bounds -c bounds.c -DH=f
bounds.c: In function 't':
bounds.c:17:8: warning: array subscript is above array bounds [-Warray-bounds]
bounds.c:19:8: warning: array subscript is above array bounds [-Warray-bounds]
bounds.c:20:8: warning: array subscript is above array bounds [-Warray-bounds]

$ gcc-snapshot -O2 -Warray-bounds -c bounds.c -DH=g
bounds.c: In function 't':
bounds.c:17:8: warning: array subscript is above array bounds [-Warray-bounds]

Note: line 17 is the first "a[f(s)] = 0;".

[Bug tree-optimization/51721] -Warray-bounds false positives and inconsistencies

[vincent-gcc at vinc17 dot net]

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

--- Comment #1 from Vincent Lefèvre <vincent-gcc at vinc17 dot net> 2011-12-31 01:50:59 UTC ---
Oops, gcc-snapshot was not GCC 4.6.2. Anyway, I get the same warnings with GCC
4.6.2 and gcc-snapshot, which is:

gcc (Debian 20111210-1) 4.7.0 20111210 (experimental) [trunk revision 182188]

[Bug tree-optimization/51721] -Warray-bounds false positives and inconsistencies

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-12-31 13:42:55 UTC ---
This is a common problem with the -Warray-bounds warning, first jump threading
(during vrp1) optimizes it into just a single s == 17 check, followed by
a[11] = 0; b[11] = 0; c[17] = 0; d[11] = 0; if true and a[s] = 0; etc. if false
(well, at the end of vrp1 the constants aren't in the array refs yet, but they
are propagated there afterwards), and as no optimization figures out the weird
if (s >> 1 == 0) check (if (s < 2) would DTRT) to determine that s is not 17,
vrp2 warns about those accesses.
Perhaps for -Warray-bounds (at least if not -Warray-bounds=2 or similar) we
shouldn't warn on code that has been jump threaded, anyway, I don't think that
is solvable for 4.7 easily.

What we perhaps could do more easily for this testcase (and could improve code
too) is during VRP for:
<bb 2>:
  D.1716_2 = s_1(D) >> 1;
  if (D.1716_2 == 0)
    goto <bb 3>;
  else
    goto <bb 12>;
(or any other constant after >>, both signed and unsigned right shift, and ==
or !=) insert ASSERT_EXPRs into both bbs, saying that the SSA_NAME in rhs1 of
the
shift is in/out of second ==/!= operand << rhs2 of shift, -""- + ((1 << rhs2) -
1) range.  In this case it would be ASSERT_EXPRs that s_1(D) <= 1 at the start
of bb 3 (and if bb 12 had only one predecessor, also that s_1(D) > 1 at bb 12
start).  Richard, what do you think about that?

[Bug tree-optimization/51721] -Warray-bounds false positives and inconsistencies

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-01 00:18:06 UTC ---
Created attachment 26209
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26209
gcc47-pr51721.patch

So far completely untested patch to optimize that if (s >> 1 == 0) by VRP.

[Bug tree-optimization/51721] -Warray-bounds false positives and inconsistencies

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-01-02
     Ever Confirmed|0                           |1

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-02 10:19:13 UTC ---
(In reply to comment #2)
> This is a common problem with the -Warray-bounds warning, first jump threading
> (during vrp1) optimizes it into just a single s == 17 check, followed by
> a[11] = 0; b[11] = 0; c[17] = 0; d[11] = 0; if true and a[s] = 0; etc. if false
> (well, at the end of vrp1 the constants aren't in the array refs yet, but they
> are propagated there afterwards), and as no optimization figures out the weird
> if (s >> 1 == 0) check (if (s < 2) would DTRT) to determine that s is not 17,
> vrp2 warns about those accesses.
> Perhaps for -Warray-bounds (at least if not -Warray-bounds=2 or similar) we
> shouldn't warn on code that has been jump threaded, anyway, I don't think that
> is solvable for 4.7 easily.
> 
> What we perhaps could do more easily for this testcase (and could improve code
> too) is during VRP for:
> <bb 2>:
>   D.1716_2 = s_1(D) >> 1;
>   if (D.1716_2 == 0)
>     goto <bb 3>;
>   else
>     goto <bb 12>;
> (or any other constant after >>, both signed and unsigned right shift, and ==
> or !=) insert ASSERT_EXPRs into both bbs, saying that the SSA_NAME in rhs1 of
> the
> shift is in/out of second ==/!= operand << rhs2 of shift, -""- + ((1 << rhs2) -
> 1) range.  In this case it would be ASSERT_EXPRs that s_1(D) <= 1 at the start
> of bb 3 (and if bb 12 had only one predecessor, also that s_1(D) > 1 at bb 12
> start).  Richard, what do you think about that?

Yeah, if that turns out to be a common pattern, though maybe restrict it to
==/!= 0 tests?  (if that simplifies the patch)

[Bug tree-optimization/51721] -Warray-bounds false positives and inconsistencies

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |jakub at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-02 10:24:55 UTC ---
Created attachment 26216
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26216
gcc47-pr51721.patch

Uptested patch, this time even with testcases.  Restricting just to
EQ_EXPR/NE_EXPR would save just 3 extra stmts and two ifs in the patch, and
restricting to EQ_EXPR/NE_EXPR with constant 0 as opposed to any constant
wouldn't simplify the patch at all.

[Bug tree-optimization/51721] -Warray-bounds false positives and inconsistencies

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-03 10:18:12 UTC ---
http://gcc.gnu.org/ml/gcc-patches/2012-01/msg00046.html
Deferring for 4.8.
We should also add ASSERT_EXPRs e.g. for
  unsigned int x;
  int D.1234;
  D.1234_2 = (int) x_1(D);
  if (D.1234_2 < 0)
(saying that x_1 has range [__INT_MAX__+1, -1U] if true and [0, __INT_MAX__] if
false and perhaps for if ((x & 0x80000000)) kind of tests.

[Bug tree-optimization/51721] -Warray-bounds false positives and inconsistencies

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-03-05 12:28:12 UTC ---
Author: jakub
Date: Mon Mar  5 12:27:55 2012
New Revision: 184927

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184927
Log:
    PR tree-optimization/51721
    * tree-vrp.c (register_edge_assert_for_2): If comparing
    lhs of right shift by constant with an integer constant,
    add ASSERT_EXPRs for the rhs1 of the right shift.

    * gcc.dg/tree-ssa/vrp63.c: New test.
    * gcc.dg/pr51721.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/pr51721.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/vrp63.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-vrp.c

[Bug tree-optimization/51721] -Warray-bounds false positives and inconsistencies

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-03-12 11:12:55 UTC ---
Author: jakub
Date: Mon Mar 12 11:12:49 2012
New Revision: 185222

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185222
Log:
    PR tree-optimization/51721
    * tree-vrp.c (register_edge_assert_for_2): Add asserts for unsvar
    if (int) unsvar cmp CST.

    * gcc.dg/tree-ssa/vrp64.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/vrp64.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-vrp.c

[Bug tree-optimization/51721] -Warray-bounds false positives and inconsistencies

[steven at gcc dot gnu.org]

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

Steven Bosscher <steven at gcc dot gnu.org> changed:

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

--- Comment #9 from Steven Bosscher <steven at gcc dot gnu.org> 2012-11-09 23:26:36 UTC ---
.