From c88662081ef0f892c9a71b2fdd967c51e92beee3 Mon Sep 17 00:00:00 2001 From: Elinor Fung <47805090+elinor-fung@users.noreply.github.com> Date: Sat, 16 Mar 2019 15:02:52 -0700 Subject: [PATCH] Fix x86 dumps from HandleFatalError showing misleading callstack (dotnet/coreclr#23289) Commit migrated from https://github.com/dotnet/coreclr/commit/5bfb7417d5841c7ff79452f92e13603c5b0459fc --- src/coreclr/src/vm/crossgencompile.cpp | 6 +++++- src/coreclr/src/vm/eepolicy.cpp | 28 +++++++++++++++++++++++++--- src/coreclr/src/vm/eepolicy.h | 2 +- src/coreclr/src/vm/encee.cpp | 1 + src/coreclr/src/vm/excep.cpp | 2 ++ src/coreclr/src/vm/exceptionhandling.cpp | 1 + 6 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/coreclr/src/vm/crossgencompile.cpp b/src/coreclr/src/vm/crossgencompile.cpp index d357bd7..de07c81 100644 --- a/src/coreclr/src/vm/crossgencompile.cpp +++ b/src/coreclr/src/vm/crossgencompile.cpp @@ -363,10 +363,14 @@ extern "C" UINT_PTR STDCALL GetCurrentIP() return 0; } -void EEPolicy::HandleFatalError(UINT exitCode, UINT_PTR address, LPCWSTR pszMessage, PEXCEPTION_POINTERS pExceptionInfo, LPCWSTR errorSource, LPCWSTR argExceptionString) +// This method must return a value to avoid getting non-actionable dumps on x86. +// If this method were a DECLSPEC_NORETURN then dumps would not provide the necessary +// context at the point of the failure +int NOINLINE EEPolicy::HandleFatalError(UINT exitCode, UINT_PTR address, LPCWSTR pszMessage, PEXCEPTION_POINTERS pExceptionInfo, LPCWSTR errorSource, LPCWSTR argExceptionString) { fprintf(stderr, "Fatal error: %08x\n", exitCode); ExitProcess(exitCode); + return -1; } //--------------------------------------------------------------------------------------- diff --git a/src/coreclr/src/vm/eepolicy.cpp b/src/coreclr/src/vm/eepolicy.cpp index bba511a..a6ecda0 100644 --- a/src/coreclr/src/vm/eepolicy.cpp +++ b/src/coreclr/src/vm/eepolicy.cpp @@ -1194,10 +1194,26 @@ void DECLSPEC_NORETURN EEPolicy::HandleFatalStackOverflow(EXCEPTION_POINTERS *pE UNREACHABLE(); } +#if defined(_TARGET_X86_) && defined(PLATFORM_WINDOWS) +// This noinline method is required to ensure that RtlCaptureContext captures +// the context of HandleFatalError. On x86 RtlCaptureContext will not capture +// the current method's context +// NOTE: explicitly turning off optimizations to force the compiler to spill to the +// stack and establish a stack frame. This is required to ensure that +// RtlCaptureContext captures the context of HandleFatalError +#pragma optimize("", off) +int NOINLINE WrapperClrCaptureContext(CONTEXT* context) +{ + ClrCaptureContext(context); + return 0; +} +#pragma optimize("", on) +#endif // defined(_TARGET_X86_) && defined(PLATFORM_WINDOWS) - - -void DECLSPEC_NORETURN EEPolicy::HandleFatalError(UINT exitCode, UINT_PTR address, LPCWSTR pszMessage /* = NULL */, PEXCEPTION_POINTERS pExceptionInfo /* = NULL */, LPCWSTR errorSource /* = NULL */, LPCWSTR argExceptionString /* = NULL */) +// This method must return a value to avoid getting non-actionable dumps on x86. +// If this method were a DECLSPEC_NORETURN then dumps would not provide the necessary +// context at the point of the failure +int NOINLINE EEPolicy::HandleFatalError(UINT exitCode, UINT_PTR address, LPCWSTR pszMessage /* = NULL */, PEXCEPTION_POINTERS pExceptionInfo /* = NULL */, LPCWSTR errorSource /* = NULL */, LPCWSTR argExceptionString /* = NULL */) { WRAPPER_NO_CONTRACT; @@ -1215,7 +1231,12 @@ void DECLSPEC_NORETURN EEPolicy::HandleFatalError(UINT exitCode, UINT_PTR addres ZeroMemory(&context, sizeof(context)); context.ContextFlags = CONTEXT_CONTROL; +#if defined(_TARGET_X86_) && defined(PLATFORM_WINDOWS) + // Add a frame to ensure that the context captured is this method and not the caller + WrapperClrCaptureContext(&context); +#else // defined(_TARGET_X86_) && defined(PLATFORM_WINDOWS) ClrCaptureContext(&context); +#endif exceptionRecord.ExceptionCode = exitCode; exceptionRecord.ExceptionAddress = reinterpret_cast< PVOID >(address); @@ -1269,6 +1290,7 @@ void DECLSPEC_NORETURN EEPolicy::HandleFatalError(UINT exitCode, UINT_PTR addres } UNREACHABLE(); + return -1; } void EEPolicy::HandleExitProcessFromEscalation(EPolicyAction action, UINT exitCode) diff --git a/src/coreclr/src/vm/eepolicy.h b/src/coreclr/src/vm/eepolicy.h index dc997db..a87527c 100644 --- a/src/coreclr/src/vm/eepolicy.h +++ b/src/coreclr/src/vm/eepolicy.h @@ -120,7 +120,7 @@ public: static void HandleExitProcess(ShutdownCompleteAction sca = SCA_ExitProcessWhenShutdownComplete); - static void DECLSPEC_NORETURN HandleFatalError(UINT exitCode, UINT_PTR address, LPCWSTR pMessage=NULL, PEXCEPTION_POINTERS pExceptionInfo= NULL, LPCWSTR errorSource=NULL, LPCWSTR argExceptionString=NULL); + static int NOINLINE HandleFatalError(UINT exitCode, UINT_PTR address, LPCWSTR pMessage=NULL, PEXCEPTION_POINTERS pExceptionInfo= NULL, LPCWSTR errorSource=NULL, LPCWSTR argExceptionString=NULL); static void DECLSPEC_NORETURN HandleFatalStackOverflow(EXCEPTION_POINTERS *pException, BOOL fSkipDebugger = FALSE); diff --git a/src/coreclr/src/vm/encee.cpp b/src/coreclr/src/vm/encee.cpp index 7291256..a13e3bf 100644 --- a/src/coreclr/src/vm/encee.cpp +++ b/src/coreclr/src/vm/encee.cpp @@ -683,6 +683,7 @@ HRESULT EditAndContinueModule::ResumeInUpdatedFunction( // If we fail for any reason we have already potentially trashed with new locals and we have also unwound any // Win32 handlers on the stack so cannot ever return from this function. EEPOLICY_HANDLE_FATAL_ERROR(CORDBG_E_ENC_INTERNAL_ERROR); + return hr; } //--------------------------------------------------------------------------------------- diff --git a/src/coreclr/src/vm/excep.cpp b/src/coreclr/src/vm/excep.cpp index 26003f0..6d62f9c 100644 --- a/src/coreclr/src/vm/excep.cpp +++ b/src/coreclr/src/vm/excep.cpp @@ -2983,6 +2983,7 @@ VOID DECLSPEC_NORETURN RaiseTheExceptionInternalOnly(OBJECTREF throwable, BOOL r // User hits 'g' // Then debugger can bring us here. EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); + UNREACHABLE(); } @@ -11945,6 +11946,7 @@ void ExceptionNotifications::GetEventArgsForNotification(ExceptionNotificationHa static LONG ExceptionNotificationFilter(PEXCEPTION_POINTERS pExceptionInfo, LPVOID pParam) { EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); + return -1; } #ifdef FEATURE_CORRUPTING_EXCEPTIONS diff --git a/src/coreclr/src/vm/exceptionhandling.cpp b/src/coreclr/src/vm/exceptionhandling.cpp index 838450b..8574caf 100644 --- a/src/coreclr/src/vm/exceptionhandling.cpp +++ b/src/coreclr/src/vm/exceptionhandling.cpp @@ -4693,6 +4693,7 @@ VOID DECLSPEC_NORETURN UnwindManagedExceptionPass1(PAL_SEHException& ex, CONTEXT _ASSERTE(!"UnwindManagedExceptionPass1: Failed to find a handler. Reached the end of the stack"); EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); + UNREACHABLE(); } VOID DECLSPEC_NORETURN DispatchManagedException(PAL_SEHException& ex, bool isHardwareException) -- 2.7.4