From 5a7c3643437c262137bd3dac7f6a0f5b9e8501be Mon Sep 17 00:00:00 2001 From: Evgeniy Stepanov Date: Wed, 23 Apr 2014 14:01:57 +0000 Subject: [PATCH] [msan] Disable chained origins in signal handlers. StackDepot is not async-signal-safe; storing a new origin to it can deadlock. llvm-svn: 206983 --- compiler-rt/lib/msan/msan.cc | 2 ++ compiler-rt/lib/msan/msan_interceptors.cc | 8 +++++ compiler-rt/lib/msan/msan_thread.h | 6 ++++ .../test/msan/chained_origin_with_signals.cc | 32 +++++++++++++++++++ 4 files changed, 48 insertions(+) create mode 100644 compiler-rt/test/msan/chained_origin_with_signals.cc diff --git a/compiler-rt/lib/msan/msan.cc b/compiler-rt/lib/msan/msan.cc index db92a5f3cf8f..8bf7c48acc7a 100644 --- a/compiler-rt/lib/msan/msan.cc +++ b/compiler-rt/lib/msan/msan.cc @@ -237,6 +237,8 @@ const char *GetOriginDescrIfStack(u32 id, uptr *pc) { } u32 ChainOrigin(u32 id, StackTrace *stack) { + if (GetCurrentThread()->InSignalHandler()) + return id; uptr idx = Min(stack->size, kStackTraceMax - 1); stack->trace[idx] = TRACE_MAKE_CHAINED(id); u32 new_id = StackDepotPut(stack->trace, idx + 1); diff --git a/compiler-rt/lib/msan/msan_interceptors.cc b/compiler-rt/lib/msan/msan_interceptors.cc index 5205094fb979..2062f7f4b06f 100644 --- a/compiler-rt/lib/msan/msan_interceptors.cc +++ b/compiler-rt/lib/msan/msan_interceptors.cc @@ -981,6 +981,12 @@ INTERCEPTOR(int, getrusage, int who, void *usage) { return res; } +class SignalHandlerScope { + public: + SignalHandlerScope() { GetCurrentThread()->EnterSignalHandler(); } + ~SignalHandlerScope() { GetCurrentThread()->LeaveSignalHandler(); } +}; + // sigactions_mu guarantees atomicity of sigaction() and signal() calls. // Access to sigactions[] is gone with relaxed atomics to avoid data race with // the signal handler. @@ -989,6 +995,7 @@ static atomic_uintptr_t sigactions[kMaxSignals]; static StaticSpinMutex sigactions_mu; static void SignalHandler(int signo) { + SignalHandlerScope signal_handler_scope; ScopedThreadLocalStateBackup stlsb; UnpoisonParam(1); @@ -999,6 +1006,7 @@ static void SignalHandler(int signo) { } static void SignalAction(int signo, void *si, void *uc) { + SignalHandlerScope signal_handler_scope; ScopedThreadLocalStateBackup stlsb; UnpoisonParam(3); __msan_unpoison(si, sizeof(__sanitizer_sigaction)); diff --git a/compiler-rt/lib/msan/msan_thread.h b/compiler-rt/lib/msan/msan_thread.h index 82ed96c0a660..bc605b89a505 100644 --- a/compiler-rt/lib/msan/msan_thread.h +++ b/compiler-rt/lib/msan/msan_thread.h @@ -38,6 +38,10 @@ class MsanThread { return addr >= stack_bottom_ && addr < stack_top_; } + bool InSignalHandler() { return in_signal_handler_; } + void EnterSignalHandler() { in_signal_handler_++; } + void LeaveSignalHandler() { in_signal_handler_--; } + MsanThreadLocalMallocStorage &malloc_storage() { return malloc_storage_; } int destructor_iterations_; @@ -54,6 +58,8 @@ class MsanThread { uptr tls_begin_; uptr tls_end_; + unsigned in_signal_handler_; + MsanThreadLocalMallocStorage malloc_storage_; }; diff --git a/compiler-rt/test/msan/chained_origin_with_signals.cc b/compiler-rt/test/msan/chained_origin_with_signals.cc new file mode 100644 index 000000000000..7141d41e028b --- /dev/null +++ b/compiler-rt/test/msan/chained_origin_with_signals.cc @@ -0,0 +1,32 @@ +// Check that stores in signal handlers are not recorded in origin history. +// This is, in fact, undesired behavior caused by our chained origins +// implementation being not async-signal-safe. + +// RUN: %clangxx_msan -fsanitize-memory-track-origins=2 -m64 -O3 %s -o %t && \ +// RUN: not %t >%t.out 2>&1 +// RUN: FileCheck %s < %t.out + +#include +#include +#include +#include + +volatile int x, y; + +void SignalHandler(int signo) { + y = x; +} + +int main(int argc, char *argv[]) { + int volatile z; + x = z; + + signal(SIGUSR1, SignalHandler); + kill(getpid(), SIGUSR1); + signal(SIGUSR1, SIG_DFL); + + return y; +} + +// CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value +// CHECK-NOT: in SignalHandler