Skip to content

Commit 1254b56

Browse files
aheejinaheejin
authored
[EH] Make rethrow's target a try label (#3568)
I was previously mistaken about `rethrow`'s argument rule and thought it only counted `catch`'s depth. But it turns out it follows the same rule `delegate`'s label: the immediate argument follows the same rule as when computing branch labels, but it only can target `try` labels (semantically it targets that `try`'s corresponding `catch`); otherwise it will be a validation failure. Unlike `delegate`, `rethrow`'s label denotes not where to rethrow, but which exception to rethrow. For example, ```wasm try $l0 catch ($l0) try $l1 catch ($l1) rethrow $l0 ;; rethrow the exception caught by 'catch ($l0)' end end ``` Refer to this comment for the more detailed informal semantics: WebAssembly/exception-handling#146 (comment) --- This also reverts some of `delegateTarget` -> `exceptionTarget` changes done in #3562 in the validator. Label validation rules apply differently for `delegate` and `rethrow` for try-catch. For example, this is valid: ```wasm try $l0 try delegate $l0 catch ($l0) end ``` But this is NOT valid: ```wasm try $l0 catch ($l0) try delegate $l0 end ``` So `try`'s label should be used within try-catch range (not catch-end range) for `delegate`s. But for the `rethrow` the rule is different. For example, this is valid: ```wasm try $l0 catch ($l0) rethrow $l0 end ``` But this is NOT valid: ```wasm try $l0 rethrow $l0 catch ($l0) end ``` So the `try`'s label should be used within catch-end range instead.
1 parent 1d3f578 commit 1254b56

31 files changed

+626
-218
lines changed

‎src/binaryen-c.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,8 +1248,9 @@ BinaryenExpressionRef BinaryenThrow(BinaryenModuleRef module,
12481248
}
12491249

12501250
BinaryenExpressionRef BinaryenRethrow(BinaryenModuleRef module,
1251-
BinaryenIndex depth) {
1252-
return static_cast<Expression*>(Builder(*(Module*)module).makeRethrow(depth));
1251+
const char* target) {
1252+
return static_cast<Expression*>(
1253+
Builder(*(Module*)module).makeRethrow(target));
12531254
}
12541255

