[Custom Descriptors] Fix GTO placeholders for unused types (#7908)

GTO emits placeholder describee types when it optimizes descriptor types
that must remain descriptors. However, it previously tried to emit a
placeholder describee type even for a descriptor type that is unused in
the module and therefore not included in the rebuilt types. These
dangling describees caused validation faillures when rebuilding the
types. Fix the problem by emitting placeholder describees only for
descriptors that will be rebuilt.

Author: tlively

src/passes/GlobalTypeOptimization.cpp

@@ -141,7 +141,7 @@ struct GlobalTypeOptimization : public Pass {
   // still need to be descriptors for their own subtypes and supertypes to be
   // valid. We will keep them descriptors by having them describe trivial new
   // placeholder types.
-  InsertOrderedMap<HeapType, Index> descriptorsOfPlaceholders;
+  std::vector<HeapType> descriptorsOfPlaceholders;
 
   void run(Module* module) override {
     if (!module->features.hasGC()) {
@@ -473,8 +473,7 @@ struct GlobalTypeOptimization : public Pass {
           auto desc = type.getDescribedType();
           assert(desc);
           if (haveUnneededDescriptors.count(*desc)) {
-            descriptorsOfPlaceholders.insert(
-              {type, descriptorsOfPlaceholders.size()});
+            descriptorsOfPlaceholders.push_back(type);
           }
         }
       }
@@ -498,20 +497,30 @@ struct GlobalTypeOptimization : public Pass {
   void updateTypes(Module& wasm) {
     class TypeRewriter : public GlobalTypeRewriter {
       GlobalTypeOptimization& parent;
+      InsertOrderedMap<HeapType, Index> placeholderIndices;
       InsertOrderedMap<HeapType, Index>::iterator placeholderIt;
 
     public:
       TypeRewriter(Module& wasm, GlobalTypeOptimization& parent)
         : GlobalTypeRewriter(wasm), parent(parent),
-          placeholderIt(parent.descriptorsOfPlaceholders.begin()) {}
+          placeholderIt(placeholderIndices.begin()) {}
 
       std::vector<HeapType> getSortedTypes(PredecessorGraph preds) override {
         auto types = GlobalTypeRewriter::getSortedTypes(std::move(preds));
+        // Some of the descriptors of placeholders may not end up being used at
+        // all, so we will not rebuild them. Record the used types that need
+        // placeholder describees and assign them placeholder indices.
+        std::unordered_set<HeapType> typeSet(types.begin(), types.end());
+        for (auto desc : parent.descriptorsOfPlaceholders) {
+          if (typeSet.count(desc)) {
+            placeholderIndices.insert({desc, placeholderIndices.size()});
+          }
+        }
+        placeholderIt = placeholderIndices.begin();
         // Prefix the types with placeholders to be overwritten with the
         // placeholder describees.
         HeapType placeholder = Struct{};
-        types.insert(
-          types.begin(), parent.descriptorsOfPlaceholders.size(), placeholder);
+        types.insert(types.begin(), placeholderIndices.size(), placeholder);
         return types;
       }
 
@@ -577,19 +586,20 @@ struct GlobalTypeOptimization : public Pass {
 
         // Until we've created all the placeholders, create a placeholder
         // describee type for the next descriptor that needs one.
-        if (placeholderIt != parent.descriptorsOfPlaceholders.end()) {
+        if (placeholderIt != placeholderIndices.end()) {
           auto descriptor = placeholderIt->first;
           typeBuilder[i].descriptor(getTempHeapType(descriptor));
           typeBuilder[i].setShared(descriptor.getShared());
+
           ++placeholderIt;
           return;
         }
 
         // Remove an unneeded describee or describe a placeholder type.
         if (auto described = oldType.getDescribedType()) {
           if (parent.haveUnneededDescriptors.count(*described)) {
-            if (auto it = parent.descriptorsOfPlaceholders.find(oldType);
-                it != parent.descriptorsOfPlaceholders.end()) {
+            if (auto it = placeholderIndices.find(oldType);
+                it != placeholderIndices.end()) {
               typeBuilder[i].describes(typeBuilder[it->second]);
             } else {
               typeBuilder[i].describes(std::nullopt);

test/lit/passes/gto-desc.wast

@@ -1173,8 +1173,33 @@
   )
 )
 
+;; Here we would optimize $unused with a placeholder, except that we don't
+;; include it in the output at all because it is unused. We should not get
+;; confused and try to emit a placeholder for it anyway.
+(module
+ (rec
+  ;; CHECK:      (rec
+  ;; CHECK-NEXT:  (type $public (sub (descriptor $public.desc (struct))))
+  (type $public (sub (descriptor $public.desc (struct))))
+  ;; CHECK:       (type $public.desc (sub (describes $public (struct))))
+  (type $public.desc (sub (describes $public (struct))))
+ )
+ (rec
+  (type $unused (descriptor $unused.desc (struct)))
+  (type $unused.desc (sub $public.desc (describes $unused (struct))))
+  ;; CHECK:      (type $private (struct))
+  (type $private (struct))
+ )
+
+ ;; CHECK:      (global $public (ref null $public) (ref.null none))
+ (global $public (export "public") (ref null $public) (ref.null none))
+ ;; CHECK:      (global $private (ref null $private) (ref.null none))
+ (global $private (ref null $private) (ref.null none))
+)
+
 ;; Sibling types $A and $B. The supertype that connects them should not stop us
 ;; from optimizing $B here, even though $A cannot be optimized.
+;; CHECK:      (export "public" (global $public))
 (module
   (rec
     ;; CHECK:      (rec