Skip to content

Conversation

@aheejin
Copy link
Member

@aheejin aheejin commented Dec 6, 2021

We do some postprocessing after parsing Try to make sure delegate
only targets trys and not blocks:

// For simplicity, we ensure that try's labels can only 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()) {
if (exceptionTargetNames.find(block->name) !=
exceptionTargetNames.end()) {
BranchUtils::replaceExceptionTargets(block, block->name, curr->name);
exceptionTargetNames.erase(block->name);
}
}
}
if (exceptionTargetNames.find(catchLabel) != exceptionTargetNames.end()) {
for (auto* catchBody : curr->catchBodies) {
BranchUtils::replaceExceptionTargets(catchBody, catchLabel, curr->name);
}
exceptionTargetNames.erase(catchLabel);
}
curr->finalize(curr->type);

But in case the outer try has neither of catch nor delegate, the
previous code just return prematurely, skipping the postprocessing part,
resulting in a binary parsing error. This PR removes that early-exiting
code.

Some test outputs have changed because trys are assigned labels after
the early exit. But those labels can be removed by other optimization
passes when there is no inner rethrow or delegate that targets them.

(On a side note, the restriction that delegate cannot target a block
has been removed a few months ago in the spec, so if a delegate
targets a block, it means it is just rethrown from that block. But I
still think this is a convenient invariant to hold at least within the
binaryen IR. I'm planning to allow parsing of delegate targeting
blocks later, but I will make them point to try when read in the
IR. At the moment the LLVM toolchain does not generate such code.)

We do some postprocessing after parsing `Try` to make sure `delegate`
only targets `try`s and not `block`s:
https://github.com/WebAssembly/binaryen/blob/9659f9b07c1196447edee68fe04c8d7dd2480652/src/wasm/wasm-binary.cpp#L6404-L6426

But in case the outer `try` has neither of `catch` nor `delegate`, the
previous code just return prematurely, skipping the postprocessing part,
resulting in a binary parsing error. This PR removes that early-exiting
code.

Some test outputs have changed because `try`s are assigned labels after
the early exit. But those labels can be removed by other optimization
passes when there is no inner `rethrow` or `delegate` that targets them.

(On a side note, the restriction that `delegate` cannot target a `block`
has been removed a few months ago in the spec, so if a `delegate`
targets a `block`, it means it is just rethrown from that block. But I
still think this is a convenient invariant to hold at least within the
binaryen IR. I'm planning to allow parsing of `delegate` targeting
`block`s later, but I will make them point to `try` when read in the
IR. At the moment the LLVM toolchain does not generate such code.)
@aheejin aheejin requested a review from kripken December 6, 2021 23:45
@aheejin aheejin merged commit f066817 into WebAssembly:main Dec 7, 2021
@aheejin aheejin deleted the catchless_try_delegate branch December 7, 2021 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants