[PATCH] D21609: [tsan] Intercept libcxx __release_shared to avoid false positive with weak_ptrs and destructors in C++

[Kuba Brecka via llvm-commits]

kubabrecka created this revision.
kubabrecka added reviewers: dvyukov, kcc, glider.
kubabrecka added subscribers: llvm-commits, zaks.anna, dcoughlin.
kubabrecka added a project: Sanitizers.
Herald added a subscriber: kubabrecka.

There is a “well-known” TSan false positive when using C++ weak_ptr/shared_ptr and code in destructors, e.g. described at <https://llvm.org/bugs/show_bug.cgi?id=22324>.  The “standard" solution is to build and use a TSan-instrumented version of libcxx, which is not trivial for end-users.  This patch tries a different approach (on OS X):  It adds an interceptor for the specific function in libc++.dylib, which implements the atomic operation that needs to be visible to TSan.

The API/ABI of libc++.dylib is stable on OS X, so adding this interceptor is compatible with all OS versions.  I went through the sources of libcxx and this seems to be the only function that needs to be intercepted.

http://reviews.llvm.org/D21609

Files:
  lib/tsan/rtl/tsan_interceptors_mac.cc
  test/tsan/Darwin/libcxx-shared-ptr.mm

Index: test/tsan/Darwin/libcxx-shared-ptr.mm
===================================================================
--- test/tsan/Darwin/libcxx-shared-ptr.mm
+++ test/tsan/Darwin/libcxx-shared-ptr.mm
@@ -0,0 +1,48 @@
+// RUN: %clang_tsan %s -o %t -framework Foundation
+// RUN: %env_tsan_opts=ignore_interceptors_accesses=1 %run %t 2>&1 | FileCheck %s
+
+#import <Foundation/Foundation.h>
+
+#import <memory>
+
+#import "../test.h"
+
+long my_global;
+
+struct MyStruct {
+  void setGlobal() {
+    my_global = 42;
+  }
+  ~MyStruct() {
+    my_global = 43;
+  }
+};
+
+int main(int argc, const char *argv[]) {
+  fprintf(stderr, "Hello world.\n");
+  print_address("addr=", 1, &my_global);
+  barrier_init(&barrier, 2);
+
+  std::shared_ptr<MyStruct> shared(new MyStruct());
+
+  dispatch_queue_t q = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
+
+  std::weak_ptr<MyStruct> weak(shared);
+  
+  dispatch_async(q, ^{
+    std::shared_ptr<MyStruct> strong = weak.lock();
+    if (!strong) exit(1);
+
+    strong->setGlobal();
+    barrier_wait(&barrier);
+  });
+
+  barrier_wait(&barrier);
+  shared.reset();
+
+  fprintf(stderr, "Done.\n");
+}
+
+// CHECK: Hello world.
+// CHECK: Done.
+// CHECK-NOT: WARNING: ThreadSanitizer
Index: lib/tsan/rtl/tsan_interceptors_mac.cc
===================================================================
--- lib/tsan/rtl/tsan_interceptors_mac.cc
+++ lib/tsan/rtl/tsan_interceptors_mac.cc
@@ -273,6 +273,26 @@
   (connection, message, replyq, new_handler);
 }
 
+// On macOS, libc++ is always linked dynamically, so intercepting works the
+// usual way.
+#define STDCXX_INTERCEPTOR TSAN_INTERCEPTOR
+
+// This adds a libc++ interceptor for:
+//     void __shared_weak_count::__release_shared() _NOEXCEPT;
+// Shared and weak pointers in C++ maintain reference counts via atomics in
+// libc++.dylib, which are TSan-invisible, and this leads to false positives in
+// destructor code.  This interceptor implements the mo_acq_rel semantics of the
+// atomic decrement that is used to check when the object's refcount drops to 0.
+STDCXX_INTERCEPTOR(void, _ZNSt3__119__shared_weak_count16__release_sharedEv,
+                   void *o) {
+  SCOPED_INTERCEPTOR_RAW(_ZNSt3__119__shared_weak_count16__release_sharedEv, o);
+  Acquire(thr, pc, (uptr)o);
+  Release(thr, pc, (uptr)o);
+  SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START();
+  REAL(_ZNSt3__119__shared_weak_count16__release_sharedEv)(o);
+  SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END();
+}
+
 }  // namespace __tsan
 
 #endif  // SANITIZER_MAC


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D21609.61547.patch
Type: text/x-patch
Size: 2549 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160622/23a18aa0/attachment.bin>