[libc-commits] [libc] [libc] add shrink in-place support for reallocations (PR #200272)

[Daniel Thornburgh via libc-commits]

================
@@ -158,6 +160,44 @@ LIBC_INLINE void FreeListHeap::free(void *ptr) {
   free_store.insert(block);
 }
 
+LIBC_INLINE bool FreeListHeap::shrink_in_place(Block *block, size_t size) {
+  size_t min_outer_size = Block::outer_size(cpp::max(size, sizeof(size_t)));
+  uintptr_t next_block_start = Block::next_possible_block_start(
+      reinterpret_cast<uintptr_t>(block) + min_outer_size, Block::MIN_ALIGN);
+  size_t new_outer_size = next_block_start - reinterpret_cast<uintptr_t>(block);
+  // only split the block if the trailing part can be inserted into freelist
+  if (block->outer_size() >= new_outer_size &&
+      block->outer_size() - new_outer_size >= FreeStore::MIN_OUTER_SIZE) {
+    // We must temporarily mark the block as free to allow splitting.
+    // A block's usable space overlaps with the next block's prev_ field. When
+    // the next block is created via splitting, its header constructor writes to
+    // its prev_ field (initializing it to 0). Since the original block is
----------------
mysterymath wrote:

Hm, I don't think it was ever actually correct to initialize the `prev_` field to zero. Actually I'm not sure why we have default initializers at all. The intent was to leave `prev_` uninitialized on block construction, allowing it to change freely with the contents of that memory. It must never be accessed by unvariant until `mark_free()`, where a placement new makes it alive as an initialized `size_t`. We should fix that instead of papering over the issue here.

But... I've come to understand that my previous standards argument that overlapping `prev_` like this was not valid... we'll need to do some surgery to `Block` to repair it. That's out of scope for this change, but the above will likely work, and it's less code than this, and it should be closer to the eventual solution.

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