[scudo] Use template specialization on Quarantine to avoid zero-length array

[ddcc]

Summary

Use a separate templated QuarantineBlocks class to avoid a zero-length array

[vitalybuka]

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

365–375

[vitalybuka]

LGTM with suggested change

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

365–375

UNREACHABLE("Not implemented!");

[mcgrathr]

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

372

Explicit specializations must appear at namespace/top-level scope, not inside the class.
You can still specialize the private class, you just have to do it with the scoped name rather than from inside the scope.
GCC does not accept this. Its code quotes the spec thusly:

/* [temp.expl.spec]                                                           
                                                                               
    An explicit specialization shall be declared in the namespace of           
    which the template is a member, or, for member templates, in the           
    namespace of which the enclosing class or enclosing class                  
    template is a member.  An explicit specialization of a member              
    function, member class or static data member of a class template           
    shall be declared in the namespace of which the class template             
    is a member.  */

[ddcc]

Move definitions out of class

[vitalybuka]

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

116–136

Maybe this to avoid exposing CachedBlock

[ddcc]

Add another template argument to avoid exposing private struct

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

void mapSecondary(Options Options, uptr CommitBase, uptr CommitSize,
   } else {
     map(reinterpret_cast<void *>(CommitBase), CommitSize, "scudo:secondary",
         MAP_RESIZABLE | (useMemoryTagging<Config>(Options) ? MAP_MEMTAG : 0) |
             Flags,
         Data);
   }
 }
 
 template <typename Config> class MapAllocatorCache {
 public:
   // Ensure the default maximum specified fits the array.
   static_assert(Config::SecondaryCacheDefaultMaxEntriesCount <=
                     Config::SecondaryCacheEntriesArraySize,
                 "");
 
   void init(s32 ReleaseToOsInterval) {
     DCHECK_EQ(EntriesCount, 0U);
     setOption(Option::MaxCacheEntriesCount,
               static_cast<sptr>(Config::SecondaryCacheDefaultMaxEntriesCount));
     setOption(Option::MaxCacheEntrySize,
               static_cast<sptr>(Config::SecondaryCacheDefaultMaxEntrySize));
     setOption(Option::ReleaseInterval, static_cast<sptr>(ReleaseToOsInterval));
   }
 
   void store(Options Options, LargeBlock::Header *H) {
     if (!canCache(H->CommitSize))
       return unmap(H);
 
     bool EntryCached = false;
     bool EmptyCache = false;
     const s32 Interval = atomic_load_relaxed(&ReleaseToOsIntervalMs);
     const u64 Time = getMonotonicTime();
     const u32 MaxCount = atomic_load_relaxed(&MaxEntriesCount);
     CachedBlock Entry;
     Entry.CommitBase = H->CommitBase;
     Entry.CommitSize = H->CommitSize;
     Entry.MapBase = H->MapBase;
do {
         // the quarantine or the cache because the permissions would be wrong so
         // just unmap it.
         break;
       }
       if (Config::SecondaryCacheQuarantineSize &&
           useMemoryTagging<Config>(Options)) {
         QuarantinePos =
             (QuarantinePos + 1) % Max(Config::SecondaryCacheQuarantineSize, 1u);
-        if (!Quarantine[QuarantinePos].CommitBase) {
-          Quarantine[QuarantinePos] = Entry;
+        if (!Quarantine.get(QuarantinePos)->CommitBase) {
+          *Quarantine.get(QuarantinePos) = Entry;
           return;
         }
-        CachedBlock PrevEntry = Quarantine[QuarantinePos];
-        Quarantine[QuarantinePos] = Entry;
+        CachedBlock PrevEntry = *Quarantine.get(QuarantinePos);
+        *Quarantine.get(QuarantinePos) = Entry;
         if (OldestTime == 0)
           OldestTime = Entry.Time;
         Entry = PrevEntry;
       }
       if (EntriesCount >= MaxCount) {
         if (IsFullEvents++ == 4U)
           EmptyCache = true;
       } else {
bool setOption(Option O, sptr Value) {
     return true;
   }
 
   void releaseToOS() { releaseOlderThan(UINT64_MAX); }
 
   void disableMemoryTagging() {
     ScopedLock L(Mutex);
     for (u32 I = 0; I != Config::SecondaryCacheQuarantineSize; ++I) {
-      if (Quarantine[I].CommitBase) {
-        unmap(reinterpret_cast<void *>(Quarantine[I].MapBase),
-              Quarantine[I].MapSize, UNMAP_ALL, &Quarantine[I].Data);
-        Quarantine[I].CommitBase = 0;
+      if (Quarantine.get(I)->CommitBase) {
+        unmap(reinterpret_cast<void *>(Quarantine.get(I)->MapBase),
+              Quarantine.get(I)->MapSize, UNMAP_ALL, &Quarantine.get(I)->Data);
+        Quarantine.get(I)->CommitBase = 0;
       }
     }
     const u32 MaxCount = atomic_load_relaxed(&MaxEntriesCount);
     for (u32 I = 0; I < MaxCount; I++)
       if (Entries[I].CommitBase)
         setMemoryPermission(Entries[I].CommitBase, Entries[I].CommitSize, 0,
                             &Entries[I].Data);
     QuarantinePos = -1U;
struct CachedBlock {
     uptr CommitSize;
     uptr MapBase;
     uptr MapSize;
     uptr BlockBegin;
     [[no_unique_address]] MapPlatformData Data;
     u64 Time;
   };
 
+  // Template specialization to avoid producing zero-length array
+  template <size_t Size> class QuarantineBlocks {
+  public:
+    CachedBlock *get(uptr Idx) { return &Blocks[Idx]; }
+  private:
+    CachedBlock Blocks[Size];
+  };
+  template <> class QuarantineBlocks<0> {
+  public:
+    CachedBlock *get(uptr UNUSED Idx) { return nullptr; }
+  };
+
   void releaseIfOlderThan(CachedBlock &Entry, u64 Time) {
     if (!Entry.CommitBase || !Entry.Time)
       return;
     if (Entry.Time > Time) {
       if (OldestTime == 0 || Entry.Time < OldestTime)
         OldestTime = Entry.Time;
       return;
     }
     releasePagesToOS(Entry.CommitBase, 0, Entry.CommitSize, &Entry.Data);
     Entry.Time = 0;
   }
 
   void releaseOlderThan(u64 Time) {
     ScopedLock L(Mutex);
     if (!EntriesCount || OldestTime == 0 || OldestTime > Time)
       return;
     OldestTime = 0;
     for (uptr I = 0; I < Config::SecondaryCacheQuarantineSize; I++)
-      releaseIfOlderThan(Quarantine[I], Time);
+      releaseIfOlderThan(*Quarantine.get(I), Time);
     for (uptr I = 0; I < Config::SecondaryCacheEntriesArraySize; I++)
       releaseIfOlderThan(Entries[I], Time);
   }
 
   HybridMutex Mutex;
   u32 EntriesCount = 0;
   u32 QuarantinePos = 0;
   atomic_u32 MaxEntriesCount = {};
   atomic_uptr MaxEntrySize = {};
   u64 OldestTime = 0;
   u32 IsFullEvents = 0;
   atomic_s32 ReleaseToOsIntervalMs = {};
 
   CachedBlock Entries[Config::SecondaryCacheEntriesArraySize] = {};
-  CachedBlock Quarantine[Config::SecondaryCacheQuarantineSize] = {};
+  QuarantineBlocks<Config::SecondaryCacheQuarantineSize> Quarantine = {};
 };
 
 template <typename Config> class MapAllocator {
 public:
   void init(GlobalStats *S, s32 ReleaseToOsInterval = -1) {
     DCHECK_EQ(AllocatedBytes, 0U);
     DCHECK_EQ(FreedBytes, 0U);
     Cache.init(ReleaseToOsInterval);