[wasm-gc] Fix br_on_* branch-type requirements

br_on_* instructions need a precisely typed branch target, as opposed
to being treated like regular br instructions.

Bug: v8:7748
Change-Id: Iedace79faf59d61cf2ce5ac88e633e07b5a2a43f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2655507
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72450}

src/wasm/baseline/liftoff-compiler.cc

@@ -4423,8 +4423,7 @@ class LiftoffCompiler {
                         obj_reg.gp(), tmp1.gp());
     }
 
-    // At this point, the object is neither null nor an i31ref. Perform
-    // a regular type check. Check for exact match first.
+    // Perform a regular type check. Check for exact match first.
     __ LoadMap(tmp1.gp(), obj_reg.gp());
     // {tmp1} now holds the object's map.
     __ emit_cond_jump(kEqual, &match, rtt.type, tmp1.gp(), rtt_reg.gp());

src/wasm/function-body-decoder-impl.h

@@ -4187,19 +4187,23 @@ class WasmFullDecoder : public WasmDecoder<validate> {
           return 0;
         }
         Control* c = control_at(branch_depth.depth);
-        Value* result_on_branch =
-            Push(rtt.type.is_bottom()
+        ValueType type_on_branch =
+            rtt.type.is_bottom()
                 ? kWasmBottom
-                     : ValueType::Ref(rtt.type.ref_index(), kNonNullable));
-        TypeCheckBranchResult check_result = TypeCheckBranch(c, true);
+                : ValueType::Ref(rtt.type.ref_index(), kNonNullable);
+
+        TypeCheckBranchResult check_result = TypeCheckBrOn(c, type_on_branch);
         if (V8_LIKELY(check_result == kReachableBranch)) {
+          // We temporarily push this value for the interface to merge it into
+          // the branch merge.
+          Value* result_on_branch = Push(type_on_branch);
           CALL_INTERFACE(BrOnCast, obj, rtt, result_on_branch,
                          branch_depth.depth);
           c->br_merge()->reached = true;
+          Pop(0);  // Drop {result_on_branch}, restore original value.
         } else if (check_result == kInvalidStack) {
           return 0;
         }
-        Pop(0);  // Drop {result_on_branch}, restore original value.
         Value* result_on_fallthrough = Push(obj.type);
         *result_on_fallthrough = obj;
         return opcode_length + branch_depth.length;
