From ccd8486462eea77b683aa8bf55af4bc064627bbd Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Thu, 29 Aug 2024 10:07:44 +0200 Subject: [PATCH] feat(heap): Update component to latest TLSF The new TLSF architecture has changed. tlsf.h public API header is now moved into an include folder. tlsf_common.h is removed from the repo. This commit updates the heap component and respective esp_rom patches to take into account this new architecture. --- components/esp_rom/patches/esp_rom_tlsf.c | 88 ++----------------- components/heap/CMakeLists.txt | 22 +++-- components/heap/linker.lf | 3 - components/heap/multi_heap.c | 12 ++- .../test_apps/heap_tests/main/test_malloc.c | 2 +- components/heap/test_multi_heap_host/Makefile | 2 +- .../test_multi_heap_host/test_multi_heap.cpp | 24 ++--- components/heap/tlsf | 2 +- tools/ci/check_public_headers_exceptions.txt | 5 ++ 9 files changed, 45 insertions(+), 115 deletions(-) diff --git a/components/esp_rom/patches/esp_rom_tlsf.c b/components/esp_rom/patches/esp_rom_tlsf.c index ad3af65d42..3a795edb3c 100644 --- a/components/esp_rom/patches/esp_rom_tlsf.c +++ b/components/esp_rom/patches/esp_rom_tlsf.c @@ -20,91 +20,12 @@ #include "esp_rom_caps.h" #include "esp_rom_tlsf.h" -/*! - * @brief Opaque types for TLSF implementation - */ +#include "tlsf_block_functions.h" +#include "tlsf_control_functions.h" + +/* Definition of types used in TLSF */ typedef void* tlsf_t; typedef void* pool_t; -typedef ptrdiff_t tlsfptr_t; - -/* ---------------------------------------------------------------- - * Bring certain inline functions, macro and structures from the - * tlsf ROM implementation to be able to compile the patch. - * ---------------------------------------------------------------- */ - -#if !defined (tlsf_assert) -#define tlsf_assert assert -#endif - -#define tlsf_cast(t, exp) ((t) (exp)) - -#define block_header_free_bit (1 << 0) -#define block_header_prev_free_bit (1 << 1) -#define block_header_overhead (sizeof(size_t)) -#define block_start_offset (offsetof(block_header_t, size) + sizeof(size_t)) - -typedef struct block_header_t -{ - /* Points to the previous physical block. */ - struct block_header_t* prev_phys_block; - - /* The size of this block, excluding the block header. */ - size_t size; - - /* Next and previous free blocks. */ - struct block_header_t* next_free; - struct block_header_t* prev_free; -} block_header_t; - -static inline __attribute__((__always_inline__)) size_t block_size(const block_header_t* block) -{ - return block->size & ~(block_header_free_bit | block_header_prev_free_bit); -} - -static inline __attribute__((__always_inline__)) int block_is_free(const block_header_t* block) -{ - return tlsf_cast(int, block->size & block_header_free_bit); -} - -static inline __attribute__((__always_inline__)) int block_is_prev_free(const block_header_t* block) -{ - return tlsf_cast(int, block->size & block_header_prev_free_bit); -} - -static inline __attribute__((always_inline)) block_header_t* block_from_ptr(const void* ptr) -{ - return tlsf_cast(block_header_t*, - tlsf_cast(unsigned char*, ptr) - block_start_offset); -} - -static inline __attribute__((always_inline)) block_header_t* offset_to_block(const void* ptr, size_t size) -{ - return tlsf_cast(block_header_t*, tlsf_cast(tlsfptr_t, ptr) + size); -} - -static inline __attribute__((always_inline)) int block_is_last(const block_header_t* block) -{ - return block_size(block) == 0; -} - -static inline __attribute__((always_inline)) void* block_to_ptr(const block_header_t* block) -{ - return tlsf_cast(void*, - tlsf_cast(unsigned char*, block) + block_start_offset); -} - -static inline __attribute__((always_inline)) block_header_t* block_next(const block_header_t* block) -{ - block_header_t* next = offset_to_block(block_to_ptr(block), - block_size(block) - block_header_overhead); - tlsf_assert(!block_is_last(block)); - return next; -} - -/* ---------------------------------------------------------------- - * End of the environment necessary to compile and link the patch - * defined below - * ---------------------------------------------------------------- */ static poison_check_pfunc_t s_poison_check_region = NULL; @@ -237,6 +158,7 @@ struct heap_tlsf_stub_table_t { extern struct heap_tlsf_stub_table_t* heap_tlsf_table_ptr; /* We will copy the ROM table and modify the functions we patch */ + struct heap_tlsf_stub_table_t heap_tlsf_patch_table_ptr; /*! diff --git a/components/heap/CMakeLists.txt b/components/heap/CMakeLists.txt index 33b822ee5d..3fb425f23b 100644 --- a/components/heap/CMakeLists.txt +++ b/components/heap/CMakeLists.txt @@ -7,16 +7,24 @@ if(${target} STREQUAL "linux") return() endif() -set(srcs - "heap_caps_base.c" - "heap_caps.c" - "heap_caps_init.c" - "multi_heap.c") +set(srcs "heap_caps_base.c" + "heap_caps.c" + "heap_caps_init.c" + "multi_heap.c") -set(includes "include") +# the root dir of TLSF submodule contains headers with static inline +# functions used in the esp_rom component for TLSF patches. Therefore, +# the tlsf/ dir must be included in the list of public includes. +set(includes "include" + "tlsf") + +# The TLSF "public" includes should only be used +# inside the heap component and are therefore added +# to the list of the private includes from the heap +# component perspective +set(priv_includes "tlsf/include") if(NOT CONFIG_HEAP_TLSF_USE_ROM_IMPL) - set(priv_includes "tlsf") list(APPEND srcs "tlsf/tlsf.c") endif() diff --git a/components/heap/linker.lf b/components/heap/linker.lf index b40ea44cce..7a65d269e7 100644 --- a/components/heap/linker.lf +++ b/components/heap/linker.lf @@ -5,9 +5,6 @@ entries: if HEAP_TLSF_USE_ROM_IMPL = n: tlsf:tlsf_block_size (noflash) tlsf:tlsf_size (noflash) - tlsf:tlsf_align_size (noflash) - tlsf:tlsf_block_size_min (noflash) - tlsf:tlsf_block_size_max (noflash) tlsf:tlsf_alloc_overhead (noflash) tlsf:tlsf_get_pool (noflash) tlsf:tlsf_malloc (noflash) diff --git a/components/heap/multi_heap.c b/components/heap/multi_heap.c index a92b4aa564..d47a6c0a3a 100644 --- a/components/heap/multi_heap.c +++ b/components/heap/multi_heap.c @@ -15,10 +15,8 @@ #include "multi_heap.h" #include "multi_heap_internal.h" -#if !CONFIG_HEAP_TLSF_USE_ROM_IMPL #include "tlsf.h" #include "tlsf_block_functions.h" -#endif /* Note: Keep platform-specific parts in this header, this source file should depend on libc only */ @@ -110,7 +108,7 @@ void multi_heap_in_rom_init(void) #else // CONFIG_HEAP_TLSF_USE_ROM_IMPL /* Check a block is valid for this heap. Used to verify parameters. */ -__attribute__((noinline)) NOCLONE_ATTR static void assert_valid_block(const heap_t *heap, const block_header_t *block) +__attribute__((noinline)) NOCLONE_ATTR static void assert_valid_block(const heap_t *heap, const multi_heap_block_handle_t block) { pool_t pool = tlsf_get_pool(heap->heap_data); void *ptr = block_to_ptr(block); @@ -175,22 +173,22 @@ multi_heap_block_handle_t multi_heap_get_first_block(multi_heap_handle_t heap) { assert(heap != NULL); pool_t pool = tlsf_get_pool(heap->heap_data); - block_header_t* block = offset_to_block(pool, -(int)block_header_overhead); + multi_heap_block_handle_t block = offset_to_block(pool, -(int)block_header_overhead); - return (multi_heap_block_handle_t)block; + return block; } multi_heap_block_handle_t multi_heap_get_next_block(multi_heap_handle_t heap, multi_heap_block_handle_t block) { assert(heap != NULL); assert_valid_block(heap, block); - block_header_t* next = block_next(block); + multi_heap_block_handle_t next = block_next(block); if(block_size(next) == 0) { //Last block: return NULL; } else { - return (multi_heap_block_handle_t)next; + return next; } } diff --git a/components/heap/test_apps/heap_tests/main/test_malloc.c b/components/heap/test_apps/heap_tests/main/test_malloc.c index 38b4ea18a0..8cd4078afd 100644 --- a/components/heap/test_apps/heap_tests/main/test_malloc.c +++ b/components/heap/test_apps/heap_tests/main/test_malloc.c @@ -176,7 +176,7 @@ TEST_CASE("test get allocated size", "[heap]") const size_t aligned_size = (alloc_sizes[i] + 3) & ~3; const size_t real_size = heap_caps_get_allocated_size(ptr_array[i]); printf("initial size: %d, requested size : %d, allocated size: %d\n", alloc_sizes[i], aligned_size, real_size); - TEST_ASSERT_EQUAL(aligned_size, real_size); + TEST_ASSERT(aligned_size <= real_size); heap_caps_free(ptr_array[i]); } diff --git a/components/heap/test_multi_heap_host/Makefile b/components/heap/test_multi_heap_host/Makefile index 7d97cbcc6f..dadfad3160 100644 --- a/components/heap/test_multi_heap_host/Makefile +++ b/components/heap/test_multi_heap_host/Makefile @@ -13,7 +13,7 @@ SOURCE_FILES = $(abspath \ main.cpp \ ) -INCLUDE_FLAGS = -I../include -I../../../tools/catch -I../tlsf +INCLUDE_FLAGS = -I../include -I../../../tools/catch -I../tlsf -I../tlsf/include GCOV ?= gcov diff --git a/components/heap/test_multi_heap_host/test_multi_heap.cpp b/components/heap/test_multi_heap_host/test_multi_heap.cpp index 36ebfb4f52..6bc66cb6e2 100644 --- a/components/heap/test_multi_heap_host/test_multi_heap.cpp +++ b/components/heap/test_multi_heap_host/test_multi_heap.cpp @@ -8,9 +8,9 @@ #include "multi_heap.h" #include "../multi_heap_config.h" -#include "../tlsf/tlsf.h" -#include "../tlsf/tlsf_common.h" +#include "../tlsf/include/tlsf.h" #include "../tlsf/tlsf_block_functions.h" +#include "../tlsf/tlsf_control_functions.h" #include #include @@ -19,7 +19,7 @@ * malloc and free and allocate memory from the host heap. Since the test * `TEST_CASE("multi_heap many random allocations", "[multi_heap]")` * calls multi_heap_allocation_impl() with sizes that can go up to 8MB, - * an allocatation on the heap will be prefered rather than the stack which + * an allocatation on the heap will be preferred rather than the stack which * might not have the necessary memory. */ static void *__malloc__(size_t bytes) @@ -62,7 +62,7 @@ TEST_CASE("multi_heap simple allocations", "[multi_heap]") REQUIRE( (intptr_t)buf < (intptr_t)(small_heap + sizeof(small_heap))); REQUIRE( multi_heap_get_allocated_size(heap, buf) >= test_alloc_size ); - REQUIRE( multi_heap_get_allocated_size(heap, buf) < test_alloc_size + 16); + printf("test alloc size %d\n", test_alloc_size); memset(buf, 0xEE, test_alloc_size); @@ -482,7 +482,7 @@ TEST_CASE("multi_heap aligned allocations", "[multi_heap]") { uint8_t test_heap[4 * 1024]; multi_heap_handle_t heap = multi_heap_register(test_heap, sizeof(test_heap)); - uint32_t aligments = 0; // starts from alignment by 4-byte boundary + uint32_t alignments = 0; // starts from alignment by 4-byte boundary size_t old_size = multi_heap_free_size(heap); size_t leakage = 1024; printf("[ALIGNED_ALLOC] heap_size before: %d \n", old_size); @@ -491,26 +491,26 @@ TEST_CASE("multi_heap aligned allocations", "[multi_heap]") multi_heap_dump(heap); printf("*********************\n"); - for(;aligments <= 256; aligments++) { + for(;alignments <= 256; alignments++) { //Use some stupid size value to test correct alignment even in strange //memory layout objects: - uint8_t *buf = (uint8_t *)multi_heap_aligned_alloc(heap, (aligments + 137), aligments ); - if(((aligments & (aligments - 1)) != 0) || (!aligments)) { + uint8_t *buf = (uint8_t *)multi_heap_aligned_alloc(heap, (alignments + 137), alignments ); + if(((alignments & (alignments - 1)) != 0) || (!alignments)) { REQUIRE( buf == NULL ); } else { REQUIRE( buf != NULL ); REQUIRE((intptr_t)buf >= (intptr_t)test_heap); REQUIRE((intptr_t)buf < (intptr_t)(test_heap + sizeof(test_heap))); - printf("[ALIGNED_ALLOC] alignment required: %u \n", aligments); + printf("[ALIGNED_ALLOC] alignment required: %u \n", alignments); printf("[ALIGNED_ALLOC] address of allocated memory: %p \n\n", (void *)buf); //Address of obtained block must be aligned with selected value - REQUIRE(((intptr_t)buf & (aligments - 1)) == 0); + REQUIRE(((intptr_t)buf & (alignments - 1)) == 0); //Write some data, if it corrupts memory probably the heap //canary verification will fail: - memset(buf, 0xA5, (aligments + 137)); + memset(buf, 0xA5, (alignments + 137)); multi_heap_free(heap, buf); } @@ -519,7 +519,7 @@ TEST_CASE("multi_heap aligned allocations", "[multi_heap]") /* Check that TLSF doesn't allocate a memory space smaller than required. * In any case, TLSF will write data in the previous block than the one * allocated. Thus, we should try to get/allocate this previous block. If - * the poisoned filled pattern has beeen overwritten by TLSF, then this + * the poisoned filled pattern has been overwritten by TLSF, then this * previous block will trigger an exception. * More info on this bug in !16296. */ const size_t size = 50; /* TLSF will round the size up */ diff --git a/components/heap/tlsf b/components/heap/tlsf index 8fc595fe22..ba64d198a8 160000 --- a/components/heap/tlsf +++ b/components/heap/tlsf @@ -1 +1 @@ -Subproject commit 8fc595fe223cd0b3b5d7b29eb86825e4bd38e6e8 +Subproject commit ba64d198a845df70b481e2c55004521ca643dea6 diff --git a/tools/ci/check_public_headers_exceptions.txt b/tools/ci/check_public_headers_exceptions.txt index 397518950e..1fe79d0775 100644 --- a/tools/ci/check_public_headers_exceptions.txt +++ b/tools/ci/check_public_headers_exceptions.txt @@ -68,6 +68,11 @@ components/cmock/CMock/src/cmock_internals.h components/openthread/openthread/ +# The following TLSF headers contain object definitions but have to be +# made public to be used in esp_rom to help patching the TLSF in ROM. +components/heap/tlsf/tlsf_block_functions.h +components/heap/tlsf/tlsf_control_functions.h + ### Here are the files that do not compile for some reason # components/app_trace/include/esp_sysview_trace.h