Re: [PATCH] have -Warray-bounds treat [max, min] ranges same as anti-ranges (PR 89720)

2019-03-19 Thread Richard Biener
On Mon, Mar 18, 2019 at 10:31 PM Jeff Law  wrote:
>
> On 3/18/19 1:03 PM, Martin Sebor wrote:
> > I the -Warray-bounds enhancement committed at the beginning
> > of the GCC 9 window I tried to correctly handle offsets in
> > MEM_REFs in the [max, min] kind of a range after converting
> > from sizetype to ptrdiff_type, but I did it wrong.  The bug
> > results in false positives in some unusual use cases that
> > I didn't consider at the time.
> >
> > The attached patch removes this incorrect handling and uses
> > the conservative anti-range handling for these cases instead.
> >
> > Martin
> >
> > PS Is there some technical reason why pointer offsets are
> > represented as the unsigned sizetype when they can be signed?
> I'm not aware of a conscious decision to treat them as unsigned or a
> particular target need to do so.  It's most likely a historical accident.

That historical accident included treating those unsigned types as
sign-extending ...

But yes, changing sizetype to ssizetype wherever we use it in
offset context would be a cleanup I guess.  But IIRC Bin tried
this and the fallout is (as usual) non-trivial to fix...

Richard.

>
> >
> > gcc-89720.diff
> >
> > PR tree-optimization/89720 - Spurious -Warray-bounds warning on a range 
> > with max < min
> >
> > gcc/ChangeLog:
> >
> >   PR tree-optimization/89720
> >   * tree-vrp.c (vrp_prop::check_mem_ref): Treat range with max < min
> >   more conservatively, the same as anti-range.
> >
> > gcc/testsuite/ChangeLog:
> >
> >   PR tree-optimization/89720
> >   * gcc.dg/Warray-bounds-42.c: New test.
> OK
> jeff


Re: [PATCH] have -Warray-bounds treat [max, min] ranges same as anti-ranges (PR 89720)

2019-03-18 Thread Jeff Law
On 3/18/19 1:03 PM, Martin Sebor wrote:
> I the -Warray-bounds enhancement committed at the beginning
> of the GCC 9 window I tried to correctly handle offsets in
> MEM_REFs in the [max, min] kind of a range after converting
> from sizetype to ptrdiff_type, but I did it wrong.  The bug
> results in false positives in some unusual use cases that
> I didn't consider at the time.
> 
> The attached patch removes this incorrect handling and uses
> the conservative anti-range handling for these cases instead.
> 
> Martin
> 
> PS Is there some technical reason why pointer offsets are
> represented as the unsigned sizetype when they can be signed?
I'm not aware of a conscious decision to treat them as unsigned or a
particular target need to do so.  It's most likely a historical accident.


> 
> gcc-89720.diff
> 
> PR tree-optimization/89720 - Spurious -Warray-bounds warning on a range with 
> max < min
> 
> gcc/ChangeLog:
> 
>   PR tree-optimization/89720
>   * tree-vrp.c (vrp_prop::check_mem_ref): Treat range with max < min
>   more conservatively, the same as anti-range.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR tree-optimization/89720
>   * gcc.dg/Warray-bounds-42.c: New test.
OK
jeff


[PATCH] have -Warray-bounds treat [max, min] ranges same as anti-ranges (PR 89720)

2019-03-18 Thread Martin Sebor

I the -Warray-bounds enhancement committed at the beginning
of the GCC 9 window I tried to correctly handle offsets in
MEM_REFs in the [max, min] kind of a range after converting
from sizetype to ptrdiff_type, but I did it wrong.  The bug
results in false positives in some unusual use cases that
I didn't consider at the time.

The attached patch removes this incorrect handling and uses
the conservative anti-range handling for these cases instead.

Martin

PS Is there some technical reason why pointer offsets are
represented as the unsigned sizetype when they can be signed?
PR tree-optimization/89720 - Spurious -Warray-bounds warning on a range with max < min

