[scudo] set/check invalid cache entries

[frs513]

Summary

made checking for invalid cache entries and setting invalid cache
entries more implicit and clear.

[frs513]

clang format code

[frs513]

passed cached block entry by reference into set header

[cferris]

compiler-rt/lib/scudo/standalone/secondary.h

85

Sorry, really small nit pick here, but I think just naming this invalidate is better. As you can see below you have code like:

Entry[].invalidateEntry();

Where it seems to make more sense as:

Entry[].invalidate();

Since you don't use isValidEntry() in the other name it feels better to have them both have Entry or neither have Entry. I'd prefer neither have Entry.

[frs513]

changed name of invalidateEntry() to invalidate() to be more concise and less repetitive.

[cferris]

LGTM.

[frs513]

rebased and resolved merge conflicts

compiler-rt/lib/scudo/standalone/secondary.h

 
 namespace {
 struct CachedBlock {
   uptr CommitBase = 0;
   uptr CommitSize = 0;
   uptr BlockBegin = 0;
   MemMapT MemMap = {};
   u64 Time = 0;
+
+  bool isValid() { return CommitBase != 0; }
+
+  void invalidate() { CommitBase = 0; }
 };
 } // namespace
 
 template <typename Config> class MapAllocatorNoCache {
 public:
   void init(UNUSED s32 ReleaseToOsInterval) {}
   bool retrieve(UNUSED uptr Size, UNUSED CachedBlock &Entry) { return false; }
   void store(UNUSED Options Options, LargeBlock::Header *H) { unmap(H); }
public:
 
   void getStats(ScopedString *Str) {
     ScopedLock L(Mutex);
     Str->append("Stats: MapAllocatorCache: EntriesCount: %d, "
                 "MaxEntriesCount: %u, MaxEntrySize: %zu\n",
                 EntriesCount, atomic_load_relaxed(&MaxEntriesCount),
                 atomic_load_relaxed(&MaxEntrySize));
     for (CachedBlock Entry : Entries) {
-      if (!Entry.CommitBase)
+      if (!Entry.isValid())
         continue;
       Str->append("StartBlockAddress: 0x%zx, EndBlockAddress: 0x%zx, "
                   "BlockSize: %zu\n",
                   Entry.CommitBase, Entry.CommitBase + Entry.CommitSize,
                   Entry.CommitSize);
     }
   }
 
do {
         // read Options and when we locked Mutex. We can't insert our entry into
         // the quarantine or the cache because the permissions would be wrong so
         // just unmap it.
         break;
       }
       if (CacheConfig::QuarantineSize && useMemoryTagging<Config>(Options)) {
         QuarantinePos =
             (QuarantinePos + 1) % Max(CacheConfig::QuarantineSize, 1u);
-        if (!Quarantine[QuarantinePos].CommitBase) {
+        if (!Quarantine[QuarantinePos].isValid()) {
           Quarantine[QuarantinePos] = Entry;
           return;
         }
         CachedBlock PrevEntry = Quarantine[QuarantinePos];
         Quarantine[QuarantinePos] = Entry;
         if (OldestTime == 0)
           OldestTime = Entry.Time;
         Entry = PrevEntry;
       }
       if (EntriesCount >= MaxCount) {
         if (IsFullEvents++ == 4U)
           EmptyCache = true;
       } else {
         for (u32 I = 0; I < MaxCount; I++) {
-          if (Entries[I].CommitBase)
+          if (Entries[I].isValid())
             continue;
           if (I != 0)
             Entries[I] = Entries[0];
           Entries[0] = Entry;
           EntriesCount++;
           if (OldestTime == 0)
             OldestTime = Entry.Time;
           EntryCached = true;
public:
   bool retrieve(uptr Size, CachedBlock &Entry) EXCLUDES(Mutex) {
     const u32 MaxCount = atomic_load_relaxed(&MaxEntriesCount);
     bool Found = false;
     {
       ScopedLock L(Mutex);
       if (EntriesCount == 0)
         return false;
       for (u32 I = 0; I < MaxCount; I++) {
-        if (!Entries[I].CommitBase)
+        if (!Entries[I].isValid())
           continue;
         if (Size > Entries[I].CommitSize)
           continue;
         Found = true;
         Entry = Entries[I];
-        Entries[I].CommitBase = 0;
+        Entries[I].invalidate();
         EntriesCount--;
         break;
       }
     }
     return Found;
   }
 
   bool canCache(uptr Size) {
bool setOption(Option O, sptr Value) {
     return true;
   }
 
   void releaseToOS() { releaseOlderThan(UINT64_MAX); }
 
   void disableMemoryTagging() EXCLUDES(Mutex) {
     ScopedLock L(Mutex);
     for (u32 I = 0; I != CacheConfig::QuarantineSize; ++I) {
-      if (Quarantine[I].CommitBase) {
+      if (Quarantine[I].isValid()) {
         MemMapT &MemMap = Quarantine[I].MemMap;
         MemMap.unmap(MemMap.getBase(), MemMap.getCapacity());
-        Quarantine[I].CommitBase = 0;
+        Quarantine[I].invalidate();
       }
     }
     const u32 MaxCount = atomic_load_relaxed(&MaxEntriesCount);
     for (u32 I = 0; I < MaxCount; I++) {
-      if (Entries[I].CommitBase) {
+      if (Entries[I].isValid()) {
         Entries[I].MemMap.setMemoryPermission(Entries[I].CommitBase,
                                               Entries[I].CommitSize, 0);
       }
     }
     QuarantinePos = -1U;
   }
 
   void disable() NO_THREAD_SAFETY_ANALYSIS { Mutex.lock(); }
 
   void enable() NO_THREAD_SAFETY_ANALYSIS { Mutex.unlock(); }
 
   void unmapTestOnly() { empty(); }
 
 private:
   void empty() {
     MemMapT MapInfo[CacheConfig::EntriesArraySize];
     uptr N = 0;
     {
       ScopedLock L(Mutex);
       for (uptr I = 0; I < CacheConfig::EntriesArraySize; I++) {
-        if (!Entries[I].CommitBase)
+        if (!Entries[I].isValid())
           continue;
         MapInfo[N] = Entries[I].MemMap;
-        Entries[I].CommitBase = 0;
+        Entries[I].invalidate();
         N++;
       }
       EntriesCount = 0;
       IsFullEvents = 0;
     }
     for (uptr I = 0; I < N; I++) {
       MemMapT &MemMap = MapInfo[I];
       MemMap.unmap(MemMap.getBase(), MemMap.getCapacity());
     }
   }
 
   void releaseIfOlderThan(CachedBlock &Entry, u64 Time) REQUIRES(Mutex) {
-    if (!Entry.CommitBase || !Entry.Time)
+    if (!Entry.isValid() || !Entry.Time)
       return;
     if (Entry.Time > Time) {
       if (OldestTime == 0 || Entry.Time < OldestTime)
         OldestTime = Entry.Time;
       return;
     }
     Entry.MemMap.releasePagesToOS(Entry.CommitBase, Entry.CommitSize);
     Entry.Time = 0;