Improve fatal err msg (dotnet/coreclr#24390)

* Improve fatal err msg

* Match SO format

* a

* Remove dead define

* More adjustments to ex msg

* And PAL

* Remove special case for SOE

* Remove excess Debug.Assert newline

* Remove excess newline

* typo

* New format

* Remove DebugProvider redundancy

* Adjustments

* Remove preceding newline

* Make other SOE and OOM consistent

* Tidy up assertion msg

* Fix missing newline after inner exception divider

* CR when no inner exception

* ToString never CR terminated

* disable corefx tests temporarily


Commit migrated from https://github.com/dotnet/coreclr/commit/b0381d7bf05cc4a342eb1f7fa33d17885f47d999

src/coreclr/src/dlls/mscorrc/mscorrc.rc

@@ -837,7 +837,7 @@ BEGIN
     IDS_EE_STRUCTARRAYTOOLARGE              "Array size exceeds addressing limitations."
     IDS_EE_TOOMANYFIELDS                    "Internal limitation: too many fields."
 
-    IDS_EE_UNHANDLED_EXCEPTION              "Unhandled Exception:"
+    IDS_EE_UNHANDLED_EXCEPTION              "Unhandled exception."
     IDS_EE_EXCEPTION_TOSTRING_FAILED        "Cannot print exception string because Exception.ToString() failed."
 
     IDS_EE_THREAD_ABORT                     "Thread was being aborted."

src/coreclr/src/pal/src/include/pal/palinternal.h

@@ -760,7 +760,7 @@ inline T* InterlockedCompareExchangePointerT(
 
 #include "volatile.h"
 
-const char StackOverflowMessage[] = "Process is terminating due to StackOverflowException.\n";
+const char StackOverflowMessage[] = "Stack overflow.\n";
 
 #endif // __cplusplus
 

src/coreclr/src/vm/comutilnative.cpp

@@ -43,8 +43,6 @@
 
 #include "arraynative.inl"
 
-#define STACK_OVERFLOW_MESSAGE   W("StackOverflowException")
-
 /*===================================IsDigit====================================
 **Returns a bool indicating whether the character passed in represents a   **
 **digit.
@@ -536,14 +534,6 @@ void ExceptionNative::GetExceptionData(OBJECTREF objException, ExceptionData *pE
 
     ZeroMemory(pED, sizeof(ExceptionData));
 
-    if (objException->GetMethodTable() == g_pStackOverflowExceptionClass) {
-        // In a low stack situation, most everything else in here will fail.
-        // <TODO>@TODO: We're not turning the guard page back on here, yet.</TODO>
-        pED->hr = COR_E_STACKOVERFLOW;
-        pED->bstrDescription = SysAllocString(STACK_OVERFLOW_MESSAGE);
-        return;
-    }
-
     GCPROTECT_BEGIN(objException);
     pED->hr = GetExceptionHResult(objException);
     pED->bstrDescription = GetExceptionDescription(objException);

src/coreclr/src/vm/eepolicy.cpp

@@ -804,8 +804,8 @@ inline void LogCallstackForLogWorker()
 // Arguments:
 //    exitCode - code of the fatal error
 //    pszMessage - error message (can be NULL)
-//    errorSource - details on the source of the error
-//    argExceptionString - exception details
+//    errorSource - details on the source of the error (can be NULL)
+//    argExceptionString - exception details (can be NULL)
 //
 // Return Value:
 //    None
@@ -817,9 +817,13 @@ void LogInfoForFatalError(UINT exitCode, LPCWSTR pszMessage, LPCWSTR errorSource
     Thread *pThread = GetThread();
     EX_TRY
     {
-        if ((exitCode == (UINT)COR_E_FAILFAST) && (errorSource == NULL))
+        if (exitCode == (UINT)COR_E_FAILFAST)
         {
-            PrintToStdErrA("FailFast:\n");
+            PrintToStdErrA("Process terminated. ");
+        }
+        else
+        {
+            PrintToStdErrA("Fatal error. ");
         }
 
         if (errorSource != NULL)
@@ -847,11 +851,7 @@ void LogInfoForFatalError(UINT exitCode, LPCWSTR pszMessage, LPCWSTR errorSource
             LogCallstackForLogWorker();
 
             if (argExceptionString != NULL) {
-                PrintToStdErrA("\n");
-                PrintToStdErrA("Exception details:");
-                PrintToStdErrA("\n");
                 PrintToStdErrW(argExceptionString);
-                PrintToStdErrA("\n");
             }
         }
     }
@@ -1042,9 +1042,8 @@ void EEPolicy::LogFatalError(UINT exitCode, UINT_PTR address, LPCWSTR pszMessage
 void DisplayStackOverflowException()
 {
     LIMITED_METHOD_CONTRACT;
-    PrintToStdErrA("\n");
 
-    PrintToStdErrA("Process is terminating due to StackOverflowException.\n");
+    PrintToStdErrA("Stack overflow.\n");
 }
 
 void DECLSPEC_NORETURN EEPolicy::HandleFatalStackOverflow(EXCEPTION_POINTERS *pExceptionInfo, BOOL fSkipDebugger)

src/coreclr/src/vm/eepolicy.h

@@ -181,7 +181,4 @@ inline EEPolicy* GetEEPolicy()
 // FailFast with specific error code and exception details
 #define EEPOLICY_HANDLE_FATAL_ERROR_USING_EXCEPTION_INFO(_exitcode, _pExceptionInfo) EEPolicy::HandleFatalError(_exitcode, GetCurrentIP(), NULL, _pExceptionInfo);
 
-// Failfast with specific error code, exception details, and debug info
-#define EEPOLICY_HANDLE_FATAL_ERROR_USING_EXCEPTION_AND_DEBUG_INFO(_exitcode, _pExceptionInfo, _isDebug) EEPolicy::HandleFatalError(_exitcode, GetCurrentIP(), NULL, _pExceptionInfo, _isDebug);
-
 #endif  // EEPOLICY_H_

src/coreclr/src/vm/eventreporter.cpp

@@ -135,7 +135,7 @@ EventReporter::EventReporter(EventReporterType type)
 
     case ERT_ManagedFailFast:
         if(!ssMessage.LoadResource(CCompRC::Optional, IDS_ER_MANAGEDFAILFAST))
-            m_Description.Append(W("Description: The application requested process termination through System.Environment.FailFast(string message)."));
+            m_Description.Append(W("Description: The application requested process termination through Environment.FailFast."));
         else
         {
             m_Description.Append(ssMessage);
@@ -145,7 +145,7 @@ EventReporter::EventReporter(EventReporterType type)
 
     case ERT_UnmanagedFailFast:
         if(!ssMessage.LoadResource(CCompRC::Optional, IDS_ER_UNMANAGEDFAILFAST))
-            m_Description.Append(W("Description: The process was terminated due to an internal error in the .NET Runtime "));
+            m_Description.Append(W("Description: The process was terminated due to an internal error in the .NET Runtime."));
         else
         {
             m_Description.Append(ssMessage);
@@ -155,7 +155,7 @@ EventReporter::EventReporter(EventReporterType type)
     case ERT_StackOverflow:
         // Fetch the localized Stack Overflow Error text or fall back on a hardcoded variant if things get dire.
         if(!ssMessage.LoadResource(CCompRC::Optional, IDS_ER_STACK_OVERFLOW))
-            m_Description.Append(W("Description: The process was terminated due to stack overflow."));
+            m_Description.Append(W("Description: The process was terminated due to a stack overflow."));
         else
         {
             m_Description.Append(ssMessage);

src/coreclr/src/vm/excep.cpp

@@ -105,7 +105,7 @@ BOOL IsExceptionFromManagedCode(const EXCEPTION_RECORD * pExceptionRecord)
 
 #ifndef DACCESS_COMPILE
 
-#define SZ_UNHANDLED_EXCEPTION W("Unhandled Exception:")
+#define SZ_UNHANDLED_EXCEPTION W("Unhandled exception.")
 #define SZ_UNHANDLED_EXCEPTION_CHARLEN ((sizeof(SZ_UNHANDLED_EXCEPTION) / sizeof(WCHAR)))
 
 
@@ -5243,8 +5243,6 @@ DefaultCatchHandlerExceptionMessageWorker(Thread* pThread,
     GCPROTECT_BEGIN(throwable);
     if (throwable != NULL)
     {
-        PrintToStdErrA("\n");
-
         if (FAILED(UtilLoadResourceString(CCompRC::Error, IDS_EE_UNHANDLED_EXCEPTION, buf, buf_size)))
         {
             wcsncpy_s(buf, buf_size, SZ_UNHANDLED_EXCEPTION, SZ_UNHANDLED_EXCEPTION_CHARLEN);
@@ -5440,22 +5438,14 @@ DefaultCatchHandler(PEXCEPTION_POINTERS pExceptionPointers,
                     // die. e.g. IsAsyncThreadException() and Exception.ToString both consume too much stack -- and can't
                     // be called here.
                     dump = FALSE;
-                    PrintToStdErrA("\n");
-
-                    if (FAILED(UtilLoadStringRC(IDS_EE_UNHANDLED_EXCEPTION, buf, buf_size)))
-                    {
-                        wcsncpy_s(buf, COUNTOF(buf), SZ_UNHANDLED_EXCEPTION, SZ_UNHANDLED_EXCEPTION_CHARLEN);
-                    }
-
-                    PrintToStdErrW(buf);
 
                     if (IsOutOfMemory)
                     {
-                        PrintToStdErrA(" OutOfMemoryException.\n");
+                        PrintToStdErrA("Out of memory.\n");
                     }
                     else
                     {
-                        PrintToStdErrA(" StackOverflowException.\n");
+                        PrintToStdErrA("Stack overflow.\n");
                     }
                 }
                 else if (SentEvent || IsAsyncThreadException(&throwable))

src/coreclr/tests/CoreFX/CoreFX.issues.rsp

@@ -51,3 +51,8 @@
 -nomethod System.Tests.AppDomainTests.MonitoringTotalAllocatedMemorySize
 -nomethod System.Tests.AppDomainTests.MonitoringTotalProcessorTime
 -nomethod System.Text.Tests.StringBuilderTests.AppendFormat
+
+# requires corefx test updates
+-nomethod System.Tests.EnvironmentTests.FailFast_ExceptionStackTrace_StackOverflowException
+-nomethod System.Tests.EnvironmentTests.FailFast_ExceptionStackTrace_InnerException
+-nomethod System.Tests.EnvironmentTests.FailFast_ExceptionStackTrace_ArgumentException

src/libraries/System.Private.CoreLib/src/System/Diagnostics/DebugProvider.Unix.cs

@@ -27,19 +27,7 @@ public static void FailCore(string stackTrace, string? message, string? detailMe
                 // In Core, we do not show a dialog.
                 // Fail in order to avoid anyone catching an exception and masking
                 // an assert failure.
-                DebugAssertException ex;
-                if (message == string.Empty) 
-                {
-                    ex = new DebugAssertException(stackTrace);
-                }
-                else if (detailMessage == string.Empty) 
-                {
-                    ex = new DebugAssertException(message, stackTrace);
-                }
-                else
-                {
-                    ex = new DebugAssertException(message, detailMessage, stackTrace);
-                }
+                DebugAssertException ex = new DebugAssertException(message, detailMessage, stackTrace);
                 Environment.FailFast(ex.Message, ex, errorSource);
             }
         }

src/libraries/System.Private.CoreLib/src/System/Diagnostics/DebugProvider.Windows.cs

@@ -23,19 +23,7 @@ public static void FailCore(string stackTrace, string? message, string? detailMe
                 // In Core, we do not show a dialog.
                 // Fail in order to avoid anyone catching an exception and masking
                 // an assert failure.
-                DebugAssertException ex;
-                if (message == string.Empty) 
-                {
-                    ex = new DebugAssertException(stackTrace);
-                }
-                else if (detailMessage == string.Empty) 
-                {
-                    ex = new DebugAssertException(message, stackTrace);
-                }
-                else
-                {
-                    ex = new DebugAssertException(message, detailMessage, stackTrace);
-                }
+                DebugAssertException ex = new DebugAssertException(message, detailMessage, stackTrace);
                 Environment.FailFast(ex.Message, ex, errorSource);
             }
         }

src/libraries/System.Private.CoreLib/src/System/Diagnostics/DebugProvider.cs

@@ -24,7 +24,7 @@ public virtual void Fail(string? message, string? detailMessage)
                 stackTrace = "";
             }
             WriteAssert(stackTrace, message, detailMessage);
-            FailCore(stackTrace, message, detailMessage, "Assertion Failed");
+            FailCore(stackTrace, message, detailMessage, "Assertion failed.");
         }
 
         internal void WriteAssert(string stackTrace, string? message, string? detailMessage)
@@ -72,19 +72,21 @@ public virtual void WriteLine(string? message)
 
         private sealed class DebugAssertException : Exception
         {
-            internal DebugAssertException(string? stackTrace) :
-                base(Environment.NewLine + stackTrace)
+            internal DebugAssertException(string? message, string? detailMessage, string? stackTrace) :
+                base(Terminate(message) + Terminate(detailMessage) + stackTrace)
             {
             }
 
-            internal DebugAssertException(string? message, string? stackTrace) :
-                base(message + Environment.NewLine + Environment.NewLine + stackTrace)
+            private static string? Terminate(string? s)
             {
-            }
+                if (s == null)
+                    return s;
 
-            internal DebugAssertException(string? message, string? detailMessage, string? stackTrace) :
-                base(message + Environment.NewLine + detailMessage + Environment.NewLine + Environment.NewLine + stackTrace)
-            {
+                s = s.Trim();
+                if (s.Length > 0)
+                    s += Environment.NewLine;
+
+                return s;
             }
         }