@@ -4250,9 +4254,13 @@ class WasmFullDecoder : public WasmDecoder<validate> {
                 ? HeapType::kFunc
                 : opcode == kExprBrOnData ? HeapType::kData : HeapType::kI31;
 
-        Value* result_on_branch = Push(ValueType::Ref(heap_type, kNonNullable));
-        TypeCheckBranchResult check_result = TypeCheckBranch(c, true);
+        ValueType type_on_branch = ValueType::Ref(heap_type, kNonNullable);
+
+        TypeCheckBranchResult check_result = TypeCheckBrOn(c, type_on_branch);
         if (V8_LIKELY(check_result == kReachableBranch)) {
+          // We temporarily push this value for the interface to merge it into
+          // the branch merge.
+          Value* result_on_branch = Push(type_on_branch);
           if (opcode == kExprBrOnFunc) {
             CALL_INTERFACE(BrOnFunc, obj, result_on_branch, branch_depth.depth);
           } else if (opcode == kExprBrOnData) {
@@ -4261,10 +4269,10 @@ class WasmFullDecoder : public WasmDecoder<validate> {
             CALL_INTERFACE(BrOnI31, obj, result_on_branch, branch_depth.depth);
           }
           c->br_merge()->reached = true;
+          Pop(0);  // Drop {result_on_branch}, restore original value.
         } else if (check_result == kInvalidStack) {
           return 0;
         }
-        Pop(0);  // Drop {result_on_branch}, restore original value.
         Value* result_on_fallthrough = Push(obj.type);
         *result_on_fallthrough = obj;
         return opcode_length + branch_depth.length;
@@ -4680,6 +4688,25 @@ class WasmFullDecoder : public WasmDecoder<validate> {
                : kInvalidStack;
   }
 
+  TypeCheckBranchResult TypeCheckBrOn(Control* c, ValueType type_on_branch) {
+    if (V8_LIKELY(control_.back().reachable())) {
+      // We only do type-checking here. This is only needed during validation.
+      if (!VALIDATE(c->br_merge()->arity == 1 &&
+                    (type_on_branch == kWasmBottom ||
+                     IsSubtypeOf(type_on_branch, (*c->br_merge())[0].type,
+                                 this->module_)))) {
+        this->DecodeError("%s must target a branch of a supertype of type %s",
+                          SafeOpcodeNameAt(this->pc_),
+                          type_on_branch.name().c_str());
+        return kInvalidStack;
+      }
+      return kReachableBranch;
+    }
+
+    return TypeCheckUnreachableMerge(*c->br_merge(), false) ? kUnreachableBranch
+                                                            : kInvalidStack;
+  }
+
   bool TypeCheckReturn() {
     int num_returns = static_cast<int>(this->sig_->return_count());
     // No type checking is needed if there are no returns.

test/cctest/wasm/test-gc.cc

@@ -356,55 +356,63 @@ WASM_COMPILED_EXEC_TEST(BrOnCast) {
                            static_cast<HeapType::Representation>(type_index)));
   const byte kTestStruct = tester.DefineFunction(
       tester.sigs.i_v(), {kWasmI32, kDataRefNull},
-      {WASM_BLOCK(WASM_LOCAL_SET(0, WASM_I32V(111)),
+      {WASM_BLOCK_R(ValueType::Ref(type_index, kNullable),
+                    WASM_LOCAL_SET(0, WASM_I32V(111)),
                     // Pipe a struct through a local so it's statically typed
-                  // as eqref.
-                  WASM_LOCAL_SET(
-                      1, WASM_STRUCT_NEW_WITH_RTT(type_index, WASM_I32V(1),
-                                                  WASM_GLOBAL_GET(rtt_index))),
+                    // as dataref.
+                    WASM_LOCAL_SET(1, WASM_STRUCT_NEW_WITH_RTT(
+                                          other_type_index, WASM_F32(1.0),
+                                          WASM_RTT_CANON(other_type_index))),
                     WASM_LOCAL_GET(1),
                     // The type check fails, so this branch isn't taken.
-                  WASM_BR_ON_CAST(0, WASM_RTT_CANON(other_type_index)),
-                  WASM_LOCAL_SET(0, WASM_I32V(222)),  // Final result.
+                    WASM_BR_ON_CAST(0, WASM_GLOBAL_GET(rtt_index)), WASM_DROP,
+
+                    WASM_LOCAL_SET(0, WASM_I32V(221)),  // (Final result) - 1
+                    WASM_LOCAL_SET(1, WASM_STRUCT_NEW_WITH_RTT(
+                                          type_index, WASM_I32V(1),
+                                          WASM_GLOBAL_GET(rtt_index))),
+                    WASM_LOCAL_GET(1),
                     // This branch is taken.
                     WASM_BR_ON_CAST(0, WASM_GLOBAL_GET(rtt_index)),
+                    WASM_GLOBAL_GET(rtt_index), WASM_GC_OP(kExprRefCast),
+
                     // Not executed due to the branch.
-                  WASM_DROP, WASM_LOCAL_SET(0, WASM_I32V(333))),
-       WASM_LOCAL_GET(0), kExprEnd});
+                    WASM_LOCAL_SET(0, WASM_I32V(333))),
+       WASM_GC_OP(kExprStructGet), type_index, 0, WASM_LOCAL_GET(0),
+       kExprI32Add, kExprEnd});
 
   const byte kTestNull = tester.DefineFunction(
       tester.sigs.i_v(), {kWasmI32, kDataRefNull},
-      {WASM_BLOCK(WASM_LOCAL_SET(0, WASM_I32V(111)),
+      {WASM_BLOCK_R(ValueType::Ref(type_index, kNullable),
+                    WASM_LOCAL_SET(0, WASM_I32V(111)),
                     WASM_LOCAL_GET(1),  // Put a nullref onto the value stack.
-                  // Neither of these branches is taken for nullref.
-                  WASM_BR_ON_CAST(0, WASM_RTT_CANON(other_type_index)),
-                  WASM_LOCAL_SET(0, WASM_I32V(222)),
-                  WASM_BR_ON_CAST(0, WASM_GLOBAL_GET(rtt_index)), WASM_DROP,
-                  WASM_LOCAL_SET(0, WASM_I32V(333))),  // Final result.
-       WASM_LOCAL_GET(0), kExprEnd});
+                    // Not taken for nullref.
+                    WASM_BR_ON_CAST(0, WASM_GLOBAL_GET(rtt_index)),
+                    WASM_RTT_CANON(type_index), WASM_GC_OP(kExprRefCast),
+
+                    WASM_LOCAL_SET(0, WASM_I32V(222))),  // Final result.
+       WASM_DROP, WASM_LOCAL_GET(0), kExprEnd});
 
   const byte kTypedAfterBranch = tester.DefineFunction(
       tester.sigs.i_v(), {kWasmI32, kDataRefNull},
       {WASM_LOCAL_SET(1, WASM_STRUCT_NEW_WITH_RTT(type_index, WASM_I32V(42),
                                                   WASM_GLOBAL_GET(rtt_index))),
-       WASM_BLOCK(WASM_LOCAL_SET(
-           // The outer block catches the struct left behind by the inner block
-           // and reads its field.
-           0,
-           WASM_STRUCT_GET(
-               type_index, 0,
+       WASM_BLOCK_I(
            // The inner block should take the early branch with a struct
            // on the stack.
            WASM_BLOCK_R(ValueType::Ref(type_index, kNonNullable),
                         WASM_LOCAL_GET(1),
                         WASM_BR_ON_CAST(0, WASM_GLOBAL_GET(rtt_index)),
                         // Returning 123 is the unreachable failure case.
-                            WASM_LOCAL_SET(0, WASM_I32V(123)), WASM_BR(1))))),
-       WASM_LOCAL_GET(0), kExprEnd});
+                        WASM_I32V(123), WASM_BR(1)),
+           // The outer block catches the struct left behind by the inner block
+           // and reads its field.
+           WASM_GC_OP(kExprStructGet), type_index, 0),
+       kExprEnd});
 
   tester.CompileModule();
   tester.CheckResult(kTestStruct, 222);
-  tester.CheckResult(kTestNull, 333);
+  tester.CheckResult(kTestNull, 222);
   tester.CheckResult(kTypedAfterBranch, 42);
 }