12551256
BinaryenExpressionRef BinaryenI31New(BinaryenModuleRef module,
@@ -2965,15 +2966,15 @@ BinaryenExpressionRef BinaryenThrowRemoveOperandAt(BinaryenExpressionRef expr,
29652966
return static_cast<Throw*>(expression)->operands.removeAt(index);
29662967
}
29672968
// Rethrow
2968-
BinaryenIndex BinaryenRethrowGetDepth(BinaryenExpressionRef expr) {
2969+
const char* BinaryenRethrowGetTarget(BinaryenExpressionRef expr) {
29692970
auto* expression = (Expression*)expr;
29702971
assert(expression->is<Rethrow>());
2971-
return static_cast<Rethrow*>(expression)->depth;
2972+
return static_cast<Rethrow*>(expression)->target.c_str();
29722973
}
2973-
void BinaryenRethrowSetDepth(BinaryenExpressionRef expr, BinaryenIndex depth) {
2974+
void BinaryenRethrowSetTarget(BinaryenExpressionRef expr, const char* target) {
29742975
auto* expression = (Expression*)expr;
29752976
assert(expression->is<Rethrow>());
2976-
static_cast<Rethrow*>(expression)->depth = depth;
2977+
static_cast<Rethrow*>(expression)->target = target;
29772978
}
29782979
// TupleMake
29792980
BinaryenIndex BinaryenTupleMakeGetNumOperands(BinaryenExpressionRef expr) {

‎src/binaryen-c.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -818,7 +818,7 @@ BinaryenThrow(BinaryenModuleRef module,
818818
BinaryenExpressionRef* operands,
819819
BinaryenIndex numOperands);
820820
BINARYEN_API BinaryenExpressionRef BinaryenRethrow(BinaryenModuleRef module,
821-
BinaryenIndex depth);
821+
const char* target);
822822
BINARYEN_API BinaryenExpressionRef
823823
BinaryenTupleMake(BinaryenModuleRef module,
824824
BinaryenExpressionRef* operands,
@@ -1829,11 +1829,11 @@ BinaryenThrowRemoveOperandAt(BinaryenExpressionRef expr, BinaryenIndex index);
18291829

18301830
// Rethrow
18311831

1832-
// Gets the depth of a `rethrow` expression.
1833-
BINARYEN_API BinaryenIndex BinaryenRethrowGetDepth(BinaryenExpressionRef expr);
1834-
// Sets the exception reference expression of a `rethrow` expression.
1835-
BINARYEN_API void BinaryenRethrowSetDepth(BinaryenExpressionRef expr,
1836-
BinaryenIndex depth);
1832+
// Gets the target catch's corresponding try label of a `rethrow` expression.
1833+
BINARYEN_API const char* BinaryenRethrowGetTarget(BinaryenExpressionRef expr);
1834+
// Sets the target catch's corresponding try label of a `rethrow` expression.
1835+
BINARYEN_API void BinaryenRethrowSetTarget(BinaryenExpressionRef expr,
1836+
const char* target);
18371837

18381838
// TupleMake
18391839

‎src/ir/branch-utils.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ void operateOnScopeNameUsesAndSentTypes(Expression* expr, T func) {
8383
} else if (auto* br = expr->dynCast<BrOn>()) {
8484
func(name, br->getCastType());
8585
} else {
86-
assert(expr->is<Try>()); // delegate
86+
assert(expr->is<Try>() || expr->is<Rethrow>()); // delegate or rethrow
8787
}
8888
});
8989
}
@@ -135,14 +135,14 @@ inline bool replacePossibleTarget(Expression* branch, Name from, Name to) {
135135
return worked;
136136
}
137137

138-
// Replace all delegate targets within the given AST.
138+
// Replace all delegate/rethrow targets within the given AST.
139139
inline void replaceExceptionTargets(Expression* ast, Name from, Name to) {
140140
struct Replacer
141141
: public PostWalker<Replacer, UnifiedExpressionVisitor<Replacer>> {
142142
Name from, to;
143143
Replacer(Name from, Name to) : from(from), to(to) {}
144144
void visitExpression(Expression* curr) {
145-
if (curr->is<Try>()) {
145+
if (curr->is<Try>() || curr->is<Rethrow>()) {
146146
operateOnScopeNameUses(curr, [&](Name& name) {
147147
if (name == from) {
148148
name = to;

‎src/js/binaryen.js-post.js

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2154,8 +2154,8 @@ function wrapModule(module, self = {}) {
21542154
self['throw'] = function(event_, operands) {
21552155
return preserveStack(() => Module['_BinaryenThrow'](module, strToStack(event_), i32sToStack(operands), operands.length));
21562156
};
2157-
self['rethrow'] = function(depth) {
2158-
return Module['_BinaryenRethrow'](module, depth);
2157+
self['rethrow'] = function(target) {
2158+
return Module['_BinaryenRethrow'](module, strToStack(target));
21592159
};
21602160

21612161
self['tuple'] = {
@@ -2916,7 +2916,7 @@ Module['getExpressionInfo'] = function(expr) {
29162916
return {
29172917
'id': id,
29182918
'type': type,
2919-
'depth': Module['_BinaryenRethrowGetDepth'](expr)
2919+
'target': UTF8ToString(Module['_BinaryenRethrowGetTarget'](expr))
29202920
};
29212921
case Module['TupleMakeId']:
29222922
return {
@@ -4287,11 +4287,12 @@ Module['Throw'] = makeExpressionWrapper({
42874287
});
42884288

42894289
Module['Rethrow'] = makeExpressionWrapper({
4290-
'getDepth'(expr) {
4291-
return Module['_BinaryenRethrowGetDepth'](expr);
4290+
'getTarget'(expr) {
4291+
const target = Module['_BinaryenRethrowGetTarget'](expr);
4292+
return target ? UTF8ToString(target) : null;
42924293
},
4293-
'setDepth'(expr, depthExpr) {
4294-
Module['_BinaryenRethrowSetDepth'](expr, depthExpr);
4294+
'setTarget'(expr, target) {
4295+
preserveStack(() => { Module['_BinaryenRethrowSetTarget'](expr, strToStack(target)) });
42954296
}
42964297
});
42974298

‎src/passes/Print.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1778,7 +1778,7 @@ struct PrintExpressionContents
17781778
}
17791779
void visitRethrow(Rethrow* curr) {
17801780
printMedium(o, "rethrow ");
1781-
o << curr->depth;
1781+
printName(curr->target, o);
17821782
}
17831783
void visitNop(Nop* curr) { printMinor(o, "nop"); }
17841784
void visitUnreachable(Unreachable* curr) { printMinor(o, "unreachable"); }

‎src/wasm-builder.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -708,9 +708,9 @@ class Builder {
708708
ret->finalize();
709709
return ret;
710710
}
711-
Rethrow* makeRethrow(Index depth) {
711+
Rethrow* makeRethrow(Name target) {
712712
auto* ret = wasm.allocator.alloc<Rethrow>();
713-
ret->depth = depth;
713+
ret->target = target;
714714
ret->finalize();
715715
return ret;
716716
}

‎src/wasm-delegations-fields.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,7 @@ switch (DELEGATE_ID) {
532532
}
533533
case Expression::Id::RethrowId: {
534534
DELEGATE_START(Rethrow);
535-
DELEGATE_FIELD_INT(Rethrow, depth);
535+
DELEGATE_FIELD_SCOPE_NAME_USE(Rethrow, target);
536536
DELEGATE_END(Rethrow);
537537
break;
538538
}

‎src/wasm-interpreter.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2402,7 +2402,8 @@ template<typename GlobalManager, typename SubType> class ModuleInstanceBase {
24022402
: public ExpressionRunner<RuntimeExpressionRunner> {
24032403
ModuleInstanceBase& instance;
24042404
FunctionScope& scope;
2405-
SmallVector<WasmException, 4> exceptionStack;
2405+
// Stack of <caught exception, caught catch's try label>
2406+
SmallVector<std::pair<WasmException, Name>, 4> exceptionStack;
24062407

24072408
public:
24082409
RuntimeExpressionRunner(ModuleInstanceBase& instance,
@@ -3048,7 +3049,7 @@ template<typename GlobalManager, typename SubType> class ModuleInstanceBase {
30483049
auto processCatchBody = [&](Expression* catchBody) {
30493050
// Push the current exception onto the exceptionStack in case
30503051
// 'rethrow's use it
3051-
exceptionStack.push_back(e);
3052+
exceptionStack.push_back(std::make_pair(e, curr->name));
30523053
// We need to pop exceptionStack in either case: when the catch body
30533054
// exits normally or when a new exception is thrown
30543055
Flow ret;
@@ -3076,8 +3077,11 @@ template<typename GlobalManager, typename SubType> class ModuleInstanceBase {
30763077
}
30773078
}
30783079
Flow visitRethrow(Rethrow* curr) {
3079-
assert(exceptionStack.size() > curr->depth);
3080-
throwException(exceptionStack[exceptionStack.size() - 1 - curr->depth]);
3080+
for (int i = exceptionStack.size() - 1; i >= 0; i--) {
3081+
if (exceptionStack[i].second == curr->target) {
3082+
throwException(exceptionStack[i].first);
3083+
}
3084+
}
30813085
WASM_UNREACHABLE("rethrow");
30823086
}
30833087
Flow visitPop(Pop* curr) {

‎src/wasm.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1335,7 +1335,7 @@ class Rethrow : public SpecificExpression<Expression::RethrowId> {
13351335
public:
13361336
Rethrow(MixedArena& allocator) {}
13371337

1338-
Index depth;
1338+
Name target;
13391339

13401340
void finalize();
13411341
};

‎src/wasm/wasm-binary.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3475,12 +3475,12 @@ Name WasmBinaryBuilder::getExceptionTargetName(int32_t offset) {
34753475
}
34763476
size_t index = breakStack.size() - 1 - offset;
34773477
if (index > breakStack.size()) {
3478-
throwError("bad delegate index (high)");
3478+
throwError("bad try index (high)");
34793479
}
3480-
BYN_TRACE("delegate target " << breakStack[index].name << std::endl);
3480+
BYN_TRACE("exception target " << breakStack[index].name << std::endl);
34813481
auto& ret = breakStack[index];
3482-
// if the delegate is in literally unreachable code, then we will not emit it
3483-
// anyhow, so do not note that the target has delegate to it
3482+
// if the delegate/rethrow is in literally unreachable code, then we will not
3483+
// emit it anyhow, so do not note that the target has a reference to it
34843484
if (!willBeIgnored) {
34853485
exceptionTargetNames.insert(ret.name);
34863486
}
@@ -5835,11 +5835,12 @@ void WasmBinaryBuilder::visitTryOrTryInBlock(Expression*& out) {
58355835
curr->delegateTarget = getExceptionTargetName(getU32LEB());
58365836
}
58375837

5838-
// For simplicity, we make try's labels only can be targeted by delegates, and
5839-
// delegates can only target try's labels. (If they target blocks or loops, it
5840-
// is a validation failure.) Because we create an inner block within each try
5841-
// and catch body, if any delegate targets those inner blocks, we should make
5842-
// them target the try's label instead.
5838+
// For simplicity, we ensure that try's labels can only be targeted by
5839+
// delegates and rethrows, and delegates/rethrows can only target try's
5840+
// labels. (If they target blocks or loops, it is a validation failure.)
5841+
// Because we create an inner block within each try and catch body, if any
5842+
// delegate/rethrow targets those inner blocks, we should make them target the
5843+
// try's label instead.
58435844
curr->name = getNextLabel();
58445845
if (auto* block = curr->body->dynCast<Block>()) {
58455846
if (block->name.is()) {
@@ -5936,7 +5937,9 @@ void WasmBinaryBuilder::visitThrow(Throw* curr) {
59365937

59375938
void WasmBinaryBuilder::visitRethrow(Rethrow* curr) {
59385939
BYN_TRACE("zz node: Rethrow\n");
5939-
curr->depth = getU32LEB();
5940+
curr->target = getExceptionTargetName(getU32LEB());
5941+
// This special target is valid only for delegates
5942+
assert(curr->target != DELEGATE_CALLER_TARGET);
59405943
curr->finalize();
59415944
}
59425945

‎src/wasm/wasm-s-parser.cpp

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2067,8 +2067,8 @@ Expression* SExpressionWasmBuilder::makeTry(Element& s) {
20672067
// We create a different name for the wrapping block, because try's name can
20682068
// be used by internal delegates
20692069
block->name = nameMapper.pushLabelName(sName);
2070-
// For simplicity, try's name canonly be targeted by delegates. Make the
2071-
// branches target the new wrapping block instead.
2070+
// For simplicity, try's name can only be targeted by delegates and
2071+
// rethrows. Make the branches target the new wrapping block instead.
20722072
BranchUtils::replaceBranchTargets(ret, ret->name, block->name);
20732073
block->list.push_back(ret);
20742074
nameMapper.popLabelName(block->name);
@@ -2078,29 +2078,6 @@ Expression* SExpressionWasmBuilder::makeTry(Element& s) {
20782078
return ret;
20792079
}
20802080

2081-
Expression*
2082-
SExpressionWasmBuilder::makeTryOrCatchBody(Element& s, Type type, bool isTry) {
2083-
if (isTry && !elementStartsWith(s, "do")) {
2084-
throw ParseException("invalid try do clause", s.line, s.col);
2085-
}
2086-
if (!isTry && !elementStartsWith(s, "catch") &&
2087-
!elementStartsWith(s, "catch_all")) {
2088-
throw ParseException("invalid catch clause", s.line, s.col);
2089-
}
2090-
if (s.size() == 1) { // (do) / (catch) / (catch_all) without instructions
2091-
return makeNop();
2092-
}
2093-
auto ret = allocator.alloc<Block>();
2094-
for (size_t i = 1; i < s.size(); i++) {
2095-
ret->list.push_back(parseExpression(s[i]));
2096-
}
2097-
if (ret->list.size() == 1) {
2098-
return ret->list[0];
2099-
}
2100-
ret->finalize(type);
2101-
return ret;
2102-
}
2103-
21042081
Expression* SExpressionWasmBuilder::makeThrow(Element& s) {
21052082
auto ret = allocator.alloc<Throw>();
21062083
Index i = 1;
@@ -2118,7 +2095,7 @@ Expression* SExpressionWasmBuilder::makeThrow(Element& s) {
21182095

21192096
Expression* SExpressionWasmBuilder::makeRethrow(Element& s) {
21202097
auto ret = allocator.alloc<Rethrow>();
2121-
ret->depth = atoi(s[1]->str().c_str());
2098+
ret->target = getLabel(*s[1], LabelType::Exception);
21222099
ret->finalize();
21232100
return ret;
21242101
}

‎src/wasm/wasm-stack.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1936,7 +1936,7 @@ void BinaryInstWriter::visitThrow(Throw* curr) {
19361936
}
19371937

19381938
void BinaryInstWriter::visitRethrow(Rethrow* curr) {
1939-
o << int8_t(BinaryConsts::Rethrow) << U32LEB(curr->depth);
1939+
o << int8_t(BinaryConsts::Rethrow) << U32LEB(getBreakIndex(curr->target));
19401940
}
19411941

19421942
void BinaryInstWriter::visitNop(Nop* curr) { o << int8_t(BinaryConsts::Nop); }

‎src/wasm/wasm-validator.cpp

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,8 @@ struct FunctionValidator : public WalkerPass<PostWalker<FunctionValidator>> {
236236
};
237237

238238
std::unordered_map<Name, BreakInfo> breakInfos;
239-
std::unordered_set<Name> exceptionTargetNames;
239+
std::unordered_set<Name> delegateTargetNames;
240+
std::unordered_set<Name> rethrowTargetNames;
240241

241242
std::set<Type> returnTypes; // types used in returns
242243

@@ -280,7 +281,7 @@ struct FunctionValidator : public WalkerPass<PostWalker<FunctionValidator>> {
280281
static void visitPreTry(FunctionValidator* self, Expression** currp) {
281282
auto* curr = (*currp)->cast<Try>();
282283
if (curr->name.is()) {
283-
self->exceptionTargetNames.insert(curr->name);
284+
self->delegateTargetNames.insert(curr->name);
284285
}
285286
}
286287

@@ -300,7 +301,8 @@ struct FunctionValidator : public WalkerPass<PostWalker<FunctionValidator>> {
300301
static void visitPreCatch(FunctionValidator* self, Expression** currp) {
301302
auto* curr = (*currp)->cast<Try>();
302303
if (curr->name.is()) {
303-
self->exceptionTargetNames.erase(curr->name);
304+
self->delegateTargetNames.erase(curr->name);
305+
self->rethrowTargetNames.insert(curr->name);
304306
}
305307
}
306308

@@ -376,7 +378,8 @@ struct FunctionValidator : public WalkerPass<PostWalker<FunctionValidator>> {
376378
void visitRefIs(RefIs* curr);
377379
void visitRefFunc(RefFunc* curr);
378380
void visitRefEq(RefEq* curr);
379-
void noteException(Name name, Expression* curr);
381+
void noteDelegate(Name name, Expression* curr);
382+
void noteRethrow(Name name, Expression* curr);
380383
void visitTry(Try* curr);
381384
void visitThrow(Throw* curr);
382385
void visitRethrow(Rethrow* curr);
@@ -2073,14 +2076,20 @@ void FunctionValidator::visitRefEq(RefEq* curr) {
20732076
"ref.eq's right argument should be a subtype of eqref");
20742077
}
20752078

2076-
void FunctionValidator::noteException(Name name, Expression* curr) {
2079+
void FunctionValidator::noteDelegate(Name name, Expression* curr) {
20772080
if (name != DELEGATE_CALLER_TARGET) {
2078-
shouldBeTrue(exceptionTargetNames.find(name) != exceptionTargetNames.end(),
2081+
shouldBeTrue(delegateTargetNames.count(name) != 0,
20792082
curr,
20802083
"all delegate targets must be valid");
20812084
}
20822085
}
20832086

2087+
void FunctionValidator::noteRethrow(Name name, Expression* curr) {
2088+
shouldBeTrue(rethrowTargetNames.count(name) != 0,
2089+
curr,
2090+
"all rethrow targets must be valid");
2091+
}
2092+
20842093
void FunctionValidator::visitTry(Try* curr) {
20852094
shouldBeTrue(getModule()->features.hasExceptionHandling(),
20862095
curr,
@@ -2122,8 +2131,10 @@ void FunctionValidator::visitTry(Try* curr) {
21222131
"try should have either catches or a delegate");
21232132

21242133
if (curr->isDelegate()) {
2125-
noteException(curr->delegateTarget, curr);
2134+
noteDelegate(curr->delegateTarget, curr);
21262135
}
2136+
2137+
rethrowTargetNames.erase(curr->name);
21272138
}
21282139

21292140
void FunctionValidator::visitThrow(Throw* curr) {
@@ -2167,8 +2178,7 @@ void FunctionValidator::visitRethrow(Rethrow* curr) {
21672178
Type(Type::unreachable),
21682179
curr,
21692180
"rethrow's type must be unreachable");
2170-
// TODO Validate the depth field. The current LLVM toolchain only generates
2171-
// depth 0 for C++ support.
2181+
noteRethrow(curr->target, curr);
21722182
}
21732183

21742184
void FunctionValidator::visitTupleMake(TupleMake* curr) {
@@ -2531,11 +2541,9 @@ void FunctionValidator::visitFunction(Function* curr) {
25312541
"function result must match, if function has returns");
25322542
}
25332543

2534-
shouldBeTrue(
2535-
breakInfos.empty(), curr->body, "all named break targets must exist");
2536-
shouldBeTrue(exceptionTargetNames.empty(),
2537-
curr->body,
2538-
"all named delegate targets must exist");
2544+
assert(breakInfos.empty());
2545+
assert(delegateTargetNames.empty());
2546+
assert(rethrowTargetNames.empty());
25392547
returnTypes.clear();
25402548
labelNames.clear();
25412549
// validate optional local names

0 commit comments

Comments
 (0)