c++: Fix unordered entity array [PR 98843]
authorNathan Sidwell <nathan@acm.org>
Fri, 29 Jan 2021 16:30:40 +0000 (08:30 -0800)
committerNathan Sidwell <nathan@acm.org>
Fri, 29 Jan 2021 17:11:46 +0000 (09:11 -0800)
A couple of module invariants are that the modules are always
allocated in ascending order and appended to the module array.  The
entity array is likewise ordered, with each module having spans in
that array in ascending order.  Prior to header-units, this was
provided by the way import declarations were encountered.  With
header-units we need to load the preprocessor state of header units
before we parse the C++, and this can lead to incorrect ordering of
the entity array.  I had made the initialization of a module's
language state a little too lazy.  This moves the allocation of entity
array spans into the initial read of a module, thus ensuring the
ordering of those spans.  We won't be looking in them until we've
loaded the language portions of that particular module, and even if we
did, we'd find NULLs there and issue a diagnostic.

PR c++/98843
gcc/cp/
* module.cc (module_state_config): Add num_entities field.
(module_state::read_entities): The entity_ary span is
already allocated.
(module_state::write_config): Write num_entities.
(module_state::read_config): Read num_entities.
(module_state::write): Set config's num_entities.
(module_state::read_initial): Allocate the entity ary
span here.
(module_state::read_language): Do not set entity_lwm
here.
gcc/testsuite/
* g++.dg/modules/pr98843_a.C: New.
* g++.dg/modules/pr98843_b.H: New.
* g++.dg/modules/pr98843_c.C: New.

gcc/cp/module.cc
gcc/testsuite/g++.dg/modules/pr98843_a.C [new file with mode: 0644]
gcc/testsuite/g++.dg/modules/pr98843_b.H [new file with mode: 0644]
gcc/testsuite/g++.dg/modules/pr98843_c.C [new file with mode: 0644]

index daf75b1600726adbf899c6afded65d4c90889e9b..2d761452505db80b709954d70cf115b083fcc49b 100644 (file)
@@ -4064,6 +4064,7 @@ import_entity_module (unsigned index)
     /* This is an index for an exported entity.  */
     return (*modules)[0];
 
+  /* Do not include the current TU (not an off-by-one error).  */
   unsigned pos = 1;
   unsigned len = modules->length () - pos;
   while (len)
