c++: ICE with using-decl [PR 99238]
authorNathan Sidwell <nathan@acm.org>
Fri, 12 Mar 2021 17:02:41 +0000 (09:02 -0800)
committerNathan Sidwell <nathan@acm.org>
Fri, 12 Mar 2021 18:23:05 +0000 (10:23 -0800)
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
gcc/cp/name-lookup.c
gcc/testsuite/g++.dg/modules/pr99238.h [new file with mode: 0644]
gcc/testsuite/g++.dg/modules/pr99238_a.H [new file with mode: 0644]
gcc/testsuite/g++.dg/modules/pr99238_b.H [new file with mode: 0644]

index 03359db..19bdfc7 100644 (file)
@@ -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 ())
        {
index d8839e2..9382a47 100644 (file)
@@ -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<tree, va_gc> *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<tree, va_gc> *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<tree, va_gc> *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 (file)
index 0000000..312641f
--- /dev/null
@@ -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 (file)
index 0000000..b594c09
--- /dev/null
@@ -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 (file)
index 0000000..070ee36
--- /dev/null
@@ -0,0 +1,8 @@
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+#include "pr99238.h"
+import "pr99238_a.H";
+namespace std
+{
+  using ::tm;
+}