diff options
author | Vladimir Sadov <vsadov@microsoft.com> | 2022-03-09 01:04:09 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-03-09 01:04:09 +0300 |
commit | 1ec6f8e134bdd27db214e3779cd0b7d57680832c (patch) | |
tree | 08e8381c25d4bb237251231308454d11f98f7150 /src/coreclr | |
parent | 154cbb0788f5393bb262060403aef606acc28783 (diff) |
[Release/6.0] Backporting recent fixes related to AVX and suspension. (#66120)
* Use CopyContext to restore saved context on X86 (#65490)
* Use CopyContext to restore saved context on X86
* PR feedback
* more PR feedback
* Do not copy XState other than AVX when redirecting for GC stress (#65825)
* Do not copy XState other than AVX
* #if defined(TARGET_X86) || defined(TARGET_AMD64)
* mask XState unconditionally
* Ensure XSTATE_MASK_AVX is set before calling EEGetThreadContext
* redundant supportsAVX, more clear comment
* PR feedback
* null-check the redirect context before using. (#65910)
* null-check the redirect context before using.
* tweak the comment
* do not allocate context if InitializeContext has unexpected results.
* EE Suspension on x86 should use RtlRestoreContext when available (#65878)
* RestoreContextSimulated
* probe for RtlRestoreContext
* ntdll.dll
* restore self-trap sequence
* PR feedback
* Clarify CopyContext in RedirectedHandledJITCaseExceptionFilter
* simpler indentation.
* restore last error on the legacy path.
* Update src/coreclr/vm/threads.h
Co-authored-by: Dan Moseley <danmose@microsoft.com>
Co-authored-by: Dan Moseley <danmose@microsoft.com>
Diffstat (limited to 'src/coreclr')
-rw-r--r-- | src/coreclr/vm/threads.h | 8 | ||||
-rw-r--r-- | src/coreclr/vm/threadsuspend.cpp | 350 |
2 files changed, 199 insertions, 159 deletions
diff --git a/src/coreclr/vm/threads.h b/src/coreclr/vm/threads.h index af2a7b32d31..acc23de0cbd 100644 --- a/src/coreclr/vm/threads.h +++ b/src/coreclr/vm/threads.h @@ -3292,6 +3292,14 @@ private: static void __stdcall RedirectedHandledJITCaseForGCStress(); #endif // defined(HAVE_GCCOVER) && USE_REDIRECT_FOR_GCSTRESS +#ifdef TARGET_X86 + // RtlRestoreContext is available on x86, but relatively recently. + // RestoreContextSimulated uses SEH machinery for a similar result on legacy OS-es. + // This function should not be used on new OS-es as the pattern is not + // guaranteed to continue working in the future. + static void RestoreContextSimulated(Thread* pThread, CONTEXT* pCtx, void* pFrame, DWORD dwLastError); +#endif + friend void CPFH_AdjustContextForThreadSuspensionRace(T_CONTEXT *pContext, Thread *pThread); #endif // FEATURE_HIJACK && !TARGET_UNIX diff --git a/src/coreclr/vm/threadsuspend.cpp b/src/coreclr/vm/threadsuspend.cpp index 57318e29e89..b92af380085 100644 --- a/src/coreclr/vm/threadsuspend.cpp +++ b/src/coreclr/vm/threadsuspend.cpp @@ -1948,6 +1948,9 @@ typedef BOOL(WINAPI* PINITIALIZECONTEXT2)(PVOID Buffer, DWORD ContextFlags, PCON PINITIALIZECONTEXT2 pfnInitializeContext2 = NULL; #ifdef TARGET_X86 +typedef VOID(__cdecl* PRTLRESTORECONTEXT)(PCONTEXT ContextRecord, struct _EXCEPTION_RECORD* ExceptionRecord); +PRTLRESTORECONTEXT pfnRtlRestoreContext = NULL; + #define CONTEXT_COMPLETE (CONTEXT_FULL | CONTEXT_FLOATING_POINT | \ CONTEXT_DEBUG_REGISTERS | CONTEXT_EXTENDED_REGISTERS | CONTEXT_EXCEPTION_REQUEST) #else @@ -1960,7 +1963,6 @@ CONTEXT* AllocateOSContextHelper(BYTE** contextBuffer) #if !defined(TARGET_UNIX) && (defined(TARGET_X86) || defined(TARGET_AMD64)) DWORD context = CONTEXT_COMPLETE; - BOOL supportsAVX = FALSE; if (pfnInitializeContext2 == NULL) { @@ -1968,13 +1970,20 @@ CONTEXT* AllocateOSContextHelper(BYTE** contextBuffer) pfnInitializeContext2 = (PINITIALIZECONTEXT2)GetProcAddress(hm, "InitializeContext2"); } - // Determine if the processor supports AVX so we could +#ifdef TARGET_X86 + if (pfnRtlRestoreContext == NULL) + { + HMODULE hm = GetModuleHandleW(_T("ntdll.dll")); + pfnRtlRestoreContext = (PRTLRESTORECONTEXT)GetProcAddress(hm, "RtlRestoreContext"); + } +#endif //TARGET_X86 + + // Determine if the processor supports AVX so we could // retrieve extended registers DWORD64 FeatureMask = GetEnabledXStateFeatures(); if ((FeatureMask & XSTATE_MASK_AVX) != 0) { context = context | CONTEXT_XSTATE; - supportsAVX = TRUE; } // Retrieve contextSize by passing NULL for Buffer @@ -1985,9 +1994,14 @@ CONTEXT* AllocateOSContextHelper(BYTE** contextBuffer) pfnInitializeContext2(NULL, context, NULL, &contextSize, xStateCompactionMask) : InitializeContext(NULL, context, NULL, &contextSize); - // The following assert is valid, but gets triggered in some Win7 runs with no impact on functionality. - // commenting this out to reduce noise, as long as Win7 is supported. - // _ASSERTE(!success && GetLastError() == ERROR_INSUFFICIENT_BUFFER); + // Spec mentions that we may get a different error (it was observed on Windows7). + // In such case the contextSize is undefined. + if (success || GetLastError() != ERROR_INSUFFICIENT_BUFFER) + { + STRESS_LOG2(LF_SYNC, LL_INFO1000, "AllocateOSContextHelper: Unexpected result from InitializeContext (success: %d, error: %d).\n", + success, GetLastError()); + return NULL; + } // So now allocate a buffer of that size and call InitializeContext again BYTE* buffer = new (nothrow)BYTE[contextSize]; @@ -1997,15 +2011,6 @@ CONTEXT* AllocateOSContextHelper(BYTE** contextBuffer) pfnInitializeContext2(buffer, context, &pOSContext, &contextSize, xStateCompactionMask): InitializeContext(buffer, context, &pOSContext, &contextSize); - // if AVX is supported set the appropriate features mask in the context - if (success && supportsAVX) - { - // This should not normally fail. - // The system silently ignores any feature specified in the FeatureMask - // which is not enabled on the processor. - success = SetXStateFeaturesMask(pOSContext, XSTATE_MASK_AVX); - } - if (!success) { delete[] buffer; @@ -2509,6 +2514,7 @@ void RedirectedThreadFrame::ExceptionUnwind() #ifndef TARGET_UNIX #ifdef TARGET_X86 + //**************************************************************************************** // This will check who caused the exception. If it was caused by the the redirect function, // the reason is to resume the thread back at the point it was redirected in the first @@ -2520,18 +2526,18 @@ void RedirectedThreadFrame::ExceptionUnwind() int RedirectedHandledJITCaseExceptionFilter( PEXCEPTION_POINTERS pExcepPtrs, // Exception data RedirectedThreadFrame *pFrame, // Frame on stack - BOOL fDone, // Whether redirect completed without exception - CONTEXT *pCtx) // Saved context + CONTEXT *pCtx, // Saved context + DWORD dwLastError) // saved last error { // !!! Do not use a non-static contract here. // !!! Contract may insert an exception handling record. // !!! This function assumes that GetCurrentSEHRecord() returns the exception record set up in - // !!! Thread::RedirectedHandledJITCase + // !!! Thread::RestoreContextSimulated // // !!! Do not use an object with dtor, since it injects a fs:0 entry. STATIC_CONTRACT_NOTHROW; STATIC_CONTRACT_GC_TRIGGERS; - STATIC_CONTRACT_MODE_ANY; + STATIC_CONTRACT_MODE_COOPERATIVE; if (pExcepPtrs->ExceptionRecord->ExceptionCode == STATUS_STACK_OVERFLOW) { @@ -2540,44 +2546,22 @@ int RedirectedHandledJITCaseExceptionFilter( // Get the thread handle Thread *pThread = GetThread(); - - STRESS_LOG2(LF_SYNC, LL_INFO100, "In RedirectedHandledJITCaseExceptionFilter fDone = %d pFrame = %p\n", fDone, pFrame); - - // If we get here via COM+ exception, gc-mode is unknown. We need it to - // be cooperative for this function. - GCX_COOP_NO_DTOR(); - - // If the exception was due to the called client, then we need to figure out if it - // is an exception that can be eaten or if it needs to be handled elsewhere. - if (!fDone) - { - if (pExcepPtrs->ExceptionRecord->ExceptionFlags & EXCEPTION_NONCONTINUABLE) - { - return (EXCEPTION_CONTINUE_SEARCH); - } - - // Get the latest thrown object - OBJECTREF throwable = CLRException::GetThrowableFromExceptionRecord(pExcepPtrs->ExceptionRecord); - - // If this is an uncatchable exception, then let the exception be handled elsewhere - if (IsUncatchable(&throwable)) - { - pThread->EnablePreemptiveGC(); - return (EXCEPTION_CONTINUE_SEARCH); - } - } -#ifdef _DEBUG - else - { - _ASSERTE(pExcepPtrs->ExceptionRecord->ExceptionCode == EXCEPTION_HIJACK); - } -#endif + STRESS_LOG1(LF_SYNC, LL_INFO100, "In RedirectedHandledJITCaseExceptionFilter pFrame = %p\n", pFrame); + _ASSERTE(pExcepPtrs->ExceptionRecord->ExceptionCode == EXCEPTION_HIJACK); // Unlink the frame in preparation for resuming in managed code pFrame->Pop(); - // Copy the saved context record into the EH context; - ReplaceExceptionContextRecord(pExcepPtrs->ContextRecord, pCtx); + // Copy everything in the saved context record into the EH context. + // Historically the EH context has enough space for every enabled context feature. + // That may not hold for the future features beyond AVX, but this codepath is + // supposed to be used only on OSes that do not have RtlRestoreContext. + CONTEXT* pTarget = pExcepPtrs->ContextRecord; + if (!CopyContext(pTarget, pCtx->ContextFlags, pCtx)) + { + STRESS_LOG1(LF_SYNC, LL_ERROR, "ERROR: Could not set context record, lastError = 0x%x\n", GetLastError()); + EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); + } DWORD espValue = pCtx->Esp; @@ -2607,6 +2591,9 @@ int RedirectedHandledJITCaseExceptionFilter( // Register the special OS handler as the top handler with the OS SetCurrentSEHRecord(pCurSEH); + // restore last error + SetLastError(dwLastError); + // Resume execution at point where thread was originally redirected return (EXCEPTION_CONTINUE_EXECUTION); } @@ -2639,6 +2626,38 @@ extern "C" PCONTEXT __stdcall GetCurrentSavedRedirectContext() return pContext; } +#ifdef TARGET_X86 + +void Thread::RestoreContextSimulated(Thread* pThread, CONTEXT* pCtx, void* pFrame, DWORD dwLastError) +{ + pThread->HandleThreadAbort(); // Might throw an exception. + + // A counter to avoid a nasty case where an + // up-stack filter throws another exception + // causing our filter to be run again for + // some unrelated exception. + int filter_count = 0; + + __try + { + // Save the instruction pointer where we redirected last. This does not race with the check + // against this variable in HandledJitCase because the GC will not attempt to redirect the + // thread until the instruction pointer of this thread is back in managed code. + pThread->m_LastRedirectIP = GetIP(pCtx); + pThread->m_SpinCount = 0; + + RaiseException(EXCEPTION_HIJACK, 0, 0, NULL); + } + __except (++filter_count == 1 + ? RedirectedHandledJITCaseExceptionFilter(GetExceptionInformation(), (RedirectedThreadFrame*)pFrame, pCtx, dwLastError) + : EXCEPTION_CONTINUE_SEARCH) + { + _ASSERTE(!"Reached body of __except in Thread::RedirectedHandledJITCase"); + } +} + +#endif // TARGET_X86 + void __stdcall Thread::RedirectedHandledJITCase(RedirectReason reason) { STATIC_CONTRACT_THROWS; @@ -2662,140 +2681,106 @@ void __stdcall Thread::RedirectedHandledJITCase(RedirectReason reason) STRESS_LOG5(LF_SYNC, LL_INFO1000, "In RedirectedHandledJITcase reason 0x%x pFrame = %p pc = %p sp = %p fp = %p", reason, &frame, GetIP(pCtx), GetSP(pCtx), GetFP(pCtx)); -#ifdef TARGET_X86 - // This will indicate to the exception filter whether or not the exception is caused - // by us or the client. - BOOL fDone = FALSE; - int filter_count = 0; // A counter to avoid a nasty case where an - // up-stack filter throws another exception - // causing our filter to be run again for - // some unrelated exception. - - __try -#endif // TARGET_X86 - { - // Make sure this thread doesn't reuse the context memory. - pThread->MarkRedirectContextInUse(pCtx); + // Make sure this thread doesn't reuse the context memory. + pThread->MarkRedirectContextInUse(pCtx); - // Link in the frame - frame.Push(); + // Link in the frame + frame.Push(); #if defined(HAVE_GCCOVER) && defined(USE_REDIRECT_FOR_GCSTRESS) // GCCOVER - if (reason == RedirectReason_GCStress) - { - _ASSERTE(pThread->PreemptiveGCDisabledOther()); - DoGcStress(frame.GetContext(), NULL); - } - else + if (reason == RedirectReason_GCStress) + { + _ASSERTE(pThread->PreemptiveGCDisabledOther()); + DoGcStress(frame.GetContext(), NULL); + } + else #endif // HAVE_GCCOVER && USE_REDIRECT_FOR_GCSTRESS - { - // Enable PGC before calling out to the client to allow runtime suspend to finish - GCX_PREEMP_NO_DTOR(); + { + _ASSERTE(reason == RedirectReason_GCSuspension || + reason == RedirectReason_DebugSuspension || + reason == RedirectReason_UserSuspension); - // Notify the interface of the pending suspension - switch (reason) { - case RedirectReason_GCSuspension: - break; - case RedirectReason_DebugSuspension: - break; - case RedirectReason_UserSuspension: - // Do nothing; - break; - default: - _ASSERTE(!"Invalid redirect reason"); - break; - } + // Actual self-suspension. + // Leave and reenter COOP mode to be trapped on the way back. + GCX_PREEMP_NO_DTOR(); + GCX_PREEMP_NO_DTOR_END(); + } - // Disable preemptive GC so we can unlink the frame - GCX_PREEMP_NO_DTOR_END(); - } + // Once we get here the suspension is over! + // We will restore the state as it was at the point of redirection + // and continue normal execution. #ifdef TARGET_X86 - pThread->HandleThreadAbort(); // Might throw an exception. - - // Indicate that the call to the service went without an exception, and that - // we're raising our own exception to resume the thread to where it was - // redirected from - fDone = TRUE; - - // Save the instruction pointer where we redirected last. This does not race with the check - // against this variable in HandledJitCase because the GC will not attempt to redirect the - // thread until the instruction pointer of this thread is back in managed code. - pThread->m_LastRedirectIP = GetIP(pCtx); - pThread->m_SpinCount = 0; - - RaiseException(EXCEPTION_HIJACK, 0, 0, NULL); + if (!pfnRtlRestoreContext) + { + RestoreContextSimulated(pThread, pCtx, &frame, dwLastError); -#else // TARGET_X86 + // we never return to the caller. + __UNREACHABLE(); + } +#endif // TARGET_X86 #if defined(HAVE_GCCOVER) && defined(USE_REDIRECT_FOR_GCSTRESS) // GCCOVER - // - // If GCStress interrupts an IL stub or inlined p/invoke while it's running in preemptive mode, it switches the mode to - // cooperative - but we will resume to preemptive below. We should not trigger an abort in that case, as it will fail - // due to the GC mode. - // - if (!pThread->m_fPreemptiveGCDisabledForGCStress) + // + // If GCStress interrupts an IL stub or inlined p/invoke while it's running in preemptive mode, it switches the mode to + // cooperative - but we will resume to preemptive below. We should not trigger an abort in that case, as it will fail + // due to the GC mode. + // + if (!pThread->m_fPreemptiveGCDisabledForGCStress) #endif - { + { - UINT_PTR uAbortAddr; - UINT_PTR uResumePC = (UINT_PTR)GetIP(pCtx); - CopyOSContext(pThread->m_OSContext, pCtx); - uAbortAddr = (UINT_PTR)COMPlusCheckForAbort(); - if (uAbortAddr) - { - LOG((LF_EH, LL_INFO100, "thread abort in progress, resuming thread under control... (handled jit case)\n")); + UINT_PTR uAbortAddr; + UINT_PTR uResumePC = (UINT_PTR)GetIP(pCtx); + CopyOSContext(pThread->m_OSContext, pCtx); + uAbortAddr = (UINT_PTR)COMPlusCheckForAbort(); + if (uAbortAddr) + { + LOG((LF_EH, LL_INFO100, "thread abort in progress, resuming thread under control... (handled jit case)\n")); - CONSISTENCY_CHECK(CheckPointer(pCtx)); + CONSISTENCY_CHECK(CheckPointer(pCtx)); - STRESS_LOG1(LF_EH, LL_INFO10, "resume under control: ip: %p (handled jit case)\n", uResumePC); + STRESS_LOG1(LF_EH, LL_INFO10, "resume under control: ip: %p (handled jit case)\n", uResumePC); - SetIP(pThread->m_OSContext, uResumePC); + SetIP(pThread->m_OSContext, uResumePC); #if defined(TARGET_ARM) - // Save the original resume PC in Lr - pCtx->Lr = uResumePC; + // Save the original resume PC in Lr + pCtx->Lr = uResumePC; - // Since we have set a new IP, we have to clear conditional execution flags too. - ClearITState(pThread->m_OSContext); + // Since we have set a new IP, we have to clear conditional execution flags too. + ClearITState(pThread->m_OSContext); #endif // TARGET_ARM - SetIP(pCtx, uAbortAddr); - } + SetIP(pCtx, uAbortAddr); } + } - // Unlink the frame in preparation for resuming in managed code - frame.Pop(); + // Unlink the frame in preparation for resuming in managed code + frame.Pop(); - { - // Allow future use of the context - pThread->UnmarkRedirectContextInUse(pCtx); + // Allow future use of the context + pThread->UnmarkRedirectContextInUse(pCtx); #if defined(HAVE_GCCOVER) && defined(USE_REDIRECT_FOR_GCSTRESS) // GCCOVER - if (pThread->m_fPreemptiveGCDisabledForGCStress) - { - pThread->EnablePreemptiveGC(); - pThread->m_fPreemptiveGCDisabledForGCStress = false; - } + if (pThread->m_fPreemptiveGCDisabledForGCStress) + { + pThread->EnablePreemptiveGC(); + pThread->m_fPreemptiveGCDisabledForGCStress = false; + } #endif - LOG((LF_SYNC, LL_INFO1000, "Resuming execution with RtlRestoreContext\n")); - - SetLastError(dwLastError); // END_PRESERVE_LAST_ERROR + LOG((LF_SYNC, LL_INFO1000, "Resuming execution with RtlRestoreContext\n")); + SetLastError(dwLastError); // END_PRESERVE_LAST_ERROR - RtlRestoreContext(pCtx, NULL); - } -#endif // TARGET_X86 - } #ifdef TARGET_X86 - __except (++filter_count == 1 - ? RedirectedHandledJITCaseExceptionFilter(GetExceptionInformation(), &frame, fDone, pCtx) - : EXCEPTION_CONTINUE_SEARCH) - { - _ASSERTE(!"Reached body of __except in Thread::RedirectedHandledJITCase"); - } + pfnRtlRestoreContext(pCtx, NULL); +#else + RtlRestoreContext(pCtx, NULL); +#endif -#endif // TARGET_X86 + // we never return to the caller. + __UNREACHABLE(); } //**************************************************************************************** @@ -2898,14 +2883,34 @@ BOOL Thread::RedirectThreadAtHandledJITCase(PFN_REDIRECTTARGET pTgt) if (!pCtx) { pCtx = m_pSavedRedirectContext = ThreadStore::GrabOSContext(&m_pOSContextBuffer); - _ASSERTE(GetSavedRedirectContext() != NULL); } + // We may not have a preallocated context. Could be short on memory when we tried to preallocate. + // We cannot allocate here since we have a thread stopped in a random place, possibly holding locks + // that we would need while allocating. + // Other ways and attempts at suspending may yet succeed, but this redirection cannot continue. + if (!pCtx) + return (FALSE); + ////////////////////////////////////// // Get and save the thread's context + BOOL bRes = true; // Always get complete context, pCtx->ContextFlags are set during Initialization - BOOL bRes = EEGetThreadContext(this, pCtx); + +#if defined(TARGET_X86) || defined(TARGET_AMD64) + // Scenarios like GC stress may indirectly disable XState features in the pCtx + // depending on the state at the time of GC stress interrupt. + // + // Make sure that AVX feature mask is set, if supported. + // + // This should not normally fail. + // The system silently ignores any feature specified in the FeatureMask + // which is not enabled on the processor. + bRes &= SetXStateFeaturesMask(pCtx, XSTATE_MASK_AVX); +#endif //defined(TARGET_X86) || defined(TARGET_AMD64) + + bRes &= EEGetThreadContext(this, pCtx); _ASSERTE(bRes && "Failed to GetThreadContext in RedirectThreadAtHandledJITCase - aborting redirect."); if (!bRes) @@ -3020,8 +3025,35 @@ BOOL Thread::RedirectCurrentThreadAtHandledJITCase(PFN_REDIRECTTARGET pTgt, CONT ////////////////////////////////////// // Get and save the thread's context - BOOL success = CopyContext(pCtx, pCtx->ContextFlags, pCurrentThreadCtx); + BOOL success = TRUE; + +#if defined(TARGET_X86) || defined(TARGET_AMD64) + // This method is called for GC stress interrupts in managed code. + // The current context may have various XState features, depending on what is used/dirty, + // but only AVX feature may contain live data. (that could change with new features in JIT) + // Besides pCtx may not have space to store other features. + // So we will mask out everything but AVX. + DWORD64 srcFeatures = 0; + success = GetXStateFeaturesMask(pCurrentThreadCtx, &srcFeatures); _ASSERTE(success); + if (!success) + return FALSE; + + // Get may return 0 if no XState is set, which Set would not accept. + if (srcFeatures != 0) + { + success = SetXStateFeaturesMask(pCurrentThreadCtx, srcFeatures & XSTATE_MASK_AVX); + _ASSERTE(success); + if (!success) + return FALSE; + } + +#endif //defined(TARGET_X86) || defined(TARGET_AMD64) + + success = CopyContext(pCtx, pCtx->ContextFlags, pCurrentThreadCtx); + _ASSERTE(success); + if (!success) + return FALSE; // Ensure that this flag is set for the next time through the normal path, // RedirectThreadAtHandledJITCase. |