[libc-commits] [libc] [libc] Use Block as a byte-backed proxy (PR #201001)

[Schrodinger ZHU Yifan 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));
----------------
SchrodingerZhu wrote:

Ah, this is really a good point. I was basically considering load/store separately, making them to be correct in dependent of each other. On a second thought, I think placement new can be used here. Previous problems arise because we always placement new a whole block which may create an object overlaps with payload but as we have separated the field, individual placement news should work just fine now.

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