[Wasm GC] Fix GlobalStructInference reasoning on unoptimizability (#5381)

We have a data structure there, typeGlobals, which maps types to the list of
globals for that type. Previously we used the convention of not having an
entry in the map to mean that a type is unoptimizable. However, this was not
used consistently, and in fact one place could insert to the map in a dangerous
way: a subtype's global is added to the list of globals of the super, and
typeGlobals[super].add(sub-global) would then effectively make an unoptimizable
super into an optimizable one.

To fix that, check for unoptimizability before propagating sub-globals. We do
still use the convention of not keeping data in typeGlobals for unoptimizable
things as it is a minor optimization to avoid wasted work.

Fixes #5379 

1 files changed, 18 insertions, 4 deletions

diff --git a/src/passes/GlobalStructInference.cpp b/src/passes/GlobalStructInference.cpp
index 50eddfd98..1444f8ad8 100644
--- a/src/passes/GlobalStructInference.cpp
+++ b/src/passes/GlobalStructInference.cpp
@@ -64,7 +64,10 @@ struct GlobalStructInference : public Pass {
   bool requiresNonNullableLocalFixups() override { return false; }
 
   // Maps optimizable struct types to the globals whose init is a struct.new of
-  // them. If a global is not present here, it cannot be optimized.
+  // them.
+  //
+  // We will remove unoptimizable types from here, so in practice, if a type is
+  // optimizable it will have an entry here, and not if not.
   std::unordered_map<HeapType, std::vector<Name>> typeGlobals;
 
   void run(Module* module) override {
@@ -152,7 +155,13 @@ struct GlobalStructInference : public Pass {
     //       fields)
     for (auto type : unoptimizable) {
       while (1) {
+        unoptimizable.insert(type);
+
+        // Also erase the globals, as we will never read them anyhow. This can
+        // allow us to skip unneeded work, when we check if typeGlobals is
+        // empty, below.
         typeGlobals.erase(type);
+
         auto super = type.getSuperType();
         if (!super) {
           break;
@@ -172,8 +181,12 @@ struct GlobalStructInference : public Pass {
           break;
         }
         curr = *super;
-        for (auto global : globals) {
-          typeGlobals[curr].push_back(global);
+
+        // As above, avoid adding pointless data for anything unoptimizable.
+        if (!unoptimizable.count(curr)) {
+          for (auto global : globals) {
+            typeGlobals[curr].push_back(global);
+          }
         }
       }
     }
@@ -209,7 +222,8 @@ struct GlobalStructInference : public Pass {
 
         // We must ignore the case of a non-struct heap type, that is, a bottom
         // type (which is all that is left after we've already ruled out
-        // unreachable).
+        // unreachable). Such things will not be in typeGlobals, which we are
+        // checking now anyhow.
         auto heapType = type.getHeapType();
         auto iter = parent.typeGlobals.find(heapType);
         if (iter == parent.typeGlobals.end()) {