Improve stack overflow reporting (#9650)

* Improve stack overflow reporting

This change modifies the SIGSEGV handling to use an alternate stack so that
we can safely detect and report stack overflow even in case when we are
really out of stack. Before, we were able to detect stack overflow and report
it only when JIT inserted stack probes (for functions with frames larger than
4kB) and so there was still space on the stack to run the sigsegv handler.
It brings in some additional complexity, since we need to switch to the original
stack of the thread once we figure out the sigsegv is not a stack overflow and
if we return from the hardware exception handler, we need to switch back to the
alternate stack before returning from the sigsegv handler.
Also, the alternate stack is created per thread and so we need to correctly destroy
it when a thread terminates and also install it on foreign threads that enter PAL.
This also requires creating fake stack frames to enable the libunwind to walk
the stack from the exception handler to the sigsegv location.

* Fix stack unwinding in CallDescrWorkerInternal

While testing the change to enable stack overflow handling, I've noticed that
the PROLOG_SAVE_REG_PAIR and PROLOG_SAVE_REG_PAIR_INDEXED macros are missing
.cfi_def_cfa_register fp. That resulted in inability to unwind through the
CallDescrWorkerInternal, since this function dynamically allocates stack slots
and so the default sp based frame doesn't work.

1 files changed, 157 insertions, 8 deletions

diff --git a/src/pal/src/exception/signal.cpp b/src/pal/src/exception/signal.cpp
index 26e2a012c5..d442afaf4c 100644
--- a/src/pal/src/exception/signal.cpp
+++ b/src/pal/src/exception/signal.cpp
@@ -27,12 +27,15 @@ SET_DEFAULT_DEBUG_CHANNEL(EXCEPT); // some headers have code with asserts, so do
 #include "pal/threadinfo.hpp"
 #include "pal/threadsusp.hpp"
 #include "pal/seh.hpp"
+#include "pal/signal.hpp"
 
 #include "pal/palinternal.h"
 #if !HAVE_MACH_EXCEPTIONS
 #include "pal/init.h"
 #include "pal/process.h"
 #include "pal/debug.h"
+#include "pal/virtual.h"
+#include "pal/utils.h"
 
 #include <signal.h>
 #include <errno.h>
@@ -40,6 +43,7 @@ SET_DEFAULT_DEBUG_CHANNEL(EXCEPT); // some headers have code with asserts, so do
 #include <sys/ucontext.h>
 #include <sys/utsname.h>
 #include <unistd.h>
+#include <sys/mman.h>
 
 #include "pal/context.h"
 
@@ -63,6 +67,13 @@ typedef void *siginfo_t;
 #endif  /* !HAVE_SIGINFO_T */
 typedef void (*SIGFUNC)(int, siginfo_t *, void *);
 
+// Return context and status for the signal_handler_worker.
+struct SignalHandlerWorkerReturnPoint
+{
+    bool returnFromHandler;
+    ucontext_t context;
+};
+
 /* internal function declarations *********************************************/
 
 static void sigill_handler(int code, siginfo_t *siginfo, void *context);
@@ -80,7 +91,7 @@ static bool common_signal_handler(int code, siginfo_t *siginfo, void *sigcontext
 static void inject_activation_handler(int code, siginfo_t *siginfo, void *context);
 #endif
 
-static void handle_signal(int signal_id, SIGFUNC sigfunc, struct sigaction *previousAction);
+static void handle_signal(int signal_id, SIGFUNC sigfunc, struct sigaction *previousAction, int additionalFlags = 0);
 static void restore_signal(int signal_id, struct sigaction *previousAction);
 
 /* internal data declarations *********************************************/
@@ -108,6 +119,88 @@ int g_common_signal_handler_context_locvar_offset = 0;
 
 /*++
 Function :
+    EnsureSignalAlternateStack
+
+    Ensure that alternate stack for signal handling is allocated for the current thread
+
+Parameters :
+    None
+
+Return :
+    TRUE in case of a success, FALSE otherwise
+--*/
+BOOL EnsureSignalAlternateStack()
+{
+    stack_t oss;
+
+    // Query the current alternate signal stack
+    int st = sigaltstack(NULL, &oss);
+
+    if ((st == 0) && (oss.ss_flags == SS_DISABLE))
+    {
+        // There is no alternate stack for SIGSEGV handling installed yet so allocate one
+
+        // We include the size of the SignalHandlerWorkerReturnPoint in the alternate stack size since the 
+        // context contained in it is large and the SIGSTKSZ was not sufficient on ARM64 during testing.
+        int altStackSize = SIGSTKSZ + ALIGN_UP(sizeof(SignalHandlerWorkerReturnPoint), 16) + VIRTUAL_PAGE_SIZE;
+        void* altStack;
+        int st = posix_memalign(&altStack, VIRTUAL_PAGE_SIZE, altStackSize);
+        if (st == 0)
+        {
+            // create a guard page for the alternate stack
+            st = mprotect(altStack, VIRTUAL_PAGE_SIZE, PROT_NONE);
+            if (st == 0)
+            {
+                stack_t ss;
+                ss.ss_sp = (char*)altStack;
+                ss.ss_size = altStackSize;
+                ss.ss_flags = 0;
+                st = sigaltstack(&ss, NULL);
+                if (st != 0)
+                {
+                    // Installation of the alternate stack failed, so revert the guard page protection
+                    int st2 = mprotect(altStack, VIRTUAL_PAGE_SIZE, PROT_READ | PROT_WRITE);
+                    _ASSERTE(st2 == 0);
+                }
+            }
+
+            if (st != 0)
+            {
+                free(altStack);
+            }
+        }
+    }
+
+    return (st == 0);
+}
+
+/*++
+Function :
+    FreeSignalAlternateStack
+
+    Free alternate stack for signal handling
+
+Parameters :
+    None
+
+Return :
+    None
+--*/
+void FreeSignalAlternateStack()
+{
+    stack_t ss, oss;
+    ss.ss_flags = SS_DISABLE;
+    int st = sigaltstack(&ss, &oss);
+    if ((st == 0) && (oss.ss_flags != SS_DISABLE))
+    {
+        int st = mprotect(oss.ss_sp, VIRTUAL_PAGE_SIZE, PROT_READ | PROT_WRITE);
+        _ASSERTE(st == 0);
+        free(oss.ss_sp);
+    }
+}
+
+/*++
+Function :
     SEHInitializeSignals
 
     Set up signal handlers to catch signals and translate them to exceptions
@@ -139,10 +232,16 @@ BOOL SEHInitializeSignals(DWORD flags)
     handle_signal(SIGTRAP, sigtrap_handler, &g_previous_sigtrap);
     handle_signal(SIGFPE, sigfpe_handler, &g_previous_sigfpe);
     handle_signal(SIGBUS, sigbus_handler, &g_previous_sigbus);
-    handle_signal(SIGSEGV, sigsegv_handler, &g_previous_sigsegv);
+    // SIGSEGV handler runs on a separate stack so that we can handle stack overflow
+    handle_signal(SIGSEGV, sigsegv_handler, &g_previous_sigsegv, SA_ONSTACK);
     handle_signal(SIGINT, sigint_handler, &g_previous_sigint);
     handle_signal(SIGQUIT, sigquit_handler, &g_previous_sigquit);
 
+    if (!EnsureSignalAlternateStack())
+    {
+        return FALSE;
+    }
+
     if (flags & PAL_INITIALIZE_REGISTER_SIGTERM_HANDLER)
     {
         handle_signal(SIGTERM, sigterm_handler, &g_previous_sigterm);
@@ -276,6 +375,28 @@ static void sigfpe_handler(int code, siginfo_t *siginfo, void *context)
 
 /*++
 Function :
+    signal_handler_worker
+
+    Handles signal on the original stack where the signal occured. 
+    Invoked via setcontext.
+
+Parameters :
+    POSIX signal handler parameter list ("man sigaction" for details)
+    returnPoint - context to which the function returns if the common_signal_handler returns
+
+    (no return value)
+--*/
+extern "C" void signal_handler_worker(int code, siginfo_t *siginfo, void *context, SignalHandlerWorkerReturnPoint* returnPoint)
+{
+    // TODO: First variable parameter says whether a read (0) or write (non-0) caused the
+    // fault. We must disassemble the instruction at record.ExceptionAddress
+    // to correctly fill in this value.
+    returnPoint->returnFromHandler = common_signal_handler(code, siginfo, context, 2, (size_t)0, (size_t)siginfo->si_addr);
+    setcontext(&returnPoint->context);
+}
+
+/*++
+Function :
     sigsegv_handler
 
     handle SIGSEGV signal (EXCEPTION_ACCESS_VIOLATION, others)
@@ -289,10 +410,38 @@ static void sigsegv_handler(int code, siginfo_t *siginfo, void *context)
 {
     if (PALIsInitialized())
     {
-        // TODO: First variable parameter says whether a read (0) or write (non-0) caused the
-        // fault. We must disassemble the instruction at record.ExceptionAddress
-        // to correctly fill in this value.
-        if (common_signal_handler(code, siginfo, context, 2, (size_t)0, (size_t)siginfo->si_addr))
+        // First check if we have a stack overflow
+        size_t sp = (size_t)GetNativeContextSP((native_context_t *)context);
+        size_t failureAddress = (size_t)siginfo->si_addr;
+
+        // If the failure address is at most one page above or below the stack pointer, 
+        // we have a stack overflow. 
+        if ((failureAddress - (sp - VIRTUAL_PAGE_SIZE)) < 2 * VIRTUAL_PAGE_SIZE)
+        {
+            (void)write(STDERR_FILENO, StackOverflowMessage, sizeof(StackOverflowMessage) - 1);
+            PROCAbort();
+        }
+
+        // Now that we know the SIGSEGV didn't happen due to a stack overflow, execute the common
+        // hardware signal handler on the original stack.
+
+        // Establish a return point in case the common_signal_handler returns
+
+        volatile bool contextInitialization = true;
+
+        SignalHandlerWorkerReturnPoint returnPoint;
+        getcontext(&returnPoint.context);        
+
+        // When the signal handler worker completes, it uses setcontext to return to this point
+
+        if (contextInitialization)
+        {
+            contextInitialization = false;
+            ExecuteHandlerOnOriginalStack(code, siginfo, context, &returnPoint);
+            _ASSERTE(FALSE); // The ExecuteHandlerOnOriginalStack should never return
+        }
+        
+        if (returnPoint.returnFromHandler)
         {
             return;
         }
@@ -666,11 +815,11 @@ Parameters :
     
 note : if sigfunc is NULL, the default signal handler is restored    
 --*/
-void handle_signal(int signal_id, SIGFUNC sigfunc, struct sigaction *previousAction)
+void handle_signal(int signal_id, SIGFUNC sigfunc, struct sigaction *previousAction, int additionalFlags)
 {
     struct sigaction newAction;
 
-    newAction.sa_flags = SA_RESTART;
+    newAction.sa_flags = SA_RESTART | additionalFlags;
 #if HAVE_SIGINFO_T
     newAction.sa_handler = NULL;
     newAction.sa_sigaction = sigfunc;