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

[George Karpenkov via Phabricator via llvm-commits]

george.karpenkov added inline comments.


================
Comment at: lib/tsan/rtl/tsan_interceptors_mac.cc:297
 
+bool IsTaggedPointer(void *obj) {
+  const uptr kPossibleTaggedBits = 0x8000000000000001ull;
----------------
can we have a short docstring for both functions? Also, `isTaggedObjCPointer`?


================
Comment at: lib/tsan/rtl/tsan_interceptors_mac.cc:299
+  const uptr kPossibleTaggedBits = 0x8000000000000001ull;
+  if (((uptr)obj & kPossibleTaggedBits) != 0) return true;
+  return false;
----------------
` return ((uptr)obj & kPossibleTaggedBits) != 0` ?


================
Comment at: lib/tsan/rtl/tsan_interceptors_mac.cc:303
+
+uptr SyncAddrForTaggedPointer(void *obj) {
+  (void)obj;
----------------
what's the point of taking in the argument then?


================
Comment at: lib/tsan/rtl/tsan_interceptors_mac.cc:312
   int result = REAL(objc_sync_enter)(obj);
-  if (obj) Acquire(thr, pc, (uptr)obj);
+  uptr sync = IsTaggedPointer(obj) ? SyncAddrForTaggedPointer(obj) : (uptr)obj;
+  if (obj) Acquire(thr, pc, sync);
----------------
can we extract this line into a function with a name explaining it's semantics?


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D49707