diff mbox series

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

Message ID 20200903082041.GI18149@tucnak
State New
Headers show
Series c++: Fix another PCH hash_map issue [PR96901] | expand

Commit Message

Jakub Jelinek Sept. 3, 2020, 8:20 a.m. UTC
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)?

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 *.


	Jakub

Comments

Jason Merrill Sept. 3, 2020, 6:29 p.m. UTC | #1
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
>
diff mbox series

Patch

--- 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