gcc/ChangeLog:

	PR tree-optimization/89720
	* tree-vrp.c (vrp_prop::check_mem_ref): Treat range with max < min
	more conservatively, the same as anti-range.

gcc/testsuite/ChangeLog:

	PR tree-optimization/89720
	* gcc.dg/Warray-bounds-42.c: New test.

Index: gcc/tree-vrp.c
===
--- gcc/tree-vrp.c	(revision 269767)
+++ gcc/tree-vrp.c	(working copy)
@@ -4546,9 +4546,9 @@ vrp_prop::check_mem_ref (location_t location, tree
   const value_range *vr = NULL;
 
   /* Determine the offsets and increment OFFRANGE for the bounds of each.
- The loop computes the the range of the final offset for expressions
- such as (A + i0 + ... + iN)[CSTOFF] where i0 through iN are SSA_NAMEs
- in some range.  */
+ The loop computes the range of the final offset for expressions such
+ as (A + i0 + ... + iN)[CSTOFF] where i0 through iN are SSA_NAMEs in
+ some range.  */
   while (TREE_CODE (arg) == SSA_NAME)
 {
   gimple *def = SSA_NAME_DEF_STMT (arg);
@@ -4583,26 +4583,21 @@ vrp_prop::check_mem_ref (location_t location, tree
 
   if (vr->kind () == VR_RANGE)
 	{
-	  if (tree_int_cst_lt (vr->min (), vr->max ()))
+	  offset_int min
+	= wi::to_offset (fold_convert (ptrdiff_type_node, vr->min ()));
+	  offset_int max
+	= wi::to_offset (fold_convert (ptrdiff_type_node, vr->max ()));
+	  if (min < max)
 	{
-	  offset_int min
-		= wi::to_offset (fold_convert (ptrdiff_type_node, vr->min ()));
-	  offset_int max
-		= wi::to_offset (fold_convert (ptrdiff_type_node, vr->max ()));
-	  if (min < max)
-		{
-		  offrange[0] += min;
-		  offrange[1] += max;
-		}
-	  else
-		{
-		  offrange[0] += max;
-		  offrange[1] += min;
-		}
+	  offrange[0] += min;
+	  offrange[1] += max;
 	}
 	  else
 	{
-	  /* Conservatively add [-MAXOBJSIZE -1, MAXOBJSIZE]
+	  /* When MIN >= MAX, the offset is effectively in a union
+		 of two ranges: [-MAXOBJSIZE -1, MAX] and [MIN, MAXOBJSIZE].
+		 Since there is no way to represent such a range across
+		 additions, conservatively add [-MAXOBJSIZE -1, MAXOBJSIZE]
 		 to OFFRANGE.  */
 	  offrange[0] += arrbounds[0];
 	  offrange[1] += arrbounds[1];
Index: gcc/testsuite/gcc.dg/Warray-bounds-42.c
===
--- gcc/testsuite/gcc.dg/Warray-bounds-42.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/Warray-bounds-42.c	(working copy)
@@ -0,0 +1,26 @@
+/* PR tree-optimization/89720 - Spurious -Warray-bounds warning on a range
+   with max < min
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+void f (char*, int);
+
+#if __SIZEOF_POINTER__ == 8
+
+void g (__INT64_TYPE__ i)
+{
+  char a[65536] = "";
+  char *p = a + (i & (__INT64_TYPE__)0x3fffLL);
+  f (p, *(p - 6));/* { dg-bogus "\\\[-Warray-bounds" } */
+}
+
+#elif __SIZEOF_POINTER__ == 4
+
+void h (__INT32_TYPE__ i)
+{
+  char a[65536] = "";
+  char *p = a + (i & (__INT32_TYPE__)0x8fffLL);
+  f (p, *(p - 6));/* { dg-bogus "\\\[-Warray-bounds" } */
+}
+
+#endif