Uh oh!
There was an error while loading. Please reload this page.
Redo resume after catch for interpreter via native EH - #129433
Merged
janvorli merged 2 commits intoJun 16, 2026
Conversation
This change redoes the reverted change and adds fix for the problem that we were hitting in the ControlledExecutionTests. Closedotnet#129270
Contributor
Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reintroduces “resume after catch” support for the CoreCLR interpreter by routing it through native exception handling across native↔managed boundaries, addressing the fatal AccessViolation failures seen in System.Runtime.Tests.ControlledExecutionTests.
Changes:
- Adds/threads a
ResumeAfterCatchExceptionhandling layer at multiple native↔managed boundaries (QCall, prestub, virtual dispatch stubs, alloc/throw helpers, P/Invoke import worker, etc.). - Simplifies interpreter
ResumeAfterCatchto throw a marker exception and adds logic to rethrow it from an appropriate native context (skipping managed frames) for correct resumption. - Cleans up interpreter frame plumbing (removes saved SP field/uses, adds defensive
m_Next != NULLchecks) and aligns UM entry prestub unwind metadata (NoHandler).
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/wasm/helpers.cpp | Adds defensive null-check when detecting/using the next interpreter frame. |
| src/coreclr/vm/virtualcallstub.cpp | Installs/uninstalls resume-after-catch handling around stub dispatch workers. |
| src/coreclr/vm/threadsuspend.cpp | Adds resume-after-catch handling to suspension/dispatch path. |
| src/coreclr/vm/threads.cpp | Adds resume-after-catch handling around interpreter context initialization; relaxes throwable-handle assumptions. |
| src/coreclr/vm/riscv64/asmhelpers.S | Switches UM entry prestub unwind handler annotation to NoHandler. |
| src/coreclr/vm/qcall.h | Wraps QCall boundary in resume-after-catch handler. |
| src/coreclr/vm/prestub.cpp | Wraps prestub worker + interpreted entry execution with resume-after-catch handling; adjusts exception catch behavior. |
| src/coreclr/vm/loongarch64/asmhelpers.S | Switches UM entry prestub unwind handler annotation to NoHandler. |
| src/coreclr/vm/jithelpers.cpp | Adds resume-after-catch handling for throw/rethrow helpers and patchpoint helpers. |
| src/coreclr/vm/interpexec.h | Removes now-stale comment tied to deleted “lowest SP” tracking. |
| src/coreclr/vm/interpexec.cpp | Removes lowest-SP tracking; adds resume context plumbing + rethrow workaround path for resume-after-catch. |
| src/coreclr/vm/i386/umthunkstub.S | Switches UM entry prestub unwind handler annotation to NoHandler. |
| src/coreclr/vm/gchelpers.cpp | Wraps GC allocation helpers and failed-allocation helper with resume-after-catch handling. |
| src/coreclr/vm/frames.h | Removes interpreter-frame saved SP storage (no longer used). |
| src/coreclr/vm/frames.cpp | Removes dependency on interpreter saved SP when updating FP registers; adds null check in interpreter detection. |
| src/coreclr/vm/exceptmacros.h | Introduces resume-after-catch install/uninstall macros used across the VM. |
| src/coreclr/vm/exceptionhandling.h | Declares resume-after-catch rethrow helper for skipping managed frames. |
| src/coreclr/vm/exceptionhandling.cpp | Adds resume-after-catch handling to managed exception dispatch paths; implements rethrow helpers; adds SSP assert in context execution helper. |
| src/coreclr/vm/excep.cpp | Adds resume-after-catch handling for managed-fault path; removes old resume-after-catch unwind helper. |
| src/coreclr/vm/eetwain.cpp | Simplifies interpreter ResumeAfterCatch to just throw the marker exception. |
| src/coreclr/vm/dllimport.cpp | Wraps P/Invoke import worker and calli IL-stub generation with resume-after-catch handling. |
| src/coreclr/vm/arm64/stubs.cpp | Adds defensive null-check when detecting/using the next interpreter frame. |
| src/coreclr/vm/arm64/asmhelpers.S | Switches UM entry prestub unwind handler annotation to NoHandler. |
| src/coreclr/vm/amd64/umthunkstub.S | Switches UM entry prestub unwind handler annotation to NoHandler. |
| src/coreclr/vm/amd64/cgenamd64.cpp | Adds defensive null-check when detecting/using the next interpreter frame. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jun 16, 2026
Open
janvorli
commented
Jun 16, 2026
MemberAuthor
/ba-g there are many WASM failures that seem to be happening on multiple PRs. |
Uh oh!
There was an error while loading. Please reload this page.
eiriktsarpalis pushed a commit
that referenced
this pull request
Jul 15, 2026
This change redoes the reverted change and adds fix for the problem that we were hitting in the ControlledExecutionTests. The problem was that when we call DispatchManagedException from the ProcessCLRException, the added (correct) popping of frames removed frame that GC would use as a starting point. So if GC happened, it would skip a bunch of managed frames and cause the issue that the ControlledExecutionTests would hit. Before my previously reverted change, there were explicit frames that were stale, but still sufficient for providing GC with a starting point. The proper fix is to keep the explicit frames removal, but add ResumableFrame pointing to the managed frame for which the ProcessCLRException was called. That will enable GC to properly start walking stack. Close#129270
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 freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This change redoes the reverted change and adds fix for the problem that
we were hitting in the ControlledExecutionTests.
The problem was that when we call DispatchManagedException from the ProcessCLRException, the added (correct) popping of frames removed frame that GC would use as a starting point. So if GC happened, it would skip a bunch of managed frames and cause the issue that the ControlledExecutionTests would hit.
Before my previously reverted change, there were explicit frames that were stale, but still sufficient for providing GC with a starting point.
The proper fix is to keep the explicit frames removal, but add ResumableFrame pointing to the managed frame for which the ProcessCLRException was called. That will enable GC to properly start walking stack.
Close#129270