[PATCH] D56238: [TSan] Support Objective-C @synchronized with tagged pointers

[Julian Lettner via Phabricator via llvm-commits]

yln updated this revision to Diff 180112.
yln retitled this revision from "[TSan] Support Objective-C @synchronized(ob) where obj is a tagged pointer" to "[TSan] Support Objective-C @synchronized with tagged pointers".
yln added a comment.

Improve/extend comments.


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56238/new/

https://reviews.llvm.org/D56238

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


Index: test/tsan/Darwin/objc-synchronize-cycle-tagged.mm
===================================================================
--- test/tsan/Darwin/objc-synchronize-cycle-tagged.mm
+++ test/tsan/Darwin/objc-synchronize-cycle-tagged.mm
@@ -1,7 +1,6 @@
 // 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>
 
@@ -12,7 +11,7 @@
 
 int main(int argc, char* argv[]) {
   assert(argc == 2);
-  int arg = atoi(argv[0]);
+  int arg = atoi(argv[1]);
 
   @autoreleasepool {
     NSObject* obj = [NSObject new];
Index: lib/tsan/rtl/tsan_interceptors_mac.cc
===================================================================
--- lib/tsan/rtl/tsan_interceptors_mac.cc
+++ lib/tsan/rtl/tsan_interceptors_mac.cc
@@ -19,6 +19,7 @@
 #include "tsan_interceptors.h"
 #include "tsan_interface.h"
 #include "tsan_interface_ann.h"
+#include "sanitizer_common/sanitizer_addrhashmap.h"
 
 #include <libkern/OSAtomic.h>
 #include <objc/objc-sync.h>
@@ -295,26 +296,36 @@
 
 #endif  // #if defined(__has_include) && __has_include(<xpc/xpc.h>)
 
-// Is the Obj-C object a tagged pointer (i.e. isn't really a valid pointer and
-// contains data in the pointers bits instead)?
-static bool IsTaggedObjCPointer(void *obj) {
+// Determines whether the Obj-C object pointer is a tagged pointer. Tagged
+// pointers encode the object data directly in their pointer bits and do not
+// have an associated memory allocation. The Obj-C runtime uses tagged pointers
+// to transparently optimize small objects.
+static bool IsTaggedObjCPointer(id obj) {
   const uptr kPossibleTaggedBits = 0x8000000000000001ull;
   return ((uptr)obj & kPossibleTaggedBits) != 0;
 }
 
-// Return an address on which we can synchronize (Acquire and Release) for a
-// Obj-C tagged pointer (which is not a valid pointer). Ideally should be a
-// derived address from 'obj', but for now just return the same global address.
-// TODO(kubamracek): Return different address for different pointers.
-static uptr SyncAddressForTaggedPointer(void *obj) {
-  (void)obj;
-  static u64 addr;
-  return (uptr)&addr;
+// Returns an address which can be used to inform TSan about synchronization
+// points (MutexLock/Unlock). The TSan infrastructure expects this to be a valid
+// address in the process space. So we simply do a small allocation here to
+// obtain a stable address (the array backing the hash map can change). The
+// memory is never free'd (leaked) and allocation and locking are slow, but this
+// code only runs for @synchronized with tagged pointers, which is very rare.
+static uptr SyncAddressForTaggedPointer(id obj) {
+  typedef AddrHashMap<uptr, 5> Map;
+  static Map Addresses;
+  Map::Handle h(&Addresses, (uptr) obj);
+  if (h.created()) {
+    *h = (uptr) InternalAlloc(/*size=*/1);
+  }
+  return *h;
 }
 
-// Address on which we can synchronize for an Objective-C object. Supports
-// tagged pointers.
-static uptr SyncAddressForObjCObject(void *obj) {
+// Returns an address on which we can synchronize given an Obj-C object pointer.
+// For normal object pointers, this is just the address of the object in memory.
+// Tagged pointers are not backed by an actual memory allocation, so we need to
+// synthesize a synchronization address.
+static uptr SyncAddressForObjCObject(id obj) {
   if (IsTaggedObjCPointer(obj)) return SyncAddressForTaggedPointer(obj);
   return (uptr)obj;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D56238.180112.patch
Type: text/x-patch
Size: 3586 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190103/07b0f606/attachment.bin>