[memprof] Switch allocator to dynamic base address #98510
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
memprof_rtl.cpp calls InitializeShadowMemory() - which dynamically/"randomly" chooses a base address for the shadow mapping - prior to InitializeAllocator(). If we are unlucky, the shadow memory may be mapped in the same region where the allocator wants to be. This patch fixes the issue by changing the allocator to dynamically choosing a base address, as suggested by Vitaly. For comparison, HWASan already dynamically chooses the base addresses for the shadow mapping and allocator. The "unlucky" failure was observed on a new buildbot (https://lab.llvm.org/buildbot/#/builders/66/builds/1361/steps/17/logs/stdio).
|
@llvm/pr-subscribers-pgo Author: Thurston Dang (thurstond) Changesmemprof_rtl.cpp calls InitializeShadowMemory() - which dynamically/"randomly" chooses a base address for the shadow mapping - prior to InitializeAllocator(). If we are unlucky, the shadow memory may be mapped in the same region where the allocator wants to be. This patch fixes the issue by changing the allocator to dynamically choosing a base address, as suggested by Vitaly. For comparison, HWASan already dynamically chooses the base addresses for the shadow mapping and allocator. The "unlucky" failure was observed on a new buildbot: https://lab.llvm.org/buildbot/#/builders/66/builds/1361/steps/17/logs/stdio Full diff: https://github.com/llvm/llvm-project/pull/98510.diff 1 Files Affected:
diff --git a/compiler-rt/lib/memprof/memprof_allocator.h b/compiler-rt/lib/memprof/memprof_allocator.h
index 89e924f80d41a..ee8034b366b67 100644
--- a/compiler-rt/lib/memprof/memprof_allocator.h
+++ b/compiler-rt/lib/memprof/memprof_allocator.h
@@ -49,7 +49,7 @@ struct MemprofMapUnmapCallback {
#if SANITIZER_APPLE
constexpr uptr kAllocatorSpace = 0x600000000000ULL;
#else
-constexpr uptr kAllocatorSpace = 0x500000000000ULL;
+constexpr uptr kAllocatorSpace = ~(uptr)0;
#endif
constexpr uptr kAllocatorSize = 0x40000000000ULL; // 4T.
typedef DefaultSizeClassMap SizeClassMap;
|
thurstond
added a commit
to thurstond/llvm-project
that referenced
this pull request
Jul 11, 2024
This ports a proposed memprof fix (llvm#98510), which has a shadow memory and allocator layout that is similar to ASan. Although we have only observed the failure for memprof on a buildbot [*], it could theoretically happen for ASan. asan_rtl.cpp calls InitializeShadowMemory() - which dynamically/"randomly" chooses a base address for the shadow mapping - prior to InitializeAllocator(). If we are unlucky, the shadow memory may be mapped in the same region where the allocator wants to be. This patch fixes the issue by changing the allocator to dynamically choosing a base address, as suggested by Vitaly. For comparison, HWASan already dynamically chooses the base addresses for the shadow mapping and allocator. [*] https://lab.llvm.org/buildbot/#/builders/66/builds/1361/steps/17/logs/stdio
vitalybuka
approved these changes
Jul 12, 2024
aaryanshukla
pushed a commit
to aaryanshukla/llvm-project
that referenced
this pull request
Jul 14, 2024
memprof_rtl.cpp calls InitializeShadowMemory() - which dynamically/"randomly" chooses a base address for the shadow mapping - prior to InitializeAllocator(). If we are unlucky, the shadow memory may be mapped in the same region where the allocator wants to be. This patch fixes the issue by changing the allocator to dynamically choosing a base address, as suggested by Vitaly. For comparison, HWASan already dynamically chooses the base addresses for the shadow mapping and allocator. The "unlucky" failure was observed on a new buildbot: https://lab.llvm.org/buildbot/#/builders/66/builds/1361/steps/17/logs/stdio --------- Co-authored-by: Vitaly Buka <vitalybuka@gmail.com>
thurstond
added a commit
that referenced
this pull request
Aug 9, 2024
This ports a fix from memprof (#98510), which has a shadow mapping that is similar to ASan (8 bytes of shadow memory per 64 bytes of app memory). This patch changes the allocator to dynamically choose a base address, as suggested by Vitaly for memprof. This simplifies ASan's #ifdef's and avoids potential conflict in the event that ASan were to switch to a dynamic shadow offset in the future [1]. [1] Since shadow memory is mapped before the allocator is mapped: - dynamic shadow and fixed allocator (old memprof): could fail if "unlucky" (e.g., https://lab.llvm.org/buildbot/#/builders/66/builds/1361/steps/17/logs/stdio) - dynamic shadow and dynamic allocator (HWASan; current memprof): always works - fixed shadow and fixed allocator (current ASan): always works, if constants are carefully chosen - fixed shadow and dynamic allocator (ASan with this patch): always works
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
memprof_rtl.cpp calls InitializeShadowMemory() - which dynamically/"randomly" chooses a base address for the shadow mapping - prior to InitializeAllocator(). If we are unlucky, the shadow memory may be mapped in the same region where the allocator wants to be.
This patch fixes the issue by changing the allocator to dynamically choosing a base address, as suggested by Vitaly. For comparison, HWASan already dynamically chooses the base addresses for the shadow mapping and allocator.
The "unlucky" failure was observed on a new buildbot: https://lab.llvm.org/buildbot/#/builders/66/builds/1361/steps/17/logs/stdio