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

[Dan Liew via Phabricator via llvm-commits]

delcypher accepted this revision.
delcypher added a comment.

Other than minor suggestions the basic idea seems okay for now.



================
Comment at: lib/tsan/rtl/tsan_interceptors_mac.cc:299
+// contains data in the pointers bits instead)?
+static bool IsTaggedPointer(void *obj) {
+  const uptr kPossibleTaggedBits = 0x8000000000000001ull;
----------------
Perhaps the name should be `IsTaggedObjCPointer`?


================
Comment at: lib/tsan/rtl/tsan_interceptors_mac.cc:308
+static uptr SyncAddressForTaggedPointer(void *obj) {
+  (void)obj;
+  static u64 addr;
----------------
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.


https://reviews.llvm.org/D49707