Skip to content

Commit

Permalink
Fix the problem with the VS2019 fix on x86 (#26957)
Browse files Browse the repository at this point in the history
The x86 was missing the same treatment of the explicit frames chain as
the other architectures. The chain needs to be repaired when a GCFrame
is destroyed and it was on a chain that is not current, but that is
still used by the exception handling stack walk.
  • Loading branch information
janvorli committed Oct 1, 2019
1 parent 08423d0 commit 4081d86
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 21 deletions.
2 changes: 2 additions & 0 deletions src/vm/exstate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ ThreadExceptionState::ThreadExceptionState()
{
#ifdef FEATURE_EH_FUNCLETS
m_pCurrentTracker = NULL;
#else
m_ppBottomFrameDuringUnwind = NULL;
#endif // FEATURE_EH_FUNCLETS

m_flag = TEF_None;
Expand Down
14 changes: 14 additions & 0 deletions src/vm/exstate.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ class EHClauseInfo;

extern StackWalkAction COMPlusUnwindCallback(CrawlFrame *pCf, ThrowCallbackType *pData);

typedef DPTR(PTR_Frame) PTR_PTR_Frame;

//
// This class serves as a forwarding and abstraction layer for the EH subsystem.
// Since we have two different implementations, this class is needed to unify
Expand Down Expand Up @@ -158,12 +160,24 @@ class ThreadExceptionState
}
#else
ExInfo m_currentExInfo;
PTR_PTR_Frame m_ppBottomFrameDuringUnwind;
public:
PTR_ExInfo GetCurrentExceptionTracker()
{
LIMITED_METHOD_CONTRACT;
return PTR_ExInfo(PTR_HOST_MEMBER_TADDR(ThreadExceptionState, this, m_currentExInfo));
}

PTR_PTR_Frame GetPtrToBottomFrameDuringUnwind()
{
return m_ppBottomFrameDuringUnwind;
}

void SetPtrToBottomFrameDuringUnwind(PTR_PTR_Frame framePtr)
{
m_ppBottomFrameDuringUnwind = framePtr;
}

#endif

#ifdef FEATURE_CORRUPTING_EXCEPTIONS
Expand Down
55 changes: 34 additions & 21 deletions src/vm/frames.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1031,43 +1031,56 @@ GCFrame::~GCFrame()

Pop();

#if defined(FEATURE_EH_FUNCLETS) && !defined(FEATURE_PAL)
#ifndef FEATURE_PAL

PTR_Frame frame = NULL;

#ifdef FEATURE_EH_FUNCLETS
PTR_ExceptionTracker pCurrentTracker = m_pCurThread->GetExceptionState()->GetCurrentExceptionTracker();
if (pCurrentTracker != NULL)
{
if (pCurrentTracker->GetLimitFrame() == this)
frame = pCurrentTracker->GetInitialExplicitFrame();
}
#else
PTR_PTR_Frame ptrToInitialFrame = m_pCurThread->GetExceptionState()->GetPtrToBottomFrameDuringUnwind();
if (ptrToInitialFrame != NULL)
{
frame = *ptrToInitialFrame;
if (frame == this)
{
// The current frame that was just popped was the EH limit frame. We need to reset the EH limit frame
// to the current frame so that it stays on the frame chain from initial explicit frame.
// The ExceptionTracker::HasFrameBeenUnwoundByAnyActiveException needs that to correctly detect
// frames that were unwound.
pCurrentTracker->ResetLimitFrame();
// The current frame that was just popped was the bottom frame used
// as an initial frame to scan stack frames.
// Update the bottom frame pointer to point to the first valid frame.
*ptrToInitialFrame = m_pCurThread->m_pFrame;
}
}
#endif // FEATURE_EH_FUNCLETS

if (frame != NULL)
{
// There is an initial explicit frame, so we need to scan the explicit frame chain starting at
// that frame to see if the current frame that is being destroyed was on the chain.

PTR_Frame frame = pCurrentTracker->GetInitialExplicitFrame();
if (frame != NULL)
while ((frame != FRAME_TOP) && (frame != this))
{
while ((frame != FRAME_TOP) && (frame != this))
PTR_Frame nextFrame = frame->PtrNextFrame();
if (nextFrame == this)
{
PTR_Frame nextFrame = frame->PtrNextFrame();
if (nextFrame == this)
{
// Repair frame chain from the initial explicit frame to the current frame,
// skipping the current GCFrame that was destroyed
frame->m_Next = m_pCurThread->m_pFrame;
break;
}
frame = nextFrame;
// Repair frame chain from the initial explicit frame to the current frame,
// skipping the current GCFrame that was destroyed
frame->m_Next = m_pCurThread->m_pFrame;
break;
}
frame = nextFrame;
_ASSERTE(frame != NULL);
}
}
#endif // FEATURE_EH_FUNCLETS && !FEATURE_PAL
#endif // !FEATURE_PAL

if (!wasCoop)
{
m_pCurThread->EnablePreemptiveGC();
}

}
}

Expand Down
15 changes: 15 additions & 0 deletions src/vm/i386/excepx86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -554,8 +554,15 @@ EXCEPTION_DISPOSITION ClrDebuggerDoUnwindAndIntercept(EXCEPTION_REGISTRATION_REC
LOG((LF_EH|LF_CORDB, LL_INFO100, "\t\t: pFunc is 0x%X\n", tct.pFunc));
LOG((LF_EH|LF_CORDB, LL_INFO100, "\t\t: pStack is 0x%X\n", tct.pStack));

_ASSERTE(pExState->GetPtrToBottomFrameDuringUnwind() == NULL);
pExState->SetPtrToBottomFrameDuringUnwind(&tct.pBottomFrame);

CallRtlUnwindSafe(pEstablisherFrame, RtlUnwindCallback, pExceptionRecord, 0);

_ASSERTE(pExState->GetPtrToBottomFrameDuringUnwind() == &tct.pBottomFrame);
_ASSERTE(*pExState->GetPtrToBottomFrameDuringUnwind() == tct.pBottomFrame);
pExState->SetPtrToBottomFrameDuringUnwind(NULL);

ExInfo* pExInfo = pThread->GetExceptionState()->GetCurrentExceptionTracker();
if (pExInfo->m_ValidInterceptionContext)
{
Expand Down Expand Up @@ -1234,9 +1241,17 @@ CPFH_RealFirstPassHandler( // ExceptionContinueSearch, etc.

LOG((LF_EH, LL_INFO100, "CPFH_RealFirstPassHandler: handler found: %s\n", tct.pFunc->m_pszDebugMethodName));

ThreadExceptionState* pExState = pThread->GetExceptionState();
_ASSERTE(pExState->GetPtrToBottomFrameDuringUnwind() == NULL);
pExState->SetPtrToBottomFrameDuringUnwind(&tct.pBottomFrame);

CallRtlUnwindSafe(pEstablisherFrame, RtlUnwindCallback, pExceptionRecord, 0);
// on x86 at least, RtlUnwind always returns

_ASSERTE(pExState->GetPtrToBottomFrameDuringUnwind() == &tct.pBottomFrame);
_ASSERTE(*pExState->GetPtrToBottomFrameDuringUnwind() == tct.pBottomFrame);
pExState->SetPtrToBottomFrameDuringUnwind(NULL);

// The CallRtlUnwindSafe could have popped the explicit frame that the tct.pBottomFrame points to (UMThunkPrestubHandler
// does that). In such case, the tct.pBottomFrame needs to be updated to point to the first valid explicit frame.
Frame* frame = pThread->GetFrame();
Expand Down

0 comments on commit 4081d86

Please sign in to comment.