From 1e6c28bceb6a6f07c46bf5af38c7674d2cbdfe20 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Tue, 26 Jan 2021 06:35:50 -0800 Subject: [PATCH 1/5] [EH] Support reading/writing of delegate This adds support for reading/writing of the new 'delegate' instruction in the folded wast format, the stack IR format, the poppy IR format, and the binary format in Binaryen. We don't have a format spec written down yet, but please refer to WebAssembly/exception-handling#137 and WebAssembly/exception-handling#146 for the informal semantics. In the current version of spec `delegate` is basically a rethrow, but with branch-like immediate argument so that it can bypass other catches/delegates in between. 'delegate' is not represented a new `Expression`, but it is rather an option within a `Try` class, like `catch`/`catch_all`. One special thing about `delegate` is, even though it is written _within_ a `try` in the folded wat format, like ```wasm (try (do ... ) (delegate $l) ) ``` In the unfolded wat format or in the binary format, `delegate` serves as a scope end instruction so there is no separate `end`: ```wasm try ... delegate $l ``` `delegate` semantically targets an outer `catch` or `delegate`, but we write `delegate` target as a `try` label because we only give labels to block-like scoping expressions. So far we has not given `Try` a label and used inner blocks or a wrapping block in case a branch targets the `try`. But in case of `delegate`, it can syntactically only target `try` and if it targets blocks or loops it is a validation failure. So after discussions in #3497, we give `Try` a label but this label can only be targeted by `delegate`s. Unfortunately this makes parsing and writing of `Try` expression somewhat complicated. Also there is one special case; if the immediate argument of `try` is the same as the depth of control flow stack, this means the 'delegate' delegates to the caller. To handle this case this adds a fake label `DELEGATE_CALLER_TARGET`, and when writing it back to the wast format writes it as an immediate value, unlike other cases in which we write labels. This uses `DELEGATE_FIELD_SCOPE_NAME_DEF/USE` to represent `try`'s label and `delegate`'s target. There are many cases that `try` and `delegate`'s labels need to be treated in the same way as block and branch labels, such as for hashing or comparing. But there are routines in which we automatically assume all label uses are branches. I thought about adding a new kind of defines such as `DELEGATE_FIELD_TRY_NAME_DEF/USE`, but I think it will also involve some duplication of existing routines or classes. So at the moment this PR chooses to use the existing `DELEGATE_FIELD_SCOPE_NAME_DEF/USE` for `try` and `delegate` labels and makes only necessary amount of changes in branch-utils. We can revisit this decision later if necessary. Many of changes to the existing test cases are because now all `try`s are automatically assigned a label. They will be removed in `RemoveUnusedNames` pass in the same way as block labels if not targeted by any delegates. This only supports reading and writing and has not been tested against any optimization passes yet. --- src/ir/ExpressionAnalyzer.cpp | 20 +- src/ir/branch-utils.h | 48 ++++- src/ir/properties.h | 10 +- src/passes/Poppify.cpp | 8 + src/passes/Print.cpp | 76 +++++++- src/passes/RemoveUnusedNames.cpp | 4 + src/passes/StackIR.cpp | 4 +- src/shared-constants.h | 1 + src/wasm-binary.h | 4 + src/wasm-delegations-fields.h | 2 + src/wasm-s-parser.h | 2 +- src/wasm-stack.h | 15 +- src/wasm-type.h | 1 - src/wasm.h | 4 + src/wasm/wasm-binary.cpp | 173 ++++++++++++------ src/wasm/wasm-s-parser.cpp | 56 ++++-- src/wasm/wasm-stack.cpp | 16 +- src/wasm/wasm-validator.cpp | 72 +++++++- src/wasm/wasm.cpp | 1 + test/break-within-catch.wasm.fromBinary | 2 +- test/exception-handling.wast | 87 +++++++++ test/exception-handling.wast.from-wast | 122 ++++++++++-- test/exception-handling.wast.fromBinary | 126 +++++++++++-- ...ption-handling.wast.fromBinary.noDebugInfo | 126 +++++++++++-- .../optimize-instructions-exceptions.wast | 2 +- test/lit/passes/poppify.wast | 38 +++- test/passes/code-pushing_all-features.txt | 6 +- test/passes/dce_all-features.txt | 8 +- test/passes/dwarf_with_exceptions.bin.txt | 12 +- ...e-stack-ir_print-stack-ir_all-features.txt | 40 +++- ...-stack-ir_print-stack-ir_all-features.wast | 18 +- ...ures_disable-typed-function-references.txt | 2 +- ...ve-unused-module-elements_all-features.txt | 2 +- test/passes/rse_all-features.txt | 22 +-- test/passes/simplify-locals_all-features.txt | 10 +- test/passes/vacuum_all-features.txt | 6 +- test/reference-types.wast.from-wast | 8 +- test/reference-types.wast.fromBinary | 8 +- ...eference-types.wast.fromBinary.noDebugInfo | 8 +- test/spec/exception-handling.wast | 38 +++- test/try-delegate.wasm | Bin 0 -> 42 bytes test/try-delegate.wasm.fromBinary | 23 +++ 42 files changed, 1048 insertions(+), 183 deletions(-) create mode 100644 test/try-delegate.wasm create mode 100644 test/try-delegate.wasm.fromBinary diff --git a/src/ir/ExpressionAnalyzer.cpp b/src/ir/ExpressionAnalyzer.cpp index 04ef52026f3..0b0adae9e27 100644 --- a/src/ir/ExpressionAnalyzer.cpp +++ b/src/ir/ExpressionAnalyzer.cpp @@ -17,6 +17,7 @@ #include "ir/iteration.h" #include "ir/load-utils.h" #include "ir/utils.h" +#include "shared-constants.h" #include "support/hash.h" #include "support/small_vector.h" #include "wasm-traversal.h" @@ -261,6 +262,9 @@ size_t ExpressionAnalyzer::hash(Expression* curr) { Hasher(Expression* curr) { stack.push_back(curr); + // DELEGATE_CALLER_TARGET is a fake target used to denote delegating to + // the caller. Add it here to prevent the unknown name error. + noteScopeName(DELEGATE_CALLER_TARGET); while (stack.size() > 0) { curr = stack.back(); @@ -327,13 +331,15 @@ size_t ExpressionAnalyzer::hash(Expression* curr) { } } void visitScopeName(Name curr) { - // Names are relative, we give the same hash for - // (block $x (br $x)) - // (block $y (br $y)) - static_assert(sizeof(Index) == sizeof(int32_t), - "wasm64 will need changes here"); - assert(internalNames.find(curr) != internalNames.end()); - rehash(digest, internalNames[curr]); + if (curr.is()) { // try's delegate target can be null + // Names are relative, we give the same hash for + // (block $x (br $x)) + // (block $y (br $y)) + static_assert(sizeof(Index) == sizeof(int32_t), + "wasm64 will need changes here"); + assert(internalNames.find(curr) != internalNames.end()); + rehash(digest, internalNames[curr]); + } } void visitNonScopeName(Name curr) { rehash(digest, uint64_t(curr.str)); } void visitType(Type curr) { rehash(digest, curr.getID()); } diff --git a/src/ir/branch-utils.h b/src/ir/branch-utils.h index a9561ef07e1..8a4f02a580b 100644 --- a/src/ir/branch-utils.h +++ b/src/ir/branch-utils.h @@ -40,9 +40,9 @@ inline bool isBranchReachable(Expression* expr) { return true; } -// Perform a generic operation on uses of scope names (branch targets) in an -// expression. The provided function receives a Name& which it can modify if it -// needs to. +// Perform a generic operation on uses of scope names (branch + delegate +// targets) in an expression. The provided function receives a Name& which it +// can modify if it needs to. template void operateOnScopeNameUses(Expression* expr, T func) { #define DELEGATE_ID expr->_id @@ -83,7 +83,7 @@ void operateOnScopeNameUsesAndSentTypes(Expression* expr, T func) { } else if (auto* br = expr->dynCast()) { func(name, br->getCastType()); } else { - WASM_UNREACHABLE("bad br type"); + assert(expr->is()); // delegate } }); } @@ -135,6 +135,46 @@ inline bool replacePossibleTarget(Expression* branch, Name from, Name to) { return worked; } +// Replace all delegate targets within the given AST. +inline void replaceDelegateTargets(Expression* ast, Name from, Name to) { + struct Replacer + : public PostWalker> { + Name from, to; + Replacer(Name from, Name to) : from(from), to(to) {} + void visitExpression(Expression* curr) { + if (curr->is()) { + operateOnScopeNameUses(curr, [&](Name& name) { + if (name == from) { + name = to; + } + }); + } + } + }; + Replacer replacer(from, to); + replacer.walk(ast); +} + +// Replace all branch targets within the given AST. +inline void replaceBranchTargets(Expression* ast, Name from, Name to) { + struct Replacer + : public PostWalker> { + Name from, to; + Replacer(Name from, Name to) : from(from), to(to) {} + void visitExpression(Expression* curr) { + if (Properties::isBranch(curr)) { + operateOnScopeNameUses(curr, [&](Name& name) { + if (name == from) { + name = to; + } + }); + } + } + }; + Replacer replacer(from, to); + replacer.walk(ast); +} + // Returns the set of targets to which we branch that are // outside of an expression. inline NameSet getExitingBranches(Expression* ast) { diff --git a/src/ir/properties.h b/src/ir/properties.h index b60c74d9804..8ae7befba26 100644 --- a/src/ir/properties.h +++ b/src/ir/properties.h @@ -69,13 +69,15 @@ inline bool isControlFlowStructure(Expression* curr) { curr->is(); } -// Check if an expression is a control flow construct with a name, -// which implies it may have breaks to it. +// Check if an expression is a control flow construct with a name, which implies +// it may have breaks or delegates to it. inline bool isNamedControlFlow(Expression* curr) { if (auto* block = curr->dynCast()) { return block->name.is(); } else if (auto* loop = curr->dynCast()) { return loop->name.is(); + } else if (auto* try_ = curr->dynCast()) { + return try_->name.is(); } return false; } @@ -104,6 +106,10 @@ inline bool isConstantExpression(const Expression* curr) { return false; } +inline bool isBranch(const Expression* curr) { + return curr->is() || curr->is() || curr->is(); +} + inline Literal getLiteral(const Expression* curr) { if (auto* c = curr->dynCast()) { return c->value; diff --git a/src/passes/Poppify.cpp b/src/passes/Poppify.cpp index b0d07e60cb0..2ab222697e6 100644 --- a/src/passes/Poppify.cpp +++ b/src/passes/Poppify.cpp @@ -128,6 +128,7 @@ struct Poppifier : BinaryenIRWriter { void emitIfElse(If* curr); void emitCatch(Try* curr, Index i); void emitCatchAll(Try* curr); + void emitDelegate(Try* curr); void emitScopeEnd(Expression* curr); void emitFunctionEnd(); void emitUnreachable(); @@ -272,6 +273,13 @@ void Poppifier::emitCatchAll(Try* curr) { scopeStack.emplace_back(Scope::Catch); } +void Poppifier::emitDelegate(Try* curr) { + auto& scope = scopeStack.back(); + assert(scope.kind == Scope::Try); + patchScope(curr->body); + scopeStack.back().instrs.push_back(curr); +} + void Poppifier::emitScopeEnd(Expression* curr) { switch (scopeStack.back().kind) { case Scope::Block: diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp index e49735af0f5..9d8dadda514 100644 --- a/src/passes/Print.cpp +++ b/src/passes/Print.cpp @@ -1764,6 +1764,10 @@ struct PrintExpressionContents void visitRefEq(RefEq* curr) { printMedium(o, "ref.eq"); } void visitTry(Try* curr) { printMedium(o, "try"); + if (curr->name.is()) { + o << ' '; + printName(curr->name, o); + } if (curr->type.isConcrete()) { o << ' ' << ResultType(curr->type); } @@ -1955,6 +1959,8 @@ struct PrintSExpression : public OverriddenVisitor { Function::DebugLocation lastPrintedLocation; bool debugInfo; + int controlFlowDepth = 0; + PrintSExpression(std::ostream& o) : o(o) { setMinify(false); if (!full) { @@ -2100,6 +2106,9 @@ struct PrintSExpression : public OverriddenVisitor { break; // that's all we can recurse, start to unwind } } + + int startControlFlowDepth = controlFlowDepth; + controlFlowDepth += stack.size(); auto* top = stack.back(); while (stack.size() > 0) { curr = stack.back(); @@ -2129,8 +2138,10 @@ struct PrintSExpression : public OverriddenVisitor { o << ' ' << curr->name; } } + controlFlowDepth = startControlFlowDepth; } void visitIf(If* curr) { + controlFlowDepth++; o << '('; printExpressionContents(curr); incIndent(); @@ -2147,8 +2158,10 @@ struct PrintSExpression : public OverriddenVisitor { if (full) { o << " ;; end if"; } + controlFlowDepth--; } void visitLoop(Loop* curr) { + controlFlowDepth++; o << '('; printExpressionContents(curr); incIndent(); @@ -2160,6 +2173,7 @@ struct PrintSExpression : public OverriddenVisitor { o << ' ' << curr->name; } } + controlFlowDepth--; } void visitBreak(Break* curr) { o << '('; @@ -2490,13 +2504,28 @@ struct PrintSExpression : public OverriddenVisitor { // (do // ... // ) - // (catch - // ... + // (catch $e + // ... + // ) + // ... + // (catch_all + // ... // ) // ) - // The parenthesis wrapping 'catch' is just a syntax and does not affect - // nested depths of instructions within. + // The parenthesis wrapping do/catch/catch_all is just a syntax and does not + // affect nested depths of instructions within. + // + // try-delegate is written in the forded format as + // (try + // (do + // ... + // ) + // (delegate $label) + // ) + // When the 'delegate' delegates to the caller, we write the argument as an + // immediate. void visitTry(Try* curr) { + controlFlowDepth++; o << '('; printExpressionContents(curr); incIndent(); @@ -2521,12 +2550,26 @@ struct PrintSExpression : public OverriddenVisitor { if (curr->hasCatchAll()) { doIndent(o, indent); printDebugDelimiterLocation(curr, curr->catchEvents.size()); - o << "(catch_all"; + o << '('; + printMedium(o, "catch_all"); incIndent(); maybePrintImplicitBlock(curr->catchBodies.back(), true); decIndent(); o << "\n"; } + controlFlowDepth--; + + if (curr->isDelegate()) { + doIndent(o, indent); + o << '('; + printMedium(o, "delegate "); + if (curr->delegateTarget == DELEGATE_CALLER_TARGET) { + o << controlFlowDepth; + } else { + printName(curr->delegateTarget, o); + } + o << ")\n"; + } decIndent(); if (full) { o << " ;; end try"; @@ -2913,6 +2956,7 @@ struct PrintSExpression : public OverriddenVisitor { } else { printFullLine(curr->body); } + assert(controlFlowDepth == 0); } else { // Print the stack IR. printStackIR(curr->stackIR.get(), o, curr); @@ -3324,6 +3368,11 @@ printStackInst(StackInst* inst, std::ostream& o, Function* func) { printMedium(o, "catch_all"); break; } + case StackInst::Delegate: { + printMedium(o, "delegate "); + printName(inst->origin->cast()->delegateTarget, o); + break; + } default: WASM_UNREACHABLE("unexpeted op"); } @@ -3339,6 +3388,7 @@ printStackIR(StackIR* ir, std::ostream& o, Function* func) { } }; + int controlFlowDepth = 0; // Stack to track indices of catches within a try SmallVector catchIndexStack; for (Index i = 0; i < (*ir).size(); i++) { @@ -3364,6 +3414,7 @@ printStackIR(StackIR* ir, std::ostream& o, Function* func) { case StackInst::BlockBegin: case StackInst::IfBegin: case StackInst::LoopBegin: { + controlFlowDepth++; doIndent(); PrintExpressionContents(func, o).visit(inst->origin); indent++; @@ -3375,6 +3426,7 @@ printStackIR(StackIR* ir, std::ostream& o, Function* func) { case StackInst::BlockEnd: case StackInst::IfEnd: case StackInst::LoopEnd: { + controlFlowDepth--; indent--; doIndent(); printMedium(o, "end"); @@ -3403,11 +3455,25 @@ printStackIR(StackIR* ir, std::ostream& o, Function* func) { indent++; break; } + case StackInst::Delegate: { + controlFlowDepth--; + indent--; + doIndent(); + printMedium(o, "delegate "); + Try* curr = inst->origin->cast(); + if (curr->delegateTarget == DELEGATE_CALLER_TARGET) { + o << controlFlowDepth; + } else { + printName(curr->delegateTarget, o); + } + break; + } default: WASM_UNREACHABLE("unexpeted op"); } std::cout << '\n'; } + assert(controlFlowDepth == 0); return o; } diff --git a/src/passes/RemoveUnusedNames.cpp b/src/passes/RemoveUnusedNames.cpp index 64640aefe5a..be7ae8e3c23 100644 --- a/src/passes/RemoveUnusedNames.cpp +++ b/src/passes/RemoveUnusedNames.cpp @@ -83,6 +83,10 @@ struct RemoveUnusedNames } } + void visitTry(Try* curr) { + handleBreakTarget(curr->name); + } + void visitFunction(Function* curr) { assert(branchesSeen.empty()); } }; diff --git a/src/passes/StackIR.cpp b/src/passes/StackIR.cpp index 0732a6499a2..6d163d897ab 100644 --- a/src/passes/StackIR.cpp +++ b/src/passes/StackIR.cpp @@ -258,6 +258,7 @@ class StackIROptimizer { case StackInst::LoopEnd: case StackInst::Catch: case StackInst::CatchAll: + case StackInst::Delegate: case StackInst::TryEnd: { return true; } @@ -284,7 +285,8 @@ class StackIROptimizer { case StackInst::BlockEnd: case StackInst::IfEnd: case StackInst::LoopEnd: - case StackInst::TryEnd: { + case StackInst::TryEnd: + case StackInst::Delegate: { return true; } default: { return false; } diff --git a/src/shared-constants.h b/src/shared-constants.h index 569fd792d44..5520f15ac24 100644 --- a/src/shared-constants.h +++ b/src/shared-constants.h @@ -60,6 +60,7 @@ extern Name CASE; extern Name BR; extern Name FUNCREF; extern Name FAKE_RETURN; +extern Name DELEGATE_CALLER_TARGET; extern Name MUT; extern Name SPECTEST; extern Name PRINT; diff --git a/src/wasm-binary.h b/src/wasm-binary.h index a4d41530ed3..c2a771a7d30 100644 --- a/src/wasm-binary.h +++ b/src/wasm-binary.h @@ -1027,6 +1027,7 @@ enum ASTNodes { Try = 0x06, Catch = 0x07, CatchAll = 0x05, + Delegate = 0x18, Throw = 0x08, Rethrow = 0x09, @@ -1417,6 +1418,8 @@ class WasmBinaryBuilder { // the names that breaks target. this lets us know if a block has breaks to it // or not. std::unordered_set breakTargetNames; + // the names that delegates target. + std::unordered_set delegateTargetNames; std::vector expressionStack; @@ -1521,6 +1524,7 @@ class WasmBinaryBuilder { Expression* getBlockOrSingleton(Type type); BreakTarget getBreakTarget(int32_t offset); + Name getDelegateTargetName(int32_t offset); void readMemoryAccess(Address& alignment, Address& offset); diff --git a/src/wasm-delegations-fields.h b/src/wasm-delegations-fields.h index 033da6314ac..9677aa4de02 100644 --- a/src/wasm-delegations-fields.h +++ b/src/wasm-delegations-fields.h @@ -515,8 +515,10 @@ switch (DELEGATE_ID) { } case Expression::Id::TryId: { DELEGATE_START(Try); + DELEGATE_FIELD_SCOPE_NAME_USE(Try, delegateTarget); DELEGATE_FIELD_CHILD_VECTOR(Try, catchBodies); DELEGATE_FIELD_NAME_VECTOR(Try, catchEvents); + DELEGATE_FIELD_SCOPE_NAME_DEF(Try, name); DELEGATE_FIELD_CHILD(Try, body); DELEGATE_END(Try); break; diff --git a/src/wasm-s-parser.h b/src/wasm-s-parser.h index 8a92c0935a3..838b90b0229 100644 --- a/src/wasm-s-parser.h +++ b/src/wasm-s-parser.h @@ -242,7 +242,7 @@ class SExpressionWasmBuilder { i++; } } - Name getLabel(Element& s); + Name getLabel(Element& s, bool isBranch = true); Expression* makeBreak(Element& s); Expression* makeBreakTable(Element& s); Expression* makeReturn(Element& s); diff --git a/src/wasm-stack.h b/src/wasm-stack.h index bd44ed72e78..7f94573fa5f 100644 --- a/src/wasm-stack.h +++ b/src/wasm-stack.h @@ -71,6 +71,7 @@ class StackInst { TryBegin, // the beginning of a try Catch, // the catch within a try CatchAll, // the catch_all within a try + Delegate, // the delegate within a try TryEnd // the ending of a try } op; @@ -109,6 +110,7 @@ class BinaryInstWriter : public OverriddenVisitor { void emitIfElse(If* curr); void emitCatch(Try* curr, Index i); void emitCatchAll(Try* curr); + void emitDelegate(Try* curr); // emit an end at the end of a block/loop/if/try void emitScopeEnd(Expression* curr); // emit an end at the end of a function @@ -169,6 +171,9 @@ class BinaryenIRWriter : public Visitor> { void emitCatchAll(Try* curr) { static_cast(this)->emitCatchAll(curr); } + void emitDelegate(Try* curr) { + static_cast(this)->emitDelegate(curr); + } void emitScopeEnd(Expression* curr) { static_cast(this)->emitScopeEnd(curr); } @@ -343,7 +348,11 @@ template void BinaryenIRWriter::visitTry(Try* curr) { emitCatchAll(curr); visitPossibleBlockContents(curr->catchBodies.back()); } - emitScopeEnd(curr); + if (curr->isDelegate()) { + emitDelegate(curr); + } else { + emitScopeEnd(curr); + } if (curr->type == Type::unreachable) { emitUnreachable(); } @@ -375,6 +384,7 @@ class BinaryenIRToBinaryWriter void emitIfElse(If* curr) { writer.emitIfElse(curr); } void emitCatch(Try* curr, Index i) { writer.emitCatch(curr, i); } void emitCatchAll(Try* curr) { writer.emitCatchAll(curr); } + void emitDelegate(Try* curr) { writer.emitDelegate(curr); } void emitScopeEnd(Expression* curr) { writer.emitScopeEnd(curr); } void emitFunctionEnd() { if (func->epilogLocation.size()) { @@ -414,6 +424,9 @@ class StackIRGenerator : public BinaryenIRWriter { void emitCatchAll(Try* curr) { stackIR.push_back(makeStackInst(StackInst::CatchAll, curr)); } + void emitDelegate(Try* curr) { + stackIR.push_back(makeStackInst(StackInst::Delegate, curr)); + } void emitFunctionEnd() {} void emitUnreachable() { stackIR.push_back(makeStackInst(Builder(module).makeUnreachable())); diff --git a/src/wasm-type.h b/src/wasm-type.h index fe1b832bc22..4188c438d4f 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -137,7 +137,6 @@ class Type { bool isRef() const; bool isFunction() const; bool isData() const; - bool isException() const; bool isNullable() const; bool isRtt() const; bool isStruct() const; diff --git a/src/wasm.h b/src/wasm.h index 7fd284756d8..876a9bfa70b 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -1306,13 +1306,17 @@ class Try : public SpecificExpression { public: Try(MixedArena& allocator) : catchEvents(allocator), catchBodies(allocator) {} + Name name; // label that can only be targeted by 'delegate's Expression* body; ArenaVector catchEvents; ExpressionList catchBodies; + Name delegateTarget; // target try's label bool hasCatchAll() const { return catchBodies.size() - catchEvents.size() == 1; } + bool isCatch() const { return !catchBodies.empty(); } + bool isDelegate() const { return delegateTarget.is(); } void finalize(); void finalize(Type type_); }; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index f3ec376d12c..d8031b858d3 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -1918,7 +1918,9 @@ void WasmBinaryBuilder::readFunctions() { debugLocation.clear(); willBeIgnored = false; // process body - assert(breakTargetNames.size() == 0); + assert(breakStack.empty()); + assert(breakTargetNames.empty()); + assert(delegateTargetNames.empty()); assert(breakStack.empty()); assert(expressionStack.empty()); assert(controlFlowStack.empty()); @@ -1926,8 +1928,9 @@ void WasmBinaryBuilder::readFunctions() { assert(depth == 0); func->body = getBlockOrSingleton(func->sig.results); assert(depth == 0); - assert(breakStack.size() == 0); - assert(breakTargetNames.size() == 0); + assert(breakStack.empty()); + assert(breakTargetNames.empty()); + assert(delegateTargetNames.empty()); if (!expressionStack.empty()) { throwError("stack not empty on function exit"); } @@ -2210,7 +2213,8 @@ void WasmBinaryBuilder::processExpressions() { } auto peek = input[pos]; if (peek == BinaryConsts::End || peek == BinaryConsts::Else || - peek == BinaryConsts::Catch || peek == BinaryConsts::CatchAll) { + peek == BinaryConsts::Catch || peek == BinaryConsts::CatchAll || + peek == BinaryConsts::Delegate) { BYN_TRACE("== processExpressions finished with unreachable" << std::endl); lastSeparator = BinaryConsts::ASTNodes(peek); @@ -2987,6 +2991,14 @@ BinaryConsts::ASTNodes WasmBinaryBuilder::readExpression(Expression*& curr) { } break; } + case BinaryConsts::Delegate: { + curr = nullptr; + if (DWARF && currFunction) { + assert(!controlFlowStack.empty()); + controlFlowStack.pop_back(); + } + break; + } case BinaryConsts::RefNull: visitRefNull((curr = allocator.alloc())->cast()); break; @@ -3376,7 +3388,8 @@ Expression* WasmBinaryBuilder::getBlockOrSingleton(Type type) { block->name = label; block->finalize(type); // maybe we don't need a block here? - if (breakTargetNames.find(block->name) == breakTargetNames.end()) { + if (breakTargetNames.find(block->name) == breakTargetNames.end() && + delegateTargetNames.find(block->name) == delegateTargetNames.end()) { block->name = Name(); if (block->list.size() == 1) { return block->list[0]; @@ -3452,6 +3465,28 @@ WasmBinaryBuilder::getBreakTarget(int32_t offset) { return ret; } +Name WasmBinaryBuilder::getDelegateTargetName(int32_t offset) { + BYN_TRACE("getDelegateTarget " << offset << std::endl); + // We always start parsing a function by creating a block label and pushing it + // in breakStack in getBlockOrSingleton, so if a 'delegate''s target is that + // block, it does not mean it targets that block; it throws to the caller. + if (breakStack.size() - 1 == size_t(offset)) { + return DELEGATE_CALLER_TARGET; + } + size_t index = breakStack.size() - 1 - offset; + if (index > breakStack.size()) { + throwError("bad delegate index (high)"); + } + BYN_TRACE("delegate target " << breakStack[index].name << std::endl); + auto& ret = breakStack[index]; + // if the delegate is in literally unreachable code, then we will not emit it + // anyhow, so do not note that the target has delegate to it + if (!willBeIgnored) { + delegateTargetNames.insert(ret.name); + } + return ret.name; +} + void WasmBinaryBuilder::visitBreak(Break* curr, uint8_t code) { BYN_TRACE("zz node: Break, code " << int32_t(code) << std::endl); BreakTarget target = getBreakTarget(getU32LEB()); @@ -5746,58 +5781,13 @@ void WasmBinaryBuilder::visitTryOrTryInBlock(Expression*& out) { curr->type = getType(); curr->body = getBlockOrSingleton(curr->type); if (lastSeparator != BinaryConsts::Catch && - lastSeparator != BinaryConsts::CatchAll) { + lastSeparator != BinaryConsts::CatchAll && + lastSeparator != BinaryConsts::Delegate) { throwError("No catch instruction within a try scope"); } - // For simplicity, we create an inner block within the catch body too, but the - // one within the 'catch' *must* be omitted when we write out the binary back - // later, because the 'catch' instruction pushes a value onto the stack and - // the inner block does not support block input parameters without multivalue - // support. - // try - // ... - // catch ;; Pushes a value onto the stack - // block ;; Inner block. Should be deleted when writing binary! - // use the pushed value - // end - // end - // - // But when input binary code is like - // try - // ... - // catch - // br 0 - // end - // - // 'br 0' accidentally happens to target the inner block, creating code like - // this in Binaryen IR, making the inner block not deletable, resulting in a - // validation error: - // (try - // ... - // (catch - // (block $label0 ;; Cannot be deleted, because there's a branch to this - // ... - // (br $label0) - // ) - // ) - // ) - // - // When this happens, we fix this by creating a block that wraps the whole - // try-catch, and making the branches target that block instead, like this: - // (block $label ;; New enclosing block, new target for the branch - // (try - // ... - // (catch - // (block ;; Now this can be deleted when writing binary - // ... - // (br $label0) - // ) - // ) - // ) - // ) - Builder builder(wasm); + // A nameless label shared by all catch body blocks Name catchLabel = getNextLabel(); breakStack.push_back({catchLabel, curr->type}); @@ -5839,8 +5829,84 @@ void WasmBinaryBuilder::visitTryOrTryInBlock(Expression*& out) { readCatchBody(Type::none); } } + breakStack.pop_back(); + + if (lastSeparator == BinaryConsts::Delegate) { + curr->delegateTarget = getDelegateTargetName(getU32LEB()); + } + + // For simplicity, we make try's labels only can be targeted by delegates, and + // delegates can only target try's labels. (If they target blocks or loops, it + // is a validation failure.) Because we create an inner block within each try + // and catch body, if any delegate targets those inner blocks, we should make + // them target the try's label instead. + curr->name = getNextLabel(); + if (auto *block = curr->body->dynCast()) { + if (block->name.is()) { + if (delegateTargetNames.find(block->name) != delegateTargetNames.end()) { + BranchUtils::replaceDelegateTargets(block, block->name, curr->name); + delegateTargetNames.erase(block->name); + } + // maybe we don't need a block here? + if (block->list.size() == 1) { + curr->body = block->list[0]; + } + } + } + if (delegateTargetNames.find(catchLabel) != delegateTargetNames.end()) { + for (auto* catchBody : curr->catchBodies) { + BranchUtils::replaceDelegateTargets(catchBody, catchLabel, curr->name); + } + delegateTargetNames.erase(catchLabel); + } curr->finalize(curr->type); + // For simplicity, we create an inner block within the catch body too, but the + // one within the 'catch' *must* be omitted when we write out the binary back + // later, because the 'catch' instruction pushes a value onto the stack and + // the inner block does not support block input parameters without multivalue + // support. + // try + // ... + // catch $e ;; Pushes value(s) onto the stack + // block ;; Inner block. Should be deleted when writing binary! + // use the pushed value + // end + // end + // + // But when input binary code is like + // try + // ... + // catch $e + // br 0 + // end + // + // 'br 0' accidentally happens to target the inner block, creating code like + // this in Binaryen IR, making the inner block not deletable, resulting in a + // validation error: + // (try + // ... + // (catch $e + // (block $label0 ;; Cannot be deleted, because there's a branch to this + // ... + // (br $label0) + // ) + // ) + // ) + // + // When this happens, we fix this by creating a block that wraps the whole + // try-catch, and making the branches target that block instead, like this: + // (block $label ;; New enclosing block, new target for the branch + // (try + // ... + // (catch $e + // (block ;; Now this can be deleted when writing binary + // ... + // (br $label0) + // ) + // ) + // ) + // ) if (breakTargetNames.find(catchLabel) == breakTargetNames.end()) { out = curr; } else { @@ -5848,7 +5914,6 @@ void WasmBinaryBuilder::visitTryOrTryInBlock(Expression*& out) { auto* block = builder.makeBlock(catchLabel, curr); out = block; } - breakStack.pop_back(); breakTargetNames.erase(catchLabel); } diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index bcce3993ec7..996862060bc 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -1859,7 +1859,7 @@ Expression* SExpressionWasmBuilder::makeCallIndirect(Element& s, return ret; } -Name SExpressionWasmBuilder::getLabel(Element& s) { +Name SExpressionWasmBuilder::getLabel(Element& s, bool isBranch) { if (s.dollared()) { return nameMapper.sourceToUnique(s.str()); } else { @@ -1876,10 +1876,14 @@ Name SExpressionWasmBuilder::getLabel(Element& s) { throw ParseException("invalid label", s.line, s.col); } if (offset == nameMapper.labelStack.size()) { - // a break to the function's scope. this means we need an automatic block, - // with a name - brokeToAutoBlock = true; - return FAKE_RETURN; + if (isBranch) { + // a break to the function's scope. this means we need an automatic + // block, with a name + brokeToAutoBlock = true; + return FAKE_RETURN; + } + // This is a delegate that delegates to the caller + return DELEGATE_CALLER_TARGET; } return nameMapper.labelStack[nameMapper.labelStack.size() - 1 - offset]; } @@ -1975,12 +1979,13 @@ Expression* SExpressionWasmBuilder::makeRefEq(Element& s) { return ret; } -// try-catch-end is written in the folded wast format as +// try can be either in the form of try-catch or try-delegate. +// try-catch is written in the folded wast format as // (try // (do // ... // ) -// (catch +// (catch $e // ... // ) // ... @@ -1991,6 +1996,14 @@ Expression* SExpressionWasmBuilder::makeRefEq(Element& s) { // Any number of catch blocks can exist, including none. Zero or one catch_all // block can exist, and if it does, it should be at the end. There should be at // least one catch or catch_all body per try. +// +// try-delegate is written in the forded format as +// (try +// (do +// ... +// ) +// (delegate $label) +// ) Expression* SExpressionWasmBuilder::makeTry(Element& s) { auto ret = allocator.alloc(); Index i = 1; @@ -2001,7 +2014,7 @@ Expression* SExpressionWasmBuilder::makeTry(Element& s) { } else { sName = "try"; } - auto label = nameMapper.pushLabelName(sName); + ret->name = nameMapper.pushLabelName(sName); Type type = parseOptionalResultType(s, i); // signature if (!elementStartsWith(*s[i], "do")) { @@ -2027,21 +2040,38 @@ Expression* SExpressionWasmBuilder::makeTry(Element& s) { ret->catchBodies.push_back(makeMaybeBlock(*s[i++], 1, type)); } + // 'delegate' cannot target its own try. So we pop the name here. + nameMapper.popLabelName(ret->name); + + if (i < s.size() && elementStartsWith(*s[i], "delegate")) { + Element& inner = *s[i++]; + if (inner.size() != 2) { + throw ParseException("invalid delegate", inner.line, inner.col); + } + ret->delegateTarget = getLabel(*inner[1], false); + } + if (i != s.size()) { throw ParseException( "there should be at most one catch_all block at the end", s.line, s.col); } - if (ret->catchBodies.empty()) { - throw ParseException("no catch bodies", s.line, s.col); + if (ret->catchBodies.empty() && !ret->isDelegate()) { + throw ParseException("no catch bodies or delegate", s.line, s.col); } ret->finalize(type); - nameMapper.popLabelName(label); + // create a break target if we must - if (BranchUtils::BranchSeeker::has(ret, label)) { + if (BranchUtils::BranchSeeker::has(ret, ret->name)) { auto* block = allocator.alloc(); - block->name = label; + // We create a different name for the wrapping block, because try's name can + // be used by internal delegates + block->name = nameMapper.pushLabelName(sName); + // For simplicity, try's name canonly be targeted by delegates. Make the + // branches target the new wrapping block instead. + BranchUtils::replaceBranchTargets(ret, ret->name, block->name); block->list.push_back(ret); + nameMapper.popLabelName(block->name); block->finalize(type); return block; } diff --git a/src/wasm/wasm-stack.cpp b/src/wasm/wasm-stack.cpp index 00b1e536858..cb9d9409328 100644 --- a/src/wasm/wasm-stack.cpp +++ b/src/wasm/wasm-stack.cpp @@ -1904,7 +1904,7 @@ void BinaryInstWriter::visitRefEq(RefEq* curr) { } void BinaryInstWriter::visitTry(Try* curr) { - breakStack.emplace_back(IMPOSSIBLE_CONTINUE); + breakStack.push_back(curr->name); o << int8_t(BinaryConsts::Try); emitResultType(curr->type); } @@ -1924,6 +1924,13 @@ void BinaryInstWriter::emitCatchAll(Try* curr) { o << int8_t(BinaryConsts::CatchAll); } +void BinaryInstWriter::emitDelegate(Try* curr) { + assert(!breakStack.empty()); + breakStack.pop_back(); + o << int8_t(BinaryConsts::Delegate) + << U32LEB(getBreakIndex(curr->delegateTarget)); +} + void BinaryInstWriter::visitThrow(Throw* curr) { o << int8_t(BinaryConsts::Throw) << U32LEB(parent.getEventIndex(curr->event)); } @@ -2216,6 +2223,9 @@ void BinaryInstWriter::emitMemoryAccess(size_t alignment, } int32_t BinaryInstWriter::getBreakIndex(Name name) { // -1 if not found + if (name == DELEGATE_CALLER_TARGET) { + return breakStack.size(); + } for (int i = breakStack.size() - 1; i >= 0; i--) { if (breakStack[i] == name) { return breakStack.size() - 1 - i; @@ -2321,6 +2331,10 @@ void StackIRToBinaryWriter::write() { writer.emitCatchAll(inst->origin->cast()); break; } + case StackInst::Delegate: { + writer.emitDelegate(inst->origin->cast()); + break; + } default: WASM_UNREACHABLE("unexpected op"); } diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 7e05bd37514..fff08ba0331 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -236,6 +236,7 @@ struct FunctionValidator : public WalkerPass> { }; std::unordered_map breakInfos; + std::unordered_set delegateTargetNames; std::set returnTypes; // types used in returns @@ -276,11 +277,52 @@ struct FunctionValidator : public WalkerPass> { void visitLoop(Loop* curr); void visitIf(If* curr); + static void visitPreTry(FunctionValidator* self, Expression** currp) { + auto* curr = (*currp)->cast(); + if (curr->name.is()) { + self->delegateTargetNames.insert(curr->name); + } + } + + // We remove try's label before proceeding to verify catch bodies because the + // following is a validation failure: + // (try $l0 + // (do ... ) + // (catch $e + // (try + // (do ...) + // (delegate $l0) ;; validation failure + // ) + // ) + // ) + // Unlike branches, if delegate's target 'catch' is located above the + // delegate, it is a validation failure. + static void visitPreCatch(FunctionValidator* self, Expression** currp) { + auto* curr = (*currp)->cast(); + if (curr->name.is()) { + self->delegateTargetNames.erase(curr->name); + } + } + // override scan to add a pre and a post check task to all nodes static void scan(FunctionValidator* self, Expression** currp) { + auto* curr = *currp; + // Treat 'Try' specially because we need to run visitPreCatch between the + // try body and catch bodies + if (curr->is()) { + self->pushTask(doVisitTry, currp); + auto& list = curr->cast()->catchBodies; + for (int i = int(list.size()) - 1; i >= 0; i--) { + self->pushTask(scan, &list[i]); + } + self->pushTask(visitPreCatch, currp); + self->pushTask(scan, &curr->cast()->body); + self->pushTask(visitPreTry, currp); + return; + } + PostWalker::scan(self, currp); - auto* curr = *currp; if (curr->is()) { self->pushTask(visitPreBlock, currp); } @@ -334,6 +376,7 @@ struct FunctionValidator : public WalkerPass> { void visitRefIs(RefIs* curr); void visitRefFunc(RefFunc* curr); void visitRefEq(RefEq* curr); + void noteDelegate(Name name, Expression* curr); void visitTry(Try* curr); void visitThrow(Throw* curr); void visitRethrow(Rethrow* curr); @@ -765,6 +808,7 @@ void FunctionValidator::noteBreak(Name name, Type valueType, Expression* curr) { } } } + void FunctionValidator::visitBreak(Break* curr) { noteBreak(curr->name, curr->value, curr); if (curr->value) { @@ -2027,6 +2071,14 @@ void FunctionValidator::visitRefEq(RefEq* curr) { "ref.eq's right argument should be a subtype of eqref"); } +void FunctionValidator::noteDelegate(Name name, Expression* curr) { + if (name != DELEGATE_CALLER_TARGET) { + shouldBeTrue(delegateTargetNames.find(name) != delegateTargetNames.end(), + curr, + "all delegate targets must be valid"); + } +} + void FunctionValidator::visitTry(Try* curr) { shouldBeTrue(getModule()->features.hasExceptionHandling(), curr, @@ -2059,6 +2111,21 @@ void FunctionValidator::visitTry(Try* curr) { shouldBeTrue(curr->catchBodies.size() - curr->catchEvents.size() <= 1, curr, "the number of catch blocks and events do not match"); + + shouldBeFalse(curr->isCatch() && curr->isDelegate(), + curr, + "try cannot have both catch and delegate at the same time"); + shouldBeTrue(curr->isCatch() || curr->isDelegate(), + curr, + "try should have either catches or a delegate"); + + // (try $l (delegate $l)) is invalid. So we pop the current try's name before + // checking the delegate target. + delegateTargetNames.erase(curr->name); + + if (curr->isDelegate()) { + noteDelegate(curr->delegateTarget, curr); + } } void FunctionValidator::visitThrow(Throw* curr) { @@ -2468,6 +2535,9 @@ void FunctionValidator::visitFunction(Function* curr) { shouldBeTrue( breakInfos.empty(), curr->body, "all named break targets must exist"); + shouldBeTrue(delegateTargetNames.empty(), + curr->body, + "all named delegate targets must exist"); returnTypes.clear(); labelNames.clear(); // validate optional local names diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp index 7bf9b8604c8..92e03f57908 100644 --- a/src/wasm/wasm.cpp +++ b/src/wasm/wasm.cpp @@ -88,6 +88,7 @@ Name CASE("case"); Name BR("br"); Name FUNCREF("funcref"); Name FAKE_RETURN("fake_return_waka123"); +Name DELEGATE_CALLER_TARGET("delegate_caller_target_waka123"); Name MUT("mut"); Name SPECTEST("spectest"); Name PRINT("print"); diff --git a/test/break-within-catch.wasm.fromBinary b/test/break-within-catch.wasm.fromBinary index d7e2dc57b35..fd16d9fcce6 100644 --- a/test/break-within-catch.wasm.fromBinary +++ b/test/break-within-catch.wasm.fromBinary @@ -4,7 +4,7 @@ (event $event$0 (attr 0) (param i32)) (func $0 (block $label$2 - (try + (try $label$3 (do (nop) ) diff --git a/test/exception-handling.wast b/test/exception-handling.wast index b96c4d25f19..399fef14c3d 100644 --- a/test/exception-handling.wast +++ b/test/exception-handling.wast @@ -102,6 +102,35 @@ ) ) + ;; nested try-catch + (try + (do + (try + (do + (throw $e-i32 (i32.const 0)) + ) + (catch $e-i32 + (drop (pop i32)) + ) + (catch_all) + ) + ) + (catch $e-i32 + (drop (pop i32)) + ) + (catch_all + (try + (do + (throw $e-i32 (i32.const 0)) + ) + (catch $e-i32 + (drop (pop i32)) + ) + (catch_all) + ) + ) + ) + ;; rethrow (try (do @@ -113,4 +142,62 @@ ) ) ) + + (func $delegate-test + ;; Inner delegates target an outer catch + (try $l0 + (do + (try + (do + (call $foo) + ) + (delegate $l0) ;; by label + ) + (try + (do + (call $foo) + ) + (delegate 0) ;; by depth + ) + ) + (catch_all) + ) + + ;; When there are both a branch and a delegate that target the same try + ;; label. Because binaryen only allows blocks and loops to be targetted by + ;; branches, we wrap the try with a block and make branches that block + ;; instead, resulting in the br and delegate target different labels in the + ;; output. + (try $l0 + (do + (try + (do + (br_if $l0 (i32.const 1)) + ) + (delegate $l0) ;; by label + ) + (try + (do + (br_if $l0 (i32.const 1)) + ) + (delegate 0) ;; by depth + ) + ) + (catch_all) + ) + + ;; The inner delegate targets the outer delegate, which in turn targets the + ;; caller. + (try $l0 + (do + (try + (do + (call $foo) + ) + (delegate $l0) + ) + ) + (delegate 0) + ) + ) ) diff --git a/test/exception-handling.wast.from-wast b/test/exception-handling.wast.from-wast index 224df569bb1..5a0a1249d2d 100644 --- a/test/exception-handling.wast.from-wast +++ b/test/exception-handling.wast.from-wast @@ -14,7 +14,7 @@ ) (func $eh_test (local $x (i32 i64)) - (try + (try $try (do (throw $e-i32 (i32.const 0) @@ -26,7 +26,7 @@ ) ) ) - (try + (try $try0 (do (throw $e-i32-i64 (i32.const 0) @@ -44,20 +44,20 @@ ) ) ) - (block $l1 - (try + (block $l11 + (try $l1 (do - (br $l1) + (br $l11) ) (catch $e-i32 (drop (pop i32) ) - (br $l1) + (br $l11) ) ) ) - (try + (try $try2 (do (nop) ) @@ -67,7 +67,7 @@ ) ) ) - (try + (try $try3 (do (call $foo) (call $bar) @@ -80,7 +80,7 @@ (call $bar) ) ) - (try + (try $try4 (do (throw $e-i32 (i32.const 0) @@ -97,7 +97,7 @@ ) ) ) - (try + (try $try5 (do (throw $e-i32 (i32.const 0) @@ -107,7 +107,7 @@ (nop) ) ) - (try + (try $try6 (do (throw $e-i32 (i32.const 0) @@ -128,7 +128,48 @@ (call $bar) ) ) - (try + (try $try7 + (do + (try $try8 + (do + (throw $e-i32 + (i32.const 0) + ) + ) + (catch $e-i32 + (drop + (pop i32) + ) + ) + (catch_all + (nop) + ) + ) + ) + (catch $e-i32 + (drop + (pop i32) + ) + ) + (catch_all + (try $try9 + (do + (throw $e-i32 + (i32.const 0) + ) + ) + (catch $e-i32 + (drop + (pop i32) + ) + ) + (catch_all + (nop) + ) + ) + ) + ) + (try $try10 (do (throw $e-i32 (i32.const 0) @@ -142,4 +183,61 @@ ) ) ) + (func $delegate-test + (try $l0 + (do + (try $try + (do + (call $foo) + ) + (delegate $l0) + ) + (try $try11 + (do + (call $foo) + ) + (delegate $l0) + ) + ) + (catch_all + (nop) + ) + ) + (block $l015 + (try $l012 + (do + (try $try13 + (do + (br_if $l015 + (i32.const 1) + ) + ) + (delegate $l012) + ) + (try $try14 + (do + (br_if $l015 + (i32.const 1) + ) + ) + (delegate $l012) + ) + ) + (catch_all + (nop) + ) + ) + ) + (try $l016 + (do + (try $try17 + (do + (call $foo) + ) + (delegate $l016) + ) + ) + (delegate 0) + ) + ) ) diff --git a/test/exception-handling.wast.fromBinary b/test/exception-handling.wast.fromBinary index d305aaaa929..d6dd331b13a 100644 --- a/test/exception-handling.wast.fromBinary +++ b/test/exception-handling.wast.fromBinary @@ -18,7 +18,7 @@ (local $2 (i32 i64)) (local $3 i32) (local $4 i32) - (try + (try $label$3 (do (throw $event$0 (i32.const 0) @@ -30,7 +30,7 @@ ) ) ) - (try + (try $label$6 (do (throw $event$2 (i32.const 0) @@ -69,20 +69,20 @@ ) ) ) - (block $label$5 - (try + (block $label$7 + (try $label$10 (do - (br $label$5) + (br $label$7) ) (catch $event$0 (drop (pop i32) ) - (br $label$5) + (br $label$7) ) ) ) - (try + (try $label$13 (do (nop) ) @@ -92,7 +92,7 @@ ) ) ) - (try + (try $label$16 (do (call $foo) (call $bar) @@ -105,7 +105,7 @@ (call $bar) ) ) - (try + (try $label$19 (do (throw $event$0 (i32.const 0) @@ -122,7 +122,7 @@ ) ) ) - (try + (try $label$22 (do (throw $event$0 (i32.const 0) @@ -132,7 +132,7 @@ (nop) ) ) - (try + (try $label$25 (do (throw $event$0 (i32.const 0) @@ -153,7 +153,48 @@ (call $bar) ) ) - (try + (try $label$34 + (do + (try $label$29 + (do + (throw $event$0 + (i32.const 0) + ) + ) + (catch $event$0 + (drop + (pop i32) + ) + ) + (catch_all + (nop) + ) + ) + ) + (catch $event$0 + (drop + (pop i32) + ) + ) + (catch_all + (try $label$33 + (do + (throw $event$0 + (i32.const 0) + ) + ) + (catch $event$0 + (drop + (pop i32) + ) + ) + (catch_all + (nop) + ) + ) + ) + ) + (try $label$37 (do (throw $event$0 (i32.const 0) @@ -167,5 +208,66 @@ ) ) ) + (func $delegate-test + (try $label$9 + (do + (block $label$1 + (try $label$4 + (do + (call $foo) + ) + (delegate $label$9) + ) + (try $label$7 + (do + (call $foo) + ) + (delegate $label$9) + ) + ) + ) + (catch_all + (nop) + ) + ) + (block $label$10 + (try $label$19 + (do + (block $label$11 + (try $label$14 + (do + (br_if $label$10 + (i32.const 1) + ) + ) + (delegate $label$19) + ) + (try $label$17 + (do + (br_if $label$10 + (i32.const 1) + ) + ) + (delegate $label$19) + ) + ) + ) + (catch_all + (nop) + ) + ) + ) + (try $label$25 + (do + (try $label$23 + (do + (call $foo) + ) + (delegate $label$25) + ) + ) + (delegate 0) + ) + ) ) diff --git a/test/exception-handling.wast.fromBinary.noDebugInfo b/test/exception-handling.wast.fromBinary.noDebugInfo index d6dc4268e6b..cf7372b6e49 100644 --- a/test/exception-handling.wast.fromBinary.noDebugInfo +++ b/test/exception-handling.wast.fromBinary.noDebugInfo @@ -18,7 +18,7 @@ (local $2 (i32 i64)) (local $3 i32) (local $4 i32) - (try + (try $label$3 (do (throw $event$0 (i32.const 0) @@ -30,7 +30,7 @@ ) ) ) - (try + (try $label$6 (do (throw $event$2 (i32.const 0) @@ -69,20 +69,20 @@ ) ) ) - (block $label$5 - (try + (block $label$7 + (try $label$10 (do - (br $label$5) + (br $label$7) ) (catch $event$0 (drop (pop i32) ) - (br $label$5) + (br $label$7) ) ) ) - (try + (try $label$13 (do (nop) ) @@ -92,7 +92,7 @@ ) ) ) - (try + (try $label$16 (do (call $0) (call $1) @@ -105,7 +105,7 @@ (call $1) ) ) - (try + (try $label$19 (do (throw $event$0 (i32.const 0) @@ -122,7 +122,7 @@ ) ) ) - (try + (try $label$22 (do (throw $event$0 (i32.const 0) @@ -132,7 +132,7 @@ (nop) ) ) - (try + (try $label$25 (do (throw $event$0 (i32.const 0) @@ -153,7 +153,48 @@ (call $1) ) ) - (try + (try $label$34 + (do + (try $label$29 + (do + (throw $event$0 + (i32.const 0) + ) + ) + (catch $event$0 + (drop + (pop i32) + ) + ) + (catch_all + (nop) + ) + ) + ) + (catch $event$0 + (drop + (pop i32) + ) + ) + (catch_all + (try $label$33 + (do + (throw $event$0 + (i32.const 0) + ) + ) + (catch $event$0 + (drop + (pop i32) + ) + ) + (catch_all + (nop) + ) + ) + ) + ) + (try $label$37 (do (throw $event$0 (i32.const 0) @@ -167,5 +208,66 @@ ) ) ) + (func $3 + (try $label$9 + (do + (block $label$1 + (try $label$4 + (do + (call $0) + ) + (delegate $label$9) + ) + (try $label$7 + (do + (call $0) + ) + (delegate $label$9) + ) + ) + ) + (catch_all + (nop) + ) + ) + (block $label$10 + (try $label$19 + (do + (block $label$11 + (try $label$14 + (do + (br_if $label$10 + (i32.const 1) + ) + ) + (delegate $label$19) + ) + (try $label$17 + (do + (br_if $label$10 + (i32.const 1) + ) + ) + (delegate $label$19) + ) + ) + ) + (catch_all + (nop) + ) + ) + ) + (try $label$25 + (do + (try $label$23 + (do + (call $0) + ) + (delegate $label$25) + ) + ) + (delegate 0) + ) + ) ) diff --git a/test/lit/passes/optimize-instructions-exceptions.wast b/test/lit/passes/optimize-instructions-exceptions.wast index e2b60e0f100..9d22224fbb8 100644 --- a/test/lit/passes/optimize-instructions-exceptions.wast +++ b/test/lit/passes/optimize-instructions-exceptions.wast @@ -5,7 +5,7 @@ (module ;; CHECK: (func $test ;; CHECK-NEXT: (if - ;; CHECK-NEXT: (try (result i32) + ;; CHECK-NEXT: (try $try (result i32) ;; CHECK-NEXT: (do ;; CHECK-NEXT: (i32.const 123) ;; CHECK-NEXT: ) diff --git a/test/lit/passes/poppify.wast b/test/lit/passes/poppify.wast index 1b9ba1fb371..bc88dd87832 100644 --- a/test/lit/passes/poppify.wast +++ b/test/lit/passes/poppify.wast @@ -178,7 +178,7 @@ ) ;; CHECK: (func $try-catch (result i32) - ;; CHECK-NEXT: (try (result i32) + ;; CHECK-NEXT: (try $try (result i32) ;; CHECK-NEXT: (do ;; CHECK-NEXT: (i32.const 0) ;; CHECK-NEXT: (throw $e @@ -208,6 +208,42 @@ ) ) + ;; CHECK: (func $try-delegate (result i32) + ;; CHECK-NEXT: (try $l0 (result i32) + ;; CHECK-NEXT: (do + ;; CHECK-NEXT: (try $try + ;; CHECK-NEXT: (do + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (throw $e + ;; CHECK-NEXT: (pop i32) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (delegate $l0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (catch $e + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $try-delegate (result i32) + (try $l0 i32 + (do + (try + (do + (throw $e + (i32.const 0) + ) + ) + (delegate $l0) + ) + ) + (catch $e + (pop i32) + ) + ) + ) + ;; CHECK: (func $tuple (result i32 i64) ;; CHECK-NEXT: (i32.const 0) ;; CHECK-NEXT: (i64.const 1) diff --git a/test/passes/code-pushing_all-features.txt b/test/passes/code-pushing_all-features.txt index 4b3d932080d..5226a7b3c9c 100644 --- a/test/passes/code-pushing_all-features.txt +++ b/test/passes/code-pushing_all-features.txt @@ -43,7 +43,7 @@ (func $can-push-past-try (local $x i32) (block $out - (try + (try $try (do (throw $e (i32.const 0) @@ -78,7 +78,7 @@ (local.set $x (i32.const 1) ) - (try + (try $try (do (call $foo) ) @@ -105,7 +105,7 @@ (local.set $x (i32.const 1) ) - (try + (try $try (do (throw $e (i32.const 0) diff --git a/test/passes/dce_all-features.txt b/test/passes/dce_all-features.txt index 447077b6ccd..575d0406599 100644 --- a/test/passes/dce_all-features.txt +++ b/test/passes/dce_all-features.txt @@ -547,7 +547,7 @@ (nop) ) (func $try_unreachable - (try + (try $try (do (unreachable) ) @@ -558,7 +558,7 @@ (call $foo) ) (func $catch_unreachable - (try + (try $try (do (nop) ) @@ -569,7 +569,7 @@ (call $foo) ) (func $both_unreachable - (try + (try $try (do (unreachable) ) @@ -616,7 +616,7 @@ ) ) (func $unnecessary-concrete-try (result i32) - (try + (try $try (do (unreachable) ) diff --git a/test/passes/dwarf_with_exceptions.bin.txt b/test/passes/dwarf_with_exceptions.bin.txt index 324e1d44d57..b11fa643747 100644 --- a/test/passes/dwarf_with_exceptions.bin.txt +++ b/test/passes/dwarf_with_exceptions.bin.txt @@ -22,7 +22,7 @@ (global.get $global$0) ) ;; code offset: 0x10 - (try + (try $label$9 (do ;; code offset: 0x12 (call $foo) @@ -47,7 +47,7 @@ ) ) ;; code offset: 0x31 - (try + (try $label$8 (do ;; code offset: 0x33 (call $foo) @@ -60,7 +60,7 @@ ;; code offset: 0x41 (catch_all ;; code offset: 0x42 - (try + (try $label$7 (do ;; code offset: 0x44 (call $__cxa_end_catch) @@ -443,7 +443,7 @@ file_names[ 1]: (global.get $global$0) ) ;; code offset: 0xe - (try + (try $label$9 (do ;; code offset: 0x10 (call $foo) @@ -468,7 +468,7 @@ file_names[ 1]: ) ) ;; code offset: 0x1f - (try + (try $label$8 (do ;; code offset: 0x21 (call $foo) @@ -481,7 +481,7 @@ file_names[ 1]: ;; code offset: 0x27 (catch_all ;; code offset: 0x28 - (try + (try $label$7 (do ;; code offset: 0x2a (call $__cxa_end_catch) diff --git a/test/passes/generate-stack-ir_optimize-stack-ir_print-stack-ir_all-features.txt b/test/passes/generate-stack-ir_optimize-stack-ir_print-stack-ir_all-features.txt index ffb2da5be26..f8e7778c502 100644 --- a/test/passes/generate-stack-ir_optimize-stack-ir_print-stack-ir_all-features.txt +++ b/test/passes/generate-stack-ir_optimize-stack-ir_print-stack-ir_all-features.txt @@ -3,17 +3,27 @@ (type $i32_=>_none (func (param i32))) (event $e0 (attr 0) (param i32)) (func $eh - try + try $try i32.const 0 throw $e0 catch $e0 drop - rethrow 0 catch_all rethrow 0 end - unreachable + try $l0 + try $try0 + i32.const 0 + throw $e0 + delegate $l0 + unreachable + catch_all + nop + end + try $l01 + nop + delegate 0 ) ) (module @@ -21,7 +31,7 @@ (type $i32_=>_none (func (param i32))) (event $e0 (attr 0) (param i32)) (func $eh (; has Stack IR ;) - (try + (try $try (do (throw $e0 (i32.const 0) @@ -31,11 +41,31 @@ (drop (pop i32) ) - (rethrow 0) ) (catch_all (rethrow 0) ) ) + (try $l0 + (do + (try $try0 + (do + (throw $e0 + (i32.const 0) + ) + ) + (delegate $l0) + ) + ) + (catch_all + (nop) + ) + ) + (try $l01 + (do + (nop) + ) + (delegate 0) + ) ) ) diff --git a/test/passes/generate-stack-ir_optimize-stack-ir_print-stack-ir_all-features.wast b/test/passes/generate-stack-ir_optimize-stack-ir_print-stack-ir_all-features.wast index 047d9c12667..f82f7e19ac6 100644 --- a/test/passes/generate-stack-ir_optimize-stack-ir_print-stack-ir_all-features.wast +++ b/test/passes/generate-stack-ir_optimize-stack-ir_print-stack-ir_all-features.wast @@ -8,11 +8,27 @@ ) (catch $e0 (drop (pop i32)) - (rethrow 0) ) (catch_all (rethrow 0) ) ) + + (try $l0 + (do + (try + (do + (throw $e0 (i32.const 0)) + ) + (delegate $l0) + ) + ) + (catch_all) + ) + + (try $l0 + (do) + (delegate 0) ;; delegate to caller + ) ) ) diff --git a/test/passes/instrument-locals_all-features_disable-typed-function-references.txt b/test/passes/instrument-locals_all-features_disable-typed-function-references.txt index edd9278513a..481d7a72b40 100644 --- a/test/passes/instrument-locals_all-features_disable-typed-function-references.txt +++ b/test/passes/instrument-locals_all-features_disable-typed-function-references.txt @@ -203,7 +203,7 @@ ) ) ) - (try + (try $try (do (nop) ) diff --git a/test/passes/remove-unused-module-elements_all-features.txt b/test/passes/remove-unused-module-elements_all-features.txt index 22751f9f597..fdd768e8b37 100644 --- a/test/passes/remove-unused-module-elements_all-features.txt +++ b/test/passes/remove-unused-module-elements_all-features.txt @@ -299,7 +299,7 @@ (export "e-export" (event $e-export)) (start $start) (func $start - (try + (try $try (do (throw $e-throw (i32.const 0) diff --git a/test/passes/rse_all-features.txt b/test/passes/rse_all-features.txt index 9a428266e9c..eaef1445526 100644 --- a/test/passes/rse_all-features.txt +++ b/test/passes/rse_all-features.txt @@ -477,7 +477,7 @@ ) (func $try1 (local $x i32) - (try + (try $try (do (nop) ) @@ -493,7 +493,7 @@ ) (func $try2 (local $x i32) - (try + (try $try (do (throw $e (i32.const 0) @@ -512,7 +512,7 @@ ) (func $try3 (local $x i32) - (try + (try $try (do (throw $e (i32.const 0) @@ -533,7 +533,7 @@ ) (func $try4 (local $x i32) - (try + (try $try (do (call $foo) (local.set $x @@ -550,7 +550,7 @@ ) (func $try5 (local $x i32) - (try + (try $try (do (local.set $x (i32.const 1) @@ -567,9 +567,9 @@ ) (func $nested-try1 (local $x i32) - (try + (try $try (do - (try + (try $try6 (do (throw $e (i32.const 0) @@ -592,9 +592,9 @@ ) (func $nested-try2 (local $x i32) - (try + (try $try (do - (try + (try $try7 (do (throw $e (i32.const 0) @@ -618,9 +618,9 @@ ) (func $nested-try3 (local $x i32) - (try + (try $try (do - (try + (try $try8 (do (throw $e (i32.const 0) diff --git a/test/passes/simplify-locals_all-features.txt b/test/passes/simplify-locals_all-features.txt index 34c13d4429c..21361af65e9 100644 --- a/test/passes/simplify-locals_all-features.txt +++ b/test/passes/simplify-locals_all-features.txt @@ -1905,7 +1905,7 @@ ) (func $pop-cannot-be-sinked (local $0 i32) - (try + (try $try (do (nop) ) @@ -1922,7 +1922,7 @@ ) (func $pop-within-catch-can-be-sinked (local $0 i32) - (try + (try $try (do (nop) ) @@ -1930,7 +1930,7 @@ (nop) (call $foo (i32.const 3) - (try (result i32) + (try $try0 (result i32) (do (i32.const 0) ) @@ -1950,7 +1950,7 @@ (local.set $0 (call $bar) ) - (try + (try $try (do (drop (local.get $0) @@ -1966,7 +1966,7 @@ (func $non-call-can-be-sinked-into-try (local $0 i32) (nop) - (try + (try $try (do (drop (i32.const 3) diff --git a/test/passes/vacuum_all-features.txt b/test/passes/vacuum_all-features.txt index 82fb7e50657..9acd26665a0 100644 --- a/test/passes/vacuum_all-features.txt +++ b/test/passes/vacuum_all-features.txt @@ -444,7 +444,7 @@ ) (func $inner-try-catch_all-test (local $0 i32) - (try + (try $try0 (do (throw $e (i32.const 0) @@ -459,9 +459,9 @@ ) (func $inner-try-catch-test (local $0 i32) - (try + (try $try (do - (try + (try $try1 (do (throw $e2 (i32.const 0) diff --git a/test/reference-types.wast.from-wast b/test/reference-types.wast.from-wast index 29aab9bea7d..9a89da4db4f 100644 --- a/test/reference-types.wast.from-wast +++ b/test/reference-types.wast.from-wast @@ -507,7 +507,7 @@ ) ) (drop - (try (result externref) + (try $try (result externref) (do (local.get $local_externref) ) @@ -520,7 +520,7 @@ ) ) (drop - (try (result funcref) + (try $try35 (result funcref) (do (ref.func $foo) ) @@ -533,7 +533,7 @@ ) ) (drop - (try (result anyref) + (try $try36 (result anyref) (do (local.get $local_externref) ) @@ -546,7 +546,7 @@ ) ) (drop - (try (result anyref) + (try $try37 (result anyref) (do (ref.func $foo) ) diff --git a/test/reference-types.wast.fromBinary b/test/reference-types.wast.fromBinary index 5e51309eca1..02be5171e1d 100644 --- a/test/reference-types.wast.fromBinary +++ b/test/reference-types.wast.fromBinary @@ -507,7 +507,7 @@ ) ) (drop - (try (result externref) + (try $label$47 (result externref) (do (local.get $local_funcref) ) @@ -520,7 +520,7 @@ ) ) (drop - (try (result funcref) + (try $label$50 (result funcref) (do (ref.func $foo) ) @@ -533,7 +533,7 @@ ) ) (drop - (try (result anyref) + (try $label$53 (result anyref) (do (local.get $local_funcref) ) @@ -546,7 +546,7 @@ ) ) (drop - (try (result anyref) + (try $label$56 (result anyref) (do (ref.func $foo) ) diff --git a/test/reference-types.wast.fromBinary.noDebugInfo b/test/reference-types.wast.fromBinary.noDebugInfo index 23c2bb03b55..9613c300b01 100644 --- a/test/reference-types.wast.fromBinary.noDebugInfo +++ b/test/reference-types.wast.fromBinary.noDebugInfo @@ -507,7 +507,7 @@ ) ) (drop - (try (result externref) + (try $label$47 (result externref) (do (local.get $1) ) @@ -520,7 +520,7 @@ ) ) (drop - (try (result funcref) + (try $label$50 (result funcref) (do (ref.func $3) ) @@ -533,7 +533,7 @@ ) ) (drop - (try (result anyref) + (try $label$53 (result anyref) (do (local.get $1) ) @@ -546,7 +546,7 @@ ) ) (drop - (try (result anyref) + (try $label$56 (result anyref) (do (ref.func $3) ) diff --git a/test/spec/exception-handling.wast b/test/spec/exception-handling.wast index ebca3a009b4..807ee0a5254 100644 --- a/test/spec/exception-handling.wast +++ b/test/spec/exception-handling.wast @@ -241,7 +241,7 @@ ) ) ) - "try's type does not match try body's type" + "try's type does not match try body's type" ) (assert_invalid @@ -263,3 +263,39 @@ ) "event's param numbers must match" ) + +(assert_invalid + (module + (func $f0 + (block $l0 + (try + (do + (try + (do) + (delegate $l0) ;; target is a block + ) + ) + (catch_all) + ) + ) + ) + ) + "all delegate targets must be valid" +) + +(assert_invalid + (module + (func $f0 + (try $l0 + (do) + (catch_all + (try + (do) + (delegate $l0) ;; the target catch is above the delegate + ) + ) + ) + ) + ) + "all delegate targets must be valid" +) diff --git a/test/try-delegate.wasm b/test/try-delegate.wasm new file mode 100644 index 0000000000000000000000000000000000000000..6578b41861c694bbd210fa84086b926b0b6f6fee GIT binary patch literal 42 ucmZQbEY4+QU|?WmVN76PU}j=u;AIB#xda*c8Q2`y93&XnfwTkzH#Y!HqXP;6 literal 0 HcmV?d00001 diff --git a/test/try-delegate.wasm.fromBinary b/test/try-delegate.wasm.fromBinary new file mode 100644 index 00000000000..4decb341c5b --- /dev/null +++ b/test/try-delegate.wasm.fromBinary @@ -0,0 +1,23 @@ +(module + (type $none_=>_none (func)) + (event $event$0 (attr 0) (param)) + (func $0 + (try $label$9 + (do + (try $label$4 + (do + ) + (delegate $label$9) + ) + ) + (catch $event$0 + (try $label$8 + (do + ) + (delegate $label$9) + ) + ) + ) + ) +) + From bd43253786fa73b222f52b4727dd8fee6aafe165 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Wed, 10 Feb 2021 09:30:13 -0800 Subject: [PATCH 2/5] clang-format --- src/passes/RemoveUnusedNames.cpp | 4 +--- src/wasm/wasm-binary.cpp | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/passes/RemoveUnusedNames.cpp b/src/passes/RemoveUnusedNames.cpp index be7ae8e3c23..53da00733d0 100644 --- a/src/passes/RemoveUnusedNames.cpp +++ b/src/passes/RemoveUnusedNames.cpp @@ -83,9 +83,7 @@ struct RemoveUnusedNames } } - void visitTry(Try* curr) { - handleBreakTarget(curr->name); - } + void visitTry(Try* curr) { handleBreakTarget(curr->name); } void visitFunction(Function* curr) { assert(branchesSeen.empty()); } }; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index d8031b858d3..878293caaff 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -5841,7 +5841,7 @@ void WasmBinaryBuilder::visitTryOrTryInBlock(Expression*& out) { // and catch body, if any delegate targets those inner blocks, we should make // them target the try's label instead. curr->name = getNextLabel(); - if (auto *block = curr->body->dynCast()) { + if (auto* block = curr->body->dynCast()) { if (block->name.is()) { if (delegateTargetNames.find(block->name) != delegateTargetNames.end()) { BranchUtils::replaceDelegateTargets(block, block->name, curr->name); From 9353426e33570cbb4ffbdc63c8f30d23ebbff21f Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Wed, 10 Feb 2021 14:04:41 -0800 Subject: [PATCH 3/5] Fix try-delegate.wasm to validate --- test/try-delegate.wasm | Bin 42 -> 59 bytes test/try-delegate.wasm.fromBinary | 25 +++++++++++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/test/try-delegate.wasm b/test/try-delegate.wasm index 6578b41861c694bbd210fa84086b926b0b6f6fee..dd39a4f3a2ddc03b22fe5caf0a11c1aea2267e7e 100644 GIT binary patch literal 59 zcmZQbEY4+QU|?WmVN76PU}k1wVBlp23UDbhaWk+vusKLDurqLT3xnD042}#^Kp{qM GkQe}r&;&yO literal 42 ucmZQbEY4+QU|?WmVN76PU}j=u;AIB#xda*c8Q2`y93&XnfwTkzH#Y!HqXP;6 diff --git a/test/try-delegate.wasm.fromBinary b/test/try-delegate.wasm.fromBinary index 4decb341c5b..fabbfebd009 100644 --- a/test/try-delegate.wasm.fromBinary +++ b/test/try-delegate.wasm.fromBinary @@ -2,21 +2,38 @@ (type $none_=>_none (func)) (event $event$0 (attr 0) (param)) (func $0 - (try $label$9 + (try $label$6 (do (try $label$4 (do ) - (delegate $label$9) + (delegate $label$6) ) ) (catch $event$0 - (try $label$8 + ) + ) + ) + (func $1 + (try $label$9 + (do + (try $label$7 (do ) - (delegate $label$9) + (catch $event$0 + (drop + (i32.const 0) + ) + (try $label$6 + (do + ) + (delegate $label$9) + ) + ) ) ) + (catch $event$0 + ) ) ) ) From 2b0d9bae5d93764078b4f4eac233ccfedecbeb28 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Thu, 11 Feb 2021 07:12:16 +0900 Subject: [PATCH 4/5] Update src/wasm/wasm-s-parser.cpp Co-authored-by: Alon Zakai --- src/wasm/wasm-s-parser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index 996862060bc..dd6efb283ff 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -1997,7 +1997,7 @@ Expression* SExpressionWasmBuilder::makeRefEq(Element& s) { // block can exist, and if it does, it should be at the end. There should be at // least one catch or catch_all body per try. // -// try-delegate is written in the forded format as +// try-delegate is written in the folded format as // (try // (do // ... From df792143b232ec9e50e48c5bcfb04963f6e76932 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Wed, 10 Feb 2021 14:16:14 -0800 Subject: [PATCH 5/5] Address comments --- src/passes/Print.cpp | 1 + src/wasm-s-parser.h | 3 ++- src/wasm/wasm-s-parser.cpp | 6 +++--- src/wasm/wasm-validator.cpp | 4 ---- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp index 9d8dadda514..9b59de4441e 100644 --- a/src/passes/Print.cpp +++ b/src/passes/Print.cpp @@ -1959,6 +1959,7 @@ struct PrintSExpression : public OverriddenVisitor { Function::DebugLocation lastPrintedLocation; bool debugInfo; + // Used to print delegate's depth argument when it throws to the caller int controlFlowDepth = 0; PrintSExpression(std::ostream& o) : o(o) { diff --git a/src/wasm-s-parser.h b/src/wasm-s-parser.h index 838b90b0229..92bc55d0add 100644 --- a/src/wasm-s-parser.h +++ b/src/wasm-s-parser.h @@ -242,7 +242,8 @@ class SExpressionWasmBuilder { i++; } } - Name getLabel(Element& s, bool isBranch = true); + enum class LabelType { Break, Delegate }; + Name getLabel(Element& s, LabelType labelType = LabelType::Break); Expression* makeBreak(Element& s); Expression* makeBreakTable(Element& s); Expression* makeReturn(Element& s); diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index dd6efb283ff..d2b24fbedef 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -1859,7 +1859,7 @@ Expression* SExpressionWasmBuilder::makeCallIndirect(Element& s, return ret; } -Name SExpressionWasmBuilder::getLabel(Element& s, bool isBranch) { +Name SExpressionWasmBuilder::getLabel(Element& s, LabelType labelType) { if (s.dollared()) { return nameMapper.sourceToUnique(s.str()); } else { @@ -1876,7 +1876,7 @@ Name SExpressionWasmBuilder::getLabel(Element& s, bool isBranch) { throw ParseException("invalid label", s.line, s.col); } if (offset == nameMapper.labelStack.size()) { - if (isBranch) { + if (labelType == LabelType::Break) { // a break to the function's scope. this means we need an automatic // block, with a name brokeToAutoBlock = true; @@ -2048,7 +2048,7 @@ Expression* SExpressionWasmBuilder::makeTry(Element& s) { if (inner.size() != 2) { throw ParseException("invalid delegate", inner.line, inner.col); } - ret->delegateTarget = getLabel(*inner[1], false); + ret->delegateTarget = getLabel(*inner[1], LabelType::Delegate); } if (i != s.size()) { diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index fff08ba0331..9ecd9d9b36e 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -2119,10 +2119,6 @@ void FunctionValidator::visitTry(Try* curr) { curr, "try should have either catches or a delegate"); - // (try $l (delegate $l)) is invalid. So we pop the current try's name before - // checking the delegate target. - delegateTargetNames.erase(curr->name); - if (curr->isDelegate()) { noteDelegate(curr->delegateTarget, curr); }