From 0e14371a2638dd27222fd63af6032b5cd1a64add Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Thu, 18 Feb 2016 23:36:43 +0100 Subject: 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. --- src/vm/frames.cpp | 22 +++++++---- src/vm/frames.h | 115 ++++++++++++++++++++++++++++++++++-------------------- 2 files changed, 86 insertions(+), 51 deletions(-) (limited to 'src/vm') diff --git a/src/vm/frames.cpp b/src/vm/frames.cpp index 27d8a6e9e5..eff32f333a 100644 --- a/src/vm/frames.cpp +++ b/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/vm/frames.h b/src/vm/frames.h index 548ca8f03e..41e35bf0f4 100644 --- a/src/vm/frames.h +++ b/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) }; -- cgit v1.2.3