[scudo] Use our own combined allocator

[cryptoad]

Summary

The reasoning behind this change is twofold:

• the current combined allocator (sanitizer_allocator_combined.h) implements features that are not relevant for Scudo, making some code redundant, and some restrictions not pertinent (alignments for example). This forced us to do some weird things between the frontend and our secondary to make things work;
• we have enough information to be able to know if a chunk will be serviced by the Primary or Secondary, allowing us to avoid extraneous calls to functions such as PointerIsMine or CanAllocate.

As a result, the new scudo-specific combined allocator is very straightforward,
and allows us to remove some now unnecessary code both in the frontend and the
secondary. Unused functions have been left in as unimplemented for now.

It turns out to also be a sizeable performance gain (3% faster in some Android
memory_replay benchmarks, doing some more on other platforms).

[cryptoad]

Small change in the Secondary.

[dvyukov]

Looks fine to me. Aleksey, do you want to take a look?

[alekseyshl]

lib/scudo/scudo_allocator.cpp

367

Add space before ":"

lib/scudo/scudo_allocator_combined.h

148

Are those UNIMPLEMENTED going to be implemented at some point?

[cryptoad]

lib/scudo/scudo_allocator_combined.h

148

To be perfectly honest, I am not sure.
I think this will mostly depend on needs expressed by the users, but there is no specific timeframe.
For example, they might be a need to walk all the allocator chunks for debugging purposes, which would involve implementing a bunch of them both on the Combined and the Secondary.

[cryptoad]

Addressing Aleksey's comment.

[alekseyshl]

I do not like the code duplication and all those UNIMPLEMENTED in ScudoCombinedAllocator and around, it's hard to reason why there's so much unused API (why is it declared then?) Also I expected to see some simplification in CombinedAllocator<>, none was necessary, maybe it means that CombinedAllocator has some potential for improvement, to be more flexible to address Scudo needs?

All that said, 3% performance gain is too big to pass on, if CombinedAllocator is not possible or too complicated to adjust for Scudo, I'm ok with this change.

[cryptoad]

In D33007#751634, @alekseyshl wrote:

I do not like the code duplication and all those UNIMPLEMENTED in ScudoCombinedAllocator and around, it's hard to reason why there's so much unused API (why is it declared then?) Also I expected to see some simplification in CombinedAllocator<>, none was necessary, maybe it means that CombinedAllocator has some potential for improvement, to be more flexible to address Scudo needs?

It's fair. I'll send out a revision removing all the unneeded code from the combined and the secondary.
As for modifying the original combined, I do not think it's doable without a significant increase in complexity and branches, which is what I am trying to get rid of.
A lot of the difference comes from the fact that the frontend of the other sanitizers defer a lot to the original combined (alignment, realloc, memset, etc), while our frontend does all that work.

[cryptoad]

Removing all the unused code from the Secondary and the Combined allocator.
The resulting Combined is tiny.
I also rearranged some code to make it clearer (at least to me).

lib/scudo/scudo_allocator.h

 typedef TwoLevelByteMap<(NumRegions >> 12), 1 << 12> ByteMap;
 # endif  // SANITIZER_WORDSIZE
 typedef DefaultSizeClassMap SizeClassMap;
 typedef SizeClassAllocator32<0, SANITIZER_MMAP_RANGE_SIZE, 0, SizeClassMap,
     RegionSizeLog, ByteMap> PrimaryAllocator;
 #endif  // SANITIZER_CAN_USE_ALLOCATOR64
 
 #include "scudo_allocator_secondary.h"
+#include "scudo_allocator_combined.h"
 
 typedef SizeClassAllocatorLocalCache<PrimaryAllocator> AllocatorCache;
 typedef ScudoLargeMmapAllocator SecondaryAllocator;
-typedef CombinedAllocator<PrimaryAllocator, AllocatorCache, SecondaryAllocator>
-    ScudoBackendAllocator;
+typedef ScudoCombinedAllocator<PrimaryAllocator, AllocatorCache,
+    SecondaryAllocator> ScudoBackendAllocator;
 
 void initScudo();
 
 void *scudoMalloc(uptr Size, AllocType Type);
 void scudoFree(void *Ptr, AllocType Type);
 void scudoSizedFree(void *Ptr, uptr Size, AllocType Type);
 void *scudoRealloc(void *Ptr, uptr Size);
 void *scudoCalloc(uptr NMemB, uptr Size);

