Skip to content

[tsan] Fix nested signal handling #138599

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.

Sign up for GitHub

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 4 commits into
base: main
Choose a base branch
from
Open

[tsan] Fix nested signal handling #138599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9123520
[tsan] Format tsan_interceptors_posix.cpp
jwidauer May 5, 2025
3c283ef
[tsan] Keep a stack of old signal sets in the signal handler
jwidauer May 5, 2025
1b21726
[tsan] Use sigsuspend and check that the signals are still blocked in…
jwidauer May 5, 2025
6bedcfb
[tsan] Store and restore old signal set stack size when using setjump…
jwidauer May 11, 2025
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
28 changes: 18 additions & 10 deletions compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
// sanitizer_common/sanitizer_common_interceptors.inc
//===----------------------------------------------------------------------===//

#include <stdarg.h>

#include "interception/interception.h"
#include "sanitizer_common/sanitizer_allocator_dlsym.h"
#include "sanitizer_common/sanitizer_atomic.h"
#include "sanitizer_common/sanitizer_errno.h"
Expand All @@ -24,16 +27,14 @@
#include "sanitizer_common/sanitizer_posix.h"
#include "sanitizer_common/sanitizer_stacktrace.h"
#include "sanitizer_common/sanitizer_tls_get_addr.h"
#include "interception/interception.h"
#include "sanitizer_common/sanitizer_vector.h"
#include "tsan_fd.h"
#include "tsan_interceptors.h"
#include "tsan_interface.h"
#include "tsan_mman.h"
#include "tsan_platform.h"
#include "tsan_suppressions.h"
#include "tsan_rtl.h"
#include "tsan_mman.h"
#include "tsan_fd.h"

#include <stdarg.h>
#include "tsan_suppressions.h"

using namespace __tsan;

Expand Down Expand Up @@ -177,7 +178,7 @@ struct ThreadSignalContext {
SignalDesc pending_signals[kSigCount];
// emptyset and oldset are too big for stack.
__sanitizer_sigset_t emptyset;
__sanitizer_sigset_t oldset;
__sanitizer::Vector<__sanitizer_sigset_t> oldset;
};

