[sanitizer_common] Use 38-bit mmap range for Fuchsia #69387
Merged
Conversation
This file contains hidden or 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
Member
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: None (PiJoules) Changes46cb8d9 unconditionally changed the mmap range to 2^48 for all riscv sanitizers. This changes the allocator tunings for the 32-bit allocator for riscv and led to a severe performance regression for our lsan tests. This effectively revers the tuning change but only for Fuchsia. Once we enable the 64-bit allocator for everything riscv, this value will be irrelevant since it's only relevant for the 32-bit allocator. Full diff: https://github.com/llvm/llvm-project/pull/69387.diff 1 Files Affected:
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform.h b/compiler-rt/lib/sanitizer_common/sanitizer_platform.h
index 5280416f8bd3029..4ee05034bed8f29 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_platform.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform.h
@@ -303,7 +303,15 @@
# define SANITIZER_MMAP_RANGE_SIZE FIRST_32_SECOND_64(1ULL << 32, 1ULL << 40)
# endif
#elif SANITIZER_RISCV64
-# define SANITIZER_MMAP_RANGE_SIZE FIRST_32_SECOND_64(1ULL << 32, 1ULL << 47)
+// FIXME: Rather than hardcoding the VMA here, we should rely on
+// GetMaxUserVirtualAddress(). This will require some refactoring though since
+// many places either hardcode some value or SANITIZER_MMAP_RANGE_SIZE is
+// assumed to be some constant integer.
+# if SANITIZER_FUCHSIA
+# define SANITIZER_MMAP_RANGE_SIZE FIRST_32_SECOND_64(1ULL << 32, 1ULL << 38)
+# else
+# define SANITIZER_MMAP_RANGE_SIZE FIRST_32_SECOND_64(1ULL << 32, 1ULL << 47)
+# endif
#elif defined(__aarch64__)
# if SANITIZER_APPLE
# if SANITIZER_OSX || SANITIZER_IOSSIM
|
46cb8d9 unconditionally changed the mmap range to 2^48 for all riscv sanitizers. This changes the allocator tunings for the 32-bit allocator for riscv and led to a severe performance regression for our lsan tests. This effectively revers the tuning change but only for Fuchsia. Once we enable the 64-bit allocator for everything riscv, this value will be irrelevant since it's only relevant for the 32-bit allocator.
PiJoules
force-pushed
the
continue-to-use-38-vma-for-fuchsia
branch
from
bf11134 to
86cee59
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
46cb8d9 unconditionally changed the mmap range to 2^48 for all riscv sanitizers. This changes the allocator tunings for the 32-bit allocator for riscv and led to a severe performance regression for our lsan tests. This effectively revers the tuning change but only for Fuchsia.
Once we enable the 64-bit allocator for everything riscv, this value will be irrelevant since it's only relevant for the 32-bit allocator.