Re: [PATCH] Remove invalid pointer BIT_AND_EXPR vrp optimization (PR tree-optimization/91597)

[Richard Biener <rguenther at suse dot de>]

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  <jakub@redhat.com>
> 	    Richard Biener  <rguenther@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 <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 247165 (AG München)