[llvm] [Support] Improve the logic for re-raising signals (PR #177864)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-support

Author: Alex Rønne Petersen (alexrp)

<details>
<summary>Changes</summary>

On most systems, checking `si_pid` is not actually valid unless `si_code` equals one of the relevant signal codes (`SI_USER`, `SI_QUEUE`, and on some OSs, `SI_LWP`), or the signal is `SIGCHLD`. So on e.g. NetBSD, we would misinterpret the `SIGSEGV` fault address as the sending PID and incorrectly conclude that the signal came from a different process.

But as far as I can tell, there's not even a valid reason for us to be checking `si_pid != getpid()`, because the signal could very well have been explicitly sent by another thread in the current process. So we really just need to check `si_code` for the aforementioned signal codes.

Darwin is the exception because it just doesn't set `si_code` at all in the case of the `SI_*` signal codes. So keep the old logic there, even though it misses the corner case of signals sent by a thread in the current process.

While here, I also changed the error check for Linux `rt_tgsigqueueinfo()` as it was not at all clear to me how we would ever see `EPERM` from sending a signal to the current thread. Besides, it's sensible to go to the `raise()` path as a last resort regardless of how `rt_tgsigqueueinfo()` failed (e.g. `ENOSYS` on an old kernel).

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


1 Files Affected:

- (modified) llvm/lib/Support/Unix/Signals.inc (+33-12) 


``````````diff
diff --git a/llvm/lib/Support/Unix/Signals.inc b/llvm/lib/Support/Unix/Signals.inc
index 56ad4fc504153..9ec63f4e27cde 100644
--- a/llvm/lib/Support/Unix/Signals.inc
+++ b/llvm/lib/Support/Unix/Signals.inc
@@ -421,20 +421,41 @@ static void SignalHandler(int Sig, siginfo_t *Info, void *) {
 #endif
 
 #if defined(__linux__)
-  // Re-raising a signal via `raise` loses the original siginfo. Recent
-  // versions of linux (>= 3.9) support processes sending a signal to itself
-  // with arbitrary signal information using a syscall. If this syscall is
-  // unsupported, errno will be set to EPERM and `raise` will be used instead.
-  int retval =
+  // Re-raising a signal via `raise` loses the original siginfo. Recent versions
+  // of Linux (>= 3.9) support a process sending a signal to itself with
+  // arbitrary signal information using a syscall. If this fails, we fall back
+  // to the `raise` path.
+  int Result =
       syscall(SYS_rt_tgsigqueueinfo, getpid(), syscall(SYS_gettid), Sig, Info);
-  if (retval != 0 && errno == EPERM)
-    raise(Sig);
-#else
-  // Signal sent from another userspace process, do not assume that continuing
-  // the execution would re-raise it.
-  if (Info->si_pid != getpid() && Info->si_pid != 0)
-    raise(Sig);
+  if (Result == 0)
+    return;
+#endif
+
+  // Was the signal generated by kill(), sigqueue(), etc?
+  bool ReraiseSignal = Info->si_code == SI_USER || Info->si_code == SI_QUEUE;
+#if defined(SI_LWP)
+  // _lwp_kill() on BSDs, Solaris/illumos, possibly others.
+  ReraiseSignal |= Info->si_code == SI_LWP;
+#endif
+
+#if defined(__APPLE__)
+  // The Darwin kernel elects not to fill out si_code with the SI_* signal
+  // codes...but at least we know that checking si_pid is valid regardless of
+  // si_code on this platform, so this is a decent proxy for answering the above
+  // question. It does unfortunately mean that we don't include signals sent via
+  // those APIs by other threads in the current process.
+  //
+  // si_pid == 0 will be the case for kernel-generated signals (i.e. like
+  // SI_KERNEL on Linux).
+  ReraiseSignal = Info->si_pid != 0 && Info->si_pid != getpid();
 #endif
+
+  // If the signal was explicitly sent, we cannot expect it to trigger again
+  // when we return from the signal handler, so we must re-raise it. The common
+  // case for this will be a signal sent by another process, but it's also
+  // possible that a thread in the current process could have sent the signal.
+  if (ReraiseSignal)
+    raise(Sig);
 }
 
 static void InfoSignalHandler(int Sig) {

``````````

</details>


https://github.com/llvm/llvm-project/pull/177864