[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[vincent-gcc at vinc17 dot net]

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

--- Comment #16 from Vincent Lefèvre  ---
Thanks, I confirm that for r244974, there is no longer any issue on the MPFR
code.

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[rguenth at gcc dot gnu.org]

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

--- Comment #15 from Richard Biener  ---
Author: rguenth
Date: Fri Jan 27 12:30:43 2017
New Revision: 244974

URL: https://gcc.gnu.org/viewcvs?rev=244974=gcc=rev
Log:
2017-01-27  Richard Biener  

PR tree-optimization/71433
* tree-vrp.c (register_new_assert_for): Revert earlier changes.
(compare_assert_loc): New function.
(process_assert_insertions): Sort and optimize assert locations
to remove duplicates and push down identical assertions on
edges to their destination block.

* gcc.dg/Warray-bounds-21.c: New testcase.

Added:
trunk/gcc/testsuite/gcc.dg/Warray-bounds-21.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/testsuite/ChangeLog
trunk/gcc/tree-vrp.c

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

 Status|REOPENED|RESOLVED
 Resolution|--- |FIXED

--- Comment #14 from Richard Biener  ---
Fixed again.  Thanks for the new testcase.

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[rguenth at gcc dot gnu.org]

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

--- Comment #13 from Richard Biener  ---
Ok, same reason but my "simple" two-predecessor "hack" doesn't get it because
we now have three...

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[vincent-gcc at vinc17 dot net]

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

--- Comment #12 from Vincent Lefèvre  ---
New test case, based on the previous one (just a __builtin_expect added):

int t[1];
int a (void);
int fct (int r, long e, int neg)
{
  int d = 0;
  if (r == 4)
r = neg ? 3 : 2;
  if (__builtin_expect(e < -52, 0))
d = r == 0 && a () ? 1 : 2;
  else
{
  int i, n = 53;
  if (e < 0)
n += e;
  for (i = 1 ; i < n / 64 + 1 ; i++)
d = t[i];
}
  return d;
}

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[vincent-gcc at vinc17 dot net]

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

Vincent Lefèvre  changed:

   What|Removed |Added

 Status|RESOLVED|REOPENED
 Resolution|FIXED   |---

--- Comment #11 from Vincent Lefèvre  ---
The warning no longer occurs on the simple test case, but it still occurs on
the MPFR code:

$ gcc-snapshot --version
gcc (Debian 20170125-1) 7.0.1 20170125 (experimental) [trunk revision 244909]
Copyright (C) 2017 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.

$ gcc-snapshot -DMPFR_WANT_ASSERT=2 -DMPFR_USE_LOGGING=1 -DHAVE_INTTYPES_H=1
-DHAVE_STDINT_H=1 -DLT_OBJDIR=\".libs/\" -DHAVE_DOUBLE_IEEE_LITTLE_ENDIAN=1
-DHAVE_LITTLE_ENDIAN=1 -DTIME_WITH_SYS_TIME=1 -DHAVE_LOCALE_H=1
-DHAVE_WCHAR_H=1 -DHAVE_STDARG=1 -DHAVE_SYS_TIME_H=1
-DHAVE_STRUCT_LCONV_DECIMAL_POINT=1 -DHAVE_STRUCT_LCONV_THOUSANDS_SEP=1
-DHAVE_ALLOCA_H=1 -DHAVE_STDINT_H=1 -DHAVE_VA_COPY=1 -DHAVE_SETLOCALE=1
-DHAVE_GETTIMEOFDAY=1 -DHAVE_SIGNAL=1 -DHAVE_SIGACTION=1 -DHAVE_LONG_LONG=1
-DHAVE_INTMAX_T=1 -DMPFR_HAVE_INTMAX_MAX=1 -DMPFR_PRINTF_MAXLM=\"j\"
-DMPFR_HAVE_NORETURN=1 -DMPFR_HAVE_BUILTIN_UNREACHABLE=1
-DMPFR_HAVE_CONSTRUCTOR_ATTR=1 -DHAVE_PTHREAD=1 -DMPFR_HAVE_FESETROUND=1
-DHAVE_DENORMS=1 -DHAVE_DENORMS_FLT=1 -DHAVE_SIGNEDZ=1 -DHAVE_ROUND=1
-DHAVE_TRUNC=1 -DHAVE_FLOOR=1 -DHAVE_CEIL=1 -DHAVE_NEARBYINT=1
-DHAVE_LDOUBLE_IEEE_EXT_LITTLE=1 -DMPFR_WANT_DECIMAL_FLOATS=1
-DMPFR_WANT_FLOAT128=1 -DMPFR_USE_STATIC_ASSERT=1 -DHAVE_CLOCK_GETTIME=1
-DHAVE_ATTRIBUTE_MODE=1 -DPRINTF_L=1 -DPRINTF_T=1
-DHAVE___GMPN_SBPI1_DIVAPPR_Q=1 -DHAVE___GMPN_INVERT_LIMB=1
-DHAVE___GMPN_RSBLSH_N=1 -DMPFR_LONG_WITHIN_LIMB=1 -DHAVE_GETRUSAGE=1 -I.  
-I/usr/local/gmp-6.1.0-debug/include  -O2 -Wall -Wold-style-declaration
-Wold-style-definition -Wmissing-parameter-type -Wmissing-prototypes
-Wmissing-declarations -Wmissing-field-initializers -Wno-error=unused-function
-Wno-error=deprecated-declarations -Werror -DMPFR_COV_CHECK -c -o get_d.o
get_d.c
get_d.c: In function 'mpfr_get_d':
get_d.c:115:24: error: array subscript is above array bounds
[-Werror=array-bounds]
 d = (d + tp[i]) / MP_BASE_AS_DOUBLE;
  ~~^~~
cc1: all warnings being treated as errors

One has:

  for (i = 1 ; i < np ; i++)
d = (d + tp[i]) / MP_BASE_AS_DOUBLE;

and here np = 1 (known at compile time).

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[rguenth at gcc dot gnu.org]

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

--- Comment #10 from Richard Biener  ---
Author: rguenth
Date: Tue Jan 17 08:38:59 2017
New Revision: 244520

URL: https://gcc.gnu.org/viewcvs?rev=244520=gcc=rev
Log:
2017-01-17  Richard Biener  

PR tree-optimization/71433
* tree-vrp.c (register_new_assert_for): Merge same asserts
on all incoming edges.
(process_assert_insertions_for): Handle insertions at the
beginning of BBs.

* gcc.dg/Warray-bounds-20.c: New testcase.

Added:
trunk/gcc/testsuite/gcc.dg/Warray-bounds-20.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/testsuite/ChangeLog
trunk/gcc/tree-vrp.c

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #9 from Richard Biener  ---
Fixed.

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

   Priority|P3  |P1
 Status|NEW |ASSIGNED
   Assignee|law at redhat dot com  |rguenth at gcc dot 
gnu.org

--- Comment #8 from Richard Biener  ---
Mine.

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Biener  ---
Before VRP we have

  # r_3 = PHI <1(2)>
  if (e_14(D) < -52)
goto ;
  else
goto ;

  :
  # r_7 = PHI 
  if (e_14(D) < -52)
goto ;
  else
goto ;

  :
  if (r_7 == 0)
goto ;
  else
goto ;

  :
  goto ;

  :
  _1 = e_14(D) < 0;
  _2 = (long int) _1;
  if (e_14(D) < 0)


so _maybe_ the fix is to go over the pending asserts we want to insert and
see if we want to insert the same one on all predecessors of a block and
then simply merge them, inserting them into the block (BB 7) itself.  I
think that should be reasonably easy to do.

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

 CC||rguenth at gcc dot gnu.org

--- Comment #6 from Richard Biener  ---
The issue is that jump-threading duplicated a tail with equal conditions so VRP
is faced with

:
if (e_14(D) < -52)
  goto ;
else
  goto ;

# r_7 = PHI 
if (e_14(D) < -52)
  goto ;
else
  goto ;

:
e_23 = ASSERT_EXPR = -52>;
goto ;

:
e_22 = ASSERT_EXPR = -52>;

:
_1 = e_14(D) < 0;
_2 = (long int) _1;
if (e_14(D) < 0)


due to the fact that VRP does not insert PHIs for asserts (the assumption is
that we'll always merge to [-INF, +INF] I guess) we lose the fact that
e_14(D) >= -52 at the entry to BB 7.

So it's basically VRPs own stupidness.  The EVRP DOM-based algorithm should be
fine here (and I hope we can rip out the SSA propagator based VRP at some
point,
I believe I now fixed most of its short-comings, loops still need to be dealt
with though - stay tuned).

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[vincent-gcc at vinc17 dot net]

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

--- Comment #5 from Vincent Lefèvre  ---
Any news?

FYI, the provided test case no longer yields the warning with Debian's
gcc-snapshot/20161006-1 package, but the issue is still present when compiling
MPFR (get_d.c). Here's a corresponding simple test case:

int t[1];
int fct (int r, long e)
{
  int d = 0;
  if (r == 4)
r = 1;
  if (e < -52)
d = r == 0 ? 1 : 2;
  else
{
  int i, n = 53;
  if (__builtin_expect (e < 0, 0))
n += e;
  for (i = 1 ; i < n / 64 + 1 ; i++)
t[i] = 0;
}
  return d;
}

Surprisingly, if I replace "int t[1];" by "int t[2];", I no longer get the
warning.

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[law at redhat dot com]

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

Jeffrey A. Law  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2016-06-09
 Ever confirmed|0   |1

--- Comment #4 from Jeffrey A. Law  ---
Thanks Vincent.   Glad to have this little test.

AFAICT before the recent jump threading changes we had something like this in
VRP1:


;;   basic block 9, loop depth 0, count 0, freq 900, maybe hot
;;prev block 8, next block 10, flags: (NEW, REACHABLE)
;;pred:   5 [50.0%]  (FALSE_VALUE,EXECUTABLE)
  e_23 = ASSERT_EXPR = -52>;
  if (e_23 < 0)
goto ;
  else
goto ;
;;succ:   10 [27.0%]  (TRUE_VALUE,EXECUTABLE)
;;11 [73.0%]  (FALSE_VALUE,EXECUTABLE)

;;   basic block 10, loop depth 0, count 0, freq 243, maybe hot
;;prev block 9, next block 11, flags: (NEW, REACHABLE)
;;pred:   9 [27.0%]  (TRUE_VALUE,EXECUTABLE)
  e_22 = ASSERT_EXPR ;
  _2 = (unsigned int) e_22;
  _3 = _2 + 53;
  n_16 = (int) _3;
;;succ:   11 [100.0%]  (FALLTHRU,EXECUTABLE)

;;   basic block 11, loop depth 0, count 0, freq 900, maybe hot
;;prev block 10, next block 12, flags: (NEW, REACHABLE)
;;pred:   9 [73.0%]  (FALSE_VALUE,EXECUTABLE)
;;10 [100.0%]  (FALLTHRU,EXECUTABLE)
  # n_9 = PHI <53(9), n_16(10)>
  goto ;

Note that as written we can pretty easily derive a range for n_16 as [1..52]
which ultimately allows us to remove the block with the out-of-range array
index as unreachable.


With the threading before VRP things are bit more complex as BB10 has 3
predecessors (due to threading).  Now it happens that on each predecessor we
have a suitable ASSERT_EXPR to narrow the range of e_13.  But we're unable to
use them in that form.

Ideally we'd like to either be smarter about the initial insertion point or
sink them  after insertion.  I'm pretty sure that would resolve this issue. 
However, it's not compile-time friendly.  I'm going to need to think a bit more
on this one.

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[vincent-gcc at vinc17 dot net]

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

--- Comment #3 from Vincent Lefèvre  ---
The bug was introduced in r236831.

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

   Keywords||diagnostic
   Target Milestone|--- |7.0

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[vincent-gcc at vinc17 dot net]

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

--- Comment #2 from Vincent Lefèvre  ---
Well, this test case also yields a warning with 20160508-1 (trunk r236009).
Here's a new test case that yields no warnings with 20160508-1, but yields one
with 20160603-1 (r237077).

int t[1];
int a (void);
int fct (int r, long e, int neg)
{
  int d = 0;
  if (r == 4)
r = neg ? 3 : 2;
  if (e < -52)
d = r == 0 && a () ? 1 : 2;
  else
{
  int i, n = 53;
  if (e < 0)
n += e;
  for (i = 1 ; i < n / 64 + 1 ; i++)
d = t[i];
}
  return d;
}

[Bug tree-optimization/71433] [7 Regression] -Warray-bounds false positive with -O2

[vincent-gcc at vinc17 dot net]

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

--- Comment #1 from Vincent Lefèvre  ---
Here's a simple test case:

int t[1];
int fct (long e)
{
  int d = 0, i, n = 52;
  if (e < 0)
n += e;
  for (i = 1 ; i < n / 64 + 1 ; i++)
d = t[i];
  return d;
}

If I replace "long" by "int" in the second line, I no longer get any warning.