From 7aae5541d3e54ee3eab1803fa8b6b6275fa00959 Mon Sep 17 00:00:00 2001 From: Max Charlamb <44248479+max-charlamb@users.noreply.github.com> Date: Tue, 21 Apr 2026 16:24:51 -0400 Subject: [PATCH 1/3] =?UTF-8?q?Remove=20ExInfo::m=5FhThrowable=20=E2=80=94?= =?UTF-8?q?=20use=20direct=20pointer=20for=20exception=20objects?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- .../Runtime/ExceptionServices/AsmOffsets.cs | 24 +++--- src/coreclr/debug/daccess/dacdbiimpl.cpp | 12 +-- src/coreclr/debug/daccess/dacimpl.h | 7 ++ src/coreclr/debug/daccess/request.cpp | 47 +++++------ src/coreclr/debug/daccess/task.cpp | 10 ++- src/coreclr/debug/ee/debugger.cpp | 11 ++- .../vm/datadescriptor/datadescriptor.inc | 2 +- src/coreclr/vm/eedbginterfaceimpl.cpp | 31 ++----- src/coreclr/vm/excep.cpp | 2 +- src/coreclr/vm/exceptionhandling.cpp | 10 +-- src/coreclr/vm/exinfo.cpp | 10 +-- src/coreclr/vm/exinfo.h | 22 +---- src/coreclr/vm/exstate.cpp | 84 ++++++------------- src/coreclr/vm/exstate.h | 2 +- src/coreclr/vm/gcenv.ee.cpp | 14 ++++ src/coreclr/vm/interoplibinterface.h | 4 +- src/coreclr/vm/interoplibinterface_objc.cpp | 9 +- src/coreclr/vm/interoplibinterface_shared.cpp | 4 +- src/coreclr/vm/threads.cpp | 13 +-- src/coreclr/vm/threads.h | 13 +-- .../Contracts/Exception_1.cs | 10 ++- .../Contracts/Thread_1.cs | 26 +++--- .../Data/ExceptionInfo.cs | 4 +- .../MockDescriptors/MockDescriptors.Thread.cs | 10 +-- src/native/managed/cdac/tests/ThreadTests.cs | 16 ++-- 25 files changed, 168 insertions(+), 229 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/ExceptionServices/AsmOffsets.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/ExceptionServices/AsmOffsets.cs index 3ffb3201206c2b..23474fdb1e629d 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/ExceptionServices/AsmOffsets.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/ExceptionServices/AsmOffsets.cs @@ -189,12 +189,12 @@ class AsmOffsets public const int SIZEOF__EHEnum = 0x20; public const int OFFSETOF__StackFrameIterator__m_pRegDisplay = 0x20; public const int OFFSETOF__ExInfo__m_pPrevExInfo = 0; - public const int OFFSETOF__ExInfo__m_pExContext = 0xa8; - public const int OFFSETOF__ExInfo__m_exception = 0xb0; - public const int OFFSETOF__ExInfo__m_kind = 0xb8; - public const int OFFSETOF__ExInfo__m_passNumber = 0xb9; - public const int OFFSETOF__ExInfo__m_idxCurClause = 0xbc; - public const int OFFSETOF__ExInfo__m_frameIter = 0xc0; + public const int OFFSETOF__ExInfo__m_pExContext = 0xa0; + public const int OFFSETOF__ExInfo__m_exception = 0xa8; + public const int OFFSETOF__ExInfo__m_kind = 0xb0; + public const int OFFSETOF__ExInfo__m_passNumber = 0xb1; + public const int OFFSETOF__ExInfo__m_idxCurClause = 0xb4; + public const int OFFSETOF__ExInfo__m_frameIter = 0xb8; public const int OFFSETOF__ExInfo__m_notifyDebuggerSP = OFFSETOF__ExInfo__m_frameIter + SIZEOF__StackFrameIterator; public const int OFFSETOF__ExInfo__m_pCatchHandler = OFFSETOF__ExInfo__m_frameIter + SIZEOF__StackFrameIterator + 0x48; public const int OFFSETOF__ExInfo__m_handlingFrameSP = OFFSETOF__ExInfo__m_frameIter + SIZEOF__StackFrameIterator + 0x50; @@ -217,12 +217,12 @@ class AsmOffsets public const int SIZEOF__EHEnum = 0x10; public const int OFFSETOF__StackFrameIterator__m_pRegDisplay = 0x14; public const int OFFSETOF__ExInfo__m_pPrevExInfo = 0; - public const int OFFSETOF__ExInfo__m_pExContext = 0x5c; - public const int OFFSETOF__ExInfo__m_exception = 0x60; - public const int OFFSETOF__ExInfo__m_kind = 0x64; - public const int OFFSETOF__ExInfo__m_passNumber = 0x65; - public const int OFFSETOF__ExInfo__m_idxCurClause = 0x68; - public const int OFFSETOF__ExInfo__m_frameIter = 0x6c; + public const int OFFSETOF__ExInfo__m_pExContext = 0x58; + public const int OFFSETOF__ExInfo__m_exception = 0x5c; + public const int OFFSETOF__ExInfo__m_kind = 0x60; + public const int OFFSETOF__ExInfo__m_passNumber = 0x61; + public const int OFFSETOF__ExInfo__m_idxCurClause = 0x64; + public const int OFFSETOF__ExInfo__m_frameIter = 0x68; public const int OFFSETOF__ExInfo__m_notifyDebuggerSP = OFFSETOF__ExInfo__m_frameIter + SIZEOF__StackFrameIterator; public const int OFFSETOF__ExInfo__m_pCatchHandler = OFFSETOF__ExInfo__m_frameIter + SIZEOF__StackFrameIterator + 0x2c; public const int OFFSETOF__ExInfo__m_handlingFrameSP = OFFSETOF__ExInfo__m_frameIter + SIZEOF__StackFrameIterator + 0x30; diff --git a/src/coreclr/debug/daccess/dacdbiimpl.cpp b/src/coreclr/debug/daccess/dacdbiimpl.cpp index 163213084a3e3a..aea784926987c9 100644 --- a/src/coreclr/debug/daccess/dacdbiimpl.cpp +++ b/src/coreclr/debug/daccess/dacdbiimpl.cpp @@ -4817,9 +4817,9 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::HasUnhandledException(VMPTR_Threa else { // most managed exceptions are just a throwable bound to a - // native exception. In that case this handle will be non-null - OBJECTHANDLE ohException = pThread->GetThrowableAsHandle(); - if (ohException != (OBJECTHANDLE)NULL) + // native exception. In that case the exception will be non-null + OBJECTREF exception = pThread->GetExceptionState()->GetThrowable(); + if (exception != NULL) { // during the UEF we set the unhandled bit, if it is set the exception // was unhandled @@ -4955,8 +4955,10 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetCurrentException(VMPTR_Thread Thread * pThread = vmThread.GetDacPtr(); - // OBJECTHANDLEs are really just TADDRs. - OBJECTHANDLE ohException = pThread->GetThrowableAsHandle(); // ohException can be NULL + // Get the exception handle. The exception object is stored directly in ExInfo::m_exception, + // but debugger APIs require a handle. Use m_LastThrownObjectHandle which is kept in sync + // via SafeSetThrowables. + OBJECTHANDLE ohException = pThread->m_LastThrownObjectHandle; if (ohException == (OBJECTHANDLE)NULL) { diff --git a/src/coreclr/debug/daccess/dacimpl.h b/src/coreclr/debug/daccess/dacimpl.h index f5078caf1a46b2..8138bb0a0e30c9 100644 --- a/src/coreclr/debug/daccess/dacimpl.h +++ b/src/coreclr/debug/daccess/dacimpl.h @@ -3397,6 +3397,13 @@ class ClrDataExceptionState : public IXCLRDataExceptionState Thread* thread, ULONG32 flags, ClrDataExStateType* exInfo, + // Target address of a slot containing the exception Object*. + // This may be a GC handle table slot (e.g. m_LastThrownObjectHandle) + // or the address of ExInfo::m_exception on the target stack. + // Both are valid: the ExInfo lives on the stack which is captured + // in dumps and stable while the target thread is suspended. The + // slot has the same lifetime as the ExInfo — both become invalid + // when ExInfo::PopExInfos calls ReleaseResources. OBJECTHANDLE throwable, ClrDataExStateType* prevExInfo); virtual ~ClrDataExceptionState(void); diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index 0d5ae0fc298036..be86a3baf6a6e5 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -3308,7 +3308,7 @@ ClrDataAccess::GetNestedExceptionData(CLRDATA_ADDRESS exception, CLRDATA_ADDRESS } else { - *exceptionObject = TO_CDADDR(*PTR_TADDR(pExData->m_hThrowable)); + *exceptionObject = dac_cast(pExData->m_exception); *nextNestedException = PTR_HOST_TO_TADDR(pExData->m_pPrevNestedInfo); } @@ -4064,35 +4064,30 @@ HRESULT ClrDataAccess::GetClrWatsonBucketsWorker(Thread * pThread, GenericModeBl // By default, there are no buckets PTR_VOID pBuckets = NULL; - // Get the handle to the throwble - OBJECTHANDLE ohThrowable = pThread->GetThrowableAsHandle(); - if (ohThrowable != NULL) + // Get the current throwable + OBJECTREF oThrowable = pThread->GetExceptionState()->GetThrowable(); + if (oThrowable != NULL) { - // Get the object from handle and check if the throwable is preallocated or not - OBJECTREF oThrowable = ObjectFromHandle(ohThrowable); - if (oThrowable != NULL) + // Does the throwable have buckets? + U1ARRAYREF refWatsonBucketArray = ((EXCEPTIONREF)oThrowable)->GetWatsonBucketReference(); + if (refWatsonBucketArray != NULL) { - // Does the throwable have buckets? - U1ARRAYREF refWatsonBucketArray = ((EXCEPTIONREF)oThrowable)->GetWatsonBucketReference(); - if (refWatsonBucketArray != NULL) - { - // Get the watson buckets from the throwable for non-preallocated - // exceptions - pBuckets = dac_cast(refWatsonBucketArray->GetDataPtr()); - } - else + // Get the watson buckets from the throwable for non-preallocated + // exceptions + pBuckets = dac_cast(refWatsonBucketArray->GetDataPtr()); + } + else + { + // This is a preallocated exception object - check if the UE Watson bucket tracker + // has any bucket details + pBuckets = pThread->GetExceptionState()->GetUEWatsonBucketTracker()->RetrieveWatsonBuckets(); + if (pBuckets == NULL) { - // This is a preallocated exception object - check if the UE Watson bucket tracker - // has any bucket details - pBuckets = pThread->GetExceptionState()->GetUEWatsonBucketTracker()->RetrieveWatsonBuckets(); - if (pBuckets == NULL) + // Since the UE watson bucket tracker does not have them, look up the current + // exception tracker + if (pThread->GetExceptionState()->GetCurrentExceptionTracker() != NULL) { - // Since the UE watson bucket tracker does not have them, look up the current - // exception tracker - if (pThread->GetExceptionState()->GetCurrentExceptionTracker() != NULL) - { - pBuckets = pThread->GetExceptionState()->GetCurrentExceptionTracker()->GetWatsonBucketTracker()->RetrieveWatsonBuckets(); - } + pBuckets = pThread->GetExceptionState()->GetCurrentExceptionTracker()->GetWatsonBucketTracker()->RetrieveWatsonBuckets(); } } } diff --git a/src/coreclr/debug/daccess/task.cpp b/src/coreclr/debug/daccess/task.cpp index c8ed4df807d135..5df1c3f28ad7fd 100644 --- a/src/coreclr/debug/daccess/task.cpp +++ b/src/coreclr/debug/daccess/task.cpp @@ -4556,13 +4556,17 @@ ClrDataExceptionState::GetPrevious( { if (m_prevExInfo) { + // Pass the address of the ExInfo's m_exception field as the "handle". + // This is not a real GC handle — it is a target address of an OBJECTREF + // slot on the stack. It has the same lifetime as the ExInfo (both are + // invalidated by PopExInfos/ReleaseResources). See dacimpl.h comment. *exState = new (nothrow) ClrDataExceptionState(m_dac, m_appDomain, m_thread, CLRDATA_EXCEPTION_DEFAULT, m_prevExInfo, - m_prevExInfo->m_hThrowable, + (OBJECTHANDLE)dac_cast(&m_prevExInfo->m_exception), m_prevExInfo->m_pPrevNestedInfo); status = *exState ? S_OK : E_OUTOFMEMORY; } @@ -4920,13 +4924,15 @@ ClrDataExceptionState::NewFromThread(ClrDataAccess* dac, exState = thread->GetExceptionState()->m_pCurrentTracker; + // Pass the address of the ExInfo's m_exception field as the "handle". + // See dacimpl.h comment on the throwable parameter. exIf = new (nothrow) ClrDataExceptionState(dac, AppDomain::GetCurrentDomain(), thread, CLRDATA_EXCEPTION_DEFAULT, exState, - exState->m_hThrowable, + (OBJECTHANDLE)dac_cast(&exState->m_exception), exState->m_pPrevNestedInfo); if (!exIf) { diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index aaacecb0b2e830..255f92d97748b5 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -7867,11 +7867,14 @@ BOOL Debugger::ShouldSendCatchHandlerFound(Thread* pThread) else { BOOL forceSendCatchHandlerFound = FALSE; - OBJECTHANDLE objHandle = pThread->GetThrowableAsHandle(); - OBJECTHANDLE retrievedHandle = m_pForceCatchHandlerFoundEventsTable->Lookup(objHandle); //destroy handle - if (retrievedHandle != NULL) + OBJECTHANDLE objHandle = pThread->m_LastThrownObjectHandle; + if (objHandle != NULL) { - forceSendCatchHandlerFound = TRUE; + OBJECTHANDLE retrievedHandle = m_pForceCatchHandlerFoundEventsTable->Lookup(objHandle); + if (retrievedHandle != NULL) + { + forceSendCatchHandlerFound = TRUE; + } } return forceSendCatchHandlerFound; } diff --git a/src/coreclr/vm/datadescriptor/datadescriptor.inc b/src/coreclr/vm/datadescriptor/datadescriptor.inc index 2e98c59ccf651a..e19fd4634f3bd0 100644 --- a/src/coreclr/vm/datadescriptor/datadescriptor.inc +++ b/src/coreclr/vm/datadescriptor/datadescriptor.inc @@ -138,7 +138,7 @@ CDAC_TYPE_END(Exception) CDAC_TYPE_BEGIN(ExceptionInfo) CDAC_TYPE_INDETERMINATE(ExceptionInfo) CDAC_TYPE_FIELD(ExceptionInfo, T_POINTER, PreviousNestedInfo, offsetof(ExInfo, m_pPrevNestedInfo)) -CDAC_TYPE_FIELD(ExceptionInfo, TYPE(ObjectHandle), ThrownObjectHandle, offsetof(ExInfo, m_hThrowable)) +CDAC_TYPE_FIELD(ExceptionInfo, T_POINTER, ThrownObject, offsetof(ExInfo, m_exception)) CDAC_TYPE_FIELD(ExceptionInfo, T_UINT32, ExceptionFlags, cdac_data::ExceptionFlagsValue) CDAC_TYPE_FIELD(ExceptionInfo, T_POINTER, StackLowBound, cdac_data::StackLowBound) CDAC_TYPE_FIELD(ExceptionInfo, T_POINTER, StackHighBound, cdac_data::StackHighBound) diff --git a/src/coreclr/vm/eedbginterfaceimpl.cpp b/src/coreclr/vm/eedbginterfaceimpl.cpp index b2c9276425a5e7..74ca9270c86094 100644 --- a/src/coreclr/vm/eedbginterfaceimpl.cpp +++ b/src/coreclr/vm/eedbginterfaceimpl.cpp @@ -239,15 +239,16 @@ OBJECTHANDLE EEDbgInterfaceImpl::GetThreadException(Thread *pThread) } CONTRACTL_END; - OBJECTHANDLE oh = pThread->GetThrowableAsHandle(); - - if (oh != NULL) + // The exception object is stored directly in ExInfo::m_exception (no handle). + // Return m_LastThrownObjectHandle which is kept in sync via SafeSetThrowables + // before any debugger notification fires. Assert this invariant in debug builds. +#ifdef _DEBUG + ExInfo* pTracker = pThread->GetExceptionState()->GetCurrentExceptionTracker(); + if (pTracker != NULL && pTracker->m_exception != NULL && pThread->m_LastThrownObjectHandle != NULL) { - return oh; + _ASSERTE(ObjectFromHandle(pThread->m_LastThrownObjectHandle) == pTracker->m_exception); } - - // Return the last thrown object if there's no current throwable. - // This logic is similar to UpdateCurrentThrowable(). +#endif return pThread->m_LastThrownObjectHandle; } @@ -261,21 +262,7 @@ bool EEDbgInterfaceImpl::IsThreadExceptionNull(Thread *pThread) } CONTRACTL_END; - // - // We're assuming that the handle on the - // thread is a strong handle and we're goona check it for - // NULL. We're also assuming something about the - // implementation of the handle here, too. - // - OBJECTHANDLE h = pThread->GetThrowableAsHandle(); - if (h == NULL) - { - return true; - } - - void *pThrowable = *((void**)h); - - return (pThrowable == NULL); + return pThread->IsThrowableNull() && pThread->IsLastThrownObjectNull(); } void EEDbgInterfaceImpl::ClearThreadException(Thread *pThread) diff --git a/src/coreclr/vm/excep.cpp b/src/coreclr/vm/excep.cpp index 49b323075cda1b..9811affde52cda 100644 --- a/src/coreclr/vm/excep.cpp +++ b/src/coreclr/vm/excep.cpp @@ -1849,7 +1849,7 @@ BOOL IsInFirstFrameOfHandler(Thread *pThread, IJitManager *pJitManager, const ME CONTRACTL_END; // if don't have a throwable the aren't processing an exception - if (IsHandleNullUnchecked(pThread->GetThrowableAsHandle())) + if (pThread->IsThrowableNull()) return FALSE; EH_CLAUSE_ENUMERATOR pEnumState; diff --git a/src/coreclr/vm/exceptionhandling.cpp b/src/coreclr/vm/exceptionhandling.cpp index 00f6db4a17d612..daca852722d215 100644 --- a/src/coreclr/vm/exceptionhandling.cpp +++ b/src/coreclr/vm/exceptionhandling.cpp @@ -2922,7 +2922,7 @@ ExInfo::StackRange::StackRange() void ExInfo::EnumMemoryRegions(CLRDataEnumMemoryFlags flags) { // ExInfo is embedded so don't enum 'this'. - OBJECTHANDLE_EnumMemoryRegions(m_hThrowable); + OBJECTREF_EnumMemoryRegions(m_exception); m_ptrs.ExceptionRecord.EnumMem(); m_ptrs.ContextRecord.EnumMem(); } @@ -2973,7 +2973,7 @@ extern "C" void QCALLTYPE AppendExceptionStackFrame(QCall::ObjectHandleOnStack e _ASSERTE(pMD == codeInfo.GetMethodDesc()); #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); } } @@ -3772,7 +3772,7 @@ CLR_BOOL SfiInitWorker(StackFrameIterator* pThis, CONTEXT* pStackwalkCtx, CLR_BO if (pMD != NULL) { GCX_COOP(); - StackTraceInfo::AppendElement(pExInfo->m_hThrowable, 0, GetRegdisplaySP(pExInfo->m_frameIter.m_crawl.GetRegisterSet()), pMD, &pExInfo->m_frameIter.m_crawl); + StackTraceInfo::AppendElement((OBJECTHANDLE)&pExInfo->m_exception, 0, GetRegdisplaySP(pExInfo->m_frameIter.m_crawl.GetRegisterSet()), pMD, &pExInfo->m_frameIter.m_crawl); #if defined(DEBUGGING_SUPPORTED) if (NotifyDebuggerOfStub(pThread, pFrame)) @@ -3954,7 +3954,7 @@ CLR_BOOL SfiNextWorker(StackFrameIterator* pThis, uint* uExCollideClauseIdx, CLR void* callbackCxt = NULL; Interop::ManagedToNativeExceptionCallback callback = Interop::GetPropagatingExceptionCallback( &codeInfo, - pTopExInfo->m_hThrowable, + pTopExInfo->m_exception, &callbackCxt); if (callback != NULL) @@ -4091,7 +4091,7 @@ CLR_BOOL SfiNextWorker(StackFrameIterator* pThis, uint* uExCollideClauseIdx, CLR if (pMD != NULL) { GCX_COOP(); - StackTraceInfo::AppendElement(pTopExInfo->m_hThrowable, 0, GetRegdisplaySP(pTopExInfo->m_frameIter.m_crawl.GetRegisterSet()), pMD, &pTopExInfo->m_frameIter.m_crawl); + StackTraceInfo::AppendElement((OBJECTHANDLE)&pTopExInfo->m_exception, 0, GetRegdisplaySP(pTopExInfo->m_frameIter.m_crawl.GetRegisterSet()), pMD, &pTopExInfo->m_frameIter.m_crawl); #if defined(DEBUGGING_SUPPORTED) if (NotifyDebuggerOfStub(pThread, pFrame)) diff --git a/src/coreclr/vm/exinfo.cpp b/src/coreclr/vm/exinfo.cpp index da05037a9f4531..580a3fac4a7cb5 100644 --- a/src/coreclr/vm/exinfo.cpp +++ b/src/coreclr/vm/exinfo.cpp @@ -13,7 +13,6 @@ ExInfo::ExInfo(Thread *pThread, EXCEPTION_RECORD *pExceptionRecord, CONTEXT *pExceptionContext, ExKind exceptionKind) : m_pPrevNestedInfo(pThread->GetExceptionState()->GetCurrentExceptionTracker()), - m_hThrowable{}, m_ptrs({pExceptionRecord, pExceptionContext}), m_fDeliveredFirstChanceNotification(FALSE), m_ExceptionCode((pExceptionRecord != PTR_NULL) ? pExceptionRecord->ExceptionCode : 0), @@ -72,14 +71,7 @@ void ExInfo::TakeExceptionPointersOwnership(PAL_SEHException* ex) void ExInfo::ReleaseResources() { - if (m_hThrowable) - { - if (!CLRException::IsPreallocatedExceptionHandle(m_hThrowable)) - { - DestroyHandle(m_hThrowable); - } - m_hThrowable = NULL; - } + m_exception = NULL; #ifndef TARGET_UNIX // Clear any held Watson Bucketing details diff --git a/src/coreclr/vm/exinfo.h b/src/coreclr/vm/exinfo.h index d6a3483c9f9100..3431f6b6b0f4b0 100644 --- a/src/coreclr/vm/exinfo.h +++ b/src/coreclr/vm/exinfo.h @@ -92,8 +92,6 @@ struct ExInfo // Previous ExInfo in the chain of exceptions rethrown from their catch / finally handlers PTR_ExInfo m_pPrevNestedInfo; - // thrown exception object handle - OBJECTHANDLE m_hThrowable; // EXCEPTION_RECORD and CONTEXT_RECORD describing the exception and its location DAC_EXCEPTION_POINTERS m_ptrs; // Information for the funclet we are calling @@ -218,12 +216,7 @@ struct ExInfo } CONTRACTL_END; - if (0 != m_hThrowable) - { - return ObjectFromHandle(m_hThrowable); - } - - return NULL; + return m_exception; } inline BOOL DeliveredFirstChanceNotification() @@ -257,19 +250,6 @@ struct ExInfo return !m_ExceptionFlags.UnwindHasStarted(); } -#ifndef DACCESS_COMPILE - void DestroyExceptionHandle() - { - // Never, ever destroy a preallocated exception handle. - if ((m_hThrowable != NULL) && !CLRException::IsPreallocatedExceptionHandle(m_hThrowable)) - { - DestroyHandle(m_hThrowable); - } - - m_hThrowable = NULL; - } -#endif // !DACCESS_COMPILE - #ifdef DACCESS_COMPILE void EnumMemoryRegions(CLRDataEnumMemoryFlags flags); #endif diff --git a/src/coreclr/vm/exstate.cpp b/src/coreclr/vm/exstate.cpp index 50ce0024b3a41f..ee81a235f090f8 100644 --- a/src/coreclr/vm/exstate.cpp +++ b/src/coreclr/vm/exstate.cpp @@ -13,18 +13,6 @@ #include "comutilnative.h" // for assertions only #endif -OBJECTHANDLE ThreadExceptionState::GetThrowableAsHandle() -{ - WRAPPER_NO_CONTRACT; - - if (m_pCurrentTracker) - { - return m_pCurrentTracker->m_hThrowable; - } - - return (OBJECTHANDLE)NULL; -} - ThreadExceptionState::ThreadExceptionState() { @@ -46,14 +34,6 @@ ThreadExceptionState::~ThreadExceptionState() #endif // !TARGET_UNIX } -#ifndef DACCESS_COMPILE - -Thread* ThreadExceptionState::GetMyThread() -{ - return (Thread*)(((BYTE*)this) - offsetof(Thread, m_ExceptionState)); -} - - OBJECTREF ThreadExceptionState::GetThrowable() { CONTRACTL @@ -64,70 +44,58 @@ OBJECTREF ThreadExceptionState::GetThrowable() } CONTRACTL_END; - if (m_pCurrentTracker && m_pCurrentTracker->m_hThrowable) + if (m_pCurrentTracker) { - return ObjectFromHandle(m_pCurrentTracker->m_hThrowable); + return m_pCurrentTracker->m_exception; } return NULL; } +BOOL ThreadExceptionState::IsThrowableNull() +{ + LIMITED_METHOD_DAC_CONTRACT; + + if (m_pCurrentTracker == NULL) + return TRUE; + + return m_pCurrentTracker->m_exception == NULL; +} + +#ifndef DACCESS_COMPILE + +Thread* ThreadExceptionState::GetMyThread() +{ + return (Thread*)(((BYTE*)this) - offsetof(Thread, m_ExceptionState)); +} + + void ThreadExceptionState::SetThrowable(OBJECTREF throwable DEBUG_ARG(SetThrowableErrorChecking stecFlags)) { CONTRACTL { - if ((throwable == NULL) || CLRException::IsPreallocatedExceptionObject(throwable)) NOTHROW; else THROWS; // From CreateHandle + NOTHROW; GC_NOTRIGGER; if (throwable == NULL) MODE_ANY; else MODE_COOPERATIVE; } CONTRACTL_END; - if (m_pCurrentTracker) - { - m_pCurrentTracker->DestroyExceptionHandle(); - } - if (throwable != NULL) { - // Non-compliant exceptions are always wrapped. - // The use of the ExceptionNative:: helper here (rather than the global ::IsException helper) - // is hokey, but we need a GC_NOTRIGGER version and it's only for an ASSERT. _ASSERTE(IsException(throwable->GetMethodTable())); - OBJECTHANDLE hNewThrowable; - - // If we're tracking one of the preallocated exception objects, then just use the global handle that - // matches it rather than creating a new one. - if (CLRException::IsPreallocatedExceptionObject(throwable)) - { - hNewThrowable = CLRException::GetPreallocatedHandleForObject(throwable); - } - else - { - AppDomain* pDomain = AppDomain::GetCurrentDomain(); - _ASSERTE(pDomain != NULL); - hNewThrowable = pDomain->CreateHandle(throwable); - } - #ifdef _DEBUG - // - // Fatal stack overflow policy ends up short-circuiting the normal exception handling - // flow such that there could be no Tracker for this SO that is in flight. In this - // situation there is no place to store the throwable in the exception state, and instead - // it is presumed that the handle to the SO exception is elsewhere. (Current knowledge - // as of 7/15/05 is that it is stored in Thread::m_LastThrownObjectHandle; - // if (stecFlags != STEC_CurrentTrackerEqualNullOkHackForFatalStackOverflow) { CONSISTENCY_CHECK(CheckPointer(m_pCurrentTracker)); } #endif - - if (m_pCurrentTracker != NULL) - { - m_pCurrentTracker->m_hThrowable = hNewThrowable; - } } + + // 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. } DWORD ThreadExceptionState::GetExceptionCode() diff --git a/src/coreclr/vm/exstate.h b/src/coreclr/vm/exstate.h index 86a838d70309be..669cbd1bdc4402 100644 --- a/src/coreclr/vm/exstate.h +++ b/src/coreclr/vm/exstate.h @@ -60,7 +60,7 @@ class ThreadExceptionState void SetThrowable(OBJECTREF throwable DEBUG_ARG(SetThrowableErrorChecking stecFlags = STEC_All)); OBJECTREF GetThrowable(); - OBJECTHANDLE GetThrowableAsHandle(); + BOOL IsThrowableNull(); DWORD GetExceptionCode(); BOOL IsComPlusException(); EXCEPTION_POINTERS* GetExceptionPointers(); diff --git a/src/coreclr/vm/gcenv.ee.cpp b/src/coreclr/vm/gcenv.ee.cpp index 6e7d047e6aa2a0..f1b400afbbac6c 100644 --- a/src/coreclr/vm/gcenv.ee.cpp +++ b/src/coreclr/vm/gcenv.ee.cpp @@ -13,6 +13,7 @@ #include "../gc/env/gcenv.ee.h" #include "threadsuspend.h" #include "interoplibinterface.h" +#include "exinfo.h" #ifdef FEATURE_COMINTEROP #include "runtimecallablewrapper.h" @@ -204,6 +205,19 @@ static void ScanStackRoots(Thread * pThread, promote_func* fn, ScanContext* sc) pGCFrame->GcScanRoots(fn, sc); pGCFrame = pGCFrame->PtrNextFrame(); } + + // Scan the ExInfo chain for exception objects held by direct pointer. + // Superseded ExInfo objects may live in logically dead parts of the stack + // that the normal GC stackwalk skips (e.g., when one exception dispatch + // supersedes a previous one). We keep them alive for post-mortem debugging + // and SOS. This mirrors NativeAOT's GcScanRootsWorker (thread.cpp:569-573). + PTR_ExInfo pExInfo = pThread->GetExceptionState()->GetCurrentExceptionTracker(); + while (pExInfo != NULL) + { + PTR_PTR_Object pRef = dac_cast(&pExInfo->m_exception); + fn(pRef, sc, 0); + pExInfo = pExInfo->GetPreviousExceptionTracker(); + } } static void ScanTailCallArgBufferRoots(Thread* pThread, promote_func* fn, ScanContext* sc) diff --git a/src/coreclr/vm/interoplibinterface.h b/src/coreclr/vm/interoplibinterface.h index 4ab07ddf5979a8..e7d8ea3e209fbb 100644 --- a/src/coreclr/vm/interoplibinterface.h +++ b/src/coreclr/vm/interoplibinterface.h @@ -43,7 +43,7 @@ class ObjCMarshalNative public: // Exceptions static void* GetPropagatingExceptionCallback( _In_ EECodeInfo* codeInfo, - _In_ OBJECTHANDLE throwable, + _In_ OBJECTREF throwable, _Outptr_ void** context); public: // GC interaction @@ -105,7 +105,7 @@ class Interop static ManagedToNativeExceptionCallback GetPropagatingExceptionCallback( _In_ EECodeInfo* codeInfo, - _In_ OBJECTHANDLE throwable, + _In_ OBJECTREF throwable, _Outptr_ void** context); // Notify started/finished when GC is running. diff --git a/src/coreclr/vm/interoplibinterface_objc.cpp b/src/coreclr/vm/interoplibinterface_objc.cpp index d44165731bb407..9fb23c76451ad1 100644 --- a/src/coreclr/vm/interoplibinterface_objc.cpp +++ b/src/coreclr/vm/interoplibinterface_objc.cpp @@ -271,15 +271,15 @@ namespace void* ObjCMarshalNative::GetPropagatingExceptionCallback( _In_ EECodeInfo* codeInfo, - _In_ OBJECTHANDLE throwable, + _In_ OBJECTREF throwableRef, _Outptr_ void** context) { CONTRACT(void*) { THROWS; - MODE_PREEMPTIVE; + MODE_COOPERATIVE; PRECONDITION(codeInfo != NULL); - PRECONDITION(throwable != NULL); + PRECONDITION(throwableRef != NULL); PRECONDITION(context != NULL); } CONTRACT_END; @@ -301,11 +301,8 @@ void* ObjCMarshalNative::GetPropagatingExceptionCallback( } { - GCX_COOP(); - OBJECTREF throwableRef = NULL; GCPROTECT_BEGIN(throwableRef); - throwableRef = ObjectFromHandle(throwable); callback = CallInvokeUnhandledExceptionPropagation( &throwableRef, method, diff --git a/src/coreclr/vm/interoplibinterface_shared.cpp b/src/coreclr/vm/interoplibinterface_shared.cpp index 5cd3378ff66b72..eb20a641b4b090 100644 --- a/src/coreclr/vm/interoplibinterface_shared.cpp +++ b/src/coreclr/vm/interoplibinterface_shared.cpp @@ -34,13 +34,13 @@ bool Interop::ShouldCheckForPendingException(_In_ PInvokeMethodDesc* md) ManagedToNativeExceptionCallback Interop::GetPropagatingExceptionCallback( _In_ EECodeInfo* codeInfo, - _In_ OBJECTHANDLE throwable, + _In_ OBJECTREF throwable, _Outptr_ void** context) { CONTRACT(ManagedToNativeExceptionCallback) { NOTHROW; - MODE_PREEMPTIVE; + MODE_COOPERATIVE; PRECONDITION(codeInfo != NULL); PRECONDITION(throwable != NULL); PRECONDITION(context != NULL); diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 292e5621b2e373..7e0808085873ca 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -3326,22 +3326,17 @@ void Thread::SafeUpdateLastThrownObject(void) } CONTRACTL_END; - OBJECTHANDLE hThrowable = GetThrowableAsHandle(); + OBJECTREF throwable = GetExceptionState()->GetThrowable(); - if (hThrowable != NULL) + if (throwable != NULL) { EX_TRY { - IGCHandleManager *pHandleTable = GCHandleUtilities::GetGCHandleManager(); - - // Creating a duplicate handle here ensures that the AD of the last thrown object - // matches the domain of the current throwable. - OBJECTHANDLE duplicateHandle = pHandleTable->CreateDuplicateHandle(hThrowable); - SetLastThrownObjectHandle(duplicateHandle); + SetLastThrownObject(throwable); } EX_CATCH { - // If we can't create a duplicate handle, we set both throwables to the preallocated OOM exception. + // If we can't create a handle, we set both throwables to the preallocated OOM exception. SafeSetThrowables(CLRException::GetPreallocatedOutOfMemoryException()); } EX_END_CATCH diff --git a/src/coreclr/vm/threads.h b/src/coreclr/vm/threads.h index ade9b6ad804998..2ea9caca10de19 100644 --- a/src/coreclr/vm/threads.h +++ b/src/coreclr/vm/threads.h @@ -1443,25 +1443,18 @@ class Thread return m_ExceptionState.GetThrowable(); } - // An unmnaged thread can check if a managed is processing an exception + // An unmanaged thread can check if a managed thread is processing an exception BOOL HasException() { LIMITED_METHOD_CONTRACT; - OBJECTHANDLE pThrowable = m_ExceptionState.GetThrowableAsHandle(); - return pThrowable && *PTR_UNCHECKED_OBJECTREF(pThrowable); - } - - OBJECTHANDLE GetThrowableAsHandle() - { - LIMITED_METHOD_CONTRACT; - return m_ExceptionState.GetThrowableAsHandle(); + return !IsThrowableNull(); } // special null test (for use when we're in the wrong GC mode) BOOL IsThrowableNull() { WRAPPER_NO_CONTRACT; - return IsHandleNullUnchecked(m_ExceptionState.GetThrowableAsHandle()); + return m_ExceptionState.IsThrowableNull(); } BOOL IsExceptionInProgress() diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Exception_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Exception_1.cs index a6911cbad06186..018f49f95aa196 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Exception_1.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Exception_1.cs @@ -18,8 +18,14 @@ TargetPointer IException.GetNestedExceptionInfo(TargetPointer exceptionInfoAddr, { Data.ExceptionInfo exceptionInfo = _target.ProcessedData.GetOrAdd(exceptionInfoAddr); nextNestedExceptionInfo = exceptionInfo.PreviousNestedInfo; - thrownObjectHandle = exceptionInfo.ThrownObjectHandle.Handle; - return exceptionInfo.ThrownObjectHandle.Object; + // 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. + Target.TypeInfo type = _target.GetTypeInfo(DataType.ExceptionInfo); + thrownObjectHandle = exceptionInfoAddr + (ulong)type.Fields[nameof(Data.ExceptionInfo.ThrownObject)].Offset; + return exceptionInfo.ThrownObject; } ExceptionData IException.GetExceptionData(TargetPointer exceptionAddr) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Thread_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Thread_1.cs index 4e2d60fc66f341..aafb4456f09996 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Thread_1.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Thread_1.cs @@ -92,7 +92,7 @@ ThreadData IThread.GetThreadData(TargetPointer threadPointer) Data.ExceptionInfo exceptionInfo = _target.ProcessedData.GetOrAdd(address); firstNestedException = exceptionInfo.PreviousNestedInfo; - if (exceptionInfo.ThrownObjectHandle.Handle != TargetPointer.Null) + if (exceptionInfo.ThrownObject != TargetPointer.Null) { uint exceptionFlags = exceptionInfo.ExceptionFlags; hasUnhandledException = (exceptionFlags & (uint)ExceptionFlags.IsUnhandled) != 0 @@ -209,37 +209,37 @@ TargetPointer IThread.GetThreadLocalStaticBase(TargetPointer threadPointer, Targ return threadLocalStaticBase; } - private (Data.Thread thread, Data.ExceptionInfo? exceptionInfo) GetThreadExceptionInfo(TargetPointer threadPointer) + private (Data.Thread thread, Data.ExceptionInfo? exceptionInfo, TargetPointer exceptionTrackerAddr) GetThreadExceptionInfo(TargetPointer threadPointer) { Data.Thread thread = _target.ProcessedData.GetOrAdd(threadPointer); TargetPointer exceptionTrackerPtr = _target.ReadPointer(thread.ExceptionTracker); Data.ExceptionInfo? exceptionInfo = (exceptionTrackerPtr == TargetPointer.Null) ? null : _target.ProcessedData.GetOrAdd(exceptionTrackerPtr); - return (thread, exceptionInfo); + return (thread, exceptionInfo, exceptionTrackerPtr); } TargetPointer IThread.GetCurrentExceptionHandle(TargetPointer threadPointer) { - var (_, exceptionInfo) = GetThreadExceptionInfo(threadPointer); + var (_, exceptionInfo, exceptionTrackerAddr) = GetThreadExceptionInfo(threadPointer); - if (exceptionInfo == null) - return TargetPointer.Null; - - if (exceptionInfo.ThrownObjectHandle.Handle == TargetPointer.Null || exceptionInfo.ThrownObjectHandle.Object == TargetPointer.Null) + if (exceptionInfo is null || exceptionInfo.ThrownObject == TargetPointer.Null) return TargetPointer.Null; - return exceptionInfo.ThrownObjectHandle.Handle; + // Return the target address of the ThrownObject field as a pseudo-handle. + // Callers dereference this address to read the exception Object*. + Target.TypeInfo type = _target.GetTypeInfo(DataType.ExceptionInfo); + return exceptionTrackerAddr + (ulong)type.Fields[nameof(Data.ExceptionInfo.ThrownObject)].Offset; } byte[] IThread.GetWatsonBuckets(TargetPointer threadPointer) { TargetPointer readFrom; - var (thread, exceptionInfo) = GetThreadExceptionInfo(threadPointer); + var (thread, exceptionInfo, _) = GetThreadExceptionInfo(threadPointer); if (exceptionInfo == null) return Array.Empty(); - Data.ObjectHandle throwableObject = exceptionInfo.ThrownObjectHandle; - if (throwableObject.Object != TargetPointer.Null) + TargetPointer thrownObject = exceptionInfo.ThrownObject; + if (thrownObject != TargetPointer.Null) { - Data.Exception exception = _target.ProcessedData.GetOrAdd(throwableObject.Object); + Data.Exception exception = _target.ProcessedData.GetOrAdd(thrownObject); if (exception.WatsonBuckets != TargetPointer.Null) { readFrom = _target.Contracts.Object.GetArrayData(exception.WatsonBuckets, out _, out _, out _); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/ExceptionInfo.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/ExceptionInfo.cs index c8d30ded52e678..191d29175d808c 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/ExceptionInfo.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/ExceptionInfo.cs @@ -13,7 +13,7 @@ public ExceptionInfo(Target target, TargetPointer address) Target.TypeInfo type = target.GetTypeInfo(DataType.ExceptionInfo); PreviousNestedInfo = target.ReadPointerField(address, type, nameof(PreviousNestedInfo)); - ThrownObjectHandle = target.ReadDataField(address, type, nameof(ThrownObjectHandle)); + ThrownObject = target.ReadPointerField(address, type, nameof(ThrownObject)); if (type.Fields.ContainsKey(nameof(ExceptionWatsonBucketTrackerBuckets))) ExceptionWatsonBucketTrackerBuckets = target.ReadPointerField(address, type, nameof(ExceptionWatsonBucketTrackerBuckets)); ExceptionFlags = target.ReadField(address, type, nameof(ExceptionFlags)); @@ -26,7 +26,7 @@ public ExceptionInfo(Target target, TargetPointer address) } public TargetPointer PreviousNestedInfo { get; } - public ObjectHandle ThrownObjectHandle { get; } + public TargetPointer ThrownObject { get; } public uint ExceptionFlags { get; } public TargetPointer StackLowBound { get; } public TargetPointer StackHighBound { get; } diff --git a/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Thread.cs b/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Thread.cs index bfa7b2a8b5416b..365573e0cbe503 100644 --- a/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Thread.cs +++ b/src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Thread.cs @@ -8,7 +8,7 @@ namespace Microsoft.Diagnostics.DataContractReader.Tests; internal sealed class MockExceptionInfo : TypedView { private const string PreviousNestedInfoFieldName = "PreviousNestedInfo"; - private const string ThrownObjectHandleFieldName = "ThrownObjectHandle"; + private const string ThrownObjectFieldName = "ThrownObject"; private const string ExceptionFlagsFieldName = "ExceptionFlags"; private const string StackLowBoundFieldName = "StackLowBound"; private const string StackHighBoundFieldName = "StackHighBound"; @@ -21,7 +21,7 @@ internal sealed class MockExceptionInfo : TypedView public static Layout CreateLayout(MockTarget.Architecture architecture) => new SequentialLayoutBuilder("ExceptionInfo", architecture) .AddPointerField(PreviousNestedInfoFieldName) - .AddPointerField(ThrownObjectHandleFieldName) + .AddPointerField(ThrownObjectFieldName) .AddUInt32Field(ExceptionFlagsFieldName) .AddPointerField(StackLowBoundFieldName) .AddPointerField(StackHighBoundFieldName) @@ -32,10 +32,10 @@ public static Layout CreateLayout(MockTarget.Architecture arc .AddPointerField(CallerOfActualHandlerFrameFieldName) .Build(); - public ulong ThrownObjectHandle + public ulong ThrownObject { - get => ReadPointerField(ThrownObjectHandleFieldName); - set => WritePointerField(ThrownObjectHandleFieldName, value); + get => ReadPointerField(ThrownObjectFieldName); + set => WritePointerField(ThrownObjectFieldName, value); } } diff --git a/src/native/managed/cdac/tests/ThreadTests.cs b/src/native/managed/cdac/tests/ThreadTests.cs index a9960aaf5d9c0b..7342010740a433 100644 --- a/src/native/managed/cdac/tests/ThreadTests.cs +++ b/src/native/managed/cdac/tests/ThreadTests.cs @@ -222,16 +222,14 @@ public void GetCurrentExceptionHandle_WithException(MockTarget.Architecture arch { 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); + // The handle is the address of the ThrownObject field — reading through it gives the object pointer + Assert.NotEqual(TargetPointer.Null, thrownObjectHandle); + Assert.Equal(expectedObject, target.ReadPointer(thrownObjectHandle)); } [Theory] @@ -266,11 +264,7 @@ public void GetCurrentExceptionHandle_HandlePointsToNull(MockTarget.Architecture { 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; }); IThread contract = target.Contracts.Thread; From b032961430bc1c3d9feb9e7edbda55f602945ada Mon Sep 17 00:00:00 2001 From: Max Charlamb <44248479+max-charlamb@users.noreply.github.com> Date: Fri, 24 Apr 2026 00:54:52 -0400 Subject: [PATCH 2/3] Fix GC hole: remove double-reporting of ExInfo::m_exception 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> --- src/coreclr/vm/eepolicy.cpp | 3 +-- src/coreclr/vm/excep.cpp | 2 +- src/coreclr/vm/exceptionhandling.cpp | 18 +++++++++--------- src/coreclr/vm/exinfo.cpp | 10 ++++++++++ src/coreclr/vm/exstate.cpp | 28 ---------------------------- src/coreclr/vm/exstate.h | 9 --------- src/coreclr/vm/threads.cpp | 16 +++++----------- src/coreclr/vm/threads.h | 5 +---- src/coreclr/vm/threads.inl | 7 ------- 9 files changed, 27 insertions(+), 71 deletions(-) diff --git a/src/coreclr/vm/eepolicy.cpp b/src/coreclr/vm/eepolicy.cpp index f756091105964e..6fcbaec5aaf61a 100644 --- a/src/coreclr/vm/eepolicy.cpp +++ b/src/coreclr/vm/eepolicy.cpp @@ -789,8 +789,7 @@ void DECLSPEC_NORETURN EEPolicy::HandleFatalStackOverflow(EXCEPTION_POINTERS *pE OBJECTHANDLE ohSO = CLRException::GetPreallocatedStackOverflowExceptionHandle(); if (ohSO != NULL) { - pThread->SafeSetThrowables(ObjectFromHandle(ohSO) - DEBUG_ARG(ThreadExceptionState::STEC_CurrentTrackerEqualNullOkHackForFatalStackOverflow), + pThread->SafeSetThrowables(ObjectFromHandle(ohSO), TRUE); } else diff --git a/src/coreclr/vm/excep.cpp b/src/coreclr/vm/excep.cpp index 9811affde52cda..69aae9f2cde364 100644 --- a/src/coreclr/vm/excep.cpp +++ b/src/coreclr/vm/excep.cpp @@ -2572,7 +2572,7 @@ void StackTraceInfo::AppendElement(OBJECTHANDLE hThrowable, UINT_PTR currentIP, LOG((LF_EH, LL_INFO10000, "StackTraceInfo::AppendElement IP = %p, SP = %p, %s::%s\n", currentIP, currentSP, pFunc ? pFunc->m_pszDebugClassName : "", pFunc ? pFunc->m_pszDebugMethodName : "" )); // Do not save stacktrace to preallocated exception. These are shared. - if (CLRException::IsPreallocatedExceptionHandle(hThrowable)) + if (CLRException::IsPreallocatedExceptionObject(ObjectFromHandle(hThrowable))) { return; } diff --git a/src/coreclr/vm/exceptionhandling.cpp b/src/coreclr/vm/exceptionhandling.cpp index daca852722d215..8d89816390b280 100644 --- a/src/coreclr/vm/exceptionhandling.cpp +++ b/src/coreclr/vm/exceptionhandling.cpp @@ -1447,7 +1447,9 @@ BOOL HandleHardwareException(PAL_SEHException* ex) exInfo.TakeExceptionPointersOwnership(ex); } - GCPROTECT_BEGIN(exInfo.m_exception); + // m_exception is GC-reported via ExInfo chain scanning in ScanStackRoots. + // Do NOT also GCPROTECT it — reporting the same location twice corrupts + // the GC's relocation logic (see clr-code-guide.md §2.1.5). UnmanagedCallersOnlyCaller throwHwEx(METHOD__EH__RH_THROWHW_EX); pThread->IncPreventAbort(); @@ -1457,8 +1459,6 @@ BOOL HandleHardwareException(PAL_SEHException* ex) DispatchExSecondPass(&exInfo); - GCPROTECT_END(); - UNREACHABLE(); } else @@ -1620,8 +1620,9 @@ VOID DECLSPEC_NORETURN DispatchManagedException(OBJECTREF throwable, CONTEXT* pE } } - GCPROTECT_BEGIN(exInfo.m_exception); - + // m_exception is GC-reported via ExInfo chain scanning in ScanStackRoots. + // Do NOT also GCPROTECT it — reporting the same location twice corrupts + // the GC's relocation logic (see clr-code-guide.md §2.1.5). UnmanagedCallersOnlyCaller throwEx(METHOD__EH__RH_THROW_EX); pThread->IncPreventAbort(); @@ -1631,7 +1632,6 @@ VOID DECLSPEC_NORETURN DispatchManagedException(OBJECTREF throwable, CONTEXT* pE DispatchExSecondPass(&exInfo); - GCPROTECT_END(); GCPROTECT_END(); UNREACHABLE(); @@ -1674,7 +1674,9 @@ VOID DECLSPEC_NORETURN DispatchRethrownManagedException(CONTEXT* pExceptionConte ExInfo exInfo(pThread, pActiveExInfo->m_ptrs.ExceptionRecord, pExceptionContext, ExKind::None); - GCPROTECT_BEGIN(exInfo.m_exception); + // m_exception is GC-reported via ExInfo chain scanning in ScanStackRoots. + // Do NOT also GCPROTECT it — reporting the same location twice corrupts + // the GC's relocation logic (see clr-code-guide.md §2.1.5). UnmanagedCallersOnlyCaller rethrow(METHOD__EH__RH_RETHROW); pThread->IncPreventAbort(); @@ -1683,8 +1685,6 @@ VOID DECLSPEC_NORETURN DispatchRethrownManagedException(CONTEXT* pExceptionConte rethrow.InvokeDirect(pActiveExInfo, &exInfo); DispatchExSecondPass(&exInfo); - GCPROTECT_END(); - UNREACHABLE(); } diff --git a/src/coreclr/vm/exinfo.cpp b/src/coreclr/vm/exinfo.cpp index 580a3fac4a7cb5..2191135dbe1846 100644 --- a/src/coreclr/vm/exinfo.cpp +++ b/src/coreclr/vm/exinfo.cpp @@ -38,6 +38,16 @@ ExInfo::ExInfo(Thread *pThread, EXCEPTION_RECORD *pExceptionRecord, CONTEXT *pEx #endif // HOST_WINDOWS { pThread->GetExceptionState()->m_pCurrentTracker = this; + + // m_exception is GC-reported via ExInfo chain scanning in ScanStackRoots + // (not via GCPROTECT — reporting the same location twice corrupts the GC + // relocation logic; see clr-code-guide.md §2.1.5). Mark the slot as + // protected in the debug OBJECTREF tracking table so that checked-build + // validation knows the reference is rooted. +#ifdef USE_CHECKED_OBJECTREFS + Thread::ObjectRefProtected(&m_exception); +#endif + m_pInitialFrame = pThread->GetFrame(); if (exceptionKind == ExKind::HardwareFault) { diff --git a/src/coreclr/vm/exstate.cpp b/src/coreclr/vm/exstate.cpp index ee81a235f090f8..8b6d928b86325e 100644 --- a/src/coreclr/vm/exstate.cpp +++ b/src/coreclr/vm/exstate.cpp @@ -70,34 +70,6 @@ Thread* ThreadExceptionState::GetMyThread() } -void ThreadExceptionState::SetThrowable(OBJECTREF throwable DEBUG_ARG(SetThrowableErrorChecking stecFlags)) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - if (throwable == NULL) MODE_ANY; else MODE_COOPERATIVE; - } - CONTRACTL_END; - - if (throwable != NULL) - { - _ASSERTE(IsException(throwable->GetMethodTable())); - -#ifdef _DEBUG - if (stecFlags != STEC_CurrentTrackerEqualNullOkHackForFatalStackOverflow) - { - CONSISTENCY_CHECK(CheckPointer(m_pCurrentTracker)); - } -#endif - } - - // 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. -} - DWORD ThreadExceptionState::GetExceptionCode() { LIMITED_METHOD_CONTRACT; diff --git a/src/coreclr/vm/exstate.h b/src/coreclr/vm/exstate.h index 669cbd1bdc4402..a38054349cc00b 100644 --- a/src/coreclr/vm/exstate.h +++ b/src/coreclr/vm/exstate.h @@ -50,15 +50,6 @@ class ThreadExceptionState public: -#ifdef _DEBUG - typedef enum - { - STEC_All, - STEC_CurrentTrackerEqualNullOkHackForFatalStackOverflow, - } SetThrowableErrorChecking; -#endif - - void SetThrowable(OBJECTREF throwable DEBUG_ARG(SetThrowableErrorChecking stecFlags = STEC_All)); OBJECTREF GetThrowable(); BOOL IsThrowableNull(); DWORD GetExceptionCode(); diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 7e0808085873ca..7d00116fcfaacf 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -3221,12 +3221,12 @@ OBJECTREF Thread::SafeSetLastThrownObject(OBJECTREF throwable) } // -// This is a nice wrapper for SetThrowable and SetLastThrownObject, which catches any exceptions caused by not +// This is a nice wrapper for SetLastThrownObject, which catches any exceptions caused by not // being able to create the handle for the throwable, and sets the throwable to the preallocated out of memory // exception instead. It also updates the last thrown object, which is always updated when the throwable is // updated. // -OBJECTREF Thread::SafeSetThrowables(OBJECTREF throwable DEBUG_ARG(ThreadExceptionState::SetThrowableErrorChecking stecFlags), +OBJECTREF Thread::SafeSetThrowables(OBJECTREF throwable, BOOL isUnhandled) { CONTRACTL @@ -3242,11 +3242,8 @@ OBJECTREF Thread::SafeSetThrowables(OBJECTREF throwable DEBUG_ARG(ThreadExceptio EX_TRY { - // Try to set the throwable. - SetThrowable(throwable DEBUG_ARG(stecFlags)); - - // Now, if the last thrown object is different, go ahead and update it. This makes sure that we re-throw - // the right object when we rethrow. + // The exception object is stored directly in ExInfo::m_exception by managed EH code, + // so we only need to update the last thrown object handle here. if (LastThrownObject() != throwable) { SetLastThrownObject(throwable); @@ -3259,12 +3256,9 @@ OBJECTREF Thread::SafeSetThrowables(OBJECTREF throwable DEBUG_ARG(ThreadExceptio } EX_CATCH { - // If either set didn't work, then set both throwables to the preallocated OOM exception, and return that - // object instead of the original throwable. + // If we can't create a handle, set the last thrown object to the preallocated OOM exception. ret = CLRException::GetPreallocatedOutOfMemoryException(); - // Neither of these will throw because we're setting with a preallocated exception. - SetThrowable(ret DEBUG_ARG(stecFlags)); SetLastThrownObject(ret, isUnhandled); } EX_END_CATCH diff --git a/src/coreclr/vm/threads.h b/src/coreclr/vm/threads.h index 2ea9caca10de19..e621ac3fbfaa8c 100644 --- a/src/coreclr/vm/threads.h +++ b/src/coreclr/vm/threads.h @@ -1433,8 +1433,6 @@ class Thread //--------------------------------------------------------------- // Last exception to be thrown //--------------------------------------------------------------- - inline void SetThrowable(OBJECTREF pThrowable - DEBUG_ARG(ThreadExceptionState::SetThrowableErrorChecking stecFlags = ThreadExceptionState::STEC_All)); OBJECTREF GetThrowable() { @@ -2667,8 +2665,7 @@ class Thread } void SafeUpdateLastThrownObject(void); - OBJECTREF SafeSetThrowables(OBJECTREF pThrowable - DEBUG_ARG(ThreadExceptionState::SetThrowableErrorChecking stecFlags = ThreadExceptionState::STEC_All), + OBJECTREF SafeSetThrowables(OBJECTREF pThrowable, BOOL isUnhandled = FALSE); bool IsLastThrownObjectStackOverflowException() diff --git a/src/coreclr/vm/threads.inl b/src/coreclr/vm/threads.inl index ada63f848853fa..554436b6847f0b 100644 --- a/src/coreclr/vm/threads.inl +++ b/src/coreclr/vm/threads.inl @@ -65,13 +65,6 @@ Frame* Thread::FindFrame(SIZE_T StackPointer) return pFrame; } -inline void Thread::SetThrowable(OBJECTREF pThrowable DEBUG_ARG(ThreadExceptionState::SetThrowableErrorChecking stecFlags)) -{ - WRAPPER_NO_CONTRACT; - - m_ExceptionState.SetThrowable(pThrowable DEBUG_ARG(stecFlags)); -} - // get the current notification (if any) from this thread inline OBJECTHANDLE Thread::GetThreadCurrNotification() { From 1192b46ff60432261e62e80fe4a45564072600c3 Mon Sep 17 00:00:00 2001 From: Max Charlamb <44248479+max-charlamb@users.noreply.github.com> Date: Tue, 28 Apr 2026 17:23:24 -0400 Subject: [PATCH 3/3] Update SafeSetThrowables comment to reflect current behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/threads.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 7d00116fcfaacf..2a4c730b2d0241 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -3221,10 +3221,10 @@ OBJECTREF Thread::SafeSetLastThrownObject(OBJECTREF throwable) } // -// This is a nice wrapper for SetLastThrownObject, which catches any exceptions caused by not -// being able to create the handle for the throwable, and sets the throwable to the preallocated out of memory -// exception instead. It also updates the last thrown object, which is always updated when the throwable is -// updated. +// This is a nice wrapper for updating the last thrown object handle, which catches any exceptions caused by not +// being able to create the handle for the throwable, and falls back to the preallocated out of memory exception +// for the last thrown object instead. The throwable itself is stored directly in ExInfo::m_exception by managed +// EH code, so this helper only updates the last thrown object state. // OBJECTREF Thread::SafeSetThrowables(OBJECTREF throwable, BOOL isUnhandled)