[compiler-rt] ed9ef9b - tsan: Consider SI_TIMER signals always asynchronous

[Marco Elver via llvm-commits]

Author: Marco Elver
Date: 2023-01-20T11:47:30+01:00
New Revision: ed9ef9b4f248c62022bb1bebe5e2597cb4f9d209

URL: https://github.com/llvm/llvm-project/commit/ed9ef9b4f248c62022bb1bebe5e2597cb4f9d209
DIFF: https://github.com/llvm/llvm-project/commit/ed9ef9b4f248c62022bb1bebe5e2597cb4f9d209.diff

LOG: tsan: Consider SI_TIMER signals always asynchronous

POSIX timer can be configured to send any kind of signal, however, it
fundamentally does not make sense to consider a timer a synchronous
signal. Teach TSan that timers are never synchronous.

The tricky bit here is correctly defining compiler-rt's siginfo
replacement, which is a rather complex struct. Extend it in a limited
way that is mostly cross-platform compatible and add offset tests in
sanitizer_platform_limits_posix.cpp.

Reviewed By: dvyukov

Differential Revision: https://reviews.llvm.org/D142117

Added: 
    

Modified: 
    compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h
    compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp
index cb647bea4df7a..fc01498aa2285 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp
@@ -1125,6 +1125,15 @@ CHECK_STRUCT_SIZE_AND_OFFSET(sigaction, sa_flags);
 CHECK_STRUCT_SIZE_AND_OFFSET(sigaction, sa_restorer);
 #endif
 
+#if SANITIZER_HAS_SIGINFO
+COMPILER_CHECK(alignof(siginfo_t) == alignof(__sanitizer_siginfo));
+using __sanitizer_siginfo_t = __sanitizer_siginfo;
+CHECK_TYPE_SIZE(siginfo_t);
+CHECK_SIZE_AND_OFFSET(siginfo_t, si_signo);
+CHECK_SIZE_AND_OFFSET(siginfo_t, si_errno);
+CHECK_SIZE_AND_OFFSET(siginfo_t, si_code);
+#endif
+
 #if SANITIZER_LINUX
 CHECK_TYPE_SIZE(__sysctl_args);
 CHECK_SIZE_AND_OFFSET(__sysctl_args, name);

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h
index 2146daf7b119e..fdc69b8a5fba5 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h
@@ -586,11 +586,31 @@ struct __sanitizer_sigset_t {
 };
 #endif
 
-struct __sanitizer_siginfo {
-  // The size is determined by looking at sizeof of real siginfo_t on linux.
-  u64 opaque[128 / sizeof(u64)];
+struct __sanitizer_siginfo_pad {
+  // Require uptr, because siginfo_t is always pointer-size aligned on Linux.
+  uptr pad[128 / sizeof(uptr)];
 };
 
+#if SANITIZER_LINUX
+# define SANITIZER_HAS_SIGINFO 1
+union __sanitizer_siginfo {
+  struct {
+    int si_signo;
+# if SANITIZER_MIPS
+    int si_code;
+    int si_errno;
+# else
+    int si_errno;
+    int si_code;
+# endif
+  };
+  __sanitizer_siginfo_pad pad;
+};
+#else
+# define SANITIZER_HAS_SIGINFO 0
+typedef __sanitizer_siginfo_pad __sanitizer_siginfo;
+#endif
+
 using __sanitizer_sighandler_ptr = void (*)(int sig);
 using __sanitizer_sigactionhandler_ptr = void (*)(int sig,
                                                   __sanitizer_siginfo *siginfo,

diff  --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index becaba424877d..97aa4b77311f1 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -128,6 +128,7 @@ const int SIGSYS = 12;
 const int SIGBUS = 7;
 const int SIGSYS = 31;
 #endif
+const int SI_TIMER = -2;
 void *const MAP_FAILED = (void*)-1;
 #if SANITIZER_NETBSD
 const int PTHREAD_BARRIER_SERIAL_THREAD = 1234567;
@@ -2165,11 +2166,19 @@ void ProcessPendingSignalsImpl(ThreadState *thr) {
 
 }  // namespace __tsan
 
-static bool is_sync_signal(ThreadSignalContext *sctx, int sig) {
+static bool is_sync_signal(ThreadSignalContext *sctx, int sig,
+                           __sanitizer_siginfo *info) {
+  // If we are sending signal to ourselves, we must process it now.
+  if (sctx && sig == sctx->int_signal_send)
+    return true;
+#if SANITIZER_HAS_SIGINFO
+  // POSIX timers can be configured to send any kind of signal; however, it
+  // doesn't make any sense to consider a timer signal as synchronous!
+  if (info->si_code == SI_TIMER)
+    return false;
+#endif
   return sig == SIGSEGV || sig == SIGBUS || sig == SIGILL || sig == SIGTRAP ||
-         sig == SIGABRT || sig == SIGFPE || sig == SIGPIPE || sig == SIGSYS ||
-         // If we are sending signal to ourselves, we must process it now.
-         (sctx && sig == sctx->int_signal_send);
+         sig == SIGABRT || sig == SIGFPE || sig == SIGPIPE || sig == SIGSYS;
 }
 
 void sighandler(int sig, __sanitizer_siginfo *info, void *ctx) {
@@ -2180,7 +2189,7 @@ void sighandler(int sig, __sanitizer_siginfo *info, void *ctx) {
     return;
   }
   // Don't mess with synchronous signals.
-  const bool sync = is_sync_signal(sctx, sig);
+  const bool sync = is_sync_signal(sctx, sig, info);
   if (sync ||
       // If we are in blocking function, we can safely process it now
       // (but check if we are in a recursive interceptor,