[libc-commits] [libc] 3c21074 - [NFC][libc][malloc] Refactor Block (#100445)

via libc-commits libc-commits at lists.llvm.org
Thu Jul 25 11:23:41 PDT 2024


Author: Daniel Thornburgh
Date: 2024-07-25T11:23:38-07:00
New Revision: 3c210740649a538d2fc2432395f2ef41c56dc8b6

URL: https://github.com/llvm/llvm-project/commit/3c210740649a538d2fc2432395f2ef41c56dc8b6
DIFF: https://github.com/llvm/llvm-project/commit/3c210740649a538d2fc2432395f2ef41c56dc8b6.diff

LOG: [NFC][libc][malloc] Refactor Block (#100445)

This decreases the surface area of the block implementation in
preparation for deeper changes to its implementation.

See #98096 

- Remove dead member functions.
- Remove last() check from next(), as described in its comment.
- Rework object lifetimes such that only block headers are actually
considered live. This simplifies their implementation.
- The allocated storage becomes live at the outer call to malloc-family
functions via a special case in the C++ standard.
- Add asserts for flag properties required by the implementation.
- Remove static from member functions that don't invalidate the block.

Added: 
    

Modified: 
    libc/src/__support/block.h
    libc/src/__support/freelist_heap.h
    libc/test/src/__support/block_test.cpp

Removed: 
    


################################################################################
diff  --git a/libc/src/__support/block.h b/libc/src/__support/block.h
index 86cb4bd7ad582..6e7186f3224b9 100644
--- a/libc/src/__support/block.h
+++ b/libc/src/__support/block.h
@@ -159,13 +159,10 @@ class Block {
     return reinterpret_cast<const cpp::byte *>(this) + BLOCK_OVERHEAD;
   }
 
-  /// Marks the block as free and merges it with any free neighbors.
-  ///
-  /// This method is static in order to consume and replace the given block
-  /// pointer. If neither member is free, the returned pointer will point to the
-  /// original block. Otherwise, it will point to the new, larger block created
-  /// by merging adjacent free blocks together.
-  static void free(Block *&block);
+  // @returns The region of memory the block manages, including the header.
+  ByteSpan region() {
+    return {reinterpret_cast<cpp::byte *>(this), outer_size()};
+  }
 
   /// Attempts to split this block.
   ///
@@ -176,75 +173,34 @@ class Block {
   /// This method may fail if the remaining space is too small to hold a new
   /// block. If this method fails for any reason, the original block is
   /// unmodified.
-  ///
-  /// This method is static in order to consume and replace the given block
-  /// pointer with a pointer to the new, smaller block.
-  static optional<Block *> split(Block *&block, size_t new_inner_size);
+  optional<Block *> split(size_t new_inner_size);
 
   /// Merges this block with the one that comes after it.
-  ///
-  /// This method is static in order to consume and replace the given block
-  /// pointer with a pointer to the new, larger block.
-  static bool merge_next(Block *&block);
+  bool merge_next();
 
-  /// Fetches the block immediately after this one.
-  ///
-  /// For performance, this always returns a block pointer, even if the returned
-  /// pointer is invalid. The pointer is valid if and only if `last()` is false.
-  ///
-  /// Typically, after calling `Init` callers may save a pointer past the end of
-  /// the list using `next()`. This makes it easy to subsequently iterate over
-  /// the list:
-  /// @code{.cpp}
-  ///   auto result = Block<>::init(byte_span);
-  ///   Block<>* begin = *result;
-  ///   Block<>* end = begin->next();
-  ///   ...
-  ///   for (auto* block = begin; block != end; block = block->next()) {
-  ///     // Do something which each block.
-  ///   }
-  /// @endcode
+  /// @returns The block immediately after this one, or a null pointer if this
+  /// is the last block.
   Block *next() const;
 
-  /// @copydoc `next`.
-  static Block *next_block(const Block *block) {
-    return block == nullptr ? nullptr : block->next();
-  }
-
   /// @returns The block immediately before this one, or a null pointer if this
   /// is the first block.
   Block *prev() const;
 
-  /// @copydoc `prev`.
-  static Block *prev_block(const Block *block) {
-    return block == nullptr ? nullptr : block->prev();
-  }
-
   /// Indicates whether the block is in use.
   ///
   /// @returns `true` if the block is in use or `false` if not.
   bool used() const { return next_ & USED_MASK; }
 
-  /// Indicates whether this block is the last block or not (i.e. whether
-  /// `next()` points to a valid block or not). This is needed because
-  /// `next()` points to the end of this block, whether there is a valid
-  /// block there or not.
-  ///
-  /// @returns `true` is this is the last block or `false` if not.
-  bool last() const { return next_ & LAST_MASK; }
-
   /// Marks this block as in use.
   void mark_used() { next_ |= USED_MASK; }
 
   /// Marks this block as free.
   void mark_free() { next_ &= ~USED_MASK; }
 
-  /// Marks this block as the last one in the chain.
+  /// Marks this block as the last one in the chain. Makes next() return
+  /// nullptr.
   constexpr void mark_last() { next_ |= LAST_MASK; }
 
-  /// Clears the last bit from this block.
-  void clear_last() { next_ &= ~LAST_MASK; }
-
   /// @brief Checks if a block is valid.
   ///
   /// @returns `true` if and only if the following conditions are met:
@@ -314,10 +270,8 @@ class Block {
   static BlockInfo allocate(Block *block, size_t alignment, size_t size);
 
 private:
-  /// Consumes the block and returns as a span of bytes.
-  static ByteSpan as_bytes(Block *&&block);
-
-  /// Consumes the span of bytes and uses it to construct and return a block.
+  /// Construct a block to represent a span of bytes. Overwrites only enough
+  /// memory for the block header; the rest of the span is left alone.
   static Block *as_block(size_t prev_outer_size, ByteSpan bytes);
 
   /// Returns a `BlockStatus` that is either VALID or indicates the reason why
@@ -329,7 +283,7 @@ class Block {
 
   /// Like `split`, but assumes the caller has already checked to parameters to
   /// ensure the split will succeed.
-  static Block *split_impl(Block *&block, size_t new_inner_size);
+  Block *split_impl(size_t new_inner_size);
 
   /// Offset from this block to the previous block. 0 if this is the first
   /// block.
@@ -389,20 +343,6 @@ Block<OffsetType, kAlign>::init(ByteSpan region) {
   return block;
 }
 
-template <typename OffsetType, size_t kAlign>
-void Block<OffsetType, kAlign>::free(Block *&block) {
-  if (block == nullptr)
-    return;
-
-  block->mark_free();
-  Block *prev = block->prev();
-
-  if (merge_next(prev))
-    block = prev;
-
-  merge_next(block);
-}
-
 template <typename OffsetType, size_t kAlign>
 bool Block<OffsetType, kAlign>::can_allocate(size_t alignment,
                                              size_t size) const {
@@ -436,7 +376,7 @@ Block<OffsetType, kAlign>::allocate(Block *block, size_t alignment,
 
     Block *original = info.block;
     optional<Block *> maybe_aligned_block =
-        Block::split(original, adjustment - BLOCK_OVERHEAD);
+        original->split(adjustment - BLOCK_OVERHEAD);
     LIBC_ASSERT(maybe_aligned_block.has_value() &&
                 "This split should always result in a new block. The check in "
                 "`can_allocate` ensures that we have enough space here to make "
@@ -445,7 +385,7 @@ Block<OffsetType, kAlign>::allocate(Block *block, size_t alignment,
     if (Block *prev = original->prev()) {
       // If there is a block before this, we can merge the current one with the
       // newly created one.
-      merge_next(prev);
+      prev->merge_next();
     } else {
       // Otherwise, this was the very first block in the chain. Now we can make
       // it the new first block.
@@ -459,7 +399,7 @@ Block<OffsetType, kAlign>::allocate(Block *block, size_t alignment,
   }
 
   // Now get a block for the requested size.
-  if (optional<Block *> next = Block::split(info.block, size))
+  if (optional<Block *> next = info.block->split(size))
     info.next = *next;
 
   return info;
@@ -467,14 +407,11 @@ Block<OffsetType, kAlign>::allocate(Block *block, size_t alignment,
 
 template <typename OffsetType, size_t kAlign>
 optional<Block<OffsetType, kAlign> *>
-Block<OffsetType, kAlign>::split(Block *&block, size_t new_inner_size) {
-  if (block == nullptr)
+Block<OffsetType, kAlign>::split(size_t new_inner_size) {
+  if (used())
     return {};
 
-  if (block->used())
-    return {};
-
-  size_t old_inner_size = block->inner_size();
+  size_t old_inner_size = inner_size();
   new_inner_size = align_up(new_inner_size, ALIGNMENT);
   if (old_inner_size < new_inner_size)
     return {};
@@ -482,61 +419,56 @@ Block<OffsetType, kAlign>::split(Block *&block, size_t new_inner_size) {
   if (old_inner_size - new_inner_size < BLOCK_OVERHEAD)
     return {};
 
-  return split_impl(block, new_inner_size);
+  return split_impl(new_inner_size);
 }
 
 template <typename OffsetType, size_t kAlign>
 Block<OffsetType, kAlign> *
-Block<OffsetType, kAlign>::split_impl(Block *&block, size_t new_inner_size) {
-  size_t prev_outer_size = block->prev_;
+Block<OffsetType, kAlign>::split_impl(size_t new_inner_size) {
   size_t outer_size1 = new_inner_size + BLOCK_OVERHEAD;
-  bool is_last = block->last();
-  ByteSpan bytes = as_bytes(cpp::move(block));
-  Block *block1 = as_block(prev_outer_size, bytes.subspan(0, outer_size1));
-  Block *block2 = as_block(outer_size1, bytes.subspan(outer_size1));
-
-  if (is_last)
-    block2->mark_last();
-  else
-    block2->next()->prev_ = block2->next_;
-
-  block = cpp::move(block1);
-  return block2;
+  bool has_next = next();
+  ByteSpan new_region = region().subspan(outer_size1);
+  LIBC_ASSERT(!used() && "used blocks cannot be split");
+  // The low order bits of outer_size1 should both be zero, and is the correct
+  // value for the flags is false.
+  next_ = outer_size1;
+  LIBC_ASSERT(!used() && next() && "incorrect first split flags");
+  Block *new_block = as_block(outer_size1, new_region);
+
+  if (has_next) {
+    // The two flags are both false, so next_ is a plain size.
+    LIBC_ASSERT(!new_block->used() && next() && "flags disrupt use of size");
+    new_block->next()->prev_ = new_block->next_;
+  } else {
+    new_block->mark_last();
+  }
+  return new_block;
 }
 
 template <typename OffsetType, size_t kAlign>
-bool Block<OffsetType, kAlign>::merge_next(Block *&block) {
-  if (block == nullptr)
-    return false;
-
-  if (block->last())
-    return false;
-
-  Block *next = block->next();
-  if (block->used() || next->used())
+bool Block<OffsetType, kAlign>::merge_next() {
+  if (used() || !next() || next()->used())
     return false;
 
-  size_t prev_outer_size = block->prev_;
-  bool is_last = next->last();
-  ByteSpan prev_bytes = as_bytes(cpp::move(block));
-  ByteSpan next_bytes = as_bytes(cpp::move(next));
-  size_t outer_size = prev_bytes.size() + next_bytes.size();
-  cpp::byte *merged = ::new (prev_bytes.data()) cpp::byte[outer_size];
-  block = as_block(prev_outer_size, ByteSpan(merged, outer_size));
+  // Extend the size and copy the last() flag from the next block to this one.
+  next_ &= SIZE_MASK;
+  next_ += next()->next_;
 
-  if (is_last)
-    block->mark_last();
-  else
-    block->next()->prev_ = block->next_;
+  if (next()) {
+    // The two flags are both false, so next_ is a plain size.
+    LIBC_ASSERT(!used() && next() && "flags disrupt use of size");
+    next()->prev_ = next_;
+  }
 
   return true;
 }
 
 template <typename OffsetType, size_t kAlign>
 Block<OffsetType, kAlign> *Block<OffsetType, kAlign>::next() const {
-  uintptr_t addr =
-      last() ? 0 : reinterpret_cast<uintptr_t>(this) + outer_size();
-  return reinterpret_cast<Block *>(addr);
+  if (next_ & LAST_MASK)
+    return nullptr;
+  return reinterpret_cast<Block *>(reinterpret_cast<uintptr_t>(this) +
+                                   outer_size());
 }
 
 template <typename OffsetType, size_t kAlign>
@@ -555,13 +487,6 @@ constexpr Block<OffsetType, kAlign>::Block(size_t prev_outer_size,
   next_ = outer_size;
 }
 
-template <typename OffsetType, size_t kAlign>
-ByteSpan Block<OffsetType, kAlign>::as_bytes(Block *&&block) {
-  size_t block_size = block->outer_size();
-  cpp::byte *bytes = new (cpp::move(block)) cpp::byte[block_size];
-  return {bytes, block_size};
-}
-
 template <typename OffsetType, size_t kAlign>
 Block<OffsetType, kAlign> *
 Block<OffsetType, kAlign>::as_block(size_t prev_outer_size, ByteSpan bytes) {
@@ -573,7 +498,7 @@ internal::BlockStatus Block<OffsetType, kAlign>::check_status() const {
   if (reinterpret_cast<uintptr_t>(this) % ALIGNMENT != 0)
     return internal::BlockStatus::MISALIGNED;
 
-  if (!last() && (this >= next() || this != next()->prev()))
+  if (next() && (this >= next() || this != next()->prev()))
     return internal::BlockStatus::NEXT_MISMATCHED;
 
   if (prev() && (this <= prev() || this != prev()->next()))

diff  --git a/libc/src/__support/freelist_heap.h b/libc/src/__support/freelist_heap.h
index ce4f14b431585..a2c714e15ba87 100644
--- a/libc/src/__support/freelist_heap.h
+++ b/libc/src/__support/freelist_heap.h
@@ -189,19 +189,19 @@ template <size_t NUM_BUCKETS> void FreeListHeap<NUM_BUCKETS>::free(void *ptr) {
   BlockType *prev = chunk_block->prev();
   BlockType *next = nullptr;
 
-  if (!chunk_block->last())
+  if (chunk_block->next())
     next = chunk_block->next();
 
   if (prev != nullptr && !prev->used()) {
     // Remove from freelist and merge
     freelist_.remove_chunk(block_to_span(prev));
     chunk_block = chunk_block->prev();
-    BlockType::merge_next(chunk_block);
+    chunk_block->merge_next();
   }
 
   if (next != nullptr && !next->used()) {
     freelist_.remove_chunk(block_to_span(next));
-    BlockType::merge_next(chunk_block);
+    chunk_block->merge_next();
   }
   // Add back to the freelist
   freelist_.add_chunk(block_to_span(chunk_block));

diff  --git a/libc/test/src/__support/block_test.cpp b/libc/test/src/__support/block_test.cpp
index 04704482b5147..ecce00b7926f9 100644
--- a/libc/test/src/__support/block_test.cpp
+++ b/libc/test/src/__support/block_test.cpp
@@ -54,7 +54,6 @@ TEST_FOR_EACH_BLOCK_TYPE(CanCreateSingleAlignedBlock) {
   EXPECT_EQ(block->prev(), static_cast<BlockType *>(nullptr));
   EXPECT_EQ(block->next(), static_cast<BlockType *>(nullptr));
   EXPECT_FALSE(block->used());
-  EXPECT_TRUE(block->last());
 }
 
 TEST_FOR_EACH_BLOCK_TYPE(CanCreateUnalignedSingleBlock) {
@@ -96,18 +95,16 @@ TEST_FOR_EACH_BLOCK_TYPE(CanSplitBlock) {
   ASSERT_TRUE(result.has_value());
   auto *block1 = *result;
 
-  result = BlockType::split(block1, kSplitN);
+  result = block1->split(kSplitN);
   ASSERT_TRUE(result.has_value());
 
   auto *block2 = *result;
 
   EXPECT_EQ(block1->inner_size(), kSplitN);
   EXPECT_EQ(block1->outer_size(), kSplitN + BlockType::BLOCK_OVERHEAD);
-  EXPECT_FALSE(block1->last());
 
   EXPECT_EQ(block2->outer_size(), kN - kSplitN - BlockType::BLOCK_OVERHEAD);
   EXPECT_FALSE(block2->used());
-  EXPECT_TRUE(block2->last());
 
   EXPECT_EQ(block1->next(), block2);
   EXPECT_EQ(block2->prev(), block1);
@@ -128,7 +125,7 @@ TEST_FOR_EACH_BLOCK_TYPE(CanSplitBlockUnaligned) {
   split_addr += alignof(BlockType) - (split_addr % alignof(BlockType));
   uintptr_t split_len = split_addr - (uintptr_t)&bytes;
 
-  result = BlockType::split(block1, kSplitN);
+  result = block1->split(kSplitN);
   ASSERT_TRUE(result.has_value());
   BlockType *block2 = *result;
 
@@ -161,11 +158,11 @@ TEST_FOR_EACH_BLOCK_TYPE(CanSplitMidBlock) {
   ASSERT_TRUE(result.has_value());
   BlockType *block1 = *result;
 
-  result = BlockType::split(block1, kSplit1);
+  result = block1->split(kSplit1);
   ASSERT_TRUE(result.has_value());
   BlockType *block2 = *result;
 
-  result = BlockType::split(block1, kSplit2);
+  result = block1->split(kSplit2);
   ASSERT_TRUE(result.has_value());
   BlockType *block3 = *result;
 
@@ -184,7 +181,7 @@ TEST_FOR_EACH_BLOCK_TYPE(CannotSplitTooSmallBlock) {
   ASSERT_TRUE(result.has_value());
   BlockType *block = *result;
 
-  result = BlockType::split(block, kSplitN);
+  result = block->split(kSplitN);
   ASSERT_FALSE(result.has_value());
 }
 
@@ -197,13 +194,7 @@ TEST_FOR_EACH_BLOCK_TYPE(CannotSplitBlockWithoutHeaderSpace) {
   ASSERT_TRUE(result.has_value());
   BlockType *block = *result;
 
-  result = BlockType::split(block, kSplitN);
-  ASSERT_FALSE(result.has_value());
-}
-
-TEST_FOR_EACH_BLOCK_TYPE(CannotSplitNull) {
-  BlockType *block = nullptr;
-  auto result = BlockType::split(block, 1);
+  result = block->split(kSplitN);
   ASSERT_FALSE(result.has_value());
 }
 
@@ -216,7 +207,7 @@ TEST_FOR_EACH_BLOCK_TYPE(CannotMakeBlockLargerInSplit) {
   ASSERT_TRUE(result.has_value());
   BlockType *block = *result;
 
-  result = BlockType::split(block, block->inner_size() + 1);
+  result = block->split(block->inner_size() + 1);
   ASSERT_FALSE(result.has_value());
 }
 
@@ -229,8 +220,7 @@ TEST_FOR_EACH_BLOCK_TYPE(CannotMakeSecondBlockLargerInSplit) {
   ASSERT_TRUE(result.has_value());
   BlockType *block = *result;
 
-  result = BlockType::split(block, block->inner_size() -
-                                       BlockType::BLOCK_OVERHEAD + 1);
+  result = block->split(block->inner_size() - BlockType::BLOCK_OVERHEAD + 1);
   ASSERT_FALSE(result.has_value());
 }
 
@@ -243,7 +233,7 @@ TEST_FOR_EACH_BLOCK_TYPE(CanMakeZeroSizeFirstBlock) {
   ASSERT_TRUE(result.has_value());
   BlockType *block = *result;
 
-  result = BlockType::split(block, 0);
+  result = block->split(0);
   ASSERT_TRUE(result.has_value());
   EXPECT_EQ(block->inner_size(), static_cast<size_t>(0));
 }
@@ -257,8 +247,7 @@ TEST_FOR_EACH_BLOCK_TYPE(CanMakeZeroSizeSecondBlock) {
   ASSERT_TRUE(result.has_value());
   BlockType *block1 = *result;
 
-  result = BlockType::split(block1,
-                            block1->inner_size() - BlockType::BLOCK_OVERHEAD);
+  result = block1->split(block1->inner_size() - BlockType::BLOCK_OVERHEAD);
   ASSERT_TRUE(result.has_value());
   BlockType *block2 = *result;
 
@@ -293,7 +282,7 @@ TEST_FOR_EACH_BLOCK_TYPE(CannotSplitUsedBlock) {
   BlockType *block = *result;
 
   block->mark_used();
-  result = BlockType::split(block, kSplitN);
+  result = block->split(kSplitN);
   ASSERT_FALSE(result.has_value());
 }
 
@@ -309,14 +298,14 @@ TEST_FOR_EACH_BLOCK_TYPE(CanMergeWithNextBlock) {
   ASSERT_TRUE(result.has_value());
   BlockType *block1 = *result;
 
-  result = BlockType::split(block1, kSplit1);
+  result = block1->split(kSplit1);
   ASSERT_TRUE(result.has_value());
 
-  result = BlockType::split(block1, kSplit2);
+  result = block1->split(kSplit2);
   ASSERT_TRUE(result.has_value());
   BlockType *block3 = *result;
 
-  EXPECT_TRUE(BlockType::merge_next(block3));
+  EXPECT_TRUE(block3->merge_next());
 
   EXPECT_EQ(block1->next(), block3);
   EXPECT_EQ(block3->prev(), block1);
@@ -334,16 +323,11 @@ TEST_FOR_EACH_BLOCK_TYPE(CannotMergeWithFirstOrLastBlock) {
   BlockType *block1 = *result;
 
   // Do a split, just to check that the checks on next/prev are 
diff erent...
-  result = BlockType::split(block1, kSplitN);
+  result = block1->split(kSplitN);
   ASSERT_TRUE(result.has_value());
   BlockType *block2 = *result;
 
-  EXPECT_FALSE(BlockType::merge_next(block2));
-}
-
-TEST_FOR_EACH_BLOCK_TYPE(CannotMergeNull) {
-  BlockType *block = nullptr;
-  EXPECT_FALSE(BlockType::merge_next(block));
+  EXPECT_FALSE(block2->merge_next());
 }
 
 TEST_FOR_EACH_BLOCK_TYPE(CannotMergeUsedBlock) {
@@ -356,131 +340,11 @@ TEST_FOR_EACH_BLOCK_TYPE(CannotMergeUsedBlock) {
   BlockType *block = *result;
 
   // Do a split, just to check that the checks on next/prev are 
diff erent...
-  result = BlockType::split(block, kSplitN);
-  ASSERT_TRUE(result.has_value());
-
-  block->mark_used();
-  EXPECT_FALSE(BlockType::merge_next(block));
-}
-
-TEST_FOR_EACH_BLOCK_TYPE(CanFreeSingleBlock) {
-  constexpr size_t kN = 1024;
-  alignas(BlockType::ALIGNMENT) array<byte, kN> bytes;
-
-  auto result = BlockType::init(bytes);
+  result = block->split(kSplitN);
   ASSERT_TRUE(result.has_value());
-  BlockType *block = *result;
 
   block->mark_used();
-  BlockType::free(block);
-  EXPECT_FALSE(block->used());
-  EXPECT_EQ(block->outer_size(), kN);
-}
-
-TEST_FOR_EACH_BLOCK_TYPE(CanFreeBlockWithoutMerging) {
-  constexpr size_t kN = 1024;
-  constexpr size_t kSplit1 = 512;
-  constexpr size_t kSplit2 = 256;
-
-  alignas(BlockType::ALIGNMENT) array<byte, kN> bytes;
-  auto result = BlockType::init(bytes);
-  ASSERT_TRUE(result.has_value());
-  BlockType *block1 = *result;
-
-  result = BlockType::split(block1, kSplit1);
-  ASSERT_TRUE(result.has_value());
-  BlockType *block2 = *result;
-
-  result = BlockType::split(block2, kSplit2);
-  ASSERT_TRUE(result.has_value());
-  BlockType *block3 = *result;
-
-  block1->mark_used();
-  block2->mark_used();
-  block3->mark_used();
-
-  BlockType::free(block2);
-  EXPECT_FALSE(block2->used());
-  EXPECT_NE(block2->prev(), static_cast<BlockType *>(nullptr));
-  EXPECT_FALSE(block2->last());
-}
-
-TEST_FOR_EACH_BLOCK_TYPE(CanFreeBlockAndMergeWithPrev) {
-  constexpr size_t kN = 1024;
-  constexpr size_t kSplit1 = 512;
-  constexpr size_t kSplit2 = 256;
-
-  alignas(BlockType::ALIGNMENT) array<byte, kN> bytes;
-  auto result = BlockType::init(bytes);
-  ASSERT_TRUE(result.has_value());
-  BlockType *block1 = *result;
-
-  result = BlockType::split(block1, kSplit1);
-  ASSERT_TRUE(result.has_value());
-  BlockType *block2 = *result;
-
-  result = BlockType::split(block2, kSplit2);
-  ASSERT_TRUE(result.has_value());
-  BlockType *block3 = *result;
-
-  block2->mark_used();
-  block3->mark_used();
-
-  BlockType::free(block2);
-  EXPECT_FALSE(block2->used());
-  EXPECT_EQ(block2->prev(), static_cast<BlockType *>(nullptr));
-  EXPECT_FALSE(block2->last());
-}
-
-TEST_FOR_EACH_BLOCK_TYPE(CanFreeBlockAndMergeWithNext) {
-  constexpr size_t kN = 1024;
-  constexpr size_t kSplit1 = 512;
-  constexpr size_t kSplit2 = 256;
-
-  alignas(BlockType::ALIGNMENT) array<byte, kN> bytes;
-  auto result = BlockType::init(bytes);
-  ASSERT_TRUE(result.has_value());
-  BlockType *block1 = *result;
-
-  result = BlockType::split(block1, kSplit1);
-  ASSERT_TRUE(result.has_value());
-  BlockType *block2 = *result;
-
-  result = BlockType::split(block2, kSplit2);
-  ASSERT_TRUE(result.has_value());
-
-  block1->mark_used();
-  block2->mark_used();
-
-  BlockType::free(block2);
-  EXPECT_FALSE(block2->used());
-  EXPECT_NE(block2->prev(), static_cast<BlockType *>(nullptr));
-  EXPECT_TRUE(block2->last());
-}
-
-TEST_FOR_EACH_BLOCK_TYPE(CanFreeUsedBlockAndMergeWithBoth) {
-  constexpr size_t kN = 1024;
-  constexpr size_t kSplit1 = 512;
-  constexpr size_t kSplit2 = 256;
-
-  alignas(BlockType::ALIGNMENT) array<byte, kN> bytes;
-  auto result = BlockType::init(bytes);
-  ASSERT_TRUE(result.has_value());
-  BlockType *block1 = *result;
-
-  result = BlockType::split(block1, kSplit1);
-  ASSERT_TRUE(result.has_value());
-  BlockType *block2 = *result;
-
-  result = BlockType::split(block2, kSplit2);
-  ASSERT_TRUE(result.has_value());
-
-  block2->mark_used();
-
-  BlockType::free(block2);
-  EXPECT_FALSE(block2->used());
-  EXPECT_EQ(block2->prev(), static_cast<BlockType *>(nullptr));
-  EXPECT_TRUE(block2->last());
+  EXPECT_FALSE(block->merge_next());
 }
 
 TEST_FOR_EACH_BLOCK_TYPE(CanCheckValidBlock) {
@@ -493,11 +357,11 @@ TEST_FOR_EACH_BLOCK_TYPE(CanCheckValidBlock) {
   ASSERT_TRUE(result.has_value());
   BlockType *block1 = *result;
 
-  result = BlockType::split(block1, kSplit1);
+  result = block1->split(kSplit1);
   ASSERT_TRUE(result.has_value());
   BlockType *block2 = *result;
 
-  result = BlockType::split(block2, kSplit2);
+  result = block2->split(kSplit2);
   ASSERT_TRUE(result.has_value());
   BlockType *block3 = *result;
 
@@ -517,15 +381,15 @@ TEST_FOR_EACH_BLOCK_TYPE(CanCheckInvalidBlock) {
   ASSERT_TRUE(result.has_value());
   BlockType *block1 = *result;
 
-  result = BlockType::split(block1, kSplit1);
+  result = block1->split(kSplit1);
   ASSERT_TRUE(result.has_value());
   BlockType *block2 = *result;
 
-  result = BlockType::split(block2, kSplit2);
+  result = block2->split(kSplit2);
   ASSERT_TRUE(result.has_value());
   BlockType *block3 = *result;
 
-  result = BlockType::split(block3, kSplit3);
+  result = block3->split(kSplit3);
   ASSERT_TRUE(result.has_value());
 
   // Corrupt a Block header.
@@ -630,7 +494,6 @@ TEST_FOR_EACH_BLOCK_TYPE(AllocateAlreadyAligned) {
   // Check the next block.
   EXPECT_NE(next, static_cast<BlockType *>(nullptr));
   EXPECT_EQ(aligned_block->next(), next);
-  EXPECT_EQ(next->next(), static_cast<BlockType *>(nullptr));
   EXPECT_EQ(reinterpret_cast<byte *>(next) + next->outer_size(),
             bytes.data() + bytes.size());
 }
@@ -674,7 +537,6 @@ TEST_FOR_EACH_BLOCK_TYPE(AllocateNeedsAlignment) {
   // Check the next block.
   EXPECT_NE(next, static_cast<BlockType *>(nullptr));
   EXPECT_EQ(aligned_block->next(), next);
-  EXPECT_EQ(next->next(), static_cast<BlockType *>(nullptr));
   EXPECT_EQ(reinterpret_cast<byte *>(next) + next->outer_size(), &*bytes.end());
 }
 
@@ -687,7 +549,7 @@ TEST_FOR_EACH_BLOCK_TYPE(PreviousBlockMergedIfNotFirst) {
   BlockType *block = *result;
 
   // Split the block roughly halfway and work on the second half.
-  auto result2 = BlockType::split(block, kN / 2);
+  auto result2 = block->split(kN / 2);
   ASSERT_TRUE(result2.has_value());
   BlockType *newblock = *result2;
   ASSERT_EQ(newblock->prev(), block);
@@ -744,20 +606,17 @@ TEST_FOR_EACH_BLOCK_TYPE(CanRemergeBlockAllocations) {
 
   // Check we have the appropriate blocks.
   ASSERT_NE(prev, static_cast<BlockType *>(nullptr));
-  ASSERT_FALSE(prev->last());
   ASSERT_EQ(aligned_block->prev(), prev);
   EXPECT_NE(next, static_cast<BlockType *>(nullptr));
   EXPECT_NE(next, static_cast<BlockType *>(nullptr));
   EXPECT_EQ(aligned_block->next(), next);
   EXPECT_EQ(next->next(), static_cast<BlockType *>(nullptr));
-  ASSERT_TRUE(next->last());
 
   // Now check for successful merges.
-  EXPECT_TRUE(BlockType::merge_next(prev));
+  EXPECT_TRUE(prev->merge_next());
   EXPECT_EQ(prev->next(), next);
-  EXPECT_TRUE(BlockType::merge_next(prev));
+  EXPECT_TRUE(prev->merge_next());
   EXPECT_EQ(prev->next(), static_cast<BlockType *>(nullptr));
-  EXPECT_TRUE(prev->last());
 
   // We should have the original buffer.
   EXPECT_EQ(reinterpret_cast<byte *>(prev), &*bytes.begin());


        


More information about the libc-commits mailing list