From bef3bbd768e1b9d85f164bef8f77df6dc7138963 Mon Sep 17 00:00:00 2001 From: Asumu Takikawa Date: Tue, 16 Feb 2021 17:22:35 -0800 Subject: [PATCH 1/2] Give `catch_all` its own opcode Previously `catch_all` shared an opcode with `else`, but the spec now allocates it the 0x19 opcode. --- src/binary-reader-ir.cc | 7 ++- src/binary-reader-logging.cc | 1 + src/binary-reader-logging.h | 1 + src/binary-reader-nop.h | 1 + src/binary-reader-objdump.cc | 1 + src/binary-reader.cc | 6 ++ src/binary-reader.h | 1 + src/binary-writer.cc | 2 +- src/interp/interp.cc | 1 + src/interp/istream.cc | 1 + src/opcode-code-table.c | 1 - src/opcode.def | 1 + src/opcode.h | 1 - src/wat-writer.cc | 4 +- test/binary/bad-multiple-catch-all.txt | 4 +- test/dump/try-catch-all.txt | 87 ++++++++++++++++++++++++++ test/gen-wasm.py | 2 +- 17 files changed, 111 insertions(+), 11 deletions(-) create mode 100644 test/dump/try-catch-all.txt diff --git a/src/binary-reader-ir.cc b/src/binary-reader-ir.cc index 53a5223348..cdda3311ba 100644 --- a/src/binary-reader-ir.cc +++ b/src/binary-reader-ir.cc @@ -145,6 +145,7 @@ class BinaryReaderIR : public BinaryReaderNop { Index default_target_depth) override; Result OnCallExpr(Index func_index) override; Result OnCatchExpr(Index event_index) override; + Result OnCatchAllExpr() override; Result OnCallIndirectExpr(Index sig_index, Index table_index) override; Result OnReturnCallExpr(Index func_index) override; Result OnReturnCallIndirectExpr(Index sig_index, Index table_index) override; @@ -759,8 +760,6 @@ Result BinaryReaderIR::OnElseExpr() { if_expr->true_.end_loc = GetLocation(); label->exprs = &if_expr->false_; label->label_type = LabelType::Else; - } else if (label->label_type == LabelType::Try) { - return AppendCatch(Catch(GetLocation())); } else { PrintError("else expression without matching if"); return Result::Error; @@ -1007,6 +1006,10 @@ Result BinaryReaderIR::OnCatchExpr(Index except_index) { return AppendCatch(Catch(Var(except_index, GetLocation()))); } +Result BinaryReaderIR::OnCatchAllExpr() { + return AppendCatch(Catch(GetLocation())); +} + Result BinaryReaderIR::OnUnwindExpr() { LabelNode* label = nullptr; CHECK_RESULT(TopLabel(&label)); diff --git a/src/binary-reader-logging.cc b/src/binary-reader-logging.cc index 2c2d1e7f04..5cebbe9267 100644 --- a/src/binary-reader-logging.cc +++ b/src/binary-reader-logging.cc @@ -794,6 +794,7 @@ DEFINE_OPCODE(OnBinaryExpr) DEFINE_INDEX_DESC(OnCallExpr, "func_index") DEFINE_INDEX_INDEX(OnCallIndirectExpr, "sig_index", "table_index") DEFINE_INDEX_DESC(OnCatchExpr, "event_index"); +DEFINE0(OnCatchAllExpr); DEFINE_OPCODE(OnCompareExpr) DEFINE_OPCODE(OnConvertExpr) DEFINE_INDEX_DESC(OnDelegateExpr, "depth"); diff --git a/src/binary-reader-logging.h b/src/binary-reader-logging.h index 30dc0712e7..70dcbae641 100644 --- a/src/binary-reader-logging.h +++ b/src/binary-reader-logging.h @@ -164,6 +164,7 @@ class BinaryReaderLogging : public BinaryReaderDelegate { Index default_target_depth) override; Result OnCallExpr(Index func_index) override; Result OnCatchExpr(Index event_index) override; + Result OnCatchAllExpr() override; Result OnCallIndirectExpr(Index sig_index, Index table_index) override; Result OnCompareExpr(Opcode opcode) override; Result OnConvertExpr(Opcode opcode) override; diff --git a/src/binary-reader-nop.h b/src/binary-reader-nop.h index 1f29a67084..f72dbf105a 100644 --- a/src/binary-reader-nop.h +++ b/src/binary-reader-nop.h @@ -234,6 +234,7 @@ class BinaryReaderNop : public BinaryReaderDelegate { Result OnCallExpr(Index func_index) override { return Result::Ok; } Result OnCallIndirectExpr(Index sig_index, Index table_index) override { return Result::Ok; } Result OnCatchExpr(Index event_index) override { return Result::Ok; } + Result OnCatchAllExpr() override { return Result::Ok; } Result OnCompareExpr(Opcode opcode) override { return Result::Ok; } Result OnConvertExpr(Opcode opcode) override { return Result::Ok; } Result OnDelegateExpr(Index depth) override { return Result::Ok; } diff --git a/src/binary-reader-objdump.cc b/src/binary-reader-objdump.cc index f03283e755..11fba30b55 100644 --- a/src/binary-reader-objdump.cc +++ b/src/binary-reader-objdump.cc @@ -596,6 +596,7 @@ void BinaryReaderObjdumpDisassemble::LogOpcode(size_t data_size, switch (current_opcode) { case Opcode::Else: case Opcode::Catch: + case Opcode::CatchAll: case Opcode::Unwind: indent_level--; default: diff --git a/src/binary-reader.cc b/src/binary-reader.cc index 495de3063c..9b365e8715 100644 --- a/src/binary-reader.cc +++ b/src/binary-reader.cc @@ -1341,6 +1341,12 @@ Result BinaryReader::ReadFunctionBody(Offset end_offset) { break; } + case Opcode::CatchAll: { + CALLBACK(OnCatchAllExpr); + CALLBACK(OnOpcodeBare); + break; + } + case Opcode::Unwind: { CALLBACK0(OnUnwindExpr); CALLBACK0(OnOpcodeBare); diff --git a/src/binary-reader.h b/src/binary-reader.h index 6ba4c9b7a5..19643aa864 100644 --- a/src/binary-reader.h +++ b/src/binary-reader.h @@ -231,6 +231,7 @@ class BinaryReaderDelegate { virtual Result OnCallExpr(Index func_index) = 0; virtual Result OnCallIndirectExpr(Index sig_index, Index table_index) = 0; virtual Result OnCatchExpr(Index event_index) = 0; + virtual Result OnCatchAllExpr() = 0; virtual Result OnCompareExpr(Opcode opcode) = 0; virtual Result OnConvertExpr(Opcode opcode) = 0; virtual Result OnDelegateExpr(Index depth) = 0; diff --git a/src/binary-writer.cc b/src/binary-writer.cc index 6ef3e71d30..c744aca4ee 100644 --- a/src/binary-writer.cc +++ b/src/binary-writer.cc @@ -976,7 +976,7 @@ void BinaryWriter::WriteExpr(const Func* func, const Expr* expr) { case TryKind::Catch: for (const Catch& catch_ : try_expr->catches) { if (catch_.IsCatchAll()) { - WriteOpcode(stream_, Opcode::Else); + WriteOpcode(stream_, Opcode::CatchAll); } else { WriteOpcode(stream_, Opcode::Catch); WriteU32Leb128(stream_, GetEventVarDepth(&catch_.var), diff --git a/src/interp/interp.cc b/src/interp/interp.cc index ce1897bb01..5988be1e5e 100644 --- a/src/interp/interp.cc +++ b/src/interp/interp.cc @@ -1736,6 +1736,7 @@ RunResult Thread::StepInternal(Trap::Ptr* out_trap) { case O::Try: case O::Catch: + case O::CatchAll: case O::Unwind: case O::Delegate: case O::Throw: diff --git a/src/interp/istream.cc b/src/interp/istream.cc index 6c531a885d..6e95667523 100644 --- a/src/interp/istream.cc +++ b/src/interp/istream.cc @@ -693,6 +693,7 @@ Instr Istream::Read(Offset* offset) const { case Opcode::Block: case Opcode::Catch: + case Opcode::CatchAll: case Opcode::Delegate: case Opcode::Else: case Opcode::End: diff --git a/src/opcode-code-table.c b/src/opcode-code-table.c index 6a746c5ef9..c3e06d036e 100644 --- a/src/opcode-code-table.c +++ b/src/opcode-code-table.c @@ -27,7 +27,6 @@ typedef enum WabtOpcodeEnum { #include "opcode.def" #undef WABT_OPCODE Invalid, - CatchAll = Else, } WabtOpcodeEnum; WABT_STATIC_ASSERT(Invalid <= WABT_OPCODE_CODE_TABLE_SIZE); diff --git a/src/opcode.def b/src/opcode.def index 2156f11840..0c661c1941 100644 --- a/src/opcode.def +++ b/src/opcode.def @@ -56,6 +56,7 @@ WABT_OPCODE(___, ___, ___, ___, 0, 0, 0x11, CallIndirect, "call_indirect WABT_OPCODE(___, ___, ___, ___, 0, 0, 0x12, ReturnCall, "return_call", "") WABT_OPCODE(___, ___, ___, ___, 0, 0, 0x13, ReturnCallIndirect, "return_call_indirect", "") WABT_OPCODE(___, ___, ___, ___, 0, 0, 0x18, Delegate, "delegate", "") +WABT_OPCODE(___, ___, ___, ___, 0, 0, 0x19, CatchAll, "catch_all", "") WABT_OPCODE(___, ___, ___, ___, 0, 0, 0x1a, Drop, "drop", "") WABT_OPCODE(___, ___, ___, I32, 0, 0, 0x1b, Select, "select", "") WABT_OPCODE(___, ___, ___, I32, 0, 0, 0x1c, SelectT, "select", "") diff --git a/src/opcode.h b/src/opcode.h index 80d3181bd9..94cd4cb8f6 100644 --- a/src/opcode.h +++ b/src/opcode.h @@ -39,7 +39,6 @@ struct Opcode { #include "src/opcode.def" #undef WABT_OPCODE Invalid, - CatchAll = Else, }; // Static opcode objects. diff --git a/src/wat-writer.cc b/src/wat-writer.cc index fd102130a4..cdea3b1b2b 100644 --- a/src/wat-writer.cc +++ b/src/wat-writer.cc @@ -881,9 +881,7 @@ Result WatWriter::ExprVisitorDelegate::OnCatchExpr( TryExpr* expr, Catch* catch_) { writer_->Dedent(); if (catch_->IsCatchAll()) { - // We use a literal instead of doing GetName() on the opcode because - // `else` and `catch_all` share an opcode. - writer_->WritePutsNewline("catch_all"); + writer_->WritePutsNewline(Opcode::CatchAll_Opcode.GetName()); } else { writer_->WritePutsSpace(Opcode::Catch_Opcode.GetName()); writer_->WriteVar(catch_->var, NextChar::Newline); diff --git a/test/binary/bad-multiple-catch-all.txt b/test/binary/bad-multiple-catch-all.txt index 5951e0ae45..fd44362595 100644 --- a/test/binary/bad-multiple-catch-all.txt +++ b/test/binary/bad-multiple-catch-all.txt @@ -17,7 +17,7 @@ section(CODE) { } (;; STDERR ;;; error: only one catch_all allowed in try block -000001b: error: OnElseExpr callback failed +000001b: error: OnCatchAllExpr callback failed error: only one catch_all allowed in try block -000001b: error: OnElseExpr callback failed +000001b: error: OnCatchAllExpr callback failed ;;; STDERR ;;) diff --git a/test/dump/try-catch-all.txt b/test/dump/try-catch-all.txt new file mode 100644 index 0000000000..703765b2de --- /dev/null +++ b/test/dump/try-catch-all.txt @@ -0,0 +1,87 @@ +;;; TOOL: run-objdump +;;; ARGS0: -v --enable-exceptions +(module + (event $e (param i32)) + (func + try $try1 (result i32) + nop + i32.const 7 + catch_all + i32.const 8 + end + drop)) +(;; STDERR ;;; +0000000: 0061 736d ; WASM_BINARY_MAGIC +0000004: 0100 0000 ; WASM_BINARY_VERSION +; section "Type" (1) +0000008: 01 ; section code +0000009: 00 ; section size (guess) +000000a: 02 ; num types +; func type 0 +000000b: 60 ; func +000000c: 01 ; num params +000000d: 7f ; i32 +000000e: 00 ; num results +; func type 1 +000000f: 60 ; func +0000010: 00 ; num params +0000011: 00 ; num results +0000009: 08 ; FIXUP section size +; section "Function" (3) +0000012: 03 ; section code +0000013: 00 ; section size (guess) +0000014: 01 ; num functions +0000015: 01 ; function 0 signature index +0000013: 02 ; FIXUP section size +; section "Event" (13) +0000016: 0d ; section code +0000017: 00 ; section size (guess) +0000018: 01 ; event count +; event 0 +0000019: 00 ; event attribute +000001a: 00 ; event signature index +0000017: 03 ; FIXUP section size +; section "DataCount" (12) +000001b: 0c ; section code +000001c: 00 ; section size (guess) +000001d: 00 ; data count +000001c: 01 ; FIXUP section size +; section "Code" (10) +000001e: 0a ; section code +000001f: 00 ; section size (guess) +0000020: 01 ; num functions +; function body 0 +0000021: 00 ; func body size (guess) +0000022: 00 ; local decl count +0000023: 06 ; try +0000024: 7f ; i32 +0000025: 01 ; nop +0000026: 41 ; i32.const +0000027: 07 ; i32 literal +0000028: 19 ; catch_all +0000029: 41 ; i32.const +000002a: 08 ; i32 literal +000002b: 0b ; end +000002c: 1a ; drop +000002d: 0b ; end +0000021: 0c ; FIXUP func body size +000001f: 0e ; FIXUP section size +; move data: [1e, 2e) -> [1b, 2b) +; truncate to 43 (0x2b) +;;; STDERR ;;) +(;; STDOUT ;;; + +try-catch-all.wasm: file format wasm 0x1 + +Code Disassembly: + +00001f func[0]: + 000020: 06 7f | try i32 + 000022: 01 | nop + 000023: 41 07 | i32.const 7 + 000025: 19 | catch_all + 000026: 41 08 | i32.const 8 + 000028: 0b | end + 000029: 1a | drop + 00002a: 0b | end +;;; STDOUT ;;) diff --git a/test/gen-wasm.py b/test/gen-wasm.py index 4dbee6bfe3..5794fbf926 100755 --- a/test/gen-wasm.py +++ b/test/gen-wasm.py @@ -268,7 +268,7 @@ # exceptions "try": 0x06, - "catch_all": 0x05, + "catch_all": 0x19, } keywords = { From a1f9e70d14d207a4ff029f4831c7083bc8823669 Mon Sep 17 00:00:00 2001 From: Asumu Takikawa Date: Tue, 16 Feb 2021 19:44:18 -0800 Subject: [PATCH 2/2] Adjust rethrow depth semantics Previously this had interpreted the rethrow depth argument as counting only catch blocks, but the spec has clarified that it should count all blocks (in a similar fashion as `br` and related instructions). --- src/type-checker.cc | 32 +++++++++--------- test/typecheck/bad-rethrow-depth.txt | 49 +++++++++++++++++++++++----- test/typecheck/rethrow.txt | 47 +++++++++++++++++++------- 3 files changed, 92 insertions(+), 36 deletions(-) diff --git a/src/type-checker.cc b/src/type-checker.cc index 9136442bbe..40325facf9 100644 --- a/src/type-checker.cc +++ b/src/type-checker.cc @@ -71,28 +71,26 @@ Result TypeChecker::GetLabel(Index depth, Label** out_label) { } Result TypeChecker::GetRethrowLabel(Index depth, Label** out_label) { - Index cur = 0, catches = 0; - std::string candidates; + if (Failed(GetLabel(depth, out_label))) { + return Result::Error; + } + + if ((*out_label)->label_type == LabelType::Catch) { + return Result::Ok; + } - while (cur < label_stack_.size()) { - *out_label = &label_stack_[label_stack_.size() - cur - 1]; - - if ((*out_label)->label_type == LabelType::Catch) { - if (catches == depth) { - return Result::Ok; - } else { - if (!candidates.empty()) { - candidates.append(", "); - } - candidates.append(std::to_string(catches)); - catches++; + std::string candidates; + for (Index idx = 0; idx < label_stack_.size(); idx++) { + LabelType type = label_stack_[label_stack_.size() - idx - 1].label_type; + if (type == LabelType::Catch) { + if (!candidates.empty()) { + candidates.append(", "); } + candidates.append(std::to_string(idx)); } - - cur++; } - if (catches == 0) { + if (candidates.empty()) { PrintError("rethrow not in try catch block"); } else { PrintError("invalid rethrow depth: %" PRIindex " (catches: %s)", depth, diff --git a/test/typecheck/bad-rethrow-depth.txt b/test/typecheck/bad-rethrow-depth.txt index 6740771d73..fb7ef4a218 100644 --- a/test/typecheck/bad-rethrow-depth.txt +++ b/test/typecheck/bad-rethrow-depth.txt @@ -4,19 +4,52 @@ (module (event $e) (func - try + try $l1 nop catch $e - block - try + block $l2 + try $l3 nop catch $e - rethrow 2 + try $l4 + rethrow $l4 + rethrow 0 + rethrow $l2 + rethrow 2 + catch $e + rethrow $l2 + rethrow 2 + end end end - end)) + end) + + (func + try + unwind + rethrow 0 + end) + ) (;; STDERR ;;; -out/test/typecheck/bad-rethrow-depth.txt:14:11: error: invalid rethrow depth: 2 (catches: 0, 1) - rethrow 2 - ^^^^^^^ +out/test/typecheck/bad-rethrow-depth.txt:15:13: error: invalid rethrow depth: 0 (catches: 1, 3) + rethrow $l4 + ^^^^^^^ +out/test/typecheck/bad-rethrow-depth.txt:16:13: error: invalid rethrow depth: 0 (catches: 1, 3) + rethrow 0 + ^^^^^^^ +out/test/typecheck/bad-rethrow-depth.txt:17:13: error: invalid rethrow depth: 2 (catches: 1, 3) + rethrow $l2 + ^^^^^^^ +out/test/typecheck/bad-rethrow-depth.txt:18:13: error: invalid rethrow depth: 2 (catches: 1, 3) + rethrow 2 + ^^^^^^^ +out/test/typecheck/bad-rethrow-depth.txt:20:13: error: invalid rethrow depth: 2 (catches: 0, 1, 3) + rethrow $l2 + ^^^^^^^ +out/test/typecheck/bad-rethrow-depth.txt:21:13: error: invalid rethrow depth: 2 (catches: 0, 1, 3) + rethrow 2 + ^^^^^^^ +out/test/typecheck/bad-rethrow-depth.txt:30:7: error: rethrow not in try catch block + rethrow 0 + ^^^^^^^ ;;; STDERR ;;) diff --git a/test/typecheck/rethrow.txt b/test/typecheck/rethrow.txt index 8b389364a5..0ba4e46581 100644 --- a/test/typecheck/rethrow.txt +++ b/test/typecheck/rethrow.txt @@ -3,27 +3,52 @@ (module (event $e) (func - try + try $l1 nop catch $e - block - try + block $l2 + try $l3 nop catch $e - rethrow 0 + try $l4 + rethrow $l3 + rethrow 1 + rethrow $l1 + rethrow 3 + catch $e + rethrow $l4 + rethrow 0 + rethrow $l3 + rethrow 1 + rethrow $l1 + rethrow 3 + end end end end) (func - try + try $l1 nop - catch $e - block - try + catch_all + block $l2 + try $l3 nop - catch $e - rethrow 1 + catch_all + try $l4 + rethrow $l3 + rethrow 1 + rethrow $l1 + rethrow 3 + catch_all + rethrow $l4 + rethrow 0 + rethrow $l3 + rethrow 1 + rethrow $l1 + rethrow 3 + end end end - end)) + end) + )