Repository: bytecodealliance/wasm-tools
Pull request #2146 (2025-04-15 21:22:32)

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.


Patch:
diff --git a/src/bin/wasm-tools/addr2line.rs b/src/bin/wasm-tools/addr2line.rs
index 989929da..9efacce4 100644
--- a/src/bin/wasm-tools/addr2line.rs
+++ b/src/bin/wasm-tools/addr2line.rs
@@ -44,7 +44,7 @@ impl Opts {
     }
 
     pub fn run(&self) -> Result<()> {
-        let wasm = self.io.parse_input_wasm()?;
+        let wasm = self.io.get_input_wasm()?;
 
         let mut modules = Addr2lineModules::parse(&wasm)
             .context("failed to parse input and read custom sections")?;
diff --git a/src/bin/wasm-tools/component.rs b/src/bin/wasm-tools/component.rs
index 15e8edae..2d2e4375 100644
--- a/src/bin/wasm-tools/component.rs
+++ b/src/bin/wasm-tools/component.rs
@@ -169,7 +169,7 @@ impl NewOpts {
 
     /// Executes the application.
     fn run(self) -> Result<()> {
-        let wasm = self.io.parse_input_wasm()?;
+        let wasm = self.io.get_input_wasm()?;
         let mut encoder = ComponentEncoder::default()
             .validate(!self.skip_validation)
             .reject_legacy_names(self.reject_legacy_names);
@@ -389,7 +389,7 @@ impl EmbedOpts {
                 },
             )
         } else {
-            self.io.parse_input_wasm()?
+            self.io.get_input_wasm()?
         };
 
         embed_component_metadata(
@@ -1046,7 +1046,7 @@ impl UnbundleOpts {
     }
 
     fn run(self) -> Result<()> {
-        let input = self.io.parse_input_wasm()?;
+        let input = self.io.get_input_wasm()?;
         if !wasmparser::Parser::is_component(&input) {
             return self.io.output_wasm(&input, self.wat);
         }
diff --git a/src/bin/wasm-tools/demangle.rs b/src/bin/wasm-tools/demangle.rs
index 1513a5db..65a649fa 100644
--- a/src/bin/wasm-tools/demangle.rs
+++ b/src/bin/wasm-tools/demangle.rs
@@ -24,7 +24,7 @@ impl Opts {
     }
 
     pub fn run(&self) -> Result<()> {
-        let input = self.io.parse_input_wasm()?;
+        let input = self.io.get_input_wasm()?;
         let mut module = wasm_encoder::Module::new();
 
         for payload in Parser::new(0).parse_all(&input) {
diff --git a/src/bin/wasm-tools/dump.rs b/src/bin/wasm-tools/dump.rs
index 525e08aa..cec6f9ed 100644
--- a/src/bin/wasm-tools/dump.rs
+++ b/src/bin/wasm-tools/dump.rs
@@ -20,7 +20,7 @@ impl Opts {
     }
 
     pub fn run(&self) -> Result<()> {
-        let input = self.io.parse_input_wasm()?;
+        let input = self.io.get_input_wasm()?;
         let output = self.io.output_writer()?;
         let mut d = Dump::new(&input, output);
         d.run()?;
diff --git a/src/bin/wasm-tools/metadata.rs b/src/bin/wasm-tools/metadata.rs
index a1197b34..852f43cc 100644
--- a/src/bin/wasm-tools/metadata.rs
+++ b/src/bin/wasm-tools/metadata.rs
@@ -48,7 +48,7 @@ impl ShowOpts {
     }
 
     pub fn run(&self) -> Result<()> {
-        let input = self.io.parse_input_wasm()?;
+        let input = self.io.get_input_wasm()?;
         let mut output = self.io.output_writer()?;
 
         let payload = wasm_metadata::Payload::from_binary(&input)?;
@@ -82,7 +82,7 @@ impl AddOpts {
     }
 
     pub fn run(&self) -> Result<()> {
-        let input = self.io.parse_input_wasm()?;
+        let input = self.io.get_input_wasm()?;
 
         let output = self.add_metadata.to_wasm(&input)?;
 
diff --git a/src/bin/wasm-tools/mutate.rs b/src/bin/wasm-tools/mutate.rs
index 9eae6da8..5072ec4d 100644
--- a/src/bin/wasm-tools/mutate.rs
+++ b/src/bin/wasm-tools/mutate.rs
@@ -54,7 +54,7 @@ impl Opts {
     }
 
     pub fn run(mut self) -> Result<()> {
-        let input_wasm = self.io.parse_input_wasm()?;
+        let input_wasm = self.io.get_input_wasm()?;
 
         // Currently `self.wasm_mutate` is typed as `'static` for the input wasm
         // due to how this subcommand is defined. To get the input wasm to live
diff --git a/src/bin/wasm-tools/objdump.rs b/src/bin/wasm-tools/objdump.rs
index db4c8f1a..33442f9f 100644
--- a/src/bin/wasm-tools/objdump.rs
+++ b/src/bin/wasm-tools/objdump.rs
@@ -20,7 +20,7 @@ impl Opts {
     }
 
     pub fn run(&self) -> Result<()> {
-        let input = self.io.parse_input_wasm()?;
+        let input = self.io.get_input_wasm()?;
 
         let mut printer = Printer {
             indices: Vec::new(),
diff --git a/src/bin/wasm-tools/print.rs b/src/bin/wasm-tools/print.rs
index a88e2de9..cdb2dd81 100644
--- a/src/bin/wasm-tools/print.rs
+++ b/src/bin/wasm-tools/print.rs
@@ -45,7 +45,7 @@ impl Opts {
     }
 
     pub fn run(&self) -> Result<()> {
-        let wasm = self.io.parse_input_wasm()?;
+        let wasm = self.io.get_input_wasm()?;
 
         let mut config = wasmprinter::Config::new();
         config.print_offsets(self.print_offsets);
diff --git a/src/bin/wasm-tools/shrink.rs b/src/bin/wasm-tools/shrink.rs
index c70f550d..844b4c5c 100644
--- a/src/bin/wasm-tools/shrink.rs
+++ b/src/bin/wasm-tools/shrink.rs
@@ -41,7 +41,7 @@ impl Opts {
     }
 
     pub fn run(self) -> Result<()> {
-        let input = self.io.parse_input_wasm()?;
+        let input = self.io.get_input_wasm()?;
         let initial_size = input.len();
 
         // Prerequisites for the predicate.
diff --git a/src/bin/wasm-tools/strip.rs b/src/bin/wasm-tools/strip.rs
index 53884612..44aa5708 100644
--- a/src/bin/wasm-tools/strip.rs
+++ b/src/bin/wasm-tools/strip.rs
@@ -32,7 +32,7 @@ impl Opts {
     }
 
     pub fn run(&self) -> Result<()> {
-        let input = self.io.parse_input_wasm()?;
+        let input = self.io.get_input_wasm()?;
         let to_delete = regex::RegexSet::new(self.delete.iter())?;
 
         let strip_custom_section = |name: &str| {
diff --git a/src/bin/wasm-tools/wast.rs b/src/bin/wasm-tools/wast.rs
index 696a5b2f..fc9f1ffd 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 the 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<Vec<u8>> = 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`.


Test patch:
diff --git a/ci/generate-spec-tests.rs b/ci/generate-spec-tests.rs
index 34961027..e16fc930 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/tests/cli/dangling_if.wat.stdout b/tests/cli/dangling_if.wat.stdout
new file mode 100644
index 00000000..45f65ca9
--- /dev/null
+++ b/tests/cli/dangling_if.wat.stdout
@@ -0,0 +1,4 @@
+(module
+  (type (;0;) (func))
+  (func (;0;) (type 0)
+    if ;; label = @1
diff --git a/tests/cli/dump/select.wat b/tests/cli/dump/select.wat
index 0ee91710..c72d8625 100644
--- a/tests/cli/dump/select.wat
+++ b/tests/cli/dump/select.wat
@@ -1,6 +1,4 @@
-;; FAIL: dump %
-;; This fails because wasmprinter can't (yet) print an invalid multi-value select.
-;; Can be changed back to a "RUN" test once that code lands.
+;; RUN: dump %
 
 (module
     (func
diff --git a/tests/cli/dump/select.wat.stderr b/tests/cli/dump/select.wat.stderr
deleted file mode 100644
index f86ca350..00000000
--- a/tests/cli/dump/select.wat.stderr
+++ /dev/null
@@ -1,1 +0,0 @@
-error: invalid result arity (at offset 0x18)
diff --git a/tests/cli/dump/select.wat.stdout b/tests/cli/dump/select.wat.stdout
new file mode 100644
index 00000000..be65160b
--- /dev/null
+++ b/tests/cli/dump/select.wat.stdout
@@ -0,0 +1,21 @@
+  0x0 | 00 61 73 6d | version 1 (Module)
+      | 01 00 00 00
+  0x8 | 01 04       | type section
+  0xa | 01          | 1 count
+--- rec group 0 (implicit) ---
+  0xb | 60 00 00    | [type 0] SubType { is_final: true, supertype_idx: None, composite_type: CompositeType { inner: Func(FuncType { params: [], results: [] }), shared: false } }
+  0xe | 03 02       | func section
+ 0x10 | 01          | 1 count
+ 0x11 | 00          | [func 0] type 0
+ 0x12 | 0a 0e       | code section
+ 0x14 | 01          | 1 count
+============== func 0 ====================
+ 0x15 | 0c          | size of function
+ 0x16 | 00          | 0 local blocks
+ 0x17 | 1b          | select
+ 0x18 | 1c 00       | ??
+ 0x1a | 1c 01 7f    | typed_select ty:I32
+ 0x1d | 1c 02       | ??
+ 0x1f | 7f          | i64_div_s
+ 0x20 | 7f          | i64_div_s
+ 0x21 | 0b          | end
diff --git a/tests/cli/print-code-section-overflow.wat.stdout b/tests/cli/print-code-section-overflow.wat.stdout
new file mode 100644
index 00000000..b55cecc0
--- /dev/null
+++ b/tests/cli/print-code-section-overflow.wat.stdout
@@ -0,0 +1,1 @@
+(module
diff --git a/tests/cli/print-dont-reserve-the-world.wat.stderr b/tests/cli/print-dont-reserve-the-world.wat.stderr
index 19f2dec5..bab93a74 100644
--- a/tests/cli/print-dont-reserve-the-world.wat.stderr
+++ b/tests/cli/print-dont-reserve-the-world.wat.stderr
@@ -1,1 +1,1 @@
-error: unexpected end-of-file (at offset 0xf)
+error: function section has non-zero count but code section is absent (at offset 0xf)
diff --git a/tests/cli/print-dont-reserve-the-world.wat.stdout b/tests/cli/print-dont-reserve-the-world.wat.stdout
new file mode 100644
index 00000000..b55cecc0
--- /dev/null
+++ b/tests/cli/print-dont-reserve-the-world.wat.stdout
@@ -0,0 +1,1 @@
+(module
diff --git a/tests/cli/print-locals-overflow.wat.stderr b/tests/cli/print-locals-overflow.wat.stderr
index 990d4303..42167d7e 100644
--- a/tests/cli/print-locals-overflow.wat.stderr
+++ b/tests/cli/print-locals-overflow.wat.stderr
@@ -1,1 +1,1 @@
-error: control frames remain at end of function body or expression (at offset 0x1d)
+error: function exceeds the maximum number of locals that can be printed
diff --git a/tests/cli/print-locals-overflow.wat.stdout b/tests/cli/print-locals-overflow.wat.stdout
new file mode 100644
index 00000000..5af803de
--- /dev/null
+++ b/tests/cli/print-locals-overflow.wat.stdout
@@ -0,0 +1,3 @@
+(module
+  (type (;0;) (func))
+  (func (;0;) (type 0)
diff --git a/tests/cli/print-no-panic-dangling-else.wat.stdout b/tests/cli/print-no-panic-dangling-else.wat.stdout
new file mode 100644
index 00000000..5af803de
--- /dev/null
+++ b/tests/cli/print-no-panic-dangling-else.wat.stdout
@@ -0,0 +1,3 @@
+(module
+  (type (;0;) (func))
+  (func (;0;) (type 0)
diff --git a/tests/cli/print-no-panic-double-end.wat.stdout b/tests/cli/print-no-panic-double-end.wat.stdout
new file mode 100644
index 00000000..cac212e7
--- /dev/null
+++ b/tests/cli/print-no-panic-double-end.wat.stdout
@@ -0,0 +1,4 @@
+(module
+  (type (;0;) (func))
+  (func (;0;) (type 0)
+    end
diff --git a/tests/cli/print-with-too-many-ends.wat.stdout b/tests/cli/print-with-too-many-ends.wat.stdout
new file mode 100644
index 00000000..cac212e7
--- /dev/null
+++ b/tests/cli/print-with-too-many-ends.wat.stdout
@@ -0,0 +1,4 @@
+(module
+  (type (;0;) (func))
+  (func (;0;) (type 0)
+    end
diff --git a/tests/cli/spec/proposals/function-references/select.wast b/tests/cli/spec/proposals/function-references/select.wast
index 0353ff1e..45b7e8c5 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 56807e81..9fa35eac 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 ccb3a587..cfc6ca9a 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 590fab84..ca68b6a7 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 \