[compiler-rt] r206983 - [msan] Disable chained origins in signal handlers.

[Evgeniy Stepanov]

Author: eugenis
Date: Wed Apr 23 09:01:57 2014
New Revision: 206983

URL: http://llvm.org/viewvc/llvm-project?rev=206983&view=rev
Log:
[msan] Disable chained origins in signal handlers.

StackDepot is not async-signal-safe; storing a new origin to it can deadlock.


Added:
    compiler-rt/trunk/test/msan/chained_origin_with_signals.cc   (with props)
Modified:
    compiler-rt/trunk/lib/msan/msan.cc
    compiler-rt/trunk/lib/msan/msan_interceptors.cc
    compiler-rt/trunk/lib/msan/msan_thread.h

Modified: compiler-rt/trunk/lib/msan/msan.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/msan/msan.cc?rev=206983&r1=206982&r2=206983&view=diff
==============================================================================
--- compiler-rt/trunk/lib/msan/msan.cc (original)
+++ compiler-rt/trunk/lib/msan/msan.cc Wed Apr 23 09:01:57 2014
@@ -237,6 +237,8 @@ const char *GetOriginDescrIfStack(u32 id
 }
 
 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);

Modified: compiler-rt/trunk/lib/msan/msan_interceptors.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/msan/msan_interceptors.cc?rev=206983&r1=206982&r2=206983&view=diff
==============================================================================
--- compiler-rt/trunk/lib/msan/msan_interceptors.cc (original)
+++ compiler-rt/trunk/lib/msan/msan_interceptors.cc Wed Apr 23 09:01:57 2014
@@ -981,6 +981,12 @@ INTERCEPTOR(int, getrusage, int who, voi
   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[kMaxS
 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));

Modified: compiler-rt/trunk/lib/msan/msan_thread.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/msan/msan_thread.h?rev=206983&r1=206982&r2=206983&view=diff
==============================================================================
--- compiler-rt/trunk/lib/msan/msan_thread.h (original)
+++ compiler-rt/trunk/lib/msan/msan_thread.h Wed Apr 23 09:01:57 2014
@@ -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_;
 };
 

Added: compiler-rt/trunk/test/msan/chained_origin_with_signals.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/msan/chained_origin_with_signals.cc?rev=206983&view=auto
==============================================================================
--- compiler-rt/trunk/test/msan/chained_origin_with_signals.cc (added)
+++ compiler-rt/trunk/test/msan/chained_origin_with_signals.cc Wed Apr 23 09:01:57 2014
@@ -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 <signal.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+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

Propchange: compiler-rt/trunk/test/msan/chained_origin_with_signals.cc
------------------------------------------------------------------------------
    svn:eol-style = LF