Skip to content

Commit

Permalink
[EH] Make rethrow's target a try label
Browse files Browse the repository at this point in the history
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 with `delegate`'s label: the immediate argument is comptuted in the
same way as 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 reverses some of `delegateTarget` -> `exceptionTarget` changes
done in WebAssembly#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.
  • Loading branch information
aheejin committed Feb 14, 2021
1 parent c12cc3f commit 513c04f
Show file tree
Hide file tree
Showing 31 changed files with 626 additions and 214 deletions.
13 changes: 7 additions & 6 deletions src/binaryen-c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1248,8 +1248,9 @@ BinaryenExpressionRef BinaryenThrow(BinaryenModuleRef module,
}

BinaryenExpressionRef BinaryenRethrow(BinaryenModuleRef module,
BinaryenIndex depth) {
return static_cast<Expression*>(Builder(*(Module*)module).makeRethrow(depth));
const char* target) {
return static_cast<Expression*>(
Builder(*(Module*)module).makeRethrow(target));
}

BinaryenExpressionRef BinaryenI31New(BinaryenModuleRef module,
Expand Down Expand Up @@ -2965,15 +2966,15 @@ BinaryenExpressionRef BinaryenThrowRemoveOperandAt(BinaryenExpressionRef expr,
return static_cast<Throw*>(expression)->operands.removeAt(index);
}
// Rethrow
BinaryenIndex BinaryenRethrowGetDepth(BinaryenExpressionRef expr) {
const char* BinaryenRethrowGetTarget(BinaryenExpressionRef expr) {
auto* expression = (Expression*)expr;
assert(expression->is<Rethrow>());
return static_cast<Rethrow*>(expression)->depth;
return static_cast<Rethrow*>(expression)->target.c_str();
}
void BinaryenRethrowSetDepth(BinaryenExpressionRef expr, BinaryenIndex depth) {
void BinaryenRethrowSetTarget(BinaryenExpressionRef expr, const char* target) {
auto* expression = (Expression*)expr;
assert(expression->is<Rethrow>());
static_cast<Rethrow*>(expression)->depth = depth;
static_cast<Rethrow*>(expression)->target = target;
}
// TupleMake
BinaryenIndex BinaryenTupleMakeGetNumOperands(BinaryenExpressionRef expr) {
Expand Down
12 changes: 6 additions & 6 deletions src/binaryen-c.h
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,7 @@ BinaryenThrow(BinaryenModuleRef module,
BinaryenExpressionRef* operands,
BinaryenIndex numOperands);
BINARYEN_API BinaryenExpressionRef BinaryenRethrow(BinaryenModuleRef module,
BinaryenIndex depth);
const char* target);
BINARYEN_API BinaryenExpressionRef
BinaryenTupleMake(BinaryenModuleRef module,
BinaryenExpressionRef* operands,
Expand Down Expand Up @@ -1829,11 +1829,11 @@ BinaryenThrowRemoveOperandAt(BinaryenExpressionRef expr, BinaryenIndex index);

// Rethrow

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

// TupleMake

Expand Down
6 changes: 3 additions & 3 deletions src/ir/branch-utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ void operateOnScopeNameUsesAndSentTypes(Expression* expr, T func) {
} else if (auto* br = expr->dynCast<BrOn>()) {
func(name, br->getCastType());
} else {
assert(expr->is<Try>()); // delegate
assert(expr->is<Try>() || expr->is<Rethrow>()); // delegate or rethrow
}
});
}
Expand Down Expand Up @@ -135,14 +135,14 @@ inline bool replacePossibleTarget(Expression* branch, Name from, Name to) {
return worked;
}

// Replace all delegate targets within the given AST.
// Replace all delegate/rethrow targets within the given AST.
inline void replaceExceptionTargets(Expression* ast, Name from, Name to) {
struct Replacer
: public PostWalker<Replacer, UnifiedExpressionVisitor<Replacer>> {
Name from, to;
Replacer(Name from, Name to) : from(from), to(to) {}
void visitExpression(Expression* curr) {
if (curr->is<Try>()) {
if (curr->is<Try>() || curr->is<Rethrow>()) {
operateOnScopeNameUses(curr, [&](Name& name) {
if (name == from) {
name = to;
Expand Down
15 changes: 8 additions & 7 deletions src/js/binaryen.js-post.js
Original file line number Diff line number Diff line change
Expand Up @@ -2154,8 +2154,8 @@ function wrapModule(module, self = {}) {
self['throw'] = function(event_, operands) {
return preserveStack(() => Module['_BinaryenThrow'](module, strToStack(event_), i32sToStack(operands), operands.length));
};
self['rethrow'] = function(depth) {
return Module['_BinaryenRethrow'](module, depth);
self['rethrow'] = function(target) {
return Module['_BinaryenRethrow'](module, strToStack(target));
};

self['tuple'] = {
Expand Down Expand Up @@ -2916,7 +2916,7 @@ Module['getExpressionInfo'] = function(expr) {
return {
'id': id,
'type': type,
'depth': Module['_BinaryenRethrowGetDepth'](expr)
'target': UTF8ToString(Module['_BinaryenRethrowGetTarget'](expr))
};
case Module['TupleMakeId']:
return {
Expand Down Expand Up @@ -4287,11 +4287,12 @@ Module['Throw'] = makeExpressionWrapper({
});

Module['Rethrow'] = makeExpressionWrapper({
'getDepth'(expr) {
return Module['_BinaryenRethrowGetDepth'](expr);
'getTarget'(expr) {
const target = Module['_BinaryenRethrowGetTarget'](expr);
return target ? UTF8ToString(target) : null;
},
'setDepth'(expr, depthExpr) {
Module['_BinaryenRethrowSetDepth'](expr, depthExpr);
'setTarget'(expr, target) {
preserveStack(() => { Module['_BinaryenRethrowSetTarget'](expr, strToStack(target)) });
}
});

Expand Down
2 changes: 1 addition & 1 deletion src/passes/Print.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1778,7 +1778,7 @@ struct PrintExpressionContents
}
void visitRethrow(Rethrow* curr) {
printMedium(o, "rethrow ");
o << curr->depth;
printName(curr->target, o);
}
void visitNop(Nop* curr) { printMinor(o, "nop"); }
void visitUnreachable(Unreachable* curr) { printMinor(o, "unreachable"); }
Expand Down
4 changes: 2 additions & 2 deletions src/wasm-builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -708,9 +708,9 @@ class Builder {
ret->finalize();
return ret;
}
Rethrow* makeRethrow(Index depth) {
Rethrow* makeRethrow(Name target) {
auto* ret = wasm.allocator.alloc<Rethrow>();
ret->depth = depth;
ret->target = target;
ret->finalize();
return ret;
}
Expand Down
2 changes: 1 addition & 1 deletion src/wasm-delegations-fields.h
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ switch (DELEGATE_ID) {
}
case Expression::Id::RethrowId: {
DELEGATE_START(Rethrow);
DELEGATE_FIELD_INT(Rethrow, depth);
DELEGATE_FIELD_SCOPE_NAME_USE(Rethrow, target);
DELEGATE_END(Rethrow);
break;
}
Expand Down
12 changes: 8 additions & 4 deletions src/wasm-interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -2400,7 +2400,8 @@ template<typename GlobalManager, typename SubType> class ModuleInstanceBase {
: public ExpressionRunner<RuntimeExpressionRunner> {
ModuleInstanceBase& instance;
FunctionScope& scope;
SmallVector<WasmException, 4> exceptionStack;
// Stack of <caught exception, caught catch's try label>
SmallVector<std::pair<WasmException, Name>, 4> exceptionStack;

public:
RuntimeExpressionRunner(ModuleInstanceBase& instance,
Expand Down Expand Up @@ -3046,7 +3047,7 @@ template<typename GlobalManager, typename SubType> class ModuleInstanceBase {
auto processCatchBody = [&](Expression* catchBody) {
// Push the current exception onto the exceptionStack in case
// 'rethrow's use it
exceptionStack.push_back(e);
exceptionStack.push_back(std::make_pair(e, curr->name));
// We need to pop exceptionStack in either case: when the catch body
// exits normally or when a new exception is thrown
Flow ret;
Expand Down Expand Up @@ -3074,8 +3075,11 @@ template<typename GlobalManager, typename SubType> class ModuleInstanceBase {
}
}
Flow visitRethrow(Rethrow* curr) {
assert(exceptionStack.size() > curr->depth);
throwException(exceptionStack[exceptionStack.size() - 1 - curr->depth]);
for (int i = exceptionStack.size() - 1; i >= 0; i--) {
if (exceptionStack[i].second == curr->target) {
throwException(exceptionStack[i].first);
}
}
WASM_UNREACHABLE("rethrow");
}
Flow visitPop(Pop* curr) {
Expand Down
2 changes: 1 addition & 1 deletion src/wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -1335,7 +1335,7 @@ class Rethrow : public SpecificExpression<Expression::RethrowId> {
public:
Rethrow(MixedArena& allocator) {}

Index depth;
Name target;

void finalize();
};
Expand Down
22 changes: 12 additions & 10 deletions src/wasm/wasm-binary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3475,12 +3475,12 @@ Name WasmBinaryBuilder::getExceptionTargetName(int32_t offset) {
}
size_t index = breakStack.size() - 1 - offset;
if (index > breakStack.size()) {
throwError("bad delegate index (high)");
throwError("bad try index (high)");
}
BYN_TRACE("delegate target " << breakStack[index].name << std::endl);
BYN_TRACE("exception 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 the delegate/rethrow is in literally unreachable code, then we will not
// emit it anyhow, so do not note that the target has a reference to it
if (!willBeIgnored) {
exceptionTargetNames.insert(ret.name);
}
Expand Down Expand Up @@ -5835,11 +5835,11 @@ void WasmBinaryBuilder::visitTryOrTryInBlock(Expression*& out) {
curr->delegateTarget = getExceptionTargetName(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.
// For simplicity, we make try's labels only can be targeted by delegates and
// rethrows, and delegates/rethrows 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/rethrow targets
// those inner blocks, we should make them target the try's label instead.
curr->name = getNextLabel();
if (auto* block = curr->body->dynCast<Block>()) {
if (block->name.is()) {
Expand Down Expand Up @@ -5936,7 +5936,9 @@ void WasmBinaryBuilder::visitThrow(Throw* curr) {

void WasmBinaryBuilder::visitRethrow(Rethrow* curr) {
BYN_TRACE("zz node: Rethrow\n");
curr->depth = getU32LEB();
curr->target = getExceptionTargetName(getU32LEB());
// This special target is valid only for delegates
assert(curr->target != DELEGATE_CALLER_TARGET);
curr->finalize();
}

Expand Down
29 changes: 3 additions & 26 deletions src/wasm/wasm-s-parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2067,8 +2067,8 @@ Expression* SExpressionWasmBuilder::makeTry(Element& s) {
// 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.
// For simplicity, try's name can only be targeted by delegates and
// rethrows. 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);
Expand All @@ -2078,29 +2078,6 @@ Expression* SExpressionWasmBuilder::makeTry(Element& s) {
return ret;
}

Expression*
SExpressionWasmBuilder::makeTryOrCatchBody(Element& s, Type type, bool isTry) {
if (isTry && !elementStartsWith(s, "do")) {
throw ParseException("invalid try do clause", s.line, s.col);
}
if (!isTry && !elementStartsWith(s, "catch") &&
!elementStartsWith(s, "catch_all")) {
throw ParseException("invalid catch clause", s.line, s.col);
}
if (s.size() == 1) { // (do) / (catch) / (catch_all) without instructions
return makeNop();
}
auto ret = allocator.alloc<Block>();
for (size_t i = 1; i < s.size(); i++) {
ret->list.push_back(parseExpression(s[i]));
}
if (ret->list.size() == 1) {
return ret->list[0];
}
ret->finalize(type);
return ret;
}

Expression* SExpressionWasmBuilder::makeThrow(Element& s) {
auto ret = allocator.alloc<Throw>();
Index i = 1;
Expand All @@ -2118,7 +2095,7 @@ Expression* SExpressionWasmBuilder::makeThrow(Element& s) {

Expression* SExpressionWasmBuilder::makeRethrow(Element& s) {
auto ret = allocator.alloc<Rethrow>();
ret->depth = atoi(s[1]->str().c_str());
ret->target = getLabel(*s[1], LabelType::Exception);
ret->finalize();
return ret;
}
Expand Down
2 changes: 1 addition & 1 deletion src/wasm/wasm-stack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1936,7 +1936,7 @@ void BinaryInstWriter::visitThrow(Throw* curr) {
}

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

void BinaryInstWriter::visitNop(Nop* curr) { o << int8_t(BinaryConsts::Nop); }
Expand Down
33 changes: 23 additions & 10 deletions src/wasm/wasm-validator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,8 @@ struct FunctionValidator : public WalkerPass<PostWalker<FunctionValidator>> {
};

std::unordered_map<Name, BreakInfo> breakInfos;
std::unordered_set<Name> exceptionTargetNames;
std::unordered_set<Name> delegateTargetNames;
std::unordered_set<Name> rethrowTargetNames;

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

Expand Down Expand Up @@ -280,7 +281,7 @@ struct FunctionValidator : public WalkerPass<PostWalker<FunctionValidator>> {
static void visitPreTry(FunctionValidator* self, Expression** currp) {
auto* curr = (*currp)->cast<Try>();
if (curr->name.is()) {
self->exceptionTargetNames.insert(curr->name);
self->delegateTargetNames.insert(curr->name);
}
}

Expand All @@ -300,7 +301,8 @@ struct FunctionValidator : public WalkerPass<PostWalker<FunctionValidator>> {
static void visitPreCatch(FunctionValidator* self, Expression** currp) {
auto* curr = (*currp)->cast<Try>();
if (curr->name.is()) {
self->exceptionTargetNames.erase(curr->name);
self->delegateTargetNames.erase(curr->name);
self->rethrowTargetNames.insert(curr->name);
}
}

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

void FunctionValidator::noteException(Name name, Expression* curr) {
void FunctionValidator::noteDelegate(Name name, Expression* curr) {
if (name != DELEGATE_CALLER_TARGET) {
shouldBeTrue(exceptionTargetNames.find(name) != exceptionTargetNames.end(),
shouldBeTrue(delegateTargetNames.find(name) != delegateTargetNames.end(),
curr,
"all delegate targets must be valid");
}
}

void FunctionValidator::noteRethrow(Name name, Expression* curr) {
shouldBeTrue(rethrowTargetNames.find(name) != rethrowTargetNames.end(),
curr,
"all rethrow targets must be valid");
}

void FunctionValidator::visitTry(Try* curr) {
shouldBeTrue(getModule()->features.hasExceptionHandling(),
curr,
Expand Down Expand Up @@ -2122,8 +2131,10 @@ void FunctionValidator::visitTry(Try* curr) {
"try should have either catches or a delegate");

if (curr->isDelegate()) {
noteException(curr->delegateTarget, curr);
noteDelegate(curr->delegateTarget, curr);
}

rethrowTargetNames.erase(curr->name);
}

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

void FunctionValidator::visitTupleMake(TupleMake* curr) {
Expand Down Expand Up @@ -2533,9 +2543,12 @@ void FunctionValidator::visitFunction(Function* curr) {

shouldBeTrue(
breakInfos.empty(), curr->body, "all named break targets must exist");
shouldBeTrue(exceptionTargetNames.empty(),
shouldBeTrue(delegateTargetNames.empty(),
curr->body,
"all named delegate targets must exist");
shouldBeTrue(rethrowTargetNames.empty(),
curr->body,
"all named rethrow targets must exist");
returnTypes.clear();
labelNames.clear();
// validate optional local names
Expand Down
Loading

0 comments on commit 513c04f

Please sign in to comment.