Re: [PATCH] c++: Fix another PCH hash_map issue [PR96901]

[Jason Merrill <jason@redhat.com>]

On 9/3/20 4:20 AM, Jakub Jelinek wrote:
> Hi!
> 
> The recent libstdc++ changes caused lots of libstdc++-v3 tests FAILs
> on i686-linux, all of them in the same spot during constexpr evaluation
> of a recursive _S_gcd call.
> The problem is yet another hash_map that used the default hasing of
> tree keys through pointer hashing which is preserved across PCH write/read.
> During PCH handling, the addresses of GC objects are changed, which means
> that the hash values of the keys in such hash tables change without those
> hash tables being rehashed.  Which in the fundef_copies_table case usually
> means we just don't find a copy of a FUNCTION_DECL body for recursive uses
> and start from scratch.  But when the hash table keeps growing, the "dead"
> elements in the hash table can sometimes reappear and break things.
> In particular what I saw under the debugger is when the fundef_copies_table
> hash map has been used on the outer _S_gcd call, it didn't find an entry for
> it, so returned a slot with *slot == NULL, which is treated as that the
> function itself is used directly (i.e. no recursion), but that addition of
> a hash table slot caused the recursive _S_gcd call to actually find
> something in the hash table, unfortunately not the new *slot == NULL spot,
> but a different one from the pre-PCH streaming which contained the returned
> toplevel (non-recursive) call entry for it, which means that for the
> recursive _S_gcd call we actually used the same trees as for the outer ones
> rather than a copy of those, which breaks constexpr evaluation.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
> ok for trunk (and given that it is latent eventually for release branches
> too)?

OK.

> 2020-09-03  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/96901
> 	* tree.h (struct decl_tree_traits): New type.
> 	(decl_tree_map): New typedef.
> 
> 	* constexpr.c (fundef_copies_table): Change type from
> 	hash_map<tree, tree> * to decl_tree_map *.
> 
> --- gcc/tree.h.jj	2020-08-31 22:50:28.545593391 +0200
> +++ gcc/tree.h	2020-09-02 17:44:45.478358927 +0200
> @@ -5453,6 +5453,11 @@ struct type_tree_cache_traits
>     : simple_cache_map_traits<tree_type_hash, tree> { };
>   typedef hash_map<tree,tree,type_tree_cache_traits> type_tree_cache_map;
>   
> +/* Similarly to decl_tree_cache_map, but without caching.  */
> +struct decl_tree_traits
> +  : simple_hashmap_traits<tree_decl_hash, tree> { };
> +typedef hash_map<tree,tree,decl_tree_traits> decl_tree_map;
> +
>   /* Initialize the abstract argument list iterator object ITER with the
>      arguments from CALL_EXPR node EXP.  */
>   static inline void
> --- gcc/cp/constexpr.c.jj	2020-09-01 22:47:49.946383670 +0200
> +++ gcc/cp/constexpr.c	2020-09-02 17:46:35.267758742 +0200
> @@ -1203,7 +1203,7 @@ maybe_initialize_constexpr_call_table (v
>   
>      This is not GC-deletable to avoid GC affecting UID generation.  */
>   
> -static GTY(()) hash_map<tree, tree> *fundef_copies_table;
> +static GTY(()) decl_tree_map *fundef_copies_table;
>   
>   /* Reuse a copy or create a new unshared copy of the function FUN.
>      Return this copy.  We use a TREE_LIST whose PURPOSE is body, VALUE
> 
> 	Jakub
>