Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/coreclr/vm/eedbginterfaceimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,6 @@ OBJECTHANDLE EEDbgInterfaceImpl::GetThreadException(Thread *pThread)
}

// Return the last thrown object if there's no current throwable.
// This logic is similar to UpdateCurrentThrowable().
return pThread->m_LastThrownObjectHandle;
}

Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/vm/eepolicy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -789,8 +789,7 @@ void DECLSPEC_NORETURN EEPolicy::HandleFatalStackOverflow(EXCEPTION_POINTERS *pE
OBJECTHANDLE ohSO = CLRException::GetPreallocatedStackOverflowExceptionHandle();
if (ohSO != NULL)
{
pThread->SafeSetThrowables(ObjectFromHandle(ohSO),
TRUE);
pThread->SetLastThrownObject(ObjectFromHandle(ohSO), TRUE);
}
else
{
Expand Down
67 changes: 6 additions & 61 deletions src/coreclr/vm/excep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,6 @@ BOOL ShouldOurUEFDisplayUI(PEXCEPTION_POINTERS pExceptionInfo)

void NotifyAppDomainsOfUnhandledException(
PEXCEPTION_POINTERS pExceptionPointers,
OBJECTREF *pThrowableIn,
BOOL useLastThrownObject);

VOID SetManagedUnhandledExceptionBit(
BOOL useLastThrownObject);

//-------------------------------------------------------------------------------
Expand Down Expand Up @@ -2021,10 +2017,10 @@ VOID DECLSPEC_NORETURN RaiseTheExceptionInternalOnly(OBJECTREF throwable)
// Always save the current object in the handle so on rethrow we can reuse it. This is important as it
// contains stack trace info.
//
// Note: we use SafeSetLastThrownObject, which will try to set the throwable and if there are any problems,
// Note: we use SetLastThrownObject, which will try to set the throwable and if there are any problems,
// it will set the throwable to something appropriate (like OOM exception) and return the new
// exception. Thus, the user's exception object can be replaced here.
throwable = pThread->SafeSetLastThrownObject(throwable);
throwable = pThread->SetLastThrownObject(throwable);

ULONG_PTR hr = GetHRFromThrowable(throwable);

Expand Down Expand Up @@ -3750,36 +3746,6 @@ BOOL InstallUnhandledExceptionFilter() {
return TRUE;
}

//
// Update the current throwable on the thread if necessary. If we're looking at one of our exceptions, and if the
// current throwable on the thread is NULL, then we'll set it to something more useful based on the
// LastThrownObject.
//
BOOL UpdateCurrentThrowable(PEXCEPTION_RECORD pExceptionRecord)
{
STATIC_CONTRACT_THROWS;
STATIC_CONTRACT_MODE_ANY;
STATIC_CONTRACT_GC_TRIGGERS;

BOOL useLastThrownObject = FALSE;

Thread* pThread = GetThread();

// GetThrowable needs cooperative.
GCX_COOP();

if ((pThread->GetThrowable() == NULL) && (pThread->LastThrownObject() != NULL))
{
// If GetThrowable is NULL and LastThrownObject is not, use lastThrownObject.
// In current (June 05) implementation, this is only used to pass to
// NotifyAppDomainsOfUnhandledException, which needs to get a throwable
// from somewhere, with which to notify the AppDomains.
useLastThrownObject = TRUE;
}

return useLastThrownObject;
}

//
// COMUnhandledExceptionFilter is used to catch all unhandled exceptions.
// The debugger will either handle the exception, attach a debugger, or
Expand Down Expand Up @@ -3953,7 +3919,7 @@ LONG InternalUnhandledExceptionFilter_Worker(
BOOL useLastThrownObject = FALSE;
if (!pParam->fIgnore && (pParam->pThread != NULL))
{
useLastThrownObject = UpdateCurrentThrowable(pParam->pExceptionInfo->ExceptionRecord);
useLastThrownObject = pParam->pThread->IsThrowableNull() && !pParam->pThread->IsLastThrownObjectNull();
}

#ifdef DEBUGGING_SUPPORTED
Expand Down Expand Up @@ -3988,29 +3954,13 @@ LONG InternalUnhandledExceptionFilter_Worker(
#endif // !TARGET_UNIX

// Send notifications to the AppDomains.
NotifyAppDomainsOfUnhandledException(pParam->pExceptionInfo, NULL, useLastThrownObject);
NotifyAppDomainsOfUnhandledException(pParam->pExceptionInfo, useLastThrownObject);
}
else
{
LOG((LF_EH, LL_INFO100, "InternalUnhandledExceptionFilter_Worker: Not collecting bucket information as thread object does not exist\n"));
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

// AppDomain.UnhandledException event could have thrown an exception that would have gone unhandled in managed code.
// The runtime swallows all such exceptions. Hence, if we are not using LastThrownObject and the current LastThrownObject
// is not the same as the one in active exception tracker (if available), then update the last thrown object.
if ((pParam->pThread != NULL) && (!useLastThrownObject))
{
GCX_COOP_NO_DTOR();

OBJECTREF oThrowable = pParam->pThread->GetThrowable();
if ((oThrowable != NULL) && (pParam->pThread->LastThrownObject() != oThrowable))
{
pParam->pThread->SafeSetLastThrownObject(oThrowable);
LOG((LF_EH, LL_INFO100, "InternalUnhandledExceptionFilter_Worker: Resetting the LastThrownObject as it appears to have changed.\n"));
}

GCX_COOP_NO_DTOR_END();
}

// Launch Watson and see if we want to debug the process
//
Expand Down Expand Up @@ -4546,7 +4496,6 @@ DefaultCatchHandler(PEXCEPTION_POINTERS pExceptionPointers,
//******************************************************************************
void NotifyAppDomainsOfUnhandledException(
PEXCEPTION_POINTERS pExceptionPointers,
OBJECTREF *pThrowableIn,
BOOL useLastThrownObject)
{
CONTRACTL
Expand Down Expand Up @@ -4580,11 +4529,7 @@ void NotifyAppDomainsOfUnhandledException(

OBJECTREF throwable;

if (pThrowableIn != NULL)
{
throwable = *pThrowableIn;
}
else if (useLastThrownObject)
if (useLastThrownObject)
{
throwable = pThread->LastThrownObject();
}
Expand Down Expand Up @@ -8194,7 +8139,7 @@ void SetupInitialThrowBucketDetails(UINT_PTR adjustedIp)
#ifdef _DEBUG
// Under OOM scenarios, its possible that when we are raising a threadabort,
// the throwable may get converted to preallocated OOM object when RaiseTheExceptionInternalOnly
// invokes Thread::SafeSetLastThrownObject. We check if this is the current case and use it in
// invokes Thread::SetLastThrownObject. We check if this is the current case and use it in
// our validation below.
BOOL fIsPreallocatedOOMExceptionForTA = FALSE;
if ((!fIsThreadAbortException) && pUEWatsonBucketTracker->CapturedForThreadAbort())
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/vm/excep.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ BOOL IsExceptionOfType(RuntimeExceptionKind reKind, OBJECTREF *pThrowable);
BOOL IsExceptionOfType(RuntimeExceptionKind reKind, Exception *pException);
BOOL IsUncatchable(OBJECTREF *pThrowable);
VOID FixupOnRethrow(Thread *pCurThread, EXCEPTION_POINTERS *pExceptionPointers);
BOOL UpdateCurrentThrowable(PEXCEPTION_RECORD pExceptionRecord);
BOOL IsStackOverflowException(Thread* pThread, EXCEPTION_RECORD* pExceptionRecord);
void WrapNonCompliantException(OBJECTREF *ppThrowable);
OBJECTREF PossiblyUnwrapThrowable(OBJECTREF throwable, Assembly *pAssembly);
Expand Down
6 changes: 1 addition & 5 deletions src/coreclr/vm/exceptionhandling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3186,12 +3186,9 @@ void CallCatchFunclet(OBJECTREF throwable, BYTE* pHandlerIP, REGDISPLAY* pvRegDi

if (!pThread->GetExceptionState()->IsExceptionInProgress())
{
pThread->SafeSetLastThrownObject(NULL);
pThread->SetLastThrownObject(NULL);
}

// Sync managed exception state, for the managed thread, based upon any active exception tracker
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this going to cause a behavior change in things like the windbg !pe command ?

Before this change, !pe issued while the thread is executing a catch block is going to print the exception that's being caught. After this change, I think it is going to print nothing.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(It would be nice if we can fix this without short-lived GCHandle allocation.)

pThread->SyncManagedExceptionState(false);

ExInfo::UpdateNonvolatileRegisters(pvRegDisplay->pCurrentContext, pvRegDisplay, FALSE);
if (pHandlerIP != NULL)
{
Expand Down Expand Up @@ -3596,7 +3593,6 @@ static void NotifyExceptionPassStarted(StackFrameIterator *pThis, Thread *pThrea
if (pExInfo->m_passNumber == 1)
{
GCX_COOP();
pThread->SafeSetThrowables(pExInfo->m_exception);
FirstChanceExceptionNotification();
EEToProfilerExceptionInterfaceWrapper::ExceptionThrown(pThread);
}
Expand Down
18 changes: 17 additions & 1 deletion src/coreclr/vm/exinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,21 @@ void ExInfo::ReleaseResources()
// static
void ExInfo::PopExInfos(Thread *pThread, void *targetSp)
{
CONTRACTL
{
NOTHROW;
GC_NOTRIGGER;
MODE_COOPERATIVE;
Comment thread
max-charlamb marked this conversation as resolved.
}
CONTRACTL_END;

STRESS_LOG1(LF_EH, LL_INFO100, "Popping ExInfos below SP=%p\n", targetSp);

ExInfo *pExInfo = (PTR_ExInfo)pThread->GetExceptionState()->GetCurrentExceptionTracker();
#if defined(DEBUGGING_SUPPORTED)
DWORD_PTR dwInterceptStackFrame = 0;

// This method may be called on an unmanaged thread, in which case no interception can be done.
// If there is no current ExInfo, there is nothing to inspect for interception.
if (pExInfo)
{
ThreadExceptionState* pExState = pThread->GetExceptionState();
Expand All @@ -131,6 +139,14 @@ void ExInfo::PopExInfos(Thread *pThread, void *targetSp)
}
#endif // DEBUGGING_SUPPORTED

// Set LTO from the exception being destroyed so that post-ExInfo consumers
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should do this only when it is actually going to be needed. For example, in a simple try { throw new Exception(); } catch { }, PopExInfos is going to be called from CallCatchFunclets, we create the LTO handle and the very next line in CallCatchFunclets is going to delete the LTO handle.

// (EX_CATCH via CLRLastThrownObjectException, ProcessCLRException bridging)
// can find the exception object after the ExInfo is gone.
if (pExInfo->m_exception != NULL)
{
pThread->SetLastThrownObject(pExInfo->m_exception);
}
Comment thread
max-charlamb marked this conversation as resolved.

pExInfo->ReleaseResources();
pExInfo = (PTR_ExInfo)pExInfo->m_pPrevNestedInfo;
}
Expand Down
77 changes: 0 additions & 77 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10479,83 +10479,6 @@ int32_t * CEEInfo::getAddrOfCaptureThreadGlobal(void **ppIndirection)
return result;
}

// This code is called if FilterException chose to handle the exception.
void CEEInfo::HandleException(struct _EXCEPTION_POINTERS *pExceptionPointers)
{
CONTRACTL {
NOTHROW;
GC_NOTRIGGER;
} CONTRACTL_END;

JIT_TO_EE_TRANSITION_LEAF();

if (IsComPlusException(pExceptionPointers->ExceptionRecord))
{
GCX_COOP();

// This is actually the LastThrown exception object.
OBJECTREF throwable = CLRException::GetThrowableFromExceptionRecord(pExceptionPointers->ExceptionRecord);

if (throwable != NULL)
{
struct
{
OBJECTREF oLastThrownObject;
OBJECTREF oCurrentThrowable;
} _gc;

ZeroMemory(&_gc, sizeof(_gc));

PTR_Thread pCurThread = GetThread();

// Setup the throwables
_gc.oLastThrownObject = throwable;

// This will be NULL if no managed exception is active. Otherwise,
// it will reference the active throwable.
_gc.oCurrentThrowable = pCurThread->GetThrowable();

GCPROTECT_BEGIN(_gc);

// JIT does not use or reference managed exceptions at all and simply swallows them,
// or lets them fly through so that they will either get caught in managed code, the VM
// or will go unhandled.
//
// Blind swallowing of managed exceptions can break the semantic of "which exception handler"
// gets to process the managed exception first. The expected handler is managed code exception
// handler (e.g. COMPlusFrameHandler on x86 and ProcessCLRException on 64bit) which will setup
// the exception tracker for the exception that will enable the expected sync between the
// LastThrownObject (LTO), setup in RaiseTheExceptionInternalOnly, and the exception tracker.
//
// However, JIT can break this by swallowing the managed exception before managed code exception
// handler gets a chance to setup an exception tracker for it. Since there is no cleanup
// done for the swallowed exception as part of the unwind (because no exception tracker may have been setup),
// we need to reset the LTO, if it is out of sync from the active throwable.
//
// Hence, check if the LastThrownObject and active-exception throwable are in sync or not.
// If not, bring them in sync.
//
// Example
// -------
// It is possible that an exception was already in progress and while processing it (e.g.
// invoking finally block), we invoked JIT that had another managed exception @ JIT-EE transition boundary
// that is swallowed by the JIT before managed code exception handler sees it. This breaks the sync between
// LTO and the active exception in the exception tracker.
if (_gc.oCurrentThrowable != _gc.oLastThrownObject)
{
// Update the LTO.
//
// Note: Incase of OOM, this will get set to OOM instance.
pCurThread->SafeSetLastThrownObject(_gc.oCurrentThrowable);
}

GCPROTECT_END();
}
}

EE_TO_JIT_TRANSITION_LEAF();
}

CORINFO_MODULE_HANDLE CEEInfo::embedModuleHandle(CORINFO_MODULE_HANDLE handle,
void **ppIndirection)
{
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/vm/jitinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,6 @@ class CEEInfo : public ICorJitInfo
TypeHandle GetTypeFromContext(CORINFO_CONTEXT_HANDLE context);
void GetTypeContext(CORINFO_CONTEXT_HANDLE context, SigTypeContext* pTypeContext);

void HandleException(struct _EXCEPTION_POINTERS* pExceptionPointers);
public:
#include "icorjitinfoimpl_generated.h"
uint32_t getClassAttribsInternal (CORINFO_CLASS_HANDLE cls);
Expand Down
Loading
Loading