Remove ExInfo::m_hThrowable — use direct pointer for exception objects#127300
Remove ExInfo::m_hThrowable — use direct pointer for exception objects#127300max-charlamb wants to merge 1 commit 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. |
| TargetPointer IThread.GetCurrentExceptionHandle(TargetPointer threadPointer) | ||
| { | ||
| var (_, exceptionInfo) = GetThreadExceptionInfo(threadPointer); | ||
|
|
||
| if (exceptionInfo == null) | ||
| return TargetPointer.Null; | ||
|
|
||
| if (exceptionInfo.ThrownObjectHandle.Handle == TargetPointer.Null || exceptionInfo.ThrownObjectHandle.Object == TargetPointer.Null) | ||
| if (exceptionInfo.ThrownObject == TargetPointer.Null) | ||
| return TargetPointer.Null; | ||
|
|
||
| return exceptionInfo.ThrownObjectHandle.Handle; | ||
| return exceptionInfo.ThrownObject; | ||
| } |
There was a problem hiding this comment.
GetCurrentExceptionHandle now returns the exception object pointer (ExceptionInfo.ThrownObject). Several existing cDAC legacy COM wrappers (e.g., Microsoft.Diagnostics.DataContractReader.Legacy.ClrDataTask.GetCurrentExceptionState) treat this value as an OBJECTHANDLE-like address and dereference it via Target.ReadPointer; returning an object address will cause them to read the method table pointer instead of the exception object. To preserve compatibility, either return a pseudo-handle address (exceptionInfoAddr + field offset of ThrownObject) or update all legacy consumers to treat this value as a direct object pointer (and stop dereferencing).
| TargetPointer IException.GetNestedExceptionInfo(TargetPointer exceptionInfoAddr, out TargetPointer nextNestedExceptionInfo, out TargetPointer thrownObjectHandle) | ||
| { | ||
| Data.ExceptionInfo exceptionInfo = _target.ProcessedData.GetOrAdd<Data.ExceptionInfo>(exceptionInfoAddr); | ||
| nextNestedExceptionInfo = exceptionInfo.PreviousNestedInfo; | ||
| thrownObjectHandle = exceptionInfo.ThrownObjectHandle.Handle; | ||
| return exceptionInfo.ThrownObjectHandle.Object; | ||
| // ThrownObject is now a direct object pointer, not a handle. | ||
| // For backward compatibility, thrownObjectHandle out-param returns TargetPointer.Null | ||
| // since there is no handle. The return value is the object pointer itself. | ||
| thrownObjectHandle = TargetPointer.Null; | ||
| return exceptionInfo.ThrownObject; |
There was a problem hiding this comment.
GetNestedExceptionInfo sets thrownObjectHandle to TargetPointer.Null for “backward compatibility”, but existing consumers of the out-param (e.g., the cDAC Legacy ClrDataExceptionState implementation) expect a dereferenceable handle address to fetch the exception object. Consider returning a pseudo-handle (exceptionInfoAddr + offset of the ThrownObject field) in thrownObjectHandle and returning the object pointer as the return value, so legacy code can continue to dereference the handle while newer code can use the direct return value.
| TADDR objAddr = dac_cast<TADDR>(m_exception); | ||
| DacEnumMemoryRegion(objAddr, sizeof(Object)); |
There was a problem hiding this comment.
In DAC builds this change enumerates only sizeof(Object) at the exception address. Previously OBJECTHANDLE_EnumMemoryRegions would also call obj->EnumMemoryRegions(), ensuring the full exception object graph is present in triage/mini dumps for SOS/debugging. Consider switching to OBJECTREF_EnumMemoryRegions(m_exception) (or equivalent) so the entire exception object is enumerated rather than just its header.
| TADDR objAddr = dac_cast<TADDR>(m_exception); | |
| DacEnumMemoryRegion(objAddr, sizeof(Object)); | |
| OBJECTREF_EnumMemoryRegions(m_exception); |
| { | ||
| WRAPPER_NO_CONTRACT; | ||
| return IsHandleNullUnchecked(m_ExceptionState.GetThrowableAsHandle()); | ||
| return !m_ExceptionState.IsExceptionInProgress(); |
There was a problem hiding this comment.
IsThrowableNull now returns !m_ExceptionState.IsExceptionInProgress(), which only checks whether a tracker exists, not whether the current tracker actually has a non-null throwable. This changes semantics for callers that used IsThrowableNull/HasException as a proxy for “is there a current exception object” (previously it checked handle/object nullness) and can return false even when the current tracker’s throwable has been cleared. Consider implementing an unchecked null test against the current tracker’s m_exception field (similar to the previous handle-based check) so IsThrowableNull reflects whether a throwable is present.
| return !m_ExceptionState.IsExceptionInProgress(); | |
| return m_ExceptionState.GetThrowable() == NULL; |
db309be to
e6688b3
Compare
e6688b3 to
2812642
Compare
2812642 to
9d54eec
Compare
| { | ||
| thread = threadBuilder.AddThread(1, 1234); | ||
| exceptionInfo = threadBuilder.GetExceptionInfo(thread); | ||
| TargetTestHelpers helpers = threadBuilder.Builder.TargetTestHelpers; | ||
| MockMemorySpace.BumpAllocator allocator = threadBuilder.Builder.CreateAllocator(0x1_0000, 0x2_0000); | ||
| MockMemorySpace.HeapFragment handleFragment = allocator.Allocate((ulong)helpers.PointerSize, "ThrownObjectHandle"); | ||
| helpers.WritePointer(handleFragment.Data, TargetPointer.Null); | ||
| exceptionInfo!.ThrownObjectHandle = handleFragment.Address; | ||
| exceptionInfo!.ThrownObject = 0; | ||
| }); |
There was a problem hiding this comment.
This test case now sets exceptionInfo.ThrownObject = 0 to represent a null exception. If GetCurrentExceptionHandle continues to return a handle/slot address, the null scenario should be represented by a non-null slot whose contents are null (or by LastThrownObjectHandle being null), matching how consumers dereference handles.
| // ThrownObject is a direct object pointer stored in ExInfo::m_exception. | ||
| // Return the address of the field as a "handle" — reading through it yields the | ||
| // exception Object*. This has the same lifetime as the ExInfo (both are invalidated | ||
| // when PopExInfos calls ReleaseResources). See dacimpl.h for the equivalent native | ||
| // DAC documentation. |
There was a problem hiding this comment.
IException.GetNestedExceptionInfo sets thrownObjectHandle to TargetPointer.Null, but downstream legacy shims (e.g., ClrDataExceptionState) treat this out-param as an address to dereference to obtain the exception object. Returning null here will break nested exception iteration (GetPrevious/GetString/etc.). Please keep providing a valid "handle-slot" address for backward compatibility, e.g., the target address of the ExceptionInfo.ThrownObject field (a slot containing an Object*), even though the return value is now the direct object pointer.
| thread = threadBuilder.AddThread(1, 1234); | ||
| exceptionInfo = threadBuilder.GetExceptionInfo(thread); | ||
| TargetTestHelpers helpers = threadBuilder.Builder.TargetTestHelpers; | ||
| MockMemorySpace.BumpAllocator allocator = threadBuilder.Builder.CreateAllocator(0x1_0000, 0x2_0000); | ||
| MockMemorySpace.HeapFragment handleFragment = allocator.Allocate((ulong)helpers.PointerSize, "ThrownObjectHandle"); | ||
| helpers.WritePointer(handleFragment.Data, expectedObject); | ||
| exceptionInfo!.ThrownObjectHandle = handleFragment.Address; | ||
| exceptionInfo!.ThrownObject = (ulong)expectedObject; | ||
| }); | ||
|
|
||
| IThread contract = target.Contracts.Thread; | ||
| TargetPointer thrownObjectHandle = contract.GetCurrentExceptionHandle(new TargetPointer(thread!.Address)); | ||
| Assert.Equal(new TargetPointer(exceptionInfo!.ThrownObjectHandle), thrownObjectHandle); | ||
| Assert.Equal(new TargetPointer(exceptionInfo!.ThrownObject), thrownObjectHandle); |
There was a problem hiding this comment.
This test now sets exceptionInfo.ThrownObject to the exception object address and asserts GetCurrentExceptionHandle returns that value. However, GetCurrentExceptionHandle is expected to return a handle-slot address (something callers can dereference to obtain the exception object), not the object address itself. If the contract continues to return a handle/slot, the test should allocate a slot, write expectedObject into it, and assert the returned pointer equals the slot address.
9d54eec to
e761d3e
Compare
e761d3e to
34f7963
Compare
| // The exception object is stored directly in ExInfo::m_exception by managed EH code. | ||
| // SetThrowable is now a no-op for the ExInfo field — the m_exception is already set. | ||
| // We keep this method so that SafeSetThrowables can still call it for the assertion | ||
| // and debug checks above. |
There was a problem hiding this comment.
ThreadExceptionState::SetThrowable is now effectively a no-op and never updates/clears the current ExInfo’s m_exception. This breaks callers that rely on SetThrowable to actually change the current throwable (e.g., Thread::SafeSetThrowables catch path that intends to replace both current throwable and LTO with the preallocated OOM exception on handle-allocation failure) and also means SafeSetThrowables(NULL) no longer clears the current tracker’s throwable, potentially keeping a stale exception OBJECTREF rooted longer than intended. Consider setting m_pCurrentTracker->m_exception = throwable when m_pCurrentTracker != NULL (and clearing it when throwable == NULL), while still allowing the fatal-SO “current tracker may be null” case.
| // The exception object is stored directly in ExInfo::m_exception by managed EH code. | |
| // SetThrowable is now a no-op for the ExInfo field — the m_exception is already set. | |
| // We keep this method so that SafeSetThrowables can still call it for the assertion | |
| // and debug checks above. | |
| if (m_pCurrentTracker != NULL) | |
| { | |
| m_pCurrentTracker->m_exception = throwable; | |
| } |
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
| PTR_ExInfo pExInfo = pThread->GetExceptionState()->GetCurrentExceptionTracker(); | ||
| while (pExInfo != NULL) | ||
| { | ||
| PTR_PTR_Object pRef = dac_cast<PTR_PTR_Object>(&pExInfo->m_exception); | ||
| fn(pRef, sc, 0); | ||
| pExInfo = pExInfo->GetPreviousExceptionTracker(); |
There was a problem hiding this comment.
ScanStackRoots reports ExInfo::m_exception as a GC root using fn(..., sc, 0). Most other explicit object-root reports in this file use CHECK_APP_DOMAIN, and omitting it can change GC root reporting semantics (especially for domain/loader allocator checks). Consider passing CHECK_APP_DOMAIN (or the appropriate flag set) when promoting the m_exception slot.
| #endif // _DEBUG | ||
|
|
||
| StackTraceInfo::AppendElement(pExInfo->m_hThrowable, ip, sp, pMD, &pExInfo->m_frameIter.m_crawl); | ||
| StackTraceInfo::AppendElement((OBJECTHANDLE)&pExInfo->m_exception, ip, sp, pMD, &pExInfo->m_frameIter.m_crawl); |
There was a problem hiding this comment.
These calls pass the address of ExInfo::m_exception cast to OBJECTHANDLE. Since this is not a real GC handle-table handle (it's an OBJECTREF slot on the stack), it would be easy for future readers to misinterpret and accidentally treat it like a handle-table entry. Consider adding a brief comment or using a named helper/cast to make the "OBJECTREF slot as pseudo-handle" intent explicit at the call site.
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 (64-bit)ScanStackRoots(gcenv.ee.cpp), mirroring NativeAOT'sGcScanRootsWorker-- this keeps superseded exception objects alive without handlesGetThrowableAsHandle()entirely -- all callers migrated to useGetThrowable()(OBJECTREF) orm_LastThrownObjectHandle(real handle on Thread)StackTraceInfo::AppendElementOBJECTREF overload to preserve foreign-exception semantics and preallocated-exception checksClrDataExceptionState::m_throwableusesm_LastThrownObjectHandle(real handle) for current exception, and target address ofm_exceptionfor nested exception iterationThrownObjectHandle->ThrownObject(direct pointer, no handle dereference)What 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).Testing