Fix the problem with the VS2019 fix on x86 (dotnet/coreclr#26957)
authorJan Vorlicek <janvorli@microsoft.com>
Tue, 1 Oct 2019 14:46:11 +0000 (16:46 +0200)
committerGitHub <noreply@github.com>
Tue, 1 Oct 2019 14:46:11 +0000 (16:46 +0200)
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.

Commit migrated from https://github.com/dotnet/coreclr/commit/4081d86f221e6694f0dfd13b99e0d5b1045d2e89

src/coreclr/src/vm/exstate.cpp
src/coreclr/src/vm/exstate.h
src/coreclr/src/vm/frames.cpp
src/coreclr/src/vm/i386/excepx86.cpp

index 59eed33..79ba262 100644 (file)
@@ -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;
index 991b987..1689872 100644 (file)
@@ -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 
@@ -158,12 +160,24 @@ public:
     }
 #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
index ebb16bf..63796d9 100644 (file)
@@ -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();
         }
-
     }
 }
 
index 62d990c..c2b5aae 100644 (file)
@@ -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)
     {
@@ -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();