Repository navigation
JIT: make Wasm frames that call a finally unwindable - #134979
Merged
Merged
Conversation
genCallFinally emits a call to the finally funclet but did not mark the calling function as needing an unwindable frame. When the callfinally was the method's only call, the prolog never stored the function table index at $fp[0]. An exception thrown from the finally then unwound into a frame with a stale index, the runtime saw no R2R caller, and the exception was reported as unhandled instead of reaching the caller's catch. Fixes b51875 under browser-wasm R2R. Also affects returning finallys that are not cloned (for example in MinOpts). Fixes #134975 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
Azure Pipelines: Successfully started running 6 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Member
Author
|
cc @dotnet/wasm-contrib |
Contributor
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
b51875 and the JIT/Methodical tests already cover this under Wasm R2R. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
steveisok
approved these changes
Sep 30, 2026
jkotas
approved these changes
Sep 30, 2026
adamperlin
approved these changes
Sep 30, 2026
lewing
enabled auto-merge (squash)
September 30, 2026 21:49
This was referenced Oct 1, 2026
lewing
added a commit
that referenced
this pull request
Oct 1, 2026
These JIT regression tests were quarantined for wasm ReadyToRun in #134960 against #134975 (an exception thrown from a `finally` was reported as unhandled instead of reaching the caller's `catch`). #134979 ("JIT: make Wasm frames that call a finally unwindable") fixed that and closed #134975, so this removes the three `ActiveIssue` attributes: - `JIT/Regression/CLR-x86-JIT/V1-M12-Beta2/b51875/b51875.cs` (runner `Regression_8`) - `JIT/Regression/CLR-x86-JIT/V1-M12-Beta2/b51875/Desktop/b51875.cs` (runner `Regression_PdbOnly_r_2`) - `JIT/Regression/CLR-x86-JIT/V1-M12-Beta2/b77707/b77707.cs` (runner `Regression_PdbOnly_r_2`) ## Local validation (macOS arm64, branch includes 9997fdf) ```bash ./build.sh -s clr+libs -os browser -c checked -lc Release src/tests/build.sh checked -arch wasm -os browser -priority1 -p:HostConfiguration=Release \ -test:JIT/Regression/Regression_8.csproj -test:JIT/Regression/Regression_PdbOnly_r_2.csproj src/tests/build.sh copynativeonly checked -arch wasm -os browser -priority1 -p:HostConfiguration=Release src/tests/build.sh generatelayoutonly crossgen2 checked -arch wasm -os browser -priority1 -p:HostConfiguration=Release # per runner, with CORE_ROOT set and DOTNET_TieredCompilation=0: RunCrossGen2=1 bash ./<Runner>.sh # R2R mode bash ./<Runner>.sh # interpreter mode ``` All builds passed. Results from `<Runner>.testResults.xml`: | Runner | Mode | b51875 | b77707 | Pass / Fail / Skip | |---|---|---|---|---| | Regression_8 | R2R (crossgen2) | Pass | n/a | 13 / 0 / 0 | | Regression_8 | Interpreter | Pass | n/a | 13 / 0 / 0 | | Regression_PdbOnly_r_2 | R2R (crossgen2) | Pass | Pass | 122 / 0 / 1 | | Regression_PdbOnly_r_2 | Interpreter | Pass | Pass | 122 / 0 / 1 | `runtime-coreclr outerloop` has no PR trigger, so it has to be started manually with `/azp run runtime-coreclr outerloop`. > [!NOTE] > This PR was generated with GitHub Copilot assistance. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On Wasm,
genCallFinallycalls the finally funclet but did not mark the calling function as needing an unwindable frame.genCallInstructionandgenEmitHelperCallboth callensureCurrentFuncIsUnwindable();genCallFinallywas the only call emitter that did not.When the callfinally is a method's only call, the prolog never stores the R2R function table index at
$fp[0]. If the finally throws, unwinding out of the funclet reads a stale value from that slot.GetWasmVirtualIPFromFunctionTableIndexreturns 0, the walk treats the caller as non-R2R, and the exception is reported as unhandled instead of reaching the caller's catch. A GC stack walk while the finally runs goes through the same broken frame.This hits b51875, where the finally never returns. It also hits a finally that returns but is not cloned onto the normal path, for example under MinOpts.
Change
genCallFinallynow callsensureCurrentFuncIsUnwindable(). The prolog is generated after block codegen, so the flag takes effect.fgWasmVirtualIPalready creates the function-index local for any method with aBBJ_CALLFINALLY. Funclets that call a finally were already unwindable.No new test: b51875 and the
JIT/Methodicalrunners already cover this under browser-wasm R2R (see below).Validation
browser-wasm, Checked,
src/tests/run.sh wasm checked --runcrossgen2tests, comparing the JIT with and without this fix (the JIT sources otherwise match):--tree=JIT/Regression --runner-filter=Regression_8(b51875)Unhandled exception. System.Exception, runner abortsb51875.AA.TestEntryPoint--tree=JIT/Methodical(6 runners)The 22
JIT/Methodicalfailures are out-of-processexplicit/*tests (refloc_*,refarg_box_f8,rotate_u2), which fail on an assertion in the out-of-process runner. There is no baseline for them because every runner crashed before reaching them without the fix. This change only affects codegen for calls to a finally.JIT disassembly confirms the main method prolog now stores the function index.
Not run:
Regression_PdbOnly_r_2, which contains the other copy of b51875.Resolves #134975
Note
This PR description was generated with GitHub Copilot.