From ec2365e07537e8b17745d75c28a2b45bf33be119 Mon Sep 17 00:00:00 2001 From: Nathaniel Shead Date: Fri, 3 May 2024 19:36:17 +1000 Subject: [PATCH] c++/modules: Fix dangling pointer with imported_temploid_friends I got notified by Linaro CI and by checking testresults that there seems to be some occasional failures in tpl-friend-4_b.C on some architectures and standards modes since r15-59-gb5f6a56940e708. I haven't been able to reproduce but looking at the backtrace I suspect the issue is that we're adding to the 'imported_temploid_friend' map a decl that is ultimately discarded, which then has its address reused by a later decl causing a failure in the assert in 'set_originating_module'. This patch fixes the problem by ensuring 'imported_temploid_friends' is correctly marked as a GTY root, and that 'duplicate_decls' properly removes entries from the map for declarations that it frees. PR c++/114275 gcc/cp/ChangeLog: * cp-tree.h (remove_defining_module): Declare. * decl.cc (duplicate_decls): Call remove_defining_module on to-be-freed newdecl. * module.cc (imported_temploid_friends): Mark as GTY root... (init_modules): ...and allocate from ggc. (trees_in::decl_value): Only track for declarations that won't be discarded. (remove_defining_module): New function. Signed-off-by: Nathaniel Shead Reviewed-by: Jason Merrill Reviewed-by: Patrick Palka --- gcc/cp/cp-tree.h | 1 + gcc/cp/decl.cc | 4 ++++ gcc/cp/module.cc | 19 ++++++++++++++++--- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 52d6841559c..4fadc9aaf48 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7420,6 +7420,7 @@ extern void set_instantiating_module (tree); extern void set_defining_module (tree); extern void maybe_key_decl (tree ctx, tree decl); extern void propagate_defining_module (tree decl, tree orig); +extern void remove_defining_module (tree decl); extern void mangle_module (int m, bool include_partition); extern void mangle_module_fini (); diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 04a151c341c..b112b70659f 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -3340,6 +3340,10 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden) if (flag_concepts) remove_constraints (newdecl); + /* And similarly for any module tracking data. */ + if (modules_p ()) + remove_defining_module (newdecl); + ggc_free (newdecl); return olddecl; diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 44dc81eed3e..520dd710549 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -2731,7 +2731,7 @@ static keyed_map_t *keyed_table; need to be attached to the same module as the temploid. This maps these decls to the temploid they are instantiated them, as there is no other easy way to get this information. */ -static hash_map *imported_temploid_friends; +static GTY((cache)) decl_tree_cache_map *imported_temploid_friends; /********************************************************************/ /* Tree streaming. The tree streaming is very specific to the tree @@ -8327,7 +8327,8 @@ trees_in::decl_value () if (TREE_CODE (inner) == FUNCTION_DECL || TREE_CODE (inner) == TYPE_DECL) if (tree owner = tree_node ()) - imported_temploid_friends->put (decl, owner); + if (is_new) + imported_temploid_friends->put (decl, owner); /* Regular typedefs will have a NULL TREE_TYPE at this point. */ unsigned tdef_flags = 0; @@ -19336,6 +19337,18 @@ propagate_defining_module (tree decl, tree orig) } } +/* DECL is being freed, clear data we don't need anymore. */ + +void +remove_defining_module (tree decl) +{ + if (!modules_p ()) + return; + + if (imported_temploid_friends) + imported_temploid_friends->remove (decl); +} + /* Create the flat name string. It is simplest to have it handy. */ void @@ -20550,7 +20563,7 @@ init_modules (cpp_reader *reader) entity_map = new entity_map_t (EXPERIMENT (1, 400)); vec_safe_reserve (entity_ary, EXPERIMENT (1, 400)); imported_temploid_friends - = new hash_map (EXPERIMENT (1, 400)); + = decl_tree_cache_map::create_ggc (EXPERIMENT (1, 400)); } #if CHECKING_P -- 2.11.4.GIT