From f810d0939f6761109de248e62c21d6dcc5fb4fbd Mon Sep 17 00:00:00 2001 From: Asumu Takikawa Date: Thu, 29 Oct 2020 18:03:58 -0700 Subject: [PATCH] Add support for new exception handling proposal The exception handling proposal switched its design from an exnref-based approach with first-class exception reference values to a version with tagged catch blocks and implicit exception references. This commit adds support for this new semantics and revises tests to match. It does not yet remove exnref, as that can be done in a separate patch. Note: it does not add the `delegate` instruction in the new proposal yet, as it does not yet have an opcode assigned. --- crates/wasmparser/src/binary_reader.rs | 9 +-- crates/wasmparser/src/operators_validator.rs | 66 ++++++++++++++------ crates/wasmparser/src/primitives.rs | 6 +- crates/wasmprinter/src/lib.rs | 19 +++--- crates/wast/src/ast/expr.rs | 58 +++++++++++++---- crates/wast/src/ast/mod.rs | 2 + crates/wast/src/resolve/names.rs | 9 ++- tests/local/exception-handling.wast | 60 ++++++++++++------ tests/local/try.wast | 15 +++-- tests/local/try.wat | 11 +++- tests/roundtrip.rs | 18 ++++++ 11 files changed, 193 insertions(+), 80 deletions(-) diff --git a/crates/wasmparser/src/binary_reader.rs b/crates/wasmparser/src/binary_reader.rs index 1c9b2b0b3..96dc78675 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 e49c10fca..e764db784 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 a8fedb796..3f92a09ed 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 066b5fd5c..9ed318548 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 b771c9609..1dd3bd0eb 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 da5ebd9b2..aeb555896 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 d7f6e3ee0..8062cee11 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 62bc99c53..94291e3ac 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 555af254b..51256ed07 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 ae7b209cb..40c711a46 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 14ba6fbf1..e8c51dc75 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")