[Support] Re-raise external signals

[guy-david]

Otherwise, the handler "swallows" the signal and the process continues to execute. While this use case is peculiar, ignoring these signals entirely seems more odd.

[llvmbot]

@llvm/pr-subscribers-llvm-support

Author: Guy David (guy-david)

Changes

Otherwise, the handler "swallows" the signal and the process continues to execute. While this use case is peculiar, ignoring these signals entirely seems more odd.

---

Full diff: https://github.com/llvm/llvm-project/pull/125854.diff

1 Files Affected:

• (modified) llvm/lib/Support/Unix/Signals.inc (+11-6)

diff --git a/llvm/lib/Support/Unix/Signals.inc b/llvm/lib/Support/Unix/Signals.inc
index 2e7b467a14bbe2..388fff7d5c9dcf 100644
--- a/llvm/lib/Support/Unix/Signals.inc
+++ b/llvm/lib/Support/Unix/Signals.inc
@@ -80,7 +80,8 @@
 
 using namespace llvm;
 
-static void SignalHandler(int Sig);     // defined below.
+static void SignalHandler(int Sig, siginfo_t *Info,
+                          void *Context); // defined below.
 static void InfoSignalHandler(int Sig); // defined below.
 
 using SignalHandlerFunctionType = void (*)();
@@ -313,8 +314,8 @@ static void RegisterHandlers() { // Not signal-safe.
 
     switch (Kind) {
     case SignalKind::IsKill:
-      NewHandler.sa_handler = SignalHandler;
-      NewHandler.sa_flags = SA_NODEFER | SA_RESETHAND | SA_ONSTACK;
+      NewHandler.sa_sigaction = SignalHandler;
+      NewHandler.sa_flags = SA_NODEFER | SA_RESETHAND | SA_ONSTACK | SA_SIGINFO;
       break;
     case SignalKind::IsInfo:
       NewHandler.sa_handler = InfoSignalHandler;
@@ -370,7 +371,7 @@ void sys::CleanupOnSignal(uintptr_t Context) {
 }
 
 // The signal handler that runs.
-static void SignalHandler(int Sig) {
+static void SignalHandler(int Sig, siginfo_t *Info, void *Context) {
   // Restore the signal behavior to default, so that the program actually
   // crashes when we return and the signal reissues.  This also ensures that if
   // we crash in our signal handler that the program will terminate immediately
@@ -401,6 +402,11 @@ static void SignalHandler(int Sig) {
     }
   }
 
+  // Signal sent from another process, do not assume that continuing the
+  // execution would re-raise it.
+  if (Info->si_pid != getpid())
+    raise(Sig);
+
   // Otherwise if it is a fault (like SEGV) run any handler.
   llvm::sys::RunSignalHandlers();
 
@@ -468,8 +474,7 @@ void llvm::sys::AddSignalHandler(sys::SignalHandlerCallback FnPtr,
 
 #if ENABLE_BACKTRACES && defined(HAVE_BACKTRACE) &&                            \
     (defined(__linux__) || defined(__FreeBSD__) ||                             \
-     defined(__FreeBSD_kernel__) || defined(__NetBSD__) ||                     \
-     defined(__OpenBSD__) || defined(__DragonFly__))
+     defined(__FreeBSD_kernel__) || defined(__NetBSD__))
 struct DlIteratePhdrData {
   void **StackTrace;
   int depth;

[aganea]

I am maybe not the best one to review this. Perhaps @dwblaikie or @MaskRay can help finding another reviewer?

[dwblaikie]

Yeah, not sure, sorry - bit outside my wheelhouse. I wonder if @d0k knows about this or knows someone who does...

[MaskRay]

Could you state your motivation behind this change?

Makes sense. I don't know why SIGQUIT in on the Kill list instead of Int list. Nevertheless, Kill/Int are handled similarly. If an external process raises SIGSEGV/SIGQUIT, the default action is termination. With the handler installed, the process should also terminate (by reraising the signal) after removing temporary files and invoking some handlers (for example, LLVM may dump Stack dump:)

It's strange that this

if (Info->si_pid != getpid())
    raise(Sig);

pattern isn't that common.

(@cracauer for https://www.cons.org/cracauer/sigint.html :) This post probably could mention signals other than SIGINT/SIGQUIT )

(@emaste)

[guy-david]

Thanks!
I was looking at the symbolizer to understand why some symbols are not printed nicely during a crash. I ran llc and signaled a SIGSEGV using kill, which to my surprise didn't make it stop.