[llvm] r216973 - BumpPtrAllocator: use uintptr_t when aligning addresses to avoid undefined behaviour

[Hans Wennborg]

Author: hans
Date: Tue Sep  2 16:51:35 2014
New Revision: 216973

URL: http://llvm.org/viewvc/llvm-project?rev=216973&view=rev
Log:
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.

Modified:
    llvm/trunk/include/llvm/Support/Allocator.h
    llvm/trunk/include/llvm/Support/MathExtras.h
    llvm/trunk/unittests/Support/AllocatorTest.cpp

Modified: llvm/trunk/include/llvm/Support/Allocator.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Allocator.h?rev=216973&r1=216972&r2=216973&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/Allocator.h (original)
+++ llvm/trunk/include/llvm/Support/Allocator.h Tue Sep  2 16:51:35 2014
@@ -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<T>()));
+      assert(Begin == (char*)alignAddr(Begin, alignOf<T>()));
       for (char *Ptr = Begin; Ptr + sizeof(T) <= End; Ptr += sizeof(T))
         reinterpret_cast<T *>(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<T>());
+      char *Begin = (char*)alignAddr(*I, alignOf<T>());
       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<T>()), (char *)Ptr + Size);
+      DestroyElements((char*)alignAddr(Ptr, alignOf<T>()), (char *)Ptr + Size);
     }
 
     Allocator.Reset();

Modified: llvm/trunk/include/llvm/Support/MathExtras.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/MathExtras.h?rev=216973&r1=216972&r2=216973&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/MathExtras.h (original)
+++ llvm/trunk/include/llvm/Support/MathExtras.h Tue Sep  2 16:51:35 2014
@@ -550,16 +550,15 @@ inline uint64_t MinAlign(uint64_t A, uin
   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)

Modified: llvm/trunk/unittests/Support/AllocatorTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/AllocatorTest.cpp?rev=216973&r1=216972&r2=216973&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/AllocatorTest.cpp (original)
+++ llvm/trunk/unittests/Support/AllocatorTest.cpp Tue Sep  2 16:51:35 2014
@@ -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;