Skip to content

Commit

Permalink
Revert 7a0da88, "scudo: Support memory tagging in the secondary alloc…
Browse files Browse the repository at this point in the history 
…ator."

We measured a 2.5 seconds (17.5%) regression in Android boot time
performance with this change.
  • Loading branch information
@pcc
pcc committed Feb 26, 2021
1 parent d7fca3f commit 9678b07
Show file tree
Hide file tree
Showing 12 changed files with 229 additions and 473 deletions.
3 changes: 0 additions & 3 deletions compiler-rt/lib/scudo/standalone/allocator_config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ struct DefaultConfig {

typedef MapAllocatorCache<DefaultConfig> SecondaryCache;
static const u32 SecondaryCacheEntriesArraySize = 32U;
static const u32 SecondaryCacheQuarantineSize = 0U;
static const u32 SecondaryCacheDefaultMaxEntriesCount = 32U;
static const uptr SecondaryCacheDefaultMaxEntrySize = 1UL << 19;
static const s32 SecondaryCacheMinReleaseToOsIntervalMs = INT32_MIN;
Expand Down Expand Up @@ -99,7 +98,6 @@ struct AndroidConfig {

typedef MapAllocatorCache<AndroidConfig> SecondaryCache;
static const u32 SecondaryCacheEntriesArraySize = 256U;
static const u32 SecondaryCacheQuarantineSize = 32U;
static const u32 SecondaryCacheDefaultMaxEntriesCount = 32U;
static const uptr SecondaryCacheDefaultMaxEntrySize = 2UL << 20;
static const s32 SecondaryCacheMinReleaseToOsIntervalMs = 0;
Expand Down Expand Up @@ -128,7 +126,6 @@ struct AndroidSvelteConfig {

typedef MapAllocatorCache<AndroidSvelteConfig> SecondaryCache;
static const u32 SecondaryCacheEntriesArraySize = 16U;
static const u32 SecondaryCacheQuarantineSize = 32U;
static const u32 SecondaryCacheDefaultMaxEntriesCount = 4U;
static const uptr SecondaryCacheDefaultMaxEntrySize = 1UL << 18;
static const s32 SecondaryCacheMinReleaseToOsIntervalMs = 0;
Expand Down
120 changes: 32 additions & 88 deletions compiler-rt/lib/scudo/standalone/combined.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,12 @@ class Allocator {
NewHeader.State = Chunk::State::Available;
Chunk::compareExchangeHeader(Allocator.Cookie, Ptr, &NewHeader, &Header);

if (allocatorSupportsMemoryTagging<Params>())
Ptr = untagPointer(Ptr);
void *BlockBegin = Allocator::getBlockBegin(Ptr, &NewHeader);
Cache.deallocate(NewHeader.ClassId, BlockBegin);
const uptr ClassId = NewHeader.ClassId;
if (LIKELY(ClassId))
Cache.deallocate(ClassId, BlockBegin);
else
Allocator.Secondary.deallocate(BlockBegin);
}

// We take a shortcut when allocating a quarantine batch by working with the
Expand Down Expand Up @@ -236,26 +238,11 @@ class Allocator {
TSD->Cache.destroy(&Stats);
}

ALWAYS_INLINE void *getHeaderTaggedPointer(void *Ptr) {
if (!allocatorSupportsMemoryTagging<Params>())
return Ptr;
auto UntaggedPtr = untagPointer(Ptr);
if (UntaggedPtr != Ptr)
return UntaggedPtr;
// Secondary, or pointer allocated while memory tagging is unsupported or
// disabled. The tag mismatch is okay in the latter case because tags will
// not be checked.
return addHeaderTag(Ptr);
}

ALWAYS_INLINE uptr addHeaderTag(uptr Ptr) {
if (!allocatorSupportsMemoryTagging<Params>())
return Ptr;
return addFixedTag(Ptr, 2);
}

ALWAYS_INLINE void *addHeaderTag(void *Ptr) {
return reinterpret_cast<void *>(addHeaderTag(reinterpret_cast<uptr>(Ptr)));
ALWAYS_INLINE void *untagPointerMaybe(void *Ptr) {
if (allocatorSupportsMemoryTagging<Params>())
return reinterpret_cast<void *>(
untagPointer(reinterpret_cast<uptr>(Ptr)));
return Ptr;
}

NOINLINE u32 collectStackTrace() {
Expand Down Expand Up @@ -352,7 +339,7 @@ class Allocator {
TSD->unlock();
}
if (UNLIKELY(ClassId == 0))
Block = Secondary.allocate(Options, Size, Alignment, &SecondaryBlockEnd,
Block = Secondary.allocate(NeededSize, Alignment, &SecondaryBlockEnd,
FillContents);

if (UNLIKELY(!Block)) {
Expand Down Expand Up @@ -448,21 +435,12 @@ class Allocator {
TaggedPtr = prepareTaggedChunk(Ptr, Size, OddEvenMask, BlockEnd);
}
storeAllocationStackMaybe(Options, Ptr);
} else {
Block = addHeaderTag(Block);
Ptr = addHeaderTag(Ptr);
if (UNLIKELY(FillContents != NoFill)) {
// This condition is not necessarily unlikely, but since memset is
// costly, we might as well mark it as such.
memset(Block, FillContents == ZeroFill ? 0 : PatternFillByte,
PrimaryT::getSizeByClassId(ClassId));
}
} else if (UNLIKELY(FillContents != NoFill)) {
// This condition is not necessarily unlikely, but since memset is
// costly, we might as well mark it as such.
memset(Block, FillContents == ZeroFill ? 0 : PatternFillByte,
PrimaryT::getSizeByClassId(ClassId));
}
} else {
Block = addHeaderTag(Block);
Ptr = addHeaderTag(Ptr);
if (UNLIKELY(useMemoryTagging<Params>(Options)))
storeTags(reinterpret_cast<uptr>(Block), reinterpret_cast<uptr>(Ptr));
}

Chunk::UnpackedHeader Header = {};
Expand Down Expand Up @@ -516,7 +494,7 @@ class Allocator {
if (UNLIKELY(!isAligned(reinterpret_cast<uptr>(Ptr), MinAlignment)))
reportMisalignedPointer(AllocatorAction::Deallocating, Ptr);

Ptr = getHeaderTaggedPointer(Ptr);
Ptr = untagPointerMaybe(Ptr);

Chunk::UnpackedHeader Header;
Chunk::loadHeader(Cookie, Ptr, &Header);
Expand Down Expand Up @@ -555,7 +533,7 @@ class Allocator {
}

void *OldTaggedPtr = OldPtr;
OldPtr = getHeaderTaggedPointer(OldPtr);
OldPtr = untagPointerMaybe(OldPtr);

// The following cases are handled by the C wrappers.
DCHECK_NE(OldPtr, nullptr);
Expand Down Expand Up @@ -591,7 +569,7 @@ class Allocator {
Chunk::Origin::Malloc);
}

void *BlockBegin = getBlockBegin(OldTaggedPtr, &OldHeader);
void *BlockBegin = getBlockBegin(OldPtr, &OldHeader);
uptr BlockEnd;
uptr OldSize;
const uptr ClassId = OldHeader.ClassId;
Expand All @@ -601,19 +579,18 @@ class Allocator {
OldSize = OldHeader.SizeOrUnusedBytes;
} else {
BlockEnd = SecondaryT::getBlockEnd(BlockBegin);
OldSize = BlockEnd - (reinterpret_cast<uptr>(OldTaggedPtr) +
OldHeader.SizeOrUnusedBytes);
OldSize = BlockEnd -
(reinterpret_cast<uptr>(OldPtr) + OldHeader.SizeOrUnusedBytes);
}
// If the new chunk still fits in the previously allocated block (with a
// reasonable delta), we just keep the old block, and update the chunk
// header to reflect the size change.
if (reinterpret_cast<uptr>(OldTaggedPtr) + NewSize <= BlockEnd) {
if (reinterpret_cast<uptr>(OldPtr) + NewSize <= BlockEnd) {
if (NewSize > OldSize || (OldSize - NewSize) < getPageSizeCached()) {
Chunk::UnpackedHeader NewHeader = OldHeader;
NewHeader.SizeOrUnusedBytes =
(ClassId ? NewSize
: BlockEnd -
(reinterpret_cast<uptr>(OldTaggedPtr) + NewSize)) &
: BlockEnd - (reinterpret_cast<uptr>(OldPtr) + NewSize)) &
Chunk::SizeOrUnusedBytesMask;
Chunk::compareExchangeHeader(Cookie, OldPtr, &NewHeader, &OldHeader);
if (UNLIKELY(ClassId && useMemoryTagging<Params>(Options))) {
Expand Down Expand Up @@ -706,30 +683,14 @@ class Allocator {
initThreadMaybe();
const uptr From = Base;
const uptr To = Base + Size;
bool MayHaveTaggedPrimary = allocatorSupportsMemoryTagging<Params>() &&
systemSupportsMemoryTagging();
auto Lambda = [this, From, To, MayHaveTaggedPrimary, Callback,
Arg](uptr Block) {
auto Lambda = [this, From, To, Callback, Arg](uptr Block) {
if (Block < From || Block >= To)
return;
uptr Chunk;
Chunk::UnpackedHeader Header;
if (MayHaveTaggedPrimary) {
// A chunk header can either have a zero tag (tagged primary) or the
// header tag (secondary, or untagged primary). We don't know which so
// try both.
ScopedDisableMemoryTagChecks x;
if (!getChunkFromBlock(Block, &Chunk, &Header) &&
!getChunkFromBlock(addHeaderTag(Block), &Chunk, &Header))
return;
} else {
if (!getChunkFromBlock(addHeaderTag(Block), &Chunk, &Header))
return;
}
if (Header.State == Chunk::State::Allocated) {
if (getChunkFromBlock(Block, &Chunk, &Header) &&
Header.State == Chunk::State::Allocated) {
uptr TaggedChunk = Chunk;
if (allocatorSupportsMemoryTagging<Params>())
TaggedChunk = untagPointer(TaggedChunk);
if (useMemoryTagging<Params>(Primary.Options.load()))
TaggedChunk = loadTag(Chunk);
Callback(TaggedChunk, getSize(reinterpret_cast<void *>(Chunk), &Header),
Expand Down Expand Up @@ -790,7 +751,7 @@ class Allocator {
return GuardedAlloc.getSize(Ptr);
#endif // GWP_ASAN_HOOKS

Ptr = getHeaderTaggedPointer(const_cast<void *>(Ptr));
Ptr = untagPointerMaybe(const_cast<void *>(Ptr));
Chunk::UnpackedHeader Header;
Chunk::loadHeader(Cookie, Ptr, &Header);
// Getting the usable size of a chunk only makes sense if it's allocated.
Expand All @@ -815,7 +776,7 @@ class Allocator {
#endif // GWP_ASAN_HOOKS
if (!Ptr || !isAligned(reinterpret_cast<uptr>(Ptr), MinAlignment))
return false;
Ptr = getHeaderTaggedPointer(const_cast<void *>(Ptr));
Ptr = untagPointerMaybe(const_cast<void *>(Ptr));
Chunk::UnpackedHeader Header;
return Chunk::isValid(Cookie, Ptr, &Header) &&
Header.State == Chunk::State::Allocated;
Expand All @@ -825,17 +786,8 @@ class Allocator {
return useMemoryTagging<Params>(Primary.Options.load());
}
void disableMemoryTagging() {
// If we haven't been initialized yet, we need to initialize now in order to
// prevent a future call to initThreadMaybe() from enabling memory tagging
// based on feature detection. But don't call initThreadMaybe() because it
// may end up calling the allocator (via pthread_atfork, via the post-init
// callback), which may cause mappings to be created with memory tagging
// enabled.
TSDRegistry.initOnceMaybe(this);
if (allocatorSupportsMemoryTagging<Params>()) {
Secondary.disableMemoryTagging();
if (allocatorSupportsMemoryTagging<Params>())
Primary.Options.clear(OptionBit::UseMemoryTagging);
}
}

void setTrackAllocationStacks(bool Track) {
Expand Down Expand Up @@ -1076,8 +1028,6 @@ class Allocator {
const uptr SizeOrUnusedBytes = Header->SizeOrUnusedBytes;
if (LIKELY(Header->ClassId))
return SizeOrUnusedBytes;
if (allocatorSupportsMemoryTagging<Params>())
Ptr = untagPointer(const_cast<void *>(Ptr));
return SecondaryT::getBlockEnd(getBlockBegin(Ptr, Header)) -
reinterpret_cast<uptr>(Ptr) - SizeOrUnusedBytes;
}
Expand All @@ -1103,14 +1053,11 @@ class Allocator {
// 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() ||
((Size - 1) >= QuarantineMaxChunkSize) ||
!NewHeader.ClassId;
const bool BypassQuarantine =
!Quarantine.getCacheSize() || ((Size - 1) >= QuarantineMaxChunkSize);
if (BypassQuarantine) {
NewHeader.State = Chunk::State::Available;
Chunk::compareExchangeHeader(Cookie, Ptr, &NewHeader, Header);
if (allocatorSupportsMemoryTagging<Params>())
Ptr = untagPointer(Ptr);
void *BlockBegin = getBlockBegin(Ptr, &NewHeader);
const uptr ClassId = NewHeader.ClassId;
if (LIKELY(ClassId)) {
Expand All @@ -1120,10 +1067,7 @@ class Allocator {
if (UnlockRequired)
TSD->unlock();
} else {
if (UNLIKELY(useMemoryTagging<Params>(Options)))
storeTags(reinterpret_cast<uptr>(BlockBegin),
reinterpret_cast<uptr>(Ptr));
Secondary.deallocate(Options, BlockBegin);
Secondary.deallocate(BlockBegin);
}
} else {
NewHeader.State = Chunk::State::Quarantined;
Expand Down
3 changes: 0 additions & 3 deletions compiler-rt/lib/scudo/standalone/common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,6 @@ void *map(void *Addr, uptr Size, const char *Name, uptr Flags = 0,
void unmap(void *Addr, uptr Size, uptr Flags = 0,
MapPlatformData *Data = nullptr);

void setMemoryPermission(uptr Addr, uptr Size, uptr Flags,
MapPlatformData *Data = nullptr);

void releasePagesToOS(uptr BaseAddress, uptr Offset, uptr Size,
MapPlatformData *Data = nullptr);

Expand Down
10 changes: 0 additions & 10 deletions compiler-rt/lib/scudo/standalone/fuchsia.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,16 +134,6 @@ void unmap(void *Addr, uptr Size, uptr Flags, MapPlatformData *Data) {
}
}

void setMemoryPermission(UNUSED uptr Addr, UNUSED uptr Size, UNUSED uptr Flags,
UNUSED MapPlatformData *Data) {
const zx_vm_option_t Prot =
(Flags & MAP_NOACCESS) ? 0 : (ZX_VM_PERM_READ | ZX_VM_PERM_WRITE);
DCHECK(Data);
DCHECK_NE(Data->Vmar, ZX_HANDLE_INVALID);
if (_zx_vmar_protect(Data->Vmar, Prot, Addr, Size) != ZX_OK)
dieOnMapUnmapError();
}

void releasePagesToOS(UNUSED uptr BaseAddress, uptr Offset, uptr Size,
MapPlatformData *Data) {
DCHECK(Data);
Expand Down
15 changes: 4 additions & 11 deletions compiler-rt/lib/scudo/standalone/linux.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,14 @@ void *map(void *Addr, uptr Size, UNUSED const char *Name, uptr Flags,
MmapProt = PROT_NONE;
} else {
MmapProt = PROT_READ | PROT_WRITE;
}
#if defined(__aarch64__)
#ifndef PROT_MTE
#define PROT_MTE 0x20
#endif
if (Flags & MAP_MEMTAG)
MmapProt |= PROT_MTE;
if (Flags & MAP_MEMTAG)
MmapProt |= PROT_MTE;
#endif
}
if (Addr) {
// Currently no scenario for a noaccess mapping with a fixed address.
DCHECK_EQ(Flags & MAP_NOACCESS, 0);
Expand All @@ -70,7 +70,7 @@ void *map(void *Addr, uptr Size, UNUSED const char *Name, uptr Flags,
return nullptr;
}
#if SCUDO_ANDROID
if (Name)
if (!(Flags & MAP_NOACCESS))
prctl(ANDROID_PR_SET_VMA, ANDROID_PR_SET_VMA_ANON_NAME, P, Size, Name);
#endif
return P;
Expand All @@ -82,13 +82,6 @@ void unmap(void *Addr, uptr Size, UNUSED uptr Flags,
dieOnMapUnmapError();
}

void setMemoryPermission(uptr Addr, uptr Size, uptr Flags,
UNUSED MapPlatformData *Data) {
int Prot = (Flags & MAP_NOACCESS) ? PROT_NONE : (PROT_READ | PROT_WRITE);
if (mprotect(reinterpret_cast<void *>(Addr), Size, Prot) != 0)
dieOnMapUnmapError();
}

void releasePagesToOS(uptr BaseAddress, uptr Offset, uptr Size,
UNUSED MapPlatformData *Data) {
void *Addr = reinterpret_cast<void *>(BaseAddress + Offset);
Expand Down
20 changes: 0 additions & 20 deletions compiler-rt/lib/scudo/standalone/memtag.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,7 @@ void setRandomTag(void *Ptr, uptr Size, uptr ExcludeMask, uptr *TaggedBegin,

#if defined(__aarch64__) || defined(SCUDO_FUZZ)

// We assume that Top-Byte Ignore is enabled if the architecture supports memory
// tagging. Not all operating systems enable TBI, so we only claim architectural
// support for memory tagging if the operating system enables TBI.
#if SCUDO_LINUX
inline constexpr bool archSupportsMemoryTagging() { return true; }
#else
inline constexpr bool archSupportsMemoryTagging() { return false; }
#endif

inline constexpr uptr archMemoryTagGranuleSize() { return 16; }

inline uptr untagPointer(uptr Ptr) { return Ptr & ((1ULL << 56) - 1); }
Expand Down Expand Up @@ -128,8 +120,6 @@ inline uptr selectRandomTag(uptr Ptr, uptr ExcludeMask) {
return TaggedPtr;
}

inline uptr addFixedTag(uptr Ptr, uptr Tag) { return Ptr | (Tag << 56); }

inline uptr storeTags(uptr Begin, uptr End) {
DCHECK(Begin % 16 == 0);
if (Begin != End) {
Expand Down Expand Up @@ -255,12 +245,6 @@ inline uptr selectRandomTag(uptr Ptr, uptr ExcludeMask) {
UNREACHABLE("memory tagging not supported");
}

inline uptr addFixedTag(uptr Ptr, uptr Tag) {
(void)Ptr;
(void)Tag;
UNREACHABLE("memory tagging not supported");
}

inline uptr storeTags(uptr Begin, uptr End) {
(void)Begin;
(void)End;
Expand Down Expand Up @@ -296,10 +280,6 @@ inline void setRandomTag(void *Ptr, uptr Size, uptr ExcludeMask,
*TaggedEnd = storeTags(*TaggedBegin, *TaggedBegin + Size);
}

inline void *untagPointer(void *Ptr) {
return reinterpret_cast<void *>(untagPointer(reinterpret_cast<uptr>(Ptr)));
}

template <typename Config>
inline constexpr bool allocatorSupportsMemoryTagging() {
return archSupportsMemoryTagging() && Config::MaySupportMemoryTagging;
Expand Down
2 changes: 1 addition & 1 deletion compiler-rt/lib/scudo/standalone/primary64.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ template <typename Config> class SizeClassAllocator64 {
void initLinkerInitialized(s32 ReleaseToOsInterval) {
// Reserve the space required for the Primary.
PrimaryBase = reinterpret_cast<uptr>(
map(nullptr, PrimarySize, nullptr, MAP_NOACCESS, &Data));
map(nullptr, PrimarySize, "scudo:primary", MAP_NOACCESS, &Data));

u32 Seed;
const u64 Time = getMonotonicTime();
Expand Down

0 comments on commit 9678b07

Please sign in to comment.