lib/scudo/scudo_allocator.cpp

struct ScudoChunk : UnpackedHeader {
   void *getAllocBeg(UnpackedHeader *Header) {
     return reinterpret_cast<void *>(
         reinterpret_cast<uptr>(this) - (Header->Offset << MinAlignmentLog));
   }
 
   // Returns the usable size for a chunk, meaning the amount of bytes from the
   // beginning of the user data to the end of the backend allocated chunk.
   uptr getUsableSize(UnpackedHeader *Header) {
-    uptr Size = getBackendAllocator().GetActuallyAllocatedSize(
-        getAllocBeg(Header));
+    uptr Size =
+        getBackendAllocator().GetActuallyAllocatedSize(getAllocBeg(Header),
+                                                       Header->FromPrimary);
     if (Size == 0)
       return 0;
     return Size - AlignedChunkHeaderSize - (Header->Offset << MinAlignmentLog);
   }
 
   // Compute the checksum of the Chunk pointer and its ChunkHeader.
   u16 computeChecksum(UnpackedHeader *Header) const {
     UnpackedHeader ZeroChecksumHeader = *Header;
void initScudo() {
 
   ScudoInitIsRunning = false;
 }
 
 struct QuarantineCallback {
   explicit QuarantineCallback(AllocatorCache *Cache)
     : Cache_(Cache) {}
 
-  // Chunk recycling function, returns a quarantined chunk to the backend.
+  // Chunk recycling function, returns a quarantined chunk to the backend,
+  // first making sure it hasn't been tampered with.
   void Recycle(ScudoChunk *Chunk) {
     UnpackedHeader Header;
     Chunk->loadHeader(&Header);
     if (UNLIKELY(Header.State != ChunkQuarantine)) {
       dieWithMessage("ERROR: invalid chunk state when recycling address %p\n",
                      Chunk);
     }
     Chunk->eraseHeader();
     void *Ptr = Chunk->getAllocBeg(&Header);
-    getBackendAllocator().Deallocate(Cache_, Ptr);
+    getBackendAllocator().Deallocate(Cache_, Ptr, Header.FromPrimary);
   }
 
-  // Internal quarantine allocation and deallocation functions.
-  void *Allocate(uptr Size) {
-    // TODO(kostyak): figure out the best way to protect the batches.
-    return getBackendAllocator().Allocate(Cache_, Size, MinAlignment);
+  // Internal quarantine allocation and deallocation functions. We first check
+  // that the batches are indeed serviced by the Primary.
+  COMPILER_CHECK(sizeof(QuarantineBatch) < SizeClassMap::kMaxSize);
+  void *Allocate(uptr Size) {
+    return getBackendAllocator().Allocate(Cache_, Size, MinAlignment, true);
   }
 
   void Deallocate(void *Ptr) {
-    getBackendAllocator().Deallocate(Cache_, Ptr);
+    getBackendAllocator().Deallocate(Cache_, Ptr, true);
   }
 
   AllocatorCache *Cache_;
 };
 
 typedef Quarantine<QuarantineCallback, ScudoChunk> ScudoQuarantine;
 typedef ScudoQuarantine::Cache ScudoQuarantineCache;
 COMPILER_CHECK(sizeof(ScudoQuarantineCache) <=
void *allocate(uptr Size, uptr Alignment, AllocType Type,
     if (Alignment < MinAlignment)
       Alignment = MinAlignment;
     if (Size >= MaxAllowedMallocSize)
       return BackendAllocator.ReturnNullOrDieOnBadRequest();
     if (Size == 0)
       Size = 1;
 
     uptr NeededSize = RoundUpTo(Size, MinAlignment) + AlignedChunkHeaderSize;
-    if (Alignment > MinAlignment)
-      NeededSize += Alignment;
-    if (NeededSize >= MaxAllowedMallocSize)
+    uptr AlignedSize = (Alignment > MinAlignment) ?
+        NeededSize + (Alignment - AlignedChunkHeaderSize) : NeededSize;
+    if (AlignedSize >= MaxAllowedMallocSize)
       return BackendAllocator.ReturnNullOrDieOnBadRequest();
 
-    // Primary backed and Secondary backed allocations have a different
-    // treatment. We deal with alignment requirements of Primary serviced
-    // allocations here, but the Secondary will take care of its own alignment
-    // needs, which means we also have to work around some limitations of the
-    // combined allocator to accommodate the situation.
-    bool FromPrimary = PrimaryAllocator::CanAllocate(NeededSize, MinAlignment);
+    // Primary and Secondary backed allocations have a different treatment. We
+    // deal with alignment requirements of Primary serviced allocations here,
+    // but the Secondary will take care of its own alignment needs.
+    bool FromPrimary = PrimaryAllocator::CanAllocate(AlignedSize, MinAlignment);
 
     void *Ptr;
     uptr Salt;
+    uptr AllocationSize = FromPrimary ? AlignedSize : NeededSize;
     uptr AllocationAlignment = FromPrimary ? MinAlignment : Alignment;
     ScudoThreadContext *ThreadContext = getThreadContextAndLock();
     if (LIKELY(ThreadContext)) {
       Salt = getPrng(ThreadContext)->getNext();
       Ptr = BackendAllocator.Allocate(getAllocatorCache(ThreadContext),
-                                      NeededSize, AllocationAlignment);
+                                      AllocationSize, AllocationAlignment,
+                                      FromPrimary);
       ThreadContext->unlock();
     } else {
       SpinMutexLock l(&FallbackMutex);
       Salt = FallbackPrng.getNext();
-      Ptr = BackendAllocator.Allocate(&FallbackAllocatorCache, NeededSize,
-                                      AllocationAlignment);
+      Ptr = BackendAllocator.Allocate(&FallbackAllocatorCache, AllocationSize,
+                                      AllocationAlignment, FromPrimary);
     }
     if (!Ptr)
       return BackendAllocator.ReturnNullOrDieOnOOM();
 
-    uptr AllocBeg = reinterpret_cast<uptr>(Ptr);
-    // If the allocation was serviced by the secondary, the returned pointer
-    // accounts for ChunkHeaderSize to pass the alignment check of the combined
-    // allocator. Adjust it here.
-    if (!FromPrimary) {
-      AllocBeg -= AlignedChunkHeaderSize;
-      if (Alignment > MinAlignment)
-        NeededSize -= Alignment;
-    }
-
     // If requested, we will zero out the entire contents of the returned chunk.
     if ((ForceZeroContents || ZeroContents) && FromPrimary)
-       memset(Ptr, 0, BackendAllocator.GetActuallyAllocatedSize(Ptr));
+       memset(Ptr, 0,
+              BackendAllocator.GetActuallyAllocatedSize(Ptr, FromPrimary));
 
+    uptr AllocBeg = reinterpret_cast<uptr>(Ptr);
     uptr UserBeg = AllocBeg + AlignedChunkHeaderSize;
-    if (!IsAligned(UserBeg, Alignment))
+    if (!IsAligned(UserBeg, Alignment)) {
+      CHECK(FromPrimary);
       UserBeg = RoundUpTo(UserBeg, Alignment);
-    CHECK_LE(UserBeg + Size, AllocBeg + NeededSize);
+    }
+    CHECK_LE(UserBeg + Size, AllocBeg + AllocationSize);
     UnpackedHeader Header = {};
     Header.State = ChunkAllocated;
     uptr Offset = UserBeg - AlignedChunkHeaderSize - AllocBeg;
     Header.Offset = Offset >> MinAlignmentLog;
     Header.AllocType = Type;
     if (FromPrimary) {
       Header.FromPrimary = FromPrimary;
       Header.SizeOrUnusedBytes = Size;
struct ScudoAllocator {
   }
 
   // Place a chunk in the quarantine. In the event of a zero-sized quarantine,
   // we directly deallocate the chunk, otherwise the flow would lead to the
   // chunk being loaded (and checked) twice, and stored (and checksummed) once,
   // with no additional security value.
   void quarantineOrDeallocateChunk(ScudoChunk *Chunk, UnpackedHeader *Header,
                                    uptr Size) {
+    bool FromPrimary = Header->FromPrimary;
     bool BypassQuarantine = (AllocatorQuarantine.GetCacheSize() == 0);
     if (BypassQuarantine) {
       Chunk->eraseHeader();
       void *Ptr = Chunk->getAllocBeg(Header);
       ScudoThreadContext *ThreadContext = getThreadContextAndLock();
       if (LIKELY(ThreadContext)) {
-        getBackendAllocator().Deallocate(getAllocatorCache(ThreadContext), Ptr);
+        getBackendAllocator().Deallocate(getAllocatorCache(ThreadContext), Ptr,
+                                         FromPrimary);
         ThreadContext->unlock();
       } else {
         SpinMutexLock Lock(&FallbackMutex);
-        getBackendAllocator().Deallocate(&FallbackAllocatorCache, Ptr);
+        getBackendAllocator().Deallocate(&FallbackAllocatorCache, Ptr,
+                                         FromPrimary);
       }
     } else {
       UnpackedHeader NewHeader = *Header;
       NewHeader.State = ChunkQuarantine;
       Chunk->compareExchangeHeader(&NewHeader, Header);
       ScudoThreadContext *ThreadContext = getThreadContextAndLock();
       if (LIKELY(ThreadContext)) {
         AllocatorQuarantine.Put(getQuarantineCache(ThreadContext),

lib/scudo/scudo_allocator_combined.h

+//===-- scudo_allocator_combined.h ------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// Scudo Combined Allocator, dispatches allocation & deallocation requests to
+/// the Primary or the Secondary backend allocators.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef SCUDO_ALLOCATOR_COMBINED_H_
+#define SCUDO_ALLOCATOR_COMBINED_H_
+
+#ifndef SCUDO_ALLOCATOR_H_
+#error "This file must be included inside scudo_allocator.h."
+#endif
+
+template <class PrimaryAllocator, class AllocatorCache,
+    class SecondaryAllocator>
+class ScudoCombinedAllocator {
+ public:
+  void InitLinkerInitialized(bool may_return_null, s32 ReleaseToOSIntervalMs) {
+    UNIMPLEMENTED();
+  }
+
+  void Init(bool AllocatorMayReturnNull, s32 ReleaseToOSIntervalMs) {
+    Primary.Init(ReleaseToOSIntervalMs);
+    Secondary.Init(AllocatorMayReturnNull);
+    Stats.Init();
+    atomic_store_relaxed(&MayReturnNull, AllocatorMayReturnNull);
+  }
+
+  void *Allocate(AllocatorCache *Cache, uptr Size, uptr Alignment,
+                 bool FromPrimary) {
+    if (FromPrimary)
+      return Cache->Allocate(&Primary, Primary.ClassID(Size));
+    return Secondary.Allocate(&Stats, Size, Alignment);
+  }
+
+  void *ReturnNullOrDieOnBadRequest() {
+    if (atomic_load_relaxed(&MayReturnNull))
+      return nullptr;
+    ReportAllocatorCannotReturnNull(false);
+  }
+
+  void *ReturnNullOrDieOnOOM() {
+    if (atomic_load_relaxed(&MayReturnNull))
+      return nullptr;
+    ReportAllocatorCannotReturnNull(true);
+  }
+
+  void SetMayReturnNull(bool AllocatorMayReturnNull) {
+    UNIMPLEMENTED();
+  }
+
+  s32 ReleaseToOSIntervalMs() const {
+    return Primary.ReleaseToOSIntervalMs();
+  }
+
+  void SetReleaseToOSIntervalMs(s32 ReleaseToOSIntervalMs) {
+    UNIMPLEMENTED();
+  }
+
+  void SetRssLimitIsExceeded(bool rss_limit_is_exceeded) {
+    UNIMPLEMENTED();
+  }
+
+  void Deallocate(AllocatorCache *Cache, void *Ptr, bool FromPrimary) {
+    if (FromPrimary)
+      Cache->Deallocate(&Primary, Primary.GetSizeClass(Ptr), Ptr);
+    else
+      Secondary.Deallocate(&Stats, Ptr);
+  }
+
+  void *Reallocate(AllocatorCache *Cache, void *Ptr, uptr NewSize,
+                   uptr Alignment) {
+    UNIMPLEMENTED();
+  }
+
+  bool PointerIsMine(void *Ptr) {
+    UNIMPLEMENTED();
+  }
+
+  bool FromPrimary(void *Ptr) {
+    return Primary.PointerIsMine(Ptr);
+  }
+
+  void *GetMetaData(const void *Ptr) {
+    UNIMPLEMENTED();
+  }
+
+  void *GetBlockBegin(const void *Ptr) {
+    UNIMPLEMENTED();
+  }
+
+  void *GetBlockBeginFastLocked(void *Ptr) {
+    UNIMPLEMENTED();
+  }
+
+  uptr GetActuallyAllocatedSize(void *Ptr, bool FromPrimary) {
+    if (FromPrimary)
+      return Primary.GetActuallyAllocatedSize(Ptr);
+    return Secondary.GetActuallyAllocatedSize(Ptr);
+  }
+
+  uptr TotalMemoryUsed() {
+    UNIMPLEMENTED();
+  }
+
+  void TestOnlyUnmap() {
+    UNIMPLEMENTED();
+  }
+
+  void InitCache(AllocatorCache *Cache) {
+    Cache->Init(&Stats);
+  }
+
+  void DestroyCache(AllocatorCache *Cache) {
+    Cache->Destroy(&Primary, &Stats);
+  }
+
+  void SwallowCache(AllocatorCache *Cache) {
+    Cache->Drain(&Primary);
+  }
+
+  void GetStats(AllocatorStatCounters StatType) const {
+    Stats.Get(StatType);
+  }
+
+  void PrintStats() {
+    UNIMPLEMENTED();
+  }
+
+  void ForceLock() {
+    UNIMPLEMENTED();
+  }
+
+  void ForceUnlock() {
+    UNIMPLEMENTED();
+  }
+
+  void ForEachChunk(ForEachChunkCallback Callback, void *Arg) {
+    UNIMPLEMENTED();
+  }
+
+ private:
+  PrimaryAllocator Primary;
+  SecondaryAllocator Secondary;
+  AllocatorGlobalStats Stats;
+  atomic_uint8_t MayReturnNull;
+};
+
+#endif  // SCUDO_ALLOCATOR_COMBINED_H_

lib/scudo/scudo_allocator_secondary.h

 # error "This file must be included inside scudo_allocator.h."
 #endif
 
 class ScudoLargeMmapAllocator {
  public:
 
   void Init(bool AllocatorMayReturnNull) {
     PageSize = GetPageSizeCached();
-    atomic_store(&MayReturnNull, AllocatorMayReturnNull, memory_order_relaxed);
+    atomic_store_relaxed(&MayReturnNull, AllocatorMayReturnNull);
   }
 
   void *Allocate(AllocatorStats *Stats, uptr Size, uptr Alignment) {
     // The Scudo frontend prevents us from allocating more than
     // MaxAllowedMallocSize, so integer overflow checks would be superfluous.
     uptr MapSize = Size + SecondaryHeaderSize;
+    if (Alignment > MinAlignment)
+      MapSize += Alignment;
     MapSize = RoundUpTo(MapSize, PageSize);
     // Account for 2 guard pages, one before and one after the chunk.
     MapSize += 2 * PageSize;
-    // The size passed to the Secondary comprises the alignment, if large
-    // enough. Subtract it here to get the requested size, including header.
-    if (Alignment > MinAlignment)
-      Size -= Alignment;
 
     uptr MapBeg = reinterpret_cast<uptr>(MmapNoAccess(MapSize));
     if (MapBeg == ~static_cast<uptr>(0))
       return ReturnNullOrDieOnOOM();
     // A page-aligned pointer is assumed after that, so check it now.
     CHECK(IsAligned(MapBeg, PageSize));
     uptr MapEnd = MapBeg + MapSize;
     // The beginning of the user area for that allocation comes after the
     // initial guard page, and both headers. This is the pointer that has to
     // abide by alignment requirements.
     uptr UserBeg = MapBeg + PageSize + HeadersSize;
 
     // In the rare event of larger alignments, we will attempt to fit the mmap
     // area better and unmap extraneous memory. This will also ensure that the
     // offset and unused bytes field of the header stay small.
     if (Alignment > MinAlignment) {
-      if (UserBeg & (Alignment - 1))
-        UserBeg += Alignment - (UserBeg & (Alignment - 1));
+      if (!IsAligned(UserBeg, Alignment)) {
+        UserBeg = (UserBeg + Alignment) & ~(Alignment - 1);
         CHECK_GE(UserBeg, MapBeg);
-      uptr NewMapBeg = RoundDownTo(UserBeg - HeadersSize, PageSize) - PageSize;
+        uptr NewMapBeg = RoundDownTo(UserBeg - HeadersSize, PageSize) -
+            PageSize;
         CHECK_GE(NewMapBeg, MapBeg);
+        if (NewMapBeg != MapBeg) {
+          UnmapOrDie(reinterpret_cast<void *>(MapBeg), NewMapBeg - MapBeg);
+          MapBeg = NewMapBeg;
+        }
+      }
       uptr NewMapEnd = RoundUpTo(UserBeg + (Size - AlignedChunkHeaderSize),
                                  PageSize) + PageSize;
       CHECK_LE(NewMapEnd, MapEnd);
-      // Unmap the extra memory if it's large enough, on both sides.
-      uptr Diff = NewMapBeg - MapBeg;
-      if (Diff > PageSize)
-        UnmapOrDie(reinterpret_cast<void *>(MapBeg), Diff);
-      Diff = MapEnd - NewMapEnd;
-      if (Diff > PageSize)
-        UnmapOrDie(reinterpret_cast<void *>(NewMapEnd), Diff);
-      MapBeg = NewMapBeg;
+      if (NewMapEnd != MapEnd) {
+        UnmapOrDie(reinterpret_cast<void *>(NewMapEnd), MapEnd - NewMapEnd);
         MapEnd = NewMapEnd;
-      MapSize = NewMapEnd - NewMapBeg;
+      }
+      MapSize = MapEnd - MapBeg;
     }
 
     uptr UserEnd = UserBeg + (Size - AlignedChunkHeaderSize);
     CHECK_LE(UserEnd, MapEnd - PageSize);
     // Actually mmap the memory, preserving the guard pages on either side.
     CHECK_EQ(MapBeg + PageSize, reinterpret_cast<uptr>(
         MmapFixedOrDie(MapBeg + PageSize, MapSize - 2 * PageSize)));
     uptr Ptr = UserBeg - AlignedChunkHeaderSize;
     SecondaryHeader *Header = getHeader(Ptr);
     Header->MapBeg = MapBeg;
     Header->MapSize = MapSize;
     // The primary adds the whole class size to the stats when allocating a
     // chunk, so we will do something similar here. But we will not account for
     // the guard pages.
     {
       SpinMutexLock l(&StatsMutex);
       Stats->Add(AllocatorStatAllocated, MapSize - 2 * PageSize);
       Stats->Add(AllocatorStatMapped, MapSize - 2 * PageSize);
     }
 
-    return reinterpret_cast<void *>(UserBeg);
+    return reinterpret_cast<void *>(Ptr);
   }
 
   void *ReturnNullOrDieOnBadRequest() {
-    if (atomic_load(&MayReturnNull, memory_order_acquire))
+    if (atomic_load_relaxed(&MayReturnNull))
       return nullptr;
     ReportAllocatorCannotReturnNull(false);
   }
 
   void *ReturnNullOrDieOnOOM() {
-    if (atomic_load(&MayReturnNull, memory_order_acquire))
+    if (atomic_load_relaxed(&MayReturnNull))
       return nullptr;
     ReportAllocatorCannotReturnNull(true);
   }
 
   void SetMayReturnNull(bool AllocatorMayReturnNull) {
-    atomic_store(&MayReturnNull, AllocatorMayReturnNull, memory_order_release);
+    UNIMPLEMENTED();
   }
 
   void Deallocate(AllocatorStats *Stats, void *Ptr) {
     SecondaryHeader *Header = getHeader(Ptr);
     {
       SpinMutexLock l(&StatsMutex);
       Stats->Sub(AllocatorStatAllocated, Header->MapSize - 2 * PageSize);
       Stats->Sub(AllocatorStatMapped, Header->MapSize - 2 * PageSize);
public:
   }
 
   void ForEachChunk(ForEachChunkCallback Callback, void *Arg) {
     UNIMPLEMENTED();
   }
 
  private:
   // A Secondary allocated chunk header contains the base of the mapping and
-  // its size. Currently, the base is always a page before the header, but
-  // we might want to extend that number in the future based on the size of
-  // the allocation.
+  // its size, which comprises the guard pages.
   struct SecondaryHeader {
     uptr MapBeg;
     uptr MapSize;
   };
   // Check that sizeof(SecondaryHeader) is a multiple of MinAlignment.
   COMPILER_CHECK((sizeof(SecondaryHeader) & (MinAlignment - 1)) == 0);
 
   SecondaryHeader *getHeader(uptr Ptr) {