Ensure br_on_cast* target type is subtype of input type (#5881)

The WasmGC spec will require that the target cast type of br_on_cast and
br_on_cast_fail be a subtype of the input type, but so far Binaryen has not
enforced this constraint, so it could produce invalid modules when optimizations
refined the input to a br_on_cast* such that it was no longer a supertype of the
cast target type.

Fix this problem by setting the cast target type to be the greatest lower bound
of the original cast target type and the current input type in
`BrOn::finalize()`. This maintains the invariant that the cast target type
should be a subtype of the input type and it also does not change cast behavior;
any value that could make the original cast succeed at runtime necessarily
inhabits both the original cast target type and the input type, so it also must
inhabit their greatest lower bound and will make the updated cast succeed as
well.

11 files changed, 56 insertions, 34 deletions

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 98e832225..0d405251e 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -7028,6 +7028,9 @@ bool WasmBinaryReader::maybeVisitBrOn(Expression*& out, uint32_t code) {
     auto castHeapType = getHeapType();
     castType = Type(castHeapType, castNullability);
     auto inputType = Type(inputHeapType, inputNullability);
+    if (!Type::isSubType(castType, inputType)) {
+      throwError("br_on_cast* cast type must be subtype of input type");
+    }
     if (!Type::isSubType(ref->type, inputType)) {
       throwError(std::string("Invalid reference type for ") +
                  ((op == BrOnCast) ? "br_on_cast" : "br_on_cast_fail"));
diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp
index 0115cdac3..0519afd83 100644
--- a/src/wasm/wasm-s-parser.cpp
+++ b/src/wasm/wasm-s-parser.cpp
@@ -2880,6 +2880,12 @@ Expression* SExpressionWasmBuilder::makeBrOnCast(Element& s, bool onFail) {
   auto name = getLabel(*s[i++]);
   auto inputType = elementToType(*s[i++]);
   auto castType = elementToType(*s[i++]);
+  if (!Type::isSubType(castType, inputType)) {
+    throw ParseException(
+      "br_on_cast* cast type must be a subtype of its input type",
+      s.line,
+      s.col);
+  }
   auto* ref = parseExpression(*s[i]);
   if (!Type::isSubType(ref->type, inputType)) {
     throw ParseException(
diff --git a/src/wasm/wasm-stack.cpp b/src/wasm/wasm-stack.cpp
index 3b7756992..115013307 100644
--- a/src/wasm/wasm-stack.cpp
+++ b/src/wasm/wasm-stack.cpp
@@ -2048,6 +2048,7 @@ void BinaryInstWriter::visitBrOn(BrOn* curr) {
         o << U32LEB(BinaryConsts::BrOnCastFail);
       }
       assert(curr->ref->type.isRef());
+      assert(Type::isSubType(curr->castType, curr->ref->type));
       uint8_t flags = (curr->ref->type.isNullable() ? 1 : 0) |
                       (curr->castType.isNullable() ? 2 : 0);
       o << flags;
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
index 9c1f9fdbb..b48ec632e 100644
--- a/src/wasm/wasm-validator.cpp
+++ b/src/wasm/wasm-validator.cpp
@@ -2583,6 +2583,11 @@ void FunctionValidator::visitBrOn(BrOn* curr) {
       curr->ref->type.getHeapType().getBottom(),
       curr,
       "br_on_cast* target type and ref type must have a common supertype");
+    shouldBeSubType(
+      curr->castType,
+      curr->ref->type,
+      curr,
+      "br_on_cast* target type must be a subtype of its input type");
   } else {
     shouldBeEqual(curr->castType,
                   Type(Type::none),
diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp
index 552ec7e57..648a25d9e 100644
--- a/src/wasm/wasm.cpp
+++ b/src/wasm/wasm.cpp
@@ -972,6 +972,13 @@ void BrOn::finalize() {
     type = Type::unreachable;
     return;
   }
+  if (op == BrOnCast || op == BrOnCastFail) {
+    // The cast type must be a subtype of the input type. If we've refined the
+    // input type so that this is no longer true, we can fix it by similarly
+    // refining the cast type in a way that will not change the cast behavior.
+    castType = Type::getGreatestLowerBound(castType, ref->type);
+    assert(castType.isRef());
+  }
   switch (op) {
     case BrOnNull:
       // If we do not branch, we flow out the existing value as non-null.
diff --git a/test/lit/cast-to-basic.wast b/test/lit/cast-to-basic.wast
index e66e3cd38..c4190dc76 100644
--- a/test/lit/cast-to-basic.wast
+++ b/test/lit/cast-to-basic.wast
@@ -31,24 +31,24 @@
     )
   )
 
-  ;; CHECK:      (func $br (type $none_=>_none)
+  ;; CHECK:      (func $br (type $anyref_=>_none) (param $anyref anyref)
   ;; CHECK-NEXT:  (drop
   ;; CHECK-NEXT:   (block $label$1 (result structref)
   ;; CHECK-NEXT:    (drop
-  ;; CHECK-NEXT:     (br_on_cast $label$1 nullref (ref struct)
-  ;; CHECK-NEXT:      (ref.null none)
+  ;; CHECK-NEXT:     (br_on_cast $label$1 anyref (ref struct)
+  ;; CHECK-NEXT:      (local.get $anyref)
   ;; CHECK-NEXT:     )
   ;; CHECK-NEXT:    )
   ;; CHECK-NEXT:    (ref.null none)
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
-  (func $br
+  (func $br (param $anyref anyref)
     (drop
       (block $l (result structref)
         (drop
-          (br_on_cast $l nullref (ref struct)
-            (ref.null none)
+          (br_on_cast $l anyref (ref struct)
+            (local.get $anyref)
           )
         )
         (ref.null none)
@@ -56,24 +56,24 @@
     )
   )
 
-  ;; CHECK:      (func $br-null (type $none_=>_none)
+  ;; CHECK:      (func $br-null (type $anyref_=>_none) (param $anyref anyref)
   ;; CHECK-NEXT:  (drop
   ;; CHECK-NEXT:   (block $label$1 (result structref)
   ;; CHECK-NEXT:    (drop
-  ;; CHECK-NEXT:     (br_on_cast $label$1 nullref structref
-  ;; CHECK-NEXT:      (ref.null none)
+  ;; CHECK-NEXT:     (br_on_cast $label$1 anyref structref
+  ;; CHECK-NEXT:      (local.get $anyref)
   ;; CHECK-NEXT:     )
   ;; CHECK-NEXT:    )
   ;; CHECK-NEXT:    (ref.null none)
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
-  (func $br-null
+  (func $br-null (param $anyref anyref)
     (drop
       (block $l (result structref)
         (drop
-          (br_on_cast $l nullref structref
-            (ref.null none)
+          (br_on_cast $l anyref structref
+            (local.get $anyref)
           )
         )
         (ref.null none)
@@ -81,24 +81,24 @@
     )
   )
 
-  ;; CHECK:      (func $br-fail-null (type $none_=>_none)
+  ;; CHECK:      (func $br-fail-null (type $anyref_=>_none) (param $anyref anyref)
   ;; CHECK-NEXT:  (drop
-  ;; CHECK-NEXT:   (block $label$1 (result structref)
+  ;; CHECK-NEXT:   (block $label$1 (result anyref)
   ;; CHECK-NEXT:    (drop
-  ;; CHECK-NEXT:     (br_on_cast_fail $label$1 nullref structref
-  ;; CHECK-NEXT:      (ref.null none)
+  ;; CHECK-NEXT:     (br_on_cast_fail $label$1 anyref structref
+  ;; CHECK-NEXT:      (local.get $anyref)
   ;; CHECK-NEXT:     )
   ;; CHECK-NEXT:    )
   ;; CHECK-NEXT:    (ref.null none)
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
-  (func $br-fail-null
+  (func $br-fail-null (param $anyref anyref)
     (drop
-      (block $l (result structref)
+      (block $l (result anyref)
         (drop
-          (br_on_cast_fail $l nullref structref
-            (ref.null none)
+          (br_on_cast_fail $l anyref structref
+            (local.get $anyref)
           )
         )
         (ref.null none)
diff --git a/test/lit/passes/code-pushing-gc.wast b/test/lit/passes/code-pushing-gc.wast
index 9587d5ab7..ffb5a25d3 100644
--- a/test/lit/passes/code-pushing-gc.wast
+++ b/test/lit/passes/code-pushing-gc.wast
@@ -7,7 +7,7 @@
   ;; CHECK-NEXT:  (drop
   ;; CHECK-NEXT:   (block $out (result (ref func))
   ;; CHECK-NEXT:    (drop
-  ;; CHECK-NEXT:     (br_on_cast $out nullfuncref (ref func)
+  ;; CHECK-NEXT:     (br_on_cast $out nullfuncref (ref nofunc)
   ;; CHECK-NEXT:      (ref.null nofunc)
   ;; CHECK-NEXT:     )
   ;; CHECK-NEXT:    )
@@ -48,7 +48,7 @@
   ;; CHECK-NEXT:     (ref.func $br_on_no)
   ;; CHECK-NEXT:    )
   ;; CHECK-NEXT:    (drop
-  ;; CHECK-NEXT:     (br_on_cast $out nullfuncref (ref func)
+  ;; CHECK-NEXT:     (br_on_cast $out nullfuncref (ref nofunc)
   ;; CHECK-NEXT:      (ref.null nofunc)
   ;; CHECK-NEXT:     )
   ;; CHECK-NEXT:    )
diff --git a/test/lit/passes/gufa-refs.wast b/test/lit/passes/gufa-refs.wast
index b82fbf148..7dacf132d 100644
--- a/test/lit/passes/gufa-refs.wast
+++ b/test/lit/passes/gufa-refs.wast
@@ -986,9 +986,9 @@
   ;; CHECK-NEXT:   (block
   ;; CHECK-NEXT:    (block
   ;; CHECK-NEXT:     (drop
-  ;; CHECK-NEXT:      (block $parent (result (ref $parent))
+  ;; CHECK-NEXT:      (block $parent (result (ref none))
   ;; CHECK-NEXT:       (drop
-  ;; CHECK-NEXT:        (br_on_cast $parent (ref $unrelated) (ref $parent)
+  ;; CHECK-NEXT:        (br_on_cast $parent (ref $unrelated) (ref none)
   ;; CHECK-NEXT:         (struct.new_default $unrelated)
   ;; CHECK-NEXT:        )
   ;; CHECK-NEXT:       )
diff --git a/test/lit/passes/merge-blocks.wast b/test/lit/passes/merge-blocks.wast
index 02772c9b6..1401bd451 100644
--- a/test/lit/passes/merge-blocks.wast
+++ b/test/lit/passes/merge-blocks.wast
@@ -20,7 +20,7 @@
  ;; CHECK-NEXT:  (drop
  ;; CHECK-NEXT:   (block $label$1 (result i31ref)
  ;; CHECK-NEXT:    (drop
- ;; CHECK-NEXT:     (br_on_cast $label$1 nullref (ref i31)
+ ;; CHECK-NEXT:     (br_on_cast $label$1 nullref (ref none)
  ;; CHECK-NEXT:      (ref.null none)
  ;; CHECK-NEXT:     )
  ;; CHECK-NEXT:    )
diff --git a/test/lit/passes/optimize-instructions-gc.wast b/test/lit/passes/optimize-instructions-gc.wast
index 4882bae9f..6d9e32d3d 100644
--- a/test/lit/passes/optimize-instructions-gc.wast
+++ b/test/lit/passes/optimize-instructions-gc.wast
@@ -2600,8 +2600,8 @@
   ;; CHECK-NEXT:  (block $outer (result (ref any))
   ;; CHECK-NEXT:   (block
   ;; CHECK-NEXT:    (drop
-  ;; CHECK-NEXT:     (block (result i31ref)
-  ;; CHECK-NEXT:      (br_on_cast_fail $outer structref i31ref
+  ;; CHECK-NEXT:     (block (result nullref)
+  ;; CHECK-NEXT:      (br_on_cast_fail $outer structref nullref
   ;; CHECK-NEXT:       (block (result structref)
   ;; CHECK-NEXT:        (br_on_cast_fail $outer anyref structref
   ;; CHECK-NEXT:         (local.get $anyref)
@@ -2635,8 +2635,8 @@
   ;; CHECK:      (func $incompatible-cast-heap-types-nullable (type $anyref_=>_anyref) (param $anyref anyref) (result anyref)
   ;; CHECK-NEXT:  (block $outer (result anyref)
   ;; CHECK-NEXT:   (ref.cast null none
-  ;; CHECK-NEXT:    (block (result i31ref)
-  ;; CHECK-NEXT:     (br_on_cast_fail $outer structref i31ref
+  ;; CHECK-NEXT:    (block (result nullref)
+  ;; CHECK-NEXT:     (br_on_cast_fail $outer structref nullref
   ;; CHECK-NEXT:      (block (result structref)
   ;; CHECK-NEXT:       (br_on_cast_fail $outer anyref structref
   ;; CHECK-NEXT:        (local.get $anyref)
@@ -2668,8 +2668,8 @@
   ;; CHECK-NEXT:  (block $outer (result anyref)
   ;; CHECK-NEXT:   (block
   ;; CHECK-NEXT:    (drop
-  ;; CHECK-NEXT:     (block (result (ref i31))
-  ;; CHECK-NEXT:      (br_on_cast_fail $outer structref (ref i31)
+  ;; CHECK-NEXT:     (block (result (ref none))
+  ;; CHECK-NEXT:      (br_on_cast_fail $outer structref (ref none)
   ;; CHECK-NEXT:       (block (result structref)
   ;; CHECK-NEXT:        (br_on_cast_fail $outer anyref structref
   ;; CHECK-NEXT:         (local.get $anyref)
diff --git a/test/lit/passes/remove-unused-brs-gc.wast b/test/lit/passes/remove-unused-brs-gc.wast
index 50ffd9583..94dbabc6b 100644
--- a/test/lit/passes/remove-unused-brs-gc.wast
+++ b/test/lit/passes/remove-unused-brs-gc.wast
@@ -95,7 +95,7 @@
 
  ;; CHECK:      (func $br_on_cast_unrelated (type $none_=>_ref?|$struct|) (result (ref null $struct))
  ;; CHECK-NEXT:  (local $nullable-struct2 (ref null $struct2))
- ;; CHECK-NEXT:  (block $block (result (ref null $struct))
+ ;; CHECK-NEXT:  (block $block (result nullref)
  ;; CHECK-NEXT:   (drop
  ;; CHECK-NEXT:    (struct.new_default $struct2)
  ;; CHECK-NEXT:   )
@@ -106,7 +106,7 @@
  ;; CHECK-NEXT:    (local.get $nullable-struct2)
  ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:   (drop
- ;; CHECK-NEXT:    (br_on_cast $block (ref null $struct2) (ref null $struct)
+ ;; CHECK-NEXT:    (br_on_cast $block (ref null $struct2) nullref
  ;; CHECK-NEXT:     (local.get $nullable-struct2)
  ;; CHECK-NEXT:    )
  ;; CHECK-NEXT:   )
@@ -165,7 +165,7 @@
  ;; CHECK-NEXT:    )
  ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:   (drop
- ;; CHECK-NEXT:    (br_on_cast_fail $block (ref null $struct2) (ref null $struct)
+ ;; CHECK-NEXT:    (br_on_cast_fail $block (ref null $struct2) nullref
  ;; CHECK-NEXT:     (local.get $nullable-struct2)
  ;; CHECK-NEXT:    )
  ;; CHECK-NEXT:   )