From 5578df7794d086111241de7f29c2a739bcc3ad3f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 11 Apr 2025 09:06:41 -0700 Subject: [PATCH 1/2] For `assert_invalid`, assert modules all parse Sort of the inverse of `assert_malformed`, but assert that invalid modules all parse successfully and just fail validation. --- ci/generate-spec-tests.rs | 3 +- src/bin/wasm-tools/wast.rs | 131 ++++++++++-------- .../proposals/function-references/select.wast | 1 + tests/cli/spec/proposals/gc/select.wast | 1 + tests/cli/spec/proposals/wasm-3.0/select.wast | 1 + tests/cli/spec/select.wast | 1 + 6 files changed, 80 insertions(+), 58 deletions(-) diff --git a/ci/generate-spec-tests.rs b/ci/generate-spec-tests.rs index 3496102745..e16fc9301e 100644 --- a/ci/generate-spec-tests.rs +++ b/ci/generate-spec-tests.rs @@ -44,7 +44,8 @@ fn copy_test(src: &Path, dst: &Path) { contents.push_str(";; --assert default \\\n"); // Allow certain assert_malformed tests to be interpreted as assert_invalid - if src.iter().any(|p| p == "binary.wast") || src.iter().any(|p| p == "global.wast") { + if src.ends_with("binary.wast") || src.ends_with("global.wast") || src.ends_with("select.wast") + { contents.push_str(";; --assert permissive \\\n"); } diff --git a/src/bin/wasm-tools/wast.rs b/src/bin/wasm-tools/wast.rs index 696a5b2f70..f1e6779674 100644 --- a/src/bin/wasm-tools/wast.rs +++ b/src/bin/wasm-tools/wast.rs @@ -198,6 +198,30 @@ impl Opts { mut module, message, } => { + // For `assert_malformed` it means that either converting this + // test case to binary should fail, or that parsing the binary + // should fail. Currently there's no distinction as to which + // step should fail. + let bytes = match module.encode() { + Ok(bytes) => bytes, + Err(e) => { + self.assert_error_matches(test, &e.to_string(), message)?; + return Ok(()); + } + }; + let mut parser = wasmparser::Parser::new(0); + parser.set_features(self.features.features()); + if let Err(e) = parse_binary_wasm(parser, &bytes) { + self.assert_error_matches(test, &format!("{e:?}"), message)?; + + // make sure validator also rejects module (not necessarily + // with same error) + if self.test_wasm_valid(test, &bytes).is_ok() { + bail!("validator thought malformed example was valid") + } + return Ok(()); + } + // The WebAssembly specification itself has a strict // distinction between `assert_invalid` and `assert_malformed` // where a "malformed" module is one that simply does not parse @@ -255,73 +279,59 @@ impl Opts { ); } - let result: Result> = module.encode().map_err(|e| e.into()); - match result { - Err(e) => { - if self.error_matches(test, &format!("{e:?}"), message) { - return Ok(()); - } - bail!( - "bad error: {e:?}\n\ - should have failed with: {message:?}\n\ - suppress this failure with `--ignore-error-messages`", - ); - } - Ok(bytes) => { - let mut parser = wasmparser::Parser::new(0); - parser.set_features(self.features.features()); - match parse_binary_wasm(parser, &bytes) { - Err(e) => { - if self.error_matches(test, &format!("{e:?}"), message) { - // make sure validator also rejects module (not necessarily with same error) - - match self.test_wasm_valid(test, &bytes) { - Ok(_) => { - bail!("validator thought malformed example was valid") - } - Err(_) => return Ok(()), - } - } - bail!( - "bad error: {e:?}\n\ - should have failed with: {message:?}\n\ - suppress this failure with `--ignore-error-messages`", - ); - } - Ok(_) => (), - } - bail!( - "encoded and parsed successfully but should have failed with: {message:?}", - ) - } - } + bail!("encoded and parsed successfully but should have failed with: {message:?}",) } WastDirective::AssertInvalid { mut module, message, - span: _, + span, } => { - let result = module - .encode() - .map_err(|e| e.into()) - .and_then(|wasm| self.test_wasm_valid(test, &wasm)); - match result { + // Similar to `assert_malformed`, there are some tests in + // upstream wasm which are specifically flagged as + // `assert_invalid` because the AST can be created for a test + // case but the binary itself shouldn't validate. Like with + // `assert_malformed`, though, wasmparser doesn't match the + // upstream specification 1:1 in all cases. This is a list of + // exceptions. + let permissive_error_messages = [ + // The typed-select instruction spec-wise takes a list of + // types, but in our AST it takes exactly one type to avoid + // heap allocation. That means that we catch + // non-1-length-lists at parse time, not validation time. + "invalid result arity", + ]; + if self.assert(Assert::Permissive) && permissive_error_messages.contains(&message) { + return self.test_wast_directive( + test, + WastDirective::AssertMalformed { + span, + module, + message, + }, + idx, + ); + } + + let wasm = module.encode()?; + let mut parser = wasmparser::Parser::new(0); + parser.set_features(self.features.features()); + + match self.test_wasm_valid(test, &wasm) { Ok(_) => bail!( "encoded and validated successfully but should have failed with: {}", message, ), Err(e) => { - if self.error_matches(test, &format!("{e:?}"), message) { - return Ok(()); - } - bail!( - "bad error: {e:?}\n\ - should have failed with: {message:?}\n\ - suppress this failure with `--ignore-error-messages`", - ); + self.assert_error_matches(test, &format!("{e:?}"), message)?; } } + + // For `assert_invalid` all modules should also parse + // successfully, so double-check that here. + if let Err(e) = parse_binary_wasm(parser, &wasm) { + bail!("failed to parse module when it should parse successfully: {e}"); + } } WastDirective::Thread(thread) => { @@ -364,10 +374,17 @@ impl Opts { enabled } - /// Returns whether `error`, from the validator, matches the expected error + /// Tests that `error`, from the validator or parser, matches the expected error /// `message`. - fn error_matches(&self, _test: &Path, error: &str, message: &str) -> bool { - error.contains(message) || self.ignore_error_messages + fn assert_error_matches(&self, _test: &Path, error: &str, message: &str) -> Result<()> { + if error.contains(message) || self.ignore_error_messages { + return Ok(()); + } + bail!( + "bad error: {error}\n\ + should have failed with: {message:?}\n\ + suppress this failure with `--ignore-error-messages`", + ); } /// Performs tests about the wasm binary `contents` associated with `test`. diff --git a/tests/cli/spec/proposals/function-references/select.wast b/tests/cli/spec/proposals/function-references/select.wast index 0353ff1ec3..45b7e8c5fb 100644 --- a/tests/cli/spec/proposals/function-references/select.wast +++ b/tests/cli/spec/proposals/function-references/select.wast @@ -1,5 +1,6 @@ ;; RUN: wast \ ;; --assert default \ +;; --assert permissive \ ;; --snapshot tests/snapshots \ ;; --ignore-error-messages \ ;; --features=wasm2,function-references,tail-call \ diff --git a/tests/cli/spec/proposals/gc/select.wast b/tests/cli/spec/proposals/gc/select.wast index 56807e81bf..9fa35eacda 100644 --- a/tests/cli/spec/proposals/gc/select.wast +++ b/tests/cli/spec/proposals/gc/select.wast @@ -1,5 +1,6 @@ ;; RUN: wast \ ;; --assert default \ +;; --assert permissive \ ;; --snapshot tests/snapshots \ ;; --ignore-error-messages \ ;; --features=wasm2,function-references,gc,tail-call \ diff --git a/tests/cli/spec/proposals/wasm-3.0/select.wast b/tests/cli/spec/proposals/wasm-3.0/select.wast index ccb3a5879f..cfc6ca9a66 100644 --- a/tests/cli/spec/proposals/wasm-3.0/select.wast +++ b/tests/cli/spec/proposals/wasm-3.0/select.wast @@ -1,5 +1,6 @@ ;; RUN: wast \ ;; --assert default \ +;; --assert permissive \ ;; --snapshot tests/snapshots \ ;; --ignore-error-messages \ ;; --features=wasm3 \ diff --git a/tests/cli/spec/select.wast b/tests/cli/spec/select.wast index 590fab842b..ca68b6a790 100644 --- a/tests/cli/spec/select.wast +++ b/tests/cli/spec/select.wast @@ -1,5 +1,6 @@ ;; RUN: wast \ ;; --assert default \ +;; --assert permissive \ ;; --snapshot tests/snapshots \ ;; --ignore-error-messages \ ;; --features=wasm2 \ From bd022d030bc914ad408b0c9eb29f46c4757deaf6 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 16 Apr 2025 09:58:50 -0500 Subject: [PATCH 2/2] Update src/bin/wasm-tools/wast.rs Co-authored-by: Nick Fitzgerald --- src/bin/wasm-tools/wast.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bin/wasm-tools/wast.rs b/src/bin/wasm-tools/wast.rs index f1e6779674..fc9f1ffdbc 100644 --- a/src/bin/wasm-tools/wast.rs +++ b/src/bin/wasm-tools/wast.rs @@ -214,8 +214,8 @@ impl Opts { if let Err(e) = parse_binary_wasm(parser, &bytes) { self.assert_error_matches(test, &format!("{e:?}"), message)?; - // make sure validator also rejects module (not necessarily - // with same error) + // Make sure validator also rejects the module (not necessarily + // with same error). if self.test_wasm_valid(test, &bytes).is_ok() { bail!("validator thought malformed example was valid") }