Repository navigation
Accept a faulted frame in the shadow stack assert of CallCatchFunclet - #135531
Merged
Merged
Conversation
A thread abort raised again at the end of a catch handler is thrown from ThrowControlForThread, which records the SSP of the frame the thread was redirected from. That frame was interrupted rather than stopped at a call, so the slot below its SSP does not hold its ControlPC, and the assert failed on Windows x64 with shadow stacks when the abort was caught in the same frame. The SSP itself is the right value to resume the frame with. Fixes dotnet#135530
dotnet-policy-service
Bot
added
the
community-contribution
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @agocke |
jkotas
reviewed
Oct 9, 2026
It aborts a thread and is a niche case.
davidwrighton
pushed a commit
to davidwrighton/runtime
that referenced
this pull request
Oct 21, 2026
…otnet#135638) When a catch block ends with a thread abort pending, the runtime raises the abort again from the address the catch returns to and looks the handlers up for that address (`EECodeManager::ResumeAfterCatch` → `COMPlusCheckForAbort` → `ThrowControlForThread`, which dispatches from a `FaultingExceptionFrame` at the exact resume address). The JIT accounts for that: `impImportLeave` places the block a catch returns to inside the EH regions that enclose the catch (see its comment on `ThreadAbortException`), and `fgOptimizeEmptyBlock` keeps that block when it is empty and in a different EH region than its successor, and adds a `GT_NO_OP` to it, "for re-raise of thread abort exceptions to work". That only works if the block generates an instruction, and whether it does is only known in codegen. When it generates none, the resume address belongs to the code that follows, and the abort is dispatched as if it had been raised there. This happens in unoptimized code (tier 0, `JITMinOpts`, debuggable code), where `fgOptimizeEmptyBlock` does not run, and in optimized code too: - the block can contain nodes that generate no code, so `fgUpdateFlowGraph` never passes it to `fgOptimizeEmptyBlock`: a block whose only statement is `GC.KeepAlive` of an argument; - the block can be the first block of a try region, which `fgUpdateFlowGraph` skips (`BBF_DONT_REMOVE`): in a loop of try/catch inside another try, the catch returns to the loop head, an empty block that begins the outer try and falls into the inner one, so the abort is raised again inside the inner try and the same catch runs again; - on xarch, the block's only instruction can be a jump that `emitRemoveJumpToNextInst` removes because the blocks between it and its target generate no code (the step block of `LeaveOutOfOuterTry` below, at tier 0). In the shapes of dotnet#135637 a `finally` the catch's `leave` passes through does not run, an enclosing catch does not see the abort, and a `try` that starts right after the catch does. This change makes `genEmitEndBlock` guarantee that the block generates an instruction: if a `BBJ_ALWAYS` block that a catch returns to has emitted nothing since its label, it emits a `nop` when the jump to the next block is omitted, and otherwise does not mark the jump as removable. The check does not depend on the EH region of the next block, because when that block emits nothing either, the address belongs to a later one: a variant that acted only when the next block, or the jump's target, was in another EH region left 7 catch-return targets with mismatched EH membership in the SuperPMI replay below (tier-0 instantiations of `System.Numerics.Tensors.Tests.Helpers.AssertEqualWithTolerance`). `fgOptimizeEmptyBlock` still adds its `GT_NO_OP`: an earlier revision of this PR removed it, and Wasm crossgen then failed because `fgUpdateFlowGraph` redirected the resume pad that `fgWasmEhFlow` adds past the now empty block, leaving it reachable only from the catch return. ### What changes Checked builds on linux-x64, the JIT from this branch against a JIT built at main 523a36f on the same runtime; three runs per configuration and shape, identical each time. The table uses standalone copies of the test's six shapes without `AggressiveOptimization`, so that each configuration applies to them (the copy of `CatchInLoop` also appends a marker before its `break`); "finally ran" is a call-free `volatile` store. | shape | tier 0 before | `JITMinOpts=1` before | `TieredCompilation=0` before | after | |---|---|---|---|---| | `CatchThenFinally` | `catch outer`, finally not run | same as tier 0 | correct | `catch finally outer` in all three | | `LeaveOutOfOuterTry` | `inner` | `inner` | correct | `inner outer` in all three | | `ConsecutiveTry` | `inner new catch` | `inner new catch` | correct | `inner` in all three | | `KeepAliveAfterCatch` | correct | correct | `inner` | `inner outer` in all three | | `KeepAliveThenFinally` | correct | correct | `catch outer`, finally not run | `catch finally outer` in all three | | `CatchInLoop` | correct | the catch runs 3 times | the catch runs 3 times, then `outer` | `catch outer` in all three | At tier 1 the two `KeepAlive` shapes fail as with `TieredCompilation=0` and pass with this change. On osx-arm64 and windows-x64 the same shapes fail with a JIT without this change, except that `LeaveOutOfOuterTry` is correct on arm64, which has no late emitter pass that removes this jump across empty blocks, and all pass with this change. In the tier-0 JitDump of `CatchThenFinally`, the catch-return block `BB03` is now a one-byte `nop` emitted in codegen at `0x1B`, and the finally's try range covers it (`EH#1: try [0014..001C)`; before: `[0014..001B)`, no code for `BB03`). In `LeaveOutOfOuterTry` the step block keeps its two-byte `jmp` (`EH#1: try [0014..001E)`; before, `emitRemoveJumpToNextInst` removed it and the range was `[0014..001C)`). In optimized code the first three shapes compile as before; for `KeepAliveAfterCatch` the block gets a `nop` that the enclosing try covers (`EH#1: try [000E..001D)`; before: `[000E..001C)`), and in `CatchInLoop` the catch returned to `0x0F`, the first instruction of its own try (`EH#0: try [000F..0017)`), and now returns to a `nop` there, with the try starting at `0x10`. ### Tests `Regressions/coreclr/GitHub_135637/test135637` has the six shapes. `KeepAliveAfterCatch`, `KeepAliveThenFinally` and `CatchInLoop` are marked `AggressiveOptimization`, so the default configuration compiles them optimized; `JITMinOpts=1` still exercises MinOpts. With the current JIT the test fails under the default configuration, `JITMinOpts=1` and `TieredCompilation=0`; with the first version of this PR it fails under the default configuration and `TieredCompilation=0`; with this change it passes in all three, on linux-x64, osx-arm64 and windows-x64 (Windows needs dotnet#135531, now merged, on machines with CET shadow stacks). Removing either half of the check makes the test fail: without the `nop` under all three configurations (linux-x64 and osx-arm64), and with the jump left removable under the default configuration and `JITMinOpts=1` (at `LeaveOutOfOuterTry`, linux-x64). Under the interpreter the test is skipped with an `[ActiveIssue]` for dotnet#135871: there the outer catch does not see the abort in `LeaveOutOfOuterTry` and `CatchInLoop` (measured with dotnet#135520 applied; without it, Checked builds stop at the assert of dotnet#135463). `JIT/Methodical` (`Methodical_d2`, `_r2`, `_do`, `_ro`, `_others`), `JIT/jit64` (`jit64_5`) and `baseservices/exceptions` on linux-x64 under the default configuration, `TieredCompilation=0` and `JITMinOpts=1`, with the JIT before and after the change: no result differs. 74 of the 79 runner scripts pass with both JITs, and the same five (`baseservices/exceptions/inproccrashreport/*`) fail with both on that machine. To look for the problem more widely I used a local diagnostic (not part of this PR). For methods without hot/cold splitting, it compares each catch-return target's address after codegen with the reported try and handler ranges in the same funclet. Replaying the linux-x64 SuperPMI collections records 2,421 mismatches with the current JIT: 2,411 in unoptimized code, and 10 in optimized code in 6 methods. In the optimized cases inspected, including `WebSocketHandle.ConnectAsync`, Kestrel's `SocketConnectionListener.AcceptAsync` and OpenTelemetry's `ProtobufOtlpResourceSerializer.SerializeResourceToBytes`, the catch returns to the first instruction of a try that does not contain the block. With this change there are no recorded mismatches. Over the EH test trees above the diagnostic records 724 mismatches under the default configuration and 738 under `JITMinOpts=1` with the current JIT, and none with this change. SuperPMI asm diffs, Checked JITs: on linux-x64, 1,591 of 3,267,793 contexts change, MinOpts +2,659 bytes and FullOpts +10 bytes, +2,669 in total; on linux-arm64 with the cross-compiled JIT, 1,488 of 3,124,627 contexts change, MinOpts +8,796 and FullOpts +32, +8,828 in total; browser-wasm has no replay failures and no code-size change. Tier-0 code size, jitting with `ReadyToRun=0` (CoreLib and the repro program, three runs per JIT, linux-x64): of 2,468 tier-0 methods whose size is stable across the runs, 4 grow (the three shapes of the repro and `System.Diagnostics.StackFrameHelper:InitializeSourceInfo`, by one byte each except `LeaveOutOfOuterTry`, which keeps a two-byte `jmp`); with `TieredCompilation=0` the 958 compared methods are unchanged. On arm64 the `nop` is four bytes. Resolves dotnet#135637 > [!NOTE] > This PR was prepared with AI assistance (Anthropic Claude, checked by OpenAI Codex) at my request. I reviewed the change, the test and the measurements before opening it. --------- Co-authored-by: Egor Bogatov <egorbo@gmail.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.
Fixes #135530.
On windows-x64 with shadow stacks, a Checked runtime asserts in
CallCatchFuncletwhen a thread abort that the runtime raises again at the end of a catch handler is caught by another handler in the same method.EECodeManager::ResumeAfterCatchsends such a thread toThrowControlForThread, andRedirectForThrowControlrecords in theFaultingExceptionFramethe SSP at the point where the thread was redirected. That frame was interrupted rather than stopped at a call, so the slot below its SSP does not hold itsControlPC, and the assert's last clause fails when the catch handler is in that frame. The SSP points at the frame's return address, which is what resuming the frame needs. The assert now also accepts a handler frame that has faulted. Release builds are unchanged.Test: new
src/tests/Regressions/coreclr/GitHub_135530, the case from the issue. It is disabled under the interpreter until #135520 (fixes #135463) is in; the interpreter fails it for that reason, not this one.Validation (Checked):
CallCatchFuncletTieredCompilation=0,JITMinOpts,JitStress1 and 2,ReadyToRun=0,GCStress0x3 and 0xCThe other probes from the issue investigation also print the same on windows-x64 as on linux-x64 with this change, including an abort raised again right in front of a try block that catches it.
Note
This PR was prepared with AI assistance (Anthropic Claude, reviewed by OpenAI Codex). The builds and runs it cites were done at the PR's base commit.