[libc-commits] [libc] [libc] Lazily initialize freelist malloc using symbols (PR #99254)
Daniel Thornburgh via libc-commits
libc-commits at lists.llvm.org
Fri Jul 19 16:19:19 PDT 2024
https://github.com/mysterymath updated https://github.com/llvm/llvm-project/pull/99254
>From caf079f871800f99373bee01132321bf6327754f 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/6] [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 dda4c42425755..621dfcd3a9ee3 100644
--- a/libc/config/baremetal/config.json
+++ b/libc/config/baremetal/config.json
@@ -15,7 +15,7 @@
},
"malloc": {
"LIBC_CONF_FREELIST_MALLOC_BUFFER_SIZE": {
- "value": 102400
+ "value": 131072
}
}
}
diff --git a/libc/src/__support/freelist_heap.h b/libc/src/__support/freelist_heap.h
index ce4f14b431585..31fca66b897b8 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 7b7e55db391fa..56aa9d77635e3 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -377,6 +377,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 b1f18b7efd7c1e59027dc324a1f4307440481137 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/6] 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 31fca66b897b8..8ca9003ff9724 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 266094a15235b27eb2cabd60cfc07ec0c59a9fe5 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/6] 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 8ca9003ff9724..463ac6a1fd591 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 2d2f6abfe57c87d941903917738ba9eba0d3104d 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/6] 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 463ac6a1fd591..ee448c04cfb22 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 27e7c184e8091e8a5726781d1b1daed80d003eeb 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/6] 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 621dfcd3a9ee3..53f232e31cc8a 100644
--- a/libc/config/baremetal/config.json
+++ b/libc/config/baremetal/config.json
@@ -12,10 +12,5 @@
"LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_MEGA_LONG_DOUBLE_TABLE": {
"value": false
}
- },
- "malloc": {
- "LIBC_CONF_FREELIST_MALLOC_BUFFER_SIZE": {
- "value": 131072
- }
}
}
diff --git a/libc/config/config.json b/libc/config/config.json
index e8feab20175f4..a6ae1bd501bee 100644
--- a/libc/config/config.json
+++ b/libc/config/config.json
@@ -65,12 +65,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)."
- }
- },
"math": {
"LIBC_CONF_MATH_OPTIMIZATIONS": {
"value": 0,
diff --git a/libc/docs/configure.rst b/libc/docs/configure.rst
index 9c641ef94570f..fe0cad09b7410 100644
--- a/libc/docs/configure.rst
+++ b/libc/docs/configure.rst
@@ -28,8 +28,6 @@ to learn about the defaults for your platform and target.
* **"codegen" options**
- ``LIBC_CONF_ENABLE_STRONG_STACK_PROTECTOR``: Enable -fstack-protector-strong to defend against stack smashing attack.
- ``LIBC_CONF_KEEP_FRAME_POINTER``: Keep frame pointer in functions for better debugging experience.
-* **"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 ee448c04cfb22..dccfd753cc45f 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 56aa9d77635e3..4704dee9d496f 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -377,13 +377,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 ee0db6b250385..caf09b03f4471 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 fba415e064d773736b6cdb9f737eecc09b6bc001 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/6] 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 dccfd753cc45f..d76c3d93dbf6e 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
More information about the libc-commits
mailing list