-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[sanitizer] Warn if allocator size exceeds max user virtual address #152428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This warns the user of incompatible configurations, such as 39-bit and 42-bit VMAs for AArch64 non-Android Linux ASan (llvm#145259).
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Thurston Dang (thurstond) ChangesThis warns the user of incompatible configurations, such as 39-bit and 42-bit VMAs for AArch64 non-Android Linux ASan (#145259). Full diff: https://github.com/llvm/llvm-project/pull/152428.diff 1 Files Affected:
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h
index 51ac1b6ae4975..56d40c19b0f0c 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h
@@ -113,6 +113,18 @@ class SizeClassAllocator64 {
// ~(uptr)0.
void Init(s32 release_to_os_interval_ms, uptr heap_start = 0) {
uptr TotalSpaceSize = kSpaceSize + AdditionalSize();
+
+ uptr MaxAddr = GetMaxUserVirtualAddress();
+ // VReport does not call the sanitizer allocator.
+ VReport(3, "Max user virtual address: 0x%zx\n", MaxAddr);
+ VReport(3, "Total space size for primary allocator: 0x%zx\n",
+ TotalSpaceSize);
+ if (TotalSpaceSize >= MaxAddr)
+ VReport(0, "Error: heap size %zx exceeds max user virtual address %zx\n",
+ TotalSpaceSize, MaxAddr);
+ // We can't easily adjust the requested heap size, because kSpaceSize is
+ // const (for optimization) and used throughout the code.
+
PremappedHeap = heap_start != 0;
if (PremappedHeap) {
CHECK(!kUsingConstantSpaceBeg);
|
VReport(3, "Max user virtual address: 0x%zx\n", MaxAddr); | ||
VReport(3, "Total space size for primary allocator: 0x%zx\n", | ||
TotalSpaceSize); | ||
// TODO: hypothetical edge case: on >48-bit VMA systems, Linux by default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not immediately clear what there is TO DO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is nothing to do in the immediate or even medium-term future. But suppose it's the year 2800: 256TB heaps are commonplace, this warning message keeps popping up. and you get woken from cryogenic sleep because you are the last-known maintainer of Android sanitizers; you'll be glad the TODO is there to hint that the check needs revising.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i mean: if it says TODO, it should clearly state what there is to do. if it's just an explanation of the state of things, remove the TODO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've revised it
This warns the user of incompatible configurations, such as 39-bit and 42-bit VMAs for AArch64 non-Android Linux ASan (#145259).