[PATCH] D51317: Keep BumpPtrAllocator::Allocate(0) from returning nullptr

[Brent Royal-Gordon via Phabricator via llvm-commits]

brentdax updated this revision to Diff 163481.
brentdax added a comment.

I think the best solution is to make BumpPtrAllocator::Allocate() consistently return nullptr for zero-size allocations.
Zero-size allocations are already extremely common in the wild; allocating space unnecessarily just imposes a softer
requirement for users to avoid doing it; a guard page is massive overkill; the noalias guarantee seems more valuable;
and users already had to be prepared to occasionally receive nullptr returns for zero-size allocations anyway.


Repository:
  rL LLVM

https://reviews.llvm.org/D51317

Files:
  include/llvm/Support/Allocator.h
  unittests/Support/AllocatorTest.cpp


Index: unittests/Support/AllocatorTest.cpp
===================================================================
--- unittests/Support/AllocatorTest.cpp
+++ unittests/Support/AllocatorTest.cpp
@@ -185,4 +185,15 @@
   EXPECT_GT(MockSlabAllocator::GetLastSlabSize(), 4096u);
 }
 
+// Test that Allocate() behaves properly when asked to allocate zero bytes.
+TEST(AllocatorTest, EmptyAllocation) {
+  BumpPtrAllocator Alloc;
+
+  void *a = Alloc.Allocate(0, 1);
+  EXPECT_EQ(a, nullptr);
+  
+  void *b = Alloc.Allocate<int>(0);
+  EXPECT_EQ(b, nullptr);
+}
+
 }  // anonymous namespace
Index: include/llvm/Support/Allocator.h
===================================================================
--- include/llvm/Support/Allocator.h
+++ include/llvm/Support/Allocator.h
@@ -209,10 +209,13 @@
     Slabs.erase(std::next(Slabs.begin()), Slabs.end());
   }
 
-  /// Allocate space at the specified alignment.
-  LLVM_ATTRIBUTE_RETURNS_NONNULL LLVM_ATTRIBUTE_RETURNS_NOALIAS void *
+  /// Allocate space at the specified alignment. Returns null if 
+  /// \a Size is zero; this does not indicate an error.
+  LLVM_ATTRIBUTE_RETURNS_NOALIAS void *
   Allocate(size_t Size, size_t Alignment) {
-    assert(Alignment > 0 && "0-byte alignnment is not allowed. Use 1 instead.");
+    assert(Alignment > 0 && "0-byte alignment is not allowed. Use 1 instead.");
+    if (Size == 0)
+      return nullptr;
 
     // Keep track of how many bytes we've allocated.
     BytesAllocated += Size;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D51317.163481.patch
Type: text/x-patch
Size: 1473 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180831/3d5c20f2/attachment.bin>