Support sigaction with SIG_DFL and SA_SIGINFO. (#55673)

Fixes #55645
dotnet · Jul 15, 2021 · 30257e6 · 30257e6
1 parent 44655fd
commit 30257e6
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 41 deletions.
diff --git a/src/coreclr/pal/src/exception/signal.cpp b/src/coreclr/pal/src/exception/signal.cpp
@@ -344,6 +344,26 @@ bool IsRunningOnAlternateStack(void *context)
 #endif // HAVE_MACH_EXCEPTIONS
 }
 
+static bool IsSaSigInfo(struct sigaction* action)
+{
+    return (action->sa_flags & SA_SIGINFO) != 0;
+}
+
+static bool IsSigDfl(struct sigaction* action)
+{
+    // macOS can return sigaction with SIG_DFL and SA_SIGINFO.
+    // SA_SIGINFO means we should use sa_sigaction, but here we want to check sa_handler.
+    // So we ignore SA_SIGINFO when sa_sigaction and sa_handler are at the same address.
+    return (&action->sa_handler == (void*)&action->sa_sigaction || !IsSaSigInfo(action)) &&
+            action->sa_handler == SIG_DFL;
+}
+
+static bool IsSigIgn(struct sigaction* action)
+{
+    return (&action->sa_handler == (void*)&action->sa_sigaction || !IsSaSigInfo(action)) &&
+            action->sa_handler == SIG_IGN;
+}
+
 /*++
 Function :
     invoke_previous_action
@@ -363,44 +383,41 @@ static void invoke_previous_action(struct sigaction* action, int code, siginfo_t
 {
     _ASSERTE(action != NULL);
 
-    if (action->sa_flags & SA_SIGINFO)
+    if (IsSigIgn(action))
     {
-        // Directly call the previous handler.
-        _ASSERTE(action->sa_sigaction != NULL);
-        action->sa_sigaction(code, siginfo, context);
-    }
-    else
-    {
-        if (action->sa_handler == SIG_IGN)
+        if (signalRestarts)
         {
-            if (signalRestarts)
-            {
-                // This signal mustn't be ignored because it will be restarted.
-                PROCAbort(code);
-            }
-            return;
+            // This signal mustn't be ignored because it will be restarted.
+            PROCAbort(code);
         }
-        else if (action->sa_handler == SIG_DFL)
+        return;
+    }
+    else if (IsSigDfl(action))
+    {
+        if (signalRestarts)
         {
-            if (signalRestarts)
-            {
-                // Restore the original and restart h/w exception.
-                restore_signal(code, action);
-            }
-            else
-            {
-                // We can't invoke the original handler because returning from the
-                // handler doesn't restart the exception.
-                PROCAbort(code);
-            }
+            // Restore the original and restart h/w exception.
+            restore_signal(code, action);
         }
         else
         {
-            // Directly call the previous handler.
-            _ASSERTE(action->sa_handler != NULL);
-            action->sa_handler(code);
+            // We can't invoke the original handler because returning from the
+            // handler doesn't restart the exception.
+            PROCAbort(code);
         }
     }
+    else if (IsSaSigInfo(action))
+    {
+        // Directly call the previous handler.
+        _ASSERTE(action->sa_sigaction != NULL);
+        action->sa_sigaction(code, siginfo, context);
+    }
+    else
+    {
+        // Directly call the previous handler.
+        _ASSERTE(action->sa_handler != NULL);
+        action->sa_handler(code);
+    }
 
     PROCNotifyProcessShutdown(IsRunningOnAlternateStack(context));
 

diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c
@@ -63,16 +63,21 @@ static bool IsSaSigInfo(struct sigaction* action)
 #pragma clang diagnostic pop
 }
 
-static bool IsSigIgn(struct sigaction* action)
+static bool IsSigDfl(struct sigaction* action)
 {
     assert(action);
-    return !IsSaSigInfo(action) && action->sa_handler == SIG_IGN;
+    // macOS can return sigaction with SIG_DFL and SA_SIGINFO.
+    // SA_SIGINFO means we should use sa_sigaction, but here we want to check sa_handler.
+    // So we ignore SA_SIGINFO when sa_sigaction and sa_handler are at the same address.
+    return (&action->sa_handler == (void*)&action->sa_sigaction || !IsSaSigInfo(action)) &&
+            action->sa_handler == SIG_DFL;
 }
 
-static bool IsSigDfl(struct sigaction* action)
+static bool IsSigIgn(struct sigaction* action)
 {
     assert(action);
-    return !IsSaSigInfo(action) && action->sa_handler == SIG_DFL;
+    return (&action->sa_handler == (void*)&action->sa_sigaction || !IsSaSigInfo(action)) &&
+            action->sa_handler == SIG_IGN;
 }
 
 static bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posixSignal)
@@ -209,15 +214,19 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context)
     if (!IsCancelableTerminationSignal(sig))
     {
         struct sigaction* origHandler = OrigActionFor(sig);
-        if (IsSaSigInfo(origHandler))
-        {
-            assert(origHandler->sa_sigaction);
-            origHandler->sa_sigaction(sig, siginfo, context);
-        }
-        else if (origHandler->sa_handler != SIG_IGN &&
-                 origHandler->sa_handler != SIG_DFL)
+        if (!IsSigDfl(origHandler) && !IsSigIgn(origHandler))
         {
-            origHandler->sa_handler(sig);
+            if (IsSaSigInfo(origHandler))
+            {
+                assert(origHandler->sa_sigaction);
+                origHandler->sa_sigaction(sig, siginfo, context);
+            }
+            else
+            {
+                assert(origHandler->sa_handler);
+                origHandler->sa_handler(sig);
+            }
+
         }
     }