[scudo][standalone] NFC corrections

[cryptoad]

Summary

A few corrections:

• rename TransferBatch::MaxCached to getMaxCached to conform with the style guide;
• move getBlockBegin from Chunk:: to Allocator::: I believe it was a fallacy to have this be a Chunk method, as chunks' relationship to backend blocks are up to the frontend allocator. It makes more sense now, particularly with regard to the offset. Update the associated chunk test as the method isn't available there anymore;
• add a forgotten \n to a log string;
• for releaseToOs, instead of starting at 1, start at 0 and continue on BatchClassId: in the end it's identical but doesn't assume a particular class id for batches;
• change a CHECK to a reportOutOfMemory: it's a clearer message

[hctim]

LGTM.

lib/scudo/standalone/chunk.h

 }
 
 INLINE
 const AtomicPackedHeader *getConstAtomicHeader(const void *Ptr) {
   return reinterpret_cast<const AtomicPackedHeader *>(
       reinterpret_cast<uptr>(Ptr) - getHeaderSize());
 }
 
-INLINE void *getBlockBegin(const void *Ptr, UnpackedHeader *Header) {
-  return reinterpret_cast<void *>(reinterpret_cast<uptr>(Ptr) -
-                                  getHeaderSize() -
-                                  (Header->Offset << SCUDO_MIN_ALIGNMENT_LOG));
-}
-
 // We do not need a cryptographically strong hash for the checksum, but a CRC
 // type function that can alert us in the event a header is invalid or
 // corrupted. Ideally slightly better than a simple xor of all fields.
 static INLINE u16 computeHeaderChecksum(u32 Cookie, const void *Ptr,
                                         UnpackedHeader *Header) {
   UnpackedHeader ZeroChecksumHeader = *Header;
   ZeroChecksumHeader.Checksum = 0;
   uptr HeaderHolder[sizeof(UnpackedHeader) / sizeof(uptr)];

lib/scudo/standalone/combined.h

void recycle(void *Ptr) {
       Chunk::loadHeader(Allocator.Cookie, Ptr, &Header);
       if (UNLIKELY(Header.State != Chunk::State::Quarantined))
         reportInvalidChunkState(AllocatorAction::Recycling, Ptr);
 
       Chunk::UnpackedHeader NewHeader = Header;
       NewHeader.State = Chunk::State::Available;
       Chunk::compareExchangeHeader(Allocator.Cookie, Ptr, &NewHeader, &Header);
 
-      void *BlockBegin = Chunk::getBlockBegin(Ptr, &Header);
+      void *BlockBegin = Allocator::getBlockBegin(Ptr, &NewHeader);
       const uptr ClassId = Header.ClassId;
       if (ClassId)
         Cache.deallocate(ClassId, BlockBegin);
       else
         Allocator.Secondary.deallocate(BlockBegin);
     }
 
     // We take a shortcut when allocating a quarantine batch by working with the
if (UNLIKELY(Header.SizeOrUnusedBytes != MaxSizeOrUnusedBytes))
       reportSanityCheckError("size (or unused bytes)");
 
     const uptr LargestClassId = SizeClassMap::LargestClassId;
     Header.ClassId = LargestClassId;
     if (UNLIKELY(Header.ClassId != LargestClassId))
       reportSanityCheckError("class ID");
   }
 
