diff --git a/compiler-rt/lib/scudo/standalone/secondary.h b/compiler-rt/lib/scudo/standalone/secondary.h index e3dc8d4ef8247..4b11d039c654a 100644 --- a/compiler-rt/lib/scudo/standalone/secondary.h +++ b/compiler-rt/lib/scudo/standalone/secondary.h @@ -321,8 +321,6 @@ class MapAllocatorCache { } CachedBlock PrevEntry = Quarantine[QuarantinePos]; Quarantine[QuarantinePos] = Entry; - if (OldestTime == 0) - OldestTime = Entry.Time; Entry = PrevEntry; } @@ -344,8 +342,6 @@ class MapAllocatorCache { insert(Entry, (Entry.Time == 0) ? DECOMMITTED : COMMITTED); - if (OldestTime == 0) - OldestTime = Entry.Time; } while (0); // ScopedLock L(Mutex); for (MemMapT &EvictMemMap : EvictionMemMaps) @@ -585,32 +581,29 @@ class MapAllocatorCache { } } - void releaseIfOlderThan(CachedBlock &Entry, u64 Time) REQUIRES(Mutex) { - if (!Entry.isValid() || !Entry.Time) - return; - if (Entry.Time > Time) { - if (OldestTime == 0 || Entry.Time < OldestTime) - OldestTime = Entry.Time; - return; - } + inline void release(CachedBlock &Entry) { + DCHECK(Entry.Time != 0); Entry.MemMap.releaseAndZeroPagesToOS(Entry.CommitBase, Entry.CommitSize); Entry.Time = 0; } void releaseOlderThan(u64 Time) EXCLUDES(Mutex) { ScopedLock L(Mutex); - if (!EntriesCount || OldestTime == 0 || OldestTime > Time) - return; - OldestTime = 0; - for (uptr I = 0; I < Config::getQuarantineSize(); I++) - releaseIfOlderThan(Quarantine[I], Time); - for (u16 I = EntryLists[COMMITTED].Head; I != CachedBlock::InvalidEntry; - I = Entries[I].Next) { - if (Entries[I].Time && Entries[I].Time <= Time) { - unlink(I, COMMITTED); - pushFront(I, DECOMMITTED); - } - releaseIfOlderThan(Entries[I], Time); + for (uptr I = 0; I < Config::getQuarantineSize(); I++) { + CachedBlock &Entry = Quarantine[I]; + if (!Entry.isValid() || Entry.Time > Time) + continue; + release(Entry); + } + + // Release oldest entries first by releasing from tail + u16 ReleaseIndex = EntryLists[COMMITTED].Tail; + while (ReleaseIndex != CachedBlock::InvalidEntry && + Entries[ReleaseIndex].Time <= Time) { + release(Entries[ReleaseIndex]); + unlink(ReleaseIndex, COMMITTED); + pushFront(ReleaseIndex, DECOMMITTED); + ReleaseIndex = Entries[ReleaseIndex].Prev; } } @@ -619,7 +612,6 @@ class MapAllocatorCache { u32 QuarantinePos GUARDED_BY(Mutex) = 0; atomic_u32 MaxEntriesCount = {}; atomic_uptr MaxEntrySize = {}; - u64 OldestTime GUARDED_BY(Mutex) = 0; atomic_s32 ReleaseToOsIntervalMs = {}; u32 CallsToRetrieve GUARDED_BY(Mutex) = 0; u32 SuccessfulRetrieves GUARDED_BY(Mutex) = 0; diff --git a/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp b/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp index e85b6abdb36d2..61f7a46140c86 100644 --- a/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp +++ b/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp @@ -308,14 +308,14 @@ struct MapAllocatorCacheTest : public Test { scudo::uptr NumEntries, scudo::uptr Size) { for (scudo::uptr I = 0; I < NumEntries; I++) { MemMaps.emplace_back(allocate(Size)); - auto &MemMap = MemMaps[I]; + auto &MemMap = MemMaps.back(); Cache->store(Options, MemMap.getBase(), MemMap.getCapacity(), MemMap.getBase(), MemMap); } } }; -TEST_F(MapAllocatorCacheTest, CacheOrder) { +TEST_F(MapAllocatorCacheTest, CacheOrderNoRelease) { std::vector MemMaps; Cache->setOption(scudo::Option::MaxCacheEntriesCount, CacheConfig::getEntriesArraySize()); @@ -336,6 +336,31 @@ TEST_F(MapAllocatorCacheTest, CacheOrder) { MemMap.unmap(); } +TEST_F(MapAllocatorCacheTest, CacheOrderWithRelease) { + std::vector MemMaps; + Cache->setOption(scudo::Option::MaxCacheEntriesCount, + CacheConfig::getEntriesArraySize()); + + fillCacheWithSameSizeBlocks(MemMaps, CacheConfig::getEntriesArraySize(), + TestAllocSize); + + // Release all entries to transfer COMMITTED list contents to + // DECOMMITTED list + Cache->releaseToOS(); + + // Retrieval order should be the inverse of insertion order + for (scudo::uptr I = CacheConfig::getEntriesArraySize(); I > 0; I--) { + scudo::uptr EntryHeaderPos; + scudo::CachedBlock Entry = + Cache->retrieve(TestAllocSize, PageSize, 0, EntryHeaderPos); + EXPECT_EQ(Entry.MemMap.getBase(), MemMaps[I - 1].getBase()); + } + + // Clean up MemMaps + for (auto &MemMap : MemMaps) + MemMap.unmap(); +} + TEST_F(MapAllocatorCacheTest, MemoryLeakTest) { std::vector MemMaps; // Fill the cache above MaxEntriesCount to force an eviction