diff --git a/crates/wasmparser/src/binary_reader.rs b/crates/wasmparser/src/binary_reader.rs index 1c9b2b0b31..96dc786756 100644 --- a/crates/wasmparser/src/binary_reader.rs +++ b/crates/wasmparser/src/binary_reader.rs @@ -1101,15 +1101,16 @@ impl<'a> BinaryReader<'a> { 0x06 => Operator::Try { ty: self.read_blocktype()?, }, - 0x07 => Operator::Catch, + 0x07 => Operator::Catch { + index: self.read_var_u32()?, + }, 0x08 => Operator::Throw { index: self.read_var_u32()?, }, - 0x09 => Operator::Rethrow, - 0x0a => Operator::BrOnExn { + 0x09 => Operator::Rethrow { relative_depth: self.read_var_u32()?, - index: self.read_var_u32()?, }, + 0x0a => Operator::Unwind, 0x0b => Operator::End, 0x0c => Operator::Br { relative_depth: self.read_var_u32()?, diff --git a/crates/wasmparser/src/operators_validator.rs b/crates/wasmparser/src/operators_validator.rs index e49c10fca7..e764db784c 100644 --- a/crates/wasmparser/src/operators_validator.rs +++ b/crates/wasmparser/src/operators_validator.rs @@ -114,6 +114,8 @@ enum FrameKind { Loop, Try, Catch, + CatchAll, + Unwind, } impl OperatorValidator { @@ -578,10 +580,24 @@ impl OperatorValidator { } Operator::Else => { let frame = self.pop_ctrl(resources)?; - if frame.kind != FrameKind::If { - bail_op_err!("else found outside of an `if` block"); + // The `catch_all` instruction shares an opcode with `else`, + // so we check the frame to see how it's interpreted. + match frame.kind { + FrameKind::If => { + self.push_ctrl(FrameKind::Else, frame.block_type, resources)? + } + FrameKind::Try | FrameKind::Catch => { + // We assume `self.features.exceptions` is true when + // these frame kinds are present. + self.control.push(Frame { + kind: FrameKind::CatchAll, + block_type: frame.block_type, + height: self.operands.len(), + unreachable: false, + }); + } + _ => bail_op_err!("else found outside of an `if` block"), } - self.push_ctrl(FrameKind::Else, frame.block_type, resources)?; } Operator::Try { ty } => { self.check_exceptions_enabled()?; @@ -591,10 +607,10 @@ impl OperatorValidator { } self.push_ctrl(FrameKind::Try, ty, resources)?; } - Operator::Catch => { + Operator::Catch { index } => { self.check_exceptions_enabled()?; let frame = self.pop_ctrl(resources)?; - if frame.kind != FrameKind::Try { + if frame.kind != FrameKind::Try && frame.kind != FrameKind::Catch { bail_op_err!("catch found outside of an `try` block"); } // Start a new frame and push `exnref` value. @@ -604,7 +620,12 @@ impl OperatorValidator { height: self.operands.len(), unreachable: false, }); - self.push_operand(Type::ExnRef)?; + // Push exception argument types. + let event_ty = event_at(&resources, index)?; + let ty = func_type_at(&resources, event_ty.type_index)?; + for ty in ty.inputs() { + self.push_operand(ty)?; + } } Operator::Throw { index } => { self.check_exceptions_enabled()?; @@ -619,25 +640,30 @@ impl OperatorValidator { } self.unreachable(); } - Operator::Rethrow => { + Operator::Rethrow { relative_depth } => { self.check_exceptions_enabled()?; - self.pop_operand(Some(Type::ExnRef))?; + // This is not a jump, but we need to check that the `rethrow` + // targets an actual `catch` to get the exception. + let (_, kind) = self.jump(relative_depth)?; + if kind != FrameKind::Catch && kind != FrameKind::CatchAll { + bail_op_err!("rethrow target was not a `catch` block"); + } self.unreachable(); } - Operator::BrOnExn { - relative_depth, - index, - } => { + Operator::Unwind => { self.check_exceptions_enabled()?; - let (ty, kind) = self.jump(relative_depth)?; - self.pop_operand(Some(Type::ExnRef))?; - // Check the exception's argument values with target block's. - let event_ty = event_at(&resources, index)?; - let exn_args = func_type_at(&resources, event_ty.type_index)?; - if Iterator::ne(exn_args.inputs(), label_types(ty, resources, kind)?) { - bail_op_err!("target block types do not match"); + // Switch from `try` to an `unwind` frame, so we can check that + // the result type is empty. + let frame = self.pop_ctrl(resources)?; + if frame.kind != FrameKind::Try { + bail_op_err!("unwind found outside of an `try` block"); } - self.push_operand(Type::ExnRef)?; + self.control.push(Frame { + kind: FrameKind::Unwind, + block_type: TypeOrFuncType::Type(Type::EmptyBlockType), + height: self.operands.len(), + unreachable: false, + }); } Operator::End => { let mut frame = self.pop_ctrl(resources)?; diff --git a/crates/wasmparser/src/primitives.rs b/crates/wasmparser/src/primitives.rs index a8fedb7961..3f92a09ed4 100644 --- a/crates/wasmparser/src/primitives.rs +++ b/crates/wasmparser/src/primitives.rs @@ -347,10 +347,10 @@ pub enum Operator<'a> { If { ty: TypeOrFuncType }, Else, Try { ty: TypeOrFuncType }, - Catch, + Catch { index: u32 }, Throw { index: u32 }, - Rethrow, - BrOnExn { relative_depth: u32, index: u32 }, + Rethrow { relative_depth: u32 }, + Unwind, End, Br { relative_depth: u32 }, BrIf { relative_depth: u32 }, diff --git a/crates/wasmprinter/src/lib.rs b/crates/wasmprinter/src/lib.rs index 066b5fd5ca..9ed318548e 100644 --- a/crates/wasmprinter/src/lib.rs +++ b/crates/wasmprinter/src/lib.rs @@ -665,7 +665,7 @@ impl Printer { // `else`/`catch` are special in that it's printed at // the previous indentation, but it doesn't actually change // our nesting level. - Operator::Else | Operator::Catch => { + Operator::Else | Operator::Catch { .. } | Operator::Unwind => { self.nesting -= 1; self.newline(); self.nesting += 1; @@ -729,24 +729,21 @@ impl Printer { self.print_blockty(ty)?; write!(self.result, " ;; label = @{}", cur_label)?; } - Catch => self.result.push_str("catch"), + Catch { index } => { + write!(self.result, "catch {}", index)?; + } Throw { index } => { write!(self.result, "throw {}", index)?; } - Rethrow => self.result.push_str("rethrow"), - BrOnExn { - relative_depth, - index, - } => { + Rethrow { relative_depth } => { write!( self.result, - "br_on_exn {} (;{};)", + "rethrow {} (;{};)", relative_depth, - label(*relative_depth), + label(*relative_depth) )?; - write!(self.result, " {}", index)?; } - + Unwind => self.result.push_str("unwind"), End => self.result.push_str("end"), Br { relative_depth } => { write!( diff --git a/crates/wast/src/ast/expr.rs b/crates/wast/src/ast/expr.rs index b771c96097..1dd3bd0ebd 100644 --- a/crates/wast/src/ast/expr.rs +++ b/crates/wast/src/ast/expr.rs @@ -84,7 +84,9 @@ enum If<'a> { enum Try<'a> { /// Next thing to parse is the `do` block. Do(Instruction<'a>), - /// Next thing to parse is the `catch` block. + /// Next thing to parse is `catch`/`catch_all`, or `unwind`. + CatchOrUnwind, + /// Next thing to parse is a `catch` block or `catch_all`. Catch, /// This `try` statement has finished parsing and if anything remains it's a /// syntax error. @@ -195,8 +197,8 @@ impl<'a> ExpressionParser<'a> { Level::Try(Try::Do(_)) => { return Err(parser.error("previous `try` had no `do`")); } - Level::Try(Try::Catch) => { - return Err(parser.error("previous `try` had no `catch`")); + Level::Try(Try::CatchOrUnwind) => { + return Err(parser.error("previous `try` had no `catch`, `catch_all`, or `unwind`")); } Level::Try(_) => { self.instrs.push(Instruction::End(None)); @@ -305,7 +307,7 @@ impl<'a> ExpressionParser<'a> { /// than an `if` as the syntactic form is: /// /// ```wat - /// (try (do $do) (catch $catch)) + /// (try (do $do) (catch $event $catch)) /// ``` /// /// where the `do` and `catch` keywords are mandatory, even for an empty @@ -328,7 +330,7 @@ impl<'a> ExpressionParser<'a> { if parser.parse::>()?.is_some() { // The state is advanced here only if the parse succeeds in // order to strictly require the keyword. - *i = Try::Catch; + *i = Try::CatchOrUnwind; self.stack.push(Level::TryArm); return Ok(true); } @@ -338,10 +340,26 @@ impl<'a> ExpressionParser<'a> { return Ok(false); } - // `catch` handled similar to `do`, including requiring the keyword. - if let Try::Catch = i { - self.instrs.push(Instruction::Catch); + // After a try's `do`, there are several possible kinds of handlers. + if let Try::CatchOrUnwind = i { + // `catch` may be followed by more `catch`s or `catch_all`. if parser.parse::>()?.is_some() { + let evt = parser.parse::>()?; + self.instrs.push(Instruction::Catch(evt)); + *i = Try::Catch; + self.stack.push(Level::TryArm); + return Ok(true); + } + // `catch_all` can only come at the end and has no argument. + if parser.parse::>()?.is_some() { + self.instrs.push(Instruction::CatchAll); + *i = Try::End; + self.stack.push(Level::TryArm); + return Ok(true); + } + // `unwind` is similar to `catch_all`. + if parser.parse::>()?.is_some() { + self.instrs.push(Instruction::Unwind); *i = Try::End; self.stack.push(Level::TryArm); return Ok(true); @@ -349,6 +367,23 @@ impl<'a> ExpressionParser<'a> { return Ok(false); } + if let Try::Catch = i { + if parser.parse::>()?.is_some() { + let evt = parser.parse::>()?; + self.instrs.push(Instruction::Catch(evt)); + *i = Try::Catch; + self.stack.push(Level::TryArm); + return Ok(true); + } + if parser.parse::>()?.is_some() { + self.instrs.push(Instruction::CatchAll); + *i = Try::End; + self.stack.push(Level::TryArm); + return Ok(true); + } + return Err(parser.error("unexpected items after `catch`")); + } + Err(parser.error("too many payloads inside of `(try)`")) } } @@ -997,11 +1032,12 @@ instructions! { V128Load64Zero(MemArg<8>) : [0xfd, 0xfd] : "v128.load64_zero", // Exception handling proposal + CatchAll : [0x05] : "catch_all", // Reuses the else opcode. Try(BlockType<'a>) : [0x06] : "try", - Catch : [0x07] : "catch", + Catch(ast::Index<'a>) : [0x07] : "catch", Throw(ast::Index<'a>) : [0x08] : "throw", - Rethrow : [0x09] : "rethrow", - BrOnExn(BrOnExn<'a>) : [0x0a] : "br_on_exn", + Rethrow(ast::Index<'a>) : [0x09] : "rethrow", + Unwind : [0x0a] : "unwind", } } diff --git a/crates/wast/src/ast/mod.rs b/crates/wast/src/ast/mod.rs index da5ebd9b2d..aeb555896f 100644 --- a/crates/wast/src/ast/mod.rs +++ b/crates/wast/src/ast/mod.rs @@ -346,6 +346,7 @@ pub mod kw { custom_keyword!(binary); custom_keyword!(block); custom_keyword!(catch); + custom_keyword!(catch_all); custom_keyword!(code); custom_keyword!(data); custom_keyword!(declare); @@ -416,6 +417,7 @@ pub mod kw { custom_keyword!(table); custom_keyword!(then); custom_keyword!(r#try = "try"); + custom_keyword!(unwind); custom_keyword!(v128); } diff --git a/crates/wast/src/resolve/names.rs b/crates/wast/src/resolve/names.rs index d7f6e3ee07..8062cee11a 100644 --- a/crates/wast/src/resolve/names.rs +++ b/crates/wast/src/resolve/names.rs @@ -1950,10 +1950,13 @@ impl<'a, 'b> ExprResolver<'a, 'b> { Throw(i) => { self.module.resolve(i, Ns::Event)?; } - BrOnExn(b) => { - self.resolve_label(&mut b.label)?; - self.module.resolve(&mut b.exn, Ns::Event)?; + Rethrow(i) => { + self.resolve_label(i)?; } + Catch(i) => { + self.module.resolve(i, Ns::Event)?; + } + BrOnCast(b) => { self.resolve_label(&mut b.label)?; self.module.resolve_heaptype(&mut b.val)?; diff --git a/tests/local/exception-handling.wast b/tests/local/exception-handling.wast index 62bc99c538..94291e3ac1 100644 --- a/tests/local/exception-handling.wast +++ b/tests/local/exception-handling.wast @@ -1,29 +1,36 @@ ;; --enable-exceptions --enable-multi-value (module (type (func (param i32 i64))) + (type (func (param i32))) (event (type 0)) - (func $check-br_on_exn-rethrow (param exnref) - block $l (result i32 i64) - local.get 0 - ;; exnref $e is on the stack at this point - br_on_exn $l 0 ;; branch to $l with $e's arguments - rethrow - end - drop - drop - ) + (event (type 1)) (func $check-throw i32.const 1 i64.const 2 throw 0 ) - (func $check-try-w-calls (result i32) - try (result i32) + (func $check-try-catch-rethrow + try (result i32 i64) call $check-throw - i32.const 0 - catch - call $check-br_on_exn-rethrow + unreachable + catch 0 + ;; the exception arguments are on the stack at this point + catch 1 + i64.const 2 + catch_all + rethrow 0 + end + drop + drop + ) + (func $check-unwind (local i32) + try i32.const 1 + local.set 0 + call $check-throw + unwind + i32.const 0 + local.set 0 end ) ) @@ -36,8 +43,21 @@ (assert_invalid (module - (type (func)) - (func (param exnref) - local.get 0 - br_on_exn 0 0)) - "unknown event: event index out of bounds") + (func try catch_all catch_all end)) + ;; we can't distinguish between `catch_all` and `else` in error cases + "else found outside of an `if` block") + +(assert_invalid + (module + (func try catch_all catch 0 end)) + "catch found outside of an `try` block") + +(assert_invalid + (module + (func try unwind i32.const 1 end)) + "type mismatch: values remaining on stack at end of block") + +(assert_invalid + (module + (func block try catch_all rethrow 1 end end)) + "rethrow target was not a `catch` block") diff --git a/tests/local/try.wast b/tests/local/try.wast index 555af254bb..51256ed07f 100644 --- a/tests/local/try.wast +++ b/tests/local/try.wast @@ -2,13 +2,13 @@ (assert_malformed (module quote - "(func (try (catch)))" + "(func (try (catch $exn)))" ) "previous `try` had no `do`") (assert_malformed (module quote - "(func (try (unreachable) (catch)))" + "(func (try (unreachable) (catch $exn)))" ) "previous `try` had no `do`") @@ -26,13 +26,18 @@ (assert_malformed (module quote - "(func (try (do) (catch) drop))" + "(func (try (do) (catch $exn) drop))" ) "expected `(`") (assert_malformed (module quote - "(func (try (do) (catch) (drop)))" + "(func (try (do) (catch $exn) (drop)))" ) - "too many payloads inside of `(try)`") + "unexpected items after `catch`") +(assert_malformed + (module quote + "(func (try (do) (unwind) (drop)))" + ) + "too many payloads inside of `(try)`") diff --git a/tests/local/try.wat b/tests/local/try.wat index ae7b209cb5..40c711a462 100644 --- a/tests/local/try.wat +++ b/tests/local/try.wat @@ -1,9 +1,14 @@ ;; --enable-exceptions (module $m - (func (try (do) (catch drop))) - (func (try (do) (catch rethrow))) + (type (func)) + (event $exn (type 0)) + (func (try (do) (catch $exn drop))) + (func (try (do) (catch $exn rethrow 0))) + (func (try (do) (catch_all rethrow 0))) + (func (try (do) (catch $exn) (catch_all rethrow 0))) + (func (try (do) (unwind nop))) (func (result i32) (try (result i32) (do (i32.const 42)) - (catch drop (i32.const 42))))) + (catch $exn drop (i32.const 42))))) diff --git a/tests/roundtrip.rs b/tests/roundtrip.rs index 14ba6fbf17..e8c51dc75f 100644 --- a/tests/roundtrip.rs +++ b/tests/roundtrip.rs @@ -135,6 +135,21 @@ fn skip_test(test: &Path, contents: &[u8]) -> bool { "interp/reference-types.txt", "expr/reference-types.txt", "parse/all-features.txt", + // uses old exception handling proposal + // FIXME: remove when wabt starts supporting the new proposal + "desugar/try.txt", + "dump/br_on_exn.txt", + "dump/rethrow.txt", + "dump/try.txt", + "dump/try-multi.txt", + "expr/br_on_exn.txt", + "expr/rethrow.txt", + "expr/try.txt", + "expr/try-multi.txt", + "roundtrip/fold-br_on_exn.txt", + "roundtrip/fold-rethrow.txt", + "roundtrip/fold-try.txt", + "roundtrip/rethrow.txt", ]; if broken.iter().any(|x| test.ends_with(x)) { return true; @@ -253,6 +268,9 @@ impl TestState { && !test.ends_with("reference-types/binary.wast") && !test.ends_with("exception-handling/binary.wast") + // wabt uses the old exceptions proposal + && !test.ends_with("local/exception-handling.wast") + // not implemented in wabt && !test.iter().any(|t| t == "module-linking") && !test.ends_with("multi-memory.wast")