diff options
author | Julian Lettner <jlettner@apple.com> | 2019-01-02 20:10:30 +0000 |
---|---|---|
committer | Julian Lettner <jlettner@apple.com> | 2019-01-02 20:10:30 +0000 |
commit | 8860149190641b83cd61e23f41909313963e352b (patch) | |
tree | ff171be3824bf52a61a8dd36a371bf3445c41639 | |
parent | 92f96584f7e27b3bc6c9dbc2442a8ef411a30778 (diff) | |
download | compiler-rt-8860149190641b83cd61e23f41909313963e352b.tar.gz |
[TSan] Enable detection of lock-order-inversions for Objective-C @synchronized
Summary:
@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.
Reviewers: dcoughlin, dvyukov, kubamracek, delcypher
Reviewed By: dvyukov
Subscribers: llvm-commits, #sanitizers
Tags: #sanitizers
Differential Revision: https://reviews.llvm.org/D55959
git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@350258 91177308-0d34-0410-b5e6-96231b3b80d8
-rw-r--r-- | lib/tsan/rtl/tsan_interceptors_mac.cc | 19 | ||||
-rw-r--r-- | test/tsan/Darwin/objc-synchronize-cycle-tagged.mm | 43 | ||||
-rw-r--r-- | test/tsan/Darwin/objc-synchronize-cycle.mm | 31 | ||||
-rw-r--r-- | test/tsan/Darwin/objc-synchronize-nested-recursive.mm | 35 |
4 files changed, 123 insertions, 5 deletions
diff --git a/lib/tsan/rtl/tsan_interceptors_mac.cc b/lib/tsan/rtl/tsan_interceptors_mac.cc index 5e8b58fc2..2d105bf93 100644 --- a/lib/tsan/rtl/tsan_interceptors_mac.cc +++ b/lib/tsan/rtl/tsan_interceptors_mac.cc @@ -21,6 +21,7 @@ #include "tsan_interface_ann.h" #include <libkern/OSAtomic.h> +#include <objc/objc-sync.h> #if defined(__has_include) && __has_include(<xpc/xpc.h>) #include <xpc/xpc.h> @@ -318,17 +319,25 @@ static uptr SyncAddressForObjCObject(void *obj) { return (uptr)obj; } -TSAN_INTERCEPTOR(int, objc_sync_enter, void *obj) { +TSAN_INTERCEPTOR(int, objc_sync_enter, id obj) { SCOPED_TSAN_INTERCEPTOR(objc_sync_enter, obj); + if (!obj) return REAL(objc_sync_enter)(obj); + uptr addr = SyncAddressForObjCObject(obj); + MutexPreLock(thr, pc, addr, MutexFlagWriteReentrant); int result = REAL(objc_sync_enter)(obj); - if (obj) Acquire(thr, pc, SyncAddressForObjCObject(obj)); + CHECK_EQ(result, OBJC_SYNC_SUCCESS); + MutexPostLock(thr, pc, addr, MutexFlagWriteReentrant); return result; } -TSAN_INTERCEPTOR(int, objc_sync_exit, void *obj) { +TSAN_INTERCEPTOR(int, objc_sync_exit, id 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 diff --git a/test/tsan/Darwin/objc-synchronize-cycle-tagged.mm b/test/tsan/Darwin/objc-synchronize-cycle-tagged.mm new file mode 100644 index 000000000..a0c132661 --- /dev/null +++ b/test/tsan/Darwin/objc-synchronize-cycle-tagged.mm @@ -0,0 +1,43 @@ +// RUN: %clangxx_tsan %s -o %t -framework Foundation -fobjc-arc %darwin_min_target_with_full_runtime_arc_support +// RUN: %run %t 6 2>&1 | FileCheck %s --check-prefix=SIX +// RUN: not %run %t 7 2>&1 | FileCheck %s --check-prefix=SEVEN +// XFAIL: * + +#import <Foundation/Foundation.h> + +static bool isTaggedPtr(id obj) { + uintptr_t ptr = (uintptr_t) obj; + return (ptr & 0x8000000000000001ull) != 0; +} + +int main(int argc, char* argv[]) { + assert(argc == 2); + int arg = atoi(argv[0]); + + @autoreleasepool { + NSObject* obj = [NSObject new]; + NSObject* num1 = @7; + NSObject* num2 = [NSNumber numberWithInt:arg]; + + assert(!isTaggedPtr(obj)); + assert(isTaggedPtr(num1) && isTaggedPtr(num2)); + + // obj -> num1 (includes num2) + @synchronized(obj) { + @synchronized(num1) { + } + } + + // num2 -> obj1 + @synchronized(num2) { + @synchronized(obj) { +// SEVEN: ThreadSanitizer: lock-order-inversion (potential deadlock) + } + } + } + + NSLog(@"PASS"); +// SIX-NOT: ThreadSanitizer +// SIX: PASS + return 0; +} diff --git a/test/tsan/Darwin/objc-synchronize-cycle.mm b/test/tsan/Darwin/objc-synchronize-cycle.mm new file mode 100644 index 000000000..fb8163115 --- /dev/null +++ b/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; +} diff --git a/test/tsan/Darwin/objc-synchronize-nested-recursive.mm b/test/tsan/Darwin/objc-synchronize-nested-recursive.mm new file mode 100644 index 000000000..ab6643e5c --- /dev/null +++ b/test/tsan/Darwin/objc-synchronize-nested-recursive.mm @@ -0,0 +1,35 @@ +// RUN: %clangxx_tsan %s -o %t -framework Foundation -fobjc-arc %darwin_min_target_with_full_runtime_arc_support +// RUN: %run %t 2>&1 | FileCheck %s + +#import <Foundation/Foundation.h> + +int main() { + @autoreleasepool { + NSObject* obj1 = [NSObject new]; + NSObject* obj2 = [NSObject new]; + + @synchronized(obj1) { + @synchronized(obj1) { + NSLog(@"nested 1-1"); +// CHECK: nested 1-1 + } + } + + @synchronized(obj1) { + @synchronized(obj2) { + @synchronized(obj1) { + @synchronized(obj2) { + NSLog(@"nested 1-2-1-2"); +// CHECK: nested 1-2-1-2 + } + } + } + } + + } + + NSLog(@"PASS"); +// CHECK-NOT: ThreadSanitizer +// CHECK: PASS + return 0; +} |