[scudo] Make Options a reference for functions.

[cferris]

Summary

Modify all places that use the Options structure to be a const
reference. The underlying structure is a u32 so making it a
reference doesn't really do anything. However, if the structure
changes in the future it already works and avoids future coders
wondering why a structure is being passed by value. This also
makes it clear that the Options should not be modified in those functions.

[Chia-hungDuan]

Nice!

[cferris]

Remove the const Options &Options = Options.load();

That's too aggressive and there is already a copy.

[cferris]

Actually do a clang-format.

[Chia-hungDuan]

Just did a quick look on the type Options, it's a wrapper of AtomicOptions which contains a single atomic_32 data member. The change may make it require 64 bits space. If we want to assume that Options will be extended then passing by reference shows more value here. (Or maybe I misunderstand something here) I agree that making it passing by reference may be reasonable in terms of software architecture. No strong opinion here, will leave you to do the final decision.

[cferris]

Yeah, I was thinking about this and it doesn't do much. However, I do think the const is useful just to make sure the compiler catches any places where we try to modify the value. And even though making it a reference doesn't really do anything, it also prevents people from coming along and asking why a structure is being passed on the stack.

So I think it's reasonable to leave this as is, but I changed the description to be clearer about this.

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

uptr Size =
         android_unsafe_frame_pointer_chase(Stack, MaxTraceSize + DiscardFrames);
     Size = Min<uptr>(Size, MaxTraceSize + DiscardFrames);
     return Depot.insert(Stack + Min<uptr>(DiscardFrames, Size), Stack + Size);
 #else
     return 0;
 #endif
   }
 
