From d643410b7afc0b4bb967aafeaeb90e19d6b26b57 Mon Sep 17 00:00:00 2001 From: Daniel Thornburgh Date: Tue, 16 Jul 2024 11:12:17 -0700 Subject: [PATCH] [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 dda4c424257556..621dfcd3a9ee3a 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 ce4f14b4315853..d7fcbdc2aa0722 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 class FreeListHeap { size_t total_free_calls; }; - FreeListHeap(span 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 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 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(block_region_end_) - - reinterpret_cast(block_region_start_); - } + cpp::span region() const { return region_; } -protected: - constexpr void set_freelist_node(typename FreeListType::FreeListNode &node, - cpp::span chunk) { - freelist_.set_freelist_node(node, chunk); - } +private: + void init(); void *allocate_impl(size_t alignment, size_t size); -private: span block_to_span(BlockType *block) { return span(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 region_; FreeListType freelist_; HeapStats heap_stats_; }; template -struct FreeListHeapBuffer : public FreeListHeap { +class FreeListHeapBuffer : public FreeListHeap { using parent = FreeListHeap; using FreeListNode = typename parent::FreeListType::FreeListNode; +public: constexpr FreeListHeapBuffer() - : FreeListHeap(&block, buffer + sizeof(buffer), BUFF_SIZE), - block(0, BUFF_SIZE), node{}, buffer{} { - block.mark_last(); + : FreeListHeap{buffer}, buffer{} {} - cpp::span chunk(buffer, sizeof(buffer)); - parent::set_freelist_node(node, chunk); - } +private: + cpp::byte buffer[BUFF_SIZE]; +}; - typename parent::BlockType block; - FreeListNode node; - cpp::byte buffer[BUFF_SIZE - sizeof(block) - sizeof(node)]; +template +class FreeListHeapSymbols : public FreeListHeap { + using parent = FreeListHeap; + using FreeListNode = typename parent::FreeListType::FreeListNode; + +public: + constexpr FreeListHeapSymbols() + : FreeListHeap{{(cpp::byte*)&_end, (size_t)&__libc_heap_limit}} {} }; +template +void FreeListHeap::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 void *FreeListHeap::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 chunk) { diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt index 7b7e55db391fae..56aa9d77635e36 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 cfffa0425ff66d..e15e74fb18864c 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 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 00000000000000..d704d07165ed9e --- /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 5815d5dfc01ab4..fc4348aed6b56a 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 fake_global_buffer; \ + void SetUp() override { \ + freelist_heap = \ + new (&fake_global_buffer) FreeListHeapBuffer; \ + } \ 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 e9d7c63a4d438f..4b774884d81b5c 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();