+  static INLINE void *getBlockBegin(const void *Ptr,
+                                    Chunk::UnpackedHeader *Header) {
+    return reinterpret_cast<void *>(reinterpret_cast<uptr>(Ptr) -
+                                    Chunk::getHeaderSize() -
+                                    (Header->Offset << MinAlignmentLog));
+  }
+
   // Return the size of a chunk as requested during its allocation.
   INLINE uptr getSize(const void *Ptr, Chunk::UnpackedHeader *Header) {
     const uptr SizeOrUnusedBytes = Header->SizeOrUnusedBytes;
     if (Header->ClassId)
       return SizeOrUnusedBytes;
-    return SecondaryT::getBlockEnd(Chunk::getBlockBegin(Ptr, Header)) -
+    return SecondaryT::getBlockEnd(getBlockBegin(Ptr, Header)) -
            reinterpret_cast<uptr>(Ptr) - SizeOrUnusedBytes;
   }
 
   ALWAYS_INLINE void initThreadMaybe(bool MinimalInit = false) {
     TSDRegistry.initThreadMaybe(this, MinimalInit);
   }
 
   void quarantineOrDeallocateChunk(void *Ptr, Chunk::UnpackedHeader *Header,
                                    uptr Size) {
     Chunk::UnpackedHeader NewHeader = *Header;
     // If the quarantine is disabled, the actual size of a chunk is 0 or larger
     // than the maximum allowed, we return a chunk directly to the backend.
     const bool BypassQuarantine = !Quarantine.getCacheSize() || !Size ||
                                   (Size > Options.QuarantineMaxChunkSize);
     if (BypassQuarantine) {
       NewHeader.State = Chunk::State::Available;
       Chunk::compareExchangeHeader(Cookie, Ptr, &NewHeader, Header);
-      void *BlockBegin = Chunk::getBlockBegin(Ptr, Header);
+      void *BlockBegin = getBlockBegin(Ptr, &NewHeader);
       const uptr ClassId = NewHeader.ClassId;
       if (ClassId) {
         bool UnlockRequired;
         auto *TSD = TSDRegistry.getTSDAndLock(&UnlockRequired);
         TSD->Cache.deallocate(ClassId, BlockBegin);
         if (UnlockRequired)
           TSD->unlock();
       } else {

lib/scudo/standalone/local_cache.h

 //===-- local_cache.h -------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef SCUDO_LOCAL_CACHE_H_
 #define SCUDO_LOCAL_CACHE_H_
 
 #include "internal_defs.h"
+#include "report.h"
 #include "stats.h"
 
 namespace scudo {
 
 template <class SizeClassAllocator> struct SizeClassAllocatorLocalCache {
   typedef typename SizeClassAllocator::SizeClassMap SizeClassMap;
 
   struct TransferBatch {
void copyToArray(void **Array) const {
       for (u32 I = 0; I < Count; I++)
         Array[I] = Batch[I];
     }
     u32 getCount() const { return Count; }
     void *get(u32 I) const {
       DCHECK_LE(I, Count);
       return Batch[I];
     }
-    static u32 MaxCached(uptr Size) {
+    static u32 getMaxCached(uptr Size) {
       return Min(MaxNumCached, SizeClassMap::getMaxCachedHint(Size));
     }
     TransferBatch *Next;
 
   private:
     u32 Count;
     void *Batch[MaxNumCached];
   };
ALWAYS_INLINE void initCacheMaybe(PerClass *C) {
     initCache();
     DCHECK_NE(C->MaxCount, 0U);
   }
 
   NOINLINE void initCache() {
     for (uptr I = 0; I < NumClasses; I++) {
       PerClass *P = &PerClassArray[I];
       const uptr Size = SizeClassAllocator::getSizeByClassId(I);
-      P->MaxCount = 2 * TransferBatch::MaxCached(Size);
+      P->MaxCount = 2 * TransferBatch::getMaxCached(Size);
       P->ClassSize = Size;
     }
   }
 
   void destroyBatch(uptr ClassId, void *B) {
     if (ClassId != SizeClassMap::BatchClassId)
       deallocate(SizeClassMap::BatchClassId, B);
   }
NOINLINE bool refill(PerClass *C, uptr ClassId) {
     destroyBatch(ClassId, B);
     return true;
   }
 
   NOINLINE void drain(PerClass *C, uptr ClassId) {
     const u32 Count = Min(C->MaxCount / 2, C->Count);
     const uptr FirstIndexToDrain = C->Count - Count;
     TransferBatch *B = createBatch(ClassId, C->Chunks[FirstIndexToDrain]);
-    CHECK(B);
+    if (UNLIKELY(!B))
+      reportOutOfMemory(
+          SizeClassAllocator::getSizeByClassId(SizeClassMap::BatchClassId));
     B->setFromArray(&C->Chunks[FirstIndexToDrain], Count);
     C->Count -= Count;
     Allocator->pushBatch(ClassId, B);
   }
 };
 
 } // namespace scudo
 
 #endif // SCUDO_LOCAL_CACHE_H_

lib/scudo/standalone/primary32.h

void printStats() {
     Printf("Stats: SizeClassAllocator32: %zuM mapped in %zu allocations; "
            "remains %zu\n",
            TotalMapped >> 20, PoppedBlocks, PoppedBlocks - PushedBlocks);
     for (uptr I = 0; I < NumClasses; I++)
       printStats(I, 0);
   }
 
   void releaseToOS() {
-    for (uptr I = 1; I < NumClasses; I++) {
+    for (uptr I = 0; I < NumClasses; I++) {
+      if (I == SizeClassMap::BatchClassId)
+        continue;
       SizeClassInfo *Sci = getSizeClassInfo(I);
       ScopedLock L(Sci->Mutex);
       releaseToOSMaybe(Sci, I, /*Force=*/true);
     }
   }
 
 private:
   static const uptr NumClasses = SizeClassMap::NumClasses;
#endif
 
   NOINLINE TransferBatch *populateFreeList(CacheT *C, uptr ClassId,
                                            SizeClassInfo *Sci) {
     const uptr Region = allocateRegion(ClassId);
     if (UNLIKELY(!Region))
       return nullptr;
     C->getStats().add(StatMapped, RegionSize);
     const uptr Size = getSizeByClassId(ClassId);
-    const u32 MaxCount = TransferBatch::MaxCached(Size);
+    const u32 MaxCount = TransferBatch::getMaxCached(Size);
     DCHECK_GT(MaxCount, 0);
     const uptr NumberOfBlocks = RegionSize / Size;
     DCHECK_GT(NumberOfBlocks, 0);
     TransferBatch *B = nullptr;
     constexpr uptr ShuffleArraySize = 48;
     void *ShuffleArray[ShuffleArraySize];
     u32 Count = 0;
     const uptr AllocatedUser = NumberOfBlocks * Size;

lib/scudo/standalone/primary64.h

Printf("Stats: Primary64: %zuM mapped (%zuM rss) in %zu allocations; "
            "remains %zu\n",
            TotalMapped >> 20, 0, PoppedBlocks, PoppedBlocks - PushedBlocks);
 
     for (uptr I = 0; I < NumClasses; I++)
       printStats(I, 0);
   }
 
   void releaseToOS() {
-    for (uptr I = 1; I < NumClasses; I++) {
+    for (uptr I = 0; I < NumClasses; I++) {
+      if (I == SizeClassMap::BatchClassId)
+        continue;
       RegionInfo *Region = getRegionInfo(I);
       ScopedLock L(Region->Mutex);
       releaseToOSMaybe(Region, I, /*Force=*/true);
     }
   }
 
 private:
   static const uptr RegionSize = 1UL << RegionSizeLog;
bool populateBatches(CacheT *C, RegionInfo *Region, uptr ClassId,
     }
     *CurrentBatch = B;
     return true;
   }
 
   NOINLINE TransferBatch *populateFreeList(CacheT *C, uptr ClassId,
                                            RegionInfo *Region) {
     const uptr Size = getSizeByClassId(ClassId);
-    const u32 MaxCount = TransferBatch::MaxCached(Size);
+    const u32 MaxCount = TransferBatch::getMaxCached(Size);
 
     const uptr RegionBeg = Region->RegionBeg;
     const uptr MappedUser = Region->MappedUser;
     const uptr TotalUserBytes = Region->AllocatedUser + MaxCount * Size;
     // Map more space for blocks, if necessary.
     if (LIKELY(TotalUserBytes > MappedUser)) {
       // Do the mmap for the user memory.
       const uptr UserMapSize =

lib/scudo/standalone/report.cc

void NORETURN reportCheckFailed(const char *File, int Line,
   ScopedErrorReport Report;
   Report.append("CHECK failed @ %s:%d %s (%llu, %llu)\n", File, Line, Condition,
                 Value1, Value2);
 }
 
 // Generic string fatal error message.
 void NORETURN reportError(const char *Message) {
   ScopedErrorReport Report;
-  Report.append("%s", Message);
+  Report.append("%s\n", Message);
 }
 
 void NORETURN reportInvalidFlag(const char *FlagType, const char *Value) {
   ScopedErrorReport Report;
   Report.append("invalid value for %s option: '%s'\n", FlagType, Value);
 }
 
 // The checksum of a chunk header is invalid. This could be caused by an

lib/scudo/standalone/tests/chunk_test.cc

TEST(ScudoChunkTest, ChunkBasic) {
   initChecksum();
   const scudo::uptr Size = 0x100U;
   scudo::Chunk::UnpackedHeader Header = {};
   void *Block = malloc(HeaderSize + Size);
   void *P = reinterpret_cast<void *>(reinterpret_cast<scudo::uptr>(Block) +
                                      HeaderSize);
   scudo::Chunk::storeHeader(Cookie, P, &Header);
   memset(P, 'A', Size);
-  EXPECT_EQ(scudo::Chunk::getBlockBegin(P, &Header), Block);
   scudo::Chunk::loadHeader(Cookie, P, &Header);
   EXPECT_TRUE(scudo::Chunk::isValid(Cookie, P, &Header));
   EXPECT_FALSE(scudo::Chunk::isValid(InvalidCookie, P, &Header));
   EXPECT_DEATH(scudo::Chunk::loadHeader(InvalidCookie, P, &Header), "");
   free(Block);
 }
 
 TEST(ScudoChunkTest, ChunkCmpXchg) {
TEST(ScudoChunkTest, CorruptHeader) {
   initChecksum();
   const scudo::uptr Size = 0x100U;
   scudo::Chunk::UnpackedHeader Header = {};
   void *Block = malloc(HeaderSize + Size);
   void *P = reinterpret_cast<void *>(reinterpret_cast<scudo::uptr>(Block) +
                                      HeaderSize);
   scudo::Chunk::storeHeader(Cookie, P, &Header);
   memset(P, 'A', Size);
-  EXPECT_EQ(scudo::Chunk::getBlockBegin(P, &Header), Block);
   scudo::Chunk::loadHeader(Cookie, P, &Header);
   // Simulate a couple of corrupted bits per byte of header data.
   for (scudo::uptr I = 0; I < sizeof(scudo::Chunk::PackedHeader); I++) {
     *(reinterpret_cast<scudo::u8 *>(Block) + I) ^= 0x42U;
     EXPECT_DEATH(scudo::Chunk::loadHeader(Cookie, P, &Header), "");
     *(reinterpret_cast<scudo::u8 *>(Block) + I) ^= 0x42U;
   }
   free(Block);
 }