Skip to content

Commit

Permalink
[EH] Fix missing outer block for catchless try (WebAssembly#6519)
Browse files Browse the repository at this point in the history 
When translating a `try` expression, we may need an 'outer' block that
wraps the newly generated `try_table` so we can jump out of the
expression when an exception does not occur. (The condition we use is
when the `try` has any catches or if the `try` is a target of any
inner `try-delegate`s:
https://github.com/WebAssembly/binaryen/blob/219e668e87b012c0634043ed702534b8be31231f/src/passes/TranslateEH.cpp#L677)

In case the `try` has either of `catch` or `delegate`, when we have the
'outer' block, we add the newly created `try_table` in the 'outer' block
and replace the whole expression with the block:
https://github.com/WebAssembly/binaryen/blob/219e668e87b012c0634043ed702534b8be31231f/src/passes/TranslateEH.cpp#L670
https://github.com/WebAssembly/binaryen/blob/219e668e87b012c0634043ed702534b8be31231f/src/passes/TranslateEH.cpp#L332-L340

But in case of a catchless `try`, we forgot to do that:
https://github.com/WebAssembly/binaryen/blob/219e668e87b012c0634043ed702534b8be31231f/src/passes/TranslateEH.cpp#L388

So this PR fixes it.
  • Loading branch information
@aheejin
aheejin committed Apr 23, 2024
1 parent 4bbae11 commit 3b9dc42
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 8 deletions.
29 changes: 21 additions & 8 deletions src/passes/TranslateEH.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,9 @@ struct TranslateToNewEH : public WalkerPass<PostWalker<TranslateToNewEH>> {
// current 'try' into 'try_table' yet; it only adds block, br, and throw_ref
// instructions to complete the conversions of inner try~delegates that target
// the current try.
void processDelegateTarget(Try* curr, Block* outerBlock) {
void processDelegateTarget(Try* curr,
Block* outerBlock,
bool& outerBlockUsedSoFar) {
Builder builder(*getModule());

// Convert
Expand Down Expand Up @@ -291,10 +293,12 @@ struct TranslateToNewEH : public WalkerPass<PostWalker<TranslateToNewEH>> {
Name delegateBrTarget = delegateTargetToBrTarget[curr->name];
Expression* innerBody = nullptr;
if (curr->type.isConcrete()) {
outerBlockUsedSoFar = true;
auto* brToOuter = builder.makeBreak(outerBlock->name, curr->body);
innerBody = builder.blockifyWithName(
brToOuter, delegateBrTarget, nullptr, Type(HeapType::exn, Nullable));
} else {
outerBlockUsedSoFar = curr->body->type != Type::unreachable;
auto* brToOuter = curr->body->type == Type::unreachable
? nullptr
: builder.makeBreak(outerBlock->name);
Expand All @@ -304,7 +308,7 @@ struct TranslateToNewEH : public WalkerPass<PostWalker<TranslateToNewEH>> {
curr->body = builder.makeThrowRef(innerBody);
}

void processDelegate(Try* curr, Block* outerBlock) {
void processDelegate(Try* curr, Block* outerBlock, bool outerBlockUsedSoFar) {
Builder builder(*getModule());
// Convert
// (try
Expand Down Expand Up @@ -332,15 +336,15 @@ struct TranslateToNewEH : public WalkerPass<PostWalker<TranslateToNewEH>> {
// If we need an outer block for other reasons (if this is a target of a
// delegate), we insert the new try_table into it. If not we just replace
// the current try with the new try_table.
if (outerBlock) {
if (outerBlock && outerBlockUsedSoFar) {
outerBlock->list.push_back(tryTable);
replaceCurrent(outerBlock);
} else {
replaceCurrent(tryTable);
}
}

void processCatches(Try* curr, Block* outerBlock) {
void processCatches(Try* curr, Block* outerBlock, bool outerBlockUsedSoFar) {
Module* wasm = getModule();
Builder builder(*wasm);

Expand Down Expand Up @@ -385,7 +389,15 @@ struct TranslateToNewEH : public WalkerPass<PostWalker<TranslateToNewEH>> {

// If we don't have any catches, we don't need to do more.
if (curr->catchBodies.empty()) { // catch-less try
replaceCurrent(tryTable);
// If we need an outer block for other reasons (if this is a target of a
// delegate), we insert the new try_table into it. If not we just replace
// the current try with the new try_table.
if (outerBlock && outerBlockUsedSoFar) {
outerBlock->list.push_back(tryTable);
replaceCurrent(outerBlock);
} else {
replaceCurrent(tryTable);
}
return;
}

Expand Down Expand Up @@ -679,13 +691,14 @@ struct TranslateToNewEH : public WalkerPass<PostWalker<TranslateToNewEH>> {
builder.makeBlock(labels->getUnique("outer"), {}, curr->type);
}

bool outerBlockUsedSoFar = false;
if (it != delegateTargetToBrTarget.end()) {
processDelegateTarget(curr, outerBlock);
processDelegateTarget(curr, outerBlock, outerBlockUsedSoFar);
}
if (curr->isDelegate()) {
processDelegate(curr, outerBlock);
processDelegate(curr, outerBlock, outerBlockUsedSoFar);
} else { // try-catch or catch-less try
processCatches(curr, outerBlock);
processCatches(curr, outerBlock, outerBlockUsedSoFar);
}
}

Expand Down
41 changes: 41 additions & 0 deletions test/lit/passes/translate-to-new-eh.wast
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2170,4 +2170,45 @@
(unreachable)
)
)

;; CHECK: (func $try-delegate-within-catchless-try (type $1)
;; CHECK-NEXT: (block $outer1
;; CHECK-NEXT: (try_table
;; CHECK-NEXT: (throw_ref
;; CHECK-NEXT: (block $l00 (result exnref)
;; CHECK-NEXT: (try_table (catch_all_ref $l00)
;; CHECK-NEXT: (call $foo)
;; CHECK-NEXT: )
;; CHECK-NEXT: (br $outer1)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; STACKIR-OPT: (func $try-delegate-within-catchless-try (type $1)
;; STACKIR-OPT-NEXT: block $outer1
;; STACKIR-OPT-NEXT: try_table
;; STACKIR-OPT-NEXT: block $l00 (result exnref)
;; STACKIR-OPT-NEXT: try_table (catch_all_ref $l00)
;; STACKIR-OPT-NEXT: call $foo
;; STACKIR-OPT-NEXT: end
;; STACKIR-OPT-NEXT: br $outer1
;; STACKIR-OPT-NEXT: end
;; STACKIR-OPT-NEXT: throw_ref
;; STACKIR-OPT-NEXT: end
;; STACKIR-OPT-NEXT: unreachable
;; STACKIR-OPT-NEXT: end
;; STACKIR-OPT-NEXT: )
(func $try-delegate-within-catchless-try
(try $l0
(do
(try
(do
(call $foo)
)
(delegate $l0)
)
)
)
)
)

0 comments on commit 3b9dc42

Please sign in to comment.