Skip to content

Commit

Permalink
Improve struct RMW validation and interpretation (#7216)
Browse files Browse the repository at this point in the history
Add missing validation rules checking that accessed fields have allowed
types. Copy the proposed upstream spec tests for this validation from
WebAssembly/shared-everything-threads#85 with
minor changes to account for differences in supported text format and
to comment out unsupported tests.

Also implement interpretation for the RMW instructions and add spec
tests testing their execution. It is simpler to implement both
validation and interpretation at once because the proposed upstream
spec tests require both to pass.
  • Loading branch information
tlively authored Jan 15, 2025
1 parent 7453b1b commit c8bfe28
Show file tree
Hide file tree
Showing 5 changed files with 986 additions and 106 deletions.
1 change: 1 addition & 0 deletions scripts/test/fuzzing.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
# the fuzzer does not support struct RMW ops
'gc-atomics.wast',
'gc-atomics-null-refs.wast',
'shared-structs.wast',
# contains too many segments to run in a wasm VM
'limit-segments_disable-bulk-memory.wast',
# https://github.com/WebAssembly/binaryen/issues/7176
Expand Down
66 changes: 64 additions & 2 deletions src/wasm-interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -1701,9 +1701,71 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
return Flow();
}

Flow visitStructRMW(StructRMW* curr) { WASM_UNREACHABLE("TODO"); }
Flow visitStructRMW(StructRMW* curr) {
NOTE_ENTER("StructRMW");
Flow ref = self()->visit(curr->ref);
if (ref.breaking()) {
return ref;
}
Flow value = self()->visit(curr->value);
if (value.breaking()) {
return value;
}
auto data = ref.getSingleValue().getGCData();
if (!data) {
trap("null ref");
}
auto& field = data->values[curr->index];
auto oldVal = field;
auto newVal = value.getSingleValue();
switch (curr->op) {
case RMWAdd:
field = field.add(newVal);
break;
case RMWSub:
field = field.sub(newVal);
break;
case RMWAnd:
field = field.and_(newVal);
break;
case RMWOr:
field = field.or_(newVal);
break;
case RMWXor:
field = field.xor_(newVal);
break;
case RMWXchg:
field = newVal;
break;
}
return oldVal;
}

Flow visitStructCmpxchg(StructCmpxchg* curr) { WASM_UNREACHABLE("TODO"); }
Flow visitStructCmpxchg(StructCmpxchg* curr) {
NOTE_ENTER("StructCmpxchg");
Flow ref = self()->visit(curr->ref);
if (ref.breaking()) {
return ref;
}
Flow expected = self()->visit(curr->expected);
if (expected.breaking()) {
return expected;
}
Flow replacement = self()->visit(curr->replacement);
if (replacement.breaking()) {
return replacement;
}
auto data = ref.getSingleValue().getGCData();
if (!data) {
trap("null ref");
}
auto& field = data->values[curr->index];
auto oldVal = field;
if (field == expected.getSingleValue()) {
field = replacement.getSingleValue();
}
return oldVal;
}

// Arbitrary deterministic limit on size. If we need to allocate a Literals
// vector that takes around 1-2GB of memory then we are likely to hit memory
Expand Down
35 changes: 31 additions & 4 deletions src/wasm/wasm-validator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3096,12 +3096,26 @@ void FunctionValidator::visitStructRMW(StructRMW* curr) {
return;
}
auto& field = fields[curr->index];
shouldBeEqual(
field.mutable_, Mutable, curr, "struct.atomic.rmw field must be mutable");
shouldBeFalse(
field.isPacked(), curr, "struct.atomic.rmw field must not be packed");
bool isAny =
field.type.isRef() &&
Type::isSubType(
field.type,
Type(HeapTypes::any.getBasic(field.type.getHeapType().getShared()),
Nullable));
if (!shouldBeTrue(field.type == Type::i32 || field.type == Type::i64 ||
(isAny && curr->op == RMWXchg),
curr,
"struct.atomic.rmw field type invalid for operation")) {
return;
}
shouldBeSubType(curr->value->type,
field.type,
curr,
"struct.atomic.rmw value must have the proper type");
shouldBeEqual(
field.mutable_, Mutable, curr, "struct.atomic.rmw field must be mutable");
}

void FunctionValidator::visitStructCmpxchg(StructCmpxchg* curr) {
Expand Down Expand Up @@ -3134,6 +3148,21 @@ void FunctionValidator::visitStructCmpxchg(StructCmpxchg* curr) {
return;
}
auto& field = fields[curr->index];
shouldBeEqual(
field.mutable_, Mutable, curr, "struct.atomic.rmw field must be mutable");
shouldBeFalse(
field.isPacked(), curr, "struct.atomic.rmw field must not be packed");
bool isEq =
field.type.isRef() &&
Type::isSubType(
field.type,
Type(HeapTypes::eq.getBasic(field.type.getHeapType().getShared()),
Nullable));
if (!shouldBeTrue(field.type == Type::i32 || field.type == Type::i64 || isEq,
curr,
"struct.atomic.rmw field type invalid for operation")) {
return;
}
shouldBeSubType(
curr->expected->type,
field.type,
Expand All @@ -3144,8 +3173,6 @@ void FunctionValidator::visitStructCmpxchg(StructCmpxchg* curr) {
field.type,
curr,
"struct.atomic.rmw.cmpxchg replacement value must have the proper type");
shouldBeEqual(
field.mutable_, Mutable, curr, "struct.atomic.rmw field must be mutable");
}

void FunctionValidator::visitArrayNew(ArrayNew* curr) {
Expand Down
100 changes: 0 additions & 100 deletions test/spec/shared-struct.wast

This file was deleted.

Loading

0 comments on commit c8bfe28

Please sign in to comment.