From 86412566424353b2d9b32d629fe7efb66346ac38 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Wed, 24 Aug 2022 16:12:57 +0200 Subject: [PATCH] Improve windows hardware exception handling performance (#74426) * Improve windows hardware exception handling performance With my recent change that modified hardware exception handling so that the related managed exception is thrown directly from the vectored exception handler, the performance of handling such exceptions have regressed. Several exception handling dotnet/performance microbenchmarks have regressed upto 15%. The reason for the regression was the larger number of stack frames between the exception raising and the actual handler frame. With a recent change that @AntonLapounov has made to fix process corrupting exceptions handling, the regression went down to 8%. This change moves the location where we raise the exception down to the ClrVectoredExceptionHandlerShim, which means to the closest possible frame to the managed code. This gets rid of the regression completely. --- src/coreclr/vm/excep.cpp | 96 ++++++++++++++------------------ src/coreclr/vm/exceptmacros.h | 11 +++- src/coreclr/vm/i386/excepx86.cpp | 6 +- 3 files changed, 55 insertions(+), 58 deletions(-) diff --git a/src/coreclr/vm/excep.cpp b/src/coreclr/vm/excep.cpp index d0955cdfb0b..183bee15b52 100644 --- a/src/coreclr/vm/excep.cpp +++ b/src/coreclr/vm/excep.cpp @@ -6723,27 +6723,16 @@ bool ShouldHandleManagedFault( #ifndef TARGET_UNIX -LONG WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo); - -enum VEH_ACTION -{ - VEH_NO_ACTION = 0, - VEH_EXECUTE_HANDLE_MANAGED_EXCEPTION, - VEH_CONTINUE_EXECUTION, - VEH_CONTINUE_SEARCH, - VEH_EXECUTE_HANDLER -}; - - +VEH_ACTION WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo); VEH_ACTION WINAPI CLRVectoredExceptionHandlerPhase3(PEXCEPTION_POINTERS pExceptionInfo); -LONG WINAPI CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) +VEH_ACTION WINAPI CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { // It is not safe to execute code inside VM after we shutdown EE. One example is DisablePreemptiveGC // will block forever. if (g_fForbidEnterEE) { - return EXCEPTION_CONTINUE_SEARCH; + return VEH_CONTINUE_SEARCH; } @@ -6833,7 +6822,7 @@ LONG WINAPI CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) pExceptionInfo->ContextRecord->Rip = hijackArgs.ReturnAddress; } - return EXCEPTION_CONTINUE_EXECUTION; + return VEH_CONTINUE_EXECUTION; } #endif @@ -6857,11 +6846,9 @@ LONG WINAPI CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) // // Not an Out-of-memory situation, so no need for a forbid fault region here // - return EXCEPTION_CONTINUE_SEARCH; + return VEH_CONTINUE_SEARCH; } - LONG retVal = 0; - // We can't probe here, because we won't return from the CLRVectoredExceptionHandlerPhase2 // on WIN64 // @@ -6872,15 +6859,10 @@ LONG WINAPI CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) CantAllocHolder caHolder; } - retVal = CLRVectoredExceptionHandlerPhase2(pExceptionInfo); - - // - //END_ENTRYPOINT_VOIDRET; - // - return retVal; + return CLRVectoredExceptionHandlerPhase2(pExceptionInfo); } -LONG WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo) +VEH_ACTION WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo) { // // DO NOT USE CONTRACTS HERE AS THIS ROUTINE MAY NEVER RETURN. You can use @@ -6914,27 +6896,16 @@ LONG WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo action = CLRVectoredExceptionHandlerPhase3(pExceptionInfo); } - if (action == VEH_CONTINUE_EXECUTION) + if ((action == VEH_CONTINUE_EXECUTION) || (action == VEH_CONTINUE_SEARCH) || (action == VEH_EXECUTE_HANDLER)) { - return EXCEPTION_CONTINUE_EXECUTION; - } - - if (action == VEH_CONTINUE_SEARCH) - { - return EXCEPTION_CONTINUE_SEARCH; - } - - if (action == VEH_EXECUTE_HANDLER) - { - return EXCEPTION_EXECUTE_HANDLER; + return action; } #if defined(FEATURE_EH_FUNCLETS) if (action == VEH_EXECUTE_HANDLE_MANAGED_EXCEPTION) { - HandleManagedFault(pExceptionInfo->ExceptionRecord, pExceptionInfo->ContextRecord); - return EXCEPTION_CONTINUE_EXECUTION; + return action; } #endif // defined(FEATURE_EH_FUNCLETS) @@ -6954,7 +6925,7 @@ LONG WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo // the choice to break the no-trigger region after taking all necessary precautions. if (IsDebuggerFault(pExceptionRecord, pExceptionInfo->ContextRecord, pExceptionRecord->ExceptionCode, GetThreadNULLOk())) { - return EXCEPTION_CONTINUE_EXECUTION; + return VEH_CONTINUE_EXECUTION; } } @@ -6986,11 +6957,11 @@ LONG WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo { // The breakpoint was not ours. Someone else can handle it. (Or if not, we'll get it again as // an unhandled exception.) - return EXCEPTION_CONTINUE_SEARCH; + return VEH_CONTINUE_SEARCH; } // The breakpoint was from managed or the runtime. Handle it. - return UserBreakpointFilter(pExceptionInfo); + return (VEH_ACTION)UserBreakpointFilter(pExceptionInfo); } #if defined(FEATURE_EH_FUNCLETS) @@ -7008,15 +6979,11 @@ LONG WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo if (fShouldHandleManagedFault) { - // - // HandleManagedFault may never return, so we cannot use a forbid fault region around it. - // - HandleManagedFault(pExceptionInfo->ExceptionRecord, pExceptionInfo->ContextRecord); - return EXCEPTION_CONTINUE_EXECUTION; -} + return VEH_EXECUTE_HANDLE_MANAGED_EXCEPTION; + } #endif // defined(FEATURE_EH_FUNCLETS) - return EXCEPTION_EXECUTE_HANDLER; + return VEH_EXECUTE_HANDLER; } /* @@ -7583,13 +7550,34 @@ LONG WINAPI CLRVectoredExceptionHandlerShim(PEXCEPTION_POINTERS pExceptionInfo) if (pThread || fExceptionInEE) { if (!bIsGCMarker) - result = CLRVectoredExceptionHandler(pExceptionInfo); - else - result = EXCEPTION_CONTINUE_EXECUTION; + { + VEH_ACTION action = CLRVectoredExceptionHandler(pExceptionInfo); + +#ifdef FEATURE_EH_FUNCLETS + if (VEH_EXECUTE_HANDLE_MANAGED_EXCEPTION == action) + { + // + // HandleManagedFault may never return, so we cannot use a forbid fault region around it. + // + HandleManagedFault(pExceptionInfo->ExceptionRecord, pExceptionInfo->ContextRecord); + return EXCEPTION_CONTINUE_EXECUTION; + } +#endif // FEATURE_EH_FUNCLETS - if (EXCEPTION_EXECUTE_HANDLER == result) + if (VEH_EXECUTE_HANDLER == action) + { + result = EXCEPTION_CONTINUE_SEARCH; + } + else + { + _ASSERTE((action == VEH_CONTINUE_EXECUTION) || (action == VEH_CONTINUE_SEARCH)); + result = (LONG)action; + } + + } + else { - result = EXCEPTION_CONTINUE_SEARCH; + result = EXCEPTION_CONTINUE_EXECUTION; } #ifdef _DEBUG diff --git a/src/coreclr/vm/exceptmacros.h b/src/coreclr/vm/exceptmacros.h index 3b33292044d..1627e3d3d65 100644 --- a/src/coreclr/vm/exceptmacros.h +++ b/src/coreclr/vm/exceptmacros.h @@ -240,7 +240,16 @@ VOID DECLSPEC_NORETURN RealCOMPlusThrowOM(); #endif // !defined(FEATURE_EH_FUNCLETS) -LONG WINAPI CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo); +enum VEH_ACTION +{ + VEH_NO_ACTION = -3, + VEH_EXECUTE_HANDLE_MANAGED_EXCEPTION = -2, + VEH_CONTINUE_EXECUTION = EXCEPTION_CONTINUE_EXECUTION, + VEH_CONTINUE_SEARCH = EXCEPTION_CONTINUE_SEARCH, + VEH_EXECUTE_HANDLER = EXCEPTION_EXECUTE_HANDLER +}; + +VEH_ACTION CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo); // Actual UEF worker prototype for use by GCUnhandledExceptionFilter. extern LONG InternalUnhandledExceptionFilter_Worker(PEXCEPTION_POINTERS pExceptionInfo); diff --git a/src/coreclr/vm/i386/excepx86.cpp b/src/coreclr/vm/i386/excepx86.cpp index d4abd2171ee..43a6e8c5183 100644 --- a/src/coreclr/vm/i386/excepx86.cpp +++ b/src/coreclr/vm/i386/excepx86.cpp @@ -1255,13 +1255,13 @@ CPFH_FirstPassHandler(EXCEPTION_RECORD *pExceptionRecord, // Call to the vectored handler to give other parts of the Runtime a chance to jump in and take over an // exception before we do too much with it. The most important point in the vectored handler is not to toggle // the GC mode. - DWORD filter = CLRVectoredExceptionHandler(&ptrs); + VEH_ACTION filter = CLRVectoredExceptionHandler(&ptrs); - if (filter == (DWORD) EXCEPTION_CONTINUE_EXECUTION) + if (filter == VEH_CONTINUE_EXECUTION) { return ExceptionContinueExecution; } - else if (filter == EXCEPTION_CONTINUE_SEARCH) + else if (filter == VEH_CONTINUE_SEARCH) { return ExceptionContinueSearch; } -- GitLab