Fix the explicit frames unwinding on Unix

This change fixes a problem when exception unwinds native frames of code that contains
explicit frames. There were two issues. First, the destructor of the explicit frame
base class Frame that was responsible for popping the frames was missing switch to
GC cooperative mode and so it could pop frames that GC was currently walking.
And second, after adding the switch to cooperative mode, it turned out that if a
thread gets blocked in the Frame destructor due to GC and the stack walker tries to
get stack cookie from FrameWithCookie wrapper of a class derived from the Frame,
it fails since the vtable pointer was already reset to the vtable of the base Frame
class and it detects that as an invalid case.

The fix was to add destructors to all final classes derived from Frame and perform
the frame popping from there.

Commit migrated from https://github.com/dotnet/coreclr/commit/0e14371a2638dd27222fd63af6032b5cd1a64add

diff --git a/src/coreclr/src/vm/frames.cpp b/src/coreclr/src/vm/frames.cpp
index 27d8a6e..eff32f3 100644 (file)
--- a/src/coreclr/src/vm/frames.cpp
+++ b/src/coreclr/src/vm/frames.cpp
@@ -481,21 +481,27 @@ VOID Frame::Pop(Thread *pThread)
     m_Next = NULL;
 }
 
-#ifdef FEATURE_PAL     
-Frame::~Frame()        
+#if defined(FEATURE_PAL) && !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE)
+void Frame::PopIfChained()
 {      
+    CONTRACTL
+    {
+        NOTHROW;
+        GC_NOTRIGGER;
+        MODE_COOPERATIVE;
+        SO_TOLERANT;
+    }
+    CONTRACTL_END;
+
     if (m_Next != NULL)
     {
+        GCX_COOP();
         // When the frame is destroyed, make sure it is no longer in the
         // frame chain managed by the Thread.
-        Thread* pThread = GetThread();
-        if (pThread != NULL && pThread->GetFrame() == this)
-        {
-            Pop(pThread);
-        }
+        Pop();
     }
 }      
-#endif // FEATURE_PAL
+#endif // FEATURE_PAL && !DACCESS_COMPILE && !CROSSGEN_COMPILE
 
 //-----------------------------------------------------------------------
 #endif // #ifndef DACCESS_COMPILE

diff --git a/src/coreclr/src/vm/frames.h b/src/coreclr/src/vm/frames.h
index 548ca8f..41e35bf 100644 (file)
--- a/src/coreclr/src/vm/frames.h
+++ b/src/coreclr/src/vm/frames.h
@@ -340,6 +340,18 @@ class ComCallMethodDesc;
 
 #ifndef DACCESS_COMPILE
 
+#if defined(FEATURE_PAL) && !defined(CROSSGEN_COMPILE)
+
+#define DEFINE_DTOR(klass)                      \
+    public:                                     \
+        virtual ~klass() { PopIfChained(); }
+
+#else
+
+#define DEFINE_DTOR(klass)
+
+#endif // FEATURE_PAL && !CROSSGEN_COMPILE
+
 #define DEFINE_VTABLE_GETTER(klass)             \
     public:                                     \
         static TADDR GetMethodFrameVPtr() {     \
@@ -349,11 +361,20 @@ class ComCallMethodDesc;
         }                                       \
         klass(bool dummy) { LIMITED_METHOD_CONTRACT; }
 
+#define DEFINE_VTABLE_GETTER_AND_DTOR(klass)    \
+        DEFINE_VTABLE_GETTER(klass)             \
+        DEFINE_DTOR(klass)
+
 #define DEFINE_VTABLE_GETTER_AND_CTOR(klass)    \
         DEFINE_VTABLE_GETTER(klass)             \
     protected:                                  \
         klass() { LIMITED_METHOD_CONTRACT; }
 
+#define DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(klass)    \
+        DEFINE_VTABLE_GETTER_AND_DTOR(klass)             \
+    protected:                                           \
+        klass() { LIMITED_METHOD_CONTRACT; }
+
 #else
 
 #define DEFINE_VTABLE_GETTER(klass)             \
@@ -363,9 +384,15 @@ class ComCallMethodDesc;
             return klass::VPtrTargetVTable();   \
         }                                       \
 
+#define DEFINE_VTABLE_GETTER_AND_DTOR(klass)    \
+        DEFINE_VTABLE_GETTER(klass)             \
+
 #define DEFINE_VTABLE_GETTER_AND_CTOR(klass)    \
         DEFINE_VTABLE_GETTER(klass)             \
 
+#define DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(klass)    \
+        DEFINE_VTABLE_GETTER_AND_CTOR(klass)             \
+
 #endif // #ifndef DACCESS_COMPILE
 
 //-----------------------------------------------------------------------------
@@ -417,10 +444,6 @@ class Frame : public FrameBase
 
 public:
 
