Fix x86 dumps from HandleFatalError showing misleading callstack (#23289)

Author: elinor-fung

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;
 }
 
 //---------------------------------------------------------------------------------------

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)

src/vm/eepolicy.h

@@ -120,7 +120,7 @@ class EEPolicy
 
     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);
 

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;
 }
 
 //---------------------------------------------------------------------------------------

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

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)