Skip to content

[Support] Don't re-raise signals sent from kernel #145759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bulbazord
Copy link
Member

When an llvm tool crashes (e.g. from a segmentation fault), SignalHandler will re-raise the signal. The effect is that crash reports now contain SignalHandler in the stack trace. The crash reports are still useful, but the presence of SignalHandler can confuse tooling and automation that deduplicate or analyze crash reports.

rdar://150464802

When an llvm tool crashes (e.g. from a segmentation fault),
SignalHandler will re-raise the signal. The effect is that crash reports
now contain SignalHandler in the stack trace. The crash reports are
still useful, but the presence of SignalHandler can confuse tooling and
automation that deduplicate or analyze crash reports.

rdar://150464802
@llvmbot
Copy link
Member

llvmbot commented Jun 25, 2025

@llvm/pr-subscribers-llvm-support

Author: Alex Langford (bulbazord)

Changes

When an llvm tool crashes (e.g. from a segmentation fault), SignalHandler will re-raise the signal. The effect is that crash reports now contain SignalHandler in the stack trace. The crash reports are still useful, but the presence of SignalHandler can confuse tooling and automation that deduplicate or analyze crash reports.

rdar://150464802


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

1 Files Affected:

  • (modified) llvm/lib/Support/Unix/Signals.inc (+3-3)
diff --git a/llvm/lib/Support/Unix/Signals.inc b/llvm/lib/Support/Unix/Signals.inc
index 6668a2953b3b2..cb38fcc559a7e 100644
--- a/llvm/lib/Support/Unix/Signals.inc
+++ b/llvm/lib/Support/Unix/Signals.inc
@@ -413,9 +413,9 @@ static void SignalHandler(int Sig, siginfo_t *Info, void *) {
     raise(Sig);
 #endif
 
-  // Signal sent from another process, do not assume that continuing the
-  // execution would re-raise it.
-  if (Info->si_pid != getpid())
+  // 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);
 }
 

@MaskRay
Copy link
Member

MaskRay commented Jun 26, 2025

I assume that si_pid==0 is macOS kernel's behavior. Can you add some description whether it is also Linux's?

@bulbazord
Copy link
Member Author

I assume that si_pid==0 is macOS kernel's behavior. Can you add some description whether it is also Linux's?

AFAICT this is the behavior for both macOS and Linux, not sure about other unix and unix-like OSes. Would you like me to add a comment here?

For completeness, I also considered looking at the si_code field because on Linux it has a SI_KERNEL value if it's sent from the kernel, but macOS doesn't seem to have an equivalent.

@pcc
Copy link
Contributor

pcc commented Jun 26, 2025

Not all crash signals sent by the kernel are synchronous (will automatically reraise). MTE asynchronous tag check faults are an example of a signal that will not reraise. Chromium would previously attempt to reraise these signals by returning from the signal handler as this PR proposes to do, which led to asynchronous tag check faults being ignored.

I fixed this bug in Chromium by using a different API to reraise the signal that preserves siginfo: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3300991

We should probably use this API at least on Linux.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants