[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

378–388

[vitalybuka]

LGTM with suggested change

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

378–388

UNREACHABLE("Not implemented!");

[mcgrathr]

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

385

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 specialization to avoid producing zero-length array
+template <typename T, size_t Size> class NonZeroLengthArray {
+public:
+  T &operator[](uptr Idx) { return values[Idx]; }
+
+private:
+  T values[Size];
+};
+template <typename T> class NonZeroLengthArray<T, 0> {
+public:
+  T &operator[](uptr UNUSED Idx) { UNREACHABLE("Unsupported!"); }
+};
+
 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));
   }
 
struct CachedBlock {
     uptr CommitSize;
     uptr MapBase;
     uptr MapSize;
     uptr BlockBegin;
     [[no_unique_address]] MapPlatformData Data;
     u64 Time;
   };
 
   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);
     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] = {};
+  NonZeroLengthArray<CachedBlock, 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);