From: Jan Vorlicek <janvorli@microsoft.com>
Date: Fri, 6 Mar 2020 22:48:35 +0000 (+0100)
Subject: Improve stack overflow stack trace dumping (#33281)
X-Git-Tag: submit/tizen/20210909.063632~9296
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=69264d7e3ad062f6f45510fb4d38cb68fb023baf;p=platform%2Fupstream%2Fdotnet%2Fruntime.git

Improve stack overflow stack trace dumping (#33281)

It was discovered that the stack overflow stack trace dumping to console
doesn't work properly in some cases due to the fact that there was not
enough stack space left to display more complex method signatures on the
call stack.

To remove such fragility, this change updates the stack trace dumping by
running the actual dumping and stack walking on a new thread and
waiting for its completion. That means that we should be able to dump
any stack trace reliably.
---

diff --git a/src/coreclr/src/vm/eepolicy.cpp b/src/coreclr/src/vm/eepolicy.cpp
index c2a472e..d01c988 100644
--- a/src/coreclr/src/vm/eepolicy.cpp
+++ b/src/coreclr/src/vm/eepolicy.cpp
@@ -851,16 +851,13 @@ public:
 // Return Value:
 //    None
 //
-inline void LogCallstackForLogWorker(bool isStackOverflow = false)
+inline void LogCallstackForLogWorker(Thread* pThread)
 {
     WRAPPER_NO_CONTRACT;
 
-    Thread* pThread = GetThread();
-    _ASSERTE (pThread);
-
     SmallStackSString WordAt;
 
-    if (isStackOverflow || !WordAt.LoadResource(CCompRC::Optional, IDS_ER_WORDAT))
+    if (!WordAt.LoadResource(CCompRC::Optional, IDS_ER_WORDAT))
     {
         WordAt.Set(W("   at"));
     }
@@ -872,7 +869,7 @@ inline void LogCallstackForLogWorker(bool isStackOverflow = false)
 
     CallStackLogger logger;
 
-    pThread->StackWalkFrames(&CallStackLogger::LogCallstackForLogCallback, &logger, QUICKUNWIND | FUNCTIONSONLY);
+    pThread->StackWalkFrames(&CallStackLogger::LogCallstackForLogCallback, &logger, QUICKUNWIND | FUNCTIONSONLY | ALLOW_ASYNC_STACK_WALK);
 
     logger.PrintStackTrace(WordAt.GetUnicode());
 
@@ -929,7 +926,7 @@ void LogInfoForFatalError(UINT exitCode, LPCWSTR pszMessage, LPCWSTR errorSource
 
         if (pThread && errorSource == NULL)
         {
-            LogCallstackForLogWorker();
+            LogCallstackForLogWorker(GetThread());
 
             if (argExceptionString != NULL) {
                 PrintToStdErrW(argExceptionString);
@@ -1127,6 +1124,13 @@ void DisplayStackOverflowException()
     PrintToStdErrA("Stack overflow.\n");
 }
 
+DWORD LogStackOverflowStackTraceThread(void* arg)
+{
+    LogCallstackForLogWorker((Thread*)arg);
+
+    return 0;
+}
+
 void DECLSPEC_NORETURN EEPolicy::HandleFatalStackOverflow(EXCEPTION_POINTERS *pExceptionInfo, BOOL fSkipDebugger)
 {
     // This is fatal error.  We do not care about SO mode any more.
@@ -1155,7 +1159,15 @@ void DECLSPEC_NORETURN EEPolicy::HandleFatalStackOverflow(EXCEPTION_POINTERS *pE
     if (InterlockedCompareExchange(&g_stackOverflowCallStackLogged, 1, 0) == 0)
     {
         DisplayStackOverflowException();
-        LogCallstackForLogWorker(true /* isStackOverflow */);
+
+        HandleHolder stackDumpThreadHandle = Thread::CreateUtilityThread(Thread::StackSize_Small, LogStackOverflowStackTraceThread, GetThread(), W(".NET Stack overflow trace logger"));
+        if (stackDumpThreadHandle != INVALID_HANDLE_VALUE)
+        {
+            // Wait for the stack trace logging completion
+            DWORD res = WaitForSingleObject(stackDumpThreadHandle, INFINITE);
+            _ASSERTE(res == WAIT_OBJECT_0);
+        }
+
         g_stackOverflowCallStackLogged = 2;
     }
     else
diff --git a/src/coreclr/src/vm/threads.cpp b/src/coreclr/src/vm/threads.cpp
index 13f1a37..5c18140 100644
--- a/src/coreclr/src/vm/threads.cpp
+++ b/src/coreclr/src/vm/threads.cpp
@@ -6475,7 +6475,7 @@ HRESULT Thread::CLRSetThreadStackGuarantee(SetThreadStackGuaranteeScope fScope)
         // -additionally, we need to provide some region to hosts to allow for lock acquisition in a hosted scenario
         //
         EXTRA_PAGES = 3;
-        INDEBUG(EXTRA_PAGES += 3);
+        INDEBUG(EXTRA_PAGES += 1);
 
         int ThreadGuardPages = CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_ThreadGuardPages);
         if (ThreadGuardPages == 0)
@@ -6489,7 +6489,7 @@ HRESULT Thread::CLRSetThreadStackGuarantee(SetThreadStackGuaranteeScope fScope)
 
 #else // HOST_64BIT
 #ifdef _DEBUG
-        uGuardSize += (3 * GetOsPageSize());    // three extra pages for debug infrastructure
+        uGuardSize += (1 * GetOsPageSize());    // one extra page for debug infrastructure
 #endif // _DEBUG
 #endif // HOST_64BIT