Skip to content
Open
Show file tree
Hide file tree
Changes from 8 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
2 changes: 2 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,8 @@ namespace __sanitizer {

const char *SanitizerToolName = "SanitizerTool";

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
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 @@ -52,6 +52,9 @@ const u64 kExternalPCBit = 1ULL << 60;

extern const char *SanitizerToolName; // Can be changed by the tool.

const int MaxSignals = 64;
extern bool signal_handler_is_from_sanitizer[MaxSignals];

extern atomic_uint32_t current_verbosity;
inline void SetVerbosity(int verbosity) {
atomic_store(&current_verbosity, verbosity, memory_order_relaxed);
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)
signal_handler_is_from_sanitizer[signum] = true;
}

void InstallDeadlySignalHandlers(SignalHandlerType handler) {
Expand Down
49 changes: 45 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,53 @@ 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 (signum >= 0 && signum < MaxSignals &&
signal_handler_is_from_sanitizer[signum] && ret != sig_err) {
// If the user sets a signal handler, it is never cloaked, even if they
// reuse a sanitizer's signal handler.
signal_handler_is_from_sanitizer[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 (signum >= 0 && signum < MaxSignals &&
signal_handler_is_from_sanitizer[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.
signal_handler_is_from_sanitizer[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;
}