tsan: yet another attempt to fix pthread_cond interceptors

Make behavior introduced in r202820 conditional (under legacy_pthread_cond flag).
The new issue that we've hit with the satellite pthread_cond_t struct is
that pthread_condattr_getpshared does not work (satellite data is not shared between processes).
The idea is that most processes do not use pthread 2.2.5.
The rare ones that use (2.2.5 is dated by 2002) must specify legacy_pthread_cond=1
on their own risk.



git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@204032 91177308-0d34-0410-b5e6-96231b3b80d8

6 files changed, 28 insertions, 10 deletions

diff --git a/lib/sanitizer_common/sanitizer_common_interceptors.inc b/lib/sanitizer_common/sanitizer_common_interceptors.inc
index 5241cdcf4..f96229145 100644
--- a/lib/sanitizer_common/sanitizer_common_interceptors.inc
+++ b/lib/sanitizer_common/sanitizer_common_interceptors.inc
@@ -2449,17 +2449,21 @@ INTERCEPTOR(int, pthread_mutex_unlock, void *m) {
 #endif
 
 #if SANITIZER_INTERCEPT_PTHREAD_COND
-// nptl implementation of pthread_cond has 2 versions (2.2.5 and 2.3.2).
+// Problem:
+// NPTL implementation of pthread_cond has 2 versions (2.2.5 and 2.3.2).
 // pthread_cond_t has different size in the different versions.
-// We can't simply always call new REAL functions from interceptors,
-// because they will corrupt memory after pthread_cond_t (old cond is smaller).
-// We can't simply always call old REAL functions from interceptors,
-// because they do not support all the features (e.g. waiting against
+// If call new REAL functions for old pthread_cond_t, they will corrupt memory
+// after pthread_cond_t (old cond is smaller).
+// If we call old REAL functions for new pthread_cond_t, we will lose  some
+// functionality (e.g. old functions do not support waiting against
 // CLOCK_REALTIME).
 // Proper handling would require to have 2 versions of interceptors as well.
 // But this is messy, in particular requires linker scripts when sanitizer
 // runtime is linked into a shared library.
-// Instead we do the following trick.
+// Instead we assume we don't have dynamic libraries built against old
+// pthread (2.2.5 is dated by 2002). And provide legacy_pthread_cond flag
+// that allows to work with old libraries (but this mode does not support
+// some features, e.g. pthread_condattr_getpshared).
 static void *init_cond(void *c, bool force = false) {
   // sizeof(pthread_cond_t) >= sizeof(uptr) in both versions.
   // So we allocate additional memory on the side large enough to hold
@@ -2468,7 +2472,7 @@ static void *init_cond(void *c, bool force = false) {
   // Note: the code assumes that PTHREAD_COND_INITIALIZER initializes
   // first word of pthread_cond_t to zero.
   // It's all relevant only for linux.
-  if (!SI_LINUX_NOT_ANDROID)
+  if (!common_flags()->legacy_pthread_cond)
     return c;
   atomic_uintptr_t *p = (atomic_uintptr_t*)c;
   uptr cond = atomic_load(p, memory_order_acquire);
@@ -2518,6 +2522,9 @@ INTERCEPTOR(int, pthread_cond_wait, void *c, void *m) {
   int res = __sanitizer::call_pthread_cancel_with_cleanup(
       (int(*)(void *c, void *m, void *abstime))REAL(pthread_cond_wait),
       cond, m, 0, (void(*)(void *arg))cond_mutex_unlock, &arg);
+  if (res == errno_EOWNERDEAD)
+    COMMON_INTERCEPTOR_MUTEX_REPAIR(ctx, m);
+  COMMON_INTERCEPTOR_MUTEX_LOCK(ctx, m);
   return res;
 }
 
@@ -2533,6 +2540,9 @@ INTERCEPTOR(int, pthread_cond_timedwait, void *c, void *m, void *abstime) {
   int res = __sanitizer::call_pthread_cancel_with_cleanup(
       REAL(pthread_cond_timedwait), cond, m, abstime,
       (void(*)(void *arg))cond_mutex_unlock, &arg);
+  if (res == errno_EOWNERDEAD)
+    COMMON_INTERCEPTOR_MUTEX_REPAIR(ctx, m);
+  COMMON_INTERCEPTOR_MUTEX_LOCK(ctx, m);
   return res;
 }
 
@@ -2558,7 +2568,7 @@ INTERCEPTOR(int, pthread_cond_destroy, void *c) {
   COMMON_INTERCEPTOR_ENTER(ctx, pthread_cond_destroy, cond);
   COMMON_INTERCEPTOR_WRITE_RANGE(ctx, c, sizeof(uptr));
   int res = REAL(pthread_cond_destroy)(cond);
-  if (SI_LINUX_NOT_ANDROID) {
+  if (common_flags()->legacy_pthread_cond) {
     // Free our aux cond and zero the pointer to not leave dangling pointers.
     WRAP(free)(cond);
     atomic_store((atomic_uintptr_t*)c, 0, memory_order_relaxed);
diff --git a/lib/sanitizer_common/sanitizer_flags.cc b/lib/sanitizer_common/sanitizer_flags.cc
index 2f52fa72a..e48a7a2d8 100644
--- a/lib/sanitizer_common/sanitizer_flags.cc
+++ b/lib/sanitizer_common/sanitizer_flags.cc
@@ -43,6 +43,7 @@ void SetCommonFlagsDefaults(CommonFlags *f) {
   f->detect_deadlocks = false;
   f->clear_shadow_mmap_threshold = 64 * 1024;
   f->color = "auto";
+  f->legacy_pthread_cond = false;
 }
 
 void ParseCommonFlagsFromString(CommonFlags *f, const char *str) {
@@ -68,6 +69,7 @@ void ParseCommonFlagsFromString(CommonFlags *f, const char *str) {
   ParseFlag(str, &f->clear_shadow_mmap_threshold,
             "clear_shadow_mmap_threshold");
   ParseFlag(str, &f->color, "color");
+  ParseFlag(str, &f->legacy_pthread_cond, "legacy_pthread_cond");
 
   // Do a sanity check for certain flags.
   if (f->malloc_context_size < 1)
diff --git a/lib/sanitizer_common/sanitizer_flags.h b/lib/sanitizer_common/sanitizer_flags.h
index 4d2d684b8..94d695461 100644
--- a/lib/sanitizer_common/sanitizer_flags.h
+++ b/lib/sanitizer_common/sanitizer_flags.h
@@ -78,6 +78,8 @@ struct CommonFlags {
   uptr clear_shadow_mmap_threshold;
   // Colorize reports: (always|never|auto).
   const char *color;
+  // Enables support for dynamic libraries linked with libpthread 2.2.5.
+  bool legacy_pthread_cond;
 };
 
 inline CommonFlags *common_flags() {
diff --git a/lib/sanitizer_common/sanitizer_linux_libcdep.cc b/lib/sanitizer_common/sanitizer_linux_libcdep.cc
index ff53b12bb..71a084330 100644
--- a/lib/sanitizer_common/sanitizer_linux_libcdep.cc
+++ b/lib/sanitizer_common/sanitizer_linux_libcdep.cc
@@ -525,7 +525,7 @@ int call_pthread_cancel_with_cleanup(int(*fn)(void *c, void *m,
   int res;
   pthread_cleanup_push(cleanup, arg);
   res = fn(c, m, abstime);
-  pthread_cleanup_pop(1);
+  pthread_cleanup_pop(0);
   return res;
 }
 
diff --git a/lib/sanitizer_common/sanitizer_mac.cc b/lib/sanitizer_common/sanitizer_mac.cc
index b6b3a0160..14201dd92 100644
--- a/lib/sanitizer_common/sanitizer_mac.cc
+++ b/lib/sanitizer_common/sanitizer_mac.cc
@@ -310,7 +310,7 @@ int call_pthread_cancel_with_cleanup(int(*fn)(void *c, void *m,
   int res;
   pthread_cleanup_push(cleanup, arg);
   res = fn(c, m, abstime);
-  pthread_cleanup_pop(1);
+  pthread_cleanup_pop(0);
   return res;
 }
 
diff --git a/lib/tsan/tests/unit/tsan_flags_test.cc b/lib/tsan/tests/unit/tsan_flags_test.cc
index 36e3a49bf..33ddaaae2 100644
--- a/lib/tsan/tests/unit/tsan_flags_test.cc
+++ b/lib/tsan/tests/unit/tsan_flags_test.cc
@@ -75,6 +75,7 @@ static const char *options1 =
   " leak_check_at_exit=0"
   " allocator_may_return_null=0"
   " print_summary=0"
+  " legacy_pthread_cond=0"
   "";
 
 static const char *options2 =
@@ -118,6 +119,7 @@ static const char *options2 =
   " leak_check_at_exit=true"
   " allocator_may_return_null=true"
   " print_summary=true"
+  " legacy_pthread_cond=true"
   "";
 
 void VerifyOptions1(Flags *f) {
@@ -161,6 +163,7 @@ void VerifyOptions1(Flags *f) {
   EXPECT_EQ(f->leak_check_at_exit, 0);
   EXPECT_EQ(f->allocator_may_return_null, 0);
   EXPECT_EQ(f->print_summary, 0);
+  EXPECT_EQ(f->legacy_pthread_cond, false);
 }
 
 void VerifyOptions2(Flags *f) {
@@ -204,6 +207,7 @@ void VerifyOptions2(Flags *f) {
   EXPECT_EQ(f->leak_check_at_exit, true);
   EXPECT_EQ(f->allocator_may_return_null, true);
   EXPECT_EQ(f->print_summary, true);
+  EXPECT_EQ(f->legacy_pthread_cond, true);
 }
 
 static const char *test_default_options;