On Tue, 3 Sep 2019, Jakub Jelinek wrote:
> Hi!
>
> As discussed in the PR, this optimization introduced in r161707
> is invalid, even when neither of the vrs include NULL, the result
> can still be NULL. While for usual uses (where the second argument
> is INTEGER_CST with all bits set except a few last ones and first argument
> is a pointer to real object) it is true, if one of the argument is a made up
> pointer (say (void *) 1), or the other argument some weird mask as in this
> testcase (1), we still get a NULL pointer from the operation.
>
> Dunno if we could say use points-to info to check that one of the arguments
> is a real object pointer and have some limited set of pointer realignments
> we'd say result in non-NULL pointers.
>
> Anyway, this patch just removes the optimization, bootstrapped/regtested on
> x86_64-linux and i686-linux, ok for trunk and 9.x (the testcase is
> miscompiled only in 9.x)?
OK for trunk and all branches you think it's worth pushing to.
Richard.
> 2019-09-03 Jakub Jelinek <ja...@redhat.com>
> Richard Biener <rguent...@suse.de>
>
> PR tree-optimization/91597
> * tree-vrp.c (extract_range_from_binary_expr): Remove unsafe
> BIT_AND_EXPR optimization for pointers, even if both operand
> ranges don't include NULL, the result can be NULL.
>
> * gcc.c-torture/execute/pr91597.c: New test.
>
> --- gcc/tree-vrp.c.jj 2019-08-12 09:32:17.811773387 +0200
> +++ gcc/tree-vrp.c 2019-09-02 19:14:14.276914085 +0200
> @@ -1741,9 +1741,7 @@ extract_range_from_binary_expr (value_ra
> {
> /* For pointer types, we are really only interested in asserting
> whether the expression evaluates to non-NULL. */
> - if (!range_includes_zero_p (&vr0) && !range_includes_zero_p (&vr1))
> - vr->set_nonzero (expr_type);
> - else if (vr0.zero_p () || vr1.zero_p ())
> + if (vr0.zero_p () || vr1.zero_p ())
> vr->set_zero (expr_type);
> else
> vr->set_varying (expr_type);
> --- gcc/testsuite/gcc.c-torture/execute/pr91597.c.jj 2019-09-02
> 19:13:07.746915030 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/pr91597.c 2019-09-02
> 19:03:32.147575358 +0200
> @@ -0,0 +1,48 @@
> +/* PR tree-optimization/91597 */
> +
> +enum E { A, B, C };
> +struct __attribute__((aligned (4))) S { enum E e; };
> +
> +enum E
> +foo (struct S *o)
> +{
> + if (((__UINTPTR_TYPE__) (o) & 1) == 0)
> + return o->e;
> + else
> + return A;
> +}
> +
> +int
> +bar (struct S *o)
> +{
> + return foo (o) == B || foo (o) == C;
> +}
> +
> +static inline void
> +baz (struct S *o, int d)
> +{
> + if (__builtin_expect (!bar (o), 0))
> + __builtin_abort ();
> + if (d > 2) return;
> + baz (o, d + 1);
> +}
> +
> +void
> +qux (struct S *o)
> +{
> + switch (o->e)
> + {
> + case A: return;
> + case B: baz (o, 0); break;
> + case C: baz (o, 0); break;
> + }
> +}
> +
> +struct S s = { C };
> +
> +int
> +main ()
> +{
> + qux (&s);
> + return 0;
> +}
>
> Jakub
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 247165 (AG München)
