[wasm][RyuJIT] Checked assert 'bbNumTryBeg <= bbNumTryLast' crossgen'ing always-suspending async methods that have a try/fault region

[lewing]

Summary

When crossgen'ing (R2R) System.Private.CoreLib for browser-wasm in a Checked build, the wasm CFG lowering asserts:

Assertion failed 'bbNumTryBeg <= bbNumTryLast' in
'System.Diagnostics.Tracing.EventPipeEventDispatcher:DispatchEventsToEventListeners(ulong,System.DateTime,long,long,System.Threading.CancellationToken):this'
during 'Wasm control flow' (IL size 87; hash 0x9a4e6db5; FullOpts)

Source: Compiler::fgVerifyHandlerTab — src/coreclr/jit/jiteh.cpp:3370 (assert(bbNumTryBeg <= bbNumTryLast)), fired at the end of the fgWasmControlFlow phase.

This blocks Checked/Debug wasm R2R crossgen of CoreLib entirely (the first asserting async method aborts the whole compile), which breaks the ability to produce Checked wasm R2R composites for debugging.

Environment

• Config: Checked wasm R2R crossgen (the assert is in fgVerifyHandlerTab, which only runs under Checked/Debug; Release does not assert but still produces the same mis-ordered layout — see below).
• This is on a branch stacked on the runtime-async wasm R2R work ([Wasm Ryujit]: implement runtime-async codegen for R2R Wasm #131167, not yet merged).
• The trigger is the merged always-suspend optimization JIT: Optimize always suspending helpers #130493 ("JIT: Optimize always suspending helpers"), which our older base predated. Rebasing onto current main picked it up and exposed the interaction.
• Reproduces deterministically on EventPipeEventDispatcher.DispatchEventsToEventListeners; the same shape appears on the IndentedTextWriter.WriteAsync family.

Root cause

The asserting method's async-transformed EH is a single try/fault region (try BB24..BB30, fault BB06). Its entries (from the try-region builder):

BB01->BB24 (normal), BB28->BB24[async], BB31->BB30[async]   <- side-entry into the try's LAST block

#130493's always-suspend path (async.cpp, const bool alwaysSuspends = call->GetAsyncInfo().AlwaysSuspends;) turns the await block into an unconditional branch to the suspension (BBJ_ALWAYS) and leaves the remainder block reachable only via async resumption. In this method the remainder is BB30, which is the try's last block. Because its synchronous in-try predecessor is removed, BB30 is now reachable only from BB31 (the resumption dispatcher, outside the try).

Consequently the region's RPO enumeration is [BB30, BB24, ...] — the try-last block before the try-begin.

The layout is produced by fgVisitBlocksInTryAwareLoopAwareRPO (compiler.hpp). That visitor only keeps a region header-first / contiguous for regions with a catch handler:

• compiler.hpp: early-out to the plain loop-aware RPO when tryRegions->NumTryCatchRegions() == 0, and the header-first region visitation is gated on tryRegion->HasCatchHandler().
• flowgraph.cpp (FlowGraphTryRegions::Build): m_numTryCatchRegions is only incremented for ehDsc->HasCatchHandler().

So a method whose only EH is a try/fault (or try/finally) takes the fast loop-aware path with no header-first enforcement. Normally that's fine because the fault region's begin is reached first via synchronous flow; but with the always-suspend transform the try-last block is reachable only via the resumption side-entry and is emitted before the header → bbNumTryBeg > bbNumTryLast → assert.

The try-region builder actually anticipates async-resumption side-entries into try/fault and try/finally regions (flowgraph.cpp, region->SetHasSideEntry() on BBF_ASYNC_RESUMPTION edges, with a comment that try/fault + try/finally "are emitted differently and tolerate multi-entry" and only try/catch side-entries are NYI'd). The gap is that the layout side doesn't keep those side-entry fault/finally regions header-first.

Why the naive fix isn't sufficient (cascade)

I prototyped making side-entry fault/finally regions go through the try-aware visitor and emitting the region header-first (added a GetTryRegionByBlock + a HasSideEntryTryRegions flag; extended the header-first/contiguous visitation to HasSideEntry() regions). That cleared the bbNumTryBeg assert on DispatchEventsToEventListeners, but cascaded into a second wasm-EH invariant on the WriteAsync family:

Assertion failed 'loops->GetLoopByHeader(succ) != nullptr' during 'Wasm control flow'

src/coreclr/jit/fgwasm.cpp:1420 — the interval builder's rule that a back-edge (succNum <= cursor) targets only a loop header. The header-first reordering turned a forward edge into a back-edge to a non-loop-header for a method that previously laid out validly on the fast path.

So a correct fix needs the try-aware RPO and the interval builder to keep async side-entry try/fault + try/finally regions consistent across all their coupled invariants (header-first ordering, contiguous loop bodies, back-edge/loop-header, try-interval nesting) — not just the single-entry try/catch case.

Current workaround (local, wasm-only)

Force the conditional suspension check on wasm so the remainder keeps a synchronous in-try predecessor (a redundant, always-true null check for these helpers — functionally identical, minor perf-only):

// async.cpp
#ifdef TARGET_WASM
    const bool alwaysSuspends = false;
#else
    const bool alwaysSuspends = call->GetAsyncInfo().AlwaysSuspends;
#endif

With this, Checked wasm R2R crossgen of CoreLib succeeds (EC=0, all R2R images emitted) and Release library suites are unaffected (e.g. Microsoft.Bcl.Memory.Tests 550/550 R2R on and off). This is only a stopgap to unblock Checked debugging; the proper fix is in the wasm EH lowering.

Note

This issue was written with the help of GitHub Copilot.

cc @AndyAyersMS (wasm EH lowering / fgWasmControlFlow), @jakobbotsch (#130493). Line references are from a branch based on #131167.

[dotnet-policy-service]

Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara
See info in area-owners.md if you want to be subscribed.

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[AndyAyersMS]

This is on a branch stacked on the runtime-async wasm R2R work

@lewing this doesn't repro with just the runtime-async changes, and I don't see any plausible draft PRs to stack there... let me know how to repro this when you get a chance.

[jakobbotsch]

If reverting the async transformation is easier then feel free to go with that. I do not think this optimization is particularly important.

[lewing]

This is on a branch stacked on the runtime-async wasm R2R work

@lewing this doesn't repro with just the runtime-async changes, and I don't see any plausible draft PRs to stack there... let me know how to repro this when you get a chance.

I will test again and get you a repro or close it out.

[AndyAyersMS]

I can repro this now in main... not sure why I couldn't before

[AndyAyersMS]

Repro requires a checked jit and release SPC.

We could probably fix this with more SCC pseudo-edge machinery, but it feels like that trick is nearly played out.

We could just fix the multi-entry tries up front and avoid these problems all together. So I am looking a more ambitious rewrite: a new phase after the EH transform that adds dispatch logic to any try header where the try has side entries. This can cascade for nested trys.

The downside is possible multiple levels of dispatch in the flow logic (async resume dispatch -> try resume dispatch -> try side entry dispatch -> scc dispatch (if unlucky)).