From 39f789df931a2c8e8be6e1f7ed243babf1d6cc08 Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Tue, 19 Mar 2024 13:56:32 +0100 Subject: [PATCH] feat(esp_rom): patch heap walker to the ROM implementation modify existing patch of TLSF rom and add multi heap patch to add the walker feature to the ROM implementation of the heap component. --- components/esp_rom/CMakeLists.txt | 2 +- .../esp_rom/esp32c2/Kconfig.soc_caps.in | 4 + components/esp_rom/esp32c2/esp_rom_caps.h | 1 + .../esp_rom/esp32c5/Kconfig.soc_caps.in | 78 ------------------- .../esp32c5/beta3/esp32c5/Kconfig.soc_caps.in | 4 + .../esp_rom/include/esp_rom_multi_heap.h | 9 ++- .../esp_rom/patches/esp_rom_multi_heap.c | 8 -- components/esp_rom/patches/esp_rom_tlsf.c | 68 +++++++++++++++- components/heap/Kconfig | 9 --- components/heap/include/esp_heap_caps.h | 4 +- components/heap/include/multi_heap.h | 2 +- .../test_apps/heap_tests/main/test_walker.c | 5 +- components/heap/tlsf | 2 +- 13 files changed, 88 insertions(+), 108 deletions(-) diff --git a/components/esp_rom/CMakeLists.txt b/components/esp_rom/CMakeLists.txt index 9b9872b1da..3b05e51d59 100644 --- a/components/esp_rom/CMakeLists.txt +++ b/components/esp_rom/CMakeLists.txt @@ -44,7 +44,7 @@ else() endif() endif() - if(CONFIG_HEAP_TLSF_USE_ROM_IMPL AND (CONFIG_ESP_ROM_TLSF_CHECK_PATCH OR CONFIG_HEAP_TLSF_CHECK_PATCH)) + if(CONFIG_HEAP_TLSF_USE_ROM_IMPL AND CONFIG_ESP_ROM_TLSF_CHECK_PATCH) # This file shall be included in the build if TLSF in ROM is activated list(APPEND sources "patches/esp_rom_tlsf.c") endif() diff --git a/components/esp_rom/esp32c2/Kconfig.soc_caps.in b/components/esp_rom/esp32c2/Kconfig.soc_caps.in index 19913c950b..c1fa90c7e8 100644 --- a/components/esp_rom/esp32c2/Kconfig.soc_caps.in +++ b/components/esp_rom/esp32c2/Kconfig.soc_caps.in @@ -39,6 +39,10 @@ config ESP_ROM_HAS_HEAP_TLSF bool default y +config ESP_ROM_TLSF_CHECK_PATCH + bool + default y + config ESP_ROM_MULTI_HEAP_WALK_PATCH bool default y diff --git a/components/esp_rom/esp32c2/esp_rom_caps.h b/components/esp_rom/esp32c2/esp_rom_caps.h index 9c71f9b9e2..1362fd77f4 100644 --- a/components/esp_rom/esp32c2/esp_rom_caps.h +++ b/components/esp_rom/esp32c2/esp_rom_caps.h @@ -15,6 +15,7 @@ #define ESP_ROM_HAS_HAL_WDT (1) // ROM has the implementation of Watchdog HAL driver #define ESP_ROM_HAS_HAL_SYSTIMER (1) // ROM has the implementation of Systimer HAL driver #define ESP_ROM_HAS_HEAP_TLSF (1) // ROM has the implementation of the tlsf and multi-heap library +#define ESP_ROM_TLSF_CHECK_PATCH (1) // ROM does not contain the patch of tlsf_check_pool() #define ESP_ROM_MULTI_HEAP_WALK_PATCH (1) // ROM does not contain the patch of multi_heap_walk() #define ESP_ROM_HAS_LAYOUT_TABLE (1) // ROM has the layout table #define ESP_ROM_HAS_SPI_FLASH (1) // ROM has the implementation of SPI Flash driver diff --git a/components/esp_rom/esp32c5/Kconfig.soc_caps.in b/components/esp_rom/esp32c5/Kconfig.soc_caps.in index 4f1584f9b3..20c519bbcb 100644 --- a/components/esp_rom/esp32c5/Kconfig.soc_caps.in +++ b/components/esp_rom/esp32c5/Kconfig.soc_caps.in @@ -7,84 +7,6 @@ if IDF_TARGET_ESP32C5_BETA3_VERSION source "$IDF_PATH/components/esp_rom/esp32c5/beta3/esp32c5/Kconfig.soc_caps.in" endif -<<<<<<< HEAD if IDF_TARGET_ESP32C5_MP_VERSION source "$IDF_PATH/components/esp_rom/esp32c5/mp/esp32c5/Kconfig.soc_caps.in" endif -======= -config ESP_ROM_HAS_CRC_BE - bool - default y - -config ESP_ROM_HAS_JPEG_DECODE - bool - default y - -config ESP_ROM_UART_CLK_IS_XTAL - bool - default y - -config ESP_ROM_USB_SERIAL_DEVICE_NUM - int - default 3 - -config ESP_ROM_HAS_RETARGETABLE_LOCKING - bool - default y - -config ESP_ROM_GET_CLK_FREQ - bool - default y - -config ESP_ROM_HAS_RVFPLIB - bool - default y - -config ESP_ROM_HAS_HAL_WDT - bool - default y - -config ESP_ROM_HAS_HAL_SYSTIMER - bool - default y - -config ESP_ROM_HAS_HEAP_TLSF - bool - default y - -config ESP_ROM_TLSF_CHECK_PATCH - bool - default y - -config ESP_ROM_MULTI_HEAP_WALK_PATCH - bool - default y - -config ESP_ROM_HAS_LAYOUT_TABLE - bool - default y - -config ESP_ROM_HAS_SPI_FLASH - bool - default y - -config ESP_ROM_WITHOUT_REGI2C - bool - default y - -config ESP_ROM_HAS_NEWLIB_NORMAL_FORMAT - bool - default y - -config ESP_ROM_WDT_INIT_PATCH - bool - default y - -config ESP_ROM_RAM_APP_NEEDS_MMU_INIT - bool - default y - -config ESP_ROM_USB_OTG_NUM - int - default -1 ->>>>>>> 9468f4df09 (feat(heap): Add walker to the heap component) diff --git a/components/esp_rom/esp32c5/beta3/esp32c5/Kconfig.soc_caps.in b/components/esp_rom/esp32c5/beta3/esp32c5/Kconfig.soc_caps.in index f2b89066e3..0f8129562c 100644 --- a/components/esp_rom/esp32c5/beta3/esp32c5/Kconfig.soc_caps.in +++ b/components/esp_rom/esp32c5/beta3/esp32c5/Kconfig.soc_caps.in @@ -51,6 +51,10 @@ config ESP_ROM_TLSF_CHECK_PATCH bool default y +config ESP_ROM_MULTI_HEAP_WALK_PATCH + bool + default y + config ESP_ROM_HAS_LAYOUT_TABLE bool default y diff --git a/components/esp_rom/include/esp_rom_multi_heap.h b/components/esp_rom/include/esp_rom_multi_heap.h index c7dfa493ae..b95ce4607d 100644 --- a/components/esp_rom/include/esp_rom_multi_heap.h +++ b/components/esp_rom/include/esp_rom_multi_heap.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -20,10 +20,13 @@ typedef struct multi_heap_info *multi_heap_handle_t; * * @param block_ptr Pointer to the block data * @param block_size The size of the block - * @param block_used Block status. True if free, false if used + * @param block_used Block status. 0: free, 1: allocated * @param user_data Opaque pointer to user defined data + * + * @return True if the walker is expected to continue the heap traversal + * False if the walker is expected to stop the traversal of the heap */ -typedef void (*multi_heap_walker_cb_t)(void *block_ptr, size_t block_size, int block_used, void *user_data); +typedef bool (*multi_heap_walker_cb_t)(void *block_ptr, size_t block_size, int block_used, void *user_data); /** * @brief Call the tlsf_walk_pool function of the heap given as parameter with diff --git a/components/esp_rom/patches/esp_rom_multi_heap.c b/components/esp_rom/patches/esp_rom_multi_heap.c index 4690bfe4e4..6a6682dbf3 100644 --- a/components/esp_rom/patches/esp_rom_multi_heap.c +++ b/components/esp_rom/patches/esp_rom_multi_heap.c @@ -7,8 +7,6 @@ /* * This file is a patch for the multi_heap.c file stored in ROM * - added function multi_heap_walk - * - added function multi_heap_walker - * - added structure walker_data_t */ #include @@ -37,12 +35,6 @@ typedef struct multi_heap_info { void* heap_data; } heap_t; -typedef struct walker_data { - void *opaque_ptr; - multi_heap_walker_cb_t walker; - multi_heap_handle_t heap; -} walker_data_t; - extern void tlsf_walk_pool(pool_t pool, tlsf_walker walker, void* user); extern pool_t tlsf_get_pool(tlsf_t tlsf); extern void multi_heap_internal_lock(multi_heap_handle_t heap); diff --git a/components/esp_rom/patches/esp_rom_tlsf.c b/components/esp_rom/patches/esp_rom_tlsf.c index 06da058328..ad3af65d42 100644 --- a/components/esp_rom/patches/esp_rom_tlsf.c +++ b/components/esp_rom/patches/esp_rom_tlsf.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -15,6 +15,7 @@ #include #include #include +#include #include "esp_rom_caps.h" #include "esp_rom_tlsf.h" @@ -24,13 +25,17 @@ */ typedef void* tlsf_t; typedef void* pool_t; -typedef void* tlsf_walker; +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) @@ -72,6 +77,30 @@ static inline __attribute__((always_inline)) block_header_t* block_from_ptr(cons 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 @@ -92,7 +121,9 @@ typedef struct integrity_t int status; } integrity_t; -static void integrity_walker(void* ptr, size_t size, int used, void* user) +typedef bool (*tlsf_walker)(void* ptr, size_t size, int used, void* user); + +static bool integrity_walker(void* ptr, size_t size, int used, void* user) { block_header_t* block = block_from_ptr(ptr); integrity_t* integ = tlsf_cast(integrity_t*, user); @@ -124,9 +155,38 @@ static void integrity_walker(void* ptr, size_t size, int used, void* user) integ->prev_status = this_status; integ->status += status; + + return true; +} + +static bool default_walker(void* ptr, size_t size, int used, void* user) +{ + (void)user; + printf("\t%p %s size: %x (%p)\n", ptr, used ? "used" : "free", (unsigned int)size, block_from_ptr(ptr)); + return true; +} + +void tlsf_walk_pool(pool_t pool, tlsf_walker walker, void* user) +{ + tlsf_walker pool_walker = walker ? walker : default_walker; + block_header_t* block = + offset_to_block(pool, -(int)block_header_overhead); + + bool ret_val = true; + while (block && !block_is_last(block) && ret_val == true) + { + ret_val = pool_walker( + block_to_ptr(block), + block_size(block), + !block_is_free(block), + user); + + if (ret_val == true) { + block = block_next(block); + } + } } -extern void tlsf_walk_pool(pool_t pool, tlsf_walker walker, void* user); int tlsf_check_pool(pool_t pool) { /* Check that the blocks are physically correct. */ diff --git a/components/heap/Kconfig b/components/heap/Kconfig index e593e4707a..723ff3d300 100644 --- a/components/heap/Kconfig +++ b/components/heap/Kconfig @@ -119,15 +119,6 @@ menu "Heap memory debugging" features will be added and bugs will be fixed in the IDF source but cannot be synced to ROM. - config HEAP_TLSF_CHECK_PATCH - bool "Patch the tlsf_check_pool() for ROM HEAP TLSF implementation" - depends on HEAP_TLSF_USE_ROM_IMPL && IDF_TARGET_ESP32C2 && ESP32C2_REV_MIN_FULL < 200 - default y - help - ROM does not contain the patch of tlsf_check_pool() allowing perform - the integrity checking on used blocks. The patch to allow such check - needs to be applied. - config HEAP_PLACE_FUNCTION_INTO_FLASH bool "Force the entire heap component to be placed in flash memory" depends on !HEAP_TLSF_USE_ROM_IMPL diff --git a/components/heap/include/esp_heap_caps.h b/components/heap/include/esp_heap_caps.h index 96e9b88977..4d590204e5 100644 --- a/components/heap/include/esp_heap_caps.h +++ b/components/heap/include/esp_heap_caps.h @@ -460,7 +460,7 @@ typedef struct walker_heap_info { typedef struct walker_block_info { void *ptr; ///< Pointer to the block data size_t size; ///< The size of the block - bool used; ///< Block status. True if free, false if used + bool used; ///< Block status. True: used, False: free } walker_block_info_t; /** @@ -472,7 +472,7 @@ typedef struct walker_block_info { * @param user_data Opaque pointer to user defined data * * @return True to proceed with the heap traversal - * False to stop th traversal of the current heap and continue + * False to stop the traversal of the current heap and continue * with the traversal of the next heap (if any) */ typedef bool (*heap_caps_walker_cb_t)(walker_heap_into_t heap_info, walker_block_info_t block_info, void *user_data); diff --git a/components/heap/include/multi_heap.h b/components/heap/include/multi_heap.h index f769fa4a7f..20e1ba977e 100644 --- a/components/heap/include/multi_heap.h +++ b/components/heap/include/multi_heap.h @@ -212,7 +212,7 @@ void multi_heap_restore_minimum_free_bytes(multi_heap_handle_t heap, const size_ * * @param block_ptr Pointer to the block data * @param block_size The size of the block - * @param block_used Block status. 0 if free, else, false + * @param block_used Block status. 0: free, 1: allocated * @param user_data Opaque pointer to user defined data * * @return True if the walker is expected to continue the heap traversal diff --git a/components/heap/test_apps/heap_tests/main/test_walker.c b/components/heap/test_apps/heap_tests/main/test_walker.c index 3a96df7df2..8db8dc449d 100644 --- a/components/heap/test_apps/heap_tests/main/test_walker.c +++ b/components/heap/test_apps/heap_tests/main/test_walker.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Unlicense OR CC0-1.0 */ @@ -85,6 +85,9 @@ TEST_CASE("heap walker corrupted heap detection", "[heap]") size_t metadata_size_tail = 0; calculate_block_metadata_size(&metadata_size_head, &metadata_size_tail); (void)metadata_size_tail; + + /* corrupting the size field of the block metadata with a size bigger + * than the heap itself */ *((uint32_t*)default_ptr - (metadata_size_head / 4) - 1) = 0xFF000000; /* Write the pass code in the first word of the allocated memory */ diff --git a/components/heap/tlsf b/components/heap/tlsf index e282f0be5a..8fc595fe22 160000 --- a/components/heap/tlsf +++ b/components/heap/tlsf @@ -1 +1 @@ -Subproject commit e282f0be5a8488f48bf04cdc74514553c03e3c64 +Subproject commit 8fc595fe223cd0b3b5d7b29eb86825e4bd38e6e8