[TSan] Add lock_during_write flag on Apple platforms to avoid deadlock (#157928)

There is a commonly used library that interposes the write call, and is
interposed itself. When running with `TSAN_OPTIONS=verbosity=(1|2)`,
this leads to a thread locking itself, resulting in the program hanging.

This patch adds a new flag `lock_during_write` to TSan (on Apple
platforms only) that, when set, allows interceptors to be bypassed
during calls to write. The flag can be inherited by children, or not,
depending on the value.

rdar://157565672

Author: DanBlackwell

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

@@ -105,6 +105,10 @@ extern "C" {
     mach_msg_type_number_t *infoCnt);
 }
 
+// Weak symbol no-op when TSan is not linked
+SANITIZER_WEAK_ATTRIBUTE extern void __tsan_set_in_internal_write_call(
+    bool value) {}
+
 namespace __sanitizer {
 
 #include "sanitizer_syscall_generic.inc"
@@ -175,7 +179,11 @@ uptr internal_read(fd_t fd, void *buf, uptr count) {
 }
 
 uptr internal_write(fd_t fd, const void *buf, uptr count) {
-  return write(fd, buf, count);
+  // We need to disable interceptors when writing in TSan
+  __tsan_set_in_internal_write_call(true);
+  uptr res = write(fd, buf, count);
+  __tsan_set_in_internal_write_call(false);
+  return res;
 }
 
 uptr internal_stat(const char *path, void *buf) {

compiler-rt/lib/tsan/rtl/tsan_flags.cpp

@@ -20,6 +20,43 @@
 #include "tsan_rtl.h"
 #include "ubsan/ubsan_flags.h"
 
+#if SANITIZER_APPLE
+namespace __sanitizer {
+
+template <>
+inline bool FlagHandler<LockDuringWriteSetting>::Parse(const char *value) {
+  if (internal_strcmp(value, "on") == 0) {
+    *t_ = kLockDuringAllWrites;
+    return true;
+  }
+  if (internal_strcmp(value, "disable_for_current_process") == 0) {
+    *t_ = kNoLockDuringWritesCurrentProcess;
+    return true;
+  }
+  if (internal_strcmp(value, "disable_for_all_processes") == 0) {
+    *t_ = kNoLockDuringWritesAllProcesses;
+    return true;
+  }
+  Printf("ERROR: Invalid value for signal handler option: '%s'\n", value);
+  return false;
+}
+
+template <>
+inline bool FlagHandler<LockDuringWriteSetting>::Format(char *buffer,
+                                                        uptr size) {
+  switch (*t_) {
+    case kLockDuringAllWrites:
+      return FormatString(buffer, size, "on");
+    case kNoLockDuringWritesCurrentProcess:
+      return FormatString(buffer, size, "disable_for_current_process");
+    case kNoLockDuringWritesAllProcesses:
+      return FormatString(buffer, size, "disable_for_all_processes");
+  }
+}
+
+}  // namespace __sanitizer
+#endif
+
 namespace __tsan {
 
 // Can be overriden in frontend.

compiler-rt/lib/tsan/rtl/tsan_flags.h

@@ -16,6 +16,14 @@
 #include "sanitizer_common/sanitizer_flags.h"
 #include "sanitizer_common/sanitizer_deadlock_detector_interface.h"
 
+#if SANITIZER_APPLE
+enum LockDuringWriteSetting {
+  kLockDuringAllWrites,
+  kNoLockDuringWritesCurrentProcess,
+  kNoLockDuringWritesAllProcesses,
+};
+#endif
+
 namespace __tsan {
 
 struct Flags : DDFlags {

compiler-rt/lib/tsan/rtl/tsan_flags.inc

@@ -80,3 +80,15 @@ TSAN_FLAG(bool, shared_ptr_interceptor, true,
 TSAN_FLAG(bool, print_full_thread_history, false,
           "If set, prints thread creation stacks for the threads involved in "
           "the report and their ancestors up to the main thread.")
+
+#if SANITIZER_APPLE
+TSAN_FLAG(LockDuringWriteSetting, lock_during_write, kLockDuringAllWrites,
+          "Determines whether to obtain a lock while writing logs or error "
+          "reports. "
+          "\"on\" - [default] lock during all writes. "
+          "\"disable_for_current_process\" - don't lock during all writes in "
+          "the current process, but do lock for all writes in child "
+          "processes."
+          "\"disable_for_all_processes\" - don't lock during all writes in "
+          "the current process and it's children processes.")
+#endif

compiler-rt/lib/tsan/rtl/tsan_interceptors.h

@@ -1,6 +1,9 @@
 #ifndef TSAN_INTERCEPTORS_H
 #define TSAN_INTERCEPTORS_H
 
+#if SANITIZER_APPLE
+#  include "sanitizer_common/sanitizer_mac.h"
+#endif
 #include "sanitizer_common/sanitizer_stacktrace.h"
 #include "tsan_rtl.h"
 
@@ -43,7 +46,12 @@ inline bool in_symbolizer() {
 #endif
 
 inline bool MustIgnoreInterceptor(ThreadState *thr) {
-  return !thr->is_inited || thr->ignore_interceptors || thr->in_ignored_lib;
+  return !thr->is_inited || thr->ignore_interceptors || thr->in_ignored_lib
+#if SANITIZER_APPLE
+         || (flags()->lock_during_write != kLockDuringAllWrites &&
+             thr->in_internal_write_call)
+#endif
+      ;
 }
 
 }  // namespace __tsan

compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp

@@ -31,6 +31,9 @@
 #include "sanitizer_common/sanitizer_tls_get_addr.h"
 #include "sanitizer_common/sanitizer_vector.h"
 #include "tsan_fd.h"
+#if SANITIZER_APPLE
+#  include "tsan_flags.h"
+#endif
 #include "tsan_interceptors.h"
 #include "tsan_interface.h"
 #include "tsan_mman.h"
@@ -1665,6 +1668,14 @@ TSAN_INTERCEPTOR(int, pthread_barrier_wait, void *b) {
 
 TSAN_INTERCEPTOR(int, pthread_once, void *o, void (*f)()) {
   SCOPED_INTERCEPTOR_RAW(pthread_once, o, f);
+#if SANITIZER_APPLE
+  if (flags()->lock_during_write != kLockDuringAllWrites &&
+      cur_thread_init()->in_internal_write_call) {
+    // This is needed to make it through process launch without hanging
+    f();
+    return 0;
+  }
+#endif
   if (o == 0 || f == 0)
     return errno_EINVAL;
   atomic_uint32_t *a;

compiler-rt/lib/tsan/rtl/tsan_rtl.cpp

@@ -40,6 +40,13 @@ SANITIZER_WEAK_DEFAULT_IMPL
 void __tsan_test_only_on_fork() {}
 #endif
 
+#if SANITIZER_APPLE
+// Override weak symbol from sanitizer_common
+extern void __tsan_set_in_internal_write_call(bool value) {
+  __tsan::cur_thread_init()->in_internal_write_call = value;
+}
+#endif
+
 namespace __tsan {
 
 #if !SANITIZER_GO
@@ -893,6 +900,13 @@ void ForkChildAfter(ThreadState* thr, uptr pc, bool start_thread) {
     ThreadIgnoreBegin(thr, pc);
     ThreadIgnoreSyncBegin(thr, pc);
   }
+
+#  if SANITIZER_APPLE
+  // This flag can have inheritance disabled - we are the child so act
+  // accordingly
+  if (flags()->lock_during_write == kNoLockDuringWritesCurrentProcess)
+    flags()->lock_during_write = kLockDuringAllWrites;
+#  endif
 }
 #endif
 

compiler-rt/lib/tsan/rtl/tsan_rtl.h

@@ -236,6 +236,10 @@ struct alignas(SANITIZER_CACHE_LINE_SIZE) ThreadState {
 
   const ReportDesc *current_report;
 
+#if SANITIZER_APPLE
+  bool in_internal_write_call;
+#endif
+
   explicit ThreadState(Tid tid);
 };
 

compiler-rt/test/tsan/Darwin/write-interpose.c

@@ -0,0 +1,50 @@
+// Test that dylibs interposing write, and then calling functions intercepted
+// by TSan don't deadlock (self-lock)
+
+// RUN: %clang_tsan %s -o %t
+// RUN: %clang_tsan %s -o %t.dylib -fno-sanitize=thread -dynamiclib -DSHARED_LIB
+
+// Note that running the below command with out `lock_during_write` should
+// deadlock (self-lock)
+// RUN: env DYLD_INSERT_LIBRARIES=%t.dylib TSAN_OPTIONS=verbosity=2:lock_during_write=disable_for_current_process %run %t 2>&1 | FileCheck %s
+
+#include <stdio.h>
+
+#if defined(SHARED_LIB)
+
+// dylib implementation - interposes write() calls
+#  include <os/lock.h>
+#  include <unistd.h>
+
+struct interpose_substitution {
+  const void *replacement;
+  const void *original;
+};
+
+#  define INTERPOSE(replacement, original)                                     \
+    __attribute__((used)) static const struct interpose_substitution           \
+        substitution_##original[]                                              \
+        __attribute__((section("__DATA, __interpose"))) = {                    \
+            {(const void *)(replacement), (const void *)(original)}}
+
+static ssize_t my_write(int fd, const void *buf, size_t count) {
+  struct os_unfair_lock_s lock = OS_UNFAIR_LOCK_INIT;
+  os_unfair_lock_lock(&lock);
+  printf("Interposed write called: fd=%d, count=%zu\n", fd, count);
+  ssize_t res = write(fd, buf, count);
+  os_unfair_lock_unlock(&lock);
+  return res;
+}
+INTERPOSE(my_write, write);
+
+#else // defined(SHARED_LIB)
+
+int main() {
+  printf("Write test completed\n");
+  return 0;
+}
+
+#endif // defined(SHARED_LIB)
+
+// CHECK: Interposed write called: fd={{[0-9]+}}, count={{[0-9]+}}
+// CHECK: Write test completed