-#if defined(FEATURE_PAL) && !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE)
-    virtual ~Frame();
-#endif // FEATURE_PAL && !DACCESS_COMPILE && !CROSSGEN_COMPILE
-
     //------------------------------------------------------------------------
     // Special characteristics of a frame
     //------------------------------------------------------------------------
@@ -848,6 +871,12 @@ protected:
     { 
         LIMITED_METHOD_CONTRACT;
     }
+
+#if defined(FEATURE_PAL) && !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE)
+    virtual ~Frame() { LIMITED_METHOD_CONTRACT; }
+
+    void PopIfChained();
+#endif // FEATURE_PAL && !DACCESS_COMPILE && !CROSSGEN_COMPILE
 };
 
 
@@ -909,7 +938,7 @@ protected:
     PTR_CONTEXT m_Regs;
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(ResumableFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(ResumableFrame)
 };
 
 
@@ -932,7 +961,7 @@ public:
 #endif
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(RedirectedThreadFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(RedirectedThreadFrame)
 };
 
 typedef DPTR(RedirectedThreadFrame) PTR_RedirectedThreadFrame;
@@ -1155,7 +1184,7 @@ public:
     virtual void UpdateRegDisplay(const PREGDISPLAY);
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER(FaultingExceptionFrame)
+    DEFINE_VTABLE_GETTER_AND_DTOR(FaultingExceptionFrame)
 };
 
 //-----------------------------------------------------------------------
@@ -1234,7 +1263,7 @@ public:
     }
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(FuncEvalFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(FuncEvalFrame)
 };
 
 typedef VPTR(FuncEvalFrame) PTR_FuncEvalFrame;
@@ -1392,7 +1421,7 @@ protected:
     LazyMachState m_MachState;       // pRetAddr points to the return address and the stack arguments
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(HelperMethodFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(HelperMethodFrame)
 };
 
 // Restores registers saved in m_MachState
@@ -1477,7 +1506,7 @@ private:
     PTR_OBJECTREF gcPtrs[1];
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(HelperMethodFrame_1OBJ)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(HelperMethodFrame_1OBJ)
 };
 
 
@@ -1538,7 +1567,7 @@ private:
     PTR_OBJECTREF gcPtrs[2];
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(HelperMethodFrame_2OBJ)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(HelperMethodFrame_2OBJ)
 };
 
 
@@ -1604,7 +1633,7 @@ private:
     UINT       m_numObjRefs;
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(HelperMethodFrame_PROTECTOBJ)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(HelperMethodFrame_PROTECTOBJ)
 };
 
 class FramedMethodFrame : public TransitionFrame
@@ -1760,7 +1789,7 @@ public:
                             TraceDestination *trace, REGDISPLAY *regs);
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(TPMethodFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(TPMethodFrame)
 };
 #endif // FEATURE_REMOTING
 
@@ -1833,7 +1862,7 @@ public:
                             TraceDestination *trace, REGDISPLAY *regs);
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(SecureDelegateFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(SecureDelegateFrame)
 };
 
 
@@ -1864,7 +1893,7 @@ class MulticastFrame : public SecureDelegateFrame
                             TraceDestination *trace, REGDISPLAY *regs);
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(MulticastFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(MulticastFrame)
 };
 
 
@@ -2005,7 +2034,7 @@ public:
 
 protected:
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(ComMethodFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(ComMethodFrame)
 };
 
 typedef DPTR(class ComMethodFrame) PTR_ComMethodFrame;
@@ -2043,7 +2072,7 @@ public:
                     TraceDestination *trace, REGDISPLAY *regs);
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(ComPlusMethodFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(ComPlusMethodFrame)
 };
 
 #endif // FEATURE_COMINTEROP
@@ -2110,7 +2139,7 @@ public:
     }
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(PInvokeCalliFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(PInvokeCalliFrame)
 };
 
 // Some context-related forwards.
@@ -2155,7 +2184,7 @@ protected:
     DPTR(HijackArgs)    m_Args;
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(HijackFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(HijackFrame)
 };
 
 #endif // FEATURE_HIJACK
@@ -2199,7 +2228,7 @@ public:
     Interception GetInterception();
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(PrestubMethodFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(PrestubMethodFrame)
 };
 
 //------------------------------------------------------------------------
@@ -2287,7 +2316,7 @@ private:
     friend class VirtualCallStubManager;
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(StubDispatchFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(StubDispatchFrame)
 };
 
 typedef VPTR(class StubDispatchFrame) PTR_StubDispatchFrame;
@@ -2341,7 +2370,7 @@ public:
 #endif
     
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(ExternalMethodFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(ExternalMethodFrame)
 };
 
 typedef VPTR(class ExternalMethodFrame) PTR_ExternalMethodFrame;
@@ -2373,7 +2402,7 @@ public:
     }
     
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(DynamicHelperFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(DynamicHelperFrame)
 };
 
 typedef VPTR(class DynamicHelperFrame) PTR_DynamicHelperFrame;
