diff --git a/src/coreclr/inc/clrhost.h b/src/coreclr/inc/clrhost.h index a1d22b6ee269d1..8d8309f8187bdb 100644 --- a/src/coreclr/inc/clrhost.h +++ b/src/coreclr/inc/clrhost.h @@ -29,17 +29,20 @@ using std::nothrow; #define _DEBUG_IMPL 1 #endif -#define BEGIN_PRESERVE_LAST_ERROR \ - { \ - DWORD __dwLastError = ::GetLastError(); \ - DEBUG_ASSURE_NO_RETURN_BEGIN(PRESERVE_LAST_ERROR); \ - { - -#define END_PRESERVE_LAST_ERROR \ - } \ - DEBUG_ASSURE_NO_RETURN_END(PRESERVE_LAST_ERROR); \ - ::SetLastError(__dwLastError); \ +struct PreserveLastErrorHolder +{ + PreserveLastErrorHolder() + { + m_dwLastError = ::GetLastError(); + } + + ~PreserveLastErrorHolder() + { + ::SetLastError(m_dwLastError); } +private: + DWORD m_dwLastError; +}; // // TRASH_LASTERROR macro sets bogus last error in debug builds to help find places that fail to save it diff --git a/src/coreclr/inc/contract.h b/src/coreclr/inc/contract.h index a8f3f6f47c2263..44be057ffc7cf5 100644 --- a/src/coreclr/inc/contract.h +++ b/src/coreclr/inc/contract.h @@ -1160,7 +1160,6 @@ typedef __SafeToUsePostCondition __PostConditionOK; Contract::RanPostconditions ___ran(__FUNCTION__); \ Contract::Operation ___op = Contract::Setup; \ BOOL ___contract_enabled = FALSE; \ - DEBUG_ASSURE_NO_RETURN_BEGIN(CONTRACT) \ ___contract_enabled = Contract::EnforceContract(); \ enum {___disabled = 0}; \ if (!___contract_enabled) \ @@ -1181,10 +1180,8 @@ typedef __SafeToUsePostCondition __PostConditionOK; } \ else \ { \ - DEBUG_OK_TO_RETURN_BEGIN(CONTRACT) \ ___run_return: \ return _returnexp; \ - DEBUG_OK_TO_RETURN_END(CONTRACT) \ } \ } \ if (0) \ @@ -1226,7 +1223,6 @@ typedef __SafeToUsePostCondition __PostConditionOK; Contract::Returner<_returntype> ___returner(RETVAL); \ Contract::RanPostconditions ___ran(__FUNCTION__); \ Contract::Operation ___op = Contract::Setup; \ - DEBUG_ASSURE_NO_RETURN_BEGIN(CONTRACT) \ BOOL ___contract_enabled = Contract::EnforceContract(); \ enum {___disabled = 0}; \ { \ @@ -1244,10 +1240,8 @@ typedef __SafeToUsePostCondition __PostConditionOK; } \ else \ { \ - DEBUG_OK_TO_RETURN_BEGIN(CONTRACT) \ ___run_return: \ return _returnexp; \ - DEBUG_OK_TO_RETURN_END(CONTRACT) \ } \ } \ if (0) \ @@ -1460,8 +1454,7 @@ typedef __SafeToUsePostCondition __PostConditionOK; #endif // __FORCE_NORUNTIME_CONTRACTS__ -#define CONTRACT_END CONTRACTL_END \ - DEBUG_ASSURE_NO_RETURN_END(CONTRACT) \ +#define CONTRACT_END CONTRACTL_END // The final expression in the RETURN macro deserves special explanation (or something.) @@ -1679,7 +1672,6 @@ class AutoCleanupContractViolationHolder : ContractViolationHolder __violationHolder_onlyOneAllowedPerScope; \ __violationHolder_onlyOneAllowedPerScope.Enter(); \ - DEBUG_ASSURE_NO_RETURN_BEGIN(CONTRACT) \ // Use this to jump out prematurely from a violation. Used for EH // when the function might not return @@ -1687,7 +1679,6 @@ class AutoCleanupContractViolationHolder : ContractViolationHolder 190024315) #define DEBUG_ASSURE_SAFE_TO_RETURN TRUE @@ -114,9 +108,6 @@ typedef __SafeToReturn __ReturnOK; #define DEBUG_ASSURE_NO_RETURN_BEGIN(arg) { #define DEBUG_ASSURE_NO_RETURN_END(arg) } -#define DEBUG_OK_TO_RETURN_BEGIN(arg) { -#define DEBUG_OK_TO_RETURN_END(arg) } - #endif // defined(_DEBUG) && !defined(JIT_BUILD) && (!defined(_MSC_FULL_VER) || _MSC_FULL_VER > 190024315) #endif // !_PREFAST_ diff --git a/src/coreclr/vm/FrameTypes.h b/src/coreclr/vm/FrameTypes.h index 59e1c56356e80e..a75ac7cbdc973c 100644 --- a/src/coreclr/vm/FrameTypes.h +++ b/src/coreclr/vm/FrameTypes.h @@ -47,8 +47,5 @@ FRAME_TYPE_NAME(DebuggerClassInitMarkFrame) FRAME_TYPE_NAME(DebuggerExitFrame) FRAME_TYPE_NAME(DebuggerU2MCatchHandlerFrame) FRAME_TYPE_NAME(ExceptionFilterFrame) -#if defined(_DEBUG) -FRAME_TYPE_NAME(AssumeByrefFromJITStackFrame) -#endif // _DEBUG #undef FRAME_TYPE_NAME diff --git a/src/coreclr/vm/callcounting.cpp b/src/coreclr/vm/callcounting.cpp index 3b2df30ed4cfcc..dfa23e90f4e8d9 100644 --- a/src/coreclr/vm/callcounting.cpp +++ b/src/coreclr/vm/callcounting.cpp @@ -760,7 +760,7 @@ PCODE CallCountingManager::OnCallCountThresholdReached(TransitionBlock *transiti PCODE codeEntryPoint = 0; - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; MAKE_CURRENT_THREAD_AVAILABLE(); @@ -823,8 +823,6 @@ PCODE CallCountingManager::OnCallCountThresholdReached(TransitionBlock *transiti frame->Pop(CURRENT_THREAD); - END_PRESERVE_LAST_ERROR; - return codeEntryPoint; } diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 12b02c90a39ba4..6a41d89d2c994a 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -2522,7 +2522,7 @@ extern "C" PT_RUNTIME_FUNCTION GetRuntimeFunctionCallback(IN ULONG ControlPc PT_RUNTIME_FUNCTION prf = NULL; // We must preserve this so that GCStress=4 eh processing doesnt kill last error. - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; #ifdef ENABLE_CONTRACTS // Some 64-bit OOM tests use the hosting interface to re-enter the CLR via @@ -2545,8 +2545,6 @@ extern "C" PT_RUNTIME_FUNCTION GetRuntimeFunctionCallback(IN ULONG ControlPc LOG((LF_EH, LL_INFO1000000, "GetRuntimeFunctionCallback(%p) returned %p\n", ControlPc, prf)); - END_PRESERVE_LAST_ERROR; - return prf; } #endif // FEATURE_EH_FUNCLETS @@ -4379,7 +4377,7 @@ extern "C" void GetRuntimeStackWalkInfo(IN ULONG64 ControlPc, WRAPPER_NO_CONTRACT; - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; if (pModuleBase) *pModuleBase = 0; @@ -4392,7 +4390,7 @@ extern "C" void GetRuntimeStackWalkInfo(IN ULONG64 ControlPc, #if defined(DACCESS_COMPILE) GetUnmanagedStackWalkInfo(ControlPc, pModuleBase, pFuncEntry); #endif // DACCESS_COMPILE - goto Exit; + return; } if (pModuleBase) @@ -4404,10 +4402,6 @@ extern "C" void GetRuntimeStackWalkInfo(IN ULONG64 ControlPc, { *pFuncEntry = (UINT_PTR)(PT_RUNTIME_FUNCTION)codeInfo.GetFunctionEntry(); } - -Exit: - ; - END_PRESERVE_LAST_ERROR; } #endif // FEATURE_EH_FUNCLETS diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index f070d81f95980a..7f16554ce79196 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -5828,7 +5828,7 @@ EXTERN_C LPVOID STDCALL NDirectImportWorker(NDirectMethodDesc* pMD) { LPVOID ret = NULL; - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; CONTRACTL { @@ -5879,8 +5879,6 @@ EXTERN_C LPVOID STDCALL NDirectImportWorker(NDirectMethodDesc* pMD) UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; UNINSTALL_MANAGED_EXCEPTION_DISPATCHER; - END_PRESERVE_LAST_ERROR; - return ret; } @@ -5891,7 +5889,7 @@ EXTERN_C LPVOID STDCALL NDirectImportWorker(NDirectMethodDesc* pMD) EXTERN_C void STDCALL VarargPInvokeStubWorker(TransitionBlock * pTransitionBlock, VASigCookie *pVASigCookie, MethodDesc *pMD) { - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; STATIC_CONTRACT_THROWS; STATIC_CONTRACT_GC_TRIGGERS; @@ -5915,13 +5913,11 @@ EXTERN_C void STDCALL VarargPInvokeStubWorker(TransitionBlock * pTransitionBlock GetILStubForCalli(pVASigCookie, pMD); pFrame->Pop(CURRENT_THREAD); - - END_PRESERVE_LAST_ERROR; } EXTERN_C void STDCALL GenericPInvokeCalliStubWorker(TransitionBlock * pTransitionBlock, VASigCookie * pVASigCookie, PCODE pUnmanagedTarget) { - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; STATIC_CONTRACT_THROWS; STATIC_CONTRACT_GC_TRIGGERS; @@ -5944,8 +5940,6 @@ EXTERN_C void STDCALL GenericPInvokeCalliStubWorker(TransitionBlock * pTransitio GetILStubForCalli(pVASigCookie, NULL); pFrame->Pop(CURRENT_THREAD); - - END_PRESERVE_LAST_ERROR; } PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD) diff --git a/src/coreclr/vm/excep.cpp b/src/coreclr/vm/excep.cpp index 769c142e25888a..3359b4e8f3208d 100644 --- a/src/coreclr/vm/excep.cpp +++ b/src/coreclr/vm/excep.cpp @@ -5896,9 +5896,10 @@ bool IsGcMarker(CONTEXT* pContext, EXCEPTION_RECORD *pExceptionRecord) { // GCStress processing can disturb last error, so preserve it. BOOL res; - BEGIN_PRESERVE_LAST_ERROR; - res = OnGcCoverageInterrupt(pContext); - END_PRESERVE_LAST_ERROR; + { + PreserveLastErrorHolder preserveLastError; + res = OnGcCoverageInterrupt(pContext); + } if (res) { return true; diff --git a/src/coreclr/vm/exceptmacros.h b/src/coreclr/vm/exceptmacros.h index 2c11c059853ba9..ff3eafd842da4c 100644 --- a/src/coreclr/vm/exceptmacros.h +++ b/src/coreclr/vm/exceptmacros.h @@ -342,8 +342,7 @@ VOID DECLSPEC_NORETURN DispatchManagedException(PAL_SEHException& ex, bool isHar bool __fExceptionCaught = false; \ SCAN_EHMARKER(); \ if (true) PAL_CPP_TRY { \ - SCAN_EHMARKER_TRY(); \ - DEBUG_ASSURE_NO_RETURN_BEGIN(IUACH) + SCAN_EHMARKER_TRY(); #define INSTALL_UNWIND_AND_CONTINUE_HANDLER \ INSTALL_UNWIND_AND_CONTINUE_HANDLER_EX \ @@ -358,11 +357,9 @@ VOID DECLSPEC_NORETURN DispatchManagedException(PAL_SEHException& ex, bool isHar bool __fExceptionCaught = false; \ SCAN_EHMARKER(); \ if (true) PAL_CPP_TRY { \ - SCAN_EHMARKER_TRY(); \ - DEBUG_ASSURE_NO_RETURN_BEGIN(IUACH); + SCAN_EHMARKER_TRY(); #define UNINSTALL_UNWIND_AND_CONTINUE_HANDLER_EX(nativeRethrow) \ - DEBUG_ASSURE_NO_RETURN_END(IUACH) \ SCAN_EHMARKER_END_TRY(); \ } \ PAL_CPP_CATCH_NON_DERIVED_NOARG (const std::bad_alloc&) \ @@ -500,12 +497,12 @@ void COMPlusCooperativeTransitionHandler(Frame* pFrame); { \ MAKE_CURRENT_THREAD_AVAILABLE(); \ BEGIN_GCX_ASSERT_PREEMP; \ - CoopTransitionHolder __CoopTransition(CURRENT_THREAD); \ - DEBUG_ASSURE_NO_RETURN_BEGIN(COOP_TRANSITION) + { \ + CoopTransitionHolder __CoopTransition(CURRENT_THREAD); #define COOPERATIVE_TRANSITION_END() \ - DEBUG_ASSURE_NO_RETURN_END(COOP_TRANSITION) \ - __CoopTransition.SuppressRelease(); \ + __CoopTransition.SuppressRelease(); \ + } \ END_GCX_ASSERT_PREEMP; \ } diff --git a/src/coreclr/vm/fcall.h b/src/coreclr/vm/fcall.h index 0b362c0cc71696..74082e7ce78cef 100644 --- a/src/coreclr/vm/fcall.h +++ b/src/coreclr/vm/fcall.h @@ -542,13 +542,9 @@ LPVOID __FCThrow(LPVOID me, enum RuntimeExceptionKind reKind, UINT resID, LPCWST #if defined(_PREFAST_) #define FORLAZYMACHSTATE_BEGINLOOP(x) x #define FORLAZYMACHSTATE_ENDLOOP(x) - #define FORLAZYMACHSTATE_DEBUG_OK_TO_RETURN_BEGIN - #define FORLAZYMACHSTATE_DEBUG_OK_TO_RETURN_END #else #define FORLAZYMACHSTATE_BEGINLOOP(x) x do #define FORLAZYMACHSTATE_ENDLOOP(x) while(x) - #define FORLAZYMACHSTATE_DEBUG_OK_TO_RETURN_BEGIN DEBUG_OK_TO_RETURN_BEGIN(LAZYMACHSTATE) - #define FORLAZYMACHSTATE_DEBUG_OK_TO_RETURN_END DEBUG_OK_TO_RETURN_END(LAZYMACHSTATE) #endif // BEGIN: before gcpoll @@ -558,10 +554,6 @@ LPVOID __FCThrow(LPVOID me, enum RuntimeExceptionKind reKind, UINT resID, LPCWST // END: after gcpoll //__fcallGcCanTrigger.Leave(__FUNCTION__, __FILE__, __LINE__); -// We have to put DEBUG_OK_TO_RETURN_BEGIN around the FORLAZYMACHSTATE -// to allow the HELPER_FRAME to be installed inside an SO_INTOLERANT region -// which does not allow a return. The return is used by FORLAZYMACHSTATE -// to capture the state, but is not an actual return, so it is ok. #define HELPER_METHOD_FRAME_BEGIN_EX_BODY(ret, helperFrame, gcpoll, allowGC) \ FORLAZYMACHSTATE_BEGINLOOP(int alwaysZero = 0;) \ { \ @@ -569,9 +561,7 @@ LPVOID __FCThrow(LPVOID me, enum RuntimeExceptionKind reKind, UINT resID, LPCWST PERMIT_HELPER_METHOD_FRAME_BEGIN(); \ CHECK_HELPER_METHOD_FRAME_PERMITTED(); \ helperFrame; \ - FORLAZYMACHSTATE_DEBUG_OK_TO_RETURN_BEGIN; \ FORLAZYMACHSTATE(CAPTURE_STATE(__helperframe.MachineState(), ret);) \ - FORLAZYMACHSTATE_DEBUG_OK_TO_RETURN_END; \ INDEBUG(__helperframe.SetAddrOfHaveCheckedRestoreState(&__haveCheckedRestoreState)); \ DEBUG_ASSURE_NO_RETURN_BEGIN(HELPER_METHOD_FRAME); \ INCONTRACT(FCallGCCanTrigger::Enter()); diff --git a/src/coreclr/vm/frames.h b/src/coreclr/vm/frames.h index be519af3b9b239..ef46f59f2f5ece 100644 --- a/src/coreclr/vm/frames.h +++ b/src/coreclr/vm/frames.h @@ -2819,36 +2819,6 @@ class ExceptionFilterFrame : public Frame #endif }; -#ifdef _DEBUG -// We use IsProtectedByGCFrame to check if some OBJECTREF pointers are protected -// against GC. That function doesn't know if a byref is from managed stack thus -// protected by JIT. AssumeByrefFromJITStackFrame is used to bypass that check if an -// OBJECTRef pointer is passed from managed code to an FCall and it's in stack. - -typedef DPTR(class AssumeByrefFromJITStackFrame) PTR_AssumeByrefFromJITStackFrame; - -class AssumeByrefFromJITStackFrame : public Frame -{ -public: -#ifndef DACCESS_COMPILE - AssumeByrefFromJITStackFrame(OBJECTREF *pObjRef) : Frame(FrameIdentifier::AssumeByrefFromJITStackFrame) - { - m_pObjRef = pObjRef; - } -#endif - - BOOL Protects_Impl(OBJECTREF *ppORef) - { - LIMITED_METHOD_CONTRACT; - return ppORef == m_pObjRef; - } - -private: - OBJECTREF *m_pObjRef; -}; //AssumeByrefFromJITStackFrame - -#endif //_DEBUG - //------------------------------------------------------------------------ // These macros GC-protect OBJECTREF pointers on the EE's behalf. // In between these macros, the GC can move but not discard the protected @@ -2919,8 +2889,7 @@ class AssumeByrefFromJITStackFrame : public Frame (OBJECTREF*)&(ObjRefStruct), \ sizeof(ObjRefStruct)/sizeof(OBJECTREF), \ FALSE); \ - /* work around unreachable code warning */ \ - if (true) { DEBUG_ASSURE_NO_RETURN_BEGIN(GCPROTECT) + { #define GCPROTECT_BEGIN_THREAD(pThread, ObjRefStruct) do { \ GCFrame __gcframe( \ @@ -2928,16 +2897,14 @@ class AssumeByrefFromJITStackFrame : public Frame (OBJECTREF*)&(ObjRefStruct), \ sizeof(ObjRefStruct)/sizeof(OBJECTREF), \ FALSE); \ - /* work around unreachable code warning */ \ - if (true) { DEBUG_ASSURE_NO_RETURN_BEGIN(GCPROTECT) + { #define GCPROTECT_ARRAY_BEGIN(ObjRefArray,cnt) do { \ GCFrame __gcframe( \ (OBJECTREF*)&(ObjRefArray), \ cnt * sizeof(ObjRefArray) / sizeof(OBJECTREF), \ FALSE); \ - /* work around unreachable code warning */ \ - if (true) { DEBUG_ASSURE_NO_RETURN_BEGIN(GCPROTECT) + { #define GCPROTECT_BEGININTERIOR(ObjRefStruct) do { \ /* work around Wsizeof-pointer-div warning as we */ \ @@ -2947,20 +2914,18 @@ class AssumeByrefFromJITStackFrame : public Frame (OBJECTREF*)&(ObjRefStruct), \ subjectSize/sizeof(OBJECTREF), \ TRUE); \ - /* work around unreachable code warning */ \ - if (true) { DEBUG_ASSURE_NO_RETURN_BEGIN(GCPROTECT) + { #define GCPROTECT_BEGININTERIOR_ARRAY(ObjRefArray,cnt) do { \ GCFrame __gcframe( \ (OBJECTREF*)&(ObjRefArray), \ cnt, \ TRUE); \ - /* work around unreachable code warning */ \ - if (true) { DEBUG_ASSURE_NO_RETURN_BEGIN(GCPROTECT) + { #define GCPROTECT_END() \ - DEBUG_ASSURE_NO_RETURN_END(GCPROTECT) } \ + } \ } while(0) @@ -2977,27 +2942,6 @@ class AssumeByrefFromJITStackFrame : public Frame #define ASSERT_ADDRESS_IN_STACK(address) _ASSERTE (Thread::IsAddressInCurrentStack (address)); -#if defined (_DEBUG) && !defined (DACCESS_COMPILE) -#define ASSUME_BYREF_FROM_JIT_STACK_BEGIN(__objRef) \ - /* make sure we are only called inside an FCall */ \ - if (__me == 0) {}; \ - /* make sure the address is in stack. If the address is an interior */ \ - /*pointer points to GC heap, the FCall still needs to protect it explicitly */ \ - ASSERT_ADDRESS_IN_STACK (__objRef); \ - do { \ - AssumeByrefFromJITStackFrame __dummyAssumeByrefFromJITStackFrame ((__objRef)); \ - __dummyAssumeByrefFromJITStackFrame.Push (); \ - /* work around unreachable code warning */ \ - if (true) { DEBUG_ASSURE_NO_RETURN_BEGIN(GC_PROTECT) - -#define ASSUME_BYREF_FROM_JIT_STACK_END() \ - DEBUG_ASSURE_NO_RETURN_END(GC_PROTECT) } \ - __dummyAssumeByrefFromJITStackFrame.Pop(); } while(0) -#else //defined (_DEBUG) && !defined (DACCESS_COMPILE) -#define ASSUME_BYREF_FROM_JIT_STACK_BEGIN(__objRef) -#define ASSUME_BYREF_FROM_JIT_STACK_END() -#endif //defined (_DEBUG) && !defined (DACCESS_COMPILE) - void ComputeCallRefMap(MethodDesc* pMD, GCRefMapBuilder * pBuilder, bool isDispatchCell); diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 28ac102756360d..9bdca9d4d90d8a 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2139,7 +2139,7 @@ extern "C" VOID JIT_PInvokeEndRarePath(); void JIT_PInvokeEndRarePath() { - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; Thread *thread = GetThread(); @@ -2164,8 +2164,6 @@ void JIT_PInvokeEndRarePath() } thread->m_pFrame->Pop(thread); - - END_PRESERVE_LAST_ERROR; } /*************************************************************/ @@ -2203,7 +2201,7 @@ void JIT_RareDisableHelper() // in the first phase. // - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; Thread *thread = GetThread(); // We execute RareDisablePreemptiveGC manually before checking any abort conditions @@ -2225,8 +2223,6 @@ void JIT_RareDisableHelper() END_QCALL; thread->DisablePreemptiveGC(); } - - END_PRESERVE_LAST_ERROR; } FCIMPL0(INT32, JIT_GetCurrentManagedThreadId) @@ -2654,7 +2650,7 @@ static PCODE PatchpointRequiredPolicy(TransitionBlock* pTransitionBlock, int* co extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransitionBlock) { - // BEGIN_PRESERVE_LAST_ERROR; + // Manually preserve the last error as we may not return normally from this method. DWORD dwLastError = ::GetLastError(); // This method may not return normally @@ -2819,7 +2815,6 @@ extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransiti #endif // Restore last error (since call below does not return) - // END_PRESERVE_LAST_ERROR; ::SetLastError(dwLastError); // Transition! @@ -2827,8 +2822,6 @@ extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransiti } DONE: - - // END_PRESERVE_LAST_ERROR; ::SetLastError(dwLastError); } @@ -3384,21 +3377,21 @@ HCIMPL3_RAW(void, JIT_ReversePInvokeEnterTrackTransitions, ReversePInvokeFrame* frame->pMD = pMD; Thread* thread = GetThreadNULLOk(); - + // If a thread instance exists and is in the // correct GC mode attempt a quick transition. if (thread != NULL && !thread->PreemptiveGCDisabled()) { frame->currentThread = thread; - + #ifdef PROFILING_SUPPORTED if (CORProfilerTrackTransitions()) { ProfilerUnmanagedToManagedTransitionMD(frame->pMD, COR_PRF_TRANSITION_CALL); } #endif - + // Manually inline the fast path in Thread::DisablePreemptiveGC(). thread->m_fPreemptiveGCDisabled.StoreWithoutBarrier(1); if (g_TrapReturningThreads != 0) diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 6458e86998ce71..6277c7f26fb305 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -2622,7 +2622,7 @@ extern "C" PCODE STDCALL PreStubWorker(TransitionBlock* pTransitionBlock, Method { PCODE pbRetVal = (PCODE)NULL; - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; STATIC_CONTRACT_THROWS; STATIC_CONTRACT_GC_TRIGGERS; @@ -2735,8 +2735,6 @@ extern "C" PCODE STDCALL PreStubWorker(TransitionBlock* pTransitionBlock, Method POSTCONDITION(pbRetVal != NULL); - END_PRESERVE_LAST_ERROR; - return pbRetVal; } @@ -3163,7 +3161,7 @@ EXTERN_C PCODE STDCALL ExternalMethodFixupWorker(TransitionBlock * pTransitionBl PCODE pCode = (PCODE)NULL; - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; MAKE_CURRENT_THREAD_AVAILABLE(); @@ -3490,8 +3488,6 @@ EXTERN_C PCODE STDCALL ExternalMethodFixupWorker(TransitionBlock * pTransitionBl pEMFrame->Pop(CURRENT_THREAD); // Pop the ExternalMethodFrame from the frame stack - END_PRESERVE_LAST_ERROR; - return pCode; } diff --git a/src/coreclr/vm/stubhelpers.cpp b/src/coreclr/vm/stubhelpers.cpp index c3ec58f129b753..d5121fc7f062df 100644 --- a/src/coreclr/vm/stubhelpers.cpp +++ b/src/coreclr/vm/stubhelpers.cpp @@ -420,7 +420,7 @@ FCIMPL1(void*, StubHelpers::GetDelegateTarget, DelegateObject *pThisUNSAFE) PCODE pEntryPoint = (PCODE)NULL; #ifdef _DEBUG - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; #endif CONTRACTL @@ -442,10 +442,6 @@ FCIMPL1(void*, StubHelpers::GetDelegateTarget, DelegateObject *pThisUNSAFE) pEntryPoint = orefThis->GetMethodPtrAux(); -#ifdef _DEBUG - END_PRESERVE_LAST_ERROR; -#endif - return (PVOID)pEntryPoint; } FCIMPLEND @@ -485,7 +481,7 @@ extern "C" void QCALLTYPE StubHelpers_ThrowInteropParamException(INT resID, INT #ifdef PROFILING_SUPPORTED extern "C" void* QCALLTYPE StubHelpers_ProfilerBeginTransitionCallback(MethodDesc* pTargetMD) { - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; QCALL_CONTRACT; @@ -495,14 +491,12 @@ extern "C" void* QCALLTYPE StubHelpers_ProfilerBeginTransitionCallback(MethodDes END_QCALL; - END_PRESERVE_LAST_ERROR; - return pTargetMD; } extern "C" void QCALLTYPE StubHelpers_ProfilerEndTransitionCallback(MethodDesc* pTargetMD) { - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; QCALL_CONTRACT; @@ -511,8 +505,6 @@ extern "C" void QCALLTYPE StubHelpers_ProfilerEndTransitionCallback(MethodDesc* ProfilerUnmanagedToManagedTransitionMD(pTargetMD, COR_PRF_TRANSITION_RETURN); END_QCALL; - - END_PRESERVE_LAST_ERROR; } #endif // PROFILING_SUPPORTED diff --git a/src/coreclr/vm/threads.h b/src/coreclr/vm/threads.h index 48580a13278432..afbc74f239c7d9 100644 --- a/src/coreclr/vm/threads.h +++ b/src/coreclr/vm/threads.h @@ -111,6 +111,7 @@ #ifndef __threads_h__ #define __threads_h__ +#include #include "vars.hpp" #include "util.hpp" #include "eventstore.hpp" @@ -5239,7 +5240,8 @@ class CoopTransitionHolder ~CoopTransitionHolder() { WRAPPER_NO_CONTRACT; - if (m_pFrame != NULL) + _ASSERTE_MSG(m_pFrame == nullptr || std::uncaught_exception(), "Early return from JIT/EE interface method"); + if (m_pFrame != nullptr) COMPlusCooperativeTransitionHandler(m_pFrame); } @@ -5248,7 +5250,7 @@ class CoopTransitionHolder LIMITED_METHOD_CONTRACT; // FRAME_TOP and NULL must be distinct values. // static_assert_no_msg(FRAME_TOP_VALUE != NULL); - m_pFrame = NULL; + m_pFrame = nullptr; } }; diff --git a/src/coreclr/vm/threadsuspend.cpp b/src/coreclr/vm/threadsuspend.cpp index 1b1ce6536f392c..6a6f3e1ff51564 100644 --- a/src/coreclr/vm/threadsuspend.cpp +++ b/src/coreclr/vm/threadsuspend.cpp @@ -2105,7 +2105,7 @@ extern void WaitForEndOfShutdown(); // why we Enable GC here! void Thread::RareDisablePreemptiveGC() { - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; CONTRACTL { NOTHROW; @@ -2115,7 +2115,7 @@ void Thread::RareDisablePreemptiveGC() if (IsAtProcessExit()) { - goto Exit; + return; } _ASSERTE (m_fPreemptiveGCDisabled); @@ -2127,7 +2127,7 @@ void Thread::RareDisablePreemptiveGC() if (!GCHeapUtilities::IsGCHeapInitialized()) { - goto Exit; + return; } if (ThreadStore::HoldingThreadStore(this)) @@ -2138,7 +2138,7 @@ void Thread::RareDisablePreemptiveGC() // may want to enter coop mode, but we have a number of other scenarios where TSL is acquired // in coop mode or when TSL-owning thread tries to swtch to coop. // We will handle all cases in the same way - by allowing the thread into coop mode without a wait. - goto Exit; + return; } STRESS_LOG1(LF_SYNC, LL_INFO1000, "RareDisablePreemptiveGC: entering. Thread state = %x\n", m_State.Load()); @@ -2257,14 +2257,12 @@ void Thread::RareDisablePreemptiveGC() } STRESS_LOG0(LF_SYNC, LL_INFO1000, "RareDisablePreemptiveGC: leaving\n"); - -Exit: ; - END_PRESERVE_LAST_ERROR; } void Thread::HandleThreadAbort () { - BEGIN_PRESERVE_LAST_ERROR; + // Only preserve the last error when we aren't throwing an exception for ThreadAbort. + DWORD lastError = GetLastError(); STATIC_CONTRACT_THROWS; STATIC_CONTRACT_GC_TRIGGERS; @@ -2322,7 +2320,7 @@ void Thread::HandleThreadAbort () } } - END_PRESERVE_LAST_ERROR; + ::SetLastError(lastError); } void Thread::PreWorkForThreadAbort() @@ -2641,9 +2639,8 @@ extern "C" PCONTEXT __stdcall GetCurrentSavedRedirectContext() PCONTEXT pContext; - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; pContext = GetThread()->GetSavedRedirectContext(); - END_PRESERVE_LAST_ERROR; return pContext; } @@ -2688,7 +2685,8 @@ void __stdcall Thread::RedirectedHandledJITCase(RedirectReason reason) // We must preserve this in case we've interrupted an IL pinvoke stub before it // was able to save the error. - DWORD dwLastError = GetLastError(); // BEGIN_PRESERVE_LAST_ERROR + // Manually preserve it as we don't return from this function normally. + DWORD dwLastError = GetLastError(); Thread *pThread = GetThread(); @@ -2774,7 +2772,7 @@ void __stdcall Thread::RedirectedHandledJITCase(RedirectReason reason) pThread->UnmarkRedirectContextInUse(pCtx); LOG((LF_SYNC, LL_INFO1000, "Resuming execution with RtlRestoreContext\n")); - SetLastError(dwLastError); // END_PRESERVE_LAST_ERROR + SetLastError(dwLastError); __asan_handle_no_return(); #ifdef TARGET_X86 @@ -4868,7 +4866,7 @@ void STDCALL OnHijackWorker(HijackArgs * pArgs) CONTRACTL_END; #ifdef HIJACK_NONINTERRUPTIBLE_THREADS - BEGIN_PRESERVE_LAST_ERROR; + PreserveLastErrorHolder preserveLastError; Thread *thread = GetThread(); @@ -4901,8 +4899,6 @@ void STDCALL OnHijackWorker(HijackArgs * pArgs) #endif // _DEBUG frame.Pop(); - - END_PRESERVE_LAST_ERROR; #else PORTABILITY_ASSERT("OnHijackWorker not implemented on this platform."); #endif // HIJACK_NONINTERRUPTIBLE_THREADS @@ -5731,7 +5727,7 @@ BOOL CheckActivationSafePoint(SIZE_T ip) // The criteria for safe activation is to be running managed code. // Also we are not interested in handling interruption if we are already in preemptive mode nor if we are single stepping - BOOL isActivationSafePoint = pThread != NULL && + BOOL isActivationSafePoint = pThread != NULL && (pThread->m_StateNC & Thread::TSNC_DebuggerIsStepping) == 0 && pThread->PreemptiveGCDisabled() && ExecutionManager::IsManagedCode(ip);