This is an archive of the discontinued LLVM Phabricator instance.

Differential D121858

[scudo] Don't unmap other low memory pages if mapping doesn't exist
ClosedPublic

Authored by ddcc on Mar 16 2022, 2:15 PM.

Details

Reviewers
vitalybuka
eugenis
cryptoad
mcgrathr
hctim
Commits
rG4787e5c34ec8: [scudo] Don't unmap other low memory pages if mapping doesn't exist
Summary

Tests can register multiple allocators, but only the first will initialize since it initializes the TSDRegistrySharedT. Then, destruction of subsequent allocator may end up unmapping a nullptr PrimaryBase with non-zero PrimarySize.

Diff Detail

Event Timeline

ddcc created this revision.Mar 16 2022, 2:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2022, 2:15 PM
ddcc requested review of this revision.Mar 16 2022, 2:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2022, 2:15 PM
Harbormaster completed remote builds in B154710: Diff 415983.Mar 16 2022, 2:45 PM
vitalybuka accepted this revision.Mar 16 2022, 3:31 PM
vitalybuka added inline comments.
compiler-rt/lib/scudo/standalone/primary64.h
93

please clangformat

This revision is now accepted and ready to land.Mar 16 2022, 3:31 PM
This revision was landed with ongoing or failed builds.Mar 17 2022, 10:17 AM
Closed by commit rG4787e5c34ec8: [scudo] Don't unmap other low memory pages if mapping doesn't exist (authored by ddcc). · Explain Why
This revision was automatically updated to reflect the committed changes.
ddcc added a commit: rG4787e5c34ec8: [scudo] Don't unmap other low memory pages if mapping doesn't exist.

Revision Contents

PathSize
compiler-rt/
lib/
scudo/
standalone/
3 lines

Diff 415983

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

Show First 20 Lines • Show All 83 Lines • ▼ Show 20 Lines void init(s32 ReleaseToOsInterval) {
setOption(Option::ReleaseInterval, static_cast<sptr>(ReleaseToOsInterval)); setOption(Option::ReleaseInterval, static_cast<sptr>(ReleaseToOsInterval));
} }
void unmapTestOnly() { void unmapTestOnly() {
for (uptr I = 0; I < NumClasses; I++) { for (uptr I = 0; I < NumClasses; I++) {
RegionInfo *Region = getRegionInfo(I); RegionInfo *Region = getRegionInfo(I);
*Region = {}; *Region = {};
} }
if (PrimaryBase)
unmap(reinterpret_cast<void *>(PrimaryBase), PrimarySize, UNMAP_ALL, &Data); unmap(reinterpret_cast<void *>(PrimaryBase), PrimarySize, UNMAP_ALL, &Data);
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-      unmap(reinterpret_cast<void *>(PrimaryBase), PrimarySize, UNMAP_ALL, &Data);
+      unmap(reinterpret_cast<void *>(PrimaryBase), PrimarySize, UNMAP_ALL,
+            &Data);
Lint: Pre-merge checks: clang-format: please reformat the code ``` - unmap(reinterpret_cast<void *>(PrimaryBase)…
vitalybukaUnsubmitted
Not Done
ReplyInline Actions

please clangformat

vitalybuka: please clangformat
PrimaryBase = 0U; PrimaryBase = 0U;
} }
TransferBatch *popBatch(CacheT *C, uptr ClassId) { TransferBatch *popBatch(CacheT *C, uptr ClassId) {
DCHECK_LT(ClassId, NumClasses); DCHECK_LT(ClassId, NumClasses);
RegionInfo *Region = getRegionInfo(ClassId); RegionInfo *Region = getRegionInfo(ClassId);
ScopedLock L(Region->Mutex); ScopedLock L(Region->Mutex);
TransferBatch *B = Region->FreeList.front(); TransferBatch *B = Region->FreeList.front();
▲ Show 20 Lines • Show All 389 Lines • Show Last 20 Lines