From ada27614074f52ffdecbdb8c493fb512105c594f Mon Sep 17 00:00:00 2001 From: Kostya Kortchinsky Date: Fri, 30 Sep 2016 19:57:21 +0000 Subject: [PATCH] [scudo] Fix an edge case in the secondary allocator Summary: s/CHECK_LT/CHECK_LE/ in the secondary allocator, as under certain circumstances Ptr + Size can be equal to MapEnd. This edge case was not found by the current tests, so those were extended to be able to catch that. Reviewers: kcc Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D25101 llvm-svn: 282913 --- compiler-rt/lib/scudo/scudo_allocator.cpp | 6 +++--- compiler-rt/lib/scudo/scudo_allocator_secondary.h | 2 +- compiler-rt/lib/scudo/scudo_utils.cpp | 4 ++-- compiler-rt/test/scudo/malloc.cpp | 16 ++++++++++------ compiler-rt/test/scudo/memalign.cpp | 23 ++++++++++++++++------- compiler-rt/test/scudo/realloc.cpp | 5 +++-- 6 files changed, 35 insertions(+), 21 deletions(-) diff --git a/compiler-rt/lib/scudo/scudo_allocator.cpp b/compiler-rt/lib/scudo/scudo_allocator.cpp index 7dd400e..9b86cfd 100644 --- a/compiler-rt/lib/scudo/scudo_allocator.cpp +++ b/compiler-rt/lib/scudo/scudo_allocator.cpp @@ -81,9 +81,9 @@ struct UnpackedHeader { u8 Unused_0_ : 4; // 2nd 8 bytes u64 Offset : 20; // Offset from the beginning of the backend - // allocation to the beginning chunk itself, in - // multiples of MinAlignment. See comment about its - // maximum value and test in init(). + // allocation to the beginning of the chunk itself, + // in multiples of MinAlignment. See comment about + // its maximum value and test in init(). u64 Unused_1_ : 28; u16 Salt : 16; }; diff --git a/compiler-rt/lib/scudo/scudo_allocator_secondary.h b/compiler-rt/lib/scudo/scudo_allocator_secondary.h index 220ce87..ac739c8 100644 --- a/compiler-rt/lib/scudo/scudo_allocator_secondary.h +++ b/compiler-rt/lib/scudo/scudo_allocator_secondary.h @@ -42,7 +42,7 @@ class ScudoLargeMmapAllocator { uptr Ptr = MapBeg + sizeof(SecondaryHeader); // TODO(kostyak): add a random offset to Ptr. CHECK_GT(Ptr + Size, MapBeg); - CHECK_LT(Ptr + Size, MapEnd); + CHECK_LE(Ptr + Size, MapEnd); SecondaryHeader *Header = getHeader(Ptr); Header->MapBeg = MapBeg - PageSize; Header->MapSize = MapSize + 2 * PageSize; diff --git a/compiler-rt/lib/scudo/scudo_utils.cpp b/compiler-rt/lib/scudo/scudo_utils.cpp index f45569e..9e6a351 100644 --- a/compiler-rt/lib/scudo/scudo_utils.cpp +++ b/compiler-rt/lib/scudo/scudo_utils.cpp @@ -34,8 +34,8 @@ namespace __scudo { FORMAT(1, 2) void NORETURN dieWithMessage(const char *Format, ...) { - // Our messages are tiny, 128 characters is more than enough. - char Message[128]; + // Our messages are tiny, 256 characters is more than enough. + char Message[256]; va_list Args; va_start(Args, Format); __sanitizer::VSNPrintf(Message, sizeof(Message), Format, Args); diff --git a/compiler-rt/test/scudo/malloc.cpp b/compiler-rt/test/scudo/malloc.cpp index 4507a52..0f452e3 100644 --- a/compiler-rt/test/scudo/malloc.cpp +++ b/compiler-rt/test/scudo/malloc.cpp @@ -8,20 +8,24 @@ #include #include +#include + int main(int argc, char **argv) { void *p; - size_t size = 1U << 8; + std::vector sizes{1, 1 << 5, 1 << 10, 1 << 15, 1 << 20}; - p = malloc(size); - if (!p) - return 1; - memset(p, 'A', size); - free(p); p = malloc(0); if (!p) return 1; free(p); + for (size_t size : sizes) { + p = malloc(size); + if (!p) + return 1; + memset(p, 'A', size); + free(p); + } return 0; } diff --git a/compiler-rt/test/scudo/memalign.cpp b/compiler-rt/test/scudo/memalign.cpp index 951d1aa..a5fb704 100644 --- a/compiler-rt/test/scudo/memalign.cpp +++ b/compiler-rt/test/scudo/memalign.cpp @@ -15,17 +15,13 @@ extern "C" void *aligned_alloc (size_t alignment, size_t size); int main(int argc, char **argv) { - void *p; + void *p = nullptr; size_t alignment = 1U << 12; - size_t size = alignment; + size_t size = 1U << 12; assert(argc == 2); + if (!strcmp(argv[1], "valid")) { - p = memalign(alignment, size); - if (!p) - return 1; - free(p); - p = nullptr; posix_memalign(&p, alignment, size); if (!p) return 1; @@ -34,6 +30,19 @@ int main(int argc, char **argv) if (!p) return 1; free(p); + // Tests various combinations of alignment and sizes + for (int i = 4; i < 20; i++) { + alignment = 1U << i; + for (int j = 1; j < 33; j++) { + size = 0x800 * j; + for (int k = 0; k < 3; k++) { + p = memalign(alignment, size - (16 * k)); + if (!p) + return 1; + free(p); + } + } + } } if (!strcmp(argv[1], "invalid")) { p = memalign(alignment - 1, size); diff --git a/compiler-rt/test/scudo/realloc.cpp b/compiler-rt/test/scudo/realloc.cpp index 3473470..dfad401 100644 --- a/compiler-rt/test/scudo/realloc.cpp +++ b/compiler-rt/test/scudo/realloc.cpp @@ -20,7 +20,7 @@ int main(int argc, char **argv) { void *p, *old_p; // Those sizes will exercise both allocators (Primary & Secondary). - std::vector sizes{1 << 5, 1 << 17}; + std::vector sizes{1, 1 << 5, 1 << 10, 1 << 15, 1 << 20}; assert(argc == 2); for (size_t size : sizes) { @@ -30,7 +30,8 @@ int main(int argc, char **argv) return 1; size = malloc_usable_size(p); // Our realloc implementation will return the same pointer if the size - // requested is lower or equal to the usable size of the associated chunk. + // requested is lower than or equal to the usable size of the associated + // chunk. p = realloc(p, size - 1); if (p != old_p) return 1; -- 2.7.4