From f3c48c513d1556332d6187477c2993b94108195d Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Tue, 7 Mar 2017 21:30:02 -0800 Subject: [PATCH] Remove TRACK_CXX_EXCEPTION_CODE_HACK and related code. (dotnet/coreclr#10018) Commit migrated from https://github.com/dotnet/coreclr/commit/08e125e154eaaa9a1f124945b54684c5b09c5b25 --- src/coreclr/src/vm/clrex.cpp | 74 +--------------------------------- src/coreclr/src/vm/i386/asmconstants.h | 12 +----- src/coreclr/src/vm/i386/asmhelpers.asm | 51 ----------------------- src/coreclr/src/vm/threads.cpp | 4 -- src/coreclr/src/vm/threads.h | 3 -- 5 files changed, 2 insertions(+), 142 deletions(-) diff --git a/src/coreclr/src/vm/clrex.cpp b/src/coreclr/src/vm/clrex.cpp index 7c70bc9..1c1501e 100644 --- a/src/coreclr/src/vm/clrex.cpp +++ b/src/coreclr/src/vm/clrex.cpp @@ -2478,10 +2478,6 @@ CLRLastThrownObjectException* CLRLastThrownObjectException::Validate() DWORD dwCurrentExceptionCode = GetCurrentExceptionCode(); -#if HAS_TRACK_CXX_EXCEPTION_CODE_HACK - DWORD dwLastCxxSEHExceptionCode = pThread->m_LastCxxSEHExceptionCode; -#endif // HAS_TRACK_CXX_EXCEPTION_CODE_HACK - if (dwCurrentExceptionCode == BOOTUP_EXCEPTION_COMPLUS) { // BOOTUP_EXCEPTION_COMPLUS can be thrown when a thread setup is failed due to reasons like @@ -2521,75 +2517,7 @@ CLRLastThrownObjectException* CLRLastThrownObjectException::Validate() // // This also ensures that the handling of BOOTUP_EXCEPTION_COMPLUS is now insync between the chk and fre builds in terms of the throwable returned. } - else - -#if HAS_TRACK_CXX_EXCEPTION_CODE_HACK // ON x86, we grab the exception code. - - // The exception code can legitimately take several values. - // The most obvious is EXCEPTION_COMPLUS, as when managed code does 'throw new Exception'. - // Another case is EXCEPTION_MSVC, when we EX_RETHROW a CLRLastThrownObjectException, which will - // throw an actual CLRLastThrownObjectException C++ exception. - // Other values are possible, if we are wrapping an SEH exception (say, AV) in - // a managed exception. In these other cases, the exception object should have - // an XCode that is the same as the exception code. - // So, if the exception code isn't EXCEPTION_COMPLUS, and isn't EXCEPTION_MSVC, then - // we shouldn't be getting a CLRLastThrownObjectException. This indicates that - // we are missing a "callout filter", which should have transformed the SEH - // exception into a COMPLUS exception. - // It also turns out that sometimes we see STATUS_UNWIND more recently than the exception - // code. In that case, we have lost the original exception code, and so can't check. - - if (dwLastCxxSEHExceptionCode != EXCEPTION_COMPLUS && - dwLastCxxSEHExceptionCode != EXCEPTION_MSVC && - dwLastCxxSEHExceptionCode != STATUS_UNWIND) - { - // Maybe there is an exception wrapping a Win32 fault. In that case, the - // last exception code won't be EXCEPTION_COMPLUS, but the last thrown exception - // will have an XCode equal to the last exception code. - - // Get the exception code from the exception object. - DWORD dwExceptionXCode = GetExceptionXCode(throwable); - - // If that code is the same as the last exception code, call it good... - if (dwLastCxxSEHExceptionCode != dwExceptionXCode) - { - // For rude thread abort, we may have updated the LastThrownObject without throwing exception. - BOOL fIsRudeThreadAbortException = - throwable == CLRException::GetPreallocatedRudeThreadAbortException(); - - // For stack overflow, we may have updated the LastThrownObject without throwing exception. - BOOL fIsStackOverflowException = - throwable == CLRException::GetPreallocatedStackOverflowException() && - (IsSOExceptionCode(dwLastCxxSEHExceptionCode)); - - // ... but if not, raise an error. - if (!fIsRudeThreadAbortException && !fIsStackOverflowException) - { - static int iSuppress = -1; - if (iSuppress == -1) - iSuppress = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_SuppressLostExceptionTypeAssert); - if (!iSuppress) - { - // Raising an assert message can cause a mode violation. - CONTRACT_VIOLATION(ModeViolation); - - // Use DbgAssertDialog to get the formatting right. - DbgAssertDialog(__FILE__, __LINE__, - "The 'current' exception is not EXCEPTION_COMPLUS, yet the runtime is\n" - " requesting the 'LastThrownObject'.\n" - "The runtime may have lost track of the type of an exception in flight.\n" - " Please get a good stack trace of the exception that was thrown first\n" - " (by re-running the app & catching first chance exceptions), find\n" - " the caller of Validate, and file a bug against the owner.\n\n" - "To suppress this assert 'set COMPlus_SuppressLostExceptionTypeAssert=1'"); - } - } - } - } - else -#endif // _x86_ - - if (throwable == NULL) + else if (throwable == NULL) { // If there isn't a LastThrownObject at all, that's a problem for GetLastThrownObject // We've lost track of the exception's type. Raise an assert. (This is configurable to allow // stress labs to turn off the assert.) diff --git a/src/coreclr/src/vm/i386/asmconstants.h b/src/coreclr/src/vm/i386/asmconstants.h index c5e9ae5..58451e0 100644 --- a/src/coreclr/src/vm/i386/asmconstants.h +++ b/src/coreclr/src/vm/i386/asmconstants.h @@ -33,9 +33,6 @@ #define DBG_FRE(dbg,fre) fre #endif -//*************************************************************************** - #define HAS_TRACK_CXX_EXCEPTION_CODE_HACK 0 - #define INITIAL_SUCCESS_COUNT 0x100 #define DynamicHelperFrameFlags_Default 0 @@ -195,14 +192,7 @@ ASMCONSTANTS_C_ASSERT(CORINFO_ArgumentException_ASM == CORINFO_ArgumentException #ifndef CROSSGEN_COMPILE // from clr/src/vm/threads.h -#if defined(TRACK_CXX_EXCEPTION_CODE_HACK) // Is C++ exception code tracking turned on? - #define Thread_m_LastCxxSEHExceptionCode 0x20 - ASMCONSTANTS_C_ASSERT(Thread_m_LastCxxSEHExceptionCode == offsetof(Thread, m_LastCxxSEHExceptionCode)) - - #define Thread_m_Context 0x3C -#else - #define Thread_m_Context 0x38 -#endif // TRACK_CXX_EXCEPTION_CODE_HACK +#define Thread_m_Context 0x38 ASMCONSTANTS_C_ASSERT(Thread_m_Context == offsetof(Thread, m_Context)) #define Thread_m_State 0x04 diff --git a/src/coreclr/src/vm/i386/asmhelpers.asm b/src/coreclr/src/vm/i386/asmhelpers.asm index 2315b3e..9df3321 100644 --- a/src/coreclr/src/vm/i386/asmhelpers.asm +++ b/src/coreclr/src/vm/i386/asmhelpers.asm @@ -61,11 +61,6 @@ endif ; FEATURE_IMPLICIT_TLS EXTERN _VarargPInvokeStubWorker@12:PROC EXTERN _GenericPInvokeCalliStubWorker@12:PROC -; To debug that LastThrownObjectException really is EXCEPTION_COMPLUS -ifdef TRACK_CXX_EXCEPTION_CODE_HACK -EXTERN __imp____CxxFrameHandler:PROC -endif - EXTERN _GetThread@0:PROC EXTERN _GetAppDomain@0:PROC @@ -271,52 +266,6 @@ _RestoreFPUContext@4 PROC public _RestoreFPUContext@4 ENDP -ifndef FEATURE_CORECLR -ifdef _DEBUG -; For C++ exceptions, we desperately need to know the SEH code. This allows us to properly -; distinguish managed exceptions from C++ exceptions from standard SEH like hard stack overflow. -; We do this by providing our own handler that squirrels away the exception code and then -; defers to the C++ service. Fortunately, two symbols exist for the C++ symbol. -___CxxFrameHandler3 PROC public - - ; We don't know what arguments are passed to us (except for the first arg on stack) - ; It turns out that EAX is part of the non-standard calling convention of this - ; function. - - push eax - push edx - - cmp dword ptr [_gThreadTLSIndex], -1 - je Chain ; CLR is not initialized yet - - call _GetThread@0 - - test eax, eax ; not a managed thread - jz Chain - - mov edx, [esp + 0ch] ; grab the first argument - mov edx, [edx] ; grab the SEH exception code - - mov dword ptr [eax + Thread_m_LastCxxSEHExceptionCode], edx - -Chain: - - pop edx - - ; [esp] contains the value of EAX we must restore. We would like - ; [esp] to contain the address of the real imported CxxFrameHandler - ; so we can chain to it. - - mov eax, [__imp____CxxFrameHandler] - mov eax, [eax] - xchg [esp], eax - - ret - -___CxxFrameHandler3 ENDP -endif ; _DEBUG -endif ; FEATURE_CORECLR - ; Register CLR exception handlers defined on the C++ side with SAFESEH. ; Note that these directives must be in a file that defines symbols that will be used during linking, ; otherwise it's possible that the resulting .obj will completly be ignored by the linker and these diff --git a/src/coreclr/src/vm/threads.cpp b/src/coreclr/src/vm/threads.cpp index 48c10ab..b6047b0 100644 --- a/src/coreclr/src/vm/threads.cpp +++ b/src/coreclr/src/vm/threads.cpp @@ -1773,10 +1773,6 @@ Thread::Thread() m_LastThrownObjectHandle = NULL; m_ltoIsUnhandled = FALSE; - #if HAS_TRACK_CXX_EXCEPTION_CODE_HACK // Is C++ exception code tracking turned on?vs - m_LastCxxSEHExceptionCode = 0; - #endif // HAS_TRACK_CXX_EXCEPTION_CODE_HACK - m_AbortReason = NULL; diff --git a/src/coreclr/src/vm/threads.h b/src/coreclr/src/vm/threads.h index d006f5f..a5ab7c8 100644 --- a/src/coreclr/src/vm/threads.h +++ b/src/coreclr/src/vm/threads.h @@ -552,9 +552,6 @@ inline Thread* GetThreadNULLOk() #define GetThreadNULLOk() GetThread() #endif -//*************************************************************************** - #define HAS_TRACK_CXX_EXCEPTION_CODE_HACK 0 - // manifest constant for waiting in the exposed classlibs const INT32 INFINITE_TIMEOUT = -1; -- 2.7.4