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 <ja...@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 <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
