[PATCH] D49707: [tsan] Fix crash in objc_sync_enter/objc_sync_exit when using an Obj-C tagged pointer

[Kuba (Brecka) Mracek via Phabricator via llvm-commits]

kubamracek added inline comments.


================
Comment at: lib/tsan/rtl/tsan_interceptors_mac.cc:308
+static uptr SyncAddressForTaggedPointer(void *obj) {
+  (void)obj;
+  static u64 addr;
----------------
delcypher wrote:
> There probably should be a `TODO(kubamracek)` or `FIXME` in here.
> 
> Do we even want to do `Acquire()` and `Release()` for tagged pointers? Even if we had a perfect implementation of `SyncAddressForTaggedPointer()` that still seems wrong because we could have two independent instances of a tagged pointer
> (e.g. create two separate NSString with same value) and those shouldn't really be considered as the same object, but they would
> be considered the same because their tagged pointer value is the same.
Yes, the entire idea of using value types (strings, integeres, timestamps) as synchronization objects is weird and most likely not what you intended anyway. Therefore this patch is really just to avoid crashing and to mirror what libobjc is actually doing when using `@synchronize` on a tagged pointer (it does actually establish synchronization if the pointer bits are exactly the same).


https://reviews.llvm.org/D49707