@@ -14480,6 +14481,7 @@ struct module_state_config {
   const char *dialect_str;
   unsigned num_imports;
   unsigned num_partitions;
+  unsigned num_entities;
   unsigned ordinary_locs;
   unsigned macro_locs;
   unsigned ordinary_loc_align;
@@ -14487,7 +14489,7 @@ struct module_state_config {
 public:
   module_state_config ()
     :dialect_str (get_dialect ()),
-     num_imports (0), num_partitions (0),
+     num_imports (0), num_partitions (0), num_entities (0),
      ordinary_locs (0), macro_locs (0), ordinary_loc_align (0)
   {
   }
@@ -15286,9 +15288,7 @@ module_state::read_entities (unsigned count, unsigned lwm, unsigned hwm)
   dump () && dump ("Reading entities");
   dump.indent ();
 
-  vec_safe_reserve (entity_ary, count);
-  unsigned ix;
-  for (ix = 0; ix != count; ix++)
+  for (binding_slot *slot = entity_ary->begin () + entity_lwm; count--; slot++)
     {
       unsigned snum = sec.u ();
       if (snum && (snum - lwm) >= (hwm - lwm))
@@ -15296,13 +15296,9 @@ module_state::read_entities (unsigned count, unsigned lwm, unsigned hwm)
       if (sec.get_overrun ())
        break;
 
-      binding_slot slot;
-      slot.u.binding = NULL_TREE;
       if (snum)
-       slot.set_lazy (snum << 2);
-      entity_ary->quick_push (slot);
+       slot->set_lazy (snum << 2);
     }
-  entity_num = ix;
 
   dump.outdent ();
   if (!sec.end (from ()))
@@ -17301,6 +17297,7 @@ module_state::write_config (elf_out *to, module_state_config &config,
 
   cfg.u (config.num_imports);
   cfg.u (config.num_partitions);
+  cfg.u (config.num_entities);
 
   cfg.u (config.ordinary_locs);
   cfg.u (config.macro_locs);
@@ -17484,6 +17481,7 @@ module_state::read_config (module_state_config &config)
 
   config.num_imports = cfg.u ();
   config.num_partitions = cfg.u ();
+  config.num_entities = cfg.u ();
 
   config.ordinary_locs = cfg.u ();
   config.macro_locs = cfg.u ();
@@ -17717,6 +17715,7 @@ module_state::write (elf_out *to, cpp_reader *reader)
 
   /* Write the entitites.  None happens if we contain namespaces or
      nothing. */
+  config.num_entities = counts[MSC_entities];
   if (counts[MSC_entities])
     write_entities (to, sccs, counts[MSC_entities], &crc);
 
@@ -17818,6 +17817,21 @@ module_state::read_initial (cpp_reader *reader)
   gcc_checking_assert (mod == MODULE_UNKNOWN);
   gcc_checking_assert (this != (*modules)[0]);
 
+  {
+    /* Allocate space in the entities array now -- that array must be
+       monotionically in step with the modules array.  */
+    entity_lwm = vec_safe_length (entity_ary);
+    entity_num = config.num_entities;
+    gcc_checking_assert (modules->length () == 1
+                        || modules->last ()->entity_lwm <= entity_lwm);
+    vec_safe_reserve (entity_ary, config.num_entities);
+
+    binding_slot slot;
+    slot.u.binding = NULL_TREE;
+    for (unsigned count = config.num_entities; count--;)
+      entity_ary->quick_push (slot);
+  }
+
   /* We'll run out of other resources before we run out of module
      indices.  */
   mod = modules->length ();
@@ -17939,8 +17953,8 @@ module_state::read_language (bool outermost)
 
   function_depth++; /* Prevent unexpected GCs.  */
 
-  /* Read the entity table.  */
-  entity_lwm = vec_safe_length (entity_ary);
+  if (counts[MSC_entities] != entity_num)
+    ok = false;
   if (ok && counts[MSC_entities]
       && !read_entities (counts[MSC_entities],
                         counts[MSC_sec_lwm], counts[MSC_sec_hwm]))
diff --git a/gcc/testsuite/g++.dg/modules/pr98843_a.C b/gcc/testsuite/g++.dg/modules/pr98843_a.C
new file mode 100644 (file)
index 0000000..97820cf
--- /dev/null
@@ -0,0 +1,5 @@
+// { dg-additional-options -fmodules-ts }
+// PR 98843 ICE due to inconsistent entity_ary order
+
+export module foo;
+export void *frob ();
diff --git a/gcc/testsuite/g++.dg/modules/pr98843_b.H b/gcc/testsuite/g++.dg/modules/pr98843_b.H
new file mode 100644 (file)
index 0000000..d6734bd
--- /dev/null
@@ -0,0 +1,12 @@
+// { dg-additional-options -fmodule-header }
+// PR 98843 ICE due to inconsistent entity_ary order
+
+template<int I> int Fn () 
+{
+  return I;
+}
+
+template<> int Fn<1> () 
+{
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/modules/pr98843_c.C b/gcc/testsuite/g++.dg/modules/pr98843_c.C
new file mode 100644 (file)
index 0000000..a176e09
--- /dev/null
@@ -0,0 +1,10 @@
+// { dg-additional-options -fmodules-ts }
+// PR 98843 ICE due to inconsistent entity_ary order
+
+import foo;
+import "pr98843_b.H";
+
+int main ()
+{
+  return Fn<1> ();
+}