Skip to content

Commit ef96275

Browse files
authored
[scudo] Allow the quarantine code to be compiled out (#151064)
Add a new configuration option QuarantineDisabled that allows all of the quarantine code to be compiled out. Add new tests that verify that the code is removed properly. On Android, this saves ~4000 bytes for 32 bit and ~6000 bytes for 64 bit. On Android, I used some microbenchmarks that do malloc/free in a loop and for allocations in the primary, the performance is about the same for both 32 bit and 64 bit. For secondary allocations, I saw ~8% speed up on 32 bit and ~3% on 64 bit speed up which feels like it could just be code size improvements.
1 parent 596640a commit ef96275

File tree

5 files changed

+143
-30
lines changed

5 files changed

+143
-30
lines changed

compiler-rt/lib/scudo/standalone/allocator_config.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ BASE_REQUIRED_TEMPLATE_TYPE(SecondaryT)
5454
// Indicates possible support for Memory Tagging.
5555
BASE_OPTIONAL(const bool, MaySupportMemoryTagging, false)
5656

57+
// Disable the quarantine code.
58+
BASE_OPTIONAL(const bool, QuarantineDisabled, false)
59+
5760
// PRIMARY_REQUIRED_TYPE(NAME)
5861
//
5962
// SizeClassMap to use with the Primary.

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ template <typename AllocatorConfig> struct PrimaryConfig {
6060
return BaseConfig<AllocatorConfig>::getMaySupportMemoryTagging();
6161
}
6262

63+
static constexpr bool getQuarantineDisabled() {
64+
return BaseConfig<AllocatorConfig>::getQuarantineDisabled();
65+
}
66+
6367
#define PRIMARY_REQUIRED_TYPE(NAME) \
6468
using NAME = typename AllocatorConfig::Primary::NAME;
6569

@@ -92,6 +96,10 @@ template <typename AllocatorConfig> struct SecondaryConfig {
9296
return BaseConfig<AllocatorConfig>::getMaySupportMemoryTagging();
9397
}
9498

99+
static constexpr bool getQuarantineDisabled() {
100+
return BaseConfig<AllocatorConfig>::getQuarantineDisabled();
101+
}
102+
95103
#define SECONDARY_REQUIRED_TEMPLATE_TYPE(NAME) \
96104
template <typename T> \
97105
using NAME = typename AllocatorConfig::Secondary::template NAME<T>;
@@ -111,6 +119,10 @@ template <typename AllocatorConfig> struct SecondaryConfig {
111119
return BaseConfig<AllocatorConfig>::getMaySupportMemoryTagging();
112120
}
113121

122+
static constexpr bool getQuarantineDisabled() {
123+
return BaseConfig<AllocatorConfig>::getQuarantineDisabled();
124+
}
125+
114126
#define SECONDARY_CACHE_OPTIONAL(TYPE, NAME, DEFAULT) \
115127
OPTIONAL_TEMPLATE(TYPE, NAME, DEFAULT, Cache::NAME) \
116128
static constexpr removeConst<TYPE>::type get##NAME() { \

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

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -184,9 +184,11 @@ class Allocator {
184184
const s32 ReleaseToOsIntervalMs = getFlags()->release_to_os_interval_ms;
185185
Primary.init(ReleaseToOsIntervalMs);
186186
Secondary.init(&Stats, ReleaseToOsIntervalMs);
187-
Quarantine.init(
188-
static_cast<uptr>(getFlags()->quarantine_size_kb << 10),
189-
static_cast<uptr>(getFlags()->thread_local_quarantine_size_kb << 10));
187+
if (!AllocatorConfig::getQuarantineDisabled()) {
188+
Quarantine.init(
189+
static_cast<uptr>(getFlags()->quarantine_size_kb << 10),
190+
static_cast<uptr>(getFlags()->thread_local_quarantine_size_kb << 10));
191+
}
190192
}
191193

192194
void enableRingBuffer() NO_THREAD_SAFETY_ANALYSIS {
@@ -276,16 +278,20 @@ class Allocator {
276278
// the last two items).
277279
void commitBack(TSD<ThisT> *TSD) {
278280
TSD->assertLocked(/*BypassCheck=*/true);
279-
Quarantine.drain(&TSD->getQuarantineCache(),
280-
QuarantineCallback(*this, TSD->getSizeClassAllocator()));
281+
if (!AllocatorConfig::getQuarantineDisabled()) {
282+
Quarantine.drain(&TSD->getQuarantineCache(),
283+
QuarantineCallback(*this, TSD->getSizeClassAllocator()));
284+
}
281285
TSD->getSizeClassAllocator().destroy(&Stats);
282286
}
283287

284288
void drainCache(TSD<ThisT> *TSD) {
285289
TSD->assertLocked(/*BypassCheck=*/true);
286-
Quarantine.drainAndRecycle(
287-
&TSD->getQuarantineCache(),
288-
QuarantineCallback(*this, TSD->getSizeClassAllocator()));
290+
if (!AllocatorConfig::getQuarantineDisabled()) {
291+
Quarantine.drainAndRecycle(
292+
&TSD->getQuarantineCache(),
293+
QuarantineCallback(*this, TSD->getSizeClassAllocator()));
294+
}
289295
TSD->getSizeClassAllocator().drain();
290296
}
291297
void drainCaches() { TSDRegistry.drainCaches(this); }
@@ -612,7 +618,8 @@ class Allocator {
612618
#endif
613619
TSDRegistry.disable();
614620
Stats.disable();
615-
Quarantine.disable();
621+
if (!AllocatorConfig::getQuarantineDisabled())
622+
Quarantine.disable();
616623
Primary.disable();
617624
Secondary.disable();
618625
disableRingBuffer();
@@ -623,7 +630,8 @@ class Allocator {
623630
enableRingBuffer();
624631
Secondary.enable();
625632
Primary.enable();
626-
Quarantine.enable();
633+
if (!AllocatorConfig::getQuarantineDisabled())
634+
Quarantine.enable();
627635
Stats.enable();
628636
TSDRegistry.enable();
629637
#ifdef GWP_ASAN_HOOKS
@@ -1252,7 +1260,8 @@ class Allocator {
12521260
// If the quarantine is disabled, the actual size of a chunk is 0 or larger
12531261
// than the maximum allowed, we return a chunk directly to the backend.
12541262
// This purposefully underflows for Size == 0.
1255-
const bool BypassQuarantine = !Quarantine.getCacheSize() ||
1263+
const bool BypassQuarantine = AllocatorConfig::getQuarantineDisabled() ||
1264+
!Quarantine.getCacheSize() ||
12561265
((Size - 1) >= QuarantineMaxChunkSize) ||
12571266
!Header->ClassId;
12581267
if (BypassQuarantine)
@@ -1642,7 +1651,8 @@ class Allocator {
16421651
uptr getStats(ScopedString *Str) {
16431652
Primary.getStats(Str);
16441653
Secondary.getStats(Str);
1645-
Quarantine.getStats(Str);
1654+
if (!AllocatorConfig::getQuarantineDisabled())
1655+
Quarantine.getStats(Str);
16461656
TSDRegistry.getStats(Str);
16471657
return Str->length();
16481658
}

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

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ class MapAllocatorCache {
312312
break;
313313
}
314314

315-
if (Config::getQuarantineSize()) {
315+
if (!Config::getQuarantineDisabled() && Config::getQuarantineSize()) {
316316
QuarantinePos =
317317
(QuarantinePos + 1) % Max(Config::getQuarantineSize(), 1u);
318318
if (!Quarantine[QuarantinePos].isValid()) {
@@ -508,14 +508,16 @@ class MapAllocatorCache {
508508

509509
void disableMemoryTagging() EXCLUDES(Mutex) {
510510
ScopedLock L(Mutex);
511-
for (u32 I = 0; I != Config::getQuarantineSize(); ++I) {
512-
if (Quarantine[I].isValid()) {
513-
MemMapT &MemMap = Quarantine[I].MemMap;
514-
unmapCallBack(MemMap);
515-
Quarantine[I].invalidate();
511+
if (!Config::getQuarantineDisabled()) {
512+
for (u32 I = 0; I != Config::getQuarantineSize(); ++I) {
513+
if (Quarantine[I].isValid()) {
514+
MemMapT &MemMap = Quarantine[I].MemMap;
515+
unmapCallBack(MemMap);
516+
Quarantine[I].invalidate();
517+
}
516518
}
519+
QuarantinePos = -1U;
517520
}
518-
QuarantinePos = -1U;
519521

520522
for (CachedBlock &Entry : LRUEntries)
521523
Entry.MemMap.setMemoryPermission(Entry.CommitBase, Entry.CommitSize, 0);
@@ -575,8 +577,9 @@ class MapAllocatorCache {
575577
if (!LRUEntries.size() || OldestTime == 0 || OldestTime > Time)
576578
return;
577579
OldestTime = 0;
578-
for (uptr I = 0; I < Config::getQuarantineSize(); I++)
579-
releaseIfOlderThan(Quarantine[I], Time);
580+
if (!Config::getQuarantineDisabled())
581+
for (uptr I = 0; I < Config::getQuarantineSize(); I++)
582+
releaseIfOlderThan(Quarantine[I], Time);
580583
for (uptr I = 0; I < Config::getEntriesArraySize(); I++)
581584
releaseIfOlderThan(Entries[I], Time);
582585
}

compiler-rt/lib/scudo/standalone/tests/combined_test.cpp

Lines changed: 94 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -623,20 +623,20 @@ SCUDO_TYPED_TEST(ScudoCombinedDeathTest, DisableMemoryTagging) {
623623
SCUDO_TYPED_TEST(ScudoCombinedTest, Stats) {
624624
auto *Allocator = this->Allocator.get();
625625

626-
scudo::uptr BufferSize = 8192;
627-
std::vector<char> Buffer(BufferSize);
628-
scudo::uptr ActualSize = Allocator->getStats(Buffer.data(), BufferSize);
629-
while (ActualSize > BufferSize) {
630-
BufferSize = ActualSize + 1024;
631-
Buffer.resize(BufferSize);
632-
ActualSize = Allocator->getStats(Buffer.data(), BufferSize);
626+
std::string Stats(10000, '\0');
627+
scudo::uptr ActualSize = Allocator->getStats(Stats.data(), Stats.size());
628+
if (ActualSize > Stats.size()) {
629+
Stats.resize(ActualSize);
630+
ActualSize = Allocator->getStats(Stats.data(), Stats.size());
633631
}
634-
std::string Stats(Buffer.begin(), Buffer.end());
632+
EXPECT_GE(Stats.size(), ActualSize);
633+
635634
// Basic checks on the contents of the statistics output, which also allows us
636635
// to verify that we got it all.
637636
EXPECT_NE(Stats.find("Stats: SizeClassAllocator"), std::string::npos);
638637
EXPECT_NE(Stats.find("Stats: MapAllocator"), std::string::npos);
639-
EXPECT_NE(Stats.find("Stats: Quarantine"), std::string::npos);
638+
// Do not explicitly check for quarantine stats since a config can disable
639+
// them. Other tests verify this (QuarantineEnabled/QuarantineDisabled).
640640
}
641641

642642
SCUDO_TYPED_TEST_SKIP_THREAD_SAFETY(ScudoCombinedTest, Drain) {
@@ -1076,3 +1076,88 @@ TEST(ScudoCombinedTest, BasicTrustyConfig) {
10761076

10771077
#endif
10781078
#endif
1079+
1080+
struct TestQuarantineSizeClassConfig {
1081+
static const scudo::uptr NumBits = 1;
1082+
static const scudo::uptr MinSizeLog = 10;
1083+
static const scudo::uptr MidSizeLog = 10;
1084+
static const scudo::uptr MaxSizeLog = 13;
1085+
static const scudo::u16 MaxNumCachedHint = 8;
1086+
static const scudo::uptr MaxBytesCachedLog = 12;
1087+
static const scudo::uptr SizeDelta = 0;
1088+
};
1089+
1090+
struct TestQuarantineConfig {
1091+
static const bool MaySupportMemoryTagging = false;
1092+
1093+
template <class A> using TSDRegistryT = scudo::TSDRegistrySharedT<A, 1U, 1U>;
1094+
1095+
struct Primary {
1096+
// Tiny allocator, its Primary only serves chunks of four sizes.
1097+
using SizeClassMap = scudo::FixedSizeClassMap<DeathSizeClassConfig>;
1098+
static const scudo::uptr RegionSizeLog = DeathRegionSizeLog;
1099+
static const scudo::s32 MinReleaseToOsIntervalMs = INT32_MIN;
1100+
static const scudo::s32 MaxReleaseToOsIntervalMs = INT32_MAX;
1101+
typedef scudo::uptr CompactPtrT;
1102+
static const scudo::uptr CompactPtrScale = 0;
1103+
static const bool EnableRandomOffset = true;
1104+
static const scudo::uptr MapSizeIncrement = 1UL << 18;
1105+
static const scudo::uptr GroupSizeLog = 18;
1106+
};
1107+
template <typename Config>
1108+
using PrimaryT = scudo::SizeClassAllocator64<Config>;
1109+
1110+
struct Secondary {
1111+
template <typename Config>
1112+
using CacheT = scudo::MapAllocatorNoCache<Config>;
1113+
};
1114+
1115+
template <typename Config> using SecondaryT = scudo::MapAllocator<Config>;
1116+
};
1117+
1118+
// Verify that the quarantine exists by default.
1119+
TEST(ScudoCombinedTest, QuarantineEnabled) {
1120+
using AllocatorT = scudo::Allocator<TestQuarantineConfig>;
1121+
auto Allocator = std::unique_ptr<AllocatorT>(new AllocatorT());
1122+
1123+
const scudo::uptr Size = 1000U;
1124+
void *P = Allocator->allocate(Size, Origin);
1125+
EXPECT_NE(P, nullptr);
1126+
Allocator->deallocate(P, Origin);
1127+
1128+
std::string Stats(10000, '\0');
1129+
scudo::uptr ActualSize = Allocator->getStats(Stats.data(), Stats.size());
1130+
if (ActualSize > Stats.size()) {
1131+
Stats.resize(ActualSize);
1132+
ActualSize = Allocator->getStats(Stats.data(), Stats.size());
1133+
}
1134+
EXPECT_GE(Stats.size(), ActualSize);
1135+
1136+
// Quarantine stats should be present.
1137+
EXPECT_NE(Stats.find("Stats: Quarantine"), std::string::npos);
1138+
}
1139+
1140+
struct TestQuarantineDisabledConfig : TestQuarantineConfig {
1141+
static const bool QuarantineDisabled = true;
1142+
};
1143+
1144+
TEST(ScudoCombinedTest, QuarantineDisabled) {
1145+
using AllocatorT = scudo::Allocator<TestQuarantineDisabledConfig>;
1146+
auto Allocator = std::unique_ptr<AllocatorT>(new AllocatorT());
1147+
1148+
const scudo::uptr Size = 1000U;
1149+
void *P = Allocator->allocate(Size, Origin);
1150+
EXPECT_NE(P, nullptr);
1151+
Allocator->deallocate(P, Origin);
1152+
1153+
std::string Stats(10000, '\0');
1154+
scudo::uptr ActualSize = Allocator->getStats(Stats.data(), Stats.size());
1155+
if (ActualSize > Stats.size()) {
1156+
Stats.resize(ActualSize);
1157+
ActualSize = Allocator->getStats(Stats.data(), Stats.size());
1158+
}
1159+
EXPECT_GE(Stats.size(), ActualSize);
1160+
1161+
// No quarantine stats should not be present.
1162+
EXPECT_EQ(Stats.find("Stats: Quarantine"), std::string::npos);
1163+
}

0 commit comments

Comments
 (0)