void EnterBlockingFunc(ThreadState *thr) {
Expand Down Expand Up @@ -558,6 +559,7 @@ static void SetJmp(ThreadState *thr, uptr sp) {
buf->shadow_stack_pos = thr->shadow_stack_pos;
ThreadSignalContext *sctx = SigCtx(thr);
buf->int_signal_send = sctx ? sctx->int_signal_send : 0;
buf->oldset_stack_size = sctx ? sctx->oldset.Size() : 0;
buf->in_blocking_func = atomic_load(&thr->in_blocking_func, memory_order_relaxed);
buf->in_signal_handler = atomic_load(&thr->in_signal_handler,
memory_order_relaxed);
Expand All @@ -574,8 +576,11 @@ static void LongJmp(ThreadState *thr, uptr *env) {
while (thr->shadow_stack_pos > buf->shadow_stack_pos)
FuncExit(thr);
ThreadSignalContext *sctx = SigCtx(thr);
if (sctx)
if (sctx) {
sctx->int_signal_send = buf->int_signal_send;
while (sctx->oldset.Size() > buf->oldset_stack_size)
sctx->oldset.PopBack();
}
atomic_store(&thr->in_blocking_func, buf->in_blocking_func,
memory_order_relaxed);
atomic_store(&thr->in_signal_handler, buf->in_signal_handler,
Expand Down Expand Up @@ -980,6 +985,7 @@ void PlatformCleanUpThreadState(ThreadState *thr) {
&thr->signal_ctx, memory_order_relaxed);
if (sctx) {
atomic_store(&thr->signal_ctx, 0, memory_order_relaxed);
sctx->oldset.Reset();
UnmapOrDie(sctx, sizeof(*sctx));
}
}
Expand Down Expand Up @@ -2176,7 +2182,8 @@ void ProcessPendingSignalsImpl(ThreadState *thr) {
return;
atomic_fetch_add(&thr->in_signal_handler, 1, memory_order_relaxed);
internal_sigfillset(&sctx->emptyset);
int res = REAL(pthread_sigmask)(SIG_SETMASK, &sctx->emptyset, &sctx->oldset);
__sanitizer_sigset_t *oldset = sctx->oldset.PushBack();
int res = REAL(pthread_sigmask)(SIG_SETMASK, &sctx->emptyset, oldset);
CHECK_EQ(res, 0);
for (int sig = 0; sig < kSigCount; sig++) {
SignalDesc *signal = &sctx->pending_signals[sig];
Expand All @@ -2186,8 +2193,9 @@ void ProcessPendingSignalsImpl(ThreadState *thr) {
&signal->ctx);
}
}
res = REAL(pthread_sigmask)(SIG_SETMASK, &sctx->oldset, 0);
res = REAL(pthread_sigmask)(SIG_SETMASK, oldset, 0);
CHECK_EQ(res, 0);
sctx->oldset.PopBack();
atomic_fetch_add(&thr->in_signal_handler, -1, memory_order_relaxed);
}

Expand Down
1 change: 1 addition & 0 deletions compiler-rt/lib/tsan/rtl/tsan_rtl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ struct JmpBuf {
uptr sp;
int int_signal_send;
bool in_blocking_func;
uptr oldset_stack_size;
uptr in_signal_handler;
uptr *shadow_stack_pos;
};
Expand Down
81 changes: 62 additions & 19 deletions compiler-rt/test/tsan/signal_recursive.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
// Test case for recursive signal handlers, adopted from:
// https://github.com/google/sanitizers/issues/478

// REQUIRES: disabled

#include "test.h"
#include <semaphore.h>
#include <signal.h>
Expand All @@ -15,8 +13,6 @@ static const int kSigRestart = SIGUSR2;

static sem_t g_thread_suspend_ack_sem;

static bool g_busy_thread_received_restart;

static volatile bool g_busy_thread_garbage_collected;

static void SaveRegistersInStack() {
Expand All @@ -30,22 +26,51 @@ static void fail(const char *what) {
exit(1);
}

static void CheckSigBlocked(const sigset_t &oldset, const sigset_t &newset,
int sig) {
const int is_old_member = sigismember(&oldset, sig);
const int is_new_member = sigismember(&newset, sig);

if (is_old_member == -1 || is_new_member == -1)
fail("sigismember failed");

if (is_old_member != is_new_member)
fail("restoring signals failed");
}

sigset_t GetCurrentSigSet() {
sigset_t set;
if (sigemptyset(&set) != 0)
fail("sigemptyset failed");

if (pthread_sigmask(SIG_BLOCK, NULL, &set) != 0)
fail("pthread_sigmask failed");

return set;
}

static void SuspendHandler(int sig) {
int old_errno = errno;
SaveRegistersInStack();

// Enable kSigRestart handling, tsan disables signals around signal handlers.
sigset_t sigset;
sigemptyset(&sigset);
pthread_sigmask(SIG_SETMASK, &sigset, 0);
const auto oldset = GetCurrentSigSet();

// Acknowledge that thread is saved and suspended
if (sem_post(&g_thread_suspend_ack_sem) != 0)
fail("sem_post failed");

// Wait for wakeup signal.
while (!g_busy_thread_received_restart)
usleep(100); // wait for kSigRestart signal
sigset_t sigset;
sigemptyset(&sigset);
if (sigsuspend(&sigset) != 0 && errno != EINTR)
fail("sigsuspend failed");

const auto newset = GetCurrentSigSet();

// Check that the same signals are blocked as before
CheckSigBlocked(oldset, newset, kSigSuspend);
CheckSigBlocked(oldset, newset, kSigRestart);

// Acknowledge that thread restarted
if (sem_post(&g_thread_suspend_ack_sem) != 0)
Expand All @@ -56,31 +81,33 @@ static void SuspendHandler(int sig) {
errno = old_errno;
}

static void RestartHandler(int sig) {
g_busy_thread_received_restart = true;
static void RestartHandler(int sig) {}

static void WaitSem() {
while (sem_wait(&g_thread_suspend_ack_sem) != 0) {
if (errno != EINTR)
fail("sem_wait failed");
}
}

static void StopWorld(pthread_t thread) {
if (pthread_kill(thread, kSigSuspend) != 0)
fail("pthread_kill failed");

while (sem_wait(&g_thread_suspend_ack_sem) != 0) {
if (errno != EINTR)
fail("sem_wait failed");
}
WaitSem();
}

static void StartWorld(pthread_t thread) {
if (pthread_kill(thread, kSigRestart) != 0)
fail("pthread_kill failed");

while (sem_wait(&g_thread_suspend_ack_sem) != 0) {
if (errno != EINTR)
fail("sem_wait failed");
}
WaitSem();
}

static void CollectGarbage(pthread_t thread) {
// Wait for the thread to start
WaitSem();

StopWorld(thread);
// Walk stacks
StartWorld(thread);
Expand All @@ -102,21 +129,37 @@ static void Init() {

void* BusyThread(void *arg) {
(void)arg;
const auto oldset = GetCurrentSigSet();

if (sem_post(&g_thread_suspend_ack_sem) != 0)
fail("sem_post failed");

while (!g_busy_thread_garbage_collected) {
usleep(100); // Tsan deadlocks without these sleeps
}

const auto newset = GetCurrentSigSet();

// Check that we have the same signals blocked as before
CheckSigBlocked(oldset, newset, kSigSuspend);
CheckSigBlocked(oldset, newset, kSigRestart);

return NULL;
}

int main(int argc, const char *argv[]) {
Init();

pthread_t busy_thread;
if (pthread_create(&busy_thread, NULL, &BusyThread, NULL) != 0)
fail("pthread_create failed");

CollectGarbage(busy_thread);
if (pthread_join(busy_thread, 0) != 0)
fail("pthread_join failed");

fprintf(stderr, "DONE\n");

return 0;
}

Expand Down
Loading