[wasm] Update br_table with the latest spec changes

The typing of br_table was relaxed in
https://github.com/WebAssembly/spec/pull/1305. Before, we had to compute
the greatest lower bound of all branch types and make sure that stack
values are subtypes of that type. Now, we have to check that the stack
values are subtypes of each individual branch. This makes a difference
only in polymorphic stacks, but greatly simplifies the code, especially
with the upcoming introduction of a much more complex type system in
wasm-gc.

Change-Id: I6e3b410cfe0e71a97623b3030b3575ef707c4900
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2827897
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73982}
This commit is contained in:
Manos Koukoutos 2021-04-15 15:42:58 +00:00 committed by Commit Bot
parent be9ff65a06
commit 05b385887e
6 changed files with 81 additions and 146 deletions

View File

@ -2874,40 +2874,40 @@ class WasmFullDecoder : public WasmDecoder<validate> {
// all branch targets as reachable after the {CALL_INTERFACE} call.
std::vector<bool> br_targets(control_.size());
// The result types of the br_table instruction. We have to check the
// stack against these types. Only needed during validation.
std::vector<ValueType> result_types;
uint32_t arity = 0;
while (iterator.has_next()) {
const uint32_t index = iterator.cur_index();
const byte* pos = iterator.pc();
uint32_t target = iterator.next();
if (!VALIDATE(ValidateBrTableTarget(target, pos, index))) return 0;
const uint32_t target = iterator.next();
if (!VALIDATE(target < control_depth())) {
this->DecodeError(pos, "invalid branch depth: %u", target);
return 0;
}
// Avoid redundant branch target checks.
if (br_targets[target]) continue;
br_targets[target] = true;
if (validate) {
if (index == 0) {
// With the first branch target, initialize the result types.
result_types = InitializeBrTableResultTypes(target);
} else if (!UpdateBrTableResultTypes(&result_types, target, pos,
index)) {
arity = control_at(target)->br_merge()->arity;
} else if (!VALIDATE(control_at(target)->br_merge()->arity == arity)) {
this->DecodeError(
pos, "br_table: label arity inconsistent with previous arity %d",
arity);
return 0;
}
TypeCheckBranchResult check_result =
TypeCheckBranch(control_at(target), false, 1);
if (V8_UNLIKELY(check_result == kInvalidStack)) return 0;
}
}
if (!VALIDATE(TypeCheckBrTable(result_types, 1))) return 0;
DCHECK(this->ok());
if (current_code_reachable_and_ok_) {
CALL_INTERFACE_IF_OK_AND_REACHABLE(BrTable, imm, key);
for (int i = 0, e = control_depth(); i < e; ++i) {
if (!br_targets[i]) continue;
control_at(i)->br_merge()->reached = true;
for (uint32_t i = 0; i < control_depth(); ++i) {
control_at(i)->br_merge()->reached |= br_targets[i];
}
}
Drop(key);
@ -3726,92 +3726,6 @@ class WasmFullDecoder : public WasmDecoder<validate> {
return prefix_len + imm.length;
}
bool ValidateBrTableTarget(uint32_t target, const byte* pos, int index) {
if (!VALIDATE(target < this->control_.size())) {
this->DecodeError(pos, "improper branch in br_table target %u (depth %u)",
index, target);
return false;
}
return true;
}
std::vector<ValueType> InitializeBrTableResultTypes(uint32_t target) {
Merge<Value>* merge = control_at(target)->br_merge();
int br_arity = merge->arity;
std::vector<ValueType> result(br_arity);
for (int i = 0; i < br_arity; ++i) {
result[i] = (*merge)[i].type;
}
return result;
}
bool UpdateBrTableResultTypes(std::vector<ValueType>* result_types,
uint32_t target, const byte* pos, int index) {
Merge<Value>* merge = control_at(target)->br_merge();
int br_arity = merge->arity;
// First we check if the arities match.
if (!VALIDATE(br_arity == static_cast<int>(result_types->size()))) {
this->DecodeError(pos,
"inconsistent arity in br_table target %u (previous "
"was %zu, this one is %u)",
index, result_types->size(), br_arity);
return false;
}
for (int i = 0; i < br_arity; ++i) {
if (this->enabled_.has_reftypes()) {
// The expected type is the biggest common sub type of all targets.
(*result_types)[i] =
CommonSubtype((*result_types)[i], (*merge)[i].type, this->module_);
} else {
// All target must have the same signature.
if (!VALIDATE((*result_types)[i] == (*merge)[i].type)) {
this->DecodeError(pos,
"inconsistent type in br_table target %u (previous "
"was %s, this one is %s)",
index, (*result_types)[i].name().c_str(),
(*merge)[i].type.name().c_str());
return false;
}
}
}
return true;
}
bool TypeCheckBrTable(const std::vector<ValueType>& result_types,
uint32_t drop_values) {
int br_arity = static_cast<int>(result_types.size());
if (V8_LIKELY(!control_.back().unreachable())) {
int available =
static_cast<int>(stack_size()) - control_.back().stack_depth;
available -= std::min(available, static_cast<int>(drop_values));
// There have to be enough values on the stack.
if (!VALIDATE(available >= br_arity)) {
this->DecodeError(
"expected %u elements on the stack for branch to @%d, found %u",
br_arity, startrel(control_.back().pc()), available);
return false;
}
Value* stack_values = stack_end_ - br_arity - drop_values;
// Type-check the topmost br_arity values on the stack.
for (int i = 0; i < br_arity; ++i) {
Value& val = stack_values[i];
if (!VALIDATE(IsSubtypeOf(val.type, result_types[i], this->module_))) {
this->DecodeError("type error in merge[%u] (expected %s, got %s)", i,
result_types[i].name().c_str(),
val.type.name().c_str());
return false;
}
}
} else { // !control_.back().reachable()
// Type-check the values on the stack.
for (int i = 0; i < br_arity; ++i) {
Peek(i + drop_values, i + 1, result_types[i]);
}
}
return this->ok();
}
uint32_t SimdConstOp(uint32_t opcode_length) {
Simd128Immediate<validate> imm(this, this->pc_ + opcode_length);
Value result = CreateValue(kWasmS128);
@ -4701,7 +4615,8 @@ class WasmFullDecoder : public WasmDecoder<validate> {
}
void DoReturn() {
DCHECK_GE(stack_size(), this->sig_->return_count());
DCHECK_IMPLIES(current_code_reachable_and_ok_,
stack_size() >= this->sig_->return_count());
CALL_INTERFACE_IF_OK_AND_REACHABLE(DoReturn, 0);
}
@ -4840,30 +4755,35 @@ class WasmFullDecoder : public WasmDecoder<validate> {
// For more descriptive call sites:
V8_INLINE void Drop(const Value& /* unused */) { Drop(1); }
// Pops values from the stack, as defined by {merge}. Thereby we type-check
// unreachable merges. Afterwards the values are pushed again on the stack
// according to the signature in {merge}. This is done so follow-up validation
// is possible.
bool TypeCheckUnreachableMerge(Merge<Value>& merge, bool conditional_branch,
uint32_t drop_values = 0) {
// Check if any values that may exist on top of the stack are compatible with
// {merge}. If {push_branch_values}, push back to the stack values based on
// the type of {merge} (this is needed for conditional branches due to their
// typing rules, and fallthroughs so that the outer control finds enough
// values on the stack).
// TODO(manoskouk): We expect this behavior to change, either due to
// relaxation of dead code verification, or the introduction of subtyping.
// {drop_values} is the number of stack values that will be dropped before the
// branch is taken. This is currently 1 for br (condition), br_table (index)
// and br_on_null (reference), and 0 for all other branches.
bool TypeCheckUnreachableMerge(Merge<Value>& merge, bool push_branch_values,
uint32_t drop_values) {
int arity = merge.arity;
// For conditional branches, stack value '0' is the condition of the branch,
// and the result values start at index '1'.
int index_offset = conditional_branch ? 1 : 0;
for (int i = arity - 1, depth = drop_values; i >= 0; --i, ++depth) {
Peek(depth, index_offset + i, merge[i].type);
Peek(depth, i, merge[i].type);
}
// Push values of the correct type onto the stack.
Drop(drop_values);
Drop(arity);
// {Drop} is adaptive for polymorphic stacks: it might drop fewer values
// than requested. So ensuring stack space here is not redundant.
EnsureStackSpace(arity + drop_values);
for (int i = 0; i < arity; i++) Push(CreateValue(merge[i].type));
// {drop_values} are about to be dropped anyway, so we can forget their
// previous types, but we do have to maintain the correct stack height.
for (uint32_t i = 0; i < drop_values; i++) {
Push(UnreachableValue(this->pc_));
if (push_branch_values) {
Drop(drop_values);
Drop(arity);
// {Drop} is adaptive for polymorphic stacks: it might drop fewer values
// than requested. So ensuring stack space here is not redundant.
EnsureStackSpace(drop_values + arity);
// Push values of the correct type onto the stack.
for (int i = 0; i < arity; i++) Push(CreateValue(merge[i].type));
// {drop_values} are about to be dropped anyway, so we can forget their
// previous types, but we do have to maintain the correct stack height.
for (uint32_t i = 0; i < drop_values; i++) {
Push(UnreachableValue(this->pc_));
}
}
return this->ok();
}
@ -4949,7 +4869,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
}
// Pop all values from the stack for type checking of existing stack
// values.
return TypeCheckUnreachableMerge(merge, false);
return TypeCheckUnreachableMerge(merge, true, 0);
}
enum TypeCheckBranchResult {
@ -4958,12 +4878,17 @@ class WasmFullDecoder : public WasmDecoder<validate> {
kInvalidStack,
};
// If the type code is reachable, check if the current stack values are
// If the current code is reachable, check if the current stack values are
// compatible with a jump to {c}, based on their number and types.
// Otherwise, we have a polymorphic stack: check if any values that may exist
// on top of the stack are compatible with {c}, and push back to the stack
// values based on the type of {c}.
TypeCheckBranchResult TypeCheckBranch(Control* c, bool conditional_branch,
// on top of the stack are compatible with {c}. If {push_branch_values},
// push back to the stack values based on the type of {c} (this is needed for
// conditional branches due to their typing rules, and fallthroughs so that
// the outer control finds enough values on the stack).
// {drop_values} is the number of stack values that will be dropped before the
// branch is taken. This is currently 1 for for br (condition), br_table
// (index) and br_on_null (reference), and 0 for all other branches.
TypeCheckBranchResult TypeCheckBranch(Control* c, bool push_branch_values,
uint32_t drop_values) {
if (V8_LIKELY(control_.back().reachable())) {
// We only do type-checking here. This is only needed during validation.
@ -4987,7 +4912,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
: kInvalidStack;
}
return TypeCheckUnreachableMerge(*c->br_merge(), conditional_branch,
return TypeCheckUnreachableMerge(*c->br_merge(), push_branch_values,
drop_values)
? kUnreachableBranch
: kInvalidStack;

View File

@ -435,13 +435,6 @@ V8_NOINLINE bool EquivalentTypes(ValueType type1, ValueType type2,
module2);
}
ValueType CommonSubtype(ValueType a, ValueType b, const WasmModule* module) {
if (a == b) return a;
if (IsSubtypeOf(a, b, module)) return a;
if (IsSubtypeOf(b, a, module)) return b;
return kWasmBottom;
}
} // namespace wasm
} // namespace internal
} // namespace v8

View File

@ -93,12 +93,6 @@ V8_INLINE bool IsHeapSubtypeOf(uint32_t subtype_index, uint32_t supertype_index,
ValueType::Ref(supertype_index, kNonNullable), module);
}
// Returns the weakest type that is a subtype of both a and b
// (which is currently always one of a, b, or kWasmBottom).
// TODO(manoskouk): Update this once we have settled on a type system for
// reference types.
ValueType CommonSubtype(ValueType a, ValueType b, const WasmModule* module);
} // namespace wasm
} // namespace internal
} // namespace v8

