From 6e885ad3287388192e52e9b524dbaa408507c0a4 Mon Sep 17 00:00:00 2001 From: Nathan Sidwell Date: Fri, 12 Mar 2021 09:02:41 -0800 Subject: [PATCH] c++: ICE with using-decl [PR 99238] This ICE was caused by a stray TREE_VISITED marker. The lookup machinery was leaving it there due to the way I'd arranged for it to be cleared. That was presuming the name_lookup::value field didn't change, and that wasn't always true in the using-decl processing. I took the opportunity to break out a helper, and then call it immediately after lookups, rather than wait until destructor time. Added some asserts the module machinery to catch further cases of this. PR c++/99238 gcc/cp/ * module.cc (depset::hash::add_binding_entity): Assert not visited. (depset::add::add_specializations): Likewise. * name-lookup.c (name_lookup::dedup): New. (name_lookup::~name_lookup): Assert not deduping. (name_lookup::restore_state): Likewise. (name_lookup::add_overload): Replace outlined code with dedup call. (name_lookup::add_value): Likewise. (name_lookup::search_namespace_only): Likewise. (name_lookup::adl_namespace_fns): Likewise. (name_lookup::adl_class_fns): Likewise. (name_lookup::search_adl): Likewise. Add clearing dedup call. (name_lookup::search_qualified): Likewise. (name_lookup::search_unqualified): Likewise. gcc/testsuite/ * g++.dg/modules/pr99238.h: New. * g++.dg/modules/pr99238_a.H: New. * g++.dg/modules/pr99238_b.H: New. --- gcc/cp/module.cc | 5 ++ gcc/cp/name-lookup.c | 79 ++++++++++-------------- gcc/testsuite/g++.dg/modules/pr99238.h | 1 + gcc/testsuite/g++.dg/modules/pr99238_a.H | 4 ++ gcc/testsuite/g++.dg/modules/pr99238_b.H | 8 +++ 5 files changed, 52 insertions(+), 45 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/pr99238.h create mode 100644 gcc/testsuite/g++.dg/modules/pr99238_a.H create mode 100644 gcc/testsuite/g++.dg/modules/pr99238_b.H diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 03359db28e18..19bdfc7cb218 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -12706,6 +12706,9 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_) *slot = data->binding; } + /* Make sure nobody left a tree visited lying about. */ + gcc_checking_assert (!TREE_VISITED (decl)); + if (flags & WMB_Using) { decl = ovl_make (decl, NULL_TREE); @@ -13000,6 +13003,8 @@ depset::hash::add_specializations (bool decl_p) have_spec:; #endif + /* Make sure nobody left a tree visited lying about. */ + gcc_checking_assert (!TREE_VISITED (spec)); depset *dep = make_dependency (spec, depset::EK_SPECIALIZATION); if (dep->is_special ()) { diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index d8839e29fe57..9382a47e1954 100644 --- a/gcc/cp/name-lookup.c +++ b/gcc/cp/name-lookup.c @@ -464,6 +464,7 @@ public: } ~name_lookup () { + gcc_checking_assert (!deduping); restore_state (); } @@ -471,6 +472,17 @@ private: /* Uncopyable, unmovable, unassignable. I am a rock. */ name_lookup (const name_lookup &); name_lookup &operator= (const name_lookup &); + public: + /* Turn on or off deduping mode. */ + void dedup (bool state) + { + if (deduping != state) + { + deduping = state; + lookup_mark (value, state); + } + } + protected: static bool seen_p (tree scope) { @@ -605,8 +617,7 @@ name_lookup::preserve_state () void name_lookup::restore_state () { - if (deduping) - lookup_mark (value, false); + gcc_checking_assert (!deduping); /* Unmark and empty this lookup's scope stack. */ for (unsigned ix = vec_safe_length (scopes); ix--;) @@ -703,12 +714,9 @@ name_lookup::add_overload (tree fns) probe = ovl_skip_hidden (probe); if (probe && TREE_CODE (probe) == OVERLOAD && OVL_DEDUP_P (probe)) - { - /* We're about to add something found by multiple paths, so - need to engage deduping mode. */ - lookup_mark (value, true); - deduping = true; - } + /* We're about to add something found by multiple paths, so need to + engage deduping mode. */ + dedup (true); } value = lookup_maybe_add (fns, value, deduping); @@ -737,12 +745,8 @@ name_lookup::add_value (tree new_val) value = ORIGINAL_NAMESPACE (value); else { - if (deduping) - { - /* Disengage deduping mode. */ - lookup_mark (value, false); - deduping = false; - } + /* Disengage deduping mode. */ + dedup (false); value = ambiguous (new_val, value); } } @@ -951,10 +955,7 @@ name_lookup::search_namespace_only (tree scope) if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val)) || (hit & 2 && BINDING_VECTOR_PARTITION_DUPS_P (val))) - { - lookup_mark (value, true); - deduping = true; - } + dedup (true); } dup_detect |= dup; } @@ -1076,6 +1077,8 @@ name_lookup::search_qualified (tree scope, bool usings) found = search_usings (scope); } + dedup (false); + return found; } @@ -1177,6 +1180,8 @@ name_lookup::search_unqualified (tree scope, cp_binding_level *level) break; } + dedup (false); + /* Restore to incoming length. */ vec_safe_truncate (queue, length); @@ -1284,15 +1289,10 @@ name_lookup::adl_namespace_fns (tree scope, bitmap imports) else if (MODULE_BINDING_PARTITION_P (bind)) dup = 2; if (unsigned hit = dup_detect & dup) - { - if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val)) - || (hit & 2 - && BINDING_VECTOR_PARTITION_DUPS_P (val))) - { - lookup_mark (value, true); - deduping = true; - } - } + if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val)) + || (hit & 2 + && BINDING_VECTOR_PARTITION_DUPS_P (val))) + dedup (true); dup_detect |= dup; } @@ -1328,11 +1328,7 @@ name_lookup::adl_class_fns (tree type) if (CP_DECL_CONTEXT (fn) != context) continue; - if (!deduping) - { - lookup_mark (value, true); - deduping = true; - } + dedup (true); /* Template specializations are never found by name lookup. (Templates themselves can be found, but not template @@ -1634,12 +1630,9 @@ name_lookup::search_adl (tree fns, vec *args) if (vec_safe_length (scopes)) { /* Now do the lookups. */ - if (fns) - { - deduping = true; - lookup_mark (fns, true); - } value = fns; + if (fns) + dedup (true); /* INST_PATH will be NULL, if this is /not/ 2nd-phase ADL. */ bitmap inst_path = NULL; @@ -1697,14 +1690,9 @@ name_lookup::search_adl (tree fns, vec *args) if (tree bind = *mslot) { - if (!deduping) - { - /* We must turn on deduping, because some - other class from this module might also - be in this namespace. */ - deduping = true; - lookup_mark (value, true); - } + /* We must turn on deduping, because some other class + from this module might also be in this namespace. */ + dedup (true); /* Add the exported fns */ if (STAT_HACK_P (bind)) @@ -1715,6 +1703,7 @@ name_lookup::search_adl (tree fns, vec *args) } fns = value; + dedup (false); } return fns; diff --git a/gcc/testsuite/g++.dg/modules/pr99238.h b/gcc/testsuite/g++.dg/modules/pr99238.h new file mode 100644 index 000000000000..312641f312c4 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99238.h @@ -0,0 +1 @@ +struct tm; diff --git a/gcc/testsuite/g++.dg/modules/pr99238_a.H b/gcc/testsuite/g++.dg/modules/pr99238_a.H new file mode 100644 index 000000000000..b594c09ec586 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99238_a.H @@ -0,0 +1,4 @@ +// PR 99238 ICE with using decl +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } +#include "pr99238.h" diff --git a/gcc/testsuite/g++.dg/modules/pr99238_b.H b/gcc/testsuite/g++.dg/modules/pr99238_b.H new file mode 100644 index 000000000000..070ee36f95ff --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99238_b.H @@ -0,0 +1,8 @@ +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } +#include "pr99238.h" +import "pr99238_a.H"; +namespace std +{ + using ::tm; +}