Fix issue where moving BitVector could cause free(nullptr)

If one std::move's a BitVector the old BitVector's storage_ will be
nulled and size set to 0. This is fine but when ~BitVector is run the
allocator will be asked to free a nullptr. Since this is generally not
expected by allocators, not supported by some and breaks the movement
semantics of C++ I've changed the behavior to only Free memory
if there is memory to be freed.

Test: ./test.py --host
Change-Id: I2716a604370c94bcea1a0989c5e6b94e45a2b063

2 files changed, 59 insertions, 1 deletions

diff --git a/libartbase/base/bit_vector.cc b/libartbase/base/bit_vector.cc
index b32b4117dd..8e3d4c9bf7 100644
--- a/libartbase/base/bit_vector.cc
+++ b/libartbase/base/bit_vector.cc
@@ -61,7 +61,10 @@ BitVector::BitVector(const BitVector& src,
 }
 
 BitVector::~BitVector() {
-  allocator_->Free(storage_);
+  if (storage_ != nullptr) {
+    // Only free if we haven't been moved out of.
+    allocator_->Free(storage_);
+  }
 }
 
 bool BitVector::SameBitsSet(const BitVector *src) const {
diff --git a/libartbase/base/bit_vector_test.cc b/libartbase/base/bit_vector_test.cc
index bdba22a203..2ef81c6726 100644
--- a/libartbase/base/bit_vector_test.cc
+++ b/libartbase/base/bit_vector_test.cc
@@ -308,4 +308,59 @@ TEST(BitVector, TransformIterator) {
   }
 }
 
+class SingleAllocator : public Allocator {
+ public:
+  SingleAllocator() : alloc_count_(0), free_count_(0) {}
+  ~SingleAllocator() {
+    EXPECT_EQ(alloc_count_, 1u);
+    EXPECT_EQ(free_count_, 1u);
+  }
+
+  void* Alloc(size_t s) override {
+    EXPECT_LT(s, 1024ull);
+    EXPECT_EQ(alloc_count_, free_count_);
+    ++alloc_count_;
+    return bytes_.begin();
+  }
+
+  void Free(void*) override {
+    ++free_count_;
+  }
+
+  uint32_t AllocCount() const {
+    return alloc_count_;
+  }
+  uint32_t FreeCount() const {
+    return free_count_;
+  }
+
+ private:
+  std::array<uint8_t, 1024> bytes_;
+  uint32_t alloc_count_;
+  uint32_t free_count_;
+};
+
+TEST(BitVector, MovementFree) {
+  SingleAllocator alloc;
+  {
+    BitVector bv(16, false, &alloc);
+    bv.SetBit(13);
+    EXPECT_EQ(alloc.FreeCount(), 0u);
+    EXPECT_EQ(alloc.AllocCount(), 1u);
+    ASSERT_TRUE(bv.GetRawStorage() != nullptr);
+    EXPECT_TRUE(bv.IsBitSet(13));
+    {
+      BitVector bv2(std::move(bv));
+      ASSERT_TRUE(bv.GetRawStorage() == nullptr);
+      EXPECT_TRUE(bv2.IsBitSet(13));
+      EXPECT_EQ(alloc.FreeCount(), 0u);
+      EXPECT_EQ(alloc.AllocCount(), 1u);
+    }
+    EXPECT_EQ(alloc.FreeCount(), 1u);
+    EXPECT_EQ(alloc.AllocCount(), 1u);
+  }
+  EXPECT_EQ(alloc.FreeCount(), 1u);
+  EXPECT_EQ(alloc.AllocCount(), 1u);
+}
+
 }  // namespace art