Skip to content
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions compiler-rt/lib/sanitizer_common/sanitizer_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ namespace __sanitizer {

const char *SanitizerToolName = "SanitizerTool";

const int MaxSignals = 64;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

constexpr
and static

Also you don't realy need MaxSignals
ARRAY_SIZE() is more trustworthy for users

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced MaxSignals with ARRAY_SIZE()

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 64?
it needs to be max signal which sanitizers install itself

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

max(
MaybeInstallSigaction(SIGSEGV, handler);
  MaybeInstallSigaction(SIGBUS, handler);
  MaybeInstallSigaction(SIGABRT, handler);
  MaybeInstallSigaction(SIGFPE, handler);
  MaybeInstallSigaction(SIGILL, handler);
  MaybeInstallSigaction(SIGTRAP, handler);
) + 1

Copy link
Contributor Author

@thurstond thurstond Oct 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The number of signals is technically dynamic, but 64 is commonly the max (https://www.chromium.org/chromium-os/developer-library/reference/linux-constants/signals/).

The constants SIGFPE, SIGBUS, SIGTRAP, SIGILL and SIGSEGV vary between platforms (https://venam.net/blog/unix/2016/10/21/unix-signals.html).

bool signal_handler_is_from_sanitizer[MaxSignals] = {0};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any locking, so should be atomic

SetSignalHandlerFromSanitizer/IsSignalHandlerFromSanitizer
should be just atomic SetSignalHandlerFromSanitizer which returns previos value

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add atomicity.

I don't think it's feasible to combine SetSignalHandlerFromSanitizer/IsSignalHandlerFromSanitizer, because of this code in the sigaction interceptor:

  if (IsSignalHandlerFromSanitizer(signum) && ret == 0) {
    if (act)
      // If the user sets a signal handler, it is never cloaked, even if they
      // reuse a sanitizer's signal handler.
      SetSignalHandlerFromSanitizer(signum, false);

    ...
  }

Copy link
Collaborator

@vitalybuka vitalybuka Oct 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you still need "Is" but you can set atomically

if (act) {
  if (ret == 0 && SetSignalHandlerFromSanitizer(signum, false)) {
    ...
  }
} else if (ret == 0 && IsSignalHandlerFromSanitizer) {
   ...
}

Note: check local ret before more expensive SetSignalHandlerFromSanitizer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


atomic_uint32_t current_verbosity;
uptr PageSizeCached;
u32 NumberOfCPUsCached;
Expand Down Expand Up @@ -111,6 +114,18 @@ const char *StripModuleName(const char *module) {
return module;
}

void SetSignalHandlerFromSanitizer(int signum, bool new_state) {
if (signum >= 0 && signum < MaxSignals)
signal_handler_is_from_sanitizer[signum] = new_state;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should n't this be in the same file as MaybeInstallSigaction?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is used in the interceptors, which are cross-platform. If I moved SetSignalHandlerFromSanitizer into sanitizer_posix_libcdep.cpp (the same file as MaybeInstallSigaction), I would need to add a lot of #if SANITIZER_POSIX into the interceptors.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signals are sanitizer_posix_libcdep specific
declare empty for other platforms

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#define SANITIZER_INTERCEPT_SIGNAL_AND_SIGACTION (!SI_WINDOWS && SI_NOT_FUCHSIA)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved

}

bool IsSignalHandlerFromSanitizer(int signum) {
if (signum >= 0 && signum < MaxSignals)
return signal_handler_is_from_sanitizer[signum];

return false;
}

void ReportErrorSummary(const char *error_message, const char *alt_tool_name) {
if (!common_flags()->print_summary)
return;
Expand Down
3 changes: 3 additions & 0 deletions compiler-rt/lib/sanitizer_common/sanitizer_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,9 @@ void ReportDeadlySignal(const SignalContext &sig, u32 tid,
void SetAlternateSignalStack();
void UnsetAlternateSignalStack();

void SetSignalHandlerFromSanitizer(int signum, bool new_state);
bool IsSignalHandlerFromSanitizer(int signum);

// Construct a one-line string:
// SUMMARY: SanitizerToolName: error_message
// and pass it to __sanitizer_report_error_summary.
Expand Down
5 changes: 5 additions & 0 deletions compiler-rt/lib/sanitizer_common/sanitizer_flags.inc
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ COMMON_FLAG(HandleSignalMode, handle_sigfpe, kHandleSignalYes,
COMMON_FLAG(bool, allow_user_segv_handler, true,
"Deprecated. True has no effect, use handle_sigbus=1. If false, "
"handle_*=1 will be upgraded to handle_*=2.")
COMMON_FLAG(bool, cloak_sanitizer_signal_handlers, false,
"If set, signal/sigaction will pretend that sanitizers did not "
"preinstall any signal handlers. If the user subsequently installs "
"a signal handler, this will disable cloaking for the respective "
"signal.")
COMMON_FLAG(bool, use_sigaltstack, true,
"If set, uses alternate stack for signal handling.")
COMMON_FLAG(bool, detect_deadlocks, true,
Expand Down
3 changes: 3 additions & 0 deletions compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,9 @@ static void MaybeInstallSigaction(int signum,
if (common_flags()->use_sigaltstack) sigact.sa_flags |= SA_ONSTACK;
CHECK_EQ(0, internal_sigaction(signum, &sigact, nullptr));
VReport(1, "Installed the sigaction for signal %d\n", signum);

if (common_flags()->cloak_sanitizer_signal_handlers)
SetSignalHandlerFromSanitizer(signum, true);
}

void InstallDeadlySignalHandlers(SignalHandlerType handler) {
Expand Down
46 changes: 42 additions & 4 deletions compiler-rt/lib/sanitizer_common/sanitizer_signal_interceptors.inc
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ using namespace __sanitizer;
#endif

#ifndef SIGNAL_INTERCEPTOR_SIGNAL_IMPL
#define SIGNAL_INTERCEPTOR_SIGNAL_IMPL(func, signum, handler) \
{ return REAL(func)(signum, handler); }
# define SIGNAL_INTERCEPTOR_SIGNAL_IMPL(func, signum, handler) \
{ \
ret = REAL(func)(signum, handler); \
}
#endif

#ifndef SIGNAL_INTERCEPTOR_SIGACTION_IMPL
Expand All @@ -35,17 +37,22 @@ using namespace __sanitizer;
Printf( \
"Warning: REAL(sigaction_symname) == nullptr. This may happen " \
"if you link with ubsan statically. Sigaction will not work.\n"); \
return -1; \
ret = -1; \
} else { \
ret = REAL(sigaction_symname)(signum, act, oldact); \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated NFC?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is necessary: the interceptor calls this macro and then we want to do more stuff (e.g., change the return value); if the macro returns immediately, we can't do the followup steps.

I can split it into a separate NFC though?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can split it into a separate NFC though?

That's what I meant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored to use lambdas per your suggestion in #162916 (comment)

A nice side effect is MSan/TSan is now supported as well.

} \
return REAL(sigaction_symname)(signum, act, oldact); \
}
#endif

#if SANITIZER_INTERCEPT_BSD_SIGNAL
INTERCEPTOR(uptr, bsd_signal, int signum, uptr handler) {
SIGNAL_INTERCEPTOR_ENTER();
if (GetHandleSignalMode(signum) == kHandleSignalExclusive) return 0;

// TODO: support cloak_sanitizer_signal_handlers
int ret;
SIGNAL_INTERCEPTOR_SIGNAL_IMPL(bsd_signal, signum, handler);
return ret;
}
#define INIT_BSD_SIGNAL COMMON_INTERCEPT_FUNCTION(bsd_signal)
#else // SANITIZER_INTERCEPT_BSD_SIGNAL
Expand All @@ -56,19 +63,50 @@ INTERCEPTOR(uptr, bsd_signal, int signum, uptr handler) {
INTERCEPTOR(uptr, signal, int signum, uptr handler) {
SIGNAL_INTERCEPTOR_ENTER();
if (GetHandleSignalMode(signum) == kHandleSignalExclusive)
// The user can neither view nor change the signal handler, regardless of
// the cloak_sanitizer_signal_handlers setting. This differs from
// sigaction().
return (uptr) nullptr;
uptr ret;
SIGNAL_INTERCEPTOR_SIGNAL_IMPL(signal, signum, handler);

if (IsSignalHandlerFromSanitizer(signum) && ret != sig_err) {
// If the user sets a signal handler, it is never cloaked, even if they
// reuse a sanitizer's signal handler.
SetSignalHandlerFromSanitizer(signum, false);
ret = sig_dfl;
}

return ret;
}
#define INIT_SIGNAL COMMON_INTERCEPT_FUNCTION(signal)

INTERCEPTOR(int, sigaction_symname, int signum,
const __sanitizer_sigaction *act, __sanitizer_sigaction *oldact) {
SIGNAL_INTERCEPTOR_ENTER();

if (GetHandleSignalMode(signum) == kHandleSignalExclusive) {
if (!oldact) return 0;
act = nullptr;
// If cloak_sanitizer_signal_handlers=true, the user can neither view nor
// change the signal handle.
// If false, the user can view but not change the signal handler. This
// differs from signal().
}
int ret;
SIGNAL_INTERCEPTOR_SIGACTION_IMPL(signum, act, oldact);

if (IsSignalHandlerFromSanitizer(signum) && ret == 0) {
if (act)
Copy link
Collaborator

@vitalybuka vitalybuka Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you make it a function somewher with signal_handler_is_from_sanitizer
and hide MaxSignals and signal_handler_is_from_sanitizer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: 6546834

// If the user sets a signal handler, it is never cloaked, even if they
// reuse a sanitizer's signal handler.
SetSignalHandlerFromSanitizer(signum, false);

if (oldact)
oldact->handler = reinterpret_cast<__sanitizer_sighandler_ptr>(sig_dfl);
}

return ret;
}
#define INIT_SIGACTION COMMON_INTERCEPT_FUNCTION(sigaction_symname)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
// Flaky errors in debuggerd with "waitpid returned unexpected pid (0)" in logcat.
// UNSUPPORTED: android && i386-target-arch

// Note: this test case is unusual because it retrieves the original
// (ASan-installed) signal handler; thus, it is incompatible with the
// cloak_sanitizer_signal_handlers runtime option.

#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// XFAIL: msan
// XFAIL: tsan

// UNSUPPORTED: android
// UNSUPPORTED: hwasan

// RUN: %clangxx -O0 %s -o %t

// Sanitizer signal handler not installed; custom signal handler installed
// RUN: %env_tool_opts=handle_segv=0:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM
// RUN: %env_tool_opts=handle_segv=0:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM

// Sanitizer signal handler installed but overriden by custom signal handler
// RUN: %env_tool_opts=handle_segv=1:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=NONDEFAULT,CUSTOM
// RUN: %env_tool_opts=handle_segv=1:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM

// Sanitizer signal handler installed immutably
// N.B. for handle_segv=2 with cloaking off, there is a pre-existing difference
// in signal vs. sigaction: signal effectively cloaks the handler.
// RUN: %env_tool_opts=handle_segv=2:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=NONDEFAULT,SANITIZER
// RUN: %env_tool_opts=handle_segv=2:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,SANITIZER

#include <signal.h>
#include <stdio.h>
#include <stdlib.h>

void handler(int signum, siginfo_t *info, void *context) {
printf("Custom signal handler\n");
exit(1);
}

int main(int argc, char *argv[]) {
struct sigaction sa = {0};
struct sigaction old = {0};
sa.sa_flags = SA_SIGINFO;
sa.sa_sigaction = &handler;
sigaction(SIGSEGV, &sa, &old);

if (reinterpret_cast<void *>(old.sa_sigaction) == SIG_DFL)
printf("Old handler: default\n");
// DEFAULT: Old handler: default
else
printf("Old handler: non-default\n");
// NONDEFAULT: Old handler: non-default

fflush(stdout);

char *c = (char *)0x123;
printf("%d\n", *c);
// CUSTOM: Custom signal handler
// SANITIZER: Sanitizer:DEADLYSIGNAL

return 0;
}
49 changes: 49 additions & 0 deletions compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_signal.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// XFAIL: msan
// XFAIL: tsan

// UNSUPPORTED: android
// UNSUPPORTED: hwasan

// RUN: %clangxx -O0 %s -o %t

// Sanitizer signal handler not installed; custom signal handler installed
// RUN: %env_tool_opts=handle_segv=0:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM
// RUN: %env_tool_opts=handle_segv=0:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM

// Sanitizer signal handler installed but overriden by custom signal handler
// RUN: %env_tool_opts=handle_segv=1:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=NONDEFAULT,CUSTOM
// RUN: %env_tool_opts=handle_segv=1:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM

// Sanitizer signal handler installed immutably
// N.B. for handle_segv=2 with cloaking off, there is a pre-existing difference
// in signal vs. sigaction: signal effectively cloaks the handler.
// RUN: %env_tool_opts=handle_segv=2:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,SANITIZER
// RUN: %env_tool_opts=handle_segv=2:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,SANITIZER

#include <signal.h>
#include <stdio.h>
#include <stdlib.h>

void my_signal_sighandler(int signum) {
printf("Custom signal handler\n");
exit(1);
}

int main(int argc, char *argv[]) {
__sighandler_t old = signal(SIGSEGV, &my_signal_sighandler);
if (old == SIG_DFL)
printf("Old handler: default\n");
// DEFAULT: Old handler: default
else
printf("Old handler: non-default\n");
// NONDEFAULT: Old handler: non-default

fflush(stdout);

char *c = (char *)0x123;
printf("%d\n", *c);
// CUSTOM: Custom signal handler
// SANITIZER: Sanitizer:DEADLYSIGNAL

return 0;
}