Fix possibility of deadlock in the SIGSEGV handler

The SIGSEGV handler was transitively calling pthread_attr_get_np to
get current stack limit no matter where the exception happened. The
problem was that if the segmentation violation happened in a C
runtime function, this could lead to deadlock since pthread_attr_get_np
is not signal safe.

The fix is to add callback that will figure out if the failing instruction
was in managed code or in one of the jit helpers and if not, it would
not attempt to handle the signal.

I have also removed non signal safe calls from the signal handlers code
path where we don't know yet whether the interrupted code was in a function
that could collide with a function that we call from the signal handler
or not. This includes TRACE and ASSERT calls that were calling string
formatting functions.

7 files changed, 159 insertions, 259 deletions

diff --git a/src/debug/debug-pal/unix/twowaypipe.cpp b/src/debug/debug-pal/unix/twowaypipe.cpp
index 3eeb15bb78..2bf919feeb 100644
--- a/src/debug/debug-pal/unix/twowaypipe.cpp
+++ b/src/debug/debug-pal/unix/twowaypipe.cpp
@@ -15,7 +15,7 @@
 
 static const char* PipeNameFormat = "/tmp/clr-debug-pipe-%d-%llu-%s";
 
-static void GetPipeName(char *name, DWORD id, const char *suffix)
+void TwoWayPipe::GetPipeName(char *name, DWORD id, const char *suffix)
 {
     UINT64 disambiguationKey;
     BOOL ret = GetProcessIdDisambiguationKey(id, &disambiguationKey);
@@ -25,8 +25,8 @@ static void GetPipeName(char *name, DWORD id, const char *suffix)
     // also try to use 0 as the value.
     _ASSERTE(ret == TRUE || disambiguationKey == 0);
 
-    int chars = _snprintf(name, PATH_MAX, PipeNameFormat, id, disambiguationKey, suffix);
-    _ASSERTE(chars > 0 && chars < PATH_MAX);
+    int chars = _snprintf(name, MaxPipeNameLength, PipeNameFormat, id, disambiguationKey, suffix);
+    _ASSERTE(chars > 0 && chars < MaxPipeNameLength);
 }
 
 // Creates a server side of the pipe. 
@@ -39,19 +39,17 @@ bool TwoWayPipe::CreateServer(DWORD id)
         return false;
 
     m_id = id;
-    char inPipeName[PATH_MAX];
-    char outPipeName[PATH_MAX];
-    GetPipeName(inPipeName, id, "in");
-    GetPipeName(outPipeName, id, "out");
+    GetPipeName(m_inPipeName, id, "in");
+    GetPipeName(m_outPipeName, id, "out");
 
-    if (mkfifo(inPipeName, S_IRWXU) == -1)
+    if (mkfifo(m_inPipeName, S_IRWXU) == -1)
     {
         return false;
     }
 
-    if (mkfifo(outPipeName, S_IRWXU) == -1)
+    if (mkfifo(m_outPipeName, S_IRWXU) == -1)
     {
-        unlink(inPipeName);
+        unlink(m_inPipeName);
         return false;
     }
 
@@ -69,21 +67,19 @@ bool TwoWayPipe::Connect(DWORD id)
         return false;
 
     m_id = id;
-    char inPipeName[PATH_MAX];
-    char outPipeName[PATH_MAX];
     //"in" and "out" are switched deliberately, because we're on the client
-    GetPipeName(inPipeName, id, "out");
-    GetPipeName(outPipeName, id, "in");
+    GetPipeName(m_inPipeName, id, "out");
+    GetPipeName(m_outPipeName, id, "in");
 
     // Pipe opening order is reversed compared to WaitForConnection()
     // in order to avaid deadlock.
-    m_outboundPipe = open(outPipeName, O_WRONLY);
+    m_outboundPipe = open(m_outPipeName, O_WRONLY);
     if (m_outboundPipe == INVALID_PIPE)
     {
         return false;
     }
 
-    m_inboundPipe = open(inPipeName, O_RDONLY);
+    m_inboundPipe = open(m_inPipeName, O_RDONLY);
     if (m_inboundPipe == INVALID_PIPE)
     {
         close(m_outboundPipe);
@@ -104,18 +100,13 @@ bool TwoWayPipe::WaitForConnection()
     if (m_state != Created)
         return false;
 
-    char inPipeName[PATH_MAX];
-    char outPipeName[PATH_MAX];
-    GetPipeName(inPipeName, m_id, "in");
-    GetPipeName(outPipeName, m_id, "out");
-
-    m_inboundPipe = open(inPipeName, O_RDONLY);
+    m_inboundPipe = open(m_inPipeName, O_RDONLY);
     if (m_inboundPipe == INVALID_PIPE)
     {
         return false;
     }
 
-    m_outboundPipe = open(outPipeName, O_WRONLY);
+    m_outboundPipe = open(m_outPipeName, O_WRONLY);
     if (m_outboundPipe == INVALID_PIPE)
     {
         close(m_inboundPipe);
@@ -185,6 +176,10 @@ int TwoWayPipe::Write(const void *data, DWORD dataSize)
 // true - success, false - failure (use GetLastError() for more details)
 bool TwoWayPipe::Disconnect()
 {
+    // IMPORTANT NOTE: This function must not call any signal unsafe functions
+    // since it is called from signal handlers.
+    // That includes ASSERT and TRACE macros.
+
     if (m_outboundPipe != INVALID_PIPE && m_outboundPipe != 0)
     {
         close(m_outboundPipe);
@@ -199,13 +194,8 @@ bool TwoWayPipe::Disconnect()
 
     if (m_state == ServerConnected || m_state == Created)
     {
-        char inPipeName[PATH_MAX];
-        GetPipeName(inPipeName, m_id, "in");
-        unlink(inPipeName);
-
-        char outPipeName[PATH_MAX];
-        GetPipeName(outPipeName, m_id, "out");
-        unlink(outPipeName);
+        unlink(m_inPipeName);
+        unlink(m_outPipeName);
     }
 
     m_state = NotInitialized;
diff --git a/src/debug/inc/twowaypipe.h b/src/debug/inc/twowaypipe.h
index 9ec38452c1..6c2903440f 100644
--- a/src/debug/inc/twowaypipe.h
+++ b/src/debug/inc/twowaypipe.h
@@ -79,8 +79,15 @@ private:
 
 
 #ifdef FEATURE_PAL
-    int m_id;                          //id that was passed to CreateServer() or Connect()
-    int m_inboundPipe, m_outboundPipe; //two one sided pipes used for communication
+
+    static const int MaxPipeNameLength = 64;
+
+    static void GetPipeName(char *name, DWORD id, const char *suffix);
+
+    int m_id;                              //id that was passed to CreateServer() or Connect()
+    int m_inboundPipe, m_outboundPipe;     //two one sided pipes used for communication
+    char m_inPipeName[MaxPipeNameLength];  //filename of the inbound pipe
+    char m_outPipeName[MaxPipeNameLength]; //filename of the outbound pipe
 
 #else
     // Connects to a one sided pipe previously created by CreateOneWayPipe.
diff --git a/src/pal/inc/pal.h b/src/pal/inc/pal.h
index 590cb64b1a..daf3813ad7 100644
--- a/src/pal/inc/pal.h
+++ b/src/pal/inc/pal.h
@@ -6487,13 +6487,15 @@ public:
 };
 
 typedef VOID (PALAPI *PHARDWARE_EXCEPTION_HANDLER)(PAL_SEHException* ex);
+typedef BOOL (PALAPI *PHARDWARE_EXCEPTION_SAFETY_CHECK_FUNCTION)(PCONTEXT contextRecord, PEXCEPTION_RECORD exceptionRecord);
 typedef DWORD (PALAPI *PGET_GCMARKER_EXCEPTION_CODE)(LPVOID ip);
 
 PALIMPORT
 VOID
 PALAPI
 PAL_SetHardwareExceptionHandler(
-    IN PHARDWARE_EXCEPTION_HANDLER exceptionHandler);
+    IN PHARDWARE_EXCEPTION_HANDLER exceptionHandler,
+    IN PHARDWARE_EXCEPTION_SAFETY_CHECK_FUNCTION exceptionCheckFunction);
 
 PALIMPORT
 VOID
diff --git a/src/pal/src/exception/seh.cpp b/src/pal/src/exception/seh.cpp
index 4cc30c23e0..5b5a116f7a 100644
--- a/src/pal/src/exception/seh.cpp
+++ b/src/pal/src/exception/seh.cpp
@@ -52,6 +52,9 @@ const UINT RESERVED_SEH_BIT = 0x800000;
 /* Internal variables definitions **********************************************/
 
 PHARDWARE_EXCEPTION_HANDLER g_hardwareExceptionHandler = NULL;
+// Function to check if an activation can be safely injected at a specified context
+PHARDWARE_EXCEPTION_SAFETY_CHECK_FUNCTION g_safeExceptionCheckFunction = NULL;
+
 PGET_GCMARKER_EXCEPTION_CODE g_getGcMarkerExceptionCode = NULL;
 
 /* Internal function definitions **********************************************/
@@ -124,9 +127,11 @@ Return value:
 VOID
 PALAPI 
 PAL_SetHardwareExceptionHandler(
-    IN PHARDWARE_EXCEPTION_HANDLER exceptionHandler)
+    IN PHARDWARE_EXCEPTION_HANDLER exceptionHandler,
+    IN PHARDWARE_EXCEPTION_SAFETY_CHECK_FUNCTION exceptionCheckFunction)
 {
     g_hardwareExceptionHandler = exceptionHandler;
+    g_safeExceptionCheckFunction = exceptionCheckFunction;
 }
 
 /*++
@@ -212,7 +217,31 @@ SEHProcessException(PEXCEPTION_POINTERS pointers)
 
         if (g_hardwareExceptionHandler != NULL)
         {
-            g_hardwareExceptionHandler(&exception);
+            _ASSERTE(g_safeExceptionCheckFunction != NULL);
+            // Check if it is safe to handle the hardware exception (the exception happened in managed code
+            // or in a jitter helper or it is a debugger breakpoint)
+            if (g_safeExceptionCheckFunction(pointers->ContextRecord, pointers->ExceptionRecord))
+            {
+                if (pointers->ExceptionRecord->ExceptionCode == EXCEPTION_ACCESS_VIOLATION)
+                {
+                    // Check if the failed access has hit a stack guard page. In such case, it
+                    // was a stack probe that detected that there is not enough stack left.
+                    void* stackLimit = CPalThread::GetStackLimit();
+                    void* stackGuard = (void*)((size_t)stackLimit - getpagesize());
+                    void* violationAddr = (void*)pointers->ExceptionRecord->ExceptionInformation[1];
+                    if ((violationAddr >= stackGuard) && (violationAddr < stackLimit))
+                    {
+                        // The exception happened in the page right below the stack limit,
+                        // so it is a stack overflow
+                        write(STDERR_FILENO, StackOverflowMessage, sizeof(StackOverflowMessage) - 1);
+                        PROCAbort();
+                    }
+                }
+
+                // The following callback returns only in case the exception was a single step or 
+                // a breakpoint and it was not handled by the debugger.
+                g_hardwareExceptionHandler(&exception);
+            }
         }
 
         if (CatchHardwareExceptionHolder::IsEnabled())
@@ -221,8 +250,7 @@ SEHProcessException(PEXCEPTION_POINTERS pointers)
         }
     }
 
-    TRACE("Unhandled hardware exception %08x at %p\n", 
-        pointers->ExceptionRecord->ExceptionCode, pointers->ExceptionRecord->ExceptionAddress);
+    // Unhandled hardware exception pointers->ExceptionRecord->ExceptionCode at pointers->ExceptionRecord->ExceptionAddress 
 }
 
 /*++
diff --git a/src/pal/src/exception/signal.cpp b/src/pal/src/exception/signal.cpp
index 25136a703b..b1b8b29264 100644
--- a/src/pal/src/exception/signal.cpp
+++ b/src/pal/src/exception/signal.cpp
@@ -73,8 +73,7 @@ static void sigbus_handler(int code, siginfo_t *siginfo, void *context);
 static void sigint_handler(int code, siginfo_t *siginfo, void *context);
 static void sigquit_handler(int code, siginfo_t *siginfo, void *context);
 
-static void common_signal_handler(PEXCEPTION_POINTERS pointers, int code, 
-                                  native_context_t *ucontext);
+static void common_signal_handler(int code, siginfo_t *siginfo, void *sigcontext, int numParams, ...);
 
 #ifdef INJECT_ACTIVATION_SIGNAL
 static void inject_activation_handler(int code, siginfo_t *siginfo, void *context);
@@ -198,25 +197,9 @@ static void sigill_handler(int code, siginfo_t *siginfo, void *context)
 {
     if (PALIsInitialized())
     {
-        EXCEPTION_RECORD record;
-        EXCEPTION_POINTERS pointers;
-        native_context_t *ucontext;
-
-        ucontext = (native_context_t *)context;
-
-        record.ExceptionCode = CONTEXTGetExceptionCodeForSignal(siginfo, ucontext);
-        record.ExceptionFlags = EXCEPTION_IS_SIGNAL;
-        record.ExceptionRecord = NULL;
-        record.ExceptionAddress = GetNativeContextPC(ucontext);
-        record.NumberParameters = 0;
-
-        pointers.ExceptionRecord = &record;
-
-        common_signal_handler(&pointers, code, ucontext);
+        common_signal_handler(code, siginfo, context, 0);
     }
 
-    TRACE("SIGILL signal was unhandled; chaining to previous sigaction\n");
-
     if (g_previous_sigill.sa_sigaction != NULL)
     {
         g_previous_sigill.sa_sigaction(code, siginfo, context);
@@ -245,25 +228,9 @@ static void sigfpe_handler(int code, siginfo_t *siginfo, void *context)
 {
     if (PALIsInitialized())
     {
-        EXCEPTION_RECORD record;
-        EXCEPTION_POINTERS pointers;
-        native_context_t *ucontext;
-
-        ucontext = (native_context_t *)context;
-
-        record.ExceptionCode = CONTEXTGetExceptionCodeForSignal(siginfo, ucontext);
-        record.ExceptionFlags = EXCEPTION_IS_SIGNAL;
-        record.ExceptionRecord = NULL;
-        record.ExceptionAddress = GetNativeContextPC(ucontext);
-        record.NumberParameters = 0;
-
-        pointers.ExceptionRecord = &record;
-
-        common_signal_handler(&pointers, code, ucontext);
+        common_signal_handler(code, siginfo, context, 0);
     }
 
-    TRACE("SIGFPE signal was unhandled; chaining to previous sigaction\n");
-
     if (g_previous_sigfpe.sa_sigaction != NULL)
     {
         g_previous_sigfpe.sa_sigaction(code, siginfo, context);
@@ -292,48 +259,12 @@ static void sigsegv_handler(int code, siginfo_t *siginfo, void *context)
 {
     if (PALIsInitialized())
     {
-        EXCEPTION_RECORD record;
-        EXCEPTION_POINTERS pointers;
-        native_context_t *ucontext;
-
-        ucontext = (native_context_t *)context;
-
-        record.ExceptionCode = CONTEXTGetExceptionCodeForSignal(siginfo, ucontext);
-        record.ExceptionFlags = EXCEPTION_IS_SIGNAL;
-        record.ExceptionRecord = NULL;
-        record.ExceptionAddress = GetNativeContextPC(ucontext);
-        record.NumberParameters = 2;
-
-        if (record.ExceptionCode == EXCEPTION_ACCESS_VIOLATION)
-        {
-            // Check if the failed access has hit a stack guard page. In such case, it
-            // was a stack probe that detected that there is not enough stack left.
-            void* stackLimit = CPalThread::GetStackLimit();
-            void* stackGuard = (void*)((size_t)stackLimit - getpagesize());
-            if ((siginfo->si_addr >= stackGuard) && (siginfo->si_addr < stackLimit))
-            {
-                // The exception happened in the page right below the stack limit,
-                // so it is a stack overflow
-                write(STDERR_FILENO, StackOverflowMessage, sizeof(StackOverflowMessage) - 1);
-                abort();
-            }
-        }
-
-        // TODO: First parameter says whether a read (0) or write (non-0) caused the
+        // 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.
-        record.ExceptionInformation[0] = 0;
-
-        // Second parameter is the address that caused the fault.
-        record.ExceptionInformation[1] = (size_t)siginfo->si_addr;
-
-        pointers.ExceptionRecord = &record;
-
-        common_signal_handler(&pointers, code, ucontext);
+        common_signal_handler(code, siginfo, context, 2, (size_t)0, (size_t)siginfo->si_addr);
     }
 
-    TRACE("SIGSEGV signal was unhandled; chaining to previous sigaction\n");
-
     if (g_previous_sigsegv.sa_sigaction != NULL)
     {
         g_previous_sigsegv.sa_sigaction(code, siginfo, context);
@@ -362,25 +293,9 @@ static void sigtrap_handler(int code, siginfo_t *siginfo, void *context)
 {
     if (PALIsInitialized())
     {
-        EXCEPTION_RECORD record;
-        EXCEPTION_POINTERS pointers;
-        native_context_t *ucontext;
-
-        ucontext = (native_context_t *)context;
-
-        record.ExceptionCode = CONTEXTGetExceptionCodeForSignal(siginfo, ucontext);
-        record.ExceptionFlags = EXCEPTION_IS_SIGNAL;
-        record.ExceptionRecord = NULL;
-        record.ExceptionAddress = GetNativeContextPC(ucontext);
-        record.NumberParameters = 0;
-
-        pointers.ExceptionRecord = &record;
-
-        common_signal_handler(&pointers, code, ucontext);
+        common_signal_handler(code, siginfo, context, 0);
     }
 
-    TRACE("SIGTRAP signal was unhandled; chaining to previous sigaction\n");
-
     if (g_previous_sigtrap.sa_sigaction != NULL)
     {
         g_previous_sigtrap.sa_sigaction(code, siginfo, context);
@@ -410,33 +325,12 @@ static void sigbus_handler(int code, siginfo_t *siginfo, void *context)
 {
     if (PALIsInitialized())
     {
-        EXCEPTION_RECORD record;
-        EXCEPTION_POINTERS pointers;
-        native_context_t *ucontext;
-
-        ucontext = (native_context_t *)context;
-
-        record.ExceptionCode = CONTEXTGetExceptionCodeForSignal(siginfo, ucontext);
-        record.ExceptionFlags = EXCEPTION_IS_SIGNAL;
-        record.ExceptionRecord = NULL;
-        record.ExceptionAddress = GetNativeContextPC(ucontext);
-        record.NumberParameters = 2;
-
-        // TODO: First parameter says whether a read (0) or write (non-0) caused the
+        // 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.
-        record.ExceptionInformation[0] = 0;
-
-        // Second parameter is the address that caused the fault.
-        record.ExceptionInformation[1] = (size_t)siginfo->si_addr;
-
-        pointers.ExceptionRecord = &record;
-
-        common_signal_handler(&pointers, code, ucontext);
+        common_signal_handler(code, siginfo, context, 2, (size_t)0, (size_t)siginfo->si_addr);
     }
 
-    TRACE("SIGBUS signal was unhandled; chaining to previous sigaction\n");
-
     if (g_previous_sigbus.sa_sigaction != NULL)
     {
         g_previous_sigbus.sa_sigaction(code, siginfo, context);
@@ -463,8 +357,6 @@ Parameters :
 --*/
 static void sigint_handler(int code, siginfo_t *siginfo, void *context)
 {
-    TRACE("SIGINT signal; chaining to previous sigaction\n");
-
     PROCNotifyProcessShutdown();
 
     // Restore the original or default handler and resend signal
@@ -485,8 +377,6 @@ Parameters :
 --*/
 static void sigquit_handler(int code, siginfo_t *siginfo, void *context)
 {
-    TRACE("SIGQUIT signal; chaining to previous sigaction\n");
-
     PROCNotifyProcessShutdown();
 
     // Restore the original or default handler and resend signal
@@ -554,8 +444,6 @@ PAL_ERROR InjectActivationInternal(CorUnix::CPalThread* pThread)
     int status = pthread_kill(pThread->GetPThreadSelf(), INJECT_ACTIVATION_SIGNAL);
     if (status != 0)
     {
-        PROCNotifyProcessShutdown();
-
         // Failure to send the signal is fatal. There are only two cases when sending
         // the signal can fail. First, if the signal ID is invalid and second, 
         // if the thread doesn't exist anymore.
@@ -619,20 +507,42 @@ Function :
     common code for all signal handlers
 
 Parameters :
-    PEXCEPTION_POINTERS pointers : exception information
     int code : signal received
-    native_context_t *ucontext : context structure given to signal handler
+    siginfo_t *siginfo : siginfo passed to the signal handler
+    void *context : context structure passed to the signal handler
+    int numParams : number of variable parameters of the exception
+    ... : variable parameters of the exception (each of size_t type)
 
     (no return value)
 Note:
     the "pointers" parameter should contain a valid exception record pointer,
     but the ContextRecord pointer will be overwritten.
 --*/
-static void common_signal_handler(PEXCEPTION_POINTERS pointers, int code, 
-                                  native_context_t *ucontext)
+static void common_signal_handler(int code, siginfo_t *siginfo, void *sigcontext, int numParams, ...)
 {
     sigset_t signal_set;
     CONTEXT context;
+    EXCEPTION_RECORD record;
+    EXCEPTION_POINTERS pointers;
+    native_context_t *ucontext;
+
+    ucontext = (native_context_t *)sigcontext;
+
+    record.ExceptionCode = CONTEXTGetExceptionCodeForSignal(siginfo, ucontext);
+    record.ExceptionFlags = EXCEPTION_IS_SIGNAL;
+    record.ExceptionRecord = NULL;
+    record.ExceptionAddress = GetNativeContextPC(ucontext);
+    record.NumberParameters = numParams;
+
+    va_list params;
+    va_start(params, numParams);
+
+    for (int i = 0; i < numParams; i++)
+    {
+        record.ExceptionInformation[i] = va_arg(params, size_t);
+    }
+
+    pointers.ExceptionRecord = &record;
 
     // Pre-populate context with data from current frame, because ucontext doesn't have some data (e.g. SS register)
     // which is required for restoring context
@@ -643,7 +553,7 @@ static void common_signal_handler(PEXCEPTION_POINTERS pointers, int code,
     // PEXCEPTION_POINTERS will contain at least the CONTEXT_CONTROL registers.
     CONTEXTFromNativeContext(ucontext, &context, CONTEXT_CONTROL | CONTEXT_INTEGER | CONTEXT_FLOATING_POINT);
 
-    pointers->ContextRecord = &context;
+    pointers.ContextRecord = &context;
 
     /* Unmask signal so we can receive it again */
     sigemptyset(&signal_set);
@@ -654,7 +564,7 @@ static void common_signal_handler(PEXCEPTION_POINTERS pointers, int code,
         ASSERT("pthread_sigmask failed; error number is %d\n", sigmaskRet);
     }
 
-    SEHProcessException(pointers);
+    SEHProcessException(&pointers);
 }
 
 /*++
diff --git a/src/pal/src/thread/context.cpp b/src/pal/src/thread/context.cpp
index 6958961be0..384c7c8e62 100644
--- a/src/pal/src/thread/context.cpp
+++ b/src/pal/src/thread/context.cpp
@@ -591,6 +591,10 @@ Return value :
 DWORD CONTEXTGetExceptionCodeForSignal(const siginfo_t *siginfo,
                                        const native_context_t *context)
 {
+    // IMPORTANT NOTE: This function must not call any signal unsafe functions
+    // since it is called from signal handlers.
+    // That includes ASSERT and TRACE macros.
+
     switch (siginfo->si_signo)
     {
         case SIGILL:
@@ -685,26 +689,24 @@ DWORD CONTEXTGetExceptionCodeForSignal(const siginfo_t *siginfo,
                 case TRAP_TRACE:    // Process trace trap
                     return EXCEPTION_SINGLE_STEP;
                 default:
-                    // We don't want to use ASSERT here since it raises SIGTRAP and we
-                    // might again end up here resulting in an infinite loop! 
-                    // so, we print out an error message and return 
-                    DBG_PRINTF(DLI_ASSERT, defdbgchan, TRUE) 
-                    ("Got unknown SIGTRAP signal (%d) with code %d\n", SIGTRAP, siginfo->si_code);
-
+                    // Got unknown SIGTRAP signal with code siginfo->si_code;
                     return EXCEPTION_ILLEGAL_INSTRUCTION;
             }
         default:
             break;
     }
 
-    ASSERT("Got unknown signal number %d with code %d\n",
-           siginfo->si_signo, siginfo->si_code);
+    // Got unknown signal number siginfo->si_signo with code siginfo->si_code;
     return EXCEPTION_ILLEGAL_INSTRUCTION;
 }
 #else   // ILL_ILLOPC
 DWORD CONTEXTGetExceptionCodeForSignal(const siginfo_t *siginfo,
                                        const native_context_t *context)
 {
+    // IMPORTANT NOTE: This function must not call any signal unsafe functions
+    // since it is called from signal handlers.
+    // That includes ASSERT and TRACE macros.
+
     int trap;
 
     if (siginfo->si_signo == SIGFPE)
@@ -713,48 +715,24 @@ DWORD CONTEXTGetExceptionCodeForSignal(const siginfo_t *siginfo,
         switch (siginfo->si_code)
         {
             case FPE_INTDIV :
-                TRACE("Got signal SIGFPE:FPE_INTDIV; raising "
-                      "EXCEPTION_INT_DIVIDE_BY_ZERO\n");
                 return EXCEPTION_INT_DIVIDE_BY_ZERO;
-                break;
             case FPE_INTOVF :
-                TRACE("Got signal SIGFPE:FPE_INTOVF; raising "
-                      "EXCEPTION_INT_OVERFLOW\n");
                 return EXCEPTION_INT_OVERFLOW;
-                break;
             case FPE_FLTDIV :
-                TRACE("Got signal SIGFPE:FPE_FLTDIV; raising "
-                      "EXCEPTION_FLT_DIVIDE_BY_ZERO\n");
                 return EXCEPTION_FLT_DIVIDE_BY_ZERO;
-                break;
             case FPE_FLTOVF :
-                TRACE("Got signal SIGFPE:FPE_FLTOVF; raising "
-                      "EXCEPTION_FLT_OVERFLOW\n");
                 return EXCEPTION_FLT_OVERFLOW;
-                break;
             case FPE_FLTUND :
-                TRACE("Got signal SIGFPE:FPE_FLTUND; raising "
-                      "EXCEPTION_FLT_UNDERFLOW\n");
                 return EXCEPTION_FLT_UNDERFLOW;
-                break;
             case FPE_FLTRES :
-                TRACE("Got signal SIGFPE:FPE_FLTRES; raising "
-                      "EXCEPTION_FLT_INEXACT_RESULT\n");
                 return EXCEPTION_FLT_INEXACT_RESULT;
-                break;
             case FPE_FLTINV :
-                TRACE("Got signal SIGFPE:FPE_FLTINV; raising "
-                      "EXCEPTION_FLT_INVALID_OPERATION\n");
                 return EXCEPTION_FLT_INVALID_OPERATION;
-                break;
             case FPE_FLTSUB :/* subscript out of range */
-                TRACE("Got signal SIGFPE:FPE_FLTSUB; raising "
-                      "EXCEPTION_FLT_INVALID_OPERATION\n");
                 return EXCEPTION_FLT_INVALID_OPERATION;
-                break;
             default:
-                ASSERT("Got unknown signal code %d\n", siginfo->si_code);
-                break;
+                // Got unknown signal code siginfo->si_code;
+                return 0;
         }
     }
 
@@ -762,71 +740,52 @@ DWORD CONTEXTGetExceptionCodeForSignal(const siginfo_t *siginfo,
     switch (trap)
     {
         case T_PRIVINFLT : /* privileged instruction */
-            TRACE("Trap code T_PRIVINFLT mapped to EXCEPTION_PRIV_INSTRUCTION\n");
             return EXCEPTION_PRIV_INSTRUCTION; 
         case T_BPTFLT :    /* breakpoint instruction */
-            TRACE("Trap code T_BPTFLT mapped to EXCEPTION_BREAKPOINT\n");
             return EXCEPTION_BREAKPOINT;
         case T_ARITHTRAP : /* arithmetic trap */
-            TRACE("Trap code T_ARITHTRAP maps to floating point exception...\n");
             return 0;      /* let the caller pick an exception code */
 #ifdef T_ASTFLT
         case T_ASTFLT :    /* system forced exception : ^C, ^\. SIGINT signal 
                               handler shouldn't be calling this function, since
                               it doesn't need an exception code */
-            ASSERT("Trap code T_ASTFLT received, shouldn't get here\n");
+            // Trap code T_ASTFLT received, shouldn't get here;
             return 0;
 #endif  // T_ASTFLT
         case T_PROTFLT :   /* protection fault */
-            TRACE("Trap code T_PROTFLT mapped to EXCEPTION_ACCESS_VIOLATION\n");
             return EXCEPTION_ACCESS_VIOLATION; 
         case T_TRCTRAP :   /* debug exception (sic) */
-            TRACE("Trap code T_TRCTRAP mapped to EXCEPTION_SINGLE_STEP\n");
             return EXCEPTION_SINGLE_STEP;
         case T_PAGEFLT :   /* page fault */
-            TRACE("Trap code T_PAGEFLT mapped to EXCEPTION_ACCESS_VIOLATION\n");
             return EXCEPTION_ACCESS_VIOLATION;
         case T_ALIGNFLT :  /* alignment fault */
-            TRACE("Trap code T_ALIGNFLT mapped to EXCEPTION_DATATYPE_MISALIGNMENT\n");
             return EXCEPTION_DATATYPE_MISALIGNMENT;
         case T_DIVIDE :
-            TRACE("Trap code T_DIVIDE mapped to EXCEPTION_INT_DIVIDE_BY_ZERO\n");
             return EXCEPTION_INT_DIVIDE_BY_ZERO;
         case T_NMI :       /* non-maskable trap */
-            TRACE("Trap code T_NMI mapped to EXCEPTION_ILLEGAL_INSTRUCTION\n");
             return EXCEPTION_ILLEGAL_INSTRUCTION;
         case T_OFLOW :
-            TRACE("Trap code T_OFLOW mapped to EXCEPTION_INT_OVERFLOW\n");
             return EXCEPTION_INT_OVERFLOW;
         case T_BOUND :     /* bound instruction fault */
-            TRACE("Trap code T_BOUND mapped to EXCEPTION_ARRAY_BOUNDS_EXCEEDED\n");
             return EXCEPTION_ARRAY_BOUNDS_EXCEEDED; 
         case T_DNA :       /* device not available fault */
-            TRACE("Trap code T_DNA mapped to EXCEPTION_ILLEGAL_INSTRUCTION\n");
             return EXCEPTION_ILLEGAL_INSTRUCTION; 
         case T_DOUBLEFLT : /* double fault */
-            TRACE("Trap code T_DOUBLEFLT mapped to EXCEPTION_ILLEGAL_INSTRUCTION\n");
             return EXCEPTION_ILLEGAL_INSTRUCTION; 
         case T_FPOPFLT :   /* fp coprocessor operand fetch fault */
-            TRACE("Trap code T_FPOPFLT mapped to EXCEPTION_FLT_INVALID_OPERATION\n");
             return EXCEPTION_FLT_INVALID_OPERATION; 
         case T_TSSFLT :    /* invalid tss fault */
-            TRACE("Trap code T_TSSFLT mapped to EXCEPTION_ILLEGAL_INSTRUCTION\n");
             return EXCEPTION_ILLEGAL_INSTRUCTION; 
         case T_SEGNPFLT :  /* segment not present fault */
-            TRACE("Trap code T_SEGNPFLT mapped to EXCEPTION_ACCESS_VIOLATION\n");
             return EXCEPTION_ACCESS_VIOLATION; 
         case T_STKFLT :    /* stack fault */
-            TRACE("Trap code T_STKFLT mapped to EXCEPTION_STACK_OVERFLOW\n");
             return EXCEPTION_STACK_OVERFLOW; 
         case T_MCHK :      /* machine check trap */
-            TRACE("Trap code T_MCHK mapped to EXCEPTION_ILLEGAL_INSTRUCTION\n");
             return EXCEPTION_ILLEGAL_INSTRUCTION; 
         case T_RESERVED :  /* reserved (unknown) */
-            TRACE("Trap code T_RESERVED mapped to EXCEPTION_ILLEGAL_INSTRUCTION\n");
             return EXCEPTION_ILLEGAL_INSTRUCTION; 
         default:
-            ASSERT("Got unknown trap code %d\n", trap);
+            // Got unknown trap code trap;
             break;
     }
     return EXCEPTION_ILLEGAL_INSTRUCTION;
diff --git a/src/vm/exceptionhandling.cpp b/src/vm/exceptionhandling.cpp
index c3c5db2073..c17faf119a 100644
--- a/src/vm/exceptionhandling.cpp
+++ b/src/vm/exceptionhandling.cpp
@@ -94,6 +94,7 @@ MethodDesc * GetUserMethodForILStub(Thread * pThread, UINT_PTR uStubSP, MethodDe
 
 #ifdef FEATURE_PAL
 VOID PALAPI HandleHardwareException(PAL_SEHException* ex);
+BOOL PALAPI IsSafeToHandleHardwareException(PCONTEXT contextRecord, PEXCEPTION_RECORD exceptionRecord);
 #endif // FEATURE_PAL
 
 static ExceptionTracker* GetTrackerMemory()
@@ -154,7 +155,7 @@ void InitializeExceptionHandling()
 
 #ifdef FEATURE_PAL
     // Register handler of hardware exceptions like null reference in PAL
-    PAL_SetHardwareExceptionHandler(HandleHardwareException);
+    PAL_SetHardwareExceptionHandler(HandleHardwareException, IsSafeToHandleHardwareException);
 
     // Register handler for determining whether the specified IP has code that is a GC marker for GCCover
     PAL_SetGetGcMarkerExceptionCode(GetGcMarkerExceptionCode);
@@ -5031,65 +5032,67 @@ bool IsDivByZeroAnIntegerOverflow(PCONTEXT pContext)
 }
 #endif //_AMD64_
 
+BOOL PALAPI IsSafeToHandleHardwareException(PCONTEXT contextRecord, PEXCEPTION_RECORD exceptionRecord)
+{
+    PCODE controlPc = GetIP(contextRecord);
+    return g_fEEStarted && (
+        exceptionRecord->ExceptionCode == STATUS_BREAKPOINT || 
+        exceptionRecord->ExceptionCode == STATUS_SINGLE_STEP ||
+        ExecutionManager::IsManagedCode(controlPc) || 
+        IsIPInMarkedJitHelper(controlPc));
+}
 
 VOID PALAPI HandleHardwareException(PAL_SEHException* ex)
 {
-    if (!g_fEEStarted)
-    {
-        return;
-    }
+    _ASSERTE(IsSafeToHandleHardwareException(&ex->ContextRecord, &ex->ExceptionRecord));
 
     if (ex->ExceptionRecord.ExceptionCode != STATUS_BREAKPOINT && ex->ExceptionRecord.ExceptionCode != STATUS_SINGLE_STEP)
     {
         // A hardware exception is handled only if it happened in a jitted code or 
         // in one of the JIT helper functions (JIT_MemSet, ...)
         PCODE controlPc = GetIP(&ex->ContextRecord);
-        BOOL isInManagedCode = ExecutionManager::IsManagedCode(controlPc);
-        if (isInManagedCode || IsIPInMarkedJitHelper(controlPc))
+        if (ExecutionManager::IsManagedCode(controlPc) && IsGcMarker(ex->ExceptionRecord.ExceptionCode, &ex->ContextRecord))
         {
-            if (isInManagedCode && IsGcMarker(ex->ExceptionRecord.ExceptionCode, &ex->ContextRecord))
-            {
-                RtlRestoreContext(&ex->ContextRecord, &ex->ExceptionRecord);
-                UNREACHABLE();
-            }
+            RtlRestoreContext(&ex->ContextRecord, &ex->ExceptionRecord);
+            UNREACHABLE();
+        }
 
-            // Create frame necessary for the exception handling
-            FrameWithCookie<FaultingExceptionFrame> fef;
+        // Create frame necessary for the exception handling
+        FrameWithCookie<FaultingExceptionFrame> fef;
 #if defined(WIN64EXCEPTIONS)
-            *((&fef)->GetGSCookiePtr()) = GetProcessGSCookie();
+        *((&fef)->GetGSCookiePtr()) = GetProcessGSCookie();
 #endif // WIN64EXCEPTIONS
+        {
+            GCX_COOP();     // Must be cooperative to modify frame chain.
+            CONTEXT context = ex->ContextRecord;
+            if (IsIPInMarkedJitHelper(controlPc))
             {
-                GCX_COOP();     // Must be cooperative to modify frame chain.
-                CONTEXT context = ex->ContextRecord;
-                if (IsIPInMarkedJitHelper(controlPc))
-                {
-                    // For JIT helpers, we need to set the frame to point to the
-                    // managed code that called the helper, otherwise the stack
-                    // walker would skip all the managed frames upto the next
-                    // explicit frame.
-                    Thread::VirtualUnwindLeafCallFrame(&context);
-                }
-                fef.InitAndLink(&context);
+                // For JIT helpers, we need to set the frame to point to the
+                // managed code that called the helper, otherwise the stack
+                // walker would skip all the managed frames upto the next
+                // explicit frame.
+                Thread::VirtualUnwindLeafCallFrame(&context);
             }
+            fef.InitAndLink(&context);
+        }
 
 #ifdef _AMD64_
-            // It is possible that an overflow was mapped to a divide-by-zero exception. 
-            // This happens when we try to divide the maximum negative value of a
-            // signed integer with -1. 
-            //
-            // Thus, we will attempt to decode the instruction @ RIP to determine if that
-            // is the case using the faulting context.
-            if ((ex->ExceptionRecord.ExceptionCode == EXCEPTION_INT_DIVIDE_BY_ZERO) &&
-                IsDivByZeroAnIntegerOverflow(&ex->ContextRecord))
-            {
-                // The exception was an integer overflow, so augment the exception code.
-                ex->ExceptionRecord.ExceptionCode = EXCEPTION_INT_OVERFLOW;
-            }
+        // It is possible that an overflow was mapped to a divide-by-zero exception. 
+        // This happens when we try to divide the maximum negative value of a
+        // signed integer with -1. 
+        //
+        // Thus, we will attempt to decode the instruction @ RIP to determine if that
+        // is the case using the faulting context.
+        if ((ex->ExceptionRecord.ExceptionCode == EXCEPTION_INT_DIVIDE_BY_ZERO) &&
+            IsDivByZeroAnIntegerOverflow(&ex->ContextRecord))
+        {
+            // The exception was an integer overflow, so augment the exception code.
+            ex->ExceptionRecord.ExceptionCode = EXCEPTION_INT_OVERFLOW;
+        }
 #endif //_AMD64_
 
-            DispatchManagedException(*ex);
-            UNREACHABLE();
-        }
+        DispatchManagedException(*ex);
+        UNREACHABLE();
     }
     else
     {
@@ -5111,6 +5114,7 @@ VOID PALAPI HandleHardwareException(PAL_SEHException* ex)
                 pThread))
             {
                 RtlRestoreContext(&ex->ContextRecord, &ex->ExceptionRecord);
+                UNREACHABLE();
             }
         }
     }