View File

@ -29,5 +29,5 @@ kExprEnd, // @21
assertThrows(
() => {builder.toModule()}, WebAssembly.CompileError,
'WebAssembly.Module(): Compiling function #0:\"main\" failed: ' +
'type error in merge[0] (expected <bot>, got i32) @+57');
'type error in merge[0] (expected f32, got i32) @+57');
})();

View File

@ -125,7 +125,7 @@ run(I, "U (iblock 0 (block br1)) drop", [unr, ...iblock, ...zero, ...block, ...b
run(I, "U (iblock 0 (block 0 brt1)) drop", [unr, ...iblock, ...zero, ...block, ...zero, ...brt1, end, end, drop]);
run(I, "U (block (iblock 0 0 brt01) drop)", [unr, ...block, ...iblock, ...zero, ...zero, ...brt01, end, drop, end]);
run(V, "(iblock (iblock U 0 brt01)) drop", [...iblock, ...iblock, unr, ...zero, ...brt01, end, end, drop]);
run(I, "(block (fblock U 0 brt01) drop)", [...iblock, ...fblock, unr, ...zero, ...brt01, end, drop, end]);
run(I, "(iblock (fblock U 0 brt01) drop 0) drop", [...iblock, ...fblock, unr, ...zero, ...brt01, end, drop, ...zero, end, drop]);
run(I, "(block (fblock U 0 brt01) drop)", [...block, ...fblock, unr, ...zero, ...brt01, end, drop, end]);
run(V, "(iblock (fblock U 0 brt01) drop 0) drop", [...iblock, ...fblock, unr, ...zero, ...brt01, end, drop, ...zero, end, drop]);
run(I, "(iblock (block (U brif 1))", [...iblock, ...block, unr, kExprBrIf, 0, end, end, kExprDrop]);

View File

@ -2644,6 +2644,29 @@ TEST_F(FunctionBodyDecoderTest, BrTable2b) {
WASM_I32V_2(67), 1, BR_TARGET(0), BR_TARGET(1))))});
}
TEST_F(FunctionBodyDecoderTest, BrTableSubtyping) {
WASM_FEATURE_SCOPE(reftypes);
WASM_FEATURE_SCOPE(typed_funcref);
WASM_FEATURE_SCOPE(gc);
TestModuleBuilder builder;
byte supertype1 = builder.AddStruct({F(kWasmI8, true), F(kWasmI16, false)});
byte supertype2 = builder.AddStruct({F(kWasmI8, true)});
byte subtype = builder.AddStruct(
{F(kWasmI8, true), F(kWasmI16, false), F(kWasmI32, true)});
module = builder.module();
ExpectValidates(
sigs.v_v(),
{WASM_BLOCK_R(
wasm::ValueType::Ref(supertype1, kNonNullable),
WASM_BLOCK_R(
wasm::ValueType::Ref(supertype2, kNonNullable),
WASM_STRUCT_NEW_DEFAULT(subtype, WASM_RTT_CANON(subtype)),
WASM_BR_TABLE(WASM_I32V(5), 1, BR_TARGET(0), BR_TARGET(1))),
WASM_UNREACHABLE),
WASM_DROP});
}
TEST_F(FunctionBodyDecoderTest, BrTable_off_end) {
static byte code[] = {B1(WASM_BR_TABLE(WASM_LOCAL_GET(0), 0, BR_TARGET(0)))};
for (size_t len = 1; len < sizeof(code); len++) {