[PATCH] Re: Fix llvm::BumpPtrAllocatorImpl::Reset()

[Kal]

Enjoy

Am 14.05.15 um 19:45 schrieb Hans Wennborg:
> On Mon, May 11, 2015 at 10:37 AM, Kal <b17c0de at gmail.com> wrote:
>> BumpPtrAllocator's Reset doesn't act as documented if Slaps.size() == 0 but
>> CustomSizedSlabs.size() != 0.
> Nice find!
>
>>       DeallocateCustomSizedSlabs();
>>       CustomSizedSlabs.clear();
> How about just moving these to the top of the function, before the "if
> (Slabs.empty()" check?
>
> Could we also add a test in unittests/Support/AllocatorTest.cpp?
>
> Cheers,
> Hans

-------------- next part --------------
Index: include/llvm/Support/Allocator.h
===================================================================
--- include/llvm/Support/Allocator.h	(revision 237561)
+++ include/llvm/Support/Allocator.h	(working copy)
@@ -187,6 +187,9 @@
   /// \brief Deallocate all but the current slab and reset the current pointer
   /// to the beginning of it, freeing all memory allocated so far.
   void Reset() {
+    DeallocateCustomSizedSlabs();
+    CustomSizedSlabs.clear();
+
     if (Slabs.empty())
       return;
 
@@ -198,8 +201,6 @@
     // Deallocate all but the first slab, and deallocate all custom-sized slabs.
     DeallocateSlabs(std::next(Slabs.begin()), Slabs.end());
     Slabs.erase(std::next(Slabs.begin()), Slabs.end());
-    DeallocateCustomSizedSlabs();
-    CustomSizedSlabs.clear();
   }
 
   /// \brief Allocate space at the specified alignment.
Index: unittests/Support/AllocatorTest.cpp
===================================================================
--- unittests/Support/AllocatorTest.cpp	(revision 237561)
+++ unittests/Support/AllocatorTest.cpp	(working copy)
@@ -61,6 +61,13 @@
 // again.
 TEST(AllocatorTest, TestReset) {
   BumpPtrAllocator Alloc;
+
+  // Allocate something larger than the SizeThreshold=4096.
+  (void)Alloc.Allocate(5000, 1);
+  Alloc.Reset();
+  // Calling Reset should free all CustomSizedSlabs.
+  EXPECT_EQ(0u, Alloc.GetNumSlabs());
+
   Alloc.Allocate(3000, 1);
   EXPECT_EQ(1U, Alloc.GetNumSlabs());
   Alloc.Allocate(3000, 1);