Fix validation errors for struct.wait (#8394)
Prior to #8378, any `assert_invalid` assertions that required a feature
to be enabled spuriously succeeded even if the test would otherwise
wrongly pass validation. The checks that were in wasm-validator.cpp
didn't run if the ref was null or unreachable, which is wrong because
the type + field index immediates could still be wrong.
Move these assertions to IRBuilder where we have the type immediate
available.
Verified by removing the assert_invalid parts and checking that the
error message matches what's in the test.
Part of #8315.
diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp
index 7cc87a0..1e7463e 100644
--- a/src/wasm/wasm-ir-builder.cpp
+++ b/src/wasm/wasm-ir-builder.cpp
@@ -2250,6 +2250,16 @@
if (!type.isStruct()) {
return Err{"expected struct type annotation on struct.wait"};
}
+ // This is likely checked in the caller by the `fieldidx` parser.
+ if (index >= type.getStruct().fields.size()) {
+ return Err{"struct.wait field index out of bounds"};
+ }
+
+ if (type.getStruct().fields.at(index).packedType !=
+ Field::PackedType::WaitQueue) {
+ return Err{"struct.wait field index must contain a `waitqueue`"};
+ }
+
StructWait curr(wasm.allocator);
curr.index = index;
CHECK_ERR(ChildPopper{*this}.visitStructWait(&curr, type));
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
index f79c96a..ae862a3 100644
--- a/src/wasm/wasm-validator.cpp
+++ b/src/wasm/wasm-validator.cpp
@@ -3517,25 +3517,12 @@
curr,
"struct.wait timeout must be an i64");
- if (curr->ref->type == Type::unreachable || curr->ref->type.isNull()) {
- return;
- }
-
- // In practice this likely fails during parsing instead.
- if (!shouldBeTrue(curr->index <
- curr->ref->type.getHeapType().getStruct().fields.size(),
- curr,
- "struct.wait index immediate should be less than the field "
- "count of the struct")) {
- return;
- }
-
- shouldBeTrue(curr->ref->type.getHeapType()
- .getStruct()
- .fields.at(curr->index)
- .packedType == Field::WaitQueue,
- curr,
- "struct.wait struct field must be a waitqueue");
+ // Checks to the ref argument's type are done in IRBuilder where we have the
+ // type annotation immediate available. We check that
+ // * The reference arg is a subtype of the type immediate
+ // * The index immediate is a valid field index of the type immediate (and
+ // thus valid for the reference's type too)
+ // * The index points to a packed waitqueue field
}
void FunctionValidator::visitArrayNew(ArrayNew* curr) {
diff --git a/test/spec/waitqueue.wast b/test/spec/waitqueue.wast
index 7383d4e..8e83708 100644
--- a/test/spec/waitqueue.wast
+++ b/test/spec/waitqueue.wast
@@ -4,7 +4,7 @@
(func (param $expected i32) (param $timeout i64) (result i32)
(struct.wait $t 0 (ref.null $t) (local.get $expected) (local.get $timeout))
)
- ) "struct.wait struct field must be a waitqueue"
+ ) "struct.wait struct field index must contain a `waitqueue`"
)
(assert_invalid
@@ -38,7 +38,7 @@
;; unreachable is allowed
(module
- (type $t (struct (field i32)))
+ (type $t (struct (field waitqueue)))
(func (param $expected i32) (param $timeout i64) (result i32)
(struct.wait $t 0 (unreachable) (local.get $expected) (local.get $timeout))
)