From e90c366be42641a98c52d0e88cfbdce41e512539 Mon Sep 17 00:00:00 2001 From: Manos Koukoutos Date: Wed, 19 May 2021 15:41:14 +0000 Subject: [PATCH] Reland "[wasm-gc] Implement br_on_cast_fail" This is a reland of 8f39a58586d0c1b53781bdb51dc0153cb5f08b2a Changes compared to original: Change the type of arguments of WASM_I32V from byte to int for MSVC compatibility. Original change's description: > [wasm-gc] Implement br_on_cast_fail > > Bug: v8:7748 > Change-Id: I7894ad51ccf8ac41a5081c272a583a4ff25c1835 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2900225 > Commit-Queue: Manos Koukoutos > Reviewed-by: Jakob Kummerow > Cr-Commit-Position: refs/heads/master@{#74652} Bug: v8:7748 Change-Id: I39f39ff6979382f5618683a8e7754f56df4ec9e3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2905599 Reviewed-by: Jakob Kummerow Commit-Queue: Manos Koukoutos Cr-Commit-Position: refs/heads/master@{#74689} --- src/wasm/baseline/liftoff-compiler.cc | 23 +++++- src/wasm/function-body-decoder-impl.h | 67 ++++++++++++++++- src/wasm/graph-builder-interface.cc | 39 ++++++---- src/wasm/wasm-opcodes-inl.h | 1 + src/wasm/wasm-opcodes.h | 1 + test/cctest/wasm/test-gc.cc | 57 +++++++++++++++ test/common/wasm/wasm-macro-gen.h | 2 + test/mjsunit/wasm/wasm-module-builder.js | 1 + .../wasm/function-body-decoder-unittest.cc | 72 ++++++++++++++++++- 9 files changed, 245 insertions(+), 18 deletions(-) diff --git a/src/wasm/baseline/liftoff-compiler.cc b/src/wasm/baseline/liftoff-compiler.cc index 16c8071eb5..e215735eb3 100644 --- a/src/wasm/baseline/liftoff-compiler.cc +++ b/src/wasm/baseline/liftoff-compiler.cc @@ -5209,7 +5209,7 @@ class LiftoffCompiler { } void BrOnCast(FullDecoder* decoder, const Value& obj, const Value& rtt, - Value* result_on_branch, uint32_t depth) { + Value* /* result_on_branch */, uint32_t depth) { // Before branching, materialize all constants. This avoids repeatedly // materializing them for each conditional branch. if (depth != decoder->control_depth() - 1) { @@ -5230,6 +5230,27 @@ class LiftoffCompiler { __ PushRegister(obj.type.kind(), obj_reg); } + void BrOnCastFail(FullDecoder* decoder, const Value& obj, const Value& rtt, + Value* /* result_on_fallthrough */, uint32_t depth) { + // Before branching, materialize all constants. This avoids repeatedly + // materializing them for each conditional branch. + if (depth != decoder->control_depth() - 1) { + __ MaterializeMergedConstants( + decoder->control_at(depth)->br_merge()->arity); + } + + Label cont_branch, fallthrough; + LiftoffRegister obj_reg = + SubtypeCheck(decoder, obj, rtt, &cont_branch, kNullFails); + __ PushRegister(obj.type.kind(), obj_reg); + __ emit_jump(&fallthrough); + + __ bind(&cont_branch); + BrOrRet(decoder, depth, 0); + + __ bind(&fallthrough); + } + // Abstract type checkers. They all return the object register and fall // through to match. LiftoffRegister DataCheck(const Value& obj, Label* no_match, diff --git a/src/wasm/function-body-decoder-impl.h b/src/wasm/function-body-decoder-impl.h index 6fd082a3c3..14cc95e735 100644 --- a/src/wasm/function-body-decoder-impl.h +++ b/src/wasm/function-body-decoder-impl.h @@ -1143,6 +1143,8 @@ struct ControlBase : public PcForErrors { F(AssertNull, const Value& obj, Value* result) \ F(BrOnCast, const Value& obj, const Value& rtt, Value* result_on_branch, \ uint32_t depth) \ + F(BrOnCastFail, const Value& obj, const Value& rtt, \ + Value* result_on_fallthrough, uint32_t depth) \ F(RefIsData, const Value& object, Value* result) \ F(RefAsData, const Value& object, Value* result) \ F(BrOnData, const Value& object, Value* value_on_branch, uint32_t br_depth) \ @@ -1928,6 +1930,7 @@ class WasmDecoder : public Decoder { return length + imm.length; } case kExprBrOnCast: + case kExprBrOnCastFail: case kExprBrOnData: case kExprBrOnFunc: case kExprBrOnI31: { @@ -2109,6 +2112,7 @@ class WasmDecoder : public Decoder { case kExprRefTest: case kExprRefCast: case kExprBrOnCast: + case kExprBrOnCastFail: return {2, 1}; case kExprArraySet: return {3, 0}; @@ -4370,7 +4374,7 @@ class WasmFullDecoder : public WasmDecoder { // significantly more convenient to pass around the values that // will be on the stack when the branch is taken. // TODO(jkummerow): Reconsider this choice. - Drop(2); // {obj} and {ret}. + Drop(2); // {obj} and {rtt}. Value result_on_branch = CreateValue( rtt.type.is_bottom() ? kWasmBottom @@ -4395,6 +4399,67 @@ class WasmFullDecoder : public WasmDecoder { Push(obj); // Restore stack state on fallthrough. return opcode_length + branch_depth.length; } + case kExprBrOnCastFail: { + BranchDepthImmediate branch_depth(this, + this->pc_ + opcode_length); + if (!this->Validate(this->pc_ + opcode_length, branch_depth, + control_.size())) { + return 0; + } + Value rtt = Peek(0, 1); + if (!VALIDATE(rtt.type.is_rtt() || rtt.type.is_bottom())) { + PopTypeError(1, rtt, "rtt"); + return 0; + } + Value obj = Peek(1, 0); + if (!VALIDATE(IsSubtypeOf(obj.type, kWasmFuncRef, this->module_) || + IsSubtypeOf(obj.type, + ValueType::Ref(HeapType::kData, kNullable), + this->module_) || + obj.type.is_bottom())) { + PopTypeError(0, obj, "subtype of (ref null func) or (ref null data)"); + return 0; + } + Control* c = control_at(branch_depth.depth); + if (c->br_merge()->arity == 0) { + this->DecodeError( + "br_on_cast_fail must target a branch of arity at least 1"); + return 0; + } + // Attention: contrary to most other instructions, we modify the stack + // before calling the interface function. This makes it significantly + // more convenient to pass around the values that will be on the stack + // when the branch is taken. In this case, we leave {obj} on the stack + // to type check the branch. + // TODO(jkummerow): Reconsider this choice. + Drop(rtt); + if (!VALIDATE(TypeCheckBranch(c, 0))) return 0; + Value result_on_fallthrough = CreateValue( + rtt.type.is_bottom() + ? kWasmBottom + : ValueType::Ref(rtt.type.ref_index(), kNonNullable)); + // This logic ensures that code generation can assume that functions + // can only be cast to function types, and data objects to data types. + if (V8_LIKELY(current_code_reachable_and_ok_)) { + if (V8_LIKELY(ObjectRelatedWithRtt(obj, rtt))) { + CALL_INTERFACE(BrOnCastFail, obj, rtt, &result_on_fallthrough, + branch_depth.depth); + } else { + // Drop {rtt} in the interface. + CALL_INTERFACE(Drop); + // Otherwise the types are unrelated. Always branch. + CALL_INTERFACE(BrOrRet, branch_depth.depth, 0); + // We know that the following code is not reachable, but according + // to the spec it technically is. Set it to spec-only reachable. + SetSucceedingCodeDynamicallyUnreachable(); + } + c->br_merge()->reached = true; + } + // Make sure the correct value is on the stack state on fallthrough. + Drop(obj); + Push(result_on_fallthrough); + return opcode_length + branch_depth.length; + } #define ABSTRACT_TYPE_CHECK(heap_type) \ case kExprRefIs##heap_type: { \ Value arg = Peek(0, 0, kWasmAnyRef); \ diff --git a/src/wasm/graph-builder-interface.cc b/src/wasm/graph-builder-interface.cc index c8b7d1834e..a81457faa7 100644 --- a/src/wasm/graph-builder-interface.cc +++ b/src/wasm/graph-builder-interface.cc @@ -1037,28 +1037,37 @@ class WasmGraphBuildingInterface { TFNode*, TFNode*, StaticKnowledge, TFNode**, TFNode**, TFNode**, TFNode**)> void BrOnCastAbs(FullDecoder* decoder, const Value& object, const Value& rtt, - Value* value_on_branch, uint32_t br_depth) { + Value* forwarding_value, uint32_t br_depth, + bool branch_on_match) { StaticKnowledge config = ComputeStaticKnowledge(object.type, rtt.type, decoder->module_); - SsaEnv* match_env = Split(decoder->zone(), ssa_env_); - SsaEnv* no_match_env = Steal(decoder->zone(), ssa_env_); - no_match_env->SetNotMerged(); + SsaEnv* branch_env = Split(decoder->zone(), ssa_env_); + SsaEnv* no_branch_env = Steal(decoder->zone(), ssa_env_); + no_branch_env->SetNotMerged(); + SsaEnv* match_env = branch_on_match ? branch_env : no_branch_env; + SsaEnv* no_match_env = branch_on_match ? no_branch_env : branch_env; (builder_->*branch_function)(object.node, rtt.node, config, &match_env->control, &match_env->effect, &no_match_env->control, &no_match_env->effect); - builder_->SetControl(no_match_env->control); - SetEnv(match_env); - value_on_branch->node = object.node; + builder_->SetControl(no_branch_env->control); + SetEnv(branch_env); + forwarding_value->node = object.node; // Currently, br_on_* instructions modify the value stack before calling // the interface function, so we don't need to drop any values here. BrOrRet(decoder, br_depth, 0); - SetEnv(no_match_env); + SetEnv(no_branch_env); } void BrOnCast(FullDecoder* decoder, const Value& object, const Value& rtt, Value* value_on_branch, uint32_t br_depth) { BrOnCastAbs<&compiler::WasmGraphBuilder::BrOnCast>( - decoder, object, rtt, value_on_branch, br_depth); + decoder, object, rtt, value_on_branch, br_depth, true); + } + + void BrOnCastFail(FullDecoder* decoder, const Value& object, const Value& rtt, + Value* value_on_fallthrough, uint32_t br_depth) { + BrOnCastAbs<&compiler::WasmGraphBuilder::BrOnCast>( + decoder, object, rtt, value_on_fallthrough, br_depth, false); } void RefIsData(FullDecoder* decoder, const Value& object, Value* result) { @@ -1073,8 +1082,8 @@ class WasmGraphBuildingInterface { void BrOnData(FullDecoder* decoder, const Value& object, Value* value_on_branch, uint32_t br_depth) { BrOnCastAbs<&compiler::WasmGraphBuilder::BrOnData>( - decoder, object, Value{nullptr, kWasmBottom}, value_on_branch, - br_depth); + decoder, object, Value{nullptr, kWasmBottom}, value_on_branch, br_depth, + true); } void RefIsFunc(FullDecoder* decoder, const Value& object, Value* result) { @@ -1089,8 +1098,8 @@ class WasmGraphBuildingInterface { void BrOnFunc(FullDecoder* decoder, const Value& object, Value* value_on_branch, uint32_t br_depth) { BrOnCastAbs<&compiler::WasmGraphBuilder::BrOnFunc>( - decoder, object, Value{nullptr, kWasmBottom}, value_on_branch, - br_depth); + decoder, object, Value{nullptr, kWasmBottom}, value_on_branch, br_depth, + true); } void RefIsI31(FullDecoder* decoder, const Value& object, Value* result) { @@ -1104,8 +1113,8 @@ class WasmGraphBuildingInterface { void BrOnI31(FullDecoder* decoder, const Value& object, Value* value_on_branch, uint32_t br_depth) { BrOnCastAbs<&compiler::WasmGraphBuilder::BrOnI31>( - decoder, object, Value{nullptr, kWasmBottom}, value_on_branch, - br_depth); + decoder, object, Value{nullptr, kWasmBottom}, value_on_branch, br_depth, + true); } void Forward(FullDecoder* decoder, const Value& from, Value* to) { diff --git a/src/wasm/wasm-opcodes-inl.h b/src/wasm/wasm-opcodes-inl.h index a63b46ee0e..bc14a4adef 100644 --- a/src/wasm/wasm-opcodes-inl.h +++ b/src/wasm/wasm-opcodes-inl.h @@ -401,6 +401,7 @@ constexpr const char* WasmOpcodes::OpcodeName(WasmOpcode opcode) { CASE_OP(RefTest, "ref.test") CASE_OP(RefCast, "ref.cast") CASE_OP(BrOnCast, "br_on_cast") + CASE_OP(BrOnCastFail, "br_on_cast_fail") CASE_OP(RefIsFunc, "ref.is_func") CASE_OP(RefIsData, "ref.is_data") CASE_OP(RefIsI31, "ref.is_i31") diff --git a/src/wasm/wasm-opcodes.h b/src/wasm/wasm-opcodes.h index 8bd8d1d463..5de6892124 100644 --- a/src/wasm/wasm-opcodes.h +++ b/src/wasm/wasm-opcodes.h @@ -671,6 +671,7 @@ bool V8_EXPORT_PRIVATE IsJSCompatibleSignature(const FunctionSig* sig, V(RefTest, 0xfb40, _) \ V(RefCast, 0xfb41, _) \ V(BrOnCast, 0xfb42, _) \ + V(BrOnCastFail, 0xfb43, _) \ V(RefIsFunc, 0xfb50, _) \ V(RefIsData, 0xfb51, _) \ V(RefIsI31, 0xfb52, _) \ diff --git a/test/cctest/wasm/test-gc.cc b/test/cctest/wasm/test-gc.cc index 7c62fa64da..9016711aab 100644 --- a/test/cctest/wasm/test-gc.cc +++ b/test/cctest/wasm/test-gc.cc @@ -457,6 +457,63 @@ WASM_COMPILED_EXEC_TEST(BrOnCast) { tester.CheckResult(kTypedAfterBranch, 42); } +WASM_COMPILED_EXEC_TEST(BrOnCastFail) { + WasmGCTester tester(execution_tier); + ValueType kDataRefNull = ValueType::Ref(HeapType::kData, kNullable); + const byte type0 = tester.DefineStruct({F(kWasmI32, true)}); + const byte type1 = + tester.DefineStruct({F(kWasmI64, true), F(kWasmI32, true)}); + + const int field0_value = 5; + const int field1_value = 25; + const int null_value = 45; + + // local_0 = value; + // if (!(local_0 instanceof type0)) goto block1; + // return static_cast(local_0).field_0; + // block1: + // if (local_0 == nullptr) goto block2; + // return static_cast(local_0).field_1; + // block2: + // return null_value; +#define FUNCTION_BODY(value) \ + WASM_LOCAL_SET(0, WASM_SEQ(value)), \ + WASM_BLOCK( \ + WASM_BLOCK_R(kDataRefNull, WASM_LOCAL_GET(0), \ + WASM_BR_ON_CAST_FAIL(0, WASM_RTT_CANON(type0)), \ + WASM_GC_OP(kExprStructGet), type0, 0, kExprReturn), \ + kExprBrOnNull, 0, WASM_RTT_CANON(type1), WASM_GC_OP(kExprRefCast), \ + WASM_GC_OP(kExprStructGet), type1, 1, kExprReturn), \ + WASM_I32V(null_value), kExprEnd + + const byte kBranchTaken = + tester.DefineFunction(tester.sigs.i_v(), {kDataRefNull}, + {FUNCTION_BODY(WASM_STRUCT_NEW_WITH_RTT( + type1, WASM_I64V(10), WASM_I32V(field1_value), + WASM_RTT_CANON(type1)))}); + + const byte kBranchNotTaken = tester.DefineFunction( + tester.sigs.i_v(), {kDataRefNull}, + {FUNCTION_BODY(WASM_STRUCT_NEW_WITH_RTT(type0, WASM_I32V(field0_value), + WASM_RTT_CANON(type0)))}); + + const byte kNull = tester.DefineFunction( + tester.sigs.i_v(), {kDataRefNull}, {FUNCTION_BODY(WASM_REF_NULL(type0))}); + + const byte kUnrelatedTypes = tester.DefineFunction( + tester.sigs.i_v(), {ValueType::Ref(type1, kNullable)}, + {FUNCTION_BODY(WASM_STRUCT_NEW_WITH_RTT(type1, WASM_I64V(10), + WASM_I32V(field1_value), + WASM_RTT_CANON(type1)))}); +#undef FUNCTION_BODY + + tester.CompileModule(); + tester.CheckResult(kBranchTaken, field1_value); + tester.CheckResult(kBranchNotTaken, field0_value); + tester.CheckResult(kNull, null_value); + tester.CheckResult(kUnrelatedTypes, field1_value); +} + WASM_COMPILED_EXEC_TEST(WasmRefEq) { WasmGCTester tester(execution_tier); byte type_index = tester.DefineStruct({F(kWasmI32, true), F(kWasmI32, true)}); diff --git a/test/common/wasm/wasm-macro-gen.h b/test/common/wasm/wasm-macro-gen.h index a8bc20cb48..87b60e40c6 100644 --- a/test/common/wasm/wasm-macro-gen.h +++ b/test/common/wasm/wasm-macro-gen.h @@ -519,6 +519,8 @@ inline WasmOpcode LoadStoreOpcodeOf(MachineType type, bool store) { // conditional branches. #define WASM_BR_ON_CAST(depth, rtt) \ rtt, WASM_GC_OP(kExprBrOnCast), static_cast(depth) +#define WASM_BR_ON_CAST_FAIL(depth, rtt) \ + rtt, WASM_GC_OP(kExprBrOnCastFail), static_cast(depth) #define WASM_REF_IS_DATA(ref) ref, WASM_GC_OP(kExprRefIsData) #define WASM_REF_AS_DATA(ref) ref, WASM_GC_OP(kExprRefAsData) diff --git a/test/mjsunit/wasm/wasm-module-builder.js b/test/mjsunit/wasm/wasm-module-builder.js index 62b286fde9..ca813a8a87 100644 --- a/test/mjsunit/wasm/wasm-module-builder.js +++ b/test/mjsunit/wasm/wasm-module-builder.js @@ -466,6 +466,7 @@ let kExprRttSub = 0x31; let kExprRefTest = 0x40; let kExprRefCast = 0x41; let kExprBrOnCast = 0x42; +let kExprBrOnCastFail = 0x43; let kExprRefIsFunc = 0x50; let kExprRefIsData = 0x51; let kExprRefIsI31 = 0x52; diff --git a/test/unittests/wasm/function-body-decoder-unittest.cc b/test/unittests/wasm/function-body-decoder-unittest.cc index 29809766c3..7dbb0e91d0 100644 --- a/test/unittests/wasm/function-body-decoder-unittest.cc +++ b/test/unittests/wasm/function-body-decoder-unittest.cc @@ -4331,7 +4331,6 @@ TEST_F(FunctionBodyDecoderTest, RefTestCast) { WASM_FEATURE_SCOPE(reftypes); WASM_FEATURE_SCOPE(typed_funcref); WASM_FEATURE_SCOPE(gc); - WASM_FEATURE_SCOPE(eh); TestModuleBuilder builder; module = builder.module(); @@ -4424,6 +4423,77 @@ TEST_F(FunctionBodyDecoderTest, RefTestCast) { "found i32.const of type i32"); } +TEST_F(FunctionBodyDecoderTest, BrOnCastOrCastFail) { + WASM_FEATURE_SCOPE(reftypes); + WASM_FEATURE_SCOPE(typed_funcref); + WASM_FEATURE_SCOPE(gc); + + TestModuleBuilder builder; + module = builder.module(); + + byte super_struct = builder.AddStruct({F(kWasmI16, true)}); + byte sub_struct = builder.AddStruct({F(kWasmI16, true), F(kWasmI32, false)}); + + ValueType supertype = ValueType::Ref(super_struct, kNullable); + ValueType subtype = ValueType::Ref(sub_struct, kNullable); + + ExpectValidates( + FunctionSig::Build(this->zone(), {kWasmI32, subtype}, {supertype}), + {WASM_I32V(42), WASM_LOCAL_GET(0), + WASM_BR_ON_CAST(0, WASM_RTT_CANON(sub_struct)), + WASM_RTT_CANON(sub_struct), WASM_GC_OP(kExprRefCast)}); + ExpectValidates( + FunctionSig::Build(this->zone(), {kWasmI32, supertype}, {supertype}), + {WASM_I32V(42), WASM_LOCAL_GET(0), + WASM_BR_ON_CAST_FAIL(0, WASM_RTT_CANON(sub_struct))}); + + // Wrong branch type. + ExpectFailure( + FunctionSig::Build(this->zone(), {}, {supertype}), + {WASM_LOCAL_GET(0), WASM_BR_ON_CAST(0, WASM_RTT_CANON(sub_struct)), + WASM_UNREACHABLE}, + kAppendEnd, "br_on_cast must target a branch of arity at least 1"); + ExpectFailure( + FunctionSig::Build(this->zone(), {subtype}, {supertype}), + {WASM_I32V(42), WASM_LOCAL_GET(0), + WASM_BR_ON_CAST_FAIL(0, WASM_RTT_CANON(sub_struct))}, + kAppendEnd, + "type error in branch[0] (expected (ref null 1), got (ref null 0))"); + + // Wrong fallthrough type. + ExpectFailure( + FunctionSig::Build(this->zone(), {subtype}, {supertype}), + {WASM_LOCAL_GET(0), WASM_BR_ON_CAST(0, WASM_RTT_CANON(sub_struct))}, + kAppendEnd, + "type error in fallthru[0] (expected (ref null 1), got (ref null 0))"); + ExpectFailure( + FunctionSig::Build(this->zone(), {supertype}, {supertype}), + {WASM_BLOCK_I(WASM_LOCAL_GET(0), + WASM_BR_ON_CAST_FAIL(0, WASM_RTT_CANON(sub_struct)))}, + kAppendEnd, "type error in branch[0] (expected i32, got (ref null 0))"); + + // Argument type error. + ExpectFailure( + FunctionSig::Build(this->zone(), {subtype}, {kWasmAnyRef}), + {WASM_LOCAL_GET(0), WASM_BR_ON_CAST(0, WASM_RTT_CANON(sub_struct)), + WASM_RTT_CANON(sub_struct), WASM_GC_OP(kExprRefCast)}, + kAppendEnd, + "br_on_cast[0] expected subtype of (ref null func) or (ref null data), " + "found local.get of type anyref"); + ExpectFailure( + FunctionSig::Build(this->zone(), {supertype}, {kWasmAnyRef}), + {WASM_LOCAL_GET(0), WASM_BR_ON_CAST_FAIL(0, WASM_RTT_CANON(sub_struct))}, + kAppendEnd, + "br_on_cast_fail[0] expected subtype of (ref null func) or (ref null " + "data), found local.get of type anyref"); + + // Trivial rtt type error. + ExpectFailure(FunctionSig::Build(this->zone(), {supertype}, {supertype}), + {WASM_LOCAL_GET(0), WASM_BR_ON_CAST_FAIL(0, WASM_I64V(42))}, + kAppendEnd, + "br_on_cast_fail[1] expected rtt, found i64.const of type i64"); +} + TEST_F(FunctionBodyDecoderTest, LocalTeeTyping) { WASM_FEATURE_SCOPE(reftypes); WASM_FEATURE_SCOPE(typed_funcref);