Skip to content

Commit

Permalink
Fix VS debugger tests issues (#98783)
Browse files Browse the repository at this point in the history 
* Fix VS debugger tests issues

This change fixes three issues I have found in the debugger related code
after I was finally able to run the VS debugger tests with .NET 9.
These are the fixed issues:
* The old EH for hardware exceptions puts incorrectly the exception
  location adjusted by -1 to the exception stack trace. The new EH
  didn't have this problem, but VS expects that to be the case, so I
  have added a compensation for it until the old EH is gone or until VS
  can be changed to support both cases.
* Exception interception was not working correctly for cases when the
  exception needed to be propagated over native runtime frames. After
  catching the rethrown SEH / PAL_SEHException and also in
  ProcessCLRException for Windows, the interception needs to be detected
  and instead of calling the DispatchManagedException, we need to
  call the Ex.RhUnwindAndIntercept again.
* The first chance exception debugger notification was sometimes sent
  before even the first stack frame was added to the exception stack
  trace. That has broken quite a number of VS tests.

* Fix missing ifdef to fix x86 build

* Fix crossdac build
  • Loading branch information
@janvorli
janvorli authored Feb 22, 2024
1 parent dcc66a7 commit 6b26684
Show file tree
Hide file tree
Showing 6 changed files with 165 additions and 39 deletions.
49 changes: 48 additions & 1 deletion src/coreclr/vm/excep.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7761,6 +7761,42 @@ void UnwindAndContinueRethrowHelperInsideCatch(Frame* pEntryFrame, Exception* pE
#endif
}

#ifdef FEATURE_EH_FUNCLETS
//
// This function continues exception interception unwind after it crossed native frames using
// standard EH / SEH.
//
VOID DECLSPEC_NORETURN ContinueExceptionInterceptionUnwind()
{
STATIC_CONTRACT_THROWS;
STATIC_CONTRACT_GC_TRIGGERS;
STATIC_CONTRACT_MODE_ANY;

GCX_COOP();

Thread *pThread = GetThread();
ThreadExceptionState* pExState = pThread->GetExceptionState();
UINT_PTR uInterceptStackFrame = 0;

pExState->GetDebuggerState()->GetDebuggerInterceptInfo(NULL, NULL,
(PBYTE*)&uInterceptStackFrame,
NULL, NULL);

PREPARE_NONVIRTUAL_CALLSITE(METHOD__EH__UNWIND_AND_INTERCEPT);
DECLARE_ARGHOLDER_ARRAY(args, 2);
args[ARGNUM_0] = PTR_TO_ARGHOLDER((ExInfo*)pExState->GetCurrentExceptionTracker());
args[ARGNUM_1] = PTR_TO_ARGHOLDER(uInterceptStackFrame);
pThread->IncPreventAbort();

//Ex.RhUnwindAndIntercept(throwable, &exInfo)
CRITICAL_CALLSITE;
CALL_MANAGED_METHOD_NORET(args)

UNREACHABLE();
}

#endif // FEATURE_EH_FUNCLETS

//
// This does the work of the Unwind and Continue Hanlder after the catch clause of that handler. The stack has been
// unwound by the time this is called. Keep that in mind when deciding where to put new code :)
Expand All @@ -7784,7 +7820,18 @@ VOID DECLSPEC_NORETURN UnwindAndContinueRethrowHelperAfterCatch(Frame* pEntryFra
#ifdef FEATURE_EH_FUNCLETS
if (g_isNewExceptionHandlingEnabled && !nativeRethrow)
{
DispatchManagedException(orThrowable);
Thread *pThread = GetThread();
ThreadExceptionState* pExState = pThread->GetExceptionState();
ExInfo *pPrevExInfo = (ExInfo*)pExState->GetCurrentExceptionTracker();
if (pPrevExInfo != NULL && pPrevExInfo->m_DebuggerExState.GetDebuggerInterceptContext() != NULL)
{
ContinueExceptionInterceptionUnwind();
UNREACHABLE();
}
else
{
DispatchManagedException(orThrowable);
}
}
else
#endif // FEATURE_EH_FUNCLETS
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/vm/excep.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,10 @@ void ResetThreadAbortState(PTR_Thread pThread, CrawlFrame *pCf, StackFrame sfCur

X86_ONLY(EXCEPTION_REGISTRATION_RECORD* GetNextCOMPlusSEHRecord(EXCEPTION_REGISTRATION_RECORD* pRec);)

#ifdef FEATURE_EH_FUNCLETS
VOID DECLSPEC_NORETURN ContinueExceptionInterceptionUnwind();
#endif // FEATURE_EH_FUNCLETS

#endif // !DACCESS_COMPILE

#endif // __excep_h__
115 changes: 77 additions & 38 deletions src/coreclr/vm/exceptionhandling.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -929,8 +929,18 @@ ProcessCLRExceptionNew(IN PEXCEPTION_RECORD pExceptionRecord,
else
{
GCX_COOP();
OBJECTREF oref = ExceptionTracker::CreateThrowable(pExceptionRecord, FALSE);
DispatchManagedException(oref, pContextRecord);
ThreadExceptionState* pExState = pThread->GetExceptionState();
ExInfo *pPrevExInfo = (ExInfo*)pExState->GetCurrentExceptionTracker();
if (pPrevExInfo != NULL && pPrevExInfo->m_DebuggerExState.GetDebuggerInterceptContext() != NULL)
{
ContinueExceptionInterceptionUnwind();
UNREACHABLE();
}
else
{
OBJECTREF oref = ExceptionTracker::CreateThrowable(pExceptionRecord, FALSE);
DispatchManagedException(oref, pContextRecord);
}
}
#endif // !HOST_UNIX
EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_EXECUTIONENGINE, _T("SEH exception leaked into managed code"));
Expand Down Expand Up @@ -4299,9 +4309,12 @@ EXCEPTION_DISPOSITION ClrDebuggerDoUnwindAndIntercept(X86_FIRST_ARG(EXCEPTION_RE
{
GCX_COOP();

ExInfo* pExInfo = (ExInfo*)pExState->GetCurrentExceptionTracker();
_ASSERTE(pExInfo != NULL);

PREPARE_NONVIRTUAL_CALLSITE(METHOD__EH__UNWIND_AND_INTERCEPT);
DECLARE_ARGHOLDER_ARRAY(args, 2);
args[ARGNUM_0] = PTR_TO_ARGHOLDER(pExState->GetCurrentExceptionTracker());
args[ARGNUM_0] = PTR_TO_ARGHOLDER(pExInfo);
args[ARGNUM_1] = PTR_TO_ARGHOLDER(uInterceptStackFrame);
pThread->IncPreventAbort();

Expand Down Expand Up @@ -7532,29 +7545,55 @@ void MarkInlinedCallFrameAsEHHelperCall(Frame* pFrame)
pInlinedCallFrame->m_Datum = (PTR_NDirectMethodDesc)((TADDR)pInlinedCallFrame->m_Datum | (TADDR)InlinedCallFrameMarker::ExceptionHandlingHelper);
}

static TADDR GetSpForDiagnosticReporting(REGDISPLAY *pRD)
{
#ifdef ESTABLISHER_FRAME_ADDRESS_IS_CALLER_SP
return CallerStackFrame::FromRegDisplay(pRD).SP;
#else
return GetSP(pRD->pCurrentContext);
#endif
}

extern "C" void QCALLTYPE AppendExceptionStackFrame(QCall::ObjectHandleOnStack exceptionObj, SIZE_T ip, SIZE_T sp, int flags, ExInfo *pExInfo)
{
QCALL_CONTRACT;

BEGIN_QCALL;
GCX_COOP();

Thread* pThread = GET_THREAD();
Frame* pFrame = pThread->GetFrame();
MarkInlinedCallFrameAsFuncletCall(pFrame);

bool canAllocateMemory = !(exceptionObj.Get() == CLRException::GetPreallocatedOutOfMemoryException()) &&
!(exceptionObj.Get() == CLRException::GetPreallocatedStackOverflowException());
{
GCX_COOP();

Frame* pFrame = pThread->GetFrame();
MarkInlinedCallFrameAsEHHelperCall(pFrame);

MethodDesc *pMD = pExInfo->m_frameIter.m_crawl.GetFunction();
bool canAllocateMemory = !(exceptionObj.Get() == CLRException::GetPreallocatedOutOfMemoryException()) &&
!(exceptionObj.Get() == CLRException::GetPreallocatedStackOverflowException());

MethodDesc *pMD = pExInfo->m_frameIter.m_crawl.GetFunction();
#if _DEBUG
EECodeInfo codeInfo(ip);
_ASSERTE(codeInfo.IsValid());
_ASSERTE(pMD == codeInfo.GetMethodDesc());
EECodeInfo codeInfo(ip);
_ASSERTE(codeInfo.IsValid());
_ASSERTE(pMD == codeInfo.GetMethodDesc());
#endif // _DEBUG

pExInfo->m_StackTraceInfo.AppendElement(canAllocateMemory, ip, sp, pMD, &pExInfo->m_frameIter.m_crawl);
pExInfo->m_StackTraceInfo.SaveStackTrace(canAllocateMemory, pExInfo->m_hThrowable, /*bReplaceStack*/FALSE, /*bSkipLastElement*/FALSE);
// Compensate for a bug in the old EH that doesn't mark faulting instructions as faulting. The VS expects that behavior.
bool hasFaulted = pExInfo->m_frameIter.m_crawl.HasFaulted();
if (hasFaulted)
{
pExInfo->m_frameIter.m_crawl.hasFaulted = false;
}
pExInfo->m_StackTraceInfo.AppendElement(canAllocateMemory, ip, sp, pMD, &pExInfo->m_frameIter.m_crawl);
pExInfo->m_StackTraceInfo.SaveStackTrace(canAllocateMemory, pExInfo->m_hThrowable, /*bReplaceStack*/FALSE, /*bSkipLastElement*/FALSE);
pExInfo->m_frameIter.m_crawl.hasFaulted = hasFaulted;
}

// Notify the debugger that we are on the first pass for a managed exception.
// Note that this callback is made for every managed frame.
TADDR spForDebugger = GetSpForDiagnosticReporting(pExInfo->m_frameIter.m_crawl.GetRegisterSet());
EEToDebuggerExceptionInterfaceWrapper::FirstChanceManagedException(pThread, ip, spForDebugger);

if (!pExInfo->DeliveredFirstChanceNotification())
{
ExceptionNotifications::DeliverFirstChanceNotification();
Expand Down Expand Up @@ -7589,15 +7628,6 @@ UINT_PTR GetEstablisherFrame(REGDISPLAY* pvRegDisplay, ExInfo* exInfo)
#endif
}

static TADDR GetSpForDiagnosticReporting(REGDISPLAY *pRD)
{
#ifdef ESTABLISHER_FRAME_ADDRESS_IS_CALLER_SP
return CallerStackFrame::FromRegDisplay(pRD).SP;
#else
return GetSP(pRD->pCurrentContext);
#endif
}

extern "C" void * QCALLTYPE CallCatchFunclet(QCall::ObjectHandleOnStack exceptionObj, BYTE* pHandlerIP, REGDISPLAY* pvRegDisplay, ExInfo* exInfo)
{
QCALL_CONTRACT;
Expand Down Expand Up @@ -7663,25 +7693,32 @@ extern "C" void * QCALLTYPE CallCatchFunclet(QCall::ObjectHandleOnStack exceptio
BOOL fIntercepted = pThread->GetExceptionState()->GetFlags()->DebuggerInterceptInfo();
if (fIntercepted)
{
_ASSERTE(pHandlerIP == NULL);
// retrieve the interception information
MethodDesc *pInterceptMD = NULL;
StackFrame sfInterceptStackFrame;
UINT_PTR uResumePC = 0;
ULONG_PTR ulRelOffset;

pThread->GetExceptionState()->GetDebuggerState()->GetDebuggerInterceptInfo(&pInterceptMD, NULL, (PBYTE*)&(sfInterceptStackFrame.SP), &ulRelOffset, NULL);
if (sfInterceptStackFrame.SP == GetSP(pvRegDisplay->pCurrentContext))
{
PCODE pStartAddress = pInterceptMD->GetNativeCode();

PCODE pStartAddress = pInterceptMD->GetNativeCode();

EECodeInfo codeInfo(pStartAddress);
_ASSERTE(codeInfo.IsValid());
EECodeInfo codeInfo(pStartAddress);
_ASSERTE(codeInfo.IsValid());

// Note that the value returned for ulRelOffset is actually the offset,
// so we need to adjust it to get the actual IP.
_ASSERTE(FitsIn<DWORD>(ulRelOffset));
uResumePC = codeInfo.GetJitManager()->GetCodeAddressForRelOffset(codeInfo.GetMethodToken(), static_cast<DWORD>(ulRelOffset));
// Note that the value returned for ulRelOffset is actually the offset,
// so we need to adjust it to get the actual IP.
_ASSERTE(FitsIn<DWORD>(ulRelOffset));
uResumePC = codeInfo.GetJitManager()->GetCodeAddressForRelOffset(codeInfo.GetMethodToken(), static_cast<DWORD>(ulRelOffset));

SetIP(pvRegDisplay->pCurrentContext, uResumePC);
SetIP(pvRegDisplay->pCurrentContext, uResumePC);
}
else
{
fIntercepted = FALSE;
}
}
#endif // DEBUGGING_SUPPORTED

Expand Down Expand Up @@ -7797,6 +7834,10 @@ extern "C" void QCALLTYPE ResumeAtInterceptionLocation(REGDISPLAY* pvRegDisplay)
MarkInlinedCallFrameAsFuncletCall(pFrame);

UINT_PTR targetSp = GetSP(pvRegDisplay->pCurrentContext);
ExInfo *pExInfo = (PTR_ExInfo)pThread->GetExceptionState()->GetCurrentExceptionTracker();

pExInfo->m_ScannedStackRange.ExtendUpperBound(targetSp);

PopExplicitFrames(pThread, (void*)targetSp);

// This must be done before we pop the ExInfos.
Expand Down Expand Up @@ -8135,10 +8176,6 @@ static void NotifyFunctionEnter(StackFrameIterator *pThis, Thread *pThread, ExIn
EEToProfilerExceptionInterfaceWrapper::ExceptionSearchFunctionLeave(pExInfo->m_pMDToReportFunctionLeave);
}
EEToProfilerExceptionInterfaceWrapper::ExceptionSearchFunctionEnter(pMD);
// Notify the debugger that we are on the first pass for a managed exception.
// Note that this callback is made for every managed frame.
TADDR spForDebugger = GetSpForDiagnosticReporting(pThis->m_crawl.GetRegisterSet());
EEToDebuggerExceptionInterfaceWrapper::FirstChanceManagedException(pThread, GetControlPC(pThis->m_crawl.GetRegisterSet()), spForDebugger);
}
else
{
Expand Down Expand Up @@ -8169,8 +8206,7 @@ extern "C" bool QCALLTYPE SfiInit(StackFrameIterator* pThis, CONTEXT* pStackwalk
// just clear the thread state.
pThread->ResetThrowControlForThread();

// Skip the SfiInit pinvoke frame
pFrame = pThread->GetFrame()->PtrNextFrame();
pFrame = pExInfo->m_pInitialFrame;

NotifyExceptionPassStarted(pThis, pThread, pExInfo);

Expand Down Expand Up @@ -8215,6 +8251,7 @@ extern "C" bool QCALLTYPE SfiInit(StackFrameIterator* pThis, CONTEXT* pStackwalk
!(pExInfo->m_exception == CLRException::GetPreallocatedStackOverflowException());

pExInfo->m_StackTraceInfo.AppendElement(canAllocateMemory, NULL, GetRegdisplaySP(pExInfo->m_frameIter.m_crawl.GetRegisterSet()), pMD, &pExInfo->m_frameIter.m_crawl);
pExInfo->m_StackTraceInfo.SaveStackTrace(canAllocateMemory, pExInfo->m_hThrowable, /*bReplaceStack*/FALSE, /*bSkipLastElement*/FALSE);

#if defined(DEBUGGING_SUPPORTED)
if (NotifyDebuggerOfStub(pThread, pFrame))
Expand Down Expand Up @@ -8461,6 +8498,8 @@ extern "C" bool QCALLTYPE SfiNext(StackFrameIterator* pThis, uint* uExCollideCla
!(pTopExInfo->m_exception == CLRException::GetPreallocatedStackOverflowException());

pTopExInfo->m_StackTraceInfo.AppendElement(canAllocateMemory, NULL, GetRegdisplaySP(pTopExInfo->m_frameIter.m_crawl.GetRegisterSet()), pMD, &pTopExInfo->m_frameIter.m_crawl);
pTopExInfo->m_StackTraceInfo.SaveStackTrace(canAllocateMemory, pTopExInfo->m_hThrowable, /*bReplaceStack*/FALSE, /*bSkipLastElement*/FALSE);

#if defined(DEBUGGING_SUPPORTED)
if (NotifyDebuggerOfStub(pThread, pFrame))
{
Expand Down
29 changes: 29 additions & 0 deletions src/coreclr/vm/exinfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ ExInfo::ExInfo(Thread *pThread, EXCEPTION_RECORD *pExceptionRecord, CONTEXT *pEx
{
m_StackTraceInfo.AllocateStackTrace();
pThread->GetExceptionState()->m_pCurrentTracker = this;
m_pInitialFrame = pThread->GetFrame();
if (exceptionKind == ExKind::HardwareFault)
{
// Hardware exception handling needs to start on the FaultingExceptionFrame, so we are
Expand Down Expand Up @@ -383,8 +384,36 @@ void ExInfo::ReleaseResources()
void ExInfo::PopExInfos(Thread *pThread, void *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 (pExInfo)
{
ThreadExceptionState* pExState = pThread->GetExceptionState();

// If the exception is intercepted, then pop trackers according to the stack frame at which
// the exception is intercepted. We must retrieve the frame pointer before we start popping trackers.
if (pExState->GetFlags()->DebuggerInterceptInfo())
{
pExState->GetDebuggerState()->GetDebuggerInterceptInfo(NULL, NULL, (PBYTE*)&dwInterceptStackFrame,
NULL, NULL);
}
}
#endif // DEBUGGING_SUPPORTED

while (pExInfo && pExInfo < (void*)targetSp)
{
#if defined(DEBUGGING_SUPPORTED)
if (g_pDebugInterface != NULL)
{
if (pExInfo->m_ScannedStackRange.GetUpperBound().SP < dwInterceptStackFrame)
{
g_pDebugInterface->DeleteInterceptContext(pExInfo->m_DebuggerExState.GetDebuggerInterceptContext());
}
}
#endif // DEBUGGING_SUPPORTED

pExInfo->ReleaseResources();
pExInfo = (PTR_ExInfo)pExInfo->m_pPrevNestedInfo;
}
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/vm/exinfo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,8 @@ struct ExInfo : public ExceptionTrackerBase
// CONTEXT and REGDISPLAY used by the StackFrameIterator for stack walking
CONTEXT m_exContext;
REGDISPLAY m_regDisplay;
// Initial explicit frame for stack walking
Frame *m_pInitialFrame;

#if defined(TARGET_UNIX)
void TakeExceptionPointersOwnership(PAL_SEHException* ex);
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/vm/stackwalk.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ namespace AsmOffsetsAsserts
class AsmOffsets;
};

#ifdef FEATURE_EH_FUNCLETS
extern "C" void QCALLTYPE AppendExceptionStackFrame(QCall::ObjectHandleOnStack exceptionObj, SIZE_T ip, SIZE_T sp, int flags, ExInfo *pExInfo);
#endif

class CrawlFrame
{
public:
Expand Down Expand Up @@ -459,6 +463,7 @@ class CrawlFrame
friend class StackFrameIterator;
#ifdef FEATURE_EH_FUNCLETS
friend class ExceptionTracker;
friend void QCALLTYPE AppendExceptionStackFrame(QCall::ObjectHandleOnStack exceptionObj, SIZE_T ip, SIZE_T sp, int flags, ExInfo *pExInfo);
#endif // FEATURE_EH_FUNCLETS

CodeManState codeManState;
Expand Down

0 comments on commit 6b26684

Please sign in to comment.