Fix assertion in br_on_cast finalization (#8476)

If the br_on_cast is invalid because its input and output types are
unrelated, the GLB used as the new cast type will be unreachable. This
caused an assertion that the GLB was a reference to fail. Bail out early
in this case and leave the unreachable cast type for the validator to
find.

Author: tlively

src/wasm/wasm-validator.cpp

@@ -3124,8 +3124,8 @@ void FunctionValidator::visitRefTest(RefTest* curr) {
     return;
   }
   shouldBeEqual(
-    curr->castType.getHeapType().getBottom(),
-    curr->ref->type.getHeapType().getBottom(),
+    HeapType(curr->castType.getHeapType().getTop()),
+    HeapType(curr->ref->type.getHeapType().getTop()),
     curr,
     "ref.test target type and ref type must have a common supertype");
 
@@ -3168,27 +3168,15 @@ void FunctionValidator::visitRefCast(RefCast* curr) {
         curr->ref->type.isRef(), curr, "ref.cast ref must have ref type")) {
     return;
   }
-  // If the cast is unreachable but not the ref (we ruled out the former
-  // earlier), then the cast is unreachable because the cast type had no
-  // common supertype with the ref, which is invalid. This is the same as the
-  // check below us, but we must do it first (as getHeapType fails otherwise).
-  if (!shouldBeUnequal(
-        curr->type,
-        Type(Type::unreachable),
-        curr,
-        "ref.cast target type and ref type must have a common supertype")) {
-    return;
-  }
   // Also error (more generically) on i32 and anything else invalid here.
   if (!shouldBeTrue(curr->type.isRef(), curr, "ref.cast must have ref type")) {
     return;
   }
   shouldBeEqual(
-    curr->type.getHeapType().getBottom(),
-    curr->ref->type.getHeapType().getBottom(),
+    HeapType(curr->type.getHeapType().getTop()),
+    HeapType(curr->ref->type.getHeapType().getTop()),
     curr,
     "ref.cast target type and ref type must have a common supertype");
-
   // We should never have a nullable cast of a non-nullable reference, since
   // that unnecessarily loses type information.
   shouldBeTrue(curr->ref->type.isNullable() || curr->type.isNonNullable(),
@@ -3260,8 +3248,8 @@ void FunctionValidator::visitBrOn(BrOn* curr) {
     }
     if (curr->ref->type != Type::unreachable) {
       shouldBeEqual(
-        curr->castType.getHeapType().getBottom(),
-        curr->ref->type.getHeapType().getBottom(),
+        HeapType(curr->castType.getHeapType().getTop()),
+        HeapType(curr->ref->type.getHeapType().getTop()),
         curr,
         "br_on_cast* target type and ref type must have a common supertype");
     }

src/wasm/wasm.cpp

@@ -1060,7 +1060,12 @@ void RefTest::finalize() {
   } else {
     type = Type::i32;
     // Do not unnecessarily lose type information.
-    castType = Type::getGreatestLowerBound(castType, ref->type);
+    auto newCastType = Type::getGreatestLowerBound(castType, ref->type);
+    if (newCastType == Type::unreachable) {
+      // This is invalid. Leave the existing types for the validator to catch.
+      return;
+    }
+    castType = newCastType;
   }
 }
 
@@ -1092,7 +1097,8 @@ void RefCast::finalize() {
 
   // We reach this before validation, so the input type might be totally wrong.
   // Return early in this case to avoid doing the wrong thing below.
-  if (!ref->type.isRef()) {
+  if (!ref->type.isRef() || !type.isRef() ||
+      ref->type.getHeapType().getTop() != type.getHeapType().getTop()) {
     return;
   }
 
@@ -1131,7 +1137,14 @@ void BrOn::finalize() {
     // cast type, we can improve the cast type in a way that will not change the
     // cast behavior. This satisfies the constraint we had before Custom
     // Descriptors that the cast type is a subtype of the input type.
-    castType = Type::getGreatestLowerBound(castType, ref->type);
+    auto newCastType = Type::getGreatestLowerBound(castType, ref->type);
+    if (newCastType == Type::unreachable) {
+      // This is not valid. Leave the original cast type in place for the
+      // validator to catch.
+      type = ref->type;
+      return;
+    }
+    castType = newCastType;
     assert(castType.isRef());
   } else if (op == BrOnCastDescEq || op == BrOnCastDescEqFail) {
     if (desc->type.isNull()) {

test/lit/validation/shared-cast.wast

@@ -0,0 +1,36 @@
+;; RUN: not wasm-opt %s --all-features 2>&1 | filecheck %s
+
+(module
+  (type $shared (shared (struct)))
+  (type $unshared (struct))
+
+  (func $test-shared-to-unshared (param $s (ref $shared))
+    ;; CHECK: [wasm-validator error in function test-shared-to-unshared] any != (shared any): ref.test target type and ref type must have a common supertype
+    (drop
+      (ref.test (ref $unshared)
+        (local.get $s)
+      )
+    )
+  )
+
+  (func $cast-shared-to-unshared (param $s (ref $shared))
+    ;; CHECK: [wasm-validator error in function cast-shared-to-unshared] any != (shared any): ref.cast target type and ref type must have a common supertype
+    (drop
+      (ref.cast (ref $unshared)
+        (local.get $s)
+      )
+    )
+  )
+
+  (func $br_on_shared-to-unshared (param $s (ref $shared))
+    (block $l (result (ref $unshared))
+      ;; CHECK: [wasm-validator error in function br_on_shared-to-unshared] any != (shared any): br_on_cast* target type and ref type must have a common supertype
+      (drop
+        (br_on_cast $l (ref $shared) (ref $unshared)
+          (local.get $s)
+        )
+      )
+      (unreachable)
+    )
+  )
+)