Re: [PATCH] Strip location wrappers inside of inchash::add_expr (PR c++/89709)

[Richard Biener <rguenther at suse dot de>]

On Thu, 14 Mar 2019, Jakub Jelinek wrote:

> Hi!
> 
> As mentioned in the PR, r267272 added STRIP_ANY_LOCATION_WRAPPERS
> at the start of operand_equal_p, but has not updated inchash::add_expr
> which needs to follow what operand_equal_p does, so that if two trees
> compare equal, they get the same hash value.
> For location wrappers at the outermost level we wouldn't even try to
> verify it, as they would be stripped before checking the hashes,
> and in the usual case it wouldn't make a difference because if not
> OEP_ADDRESS_OF STRIP_NOPS strips also the location wrappers.
> But if OEP_ADDRESS_OF is set and we have e.g. a location wrapper in one
> of the operands and not in the other one (the ICE is on
> <ADDR_EXPR <COMPONENT_REF <location_wrapper <VAR_DECL <x>>, field>>>
> vs.
> <ADDR_EXPR <COMPONENT_REF <VAR_DECL <x>, field>>>
> which compares equal but has different hashes), then we need to make sure
> they have the same hash.
> 
> The following patch fixes that and also moves the operand_equal_p location
> wrapper stripping after the hash value computation code to check the hashes
> in all cases.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2019-03-14  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/89709
> 	* tree.c (inchash::add_expr): Strip any location wrappers.
> 	* fold-const.c (operand_equal_p): Move stripping of location wrapper
> 	after hash verification.
> 
> 	* g++.dg/cpp0x/constexpr-89709.C: New test.
> 
> --- gcc/tree.c.jj	2019-03-11 22:56:45.511835239 +0100
> +++ gcc/tree.c	2019-03-14 11:39:55.898838708 +0100
> @@ -7743,6 +7743,8 @@ add_expr (const_tree t, inchash::hash &h
>        return;
>      }
>  
> +  STRIP_ANY_LOCATION_WRAPPER (t);
> +
>    if (!(flags & OEP_ADDRESS_OF))
>      STRIP_NOPS (t);
>  
> --- gcc/fold-const.c.jj	2019-03-05 10:03:09.818780210 +0100
> +++ gcc/fold-const.c	2019-03-14 11:47:12.117791365 +0100
> @@ -2942,9 +2942,6 @@ combine_comparisons (location_t loc,
>  int
>  operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
>  {
> -  STRIP_ANY_LOCATION_WRAPPER (arg0);
> -  STRIP_ANY_LOCATION_WRAPPER (arg1);
> -
>    /* When checking, verify at the outermost operand_equal_p call that
>       if operand_equal_p returns non-zero then ARG0 and ARG1 has the same
>       hash value.  */
> @@ -2967,6 +2964,9 @@ operand_equal_p (const_tree arg0, const_
>  	return 0;
>      }
>  
> +  STRIP_ANY_LOCATION_WRAPPER (arg0);
> +  STRIP_ANY_LOCATION_WRAPPER (arg1);
> +
>    /* If either is ERROR_MARK, they aren't equal.  */
>    if (TREE_CODE (arg0) == ERROR_MARK || TREE_CODE (arg1) == ERROR_MARK
>        || TREE_TYPE (arg0) == error_mark_node
> --- gcc/testsuite/g++.dg/cpp0x/constexpr-89709.C.jj	2019-03-14 11:41:28.270346813 +0100
> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-89709.C	2019-03-14 11:40:59.161816938 +0100
> @@ -0,0 +1,18 @@
> +// PR c++/89709
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-O" }
> +
> +struct A { int i; };
> +A a;
> +
> +constexpr int *
> +foo ()
> +{
> +  return &a.i;
> +}
> +
> +bool
> +bar ()
> +{
> +  return foo () == &a.i;
> +}
> 
> 
> 	Jakub
> 

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