[Wasm GC] Avoid refining in TypeUpdating unreachability propagation (#5616)

That code should only propagate unreachability, and not refine. If it refines when
we call finalize() then other code around it might end up invalid (as it could be
partially refined; see testcase).

2 files changed, 87 insertions, 1 deletions

diff --git a/src/ir/type-updating.h b/src/ir/type-updating.h
index d224f93dc..160934da1 100644
--- a/src/ir/type-updating.h
+++ b/src/ir/type-updating.h
@@ -248,14 +248,22 @@ struct TypeUpdater
           return; // did not turn
         }
       } else if (auto* iff = curr->dynCast<If>()) {
-        // may not be unreachable if just one side is
+        // We only want to change a concrete type to unreachable here, so undo
+        // anything else. Other changes can be a problem, like refining the type
+        // of an if for GC-using code, as the code all around us only assumes we
+        // are propagating unreachability and not doing a full refinalize.
+        auto old = iff->type;
         iff->finalize();
         if (curr->type != Type::unreachable) {
+          iff->type = old;
           return; // did not turn
         }
       } else if (auto* tryy = curr->dynCast<Try>()) {
+        // See comment on If, above.
+        auto old = tryy->type;
         tryy->finalize();
         if (curr->type != Type::unreachable) {
+          tryy->type = old;
           return; // did not turn
         }
       } else {
@@ -303,9 +311,13 @@ struct TypeUpdater
     if (!curr->type.isConcrete()) {
       return; // nothing concrete to change to unreachable
     }
+    // See comment in propagateTypesUp() for If regarding restoring the type.
+    auto old = curr->type;
     curr->finalize();
     if (curr->type == Type::unreachable) {
       propagateTypesUp(curr);
+    } else {
+      curr->type = old;
     }
   }
 
@@ -313,9 +325,13 @@ struct TypeUpdater
     if (!curr->type.isConcrete()) {
       return; // nothing concrete to change to unreachable
     }
+    // See comment in propagateTypesUp() for Try regarding restoring the type.
+    auto old = curr->type;
     curr->finalize();
     if (curr->type == Type::unreachable) {
       propagateTypesUp(curr);
+    } else {
+      curr->type = old;
     }
   }
 
diff --git a/test/lit/passes/dce_all-features.wast b/test/lit/passes/dce_all-features.wast
index 24bb751fc..68f809926 100644
--- a/test/lit/passes/dce_all-features.wast
+++ b/test/lit/passes/dce_all-features.wast
@@ -1428,3 +1428,73 @@
     )
   )
 )
+
+(module
+  ;; CHECK:      (type $none_=>_anyref (func (result anyref)))
+
+  ;; CHECK:      (func $if (type $none_=>_anyref) (result anyref)
+  ;; CHECK-NEXT:  (ref.cast null i31
+  ;; CHECK-NEXT:   (if (result i31ref)
+  ;; CHECK-NEXT:    (i32.const 0)
+  ;; CHECK-NEXT:    (unreachable)
+  ;; CHECK-NEXT:    (i31.new
+  ;; CHECK-NEXT:     (i32.const 42)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $if (result anyref)
+    ;; We should not refinalize while doing DCE. The cast should remain a
+    ;; nullable one, and the if should keep returning a nullable value. (If we
+    ;; refinalized only the if then the cast would be invalid, since we cannot
+    ;; have a nullable cast of a non-nullable input.)
+    ;;
+    ;; In other words, we can propagate unreachability in DCE, but should cause
+    ;; no other type changes.
+    (ref.cast null i31
+      (if (result i31ref)
+        (i32.const 0)
+        (block (result i31ref)
+          (unreachable)
+        )
+        (i31.new
+          (i32.const 42)
+        )
+      )
+    )
+  )
+
+  ;; CHECK:      (func $try (type $none_=>_anyref) (result anyref)
+  ;; CHECK-NEXT:  (try $try (result i31ref)
+  ;; CHECK-NEXT:   (do
+  ;; CHECK-NEXT:    (drop
+  ;; CHECK-NEXT:     (call $try)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:    (unreachable)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:   (catch_all
+  ;; CHECK-NEXT:    (i31.new
+  ;; CHECK-NEXT:     (i32.const 42)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $try (result anyref)
+    ;; As above, but for try.
+    (try (result i31ref)
+      (do
+        (block (result i31ref)
+          (drop
+            (call $try)
+          )
+          (unreachable)
+        )
+      )
+      (catch_all
+        (i31.new
+          (i32.const 42)
+        )
+      )
+    )
+  )
+)