[libc-commits] [libc] [libc] Add aligned_alloc (PR #96586)

[Nick Desaulniers via libc-commits]

================
@@ -357,6 +410,66 @@ void Block<OffsetType, kAlign>::free(Block *&block) {
   merge_next(block);
 }
 
+template <typename OffsetType, size_t kAlign>
+bool Block<OffsetType, kAlign>::can_allocate(size_t alignment,
+                                             size_t size) const {
+  if (is_usable_space_aligned(alignment) && inner_size() >= size)
+    return true; // Size and alignment constraints met.
+
+  // Either the alignment isn't met or we don't have enough size.
+  // If we don't meet alignment, we can always adjust such that we do meet the
+  // alignment. If we meet the alignment but just don't have enough size. This
+  // check will fail anyway.
+  size_t adjustment = padding_for_alignment(alignment);
+  return inner_size() >= size + adjustment;
+}
+
+template <typename OffsetType, size_t kAlign>
+typename Block<OffsetType, kAlign>::BlockInfo
+Block<OffsetType, kAlign>::allocate(Block *block, size_t alignment,
+                                    size_t size) {
+  BlockInfo info{block, /*prev=*/nullptr, /*next=*/nullptr};
+
+  if (!info.block->is_usable_space_aligned(alignment)) {
+    size_t adjustment = info.block->padding_for_alignment(alignment);
+    size_t new_inner_size = adjustment - BLOCK_OVERHEAD;
+    LIBC_ASSERT(new_inner_size % ALIGNMENT == 0 &&
+                "The adjustment calculation should always return a new size "
+                "that's a multiple of ALIGNMENT");
+
+    Block *original = info.block;
+    optional<Block *> maybe_aligned_block =
+        Block::split(original, 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 "
+                "two blocks.");
----------------
nickdesaulniers wrote:

Why don't we triple check `block->can_allocate(alignment, size)` within this `LIBC_ASSERT` then?

There's an implicit requirement on this method that the `block` has previously been checked via `can_allocate`.  That requirement should be in a comment at above the function definition (or declaration) rather than buried in an assert in the definition (prefer BOTH).

I almost wonder if we should explicitly check that and return early if `can_allocate` returns false, regardless of asserts being enabled or not? Though I guess the interface isn't designed to be falliable.

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