[libc-commits] [libc] [libc] Migrate `Block` to `BlockRef` in baremetal allocator (PR #201001)

[Daniel Thornburgh via libc-commits]

================
@@ -275,79 +285,90 @@ class Block {
     // So the maximum distance would be G - L. As a special case, if L is 1
     // (unaligned), the max distance is G - 1.
     //
-    // This block's usable space is aligned to MIN_ALIGN >= Block. With zero
-    // padding, the next block's usable space is sizeof(Block) past it, which is
-    // a point aligned to Block. Thus the max padding needed is alignment -
-    // alignof(Block).
-    if (add_overflow(size, alignment - alignof(Block), size))
+    // This block's usable space is aligned to MIN_ALIGN >= header alignment.
+    // With zero padding, the next block's usable space is HEADER_SIZE past it,
+    // which is aligned to header alignment. Thus the max padding needed is
+    // alignment - alignof(size_t).
+    if (add_overflow(size, alignment - alignof(size_t), size))
       return 0;
     return size;
   }
 
-  // This is the return type for `allocate` which can split one block into up to
-  // three blocks.
-  struct BlockInfo {
-    // This is the newly aligned block. It will have the alignment requested by
-    // a call to `allocate` and at most `size`.
-    Block *block;
-
-    // If the usable_space in the new block was not aligned according to the
-    // `alignment` parameter, we will need to split into this block and the
-    // `block` to ensure `block` is properly aligned. In this case, `prev` will
-    // be a pointer to this new "padding" block. `prev` will be nullptr if no
-    // new block was created or we were able to merge the block before the
-    // original block with the "padding" block.
-    Block *prev;
-
-    // This is the remainder of the next block after splitting the `block`
-    // according to `size`. This can happen if there's enough space after the
-    // `block`.
-    Block *next;
-  };
+  struct BlockInfo;
 
   // Divide a block into up to 3 blocks according to `BlockInfo`. Behavior is
   // undefined if allocation is not possible for the given size and alignment.
-  static BlockInfo allocate(Block *block, size_t alignment, size_t size);
+  static BlockInfo allocate(BlockRef block, size_t alignment, size_t size);
 
   // These two functions may wrap around.
   LIBC_INLINE static uintptr_t
   next_possible_block_start(uintptr_t ptr,
                             size_t usable_space_alignment = MIN_ALIGN) {
-    return align_up(ptr + sizeof(Block), usable_space_alignment) -
-           sizeof(Block);
+    return align_up(ptr + HEADER_SIZE, usable_space_alignment) - HEADER_SIZE;
   }
   LIBC_INLINE static uintptr_t
   prev_possible_block_start(uintptr_t ptr,
                             size_t usable_space_alignment = MIN_ALIGN) {
-    return align_down(ptr, usable_space_alignment) - sizeof(Block);
+    return align_down(ptr, usable_space_alignment) - HEADER_SIZE;
   }
 
+  /// Only for testing.
+  static constexpr size_t PREV_FIELD_SIZE = sizeof(size_t);
+
 private:
   /// 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.
-  LIBC_INLINE static Block *as_block(ByteSpan bytes) {
-    LIBC_ASSERT(reinterpret_cast<uintptr_t>(bytes.data()) % alignof(Block) ==
+  LIBC_INLINE static BlockRef as_block(ByteSpan bytes) {
+    LIBC_ASSERT(reinterpret_cast<uintptr_t>(bytes.data()) % alignof(size_t) ==
                     0 &&
                 "block start must be suitably aligned");
-    return ::new (bytes.data()) Block(bytes.size(), /*is_last=*/false);
+    BlockRef block(bytes.data());
+    block.store_next(bytes.size());
+    return block;
   }
 
   LIBC_INLINE static void make_last_block(cpp::byte *start) {
-    LIBC_ASSERT(reinterpret_cast<uintptr_t>(start) % alignof(Block) == 0 &&
+    LIBC_ASSERT(reinterpret_cast<uintptr_t>(start) % alignof(size_t) == 0 &&
                 "block start must be suitably aligned");
-    ::new (start) Block(sizeof(Block), /*is_last=*/true);
+    BlockRef last(start);
+    last.store_next(HEADER_SIZE | LAST_MASK);
+  }
+
+  LIBC_INLINE cpp::byte *field_ptr(size_t offset) const {
+    cpp::byte *ptr = header_ptr + offset;
+    LIBC_ASSERT(reinterpret_cast<uintptr_t>(ptr) % alignof(size_t) == 0 &&
+                "block metadata fields must be aligned");
+    // BlockRef points to block header which should be well-aligned. However,
+    // the compiler may not know this information. Adding assume aligned to
+    // tell the compiler to emit a single load/store even when unaligned access
+    // is disallowed.
+#if __has_builtin(__builtin_assume_aligned)
+    return reinterpret_cast<cpp::byte *>(
+        __builtin_assume_aligned(ptr, alignof(size_t)));
+#else
+    return ptr;
+#endif
+  }
+
+  LIBC_INLINE size_t load_field(size_t offset) const {
+    size_t value;
+    inline_memcpy(&value, field_ptr(offset), sizeof(value));
+    return value;
+  }
+
+  LIBC_INLINE void store_field(size_t offset, size_t value) const {
+    inline_memcpy(field_ptr(offset), &value, sizeof(value));
----------------
mysterymath wrote:

Yeah, I think using placement new should be safe here.

In the case @PiJoules mentioned:
```
*user_float_ptr = 3.14f;  // user_float_ptr points to the prev of block B
a_block.mark_free();  // calls placement new
```

The user object should be considered to have been destroyed (by `free()` or `operator delete`) by the time that we create an object that overlaps with its storage. And all of the header objects are implicit lifetime types, so the old object should be destroyed whenever we placement new the new object with its new value.

https://github.com/llvm/llvm-project/pull/201001