vrp_prop::check_array_ref fixes (PR tree-optimization/83044)

Hi!

On the following testcase we ICE, because eltsize is 0 and when we
int_const_binop (TRUNC_DIV_EXPR, maxbound, size_int (0)), that of course
returns NULL.
The upper bound for zero sized elements is just not meaningful, if we use
any index we still won't reach maximum object size that way.

While looking at that, I've discovered a couple of other issues though.

One is that the off subtraction seems misplaced.
get_addr_base_and_unit_offset computes an offset in bytes, so it corresponds
to the __PTRDIFF_MAX__ limit on object size (also in bytes), not to the
__PTRDIFF_MAX__ limit divided by element size.
So I believe we first want to subtract off (and only if it is positive, we
do not want to enlarge the limit) from __PTRDIFF_MAX__ and then divide
rather than what the code was doing before.

Another thing is that the code has been mixing up types randomly, something
being in ptrdiff_type_node, something in sizetype.

And yet another thing is that even if we can't compute any sensible upper
bound (e.g. the eltsize 0 case; I believe the VLA case just won't happen
here, because we replace the VLAs with dereferences of a pointer I believe
the ARRAY_REFs should have been transformed into POINTER_PLUS anyway), we
can still warn about indexes smaller than low bound (especially if the low
bound is not zero like in Fortran).

The Warray-bounds.c testcase needed adjustment, because it was written with
the same bad computation of the maximum - __PTRDIFF_MAX__ / sizeof (elt) -
offset rather than (__PTRDIFF_MAX__ - offset) / sizeof (elt).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-11-22  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/83044
	* tree-vrp.c (vrp_prop::check_array_ref): If eltsize is not
	INTEGER_CST or is 0, clear up_bound{,_p1} and later ignore tests
	that need the upper bound.  Subtract offset from
	get_addr_base_and_unit_offset only if positive and subtract it
	before division by eltsize rather than after it.

	* gcc.dg/pr83044.c: New test.
	* c-c++-common/Warray-bounds.c (fb): Fix up MAX value.


	Jakub

On Wed, 22 Nov 2017, Jakub Jelinek wrote:

> Hi!
> 
> On the following testcase we ICE, because eltsize is 0 and when we
> int_const_binop (TRUNC_DIV_EXPR, maxbound, size_int (0)), that of course
> returns NULL.
> The upper bound for zero sized elements is just not meaningful, if we use
> any index we still won't reach maximum object size that way.
> 
> While looking at that, I've discovered a couple of other issues though.
> 
> One is that the off subtraction seems misplaced.
> get_addr_base_and_unit_offset computes an offset in bytes, so it corresponds
> to the __PTRDIFF_MAX__ limit on object size (also in bytes), not to the
> __PTRDIFF_MAX__ limit divided by element size.
> So I believe we first want to subtract off (and only if it is positive, we
> do not want to enlarge the limit) from __PTRDIFF_MAX__ and then divide
> rather than what the code was doing before.
> 
> Another thing is that the code has been mixing up types randomly, something
> being in ptrdiff_type_node, something in sizetype.
> 
> And yet another thing is that even if we can't compute any sensible upper
> bound (e.g. the eltsize 0 case; I believe the VLA case just won't happen
> here, because we replace the VLAs with dereferences of a pointer I believe
> the ARRAY_REFs should have been transformed into POINTER_PLUS anyway), we
> can still warn about indexes smaller than low bound (especially if the low
> bound is not zero like in Fortran).
> 
> The Warray-bounds.c testcase needed adjustment, because it was written with
> the same bad computation of the maximum - __PTRDIFF_MAX__ / sizeof (elt) -
> offset rather than (__PTRDIFF_MAX__ - offset) / sizeof (elt).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.  Thanks for cleaning this up - looks I got lazy during the lengthy
review process :/

Richard.

