[wasm] Don't let native cleanup change the GC mode across a catch resumption - #133547
[wasm] Don't let native cleanup change the GC mode across a catch resumption#133547davidwrighton wants to merge 6 commits into
Conversation
The wasm R2R catch-resumption path resumes managed code by throwing a native exception tag from RtlRestoreContext. Native cleanup that runs during that unwind can transition the thread's GC mode, so managed code resumes in the wrong mode and later trips the OBJECTREF/preemptive-mode assert in Object::Validate (dotnet#133219). Rather than repairing the mode at the resumption point, make the illegal transition itself fail where it happens. Add a thread_local t_gcModeSwitchPermitted, cleared just before ThrowRtlRestoreContextTag and restored by a new CORINFO_HELP_JIT_RESUME_AFTER_CATCH helper emitted into the BBF_CATCH_RESUMPTION block, and assert on it at every site that changes a thread's GC mode: Thread::DisablePreemptiveGC, Thread::EnablePreemptiveGC, the four reverse-P/Invoke helpers that hand-inline the transition, and the wasm P/Invoke and GC-poll helpers. Only asserts read the flag, so release builds pay only the two stores. Helper mappings, the JIT-EE GUID, and R2R version 28.1 are synchronized. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
On wasm, RtlRestoreContext resumes managed code by throwing a custom WebAssembly exception tag. Clang lowers destructor cleanups to catch_all, which intercepts that foreign tag, runs the cleanup, and rethrows, so any RAII GC mode holder on the unwound frames flips the thread's GC mode and managed code resumes in the wrong mode. catch (...) lowers to a catch of the C++ tag only, so a region built from an explicit try/catch (...) is skipped on the resume path while still restoring the mode for real C++ exceptions. Add GCX_COOP_REGION_BEGIN/END, GCX_PREEMP_REGION_BEGIN/END and GCX_MAYBE_COOP_REGION_BEGIN/END, and convert the holders whose scope spans a transition into managed code: * RuntimeMethodHandle_InvokeMethod * the interpreter (interpexec.cpp) and its entry points in prestub.cpp * CustomAttribute_CreateCustomAttributeInstance, which invokes the attribute constructor through CallDescrWorker A region is only equivalent to the corresponding holder when control falls out of it normally, so sites with an early return were restructured to assign to a result variable declared before the region. Also add src/tests/Common/scripts/clean-il-cg2.sh, which removes the stale per-test IL-CG2 crossgen output directories that otherwise keep runtime tests running against R2R images built by an older compiler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The t_gcModeSwitchPermitted diagnostic requires that every catch resumption point ends with a call to CORINFO_HELP_JIT_RESUME_AFTER_CATCH to lift the restriction again. Emitting that helper unconditionally costs code size in shipping images, so make it opt-in: - crossgen2 gains --verify-gc-mode-transitions, which passes CORJIT_FLAG_VERIFY_GC_MODE_TRANSITIONS to the JIT and stamps READYTORUN_FLAG_VERIFY_GC_MODE_TRANSITIONS into the R2R header. - The wasm JIT only emits the helper when that flag is set on an R2R compile. - RtlRestoreContext clears t_gcModeSwitchPermitted only when the frame the catch will resume into belongs to an image carrying the header flag. The CONTEXT at that point holds a resume case index rather than a code address, so the resume target is identified in CallCatchFunclet, where the handler frame's virtual IP is still available, and recorded in a thread-local for RtlRestoreContext to consume. - The switch is enabled for System.Private.CoreLib in Debug and Checked builds, and by default for RunCrossGen2 test runs. R2R minor version is bumped to 2 for the new header flag. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
UnwindAndContinueRethrowHelperAfterCatch and the PAL_SEHException overload of DispatchManagedException both sit on the native stack between the throw and the managed catch continuation, so their RAII GC mode holders run while the restore-context unwind passes through and resume managed code in the wrong mode. Convert both to GCX_COOP_REGION_BEGIN/END. Both are noreturn, so the region ends in UNREACHABLE(). Also restrict the region macros to their explicit try/catch form on wasm and use the plain holder elsewhere, and fix the non-wasm GCX_PREEMP_REGION_BEGIN/END definitions, which were a copy of the COOP ones and left the PREEMP macros undefined off wasm. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
It was a local development aid for clearing stale IL-CG2 directories between crossgen2 test runs and does not belong in the product tree. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The check exists only to fire asserts, so a release runtime has no reason to carry it. Gate t_gcModeSwitchPermitted, its two stores, and the resume target lookup on _DEBUG; ResumeTargetVerifiesGCModeTransitions becomes an inline returning false everywhere else. Flow the lookup's result through normal control flow rather than a second thread local. CallCatchFunclet already has to ask the question early, because the context handed to the resume path stores a resume case index rather than a code address, but it can hold the answer in a local and apply it immediately before calling ResumeAfterCatch. Everything between that call and the resumption point is MODE_ANY and transitions nothing, so the window is unchanged, and RtlRestoreContext, ICodeManager::ResumeAfterCatch and ClrRestoreNonvolatileContext all keep their original shape. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tagging @dotnet/jit-contrib for JIT-EE GUID update |
|
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. |
|
Tagging subscribers to this area: @dotnet/crossgen-contrib |
There was a problem hiding this comment.
🔵 Needs a closer look
It spans CoreCLR VM + JIT + ReadyToRun format/versioning and wasm EH/GC invariants, and needs careful maintainer validation beyond automated review.
Pull request overview
This PR addresses a WebAssembly-specific correctness issue where native unwinding during catch resumption can unintentionally flip a thread’s GC mode (due to RAII GC mode holders running in native cleanups), causing managed code to resume in the wrong mode. It introduces explicit GC-mode “region” macros for affected native frames and adds debug-only diagnostics that detect illegal GC-mode transitions during the restore-context unwind window, gated by an opt-in R2R flag emitted by crossgen2.
Changes:
- Add
GCX_*_REGION_BEGIN/ENDmacros and convert key VM call boundaries to use explicit GC-mode regions on wasm to avoid destructor-triggered transitions during foreign-tag unwinds. - Add debug-only “GC mode transition permitted” tracking and a new
CORINFO_HELP_JIT_RESUME_AFTER_CATCHhelper emitted at wasm catch-resumption points when an image opts in. - Add crossgen2 plumbing (
--verify-gc-mode-transitions) and bump ReadyToRun minor version to record/propagate the new diagnostic flag and helper.
File summaries
| File | Description |
|---|---|
| src/tests/Common/CLRTest.CrossGen.targets | Pass --verify-gc-mode-transitions in RunCrossGen2 test runs. |
| src/coreclr/vm/wasm/helpers.cpp | Add resume-target flag check + wasm helper to re-permit GC transitions after catch resumption; assert on forbidden transitions in key wasm helpers. |
| src/coreclr/vm/util.hpp | Introduce GCX_*_REGION_BEGIN/END macros with wasm-specific try/catch behavior to avoid foreign-tag cleanup side effects. |
| src/coreclr/vm/threads.h | Add debug-only t_gcModeSwitchPermitted + assertion macro; declare ResumeTargetVerifiesGCModeTransitions. |
| src/coreclr/vm/threads.cpp | Define debug-only t_gcModeSwitchPermitted defaulting to true. |
| src/coreclr/vm/reflectioninvocation.cpp | Convert a key QCall site to GCX_COOP_REGION_*. |
| src/coreclr/vm/readytoruninfo.h | Expose ReadyToRunInfo::VerifiesGCModeTransitions() based on new R2R header flag. |
| src/coreclr/vm/prestub.cpp | Convert interpreter boundary GC mode scopes to region macros. |
| src/coreclr/vm/jithelpers.cpp | Add/declare JIT_ResumeAfterCatch helper stub for non-wasm and declaration for wasm. |
| src/coreclr/vm/interpexec.cpp | Convert multiple interpreter managed/native transition points to GCX_PREEMP_REGION_*. |
| src/coreclr/vm/exceptionhandling.cpp | Use GCX_COOP_REGION_* in DispatchManagedException; clear/restore GC-mode-switch permission around ResumeAfterCatch based on resume target. |
| src/coreclr/vm/excep.cpp | Convert to GCX_COOP_REGION_* in rethrow helper after catch. |
| src/coreclr/vm/customattribute.cpp | Convert QCall and preemptive transition region(s) to region macros. |
| src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs | Add CORJIT_FLAG_VERIFY_GC_MODE_TRANSITIONS. |
| src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs | Add CORINFO_HELP_JIT_RESUME_AFTER_CATCH. |
| src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs | Add new R2R flag + helper enum value. |
| src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs | Bump R2R minor version constant to 2. |
| src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/ReadyToRunSignature.cs | Pretty-print the new helper in signatures. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs | Map new CORINFO helper to new R2R helper id. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilationBuilder.cs | Plumb the new verify option into JIT flags and R2R header flags. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs | Propagate the new verification flag into rewritten component outputs. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ReadyToRunHeaderNode.cs | Expose header flags for downstream propagation. |
| src/coreclr/tools/aot/crossgen2/Properties/Resources.resx | Add option text for --verify-gc-mode-transitions. |
| src/coreclr/tools/aot/crossgen2/Program.cs | Wire --verify-gc-mode-transitions into the compilation builder. |
| src/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs | Define and register --verify-gc-mode-transitions option. |
| src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h | Bump R2R minor version constant to 2 for NativeAOT compatibility. |
| src/coreclr/jit/jitee.h | Add JIT flag mapping for VERIFY_GC_MODE_TRANSITIONS. |
| src/coreclr/jit/fgwasm.cpp | Emit CORINFO_HELP_JIT_RESUME_AFTER_CATCH at catch resumption blocks when opted in. |
| src/coreclr/inc/readytorunhelpers.h | Add READYTORUN helper mapping for ResumeAfterCatch. |
| src/coreclr/inc/readytorun.h | Bump R2R minor version, add new flag bit, add new helper enum value. |
| src/coreclr/inc/jithelpers.h | Add JIT helper table entry for CORINFO_HELP_JIT_RESUME_AFTER_CATCH. |
| src/coreclr/inc/jiteeversionguid.h | Update the JIT-EE version GUID due to interface changes. |
| src/coreclr/inc/corjitflags.h | Add CORJIT_FLAG_VERIFY_GC_MODE_TRANSITIONS. |
| src/coreclr/inc/corinfo.h | Add CORINFO_HELP_JIT_RESUME_AFTER_CATCH. |
| src/coreclr/crossgen-corelib.proj | Enable verification scaffolding for CoreLib crossgen in Debug/Checked builds. |
Review details
Suppressed comments (2)
src/coreclr/vm/util.hpp:231
- On TARGET_WASM, GCX_PREEMP_REGION_* doesn't introduce a scope, unlike the non-WASM expansion. This can lead to platform-specific
__gcHolderredeclaration failures when multiple regions are used in a single scope and makes the macro behavior inconsistent across platforms. Adding an outer{ ... }scope would align the WASM and non-WASM definitions.
src/coreclr/vm/util.hpp:267 - On TARGET_WASM, GCX_MAYBE_COOP_REGION_* doesn't introduce a scope, while the non-WASM version does. This inconsistency can create
__gcHolderredeclaration errors on wasm in patterns that are valid off-wasm (multiple regions in the same scope). Consider adding an outer{ ... }scope to the WASM definition to match the non-WASM behavior.
- Files reviewed: 35/35 changed files
- Comments generated: 2
- Review effort level: Lite
| #define GCX_COOP_REGION_BEGIN() GCX_COOP_NO_DTOR(); try { do {} while (0) | ||
| #define GCX_COOP_REGION_END() } catch (...) { GCX_COOP_NO_DTOR_END(); throw; } GCX_COOP_NO_DTOR_END(); do {} while (0) |
|
I'm not sure the assert is worth all the infra I needed to make it work, but it WAS useful for verifying that I've fixed the issues I found. |
| // tag. When this image is compiled with GC mode transition verification, RtlRestoreContext | ||
| // forbids GC mode transitions for the duration of that native unwind; managed code is about | ||
| // to run again here, so re-permit them. | ||
| GenTree* resumeAfterCatch = gtNewHelperCallNode(CORINFO_HELP_JIT_RESUME_AFTER_CATCH, TYP_VOID); |
There was a problem hiding this comment.
Could you also classify CORINFO_HELP_JIT_RESUME_AFTER_CATCH in HelperCallProperties::init()? It currently defaults to a potentially throwing, heap-mutating helper, but its implementation only updates debug TLS state. Adding it to the ExceptionSetFlags::None group beside CORINFO_HELP_JIT_PINVOKE_END should match its actual behavior.
Note
This comment was generated with GitHub Copilot.
Fixes #133219
The problem
On wasm there is no way to restore a register context, so
RtlRestoreContextresumes managedcode at a catch continuation by throwing a native WebAssembly exception tag and letting it
propagate up to the frame that will resume. Everything between the throw and the resumption
point gets unwound by the native EH machinery on the way there.
That interacts badly with RAII GC mode holders. With
-fwasm-exceptions, clang lowers a C++cleanup (a destructor on an unwind edge) to
catch_all, andcatch_allintercepts foreigntags — including ours. So every
GCX_COOP/GCX_PREEMPholder sitting on a native framebetween the throw and the resumption point runs its destructor and flips the thread's GC mode
on the way past. Managed code then resumes in the wrong mode.
The visible symptom is the assert in #133219:
which fires later, at an arbitrary point after the resumption, once managed code touches an
object reference while the thread is preemptive. The distance between the corruption and the
report is what makes this expensive to diagnose.
Note that
catch (...)does not have this problem: clang lowers it to a catch of the C++tag only, so a foreign tag propagates straight past it. That asymmetry is what the fix is
built on.
What this does
1. Explicit GC mode regions instead of RAII holders
New macro pairs in
vm/util.hpp:On wasm these expand to the
_NO_DTORholder wrapped in an explicittry/catch (...).A real C++ exception is caught, the mode is restored, and the exception is rethrown — the same
observable behavior as the destructor. The restore-context tag is not caught, so it passes
through with the mode left alone, which is exactly what the resuming managed code needs.
Off wasm they expand to the ordinary holder in a nested scope, so there is no codegen change
on any other platform.
return,break,continue, orgotothatescapes the region skips the
_ENDmacro and leaks the transition. Every converted site waschecked for this, and a few needed a result variable hoisted above the
_BEGINso controlfalls out of the region normally. The macros are commented accordingly.
Converted sites — all of them native frames that can sit between a throw and a managed catch
resumption:
reflectioninvocation.cpp—RuntimeMethodHandle_InvokeMethodinterpexec.cpp— 11 sites around the interpreter's managed call boundariesprestub.cpp— the holder spanningInterpExecMethod, and the neighbouringGCX_PREEMPcustomattribute.cpp—CustomAttribute_CreateCustomAttributeInstance, which reachesCallDescrWorkerthroughMethodDescCallSite::CallWithValueTypesexcep.cpp—UnwindAndContinueRethrowHelperAfterCatchexceptionhandling.cpp—DispatchManagedException(PAL_SEHException&, bool)2. Diagnostic scaffolding so the next one of these fails at the scene
Fixing the known sites doesn't help with the ones nobody has found yet, and the failure mode
is a delayed, unrelated-looking assert. So the illegal transition now fails where it happens.
A
thread_local t_gcModeSwitchPermittedis cleared for the duration of the resume and set againby a new
CORINFO_HELP_JIT_RESUME_AFTER_CATCHhelper emitted into theBBF_CATCH_RESUMPTIONblock. Every site that changes a thread's GC mode asserts on it:
Thread::DisablePreemptiveGC,Thread::EnablePreemptiveGC, the four reverse-P/Invoke helpers that hand-inline the transition,and the wasm P/Invoke and GC-poll helpers.
Only asserts consume the flag, so the whole mechanism is
_DEBUG-only — the variable, its twostores, and the resume-target lookup all compile away, and a release runtime carries none of it.
Identifying the resume target is not as direct as it looks. The
CONTEXThanded to the resumepath carries the catch funclet's return value in its IP slot, which on wasm is a dispatcher
case index, not a code address —
GetIPreturns 0 there. The lookup therefore happens inCallCatchFunclet, where the handler frame's control PC is still available, and the answer isheld in a local and applied immediately before the
ResumeAfterCatchcall. Everything betweenthat point and the resumption is
MODE_ANYand transitions nothing, so the restriction stillcovers the whole window without
RtlRestoreContext,ICodeManager::ResumeAfterCatchorClrRestoreNonvolatileContextchanging shape.3. The scaffolding is opt-in
CORINFO_HELP_JIT_RESUME_AFTER_CATCHis pure overhead in a shipping image, and the flag mustonly be cleared when the code being resumed into will actually set it again — otherwise it
stays clear for the rest of the thread's life and the assert misfires on the next legitimate
transition.
So crossgen2 gains
--verify-gc-mode-transitions, which passesCORJIT_FLAG_VERIFY_GC_MODE_TRANSITIONSto the JIT and stampsREADYTORUN_FLAG_VERIFY_GC_MODE_TRANSITIONSinto the R2R header. The JIT emits the helper onlyunder that flag, and the VM clears
t_gcModeSwitchPermittedonly when the resume target belongsto an image carrying it. The switch is enabled for System.Private.CoreLib in Debug and Checked
builds, and by default for
RunCrossGen2test runs. R2R minor version is bumped to 2.Validation
JIT/optunderbrowser-wasm+RunCrossGen2, Checked runtime:The one remaining failure is
JIT/opt/Regressions/Regression4, an unrelatedLoadFromAssemblyPath("")→ArgumentExceptionthat fails identically without this change.The scaffolding was confirmed to be live rather than dead: reverting just the
reflectioninvocation.cppconversion reproduces an abort at the transition site(
threads.h:1297, "GC mode transition while a restore-context unwind is in progress") insteadof the original delayed OBJECTREF assert. Restoring it returns the run to 402/1/0.
crossgen2 side verified independently: with the switch,
compares.dllshows 35CORINFO_HELP_JIT_RESUME_AFTER_CATCHsites and R2R header flags0x0000104b; without it, 0sites and
0x0000004b.Builds clean on
browser-wasmandlinux-x64, in both Checked and Release runtimeconfigurations — Release is what exercises the compiled-away form of the scaffolding.
Scope
#133219 lists 17 failing browser-wasm R2R work items. Only
JIT.optwas failingon the OBJECTREF assert; I ran the other 16 and confirmed their failures are unrelated
(SIGSEGV,
null function or function signature mismatch,RuntimeError: unreachable, the1000-parameter limit, an EH stackwalk assert, and a crossgen2
lower.cppassert). This changeneither fixes nor regresses any of them — that classification is posted on the issue.