@@ -2409,7 +2438,7 @@ class StubHelperFrame : public TransitionFrame
 
 private:
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(StubHelperFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(StubHelperFrame)
 };
 #endif // _TARGET_X86_
 
@@ -2460,7 +2489,7 @@ public:
 
 private:
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(ComPrestubMethodFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(ComPrestubMethodFrame)
 };
 
 #endif // FEATURE_COMINTEROP
@@ -2531,7 +2560,7 @@ private:
     BOOL          m_MaybeInterior;
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER(GCFrame)
+    DEFINE_VTABLE_GETTER_AND_DTOR(GCFrame)
 };
 
 #ifdef FEATURE_INTERPRETER
@@ -2554,7 +2583,7 @@ public:
     MethodDesc* GetFunction();
 #endif
 
-    DEFINE_VTABLE_GETTER(InterpreterFrame)
+    DEFINE_VTABLE_GETTER_AND_DTOR(InterpreterFrame)
 
 };
 
@@ -2580,7 +2609,7 @@ class GCSafeCollectionFrame : public Frame
 
         VOID Pop();
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER(GCSafeCollectionFrame)
+    DEFINE_VTABLE_GETTER_AND_DTOR(GCSafeCollectionFrame)
 };
 #endif // FEATURE_REMOTING
 
@@ -2622,7 +2651,7 @@ private:
     PTR_ByRefInfo m_brInfo;
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(ProtectByRefsFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(ProtectByRefsFrame)
 };
 
 
@@ -2682,7 +2711,7 @@ private:
     ValueClassInfo *m_pVCInfo;
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER(ProtectValueClassFrame)
+    DEFINE_VTABLE_GETTER_AND_DTOR(ProtectValueClassFrame)
 };
 
 
@@ -2725,7 +2754,7 @@ public:
     }
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER(DebuggerClassInitMarkFrame)
+    DEFINE_VTABLE_GETTER_AND_DTOR(DebuggerClassInitMarkFrame)
 };
 
 
@@ -2762,7 +2791,7 @@ public:
     }
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER(DebuggerSecurityCodeMarkFrame)
+    DEFINE_VTABLE_GETTER_AND_DTOR(DebuggerSecurityCodeMarkFrame)
 };
 
 //------------------------------------------------------------------------
@@ -2817,7 +2846,7 @@ public:
     }
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER(DebuggerExitFrame)
+    DEFINE_VTABLE_GETTER_AND_DTOR(DebuggerExitFrame)
 };
 
 //---------------------------------------------------------------------------------------
@@ -2857,7 +2886,7 @@ public:
     }
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER(DebuggerU2MCatchHandlerFrame)
+    DEFINE_VTABLE_GETTER_AND_DTOR(DebuggerU2MCatchHandlerFrame)
 };
 
 
@@ -2893,7 +2922,7 @@ public:
 protected:
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(UMThkCallFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(UMThkCallFrame)
 };
 #endif // _TARGET_X86_
 
@@ -2951,7 +2980,7 @@ public:
     }
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(ReverseEnterRuntimeFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(ReverseEnterRuntimeFrame)
 };
 
 //-----------------------------------------------------------------------------
@@ -2975,7 +3004,7 @@ public:
 #endif
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(LeaveRuntimeFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(LeaveRuntimeFrame)
 };
 #endif
 
@@ -3150,7 +3179,7 @@ public:
     VOID Init();
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(InlinedCallFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(InlinedCallFrame)
 };
 
 //------------------------------------------------------------------------
@@ -3241,7 +3270,7 @@ public:
 #endif
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER(ContextTransitionFrame)
+    DEFINE_VTABLE_GETTER_AND_DTOR(ContextTransitionFrame)
 };
 
 // TODO [DAVBR]: For the full fix for VsWhidbey 450273, this
@@ -3379,7 +3408,7 @@ public:
 
 private:
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(TailCallFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(TailCallFrame)
 };
 
 //------------------------------------------------------------------------
@@ -3425,7 +3454,7 @@ public:
 
 private:
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(ExceptionFilterFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(ExceptionFilterFrame)
 };
 
 #ifdef _DEBUG
@@ -3454,7 +3483,7 @@ private:
     OBJECTREF *m_pObjRef;
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER(AssumeByrefFromJITStack)
+    DEFINE_VTABLE_GETTER_AND_DTOR(AssumeByrefFromJITStack)
 }; //AssumeByrefFromJITStack
 
 #endif //_DEBUG
@@ -3647,7 +3676,7 @@ public:
     void SetAssembly(Assembly *pAssembly) { LIMITED_METHOD_CONTRACT; m_pAssembly = pAssembly; }
 
     // Keep as last entry in class
-    DEFINE_VTABLE_GETTER_AND_CTOR(SecurityContextFrame)
+    DEFINE_VTABLE_GETTER_AND_CTOR_AND_DTOR(SecurityContextFrame)
 };