> 2017-11-22  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/83044
> 	* tree-vrp.c (vrp_prop::check_array_ref): If eltsize is not
> 	INTEGER_CST or is 0, clear up_bound{,_p1} and later ignore tests
> 	that need the upper bound.  Subtract offset from
> 	get_addr_base_and_unit_offset only if positive and subtract it
> 	before division by eltsize rather than after it.
> 
> 	* gcc.dg/pr83044.c: New test.
> 	* c-c++-common/Warray-bounds.c (fb): Fix up MAX value.
> 
> --- gcc/tree-vrp.c.jj	2017-11-17 08:40:28.000000000 +0100
> +++ gcc/tree-vrp.c	2017-11-21 14:13:47.184974061 +0100
> @@ -4795,24 +4795,30 @@ vrp_prop::check_array_ref (location_t lo
>  	 the size of the largest object is PTRDIFF_MAX.  */
>        tree eltsize = array_ref_element_size (ref);
>  
> -      /* FIXME: Handle VLAs.  */
> -      if (TREE_CODE (eltsize) != INTEGER_CST)
> -	return;
> -
> -      tree maxbound = TYPE_MAX_VALUE (ptrdiff_type_node);
> -
> -      up_bound_p1 = int_const_binop (TRUNC_DIV_EXPR, maxbound, eltsize);
> -
> -      tree arg = TREE_OPERAND (ref, 0);
> -
> -      HOST_WIDE_INT off;
> -      if (get_addr_base_and_unit_offset (arg, &off))
> -	up_bound_p1 = wide_int_to_tree (sizetype,
> -					wi::sub (wi::to_wide (up_bound_p1),
> -						 off));
> -
> -      up_bound = int_const_binop (MINUS_EXPR, up_bound_p1,
> -				  build_int_cst (ptrdiff_type_node, 1));
> +      if (TREE_CODE (eltsize) != INTEGER_CST
> +	  || integer_zerop (eltsize))
> +	{
> +	  up_bound = NULL_TREE;
> +	  up_bound_p1 = NULL_TREE;
> +	}
> +      else
> +	{
> +	  tree maxbound = TYPE_MAX_VALUE (ptrdiff_type_node);
> +	  tree arg = TREE_OPERAND (ref, 0);
> +	  HOST_WIDE_INT off;
> +
> +	  if (get_addr_base_and_unit_offset (arg, &off) && off > 0)
> +	    maxbound = wide_int_to_tree (sizetype,
> +					 wi::sub (wi::to_wide (maxbound),
> +						  off));
> +	  else
> +	    maxbound = fold_convert (sizetype, maxbound);
> +
> +	  up_bound_p1 = int_const_binop (TRUNC_DIV_EXPR, maxbound, eltsize);
> +
> +	  up_bound = int_const_binop (MINUS_EXPR, up_bound_p1,
> +				      build_int_cst (ptrdiff_type_node, 1));
> +	}
>      }
>    else
>      up_bound_p1 = int_const_binop (PLUS_EXPR, up_bound,
> @@ -4823,7 +4829,7 @@ vrp_prop::check_array_ref (location_t lo
>    tree artype = TREE_TYPE (TREE_OPERAND (ref, 0));
>  
>    /* Empty array.  */
> -  if (tree_int_cst_equal (low_bound, up_bound_p1))
> +  if (up_bound && tree_int_cst_equal (low_bound, up_bound_p1))
>      {
>        warning_at (location, OPT_Warray_bounds,
>  		  "array subscript %E is above array bounds of %qT",
> @@ -4843,7 +4849,8 @@ vrp_prop::check_array_ref (location_t lo
>  
>    if (vr && vr->type == VR_ANTI_RANGE)
>      {
> -      if (TREE_CODE (up_sub) == INTEGER_CST
> +      if (up_bound
> +	  && TREE_CODE (up_sub) == INTEGER_CST
>            && (ignore_off_by_one
>  	      ? tree_int_cst_lt (up_bound, up_sub)
>  	      : tree_int_cst_le (up_bound, up_sub))
> @@ -4856,7 +4863,8 @@ vrp_prop::check_array_ref (location_t lo
>            TREE_NO_WARNING (ref) = 1;
>          }
>      }
> -  else if (TREE_CODE (up_sub) == INTEGER_CST
> +  else if (up_bound
> +	   && TREE_CODE (up_sub) == INTEGER_CST
>  	   && (ignore_off_by_one
>  	       ? !tree_int_cst_le (up_sub, up_bound_p1)
>  	       : !tree_int_cst_le (up_sub, up_bound)))
> --- gcc/testsuite/gcc.dg/pr83044.c.jj	2017-11-21 14:15:29.221696588 +0100
> +++ gcc/testsuite/gcc.dg/pr83044.c	2017-11-21 13:27:58.000000000 +0100
> @@ -0,0 +1,14 @@
> +/* PR tree-optimization/83044 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wall -std=gnu89 -O2" } */
> +
> +struct A { int b[0]; };
> +struct B { struct A c[0]; };
> +void bar (int *);
> +
> +void
> +foo (void)
> +{
> +  struct B d;
> +  bar (d.c->b);
> +}
> --- gcc/testsuite/c-c++-common/Warray-bounds.c.jj	2017-11-17 08:40:32.000000000 +0100
> +++ gcc/testsuite/c-c++-common/Warray-bounds.c	2017-11-22 11:30:29.047189568 +0100
> @@ -200,7 +200,7 @@ void fb (struct B *p)
>  
>    T (p->a1x[9].a1[0]);
>  
> -  enum { MAX = DIFF_MAX / sizeof *p->a1x - sizeof *p };
> +  enum { MAX = (DIFF_MAX - sizeof *p) / sizeof *p->a1x };
>  
>    T (p->a1x[DIFF_MIN].a1);                /* { dg-warning "array subscript -\[0-9\]+ is below array bounds" } */
>    T (p->a1x[-1].a1);                      /* { dg-warning "array subscript -1 is below array bounds" } */
> 
> 	Jakub
> 
>