Re: [PATCH] vrp_prop::check_array_ref fixes (PR tree-optimization/83044)

[Richard Biener <rguenther at suse dot de>]

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
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)