1 files changed, 32 insertions, 17 deletions

diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc b/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc
index d80809dc689..fc742a56ace 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc
@@ -101,14 +101,15 @@ typedef long long_t;  // NOLINT
 # define F_TLOCK 2      /* Test and lock a region for exclusive use.  */
 # define F_TEST  3      /* Test a region for other processes locks.  */
 
-typedef void (*sighandler_t)(int sig);
-
 #define errno (*__errno_location())
 
+typedef void (*sighandler_t)(int sig);
+typedef void (*sigactionhandler_t)(int sig, my_siginfo_t *siginfo, void *uctx);
+
 struct sigaction_t {
   union {
     sighandler_t sa_handler;
-    void (*sa_sigaction)(int sig, my_siginfo_t *siginfo, void *uctx);
+    sigactionhandler_t sa_sigaction;
   };
 #if SANITIZER_FREEBSD
   int sa_flags;
@@ -1873,15 +1874,18 @@ static void CallUserSignalHandler(ThreadState *thr, bool sync, bool acquire,
   // Ensure that the handler does not spoil errno.
   const int saved_errno = errno;
   errno = 99;
-  // Need to remember pc before the call, because the handler can reset it.
-  uptr pc = sigact ?
+  // This code races with sigaction. Be careful to not read sa_sigaction twice.
+  // Also need to remember pc for reporting before the call,
+  // because the handler can reset it.
+  volatile uptr pc = sigact ?
      (uptr)sigactions[sig].sa_sigaction :
      (uptr)sigactions[sig].sa_handler;
-  pc += 1;  // return address is expected, OutputReport() will undo this
-  if (sigact)
-    sigactions[sig].sa_sigaction(sig, info, uctx);
-  else
-    sigactions[sig].sa_handler(sig);
+  if (pc != (uptr)SIG_DFL && pc != (uptr)SIG_IGN) {
+    if (sigact)
+      ((sigactionhandler_t)pc)(sig, info, uctx);
+    else
+      ((sighandler_t)pc)(sig);
+  }
   // We do not detect errno spoiling for SIGTERM,
   // because some SIGTERM handlers do spoil errno but reraise SIGTERM,
   // tsan reports false positive in such case.
@@ -1891,7 +1895,9 @@ static void CallUserSignalHandler(ThreadState *thr, bool sync, bool acquire,
   // signal; and it looks too fragile to intercept all ways to reraise a signal.
   if (flags()->report_bugs && !sync && sig != SIGTERM && errno != 99) {
     VarSizeStackTrace stack;
-    ObtainCurrentStack(thr, pc, &stack);
+    // Add 1 to pc because return address is expected,
+    // OutputReport() will undo this.
+    ObtainCurrentStack(thr, pc + 1, &stack);
     ThreadRegistryLock l(ctx->thread_registry);
     ScopedReport rep(ReportTypeErrnoInSignal);
     if (!IsFiredSuppression(ctx, rep, stack)) {
@@ -1917,11 +1923,8 @@ void ProcessPendingSignals(ThreadState *thr) {
     SignalDesc *signal = &sctx->pending_signals[sig];
     if (signal->armed) {
       signal->armed = false;
-      if (sigactions[sig].sa_handler != SIG_DFL
-          && sigactions[sig].sa_handler != SIG_IGN) {
-        CallUserSignalHandler(thr, false, true, signal->sigaction,
-            sig, &signal->siginfo, &signal->ctx);
-      }
+      CallUserSignalHandler(thr, false, true, signal->sigaction, sig,
+          &signal->siginfo, &signal->ctx);
     }
   }
   pthread_sigmask(SIG_SETMASK, &oldset, 0);
@@ -2003,7 +2006,19 @@ TSAN_INTERCEPTOR(int, sigaction, int sig, sigaction_t *act, sigaction_t *old) {
     internal_memcpy(old, &sigactions[sig], sizeof(*old));
   if (act == 0)
     return 0;
-  internal_memcpy(&sigactions[sig], act, sizeof(*act));
+  // Copy act into sigactions[sig].
+  // Can't use struct copy, because compiler can emit call to memcpy.
+  // Can't use internal_memcpy, because it copies byte-by-byte,
+  // and signal handler reads the sa_handler concurrently. It it can read
+  // some bytes from old value and some bytes from new value.
+  // Use volatile to prevent insertion of memcpy.
+  sigactions[sig].sa_handler = *(volatile sighandler_t*)&act->sa_handler;
+  sigactions[sig].sa_flags = *(volatile int*)&act->sa_flags;
+  internal_memcpy(&sigactions[sig].sa_mask, &act->sa_mask,
+      sizeof(sigactions[sig].sa_mask));
+#if !SANITIZER_FREEBSD
+  sigactions[sig].sa_restorer = act->sa_restorer;
+#endif
   sigaction_t newact;
   internal_memcpy(&newact, act, sizeof(newact));
   REAL(sigfillset)(&newact.sa_mask);