[PATCH] D56238: [TSan] Support Objective-C @synchronized(ob) where obj is a tagged pointer

[Julian Lettner via Phabricator via llvm-commits]

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

Objective-C employs tagged pointers, that is, small objects/values may be encoded directly in the pointer bits. The resulting pointer is not backed by an allocation/does not point to a valid memory. TSan infrastructure requires a valid address for `Acquire/Release` and `Mutex{Lock/Unlock}`.
This patch establishes such a mapping via a "dummy allocation" for each encountered tagged pointer value.


Repository:
  rCRT Compiler Runtime

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>
@@ -303,13 +304,18 @@
 }
 
 // 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.
+// Obj-C tagged pointer (which is not a valid pointer). We allocate some memory
+// to obtain a stable address (the backing array in the hash map could change).
+// The memory is never free'd and allocation and locking is slow, but this code
+// only runs for @synchronized with tagged pointers, which should be very rare.
 static uptr SyncAddressForTaggedPointer(void *obj) {
-  (void)obj;
-  static u64 addr;
-  return (uptr)&addr;
+  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


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