tsan: minor MetaMap tweaks

[dvyukov]

Summary

1. Add some comments.
2. Use kInvalidStackID instead of literal 0.
3. Add more LIKELY/UNLIKELY.

[melver]

compiler-rt/lib/tsan/rtl/tsan_sync.h

49

s/decriptor/descriptor/
?

[vitalybuka]

Add some comments.
Use kInvalidStackID instead of literal 0.
Add more LIKELY/UNLIKELY.

Descriptions says it's 3 separate patches which even do not need a review :)

[dvyukov]

s/decriptor/descriptor/

[dvyukov]

In D107371#2922890, @vitalybuka wrote:

Add some comments.
Use kInvalidStackID instead of literal 0.
Add more LIKELY/UNLIKELY.

Descriptions says it's 3 separate patches which even do not need a review :)

Would you prefer smaller changes in future?
I am just trying to sort out a 6000-line diff, so thought some aggregation will reduce overheads on both sides.

compiler-rt/lib/tsan/rtl/tsan_sync.h

enum MutexFlags {
   MutexFlagDoPreLockOnPostLock = 1 << 29,
   // Must list all mutex creation flags.
   MutexCreationFlagMask        = MutexFlagLinkerInit |
                                  MutexFlagWriteReentrant |
                                  MutexFlagReadReentrant |
                                  MutexFlagNotStatic,
 };
 
+// SyncVar is a descriptor of a user synchronization object
+// (mutex or an atomic variable).
 struct SyncVar {
   SyncVar();
 
   uptr addr;  // overwritten by DenseSlabAlloc freelist
   Mutex mtx;
   u64 uid;  // Globally unique id.
   StackID creation_stack_id;
   Tid owner_tid;  // Set only by exclusive owners.
void UpdateFlags(u32 flagz) {
     if (current & MutexCreationFlagMask)
       return;
     // Note: this can be called from MutexPostReadLock which holds only read
     // lock on the SyncVar.
     atomic_store_relaxed(&flags, current | (flagz & MutexCreationFlagMask));
   }
 };
 
-/* MetaMap allows to map arbitrary user pointers onto various descriptors.
-   Currently it maps pointers to heap block descriptors and sync var descs.
-   It uses 1/2 direct shadow, see tsan_platform.h.
-*/
+// MetaMap maps app addresses to heap block (MBlock) and sync var (SyncVar)
+// descriptors. It uses 1/2 direct shadow, see tsan_platform.h for the mapping.
 class MetaMap {
  public:
   MetaMap();
 
   void AllocBlock(ThreadState *thr, uptr pc, uptr p, uptr sz);
   uptr FreeBlock(Processor *proc, uptr p);
   bool FreeRange(Processor *proc, uptr p, uptr sz);
   void ResetRange(Processor *proc, uptr p, uptr sz);

compiler-rt/lib/tsan/rtl/tsan_sync.cpp

 SyncVar::SyncVar() : mtx(MutexTypeSyncVar) { Reset(0); }
 
 void SyncVar::Init(ThreadState *thr, uptr pc, uptr addr, u64 uid,
                    bool save_stack) {
   this->addr = addr;
   this->uid = uid;
   this->next = 0;
 
-  creation_stack_id = 0;
+  creation_stack_id = kInvalidStackID;
   if (save_stack && !SANITIZER_GO)  // Go does not use them
     creation_stack_id = CurrentStackId(thr, pc);
   if (common_flags()->detect_deadlocks)
     DDMutexInit(thr, pc, this);
 }
 
 void SyncVar::Reset(Processor *proc) {
   uid = 0;
-  creation_stack_id = 0;
+  creation_stack_id = kInvalidStackID;
   owner_tid = kInvalidTid;
   last_lock = 0;
   recursion = 0;
   atomic_store_relaxed(&flags, 0);
 
   if (proc == 0) {
     CHECK_EQ(clock.size(), 0);
     CHECK_EQ(read_clock.size(), 0);
for (u32 idx = idx0; idx && !(idx & kFlagBlock);) {
           sync_alloc_.Free(&thr->proc()->sync_cache, myidx);
         }
         return s;
       }
       idx = s->next;
     }
     if (!create)
       return nullptr;
-    if (*meta != idx0) {
+    if (UNLIKELY(*meta != idx0)) {
       idx0 = *meta;
       continue;
     }
 
-    if (myidx == 0) {
+    if (LIKELY(myidx == 0)) {
       const u64 uid = atomic_fetch_add(&uid_gen_, 1, memory_order_relaxed);
       myidx = sync_alloc_.Alloc(&thr->proc()->sync_cache);
       mys = sync_alloc_.Map(myidx);
       mys->Init(thr, pc, addr, uid, save_stack);
     }
     mys->next = idx0;
     if (atomic_compare_exchange_strong((atomic_uint32_t*)meta, &idx0,
         myidx | kFlagSync, memory_order_release)) {