From d52e9a143f254be7ac1f2e648f3c3dbe278f4711 Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Tue, 2 Sep 2014 21:51:35 +0000 Subject: [PATCH] BumpPtrAllocator: use uintptr_t when aligning addresses to avoid undefined behaviour In theory, alignPtr() could push a pointer beyond the end of the current slab, making comparisons with that pointer undefined behaviour. Use an integer type to avoid this. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@216973 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Support/Allocator.h | 38 ++++++++++++++++------------- include/llvm/Support/MathExtras.h | 9 +++---- unittests/Support/AllocatorTest.cpp | 2 +- 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/include/llvm/Support/Allocator.h b/include/llvm/Support/Allocator.h index 9ea57039a84..05dccec94de 100644 --- a/include/llvm/Support/Allocator.h +++ b/include/llvm/Support/Allocator.h @@ -210,16 +210,17 @@ public: BytesAllocated += Size; // Allocate the aligned space, going forwards from CurPtr. - char *Ptr = alignPtr(CurPtr, Alignment); + uintptr_t AlignedAddr = alignAddr(CurPtr, Alignment); // Check if we can hold it. - if (Ptr + Size <= End) { - CurPtr = Ptr + Size; + if (AlignedAddr + Size <= (uintptr_t)End) { + char *AlignedPtr = (char*)AlignedAddr; + CurPtr = AlignedPtr + Size; // Update the allocation point of this memory block in MemorySanitizer. // Without this, MemorySanitizer messages for values originated from here // will point to the allocation of the entire slab. - __msan_allocated_memory(Ptr, Size); - return Ptr; + __msan_allocated_memory(AlignedPtr, Size); + return AlignedPtr; } // If Size is really big, allocate a separate slab for it. @@ -228,19 +229,22 @@ public: void *NewSlab = Allocator.Allocate(PaddedSize, 0); CustomSizedSlabs.push_back(std::make_pair(NewSlab, PaddedSize)); - Ptr = alignPtr((char *)NewSlab, Alignment); - assert((uintptr_t)Ptr + Size <= (uintptr_t)NewSlab + PaddedSize); - __msan_allocated_memory(Ptr, Size); - return Ptr; + uintptr_t AlignedAddr = alignAddr(NewSlab, Alignment); + assert(AlignedAddr + Size <= (uintptr_t)NewSlab + PaddedSize); + char *AlignedPtr = (char*)AlignedAddr; + __msan_allocated_memory(AlignedPtr, Size); + return AlignedPtr; } // Otherwise, start a new slab and try again. StartNewSlab(); - Ptr = alignPtr(CurPtr, Alignment); - CurPtr = Ptr + Size; - assert(CurPtr <= End && "Unable to allocate memory!"); - __msan_allocated_memory(Ptr, Size); - return Ptr; + AlignedAddr = alignAddr(CurPtr, Alignment); + assert(AlignedAddr + Size <= (uintptr_t)End && + "Unable to allocate memory!"); + char *AlignedPtr = (char*)AlignedAddr; + CurPtr = AlignedPtr + Size; + __msan_allocated_memory(AlignedPtr, Size); + return AlignedPtr; } // Pull in base class overloads. @@ -373,7 +377,7 @@ public: /// all memory allocated so far. void DestroyAll() { auto DestroyElements = [](char *Begin, char *End) { - assert(Begin == alignPtr(Begin, alignOf())); + assert(Begin == (char*)alignAddr(Begin, alignOf())); for (char *Ptr = Begin; Ptr + sizeof(T) <= End; Ptr += sizeof(T)) reinterpret_cast(Ptr)->~T(); }; @@ -382,7 +386,7 @@ public: ++I) { size_t AllocatedSlabSize = BumpPtrAllocator::computeSlabSize( std::distance(Allocator.Slabs.begin(), I)); - char *Begin = alignPtr((char *)*I, alignOf()); + char *Begin = (char*)alignAddr(*I, alignOf()); char *End = *I == Allocator.Slabs.back() ? Allocator.CurPtr : (char *)*I + AllocatedSlabSize; @@ -392,7 +396,7 @@ public: for (auto &PtrAndSize : Allocator.CustomSizedSlabs) { void *Ptr = PtrAndSize.first; size_t Size = PtrAndSize.second; - DestroyElements(alignPtr((char *)Ptr, alignOf()), (char *)Ptr + Size); + DestroyElements((char*)alignAddr(Ptr, alignOf()), (char *)Ptr + Size); } Allocator.Reset(); diff --git a/include/llvm/Support/MathExtras.h b/include/llvm/Support/MathExtras.h index 0abba62a2c2..adfca78fbea 100644 --- a/include/llvm/Support/MathExtras.h +++ b/include/llvm/Support/MathExtras.h @@ -550,16 +550,15 @@ inline uint64_t MinAlign(uint64_t A, uint64_t B) { return (A | B) & (1 + ~(A | B)); } -/// \brief Aligns \c Ptr to \c Alignment bytes, rounding up. +/// \brief Aligns \c Addr to \c Alignment bytes, rounding up. /// /// Alignment should be a power of two. This method rounds up, so -/// AlignPtr(7, 4) == 8 and AlignPtr(8, 4) == 8. -inline char *alignPtr(char *Ptr, size_t Alignment) { +/// alignAddr(7, 4) == 8 and alignAddr(8, 4) == 8. +inline uintptr_t alignAddr(void *Addr, size_t Alignment) { assert(Alignment && isPowerOf2_64((uint64_t)Alignment) && "Alignment is not a power of two!"); - return (char *)(((uintptr_t)Ptr + Alignment - 1) & - ~(uintptr_t)(Alignment - 1)); + return (((uintptr_t)Addr + Alignment - 1) & ~(uintptr_t)(Alignment - 1)); } /// NextPowerOf2 - Returns the next power of two (in 64-bits) diff --git a/unittests/Support/AllocatorTest.cpp b/unittests/Support/AllocatorTest.cpp index dc224925fdd..7789df5dc6f 100644 --- a/unittests/Support/AllocatorTest.cpp +++ b/unittests/Support/AllocatorTest.cpp @@ -130,7 +130,7 @@ public: void *MemBase = malloc(Size + Alignment - 1 + sizeof(void*)); // Find the slab start. - void *Slab = alignPtr((char *)MemBase + sizeof(void *), Alignment); + void *Slab = (void *)alignAddr((char*)MemBase + sizeof(void *), Alignment); // Hold a pointer to the base so we can free the whole malloced block. ((void**)Slab)[-1] = MemBase; -- 2.17.1