[wasm-gc] Fix nullability typing for new br_on_cast instructions

Bug: v8:7748
Change-Id: I6bbbdc2378b1b6760e1ddd1f024b57d8c8f4a50d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4100909
Reviewed-by: Manos Koukoutos <manoskouk@chromium.org>
Commit-Queue: Matthias Liedtke <mliedtke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84898}
This commit is contained in:
Matthias Liedtke 2022-12-16 11:40:37 +01:00 committed by V8 LUCI CQ
parent 94f80d253c
commit b87fd354d1
5 changed files with 189 additions and 36 deletions

View File

@ -5214,9 +5214,7 @@ class WasmFullDecoder : public WasmDecoder<ValidationTag, decoding_mode> {
Drop(obj);
bool null_succeeds = opcode == kExprBrOnCastNull;
Push(CreateValue(ValueType::RefMaybeNull(
imm.type, (obj.type.is_bottom() || !null_succeeds)
? kNonNullable
: obj.type.nullability())));
target_type, null_succeeds ? kNullable : kNonNullable)));
// The {value_on_branch} parameter we pass to the interface must
// be pointer-identical to the object on the stack.
Value* value_on_branch = stack_value(1);
@ -5258,6 +5256,17 @@ class WasmFullDecoder : public WasmDecoder<ValidationTag, decoding_mode> {
Drop(1); // value_on_branch
Push(obj); // Restore stack state on fallthrough.
if (current_code_reachable_and_ok_ && null_succeeds) {
// As null branches, the type on fallthrough will be the non-null
// variant of the input type.
// Note that this is handled differently for br_on_cast_fail for which
// the Forward is handled by TurboFan.
// TODO(mliedtke): This currently deviates from the spec and is
// discussed at
// https://github.com/WebAssembly/gc/issues/342#issuecomment-1354505307.
stack_value(1)->type = obj.type.AsNonNull();
CALL_INTERFACE(Forward, obj, stack_value(1));
}
return pc_offset;
}
case kExprBrOnCastDeprecated: {
@ -5380,9 +5389,19 @@ class WasmFullDecoder : public WasmDecoder<ValidationTag, decoding_mode> {
return 0;
}
if (!VALIDATE(TypeCheckBranch<true>(c, 0))) return 0;
bool null_succeeds = opcode == kExprBrOnCastFailNull;
Value result_on_fallthrough = CreateValue(ValueType::Ref(target_type));
if (null_succeeds) {
// If null is treated as a successful cast, then the branch type is
// guaranteed to be non-null.
Drop(obj);
Push(CreateValue(obj.type.AsNonNull()));
}
if (!VALIDATE(TypeCheckBranch<true>(c, 0))) return 0;
Value result_on_fallthrough = CreateValue(ValueType::RefMaybeNull(
target_type, (obj.type.is_bottom() || !null_succeeds)
? kNonNullable
: obj.type.nullability()));
if (V8_LIKELY(current_code_reachable_and_ok_)) {
// This logic ensures that code generation can assume that functions
// can only be cast between compatible types.
@ -5393,6 +5412,7 @@ class WasmFullDecoder : public WasmDecoder<ValidationTag, decoding_mode> {
}
// The types are incompatible (i.e. neither of the two types is a
// subtype of the other). Always branch.
CALL_INTERFACE(Forward, obj, stack_value(1));
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.

View File

@ -1278,20 +1278,31 @@ class WasmGraphBuildingInterface {
&match_env->control, &match_env->effect,
&no_match_env->control, &no_match_env->effect);
builder_->SetControl(no_branch_env->control);
SetEnv(branch_env);
if (branch_on_match) {
SetEnv(branch_env);
// Narrow type for the successful cast target branch.
SetAndTypeNode(forwarding_value,
builder_->TypeGuard(object.node, forwarding_value->type));
}
// 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_branch_env);
if (!branch_on_match) {
Forward(decoder, object, forwarding_value);
// 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_branch_env);
// Note: Differently to below for !{branch_on_match}, we do not Forward
// the value here to perform a TypeGuard. It can't be done here due to
// asymmetric decoder code. A Forward here would be poped from the stack
// and ignored by the decoder. Therefore the decoder has to call Forward
// itself.
} else {
SetEnv(branch_env);
// It is necessary in case of {null_succeeds} to forward the value.
// This will add a TypeGuard to the non-null type (as in this case the
// object is non-nullable).
Forward(decoder, object, decoder->stack_value(1));
BrOrRet(decoder, br_depth, 0);
SetEnv(no_branch_env);
// Narrow type for the successful cast fallthrough branch.
SetAndTypeNode(forwarding_value,
builder_->TypeGuard(object.node, forwarding_value->type));
Forward(decoder, object, forwarding_value);
}
}
@ -1345,26 +1356,32 @@ class WasmGraphBuildingInterface {
}
void BrOnCastFailAbstract(FullDecoder* decoder, const Value& object,
HeapType type, Value* value_on_branch,
HeapType type, Value* value_on_fallthrough,
uint32_t br_depth, bool null_succeeds) {
switch (type.representation()) {
case HeapType::kEq:
return BrOnNonEq(decoder, object, value_on_branch, br_depth,
return BrOnNonEq(decoder, object, value_on_fallthrough, br_depth,
null_succeeds);
case HeapType::kI31:
return BrOnNonI31(decoder, object, value_on_branch, br_depth,
return BrOnNonI31(decoder, object, value_on_fallthrough, br_depth,
null_succeeds);
case HeapType::kStruct:
return BrOnNonStruct(decoder, object, value_on_branch, br_depth,
return BrOnNonStruct(decoder, object, value_on_fallthrough, br_depth,
null_succeeds);
case HeapType::kArray:
return BrOnNonArray(decoder, object, value_on_branch, br_depth,
return BrOnNonArray(decoder, object, value_on_fallthrough, br_depth,
null_succeeds);
case HeapType::kNone:
case HeapType::kNoExtern:
case HeapType::kNoFunc:
DCHECK(null_succeeds);
return BrOnNonNull(decoder, object, value_on_branch, br_depth, true);
// We need to store a node in the stack where the decoder so far only
// pushed a value and expects the `BrOnCastFailAbstract` to set it.
// TODO(7748): The compiler shouldn't have to access the stack used by
// the decoder ideally.
Forward(decoder, object, decoder->stack_value(1));
return BrOnNonNull(decoder, object, value_on_fallthrough, br_depth,
true);
case HeapType::kAny:
// Any may never need a cast as it is either implicitly convertible or
// never convertible for any given type.

View File

@ -59,3 +59,114 @@ d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
assertEquals(-1, wasm.refCastRemovesNullability(-1));
assertTraps(kTrapIllegalCast, () => wasm.refCastRemovesNullability(null));
})();
(function TestBrOnCastNullability() {
print(arguments.callee.name);
let builder = new WasmModuleBuilder();
let consumeNonNull =
builder.addFunction(`consumeNonNull`,
makeSig([wasmRefType(kWasmAnyRef)], []))
.addBody([]);
let i31ToI32 =
builder.addFunction(`i31ToI32`,
makeSig([wasmRefType(kWasmI31Ref)], [kWasmI32]))
.addBody([kExprLocalGet, 0, kGCPrefix, kExprI31GetS]);
builder.addFunction(`brOnCastNullNonNullOnPassThrough`,
makeSig([kWasmExternRef], [kWasmI32]))
.addBody([
kExprBlock, kWasmRefNull, kI31RefCode,
kExprLocalGet, 0,
kGCPrefix, kExprExternInternalize,
kGCPrefix, kExprBrOnCastNull, 0, kI31RefCode,
// As null branches, the type here is guaranteed to be non-null.
kExprCallFunction, consumeNonNull.index,
kExprI32Const, 0,
kExprReturn,
kExprEnd,
kGCPrefix, kExprI31GetS,
kExprReturn,
]).exportFunc();
builder.addFunction(`brOnCastNonNullOnBranch`,
makeSig([kWasmExternRef], [kWasmI32]))
.addBody([
// The value is guaranteed to be non-null on branch.
kExprBlock, kWasmRef, kI31RefCode,
kExprLocalGet, 0,
kGCPrefix, kExprExternInternalize,
kGCPrefix, kExprBrOnCast, 0, kI31RefCode,
kExprDrop,
kExprI32Const, 0,
kExprReturn,
kExprEnd,
kExprCallFunction, i31ToI32.index,
kExprReturn,
]).exportFunc();
let instance = builder.instantiate();
let wasm = instance.exports;
assertTraps(kTrapNullDereference, () => wasm.brOnCastNullNonNullOnPassThrough(null));
assertEquals(42, wasm.brOnCastNullNonNullOnPassThrough(42));
assertEquals(0, wasm.brOnCastNullNonNullOnPassThrough("cast fails"));
assertEquals(0, wasm.brOnCastNonNullOnBranch(null));
assertEquals(42, wasm.brOnCastNonNullOnBranch(42));
assertEquals(0, wasm.brOnCastNonNullOnBranch("cast fails"));
})();
(function TestBrOnCastFailNullability() {
print(arguments.callee.name);
let builder = new WasmModuleBuilder();
let consumeNonNull =
builder.addFunction(`consumeNonNull`,
makeSig([wasmRefType(kWasmAnyRef)], []))
.addBody([]);
let i31ToI32 =
builder.addFunction(`i31ToI32`,
makeSig([wasmRefType(kWasmI31Ref)], [kWasmI32]))
.addBody([kExprLocalGet, 0, kGCPrefix, kExprI31GetS]);
builder.addFunction(`brOnCastFailNonNullOnPassThrough`,
makeSig([kWasmExternRef], [kWasmI32]))
.addBody([
kExprBlock, kWasmRefNull, kAnyRefCode,
kExprLocalGet, 0,
kGCPrefix, kExprExternInternalize,
kGCPrefix, kExprBrOnCastFail, 0, kI31RefCode,
// As null branches, the type here is guaranteed to be non-null.
kExprCallFunction, i31ToI32.index,
kExprReturn,
kExprEnd,
kExprDrop,
kExprI32Const, 1,
kExprReturn,
]).exportFunc();
builder.addFunction(`brOnCastFailNullNonNullOnBranch`,
makeSig([kWasmExternRef], [kWasmI32]))
.addBody([
// The value is guaranteed to be non-null on branch.
kExprBlock, kWasmRef, kAnyRefCode,
kExprLocalGet, 0,
kGCPrefix, kExprExternInternalize,
kGCPrefix, kExprBrOnCastFailNull, 0, kI31RefCode,
kGCPrefix, kExprI31GetS,
kExprReturn,
kExprEnd,
kExprCallFunction, consumeNonNull.index,
kExprI32Const, 1,
kExprReturn,
]).exportFunc();
let instance = builder.instantiate();
let wasm = instance.exports;
assertEquals(1, wasm.brOnCastFailNonNullOnPassThrough(null));
assertEquals(42, wasm.brOnCastFailNonNullOnPassThrough(42));
assertEquals(1, wasm.brOnCastFailNonNullOnPassThrough("cast fails"));
assertTraps(kTrapNullDereference, () => wasm.brOnCastFailNullNonNullOnBranch(null));
assertEquals(42, wasm.brOnCastFailNullNonNullOnBranch(42));
assertEquals(1, wasm.brOnCastFailNullNonNullOnBranch("cast fails"));
})();

View File

@ -263,7 +263,7 @@ d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
builder.addFunction(`brOnCastFailNull_${name}`,
makeSig([kWasmFuncRef], [kWasmI32]))
.addBody([
kExprBlock, kFuncRefCode,
kExprBlock, kWasmRef, kFuncRefCode,
kExprLocalGet, 0,
kGCPrefix, kExprBrOnCastFailNull, 0, typeCode,
kExprI32Const, 0,
@ -542,7 +542,7 @@ d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
builder.addFunction('castFailNullToExternRef',
makeSig([kWasmExternRef], [kWasmI32]))
.addBody([
kExprBlock, kWasmRefNull, kExternRefCode,
kExprBlock, kWasmRef, kExternRefCode,
kExprLocalGet, 0,
kGCPrefix, kExprBrOnCastFailNull, 0, kExternRefCode,
kExprI32Const, 0,
@ -556,7 +556,7 @@ d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
builder.addFunction('castFailNullToNullExternRef',
makeSig([kWasmExternRef], [kWasmI32]))
.addBody([
kExprBlock, kWasmRefNull, kExternRefCode,
kExprBlock, kWasmRef, kExternRefCode,
kExprLocalGet, 0,
kGCPrefix, kExprBrOnCastFailNull, 0, kNullExternRefCode,
kExprI32Const, 0,
@ -629,6 +629,11 @@ d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
let structSub = builder.addStruct([makeField(kWasmI32, true)], structSuper);
let array = builder.addArray(kWasmI32);
// Helpers to be able to instantiate a true externref value from wasm.
let createExternSig = builder.addType(makeSig([], [kWasmExternRef]));
let createExternIdx = builder.addImport('import', 'createExtern', createExternSig);
let createExtern = () => undefined;
let types = {
any: kWasmAnyRef,
eq: kWasmEqRef,
@ -646,6 +651,7 @@ d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
structSuper: [kExprI32Const, 42, kGCPrefix, kExprStructNew, structSuper],
structSub: [kExprI32Const, 42, kGCPrefix, kExprStructNew, structSub],
array: [kExprI32Const, 42, kGCPrefix, kExprArrayNewFixed, array, 1],
any: [kExprCallFunction, createExternIdx, kGCPrefix, kExprExternInternalize],
};
// Each Test lists the following:
@ -657,9 +663,9 @@ d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
let tests = [
{
source: 'any',
values: ['nullref', 'i31ref', 'structSuper', 'structSub', 'array'],
values: ['nullref', 'i31ref', 'structSuper', 'structSub', 'array', 'any'],
targets: {
any: ['i31ref', 'structSuper', 'structSub', 'array'],
any: ['i31ref', 'structSuper', 'structSub', 'array', 'any'],
eq: ['i31ref', 'structSuper', 'structSub', 'array'],
struct: ['structSuper', 'structSub'],
anyArray: ['array'],
@ -825,7 +831,7 @@ d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
builder.addFunction(`brOnCastFailNull_${test.source}_to_${target}`,
makeSig([wasmRefType(creatorType)], [kWasmI32]))
.addBody([
kExprBlock, kWasmRefNull, sourceHeapType,
kExprBlock, kWasmRef, sourceHeapType,
kExprLocalGet, 0,
kExprCallRef, ...wasmUnsignedLeb(creatorType),
kGCPrefix, kExprBrOnCastFailNull, 0, heapType,
@ -840,7 +846,7 @@ d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
}
}
let instance = builder.instantiate();
let instance = builder.instantiate({import: {createExtern}});
let wasm = instance.exports;
for (let test of tests) {

View File

@ -4360,12 +4360,11 @@ TEST_F(FunctionBodyDecoderTest, BrOnCastOrCastFail) {
{WASM_I32V(42), WASM_LOCAL_GET(0), WASM_BR_ON_CAST_FAIL(0, sub_struct)},
kAppendEnd,
"type error in branch[0] (expected (ref null 1), got (ref null 0))");
ExpectFailure(
FunctionSig::Build(this->zone(), {subtype}, {supertype}),
{WASM_I32V(42), WASM_LOCAL_GET(0),
WASM_BR_ON_CAST_FAIL_NULL(0, sub_struct)},
kAppendEnd,
"type error in branch[0] (expected (ref null 1), got (ref null 0))");
ExpectFailure(FunctionSig::Build(this->zone(), {subtype}, {supertype}),
{WASM_I32V(42), WASM_LOCAL_GET(0),
WASM_BR_ON_CAST_FAIL_NULL(0, sub_struct)},
kAppendEnd,
"type error in branch[0] (expected (ref null 1), got (ref 0))");
// Wrong fallthrough type.
ExpectFailure(
@ -4380,7 +4379,7 @@ TEST_F(FunctionBodyDecoderTest, BrOnCastOrCastFail) {
{WASM_BLOCK_I(WASM_LOCAL_GET(0),
WASM_BR_ON_CAST_FAIL_NULL(0, sub_struct))},
kAppendEnd,
"type error in branch[0] (expected i32, got (ref null 0))");
"type error in branch[0] (expected i32, got (ref 0))");
// Argument type error.
ExpectFailure(FunctionSig::Build(this->zone(), {subtype}, {kWasmExternRef}),