-  uptr computeOddEvenMaskForPointerMaybe(Options Options, uptr Ptr,
+  uptr computeOddEvenMaskForPointerMaybe(const Options &Options, uptr Ptr,
                                          uptr ClassId) {
     if (!Options.get(OptionBit::UseOddEvenTags))
       return 0;
 
     // If a chunk's tag is odd, we want the tags of the surrounding blocks to be
     // even, and vice versa. Blocks are laid out Size bytes apart, and adding
     // Size to Ptr will flip the least significant set bit of Size in Ptr, so
     // that bit will have the pattern 010101... for consecutive blocks, which we
inline uptr getSize(const void *Ptr, Chunk::UnpackedHeader *Header) {
     if (LIKELY(Header->ClassId))
       return SizeOrUnusedBytes;
     if (allocatorSupportsMemoryTagging<Config>())
       Ptr = untagPointer(const_cast<void *>(Ptr));
     return SecondaryT::getBlockEnd(getBlockBegin(Ptr, Header)) -
            reinterpret_cast<uptr>(Ptr) - SizeOrUnusedBytes;
   }
 
-  void quarantineOrDeallocateChunk(Options Options, void *TaggedPtr,
+  void quarantineOrDeallocateChunk(const Options &Options, void *TaggedPtr,
                                    Chunk::UnpackedHeader *Header,
                                    uptr Size) NO_THREAD_SAFETY_ANALYSIS {
     void *Ptr = getHeaderTaggedPointer(TaggedPtr);
     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.
     // This purposefully underflows for Size == 0.
     const bool BypassQuarantine = !Quarantine.getCacheSize() ||
if (RoundOldPtr >= NewPtr) {
       // Set the memory tag of the region
       // [RoundOldPtr, roundUp(NewPtr, archMemoryTagGranuleSize()))
       // to the pointer tag stored in OldPtr.
       RoundNewPtr = storeTags(RoundOldPtr, NewPtr);
     }
     storeEndMarker(RoundNewPtr, NewSize, BlockEnd);
   }
 
-  void storePrimaryAllocationStackMaybe(Options Options, void *Ptr) {
+  void storePrimaryAllocationStackMaybe(const Options &Options, void *Ptr) {
     if (!UNLIKELY(Options.get(OptionBit::TrackAllocationStacks)))
       return;
     auto *Ptr32 = reinterpret_cast<u32 *>(Ptr);
     Ptr32[MemTagAllocationTraceIndex] = collectStackTrace();
     Ptr32[MemTagAllocationTidIndex] = getThreadID();
   }
 
   void storeRingBufferEntry(void *Ptr, u32 AllocationTrace, u32 AllocationTid,
void storeRingBufferEntry(void *Ptr, u32 AllocationTrace, u32 AllocationTid,
     atomic_store_relaxed(&Entry->AllocationSize, AllocationSize);
     atomic_store_relaxed(&Entry->DeallocationTrace, DeallocationTrace);
     atomic_store_relaxed(&Entry->DeallocationTid, DeallocationTid);
     __atomic_signal_fence(__ATOMIC_SEQ_CST);
 
     atomic_store_relaxed(&Entry->Ptr, reinterpret_cast<uptr>(Ptr));
   }
 
-  void storeSecondaryAllocationStackMaybe(Options Options, void *Ptr,
+  void storeSecondaryAllocationStackMaybe(const Options &Options, void *Ptr,
                                           uptr Size) {
     if (!UNLIKELY(Options.get(OptionBit::TrackAllocationStacks)))
       return;
 
     u32 Trace = collectStackTrace();
     u32 Tid = getThreadID();
 
     auto *Ptr32 = reinterpret_cast<u32 *>(Ptr);
     Ptr32[MemTagAllocationTraceIndex] = Trace;
     Ptr32[MemTagAllocationTidIndex] = Tid;
 
     storeRingBufferEntry(untagPointer(Ptr), Trace, Tid, Size, 0, 0);
   }
 
-  void storeDeallocationStackMaybe(Options Options, void *Ptr, u8 PrevTag,
-                                   uptr Size) {
+  void storeDeallocationStackMaybe(const Options &Options, void *Ptr,
+                                   u8 PrevTag, uptr Size) {
     if (!UNLIKELY(Options.get(OptionBit::TrackAllocationStacks)))
       return;
 
     auto *Ptr32 = reinterpret_cast<u32 *>(Ptr);
     u32 AllocationTrace = Ptr32[MemTagAllocationTraceIndex];
     u32 AllocationTid = Ptr32[MemTagAllocationTidIndex];
 
     u32 DeallocationTrace = collectStackTrace();

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

struct Options {
   bool get(OptionBit Opt) const { return Val & (1U << static_cast<u32>(Opt)); }
 
   FillContentsMode getFillContentsMode() const {
     return static_cast<FillContentsMode>(
         (Val >> static_cast<u32>(OptionBit::FillContents0of2)) & 3);
   }
 };
 
-template <typename Config> bool useMemoryTagging(Options Options) {
+template <typename Config> bool useMemoryTagging(const Options &Options) {
   return allocatorSupportsMemoryTagging<Config>() &&
          Options.get(OptionBit::UseMemoryTagging);
 }
 
 struct AtomicOptions {
   atomic_u32 Val = {};
 
   Options load() const { return Options{atomic_load_relaxed(&Val)}; }

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

struct CachedBlock {
   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); }
+  void store(UNUSED const Options &Options, LargeBlock::Header *H) { unmap(H); }
   bool canCache(UNUSED uptr Size) { return false; }
   void disable() {}
   void enable() {}
   void releaseToOS() {}
   void disableMemoryTagging() {}
   void unmapTestOnly() {}
   bool setOption(Option O, UNUSED sptr Value) {
     if (O == Option::ReleaseInterval || O == Option::MaxCacheEntriesCount ||
         O == Option::MaxCacheEntrySize)
       return false;
     // Not supported by the Secondary Cache, but not an error either.
     return true;
   }
 
   void getStats(UNUSED ScopedString *Str) {
     Str->append("Secondary Cache Disabled\n");
   }
 };
 
 static const uptr MaxUnusedCachePages = 4U;
 
 template <typename Config>
-bool mapSecondary(Options Options, uptr CommitBase, uptr CommitSize,
+bool mapSecondary(const Options &Options, uptr CommitBase, uptr CommitSize,
                   uptr AllocPos, uptr Flags, MemMapT &MemMap) {
   Flags |= MAP_RESIZABLE;
   Flags |= MAP_ALLOWNOMEM;
 
   const uptr MaxUnusedCacheBytes = MaxUnusedCachePages * getPageSizeCached();
   if (useMemoryTagging<Config>(Options) && CommitSize > MaxUnusedCacheBytes) {
     const uptr UntaggedPos = Max(AllocPos, CommitBase + MaxUnusedCacheBytes);
     return MemMap.remap(CommitBase, UntaggedPos - CommitBase, "scudo:secondary",
void init(s32 ReleaseToOsInterval) NO_THREAD_SAFETY_ANALYSIS {
     DCHECK_EQ(EntriesCount, 0U);
     setOption(Option::MaxCacheEntriesCount,
               static_cast<sptr>(CacheConfig::DefaultMaxEntriesCount));
     setOption(Option::MaxCacheEntrySize,
               static_cast<sptr>(CacheConfig::DefaultMaxEntrySize));
     setOption(Option::ReleaseInterval, static_cast<sptr>(ReleaseToOsInterval));
   }
 
-  void store(Options Options, LargeBlock::Header *H) EXCLUDES(Mutex) {
+  void store(const Options &Options, LargeBlock::Header *H) EXCLUDES(Mutex) {
     if (!canCache(H->CommitSize))
       return unmap(H);
 
     bool EntryCached = false;
     bool EmptyCache = false;
     const s32 Interval = atomic_load_relaxed(&ReleaseToOsIntervalMs);
     const u64 Time = getMonotonicTimeFast();
     const u32 MaxCount = atomic_load_relaxed(&MaxEntriesCount);
void init(GlobalStats *S,
     DCHECK_EQ(AllocatedBytes, 0U);
     DCHECK_EQ(FreedBytes, 0U);
     Cache.init(ReleaseToOsInterval);
     Stats.init();
     if (LIKELY(S))
       S->link(&Stats);
   }
 
-  void *allocate(Options Options, uptr Size, uptr AlignmentHint = 0,
+  void *allocate(const Options &Options, uptr Size, uptr AlignmentHint = 0,
                  uptr *BlockEnd = nullptr,
                  FillContentsMode FillContents = NoFill);
 
-  void deallocate(Options Options, void *Ptr);
+  void deallocate(const Options &Options, void *Ptr);
 
   static uptr getBlockEnd(void *Ptr) {
     auto *B = LargeBlock::getHeader<Config>(Ptr);
     return B->CommitBase + B->CommitSize;
   }
 
   static uptr getBlockSize(void *Ptr) {
     return getBlockEnd(Ptr) - reinterpret_cast<uptr>(Ptr);
template <typename F> void iterateOverBlocks(F Callback) const {
     for (const auto &H : InUseBlocks) {
       uptr Ptr = reinterpret_cast<uptr>(&H) + LargeBlock::getHeaderSize();
       if (allocatorSupportsMemoryTagging<Config>())
         Ptr = untagPointer(Ptr);
       Callback(Ptr);
     }
   }
 
-  inline void setHeader(Options Options, CachedBlock &Entry,
+  inline void setHeader(const Options &Options, CachedBlock &Entry,
                         LargeBlock::Header *H, bool &Zeroed) {
     Zeroed = Entry.Time == 0;
     if (useMemoryTagging<Config>(Options)) {
       Entry.MemMap.setMemoryPermission(Entry.CommitBase, Entry.CommitSize, 0);
       // Block begins after the LargeBlock::Header
       uptr NewBlockBegin = reinterpret_cast<uptr>(H + 1);
       if (Zeroed) {
         storeTags(LargeBlock::addHeaderTag<Config>(Entry.CommitBase),

 // Due to the sparsity of address space available on those platforms, requesting
 // an allocation from the Secondary with a large alignment would end up wasting
 // VA space (even though we are not committing the whole thing), hence the need
 // to trim off some of the reserved space.
 // For allocations requested with an alignment greater than or equal to a page,
 // the committed memory will amount to something close to Size - AlignmentHint
 // (pending rounding and headers).
 template <typename Config>
-void *MapAllocator<Config>::allocate(Options Options, uptr Size, uptr Alignment,
-                                     uptr *BlockEndPtr,
+void *MapAllocator<Config>::allocate(const Options &Options, uptr Size,
+                                     uptr Alignment, uptr *BlockEndPtr,
                                      FillContentsMode FillContents) {
   if (Options.get(OptionBit::AddLargeAllocationSlack))
     Size += 1UL << SCUDO_MIN_ALIGNMENT_LOG;
   Alignment = Max(Alignment, uptr(1U) << SCUDO_MIN_ALIGNMENT_LOG);
   const uptr PageSize = getPageSizeCached();
   uptr RoundedSize =
       roundUp(roundUp(Size, Alignment) + LargeBlock::getHeaderSize() +
                   Chunk::getHeaderSize(),
if (BlockEndPtr)
     NumberOfAllocs++;
     Stats.add(StatAllocated, CommitSize);
     Stats.add(StatMapped, H->MemMap.getCapacity());
   }
   return reinterpret_cast<void *>(HeaderPos + LargeBlock::getHeaderSize());
 }
 
 template <typename Config>
-void MapAllocator<Config>::deallocate(Options Options, void *Ptr)
+void MapAllocator<Config>::deallocate(const Options &Options, void *Ptr)
     EXCLUDES(Mutex) {
   LargeBlock::Header *H = LargeBlock::getHeader<Config>(Ptr);
   const uptr CommitSize = H->CommitSize;
   {
     ScopedLock L(Mutex);
     InUseBlocks.remove(H);
     FreedBytes += CommitSize;
     NumberOfFrees++;