Uh oh!
There was an error while loading. Please reload this page.
JIT/wasm: more EH related fixes - #129716
Conversation
Inject unreachable in-try blocks as successors of the try entry in VisitWasmSuccs so the wasm DFS visits them and the layout pass keeps them inside the try region; otherwise they end up past fgFirstFuncletBB and break EH-contiguity (b27824). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
AndyAyersMS
commented
Jun 22, 2026
Yet another wasm control flow fix (hopefully the last of these, barring async). We may have unreachable blocks in a try (say inner trys). We need to place these properly for wasm. Pretend they are reachable from the enclosing try entry (like we do for throw helpers). @adamperlin PTAL With this all the Pri-1 tests under JIT pass for me (or are properly skipped) when the test assemblies are built R2R. |
There was a problem hiding this comment.
Pull request overview
This PR updates the WASM-specific successor enumeration used by the JIT’s DFS/layout machinery so that certain “unreachable” blocks inside a try region (notably, blocks with no recorded predecessors) are still discovered during WASM DFS. This helps keep those blocks laid out within their try region to preserve EH contiguity.
Changes:
- Extend
FgWasm::VisitWasmSuccsto additionally visit unreachable in-try blocks as successors of the try entry. - Filter injected blocks to avoid throw helpers and blocks not in the same EH region as the try entry.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Move the per-try-entry scan from FgWasm::VisitWasmSuccs to FlowGraphTryRegions::Build, which now populates a vector of in-try blocks that won't be reached by the DFS. The wasm successor enumerator just walks that vector. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
I have another EH fix I'm working on that I'll add here shortly. (edit: turned out to be several similar fixes) |
Compute the [exnref]-wrapper's depth on the control flow stack instead of assuming depth 0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The wasm validator treats throw-helper calls as regular calls that return, so without an explicit unreachable subsequent code is not polymorphic and a containing block-exnref wrapper's end fails to validate when the throw is the only fall-through path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ast block When a function or funclet ends with a BBJ_ALWAYS (e.g., backedge of an infinite loop), pop any still-open wasm intervals, emit unreachable so the implicit return is polymorphic, then emit the function-body end terminator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The wrapper's end is only meant to be reached via catch_ref (which pushes the exnref). Code emitted in the wrapper body between an inner Try and the wrapper end can leave a non-polymorphic stack; emit unreachable right before the wrapper's end so normal-flow fall-through paths satisfy the wrapper's [exnref] signature. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use bbPreds == nullptr as the unreachability test and `continue` past the m_blocks AddElemD and pred-edge enumeration, since the block's DFS index is meaningless and would corrupt the region's bit set. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
AndyAyersMS
commented
Jun 23, 2026
These were hidden from me for a time because corerun.js / runtime silently fall back to the interpreter if the wasm module fails to validate. |
AndyAyersMS
commented
Jun 23, 2026
Remaining pri-1 failures (under JIT) when tests are R2R'd are either SIMD confusion or hitting a Wasm limit. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- flowgraph: key the unreachable-block detection off DFS membership when a dfsTree is provided; pred-less is only a valid proxy when no DFS exists (blocks injected via VisitWasmSuccs as fake successors can be in the DFS while still having no real preds). - codegen: extract genEmitFunctionEnd helper that mirrors the per-interval bookkeeping of genEmitStartBlock's pop loop (Try -> trailing unreachable, ExnRefWrapper -> preceding unreachable + trailing local.set), and use it from both BBJ_THROW and BBJ_ALWAYS at function/funclet end. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
AndyAyersMS
commented
Jun 23, 2026
My ability to confidently state R2R is working should be questioned. Looks like there is still an issue with EH unwinding to sort through. What I can say is that all R2R images validate (modulo the ~7 simd and too many parameters failures) and are loaded by the runtime and used. But not all tests pass yet, and because of merged test runners I don't have clean read on how many fail. |
AndyAyersMS
commented
Jun 23, 2026
Failure seems to be (in part) that we don't always make root methods unwindable when they need to be. Looking at fixes now, but that will be a separate PR. |
The cross-platform code at the start of the BBJ_THROW handler emits INS_BREAKPOINT (== INS_unreachable on wasm) in several cases, and the prior commit emitted another INS_unreachable unconditionally after, giving two unreachable ops in a row. genEmitFunctionEnd also added a third terminal unreachable when the BBJ_THROW was the last block. #ifdef the cross-platform BREAKPOINT logic off for wasm and just emit INS_unreachable directly; pass emitTerminalUnreachable=false to genEmitFunctionEnd so it doesn't add yet another. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
The implicit call_indirect to the finally funclet is emitted at codegen time from BBJ_CALLFINALLY blocks, but the per-block virtualIP refresh was skipped because the blocks are LIR-empty. fp+4 stayed at the try region's virtualIP, so when the finally threw, the runtime EH walker saw IP inside try and re-dispatched the same finally during unwind - running it twice. Extend the condition to also refresh BBJ_CALLFINALLY blocks. LIR inserted into the block runs before genCallFinally emits its call. Fixes Test_throwinfinally_basics and similar try/finally tests under R2R. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
adamperlin
left a comment
There was a problem hiding this comment.
I think this overall looks good to me. Just a fairly minor comment and a question about the try block unreachability heuristic.
Uh oh!
There was an error while loading. Please reload this page.
Fix remaining Pri1 validation errors. * Inject unreachable in-try blocks as successors of the try entry in VisitWasmSuccs * Handle methods with return values that never return via appropriate `unreachable` * Likewise emit `unreachable` in a few more cases where we do not have fall through and might have type mismatches * Compute depth of the exnref wrapper block rather than assuming it is always at top of stack * Ensure we emit a VIP when we transition from a try to a callfinally Fixes#129743. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix remaining Pri1 validation errors.
unreachableunreachablein a few more cases where we do not have fall through and might have type mismatchesFixes#129743.