[libc-commits] [libc] [libc] Lazily initialize freelist malloc using symbols (PR #99254)

Daniel Thornburgh via libc-commits libc-commits at lists.llvm.org
Thu Jul 25 11:40:37 PDT 2024


https://github.com/mysterymath updated https://github.com/llvm/llvm-project/pull/99254

>From 060f31fb9b4b11929eba70279c065a01d66537a9 Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Tue, 16 Jul 2024 11:12:17 -0700
Subject: [PATCH 1/9] [libc] Lazily initialize freelist malloc using symbols

This allows the user to customize the size of the heap provided by
overriding the weak symbol __libc_heap_limit at link time. The heap
begins at _end and ends __libc_heap_limit bytes afterwards. It also
prevents a completely unused heap from requiring any space. (It's
reasonably common to link in malloc/free to handle exceptional
situations that can never actually be encountered.)

I'd think this should eventually be replaced with an implemenation based
on sbrk(), with sbrk() implemented generically in terms of brk(), which
would be a system call on e.g. Linux and a bump pointer from _end up to
__libc_heap_limit on baremetal. This would allow the allocator to be
more flexible (e.g., an implementation could swap out brk() and do
dynamic memory availability checks), to manage the heap better by
keeping better track of "wilderness" and to work properly as a malloc on
Linux.

Incidentally, this sets the default heap limit to 128KiB, from 102400
bytes, which had been reported as "1GiB".
---
 libc/config/baremetal/config.json             |  2 +-
 libc/src/__support/freelist_heap.h            | 75 ++++++++++---------
 libc/src/stdlib/CMakeLists.txt                |  1 +
 libc/src/stdlib/freelist_malloc.cpp           |  9 +--
 libc/src/stdlib/heap_limit.S                  |  2 +
 .../test/src/__support/freelist_heap_test.cpp |  7 +-
 .../src/__support/freelist_malloc_test.cpp    | 15 +++-
 7 files changed, 63 insertions(+), 48 deletions(-)
 create mode 100644 libc/src/stdlib/heap_limit.S

diff --git a/libc/config/baremetal/config.json b/libc/config/baremetal/config.json
index 12f4c2aa3a805..ebab661cb8d56 100644
--- a/libc/config/baremetal/config.json
+++ b/libc/config/baremetal/config.json
@@ -20,7 +20,7 @@
   },
   "malloc": {
     "LIBC_CONF_FREELIST_MALLOC_BUFFER_SIZE": {
-      "value": 102400
+      "value": 131072
     }
   },
   "qsort": {
diff --git a/libc/src/__support/freelist_heap.h b/libc/src/__support/freelist_heap.h
index a2c714e15ba87..180c42c9c55b2 100644
--- a/libc/src/__support/freelist_heap.h
+++ b/libc/src/__support/freelist_heap.h
@@ -20,6 +20,9 @@
 #include "src/string/memory_utils/inline_memcpy.h"
 #include "src/string/memory_utils/inline_memset.h"
 
+extern char _end;
+extern char __libc_heap_limit;
+
 namespace LIBC_NAMESPACE_DECL {
 
 using cpp::optional;
@@ -47,17 +50,10 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
     size_t total_free_calls;
   };
 
-  FreeListHeap(span<cpp::byte> region)
-      : FreeListHeap(&*region.begin(), &*region.end(), region.size()) {
-    auto result = BlockType::init(region);
-    BlockType *block = *result;
-    freelist_.add_chunk(block_to_span(block));
-  }
-
-  constexpr FreeListHeap(void *start, cpp::byte *end, size_t total_bytes)
-      : block_region_start_(start), block_region_end_(end),
-        freelist_(DEFAULT_BUCKETS), heap_stats_{} {
-    heap_stats_.total_bytes = total_bytes;
+  constexpr FreeListHeap(span<cpp::byte> region)
+      : is_initialized_(false), region_(region), freelist_(DEFAULT_BUCKETS),
+        heap_stats_{} {
+    heap_stats_.total_bytes = region.size();
   }
 
   void *allocate(size_t size);
@@ -69,61 +65,68 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
   void *calloc(size_t num, size_t size);
 
   const HeapStats &heap_stats() const { return heap_stats_; }
-  void reset_heap_stats() { heap_stats_ = {}; }
 
-  void *region_start() const { return block_region_start_; }
-  size_t region_size() const {
-    return reinterpret_cast<uintptr_t>(block_region_end_) -
-           reinterpret_cast<uintptr_t>(block_region_start_);
-  }
+  cpp::span<cpp::byte> region() const { return region_; }
 
-protected:
-  constexpr void set_freelist_node(typename FreeListType::FreeListNode &node,
-                                   cpp::span<cpp::byte> chunk) {
-    freelist_.set_freelist_node(node, chunk);
-  }
+private:
+  void init();
 
   void *allocate_impl(size_t alignment, size_t size);
 
-private:
   span<cpp::byte> block_to_span(BlockType *block) {
     return span<cpp::byte>(block->usable_space(), block->inner_size());
   }
 
   bool is_valid_ptr(void *ptr) {
-    return ptr >= block_region_start_ && ptr < block_region_end_;
+    return ptr >= region_.begin() && ptr < region_.end();
   }
 
-  void *block_region_start_;
-  void *block_region_end_;
+  bool is_initialized_;
+  cpp::span<cpp::byte> region_;
   FreeListType freelist_;
   HeapStats heap_stats_;
 };
 
 template <size_t BUFF_SIZE, size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()>
-struct FreeListHeapBuffer : public FreeListHeap<NUM_BUCKETS> {
+class FreeListHeapBuffer : public FreeListHeap<NUM_BUCKETS> {
   using parent = FreeListHeap<NUM_BUCKETS>;
   using FreeListNode = typename parent::FreeListType::FreeListNode;
 
+public:
   constexpr FreeListHeapBuffer()
-      : FreeListHeap<NUM_BUCKETS>(&block, buffer + sizeof(buffer), BUFF_SIZE),
-        block(0, BUFF_SIZE), node{}, buffer{} {
-    block.mark_last();
+      : FreeListHeap<NUM_BUCKETS>{buffer}, buffer{} {}
 
-    cpp::span<cpp::byte> chunk(buffer, sizeof(buffer));
-    parent::set_freelist_node(node, chunk);
-  }
+private:
+  cpp::byte buffer[BUFF_SIZE];
+};
+
+template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()>
+class FreeListHeapSymbols : public FreeListHeap<NUM_BUCKETS> {
+  using parent = FreeListHeap<NUM_BUCKETS>;
+  using FreeListNode = typename parent::FreeListType::FreeListNode;
 
-  typename parent::BlockType block;
-  FreeListNode node;
-  cpp::byte buffer[BUFF_SIZE - sizeof(block) - sizeof(node)];
+public:
+  constexpr FreeListHeapSymbols()
+      : FreeListHeap<NUM_BUCKETS>{
+            {(cpp::byte *)&_end, (size_t)&__libc_heap_limit}} {}
 };
 
+template <size_t NUM_BUCKETS> void FreeListHeap<NUM_BUCKETS>::init() {
+  LIBC_ASSERT(!is_initialized_ && "duplicate initialization");
+  auto result = BlockType::init(region_);
+  BlockType *block = *result;
+  freelist_.add_chunk(block_to_span(block));
+  is_initialized_ = true;
+}
+
 template <size_t NUM_BUCKETS>
 void *FreeListHeap<NUM_BUCKETS>::allocate_impl(size_t alignment, size_t size) {
   if (size == 0)
     return nullptr;
 
+  if (!is_initialized_)
+    init();
+
   // Find a chunk in the freelist. Split it if needed, then return.
   auto chunk =
       freelist_.find_chunk_if([alignment, size](span<cpp::byte> chunk) {
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt
index 03d8c48751840..ced0936bc4043 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -388,6 +388,7 @@ if(NOT LIBC_TARGET_OS_IS_GPU)
       freelist_malloc
       SRCS
         freelist_malloc.cpp
+        heap_limit.S
       HDRS
         malloc.h
       DEPENDS
diff --git a/libc/src/stdlib/freelist_malloc.cpp b/libc/src/stdlib/freelist_malloc.cpp
index cfffa0425ff66..e15e74fb18864 100644
--- a/libc/src/stdlib/freelist_malloc.cpp
+++ b/libc/src/stdlib/freelist_malloc.cpp
@@ -19,16 +19,13 @@
 namespace LIBC_NAMESPACE_DECL {
 
 namespace {
-#ifdef LIBC_FREELIST_MALLOC_SIZE
-// This is set via the LIBC_CONF_FREELIST_MALLOC_BUFFER_SIZE configuration.
-constexpr size_t SIZE = LIBC_FREELIST_MALLOC_SIZE;
-#else
+#ifndef LIBC_FREELIST_MALLOC_SIZE
 #error "LIBC_FREELIST_MALLOC_SIZE was not defined for this build."
 #endif
-LIBC_CONSTINIT FreeListHeapBuffer<SIZE> freelist_heap_buffer;
+FreeListHeapSymbols<> freelist_heap_symbols;
 } // namespace
 
-FreeListHeap<> *freelist_heap = &freelist_heap_buffer;
+FreeListHeap<> *freelist_heap = &freelist_heap_symbols;
 
 LLVM_LIBC_FUNCTION(void *, malloc, (size_t size)) {
   return freelist_heap->allocate(size);
diff --git a/libc/src/stdlib/heap_limit.S b/libc/src/stdlib/heap_limit.S
new file mode 100644
index 0000000000000..d704d07165ed9
--- /dev/null
+++ b/libc/src/stdlib/heap_limit.S
@@ -0,0 +1,2 @@
+.weak __libc_heap_limit
+.set __libc_heap_limit, LIBC_FREELIST_MALLOC_SIZE
diff --git a/libc/test/src/__support/freelist_heap_test.cpp b/libc/test/src/__support/freelist_heap_test.cpp
index 5815d5dfc01ab..fc4348aed6b56 100644
--- a/libc/test/src/__support/freelist_heap_test.cpp
+++ b/libc/test/src/__support/freelist_heap_test.cpp
@@ -30,6 +30,11 @@ using LIBC_NAMESPACE::freelist_heap;
 #define TEST_FOR_EACH_ALLOCATOR(TestCase, BufferSize)                          \
   class LlvmLibcFreeListHeapTest##TestCase : public testing::Test {            \
   public:                                                                      \
+    FreeListHeapBuffer<BufferSize> fake_global_buffer;                         \
+    void SetUp() override {                                                    \
+      freelist_heap =                                                          \
+          new (&fake_global_buffer) FreeListHeapBuffer<BufferSize>;            \
+    }                                                                          \
     void RunTest(FreeListHeap<> &allocator, [[maybe_unused]] size_t N);        \
   };                                                                           \
   TEST_F(LlvmLibcFreeListHeapTest##TestCase, TestCase) {                       \
@@ -37,7 +42,7 @@ using LIBC_NAMESPACE::freelist_heap;
         cpp::byte buf[BufferSize] = {cpp::byte(0)};                            \
     FreeListHeap<> allocator(buf);                                             \
     RunTest(allocator, BufferSize);                                            \
-    RunTest(*freelist_heap, freelist_heap->region_size());                     \
+    RunTest(*freelist_heap, freelist_heap->region().size());                   \
   }                                                                            \
   void LlvmLibcFreeListHeapTest##TestCase::RunTest(FreeListHeap<> &allocator,  \
                                                    size_t N)
diff --git a/libc/test/src/__support/freelist_malloc_test.cpp b/libc/test/src/__support/freelist_malloc_test.cpp
index e9d7c63a4d438..4b774884d81b5 100644
--- a/libc/test/src/__support/freelist_malloc_test.cpp
+++ b/libc/test/src/__support/freelist_malloc_test.cpp
@@ -14,15 +14,22 @@
 #include "test/UnitTest/Test.h"
 
 using LIBC_NAMESPACE::freelist_heap;
+using LIBC_NAMESPACE::FreeListHeapBuffer;
 
-TEST(LlvmLibcFreeListMalloc, MallocStats) {
+class LlvmLibcFreeListMalloc : public LIBC_NAMESPACE::testing::Test {
+protected:
+  void SetUp() override {
+    freelist_heap = new (&buffer) FreeListHeapBuffer<1024>;
+  }
+
+  FreeListHeapBuffer<1024> buffer;
+};
+
+TEST_F(LlvmLibcFreeListMalloc, MallocStats) {
   constexpr size_t kAllocSize = 256;
   constexpr size_t kCallocNum = 4;
   constexpr size_t kCallocSize = 64;
 
-  freelist_heap->reset_heap_stats(); // Do this because other tests might've
-                                     // called the same global allocator.
-
   void *ptr1 = LIBC_NAMESPACE::malloc(kAllocSize);
 
   const auto &freelist_heap_stats = freelist_heap->heap_stats();

>From b0bbb40824151dff064e1a3e7ad5a9b5f69d4fb1 Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Wed, 17 Jul 2024 09:59:12 -0700
Subject: [PATCH 2/9] Extern "C" in namespace for heap symbols

---
 libc/src/__support/freelist_heap.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libc/src/__support/freelist_heap.h b/libc/src/__support/freelist_heap.h
index 180c42c9c55b2..ed755e23059b5 100644
--- a/libc/src/__support/freelist_heap.h
+++ b/libc/src/__support/freelist_heap.h
@@ -20,11 +20,11 @@
 #include "src/string/memory_utils/inline_memcpy.h"
 #include "src/string/memory_utils/inline_memset.h"
 
-extern char _end;
-extern char __libc_heap_limit;
-
 namespace LIBC_NAMESPACE_DECL {
 
+extern "C" char _end;
+extern "C" char __libc_heap_limit;
+
 using cpp::optional;
 using cpp::span;
 

>From 3e6faed1c9505f18cab12744bd4728d4104c04a7 Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Wed, 17 Jul 2024 14:52:48 -0700
Subject: [PATCH 3/9] Address comments:

- Move preprocessor check to assembly file
- Add LIBC_CONSTINIT back to heap definition; make non-constexpr cast
  from &__heap_limit to size_t occur at init time to restore constexpr
  init.
---
 libc/src/__support/freelist_heap.h  | 32 +++++++++++++++--------------
 libc/src/stdlib/freelist_malloc.cpp |  8 +-------
 libc/src/stdlib/heap_limit.S        |  3 +++
 3 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/libc/src/__support/freelist_heap.h b/libc/src/__support/freelist_heap.h
index ed755e23059b5..964e6aedddea6 100644
--- a/libc/src/__support/freelist_heap.h
+++ b/libc/src/__support/freelist_heap.h
@@ -22,8 +22,8 @@
 
 namespace LIBC_NAMESPACE_DECL {
 
-extern "C" char _end;
-extern "C" char __libc_heap_limit;
+extern "C" cpp::byte _end;
+extern "C" cpp::byte __libc_heap_limit;
 
 using cpp::optional;
 using cpp::span;
@@ -50,9 +50,14 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
     size_t total_free_calls;
   };
 
+  constexpr FreeListHeap()
+      : is_initialized_(false), region_(&_end, size_t{0}),
+        heap_limit_(&__libc_heap_limit), freelist_(DEFAULT_BUCKETS),
+        heap_stats_{} {}
+
   constexpr FreeListHeap(span<cpp::byte> region)
-      : is_initialized_(false), region_(region), freelist_(DEFAULT_BUCKETS),
-        heap_stats_{} {
+      : is_initialized_(false), region_(region), heap_limit_{},
+    freelist_(DEFAULT_BUCKETS), heap_stats_{} {
     heap_stats_.total_bytes = region.size();
   }
 
@@ -83,6 +88,10 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
 
   bool is_initialized_;
   cpp::span<cpp::byte> region_;
+
+  // Kept to initialize region_ by non-constexpr cast to size_t
+  cpp::byte *heap_limit_;
+
   FreeListType freelist_;
   HeapStats heap_stats_;
 };
@@ -100,19 +109,12 @@ class FreeListHeapBuffer : public FreeListHeap<NUM_BUCKETS> {
   cpp::byte buffer[BUFF_SIZE];
 };
 
-template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()>
-class FreeListHeapSymbols : public FreeListHeap<NUM_BUCKETS> {
-  using parent = FreeListHeap<NUM_BUCKETS>;
-  using FreeListNode = typename parent::FreeListType::FreeListNode;
-
-public:
-  constexpr FreeListHeapSymbols()
-      : FreeListHeap<NUM_BUCKETS>{
-            {(cpp::byte *)&_end, (size_t)&__libc_heap_limit}} {}
-};
-
 template <size_t NUM_BUCKETS> void FreeListHeap<NUM_BUCKETS>::init() {
   LIBC_ASSERT(!is_initialized_ && "duplicate initialization");
+  if (heap_limit_) {
+    region_ = {region_.begin(), (size_t)heap_limit_};
+    heap_stats_.total_bytes = region_.size();
+  }
   auto result = BlockType::init(region_);
   BlockType *block = *result;
   freelist_.add_chunk(block_to_span(block));
diff --git a/libc/src/stdlib/freelist_malloc.cpp b/libc/src/stdlib/freelist_malloc.cpp
index e15e74fb18864..47240bc53aa37 100644
--- a/libc/src/stdlib/freelist_malloc.cpp
+++ b/libc/src/stdlib/freelist_malloc.cpp
@@ -18,13 +18,7 @@
 
 namespace LIBC_NAMESPACE_DECL {
 
-namespace {
-#ifndef LIBC_FREELIST_MALLOC_SIZE
-#error "LIBC_FREELIST_MALLOC_SIZE was not defined for this build."
-#endif
-FreeListHeapSymbols<> freelist_heap_symbols;
-} // namespace
-
+static LIBC_CONSTINIT FreeListHeap<> freelist_heap_symbols;
 FreeListHeap<> *freelist_heap = &freelist_heap_symbols;
 
 LLVM_LIBC_FUNCTION(void *, malloc, (size_t size)) {
diff --git a/libc/src/stdlib/heap_limit.S b/libc/src/stdlib/heap_limit.S
index d704d07165ed9..2a0a6665deb56 100644
--- a/libc/src/stdlib/heap_limit.S
+++ b/libc/src/stdlib/heap_limit.S
@@ -1,2 +1,5 @@
+#ifndef LIBC_FREELIST_MALLOC_SIZE
+#error "LIBC_FREELIST_MALLOC_SIZE was not defined for this build."
+#endif
 .weak __libc_heap_limit
 .set __libc_heap_limit, LIBC_FREELIST_MALLOC_SIZE

>From d0322382338f9a35b43f4151e9fcd2e64dd14588 Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Wed, 17 Jul 2024 15:40:46 -0700
Subject: [PATCH 4/9] Default initializers to clean up constructors

---
 libc/src/__support/freelist_heap.h | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/libc/src/__support/freelist_heap.h b/libc/src/__support/freelist_heap.h
index 964e6aedddea6..0fda7c3b2c9dc 100644
--- a/libc/src/__support/freelist_heap.h
+++ b/libc/src/__support/freelist_heap.h
@@ -51,13 +51,10 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
   };
 
   constexpr FreeListHeap()
-      : is_initialized_(false), region_(&_end, size_t{0}),
-        heap_limit_(&__libc_heap_limit), freelist_(DEFAULT_BUCKETS),
-        heap_stats_{} {}
+      : region_(&_end, size_t{0}), heap_limit_(&__libc_heap_limit) {}
 
   constexpr FreeListHeap(span<cpp::byte> region)
-      : is_initialized_(false), region_(region), heap_limit_{},
-    freelist_(DEFAULT_BUCKETS), heap_stats_{} {
+      : region_(region), heap_limit_{} {
     heap_stats_.total_bytes = region.size();
   }
 
@@ -86,14 +83,14 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
     return ptr >= region_.begin() && ptr < region_.end();
   }
 
-  bool is_initialized_;
+  bool is_initialized_ = false;
   cpp::span<cpp::byte> region_;
 
   // Kept to initialize region_ by non-constexpr cast to size_t
   cpp::byte *heap_limit_;
 
-  FreeListType freelist_;
-  HeapStats heap_stats_;
+  FreeListType freelist_{DEFAULT_BUCKETS};
+  HeapStats heap_stats_{};
 };
 
 template <size_t BUFF_SIZE, size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()>

>From 1edd9aded71d16ee47a0b6ffadc1496d4e843822 Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Fri, 19 Jul 2024 15:05:36 -0700
Subject: [PATCH 5/9] Make __libc_heap_limit an end symbol, not a size

This is more conventional and it dodges the issue of needing to provide
a default size entirely. Setting up a baremetal heap intrinsically
requires some linker script work, and __libc_heap_limit will be an
undefined reference hinting to the user that this work must be done.

Once we have a morecore function, a baremetal morecore() would use
__libc_heap_limit, a Linux one would use sbrk(), and a test one would
use a buffer.
---
 libc/config/baremetal/config.json           |  5 ----
 libc/config/config.json                     |  6 -----
 libc/docs/configure.rst                     |  2 --
 libc/src/__support/freelist_heap.h          | 26 +++++++--------------
 libc/src/stdlib/CMakeLists.txt              |  3 ---
 libc/src/stdlib/heap_limit.S                |  5 ----
 libc/test/src/__support/CMakeLists.txt      |  1 +
 libc/test/src/__support/heap_limit_fake.cpp | 20 ++++++++++++++++
 8 files changed, 29 insertions(+), 39 deletions(-)
 delete mode 100644 libc/src/stdlib/heap_limit.S
 create mode 100644 libc/test/src/__support/heap_limit_fake.cpp

diff --git a/libc/config/baremetal/config.json b/libc/config/baremetal/config.json
index ebab661cb8d56..f0ff268b56275 100644
--- a/libc/config/baremetal/config.json
+++ b/libc/config/baremetal/config.json
@@ -18,11 +18,6 @@
       "value": false
     }
   },
-  "malloc": {
-    "LIBC_CONF_FREELIST_MALLOC_BUFFER_SIZE": {
-      "value": 131072
-    }
-  },
   "qsort": {
     "LIBC_CONF_QSORT_IMPL": {
       "value": "LIBC_QSORT_HEAP_SORT"
diff --git a/libc/config/config.json b/libc/config/config.json
index 2bf432ecae342..3532925b745e7 100644
--- a/libc/config/config.json
+++ b/libc/config/config.json
@@ -71,12 +71,6 @@
       "doc": "Default number of spins before blocking if a rwlock is in contention (default to 100)."
     }
   },
-  "malloc": {
-    "LIBC_CONF_FREELIST_MALLOC_BUFFER_SIZE": {
-      "value": 1073741824,
-      "doc": "Default size for the constinit freelist buffer used for the freelist malloc implementation (default 1o 1GB)."
-    }
-  },
   "unistd": {
     "LIBC_CONF_ENABLE_TID_CACHE": {
       "value": true,
diff --git a/libc/docs/configure.rst b/libc/docs/configure.rst
index b81922367d8b7..1936c8791c129 100644
--- a/libc/docs/configure.rst
+++ b/libc/docs/configure.rst
@@ -30,8 +30,6 @@ to learn about the defaults for your platform and target.
     - ``LIBC_CONF_KEEP_FRAME_POINTER``: Keep frame pointer in functions for better debugging experience.
 * **"errno" options**
     - ``LIBC_CONF_ERRNO_MODE``: The implementation used for errno, acceptable values are LIBC_ERRNO_MODE_DEFAULT, LIBC_ERRNO_MODE_UNDEFINED, LIBC_ERRNO_MODE_THREAD_LOCAL, LIBC_ERRNO_MODE_SHARED, LIBC_ERRNO_MODE_EXTERNAL, and LIBC_ERRNO_MODE_SYSTEM.
-* **"malloc" options**
-    - ``LIBC_CONF_FREELIST_MALLOC_BUFFER_SIZE``: Default size for the constinit freelist buffer used for the freelist malloc implementation (default 1o 1GB).
 * **"math" options**
     - ``LIBC_CONF_MATH_OPTIMIZATIONS``: Configures optimizations for math functions. Values accepted are LIBC_MATH_SKIP_ACCURATE_PASS, LIBC_MATH_SMALL_TABLES, LIBC_MATH_NO_ERRNO, LIBC_MATH_NO_EXCEPT, and LIBC_MATH_FAST.
 * **"printf" options**
diff --git a/libc/src/__support/freelist_heap.h b/libc/src/__support/freelist_heap.h
index 0fda7c3b2c9dc..03ab58dbbe595 100644
--- a/libc/src/__support/freelist_heap.h
+++ b/libc/src/__support/freelist_heap.h
@@ -50,12 +50,10 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
     size_t total_free_calls;
   };
 
-  constexpr FreeListHeap()
-      : region_(&_end, size_t{0}), heap_limit_(&__libc_heap_limit) {}
+  constexpr FreeListHeap() : begin_(&_end), end_(&__libc_heap_limit) {}
 
   constexpr FreeListHeap(span<cpp::byte> region)
-      : region_(region), heap_limit_{} {
-    heap_stats_.total_bytes = region.size();
+      : begin_(region.begin()), end_(region.end()) {
   }
 
   void *allocate(size_t size);
@@ -68,7 +66,7 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
 
   const HeapStats &heap_stats() const { return heap_stats_; }
 
-  cpp::span<cpp::byte> region() const { return region_; }
+  cpp::span<cpp::byte> region() const { return {begin_, end_}; }
 
 private:
   void init();
@@ -79,16 +77,11 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
     return span<cpp::byte>(block->usable_space(), block->inner_size());
   }
 
-  bool is_valid_ptr(void *ptr) {
-    return ptr >= region_.begin() && ptr < region_.end();
-  }
+  bool is_valid_ptr(void *ptr) { return ptr >= begin_ && ptr < end_; }
 
   bool is_initialized_ = false;
-  cpp::span<cpp::byte> region_;
-
-  // Kept to initialize region_ by non-constexpr cast to size_t
-  cpp::byte *heap_limit_;
-
+  cpp::byte *begin_;
+  cpp::byte *end_;
   FreeListType freelist_{DEFAULT_BUCKETS};
   HeapStats heap_stats_{};
 };
@@ -108,11 +101,8 @@ class FreeListHeapBuffer : public FreeListHeap<NUM_BUCKETS> {
 
 template <size_t NUM_BUCKETS> void FreeListHeap<NUM_BUCKETS>::init() {
   LIBC_ASSERT(!is_initialized_ && "duplicate initialization");
-  if (heap_limit_) {
-    region_ = {region_.begin(), (size_t)heap_limit_};
-    heap_stats_.total_bytes = region_.size();
-  }
-  auto result = BlockType::init(region_);
+  heap_stats_.total_bytes = region().size();
+  auto result = BlockType::init(region());
   BlockType *block = *result;
   freelist_.add_chunk(block_to_span(block));
   is_initialized_ = true;
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt
index ced0936bc4043..d79acb390ff91 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -388,13 +388,10 @@ if(NOT LIBC_TARGET_OS_IS_GPU)
       freelist_malloc
       SRCS
         freelist_malloc.cpp
-        heap_limit.S
       HDRS
         malloc.h
       DEPENDS
         libc.src.__support.freelist_heap
-      COMPILE_OPTIONS
-        -DLIBC_FREELIST_MALLOC_SIZE=${LIBC_CONF_FREELIST_MALLOC_BUFFER_SIZE}
     )
     get_target_property(freelist_malloc_is_skipped libc.src.stdlib.freelist_malloc "SKIPPED")
     if(LIBC_TARGET_OS_IS_BAREMETAL AND NOT freelist_malloc_is_skipped)
diff --git a/libc/src/stdlib/heap_limit.S b/libc/src/stdlib/heap_limit.S
deleted file mode 100644
index 2a0a6665deb56..0000000000000
--- a/libc/src/stdlib/heap_limit.S
+++ /dev/null
@@ -1,5 +0,0 @@
-#ifndef LIBC_FREELIST_MALLOC_SIZE
-#error "LIBC_FREELIST_MALLOC_SIZE was not defined for this build."
-#endif
-.weak __libc_heap_limit
-.set __libc_heap_limit, LIBC_FREELIST_MALLOC_SIZE
diff --git a/libc/test/src/__support/CMakeLists.txt b/libc/test/src/__support/CMakeLists.txt
index f994f65e08cbb..7fb6fe2c84846 100644
--- a/libc/test/src/__support/CMakeLists.txt
+++ b/libc/test/src/__support/CMakeLists.txt
@@ -36,6 +36,7 @@ if(LLVM_LIBC_FULL_BUILD)
     SRCS
       freelist_heap_test.cpp
       freelist_malloc_test.cpp
+      heap_limit_fake.cpp
     DEPENDS
       libc.src.__support.CPP.span
       libc.src.__support.freelist_heap
diff --git a/libc/test/src/__support/heap_limit_fake.cpp b/libc/test/src/__support/heap_limit_fake.cpp
new file mode 100644
index 0000000000000..a03174ef7bdb7
--- /dev/null
+++ b/libc/test/src/__support/heap_limit_fake.cpp
@@ -0,0 +1,20 @@
+//===-- Test fake definition for __libc_heaplimit -------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/__support/CPP/cstddef.h"
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+extern "C" {
+// This isn't used in the unit tests, but it must be defined for the non-fake
+// version of the heap to work.
+cpp::byte __libc_heap_limit;
+}
+
+} // namespace LIBC_NAMESPACE_DECL

>From f606dd3271e31714f7728c1e556a66c97ecb8308 Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Fri, 19 Jul 2024 16:10:50 -0700
Subject: [PATCH 6/9] Address review comments

- __libc_heap_limit -> __llvm_libc_heap_limit
---
 libc/src/__support/freelist_heap.h          | 4 ++--
 libc/test/src/__support/heap_limit_fake.cpp | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/libc/src/__support/freelist_heap.h b/libc/src/__support/freelist_heap.h
index 03ab58dbbe595..fed0a39b878b9 100644
--- a/libc/src/__support/freelist_heap.h
+++ b/libc/src/__support/freelist_heap.h
@@ -23,7 +23,7 @@
 namespace LIBC_NAMESPACE_DECL {
 
 extern "C" cpp::byte _end;
-extern "C" cpp::byte __libc_heap_limit;
+extern "C" cpp::byte __llvm_libc_heap_limit;
 
 using cpp::optional;
 using cpp::span;
@@ -50,7 +50,7 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
     size_t total_free_calls;
   };
 
-  constexpr FreeListHeap() : begin_(&_end), end_(&__libc_heap_limit) {}
+  constexpr FreeListHeap() : begin_(&_end), end_(&__llvm_libc_heap_limit) {}
 
   constexpr FreeListHeap(span<cpp::byte> region)
       : begin_(region.begin()), end_(region.end()) {
diff --git a/libc/test/src/__support/heap_limit_fake.cpp b/libc/test/src/__support/heap_limit_fake.cpp
index a03174ef7bdb7..6be79880c45c5 100644
--- a/libc/test/src/__support/heap_limit_fake.cpp
+++ b/libc/test/src/__support/heap_limit_fake.cpp
@@ -1,4 +1,4 @@
-//===-- Test fake definition for __libc_heaplimit -------------------------===//
+//===-- Test fake definition for __llvm_libc_heaplimit --------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -14,7 +14,7 @@ namespace LIBC_NAMESPACE_DECL {
 extern "C" {
 // This isn't used in the unit tests, but it must be defined for the non-fake
 // version of the heap to work.
-cpp::byte __libc_heap_limit;
+cpp::byte __llvm_libc_heap_limit;
 }
 
 } // namespace LIBC_NAMESPACE_DECL

>From 9e86f735c512850cd6830227db0633ed48c2cf85 Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Mon, 22 Jul 2024 12:32:11 -0700
Subject: [PATCH 7/9] Include fake for _end too.

---
 libc/test/src/__support/heap_limit_fake.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libc/test/src/__support/heap_limit_fake.cpp b/libc/test/src/__support/heap_limit_fake.cpp
index 6be79880c45c5..82405499c1f5d 100644
--- a/libc/test/src/__support/heap_limit_fake.cpp
+++ b/libc/test/src/__support/heap_limit_fake.cpp
@@ -12,8 +12,9 @@
 namespace LIBC_NAMESPACE_DECL {
 
 extern "C" {
-// This isn't used in the unit tests, but it must be defined for the non-fake
+// These aren't used in the unit tests, but they must be defined for the non-fake
 // version of the heap to work.
+cpp::byte _end;
 cpp::byte __llvm_libc_heap_limit;
 }
 

>From d57848f202c53a7a6f4518b4b8cef994531af33a Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Mon, 22 Jul 2024 12:52:29 -0700
Subject: [PATCH 8/9] Define _end and __llvm_libc_heap_limit to fake buffer for
 malloc test

---
 libc/test/src/__support/CMakeLists.txt        |  2 +-
 libc/test/src/__support/fake_heap.s           | 15 +++++++++++++
 .../src/__support/freelist_malloc_test.cpp    | 11 +---------
 libc/test/src/__support/heap_limit_fake.cpp   | 21 -------------------
 4 files changed, 17 insertions(+), 32 deletions(-)
 create mode 100644 libc/test/src/__support/fake_heap.s
 delete mode 100644 libc/test/src/__support/heap_limit_fake.cpp

diff --git a/libc/test/src/__support/CMakeLists.txt b/libc/test/src/__support/CMakeLists.txt
index 7fb6fe2c84846..90de520405981 100644
--- a/libc/test/src/__support/CMakeLists.txt
+++ b/libc/test/src/__support/CMakeLists.txt
@@ -34,9 +34,9 @@ if(LLVM_LIBC_FULL_BUILD)
     SUITE
       libc-support-tests
     SRCS
+      fake_heap.s
       freelist_heap_test.cpp
       freelist_malloc_test.cpp
-      heap_limit_fake.cpp
     DEPENDS
       libc.src.__support.CPP.span
       libc.src.__support.freelist_heap
diff --git a/libc/test/src/__support/fake_heap.s b/libc/test/src/__support/fake_heap.s
new file mode 100644
index 0000000000000..69522f53c8b1f
--- /dev/null
+++ b/libc/test/src/__support/fake_heap.s
@@ -0,0 +1,15 @@
+//===-- Test fake definition for heap symbols -----------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+.globl _end, __llvm_libc_heap_limit
+
+.bss
+_end:
+.fill 1024
+__llvm_libc_heap_limit:
+
diff --git a/libc/test/src/__support/freelist_malloc_test.cpp b/libc/test/src/__support/freelist_malloc_test.cpp
index 4b774884d81b5..66a923f778218 100644
--- a/libc/test/src/__support/freelist_malloc_test.cpp
+++ b/libc/test/src/__support/freelist_malloc_test.cpp
@@ -16,16 +16,7 @@
 using LIBC_NAMESPACE::freelist_heap;
 using LIBC_NAMESPACE::FreeListHeapBuffer;
 
-class LlvmLibcFreeListMalloc : public LIBC_NAMESPACE::testing::Test {
-protected:
-  void SetUp() override {
-    freelist_heap = new (&buffer) FreeListHeapBuffer<1024>;
-  }
-
-  FreeListHeapBuffer<1024> buffer;
-};
-
-TEST_F(LlvmLibcFreeListMalloc, MallocStats) {
+TEST(LlvmLibcFreeListMalloc, MallocStats) {
   constexpr size_t kAllocSize = 256;
   constexpr size_t kCallocNum = 4;
   constexpr size_t kCallocSize = 64;
diff --git a/libc/test/src/__support/heap_limit_fake.cpp b/libc/test/src/__support/heap_limit_fake.cpp
deleted file mode 100644
index 82405499c1f5d..0000000000000
--- a/libc/test/src/__support/heap_limit_fake.cpp
+++ /dev/null
@@ -1,21 +0,0 @@
-//===-- Test fake definition for __llvm_libc_heaplimit --------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "src/__support/CPP/cstddef.h"
-#include "src/__support/macros/config.h"
-
-namespace LIBC_NAMESPACE_DECL {
-
-extern "C" {
-// These aren't used in the unit tests, but they must be defined for the non-fake
-// version of the heap to work.
-cpp::byte _end;
-cpp::byte __llvm_libc_heap_limit;
-}
-
-} // namespace LIBC_NAMESPACE_DECL

>From 300cdf8671fa8ab1d07b694bcdd7ab6d7b9ff7d9 Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Mon, 22 Jul 2024 13:02:28 -0700
Subject: [PATCH 9/9] Clang format

---
 libc/src/__support/freelist_heap.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/libc/src/__support/freelist_heap.h b/libc/src/__support/freelist_heap.h
index fed0a39b878b9..fed00d06716cf 100644
--- a/libc/src/__support/freelist_heap.h
+++ b/libc/src/__support/freelist_heap.h
@@ -53,8 +53,7 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
   constexpr FreeListHeap() : begin_(&_end), end_(&__llvm_libc_heap_limit) {}
 
   constexpr FreeListHeap(span<cpp::byte> region)
-      : begin_(region.begin()), end_(region.end()) {
-  }
+      : begin_(region.begin()), end_(region.end()) {}
 
   void *allocate(size_t size);
   void *aligned_allocate(size_t alignment, size_t size);



More information about the libc-commits mailing list