Remove ExInfo::m_hThrowable - use direct pointer for exception objects#127300
Remove ExInfo::m_hThrowable - use direct pointer for exception objects#127300max-charlamb merged 15 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes ExInfo::m_hThrowable (GC-handle indirection) and standardizes on the existing direct OBJECTREF m_exception path, aligning CoreCLR with NativeAOT and updating DAC/cDAC consumers accordingly.
Changes:
- Remove
m_hThrowableusage and migrate exception-object reads tom_exceptionacross EH, interop propagation, debugger/DAC paths. - Add explicit GC root scanning for the
ExInfochain to keep superseded exception objects alive without handles. - Update cDAC contracts/tests and DAC exception state plumbing to reflect the new representation.
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/native/managed/cdac/tests/ThreadTests.cs | Updates tests to use the new thrown-object representation. |
| src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Thread.cs | Updates mock ExceptionInfo layout to expose ThrownObject instead of ThrownObjectHandle. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/ExceptionInfo.cs | Reads ThrownObject as a pointer field instead of a handle wrapper. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Thread_1.cs | Returns current exception “handle” using ThrownObject and updates Watson bucket lookup accordingly. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Exception_1.cs | Updates nested exception info to return direct thrown object pointer. |
| src/coreclr/vm/threads.h | Removes handle-based throwable accessors and adjusts HasException/IsThrowableNull logic. |
| src/coreclr/vm/threads.cpp | Updates last-thrown synchronization to use OBJECTREF throwable. |
| src/coreclr/vm/interoplibinterface_shared.cpp | Changes propagating-exception callback signature/GC mode to take OBJECTREF. |
| src/coreclr/vm/interoplibinterface_objc.cpp | Switches ObjC propagation callback to accept OBJECTREF and removes handle dereference. |
| src/coreclr/vm/interoplibinterface.h | Updates declarations to match OBJECTREF callback signatures. |
| src/coreclr/vm/gcenv.ee.cpp | Adds GC root scanning of ExInfo chain for direct exception object references. |
| src/coreclr/vm/exstate.h | Removes GetThrowableAsHandle from ThreadExceptionState. |
| src/coreclr/vm/exstate.cpp | Removes handle-based throwable retrieval; GetThrowable returns m_exception. |
| src/coreclr/vm/exinfo.h | Removes m_hThrowable and returns m_exception directly from GetThrowable(). |
| src/coreclr/vm/exinfo.cpp | Drops handle lifecycle management; clears m_exception during resource release. |
| src/coreclr/vm/exceptionhandling.cpp | Updates DAC memory enumeration and stacktrace append paths to use m_exception. |
| src/coreclr/vm/excep.cpp | Updates stacktrace appending to preserve foreign/preallocated semantics with OBJECTREF. |
| src/coreclr/vm/eedbginterfaceimpl.cpp | Switches debugger exception retrieval logic to rely on m_LastThrownObjectHandle. |
| src/coreclr/vm/datadescriptor/datadescriptor.inc | Updates cDAC descriptor field to ThrownObject at offsetof(ExInfo, m_exception). |
| src/coreclr/debug/ee/debugger.cpp | Uses m_LastThrownObjectHandle for force-catch-handler lookup. |
| src/coreclr/debug/daccess/task.cpp | Changes ClrDataExceptionState::m_throwable type to TADDR and passes &m_exception. |
| src/coreclr/debug/daccess/request.cpp | Reads exception object directly from m_exception and updates Watson bucket retrieval. |
| src/coreclr/debug/daccess/dacimpl.h | Updates ClrDataExceptionState signature/storage for TADDR throwable. |
| src/coreclr/debug/daccess/dacdbiimpl.cpp | Switches “current exception” debugger handle to m_LastThrownObjectHandle. |
| src/coreclr/System.Private.CoreLib/src/System/Runtime/ExceptionServices/AsmOffsets.cs | Updates managed EH asm offsets to reflect the new ExInfo layout. |
db309be to
e6688b3
Compare
e6688b3 to
2812642
Compare
2812642 to
9d54eec
Compare
9d54eec to
e761d3e
Compare
e761d3e to
34f7963
Compare
34f7963 to
adffa37
Compare
Replace the GCHandle-based m_hThrowable field in ExInfo with direct use of the existing m_exception OBJECTREF field, matching NativeAOT's approach. Key changes: - Remove OBJECTHANDLE m_hThrowable from ExInfo, saving 8 bytes (64-bit) - Update AsmOffsets constants for the new field layout - Add GC root scanning of ExInfo chain in ScanStackRoots (gcenv.ee.cpp), mirroring NativeAOT's GcScanRootsWorker pattern - Simplify GetThrowable() to return m_exception directly - SetThrowable() no longer creates GC handles for ExInfo - Remove GetThrowableAsHandle() entirely — all callers migrated to use GetThrowable() (OBJECTREF) or m_LastThrownObjectHandle (real handle) - Update StackTraceInfo::AppendElement OBJECTREF overload to preserve foreign-exception semantics and preallocated exception checks - Update Interop propagation callback to take OBJECTREF instead of handle - Update DAC code (request.cpp, task.cpp, dacdbiimpl.cpp) to use m_exception directly - Update debugger code (eedbginterfaceimpl.cpp, debugger.cpp) to use m_LastThrownObjectHandle for handle-based APIs - Update cDAC: ThrownObjectHandle -> ThrownObject (direct pointer) - Update cDAC contracts, data classes, and tests This eliminates ~5 interlocked handle alloc/destroy ops per exception throw, removes OOM fallback paths, and unblocks cDAC unification. Thread::m_LastThrownObjectHandle remains as-is (separate work item). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
adffa37 to
7aae554
Compare
The ExInfo::m_exception field was being reported to the GC twice: once via GCPROTECT_BEGIN and once via ExInfo chain scanning in ScanStackRoots. The CLR code guide (section 2.1.5) explicitly states that reporting the same location twice corrupts the GC's relocation logic. Remove the GCPROTECT_BEGIN/END for m_exception and rely solely on chain scanning (matching NativeAOT's model). Add Thread::ObjectRefProtected calls in checked builds to satisfy the debug OBJECTREF tracking table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Results for
|
| Method | Toolchain | Mean | Error | Ratio |
|---|---|---|---|---|
| ThrowCatch | PR #127300 | 680.8 ns | 9.82 ns | 1.00 |
| ThrowCatch | main | 715.2 ns | 4.02 ns | 1.05 |
| NestedCatchThrowNew | PR #127300 | 12,216.4 ns | 119.83 ns | 1.00 |
| NestedCatchThrowNew | main | 12,446.6 ns | 112.60 ns | 1.02 |
|
@max-charlamb, unless you already did, can you also please run the private diagnostic tests before merging this PR? |
AppendElementImpl had only one caller after removing the OBJECTHANDLE overload. Inline it into AppendElement and remove the separate function. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
This comment has been minimized.
This comment has been minimized.
Verified on the private diagnostics suite |
dac_cast<TADDR>(&m_exception) returns the host address in DAC builds, but we need the target address. PTR_HOST_MEMBER_TADDR correctly computes the target address of the field. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Note This review was generated by Copilot. 🤖 Copilot Code Review — PR #127300Holistic AssessmentMotivation: The PR removes the GC handle indirection ( Approach: The approach is well-reasoned. GC reporting is moved to an explicit scan of the ExInfo chain in Summary: Detailed Findings✅ GC Correctness — ExInfo chain scanning is soundThe new scanning loop in
✅ Cleanup Completeness — No leftover referencesVerified: no remaining references to
|
The ExInfo::m_exception slot is GC-reported via the ExInfo chain scanner in ScanStackRoots (gcenv.ee.cpp). Reporting the same OBJECTREF location to the GC twice (once via GCPROTECT, once via the chain scanner) corrupts the relocate phase: the slot gets relocated twice, producing a garbage pointer that AVs on the next read. This was missed in the earlier conversion of the three sites in exceptionhandling.cpp. It was reproducible on x86 checked builds running JitTest_throw_SEH, where SEH (hardware faults) routes through HandleManagedFault. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/ba-g Test failures are unrelated to PR changes |
Two changes responding to PR review feedback: 1. Remove dead CEEInfo::HandleException. The function has been unreachable since 2016 (commit 4d9f4b8 `Remove SEH interactions between the JIT and the EE'') which replaced the old ICorJitInfo::FilterException/HandleException pair with runWithErrorTrap. The function is private, non-virtual, not part of the ICorJitInfo interface, and has zero callers in coreclr, the JIT, the AOT thunks, or SuperPMI (the SuperPMI Packet_HandleException slot is commented out). Removing it also retires the long-stale comment about `sync between the LTO and the exception tracker'' that pre-dates the ExInfo redesign and the lazy-LTO model from dotnet#127300/dotnet#127649. 2. Reorder declarations in threads.h so SetLastThrownObject precedes SetSOForLastThrownObject, matching the order of the definitions in threads.cpp. Also update an unrelated stale comment in ExInfo::PopExInfos: the `unmanaged thread'' rationale is incorrect because both UMThunkUnwindFrameChainHandler and CallDescrWorkerUnwindFrameChainHandler short-circuit unmanaged threads before reaching PopExInfos, and the function carries a MODE_COOPERATIVE contract. No behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…xception dispatch (#127741) After #127300 removed ExInfo::m_hThrowable and SetThrowable(), Thread::m_LastThrownObjectHandle is no longer updated during active exception dispatch. This causes a staleclastThrownObjectHandle to ge returned by GetThreadData when a debugger breaks into the target mid-dispatch. When debugging a .NET 11 process with SOS (e.g. via dotnet-dump or WinDbg), the following commands fail if the debuggee is stopped during exception dispatch (for example, with SXE CLR): - !PrintException — reports "Invalid exception object" because it dereferences the stale handle and gets a null or recycled pointer - !Threads — shows no exception column for threads that are actively throwing - !clrstack -a with exception context — may show incomplete or missing exception info
Note
This PR was authored with the assistance of GitHub Copilot.
Summary
Replace the GCHandle-based
m_hThrowablefield in ExInfo with direct use of the existingm_exceptionOBJECTREF field, matching NativeAOT's approach.Motivation
CoreCLR's ExInfo stored exception objects via two redundant fields:
OBJECTHANDLE m_hThrowable(GC handle table indirection) andOBJECTREF m_exception(direct pointer used by the new EH path shared with NativeAOT). NativeAOT only hasm_exception. The handle added allocation/deallocation overhead (~5 interlocked ops per throw) and an extra pointer indirection on every read, but none of the 15 consumers actually required OBJECTHANDLE guarantees - they all ran in cooperative GC mode and immediately dereferenced the handle.Key Changes
OBJECTHANDLE m_hThrowablefrom ExInfo, saving 8 bytes (x64) / 4 bytes (x86)ScanStackRoots(gcenv.ee.cpp), mirroring NativeAOT'sGcScanRootsWorker- this keeps superseded exception objects alive without handlesGCPROTECT_BEGIN(exInfo.m_exception)from all 3 dispatch entry points - the chain scanning already reports&m_exceptionto the GC, and reporting the same location twice corrupts the GC's relocation logic (clr-code-guide.md section 2.1.5). Debug OBJECTREF tracking is satisfied viaThread::ObjectRefProtectedin the ExInfo constructor.ExInfo::GetThrowableAsPseudoHandle()- returns the target address ofm_exceptionas a pseudo-handle (not a real GC handle table entry). UsesPTR_HOST_MEMBER_TADDRfor correct DAC target address computation. The slot is updated during GC by the ExInfo chain scanner.GetThrowableAsHandle()- replaced byGetThrowableAsPseudoHandle()on ExInfo, ThreadExceptionState, and ThreadSetThrowable()entirely - managed EH code writesm_exceptiondirectly; theSetThrowableErrorCheckingenum andSTEC_*constants are also removed.GetMyThread()- dead function with zero callersAppendElementImplintoAppendElement- only one caller remained after removing the OBJECTHANDLE overload. The merged function handles foreign exception semantics and preallocated exception checks.AppendElement- all callers now pass OBJECTREF directly (including prestub.cpp and threads.cpp viaObjectFromHandle)StackTraceInfo::AppendElementpreallocated-exception check: changed fromIsPreallocatedExceptionHandletoIsPreallocatedExceptionObject(ObjectFromHandle(...))GetCurrentExceptionreads from ExInfo first viaGetThrowableAsPseudoHandle(), falls back tom_LastThrownObjectHandle.GetThreadExceptionuses same pattern.ThrownObjectHandle->ThrownObject(direct pointer, no handle dereference);GetCurrentExceptionHandlereturns field address as pseudo-handle for backward compatibilityWhat stays unchanged
Thread::m_LastThrownObjectHandleremains as an OBJECTHANDLE - it is required by the ICorDebug managed debugging protocol (SendExceptionHelperAndBlockisMODE_ANY, right-side debugger reads through handle cross-process viaBuildFromGCHandle).Efficiency
Per exception throw, this eliminates:
The only added cost is ~2 pointer reads per thread per GC for ExInfo chain walking - negligible since exception chains are almost always 1-2 nodes deep.
Testing