[Wasm GC] Fix invalid intermediate IR in OptimizeInstructions (#4169)

We added an optional ReFinalize in OptimizeInstructions at some point,
but that is not valid: The ReFinalize only updates types when all other
works is done, but the pass works incrementally. The bug the fuzzer found
is that a child is changed to be unreachable, and then the parent is
optimized before finalize() is called on it, which led to an assertion being
hit (as the child was unreachable but not the parent, which should also
be).

To fix this, do not change types in this pass. Emit an extra block with a
declared type when necessary. Other passes can remove the extra block.

1 files changed, 9 insertions, 12 deletions

diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp
index d18990e8e..387d76b37 100644
--- a/src/passes/OptimizeInstructions.cpp
+++ b/src/passes/OptimizeInstructions.cpp
@@ -207,8 +207,6 @@ struct OptimizeInstructions
 
   bool fastMath;
 
-  bool refinalize = false;
-
   void doWalkFunction(Function* func) {
     fastMath = getPassOptions().fastMath;
 
@@ -222,11 +220,6 @@ struct OptimizeInstructions
     // Main walk.
     super::doWalkFunction(func);
 
-    // If we need to update parent types, do so.
-    if (refinalize) {
-      ReFinalize().walkFunctionInModule(func, getModule());
-    }
-
     // Final optimizations.
     {
       FinalOptimizer optimizer(getPassOptions());
@@ -1383,11 +1376,12 @@ struct OptimizeInstructions
         // This cast cannot succeed. If the input is not a null, it will
         // definitely trap.
         if (fallthrough->type.isNonNullable()) {
-          // Our type will now be unreachable; update the parents.
-          refinalize = true;
+          // Make sure to emit a block with the same type as us; leave updating
+          // types for other passes.
           replaceCurrent(builder.makeBlock({builder.makeDrop(curr->ref),
                                             builder.makeDrop(curr->rtt),
-                                            builder.makeUnreachable()}));
+                                            builder.makeUnreachable()},
+                                           curr->type));
           return;
         }
         // Otherwise, we are not sure what it is, and need to wait for runtime
@@ -1575,8 +1569,11 @@ struct OptimizeInstructions
       // drop, which is no worse, and the value and the drop can be optimized
       // out later if the value has no side effects.
       Builder builder(*getModule());
-      replaceCurrent(builder.makeSequence(builder.makeDrop(curr->value),
-                                          builder.makeUnreachable()));
+      // Make sure to emit a block with the same type as us; leave updating
+      // types for other passes.
+      replaceCurrent(builder.makeBlock(
+        {builder.makeDrop(curr->value), builder.makeUnreachable()},
+        curr->type));
       return;
     }