diff --git a/ci/generate-spec-tests.rs b/ci/generate-spec-tests.rs index e16fc9301e..33ec790134 100644 --- a/ci/generate-spec-tests.rs +++ b/ci/generate-spec-tests.rs @@ -44,11 +44,21 @@ 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.ends_with("binary.wast") || src.ends_with("global.wast") || src.ends_with("select.wast") + if src.ends_with("binary.wast") + || src.ends_with("global.wast") + || src.ends_with("select.wast") + || src.ends_with("try_table.wast") { contents.push_str(";; --assert permissive \\\n"); } + // For this test it has legacy instructions which, for roundabout reasons, + // are attempted to be printed in the folded format but that doesn't work. + // Exclude this test for now. + if src.ends_with("try_table.wast") { + contents.push_str(";; --assert no-test-folded \\\n"); + } + contents.push_str(";; --snapshot tests/snapshots \\\n"); // This test specifically tests various forms of unicode which are diff --git a/crates/wasmparser/src/readers/core/operators.rs b/crates/wasmparser/src/readers/core/operators.rs index 6488e29927..e4e35d7d08 100644 --- a/crates/wasmparser/src/readers/core/operators.rs +++ b/crates/wasmparser/src/readers/core/operators.rs @@ -336,7 +336,13 @@ crate::for_each_operator!(define_operator); #[derive(Clone)] pub struct OperatorsReader<'a> { reader: BinaryReader<'a>, - blocks: Vec, + /// Block depth this reader is currently operating at. + /// + /// Increments for instructions like `block` and decrements for instructions + /// like `end`. Used to rule out syntactically invalid modules (as defined + /// by the wasm spec currently) in a cheap but not 100% rigorous fashion. + /// More checks are still performed in the validator about block depth. + depth: u32, data_index_occurred: Option, } @@ -345,7 +351,7 @@ impl<'a> OperatorsReader<'a> { pub fn new(reader: BinaryReader<'a>) -> OperatorsReader<'a> { OperatorsReader { reader, - blocks: vec![FrameKind::Block], + depth: 1, data_index_occurred: None, } } @@ -377,7 +383,7 @@ impl<'a> OperatorsReader<'a> { } fn ensure_stack_empty(&self) -> Result<()> { - if !self.blocks.is_empty() { + if self.depth != 0 { bail!( self.original_position(), "control frames remain at end of function body or expression" @@ -410,33 +416,6 @@ impl<'a> OperatorsReader<'a> { Ok((self.read()?, pos)) } - fn enter(&mut self, k: FrameKind) { - self.blocks.push(k) - } - - fn expect_block(&mut self, k: FrameKind, found: &str) -> Result<()> { - match self.blocks.last() { - None => bail!( - self.original_position(), - "empty stack found where {:?} expected", - k - ), - Some(x) if *x == k => Ok(()), - Some(_) => bail!( - self.original_position(), - "`{}` found outside `{:?}` block", - found, - k - ), - } - } - - fn end(&mut self) -> Result<()> { - assert!(!self.blocks.is_empty()); - self.blocks.pop(); - Ok(()) - } - /// Visit the next available operator with the specified [`VisitOperator`] instance. /// /// Note that this does not implicitly propagate any additional information such as instruction @@ -486,7 +465,7 @@ impl<'a> OperatorsReader<'a> { where T: VisitOperator<'a>, { - if self.blocks.is_empty() { + if self.depth == 0 { bail!( self.original_position(), "operators remaining after end of function body or expression" @@ -498,46 +477,28 @@ impl<'a> OperatorsReader<'a> { 0x00 => visitor.visit_unreachable(), 0x01 => visitor.visit_nop(), 0x02 => { - self.enter(FrameKind::Block); + self.depth += 1; visitor.visit_block(self.reader.read_block_type()?) } 0x03 => { - self.enter(FrameKind::Loop); + self.depth += 1; visitor.visit_loop(self.reader.read_block_type()?) } 0x04 => { - self.enter(FrameKind::If); + self.depth += 1; visitor.visit_if(self.reader.read_block_type()?) } - 0x05 => { - self.expect_block(FrameKind::If, "else")?; - visitor.visit_else() - } + 0x05 => visitor.visit_else(), 0x06 => { - if !self.reader.legacy_exceptions() { - bail!( - pos, - "legacy_exceptions feature required for try instruction" - ); - } - self.enter(FrameKind::LegacyTry); + self.depth += 1; visitor.visit_try(self.reader.read_block_type()?) } - 0x07 => { - if !self.reader.legacy_exceptions() { - bail!( - pos, - "legacy_exceptions feature required for catch instruction" - ); - } - self.expect_block(FrameKind::LegacyTry, "catch")?; - visitor.visit_catch(self.reader.read_var_u32()?) - } + 0x07 => visitor.visit_catch(self.reader.read_var_u32()?), 0x08 => visitor.visit_throw(self.reader.read_var_u32()?), 0x09 => visitor.visit_rethrow(self.reader.read_var_u32()?), 0x0a => visitor.visit_throw_ref(), 0x0b => { - self.end()?; + self.depth -= 1; visitor.visit_end() } 0x0c => visitor.visit_br(self.reader.read_var_u32()?), @@ -558,20 +519,10 @@ impl<'a> OperatorsReader<'a> { 0x14 => visitor.visit_call_ref(self.reader.read()?), 0x15 => visitor.visit_return_call_ref(self.reader.read()?), 0x18 => { - self.expect_block(FrameKind::LegacyTry, "delegate")?; - self.blocks.pop(); + self.depth -= 1; visitor.visit_delegate(self.reader.read_var_u32()?) } - 0x19 => { - if !self.reader.legacy_exceptions() { - bail!( - pos, - "legacy_exceptions feature required for catch_all instruction" - ); - } - self.expect_block(FrameKind::LegacyTry, "catch_all")?; - visitor.visit_catch_all() - } + 0x19 => visitor.visit_catch_all(), 0x1a => visitor.visit_drop(), 0x1b => visitor.visit_select(), 0x1c => { @@ -590,7 +541,7 @@ impl<'a> OperatorsReader<'a> { } } 0x1f => { - self.enter(FrameKind::TryTable); + self.depth += 1; visitor.visit_try_table(self.reader.read()?) } diff --git a/src/bin/wasm-tools/wast.rs b/src/bin/wasm-tools/wast.rs index afcc847c98..9a655a0976 100644 --- a/src/bin/wasm-tools/wast.rs +++ b/src/bin/wasm-tools/wast.rs @@ -251,6 +251,12 @@ impl Opts { // two error messages are swapped to `assert_invalid`. "malformed mutability", "integer too large", + // The upstream specification requires that the legacy + // exceptions proposal is textually invalid (e.g. + // `assert_malformed`). This crate supports it though as + // "just another proposal", so it's not malformed but it's + // invalid. + "unexpected token", ]; if self.assert(Assert::Permissive) && permissive_error_messages.contains(&message) { return self.test_wast_directive( diff --git a/tests/cli/invalid.wast b/tests/cli/invalid.wast index 6a627af060..3d2c2442c0 100644 --- a/tests/cli/invalid.wast +++ b/tests/cli/invalid.wast @@ -1,4 +1,4 @@ -;; RUN: wast --assert default --snapshot tests/snapshots % +;; RUN: wast --assert default,no-test-folded --snapshot tests/snapshots % (assert_invalid (module @@ -6,7 +6,7 @@ (func table.init 0 100)) "unknown elem segment") -(assert_malformed +(assert_invalid (module (func else)) - "`else` found outside `If` block") + "else found outside of an `if` block ") diff --git a/tests/cli/legacy-exceptions/disabled.wast b/tests/cli/legacy-exceptions/disabled.wast new file mode 100644 index 0000000000..07385e10bc --- /dev/null +++ b/tests/cli/legacy-exceptions/disabled.wast @@ -0,0 +1,13 @@ +;; RUN: wast % + +(assert_invalid + (module (func try end)) + "legacy exceptions support is not enabled") + +(assert_invalid + (module (func catch 0)) + "legacy exceptions support is not enabled") + +(assert_invalid + (module (func catch_all)) + "legacy exceptions support is not enabled") diff --git a/tests/cli/print-no-panic-dangling-else.wat b/tests/cli/print-no-panic-dangling-else.wat index c38c1b38cc..7c6307f1af 100644 --- a/tests/cli/print-no-panic-dangling-else.wat +++ b/tests/cli/print-no-panic-dangling-else.wat @@ -1,4 +1,4 @@ -;; FAIL: print % +;; RUN: print % (module (func else) diff --git a/tests/cli/print-no-panic-dangling-else.wat.stderr b/tests/cli/print-no-panic-dangling-else.wat.stderr deleted file mode 100644 index 483dfe7f28..0000000000 --- a/tests/cli/print-no-panic-dangling-else.wat.stderr +++ /dev/null @@ -1 +0,0 @@ -error: `else` found outside `If` block (at offset 0x18) diff --git a/tests/cli/print-no-panic-dangling-else.wat.stdout b/tests/cli/print-no-panic-dangling-else.wat.stdout index 5af803de5c..e43ca79fee 100644 --- a/tests/cli/print-no-panic-dangling-else.wat.stdout +++ b/tests/cli/print-no-panic-dangling-else.wat.stdout @@ -1,3 +1,6 @@ (module (type (;0;) (func)) (func (;0;) (type 0) + else + ) +) diff --git a/tests/cli/spec/proposals/exception-handling/try_table.wast b/tests/cli/spec/proposals/exception-handling/try_table.wast index a050a68423..1a606c40db 100644 --- a/tests/cli/spec/proposals/exception-handling/try_table.wast +++ b/tests/cli/spec/proposals/exception-handling/try_table.wast @@ -1,5 +1,7 @@ ;; RUN: wast \ ;; --assert default \ +;; --assert permissive \ +;; --assert no-test-folded \ ;; --snapshot tests/snapshots \ ;; --ignore-error-messages \ ;; --features=wasm2,exceptions,tail-call \ diff --git a/tests/cli/spec/proposals/wasm-3.0/try_table.wast b/tests/cli/spec/proposals/wasm-3.0/try_table.wast index b7d00beb42..89b93cf256 100644 --- a/tests/cli/spec/proposals/wasm-3.0/try_table.wast +++ b/tests/cli/spec/proposals/wasm-3.0/try_table.wast @@ -1,5 +1,7 @@ ;; RUN: wast \ ;; --assert default \ +;; --assert permissive \ +;; --assert no-test-folded \ ;; --snapshot tests/snapshots \ ;; --ignore-error-messages \ ;; --features=wasm3 \ diff --git a/tests/snapshots/cli/invalid.wast.json b/tests/snapshots/cli/invalid.wast.json index 74fa700c59..c212f2a1bb 100644 --- a/tests/snapshots/cli/invalid.wast.json +++ b/tests/snapshots/cli/invalid.wast.json @@ -9,11 +9,11 @@ "text": "unknown elem segment" }, { - "type": "assert_malformed", + "type": "assert_invalid", "line": 10, "filename": "invalid.1.wasm", "module_type": "binary", - "text": "`else` found outside `If` block" + "text": "else found outside of an `if` block " } ] } \ No newline at end of file