Wasmtime GitHub notifications bot (Sep 05 2025 at 20:39): Wasmtime GitHub notifications bot (Sep 05 2025 at 20:39):cfallin requested alexcrichton for a review on PR #11629.
Wasmtime GitHub notifications bot (Sep 05 2025 at 20:39):cfallin opened PR #11629 from cfallin:oh-exception-handler-what-is-your-address to bytecodealliance:main:
This is designed to enable applications such as #11592 that use alternative unwinding mechanisms that may not necessarily want to walk a stack and look up exception tables. The idea is that whenever it would be valid to resume to an exception handler that is active on the stack, we can provide the same PC as a first-class runtime value that would be found in the exception table for the given handler edge. A "custom" resume step can then use this PC as a resume-point as long as it follows the relevant exception ABI (i.e.: restore SP, FP, any other saved registers that the exception ABI specifies, and provide appropriate payload value(s)).
Handlers are associated with edges out of
try_calls (ortry_call_indirects); and edges specifically, not blocks, because there could be multiple out-edges to one block. The instruction thus takes the block that contains the try-call and an immediate that indexes its exceptional edges.This CLIF instruction required a bit of infrastructure to (i) allow naming raw blocks, not just block calls, as instruction arguments, and (ii) allow getting the MachLabel for any other lowered block during lowering. But given that, the lowerings themselves are straightforward uses of MachBuffer labels to fix-up PC-relative address-loading instructions (e.g.,
LEAorADRorAUIPC+ADDI).<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Sep 05 2025 at 20:39):cfallin requested wasmtime-compiler-reviewers for a review on PR #11629.
Wasmtime GitHub notifications bot (Sep 05 2025 at 20:59):cfallin updated PR #11629.
Wasmtime GitHub notifications bot (Sep 05 2025 at 21:00):cfallin updated PR #11629.
Wasmtime GitHub notifications bot (Sep 05 2025 at 21:10):fitzgen submitted PR review:
LGTM, thanks for whipping this up!
Wasmtime GitHub notifications bot (Sep 05 2025 at 21:11):cfallin has enabled auto merge for PR #11629.
Wasmtime GitHub notifications bot (Sep 05 2025 at 21:28):alexcrichton submitted PR review:
Thanks for this! This'll definitely help cleanup #11592. I think the main thing that I was missing was that the target in question is a
MachLabelthat theMachBufferalready has on its creation since it reserves for all blocks. That makes total sense in retrospect, but bypasses what I was worried about where new labels might need be created and bound to previously-added addresses.Otherwise I'm curious below about the lookup of the handler in vcode where it doesn't have an adjust-by-1 I'd otherwise expect. Also I'm curious about how this interacts with tables that have a mixture of blocks/defaults/contexts, specifically contexts, and clarifying that the index effectively skips all context arguments?
Wasmtime GitHub notifications bot (Sep 05 2025 at 21:28):alexcrichton created PR review comment:
Mind adding a few more tests here?
- Is it valid to have 0 handlers on a
try_call? If so can this test that a 0 index is out-of-bounds?- A valid test that selects a default handler.
- A valid test that selects a non-default handler.
- A valid test that selects a handler from a non-length-1 list of handlers.
- A valid test which selects the Nth handler when there's also context arguments interspersed throughout the list of handlers. (the index is a block index, so not an index in the exception table data right?)
Wasmtime GitHub notifications bot (Sep 05 2025 at 21:28):alexcrichton created PR review comment:
Is the -1 here to skip the non-exceptional block at the front? If so mind leaving a comment to that effect? (and maybe model this as
if etd.all_branches[1..].get(index).is_none())
Wasmtime GitHub notifications bot (Sep 05 2025 at 21:28):alexcrichton created PR review comment:
As a mild bikeshed these can probably be named something like "load the label address in this register" since it's not really relate to exceptions. I'm a bit sad a new MInst was needed everywhere but on-the-whole this is a great change!
Wasmtime GitHub notifications bot (Sep 05 2025 at 21:28):alexcrichton created PR review comment:
In
verify_try_call_handler_indexbelow there's an offset-by-1 to skip what I presume is the default normal-return path, but this doesn't seem to also have a -1 or adjust-by-1 anywhere. Is that intentional?
Wasmtime GitHub notifications bot (Sep 05 2025 at 22:11):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Sep 05 2025 at 22:11):cfallin created PR review comment:
Done!
Wasmtime GitHub notifications bot (Sep 05 2025 at 22:11):cfallin updated PR #11629.
Wasmtime GitHub notifications bot (Sep 05 2025 at 22:11):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Sep 05 2025 at 22:11):cfallin created PR review comment:
Yes, this helper returns any successor, not limited to exceptional edges. Separately there is the invariant that exceptional edges come first out of a
try_callblock, so we can use the exceptional-edge index as the block successor index where we invoke this. (There's a comment inblock_exn_successor_labelinmachinst/isle.rswhere this helper is called noting this.)
Wasmtime GitHub notifications bot (Sep 05 2025 at 22:11):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Sep 05 2025 at 22:11):cfallin created PR review comment:
The non-exceptional block is actually stored last (I originally had it the other way but that came out of code-review discussion IIRC); but the
- 1is just because for N out-edges there are N-1 handlers we can select. Will add a comment.
Wasmtime GitHub notifications bot (Sep 05 2025 at 22:20):cfallin updated PR #11629.
Wasmtime GitHub notifications bot (Sep 05 2025 at 22:21):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Sep 05 2025 at 22:21):cfallin created PR review comment:
Sure, added in 9db98f47b8622f35f73efc09c4d799094560119a (three tests to cover all those cases).
To confirm: zero handlers is valid (first test); tagged and default can be selected (second and third tests); the index is among handlers, in the order they appear, skipping dynamic context table items.
Wasmtime GitHub notifications bot (Sep 05 2025 at 22:22):cfallin commented on PR #11629:
Updated, thanks @alexcrichton ! Will wait for your r+ as well before merging.
Wasmtime GitHub notifications bot (Sep 05 2025 at 22:26):alexcrichton has enabled auto merge for PR #11629.
Wasmtime GitHub notifications bot (Sep 05 2025 at 22:27):alexcrichton commented on PR #11629:
Oh protocol-wise I'm fine having feedback handled in a follow-up PR myself, but thanks though! I'm kind of tired so I'm going to call it for today, but I'll once-over on Monday (I suspect everything is fine though)
Wasmtime GitHub notifications bot (Sep 05 2025 at 23:44):alexcrichton merged PR #11629.
Last updated: Dec 06 2025 at 07:03 UTC