[PATCH] D55959: [TSan] Enable detection of lock-order-inversions for Objective-C @synchronized

[Julian Lettner via Phabricator via llvm-commits]

yln created this revision.
yln added reviewers: dcoughlin, dvyukov, kubamracek, delcypher.
yln added a project: Sanitizers.
Herald added subscribers: Sanitizers, llvm-commits.

@synchronized semantics can be synthesized by using existing mutex_[un]lock operations.

  @synchronized(obj) {
    // ...
  }
  
  => 
  {
    mutex_lock(obj);
    // ...
    mutex_unlock(obj);
  }

Let me know whether you think this a good idea.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D55959

Files:
  compiler-rt/lib/tsan/rtl/tsan_interceptors_mac.cc
  compiler-rt/test/tsan/Darwin/objc-synchronize-cycle.mm


Index: compiler-rt/test/tsan/Darwin/objc-synchronize-cycle.mm
===================================================================
--- /dev/null
+++ compiler-rt/test/tsan/Darwin/objc-synchronize-cycle.mm
@@ -0,0 +1,31 @@
+// RUN: %clangxx_tsan %s -o %t -framework Foundation -fobjc-arc %darwin_min_target_with_full_runtime_arc_support
+// RUN:                                   not %run %t 2>&1 | FileCheck %s
+// RUN: %env_tsan_opts=detect_deadlocks=1 not %run %t 2>&1 | FileCheck %s
+// RUN: %env_tsan_opts=detect_deadlocks=0     %run %t 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+#import <Foundation/Foundation.h>
+
+int main() {
+  @autoreleasepool {
+    NSObject* obj1 = [NSObject new];
+    NSObject* obj2 = [NSObject new];
+
+    // obj1 -> obj2
+    @synchronized(obj1) {
+      @synchronized(obj2) {
+      }
+    }
+
+    // obj1 -> obj1
+    @synchronized(obj2) {
+      @synchronized(obj1) {
+// CHECK: ThreadSanitizer: lock-order-inversion (potential deadlock)
+      }
+    }
+  }
+
+  NSLog(@"PASS");
+// DISABLED-NOT: ThreadSanitizer
+// DISABLED: PASS
+  return 0;
+}
Index: compiler-rt/lib/tsan/rtl/tsan_interceptors_mac.cc
===================================================================
--- compiler-rt/lib/tsan/rtl/tsan_interceptors_mac.cc
+++ compiler-rt/lib/tsan/rtl/tsan_interceptors_mac.cc
@@ -318,17 +318,27 @@
   return (uptr)obj;
 }
 
+// Defined in <objc-sync.h>
+#define OBJC_SYNC_SUCCESS 0
+
 TSAN_INTERCEPTOR(int, objc_sync_enter, void *obj) {
   SCOPED_TSAN_INTERCEPTOR(objc_sync_enter, obj);
-  int result = REAL(objc_sync_enter)(obj);
-  if (obj) Acquire(thr, pc, SyncAddressForObjCObject(obj));
+  if (!obj) return REAL(objc_sync_enter)(obj);
+  uptr addr = SyncAddressForObjCObject(obj);
+  MutexPreLock(thr, pc, addr);
+  int result = REAL(objc_sync_enter)(obj); // Always returns OBJC_SYNC_SUCCESS
+  MutexPostLock(thr, pc, addr);
   return result;
 }
 
 TSAN_INTERCEPTOR(int, objc_sync_exit, void *obj) {
   SCOPED_TSAN_INTERCEPTOR(objc_sync_exit, obj);
-  if (obj) Release(thr, pc, SyncAddressForObjCObject(obj));
-  return REAL(objc_sync_exit)(obj);
+  if (!obj) return REAL(objc_sync_exit)(obj);
+  uptr addr = SyncAddressForObjCObject(obj);
+  MutexUnlock(thr, pc, addr);
+  int result = REAL(objc_sync_exit)(obj);
+  if (result != OBJC_SYNC_SUCCESS) MutexInvalidAccess(thr, pc, addr);
+  return result;
 }
 
 // On macOS, libc++ is always linked dynamically, so intercepting works the


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55959.179167.patch
Type: text/x-patch
Size: 2437 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181220/763fb323/attachment.bin>