From 7fcd964b1df99c13efbd505fd6d3fa25324b6af1 Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Thu, 3 Nov 2022 08:44:56 +0100 Subject: [PATCH 1/6] heap: add selective placement of function in IRAM This commit aims to place in the IRAM section only the functions that are relevent for performance instead of placing the entire content of multi_heap.c, mullti_heap_poisoning.c and tlsf.c in the IRAM. --- components/heap/heap_private.h | 2 +- components/heap/linker.lf | 52 ++++++++++++++++++++++++++++++++-- components/heap/multi_heap.c | 19 ++----------- 3 files changed, 53 insertions(+), 20 deletions(-) diff --git a/components/heap/heap_private.h b/components/heap/heap_private.h index 1d2065cd473..51b6773ffad 100644 --- a/components/heap/heap_private.h +++ b/components/heap/heap_private.h @@ -43,7 +43,7 @@ extern SLIST_HEAD(registered_heap_ll, heap_t_) registered_heaps; bool heap_caps_match(const heap_t *heap, uint32_t caps); /* return all possible capabilities (across all priorities) for a given heap */ -inline static IRAM_ATTR uint32_t get_all_caps(const heap_t *heap) +inline static uint32_t get_all_caps(const heap_t *heap) { if (heap->heap == NULL) { return 0; diff --git a/components/heap/linker.lf b/components/heap/linker.lf index be3f7796c9f..2d68361ad96 100644 --- a/components/heap/linker.lf +++ b/components/heap/linker.lf @@ -2,7 +2,53 @@ archive: libheap.a entries: if HEAP_TLSF_USE_ROM_IMPL = n: - tlsf (noflash) - multi_heap (noflash) + tlsf:tlsf_ffs (noflash) + tlsf:tlsf_fls (noflash) + 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) + tlsf:tlsf_memalign_offs (noflash) + tlsf:tlsf_memalign (noflash) + tlsf:tlsf_free (noflash) + tlsf:tlsf_realloc (noflash) + + multi_heap:assert_valid_block (noflash) + multi_heap:multi_heap_get_block_address_impl (noflash) + multi_heap:multi_heap_get_allocated_size_impl (noflash) + multi_heap:multi_heap_set_lock (noflash) + multi_heap:multi_heap_get_first_block (noflash) + multi_heap:multi_heap_get_next_block (noflash) + multi_heap:multi_heap_is_free (noflash) + multi_heap:multi_heap_malloc_impl (noflash) + multi_heap:multi_heap_free_impl (noflash) + multi_heap:multi_heap_realloc_impl (noflash) + multi_heap:multi_heap_aligned_alloc_impl_offs (noflash) + multi_heap:multi_heap_aligned_alloc_impl (noflash) + + if HEAP_TLSF_USE_ROM_IMPL = y: + multi_heap:_multi_heap_lock (noflash) + multi_heap:_multi_heap_unlock (noflash) + multi_heap:multi_heap_in_rom_init (noflash) + if HEAP_POISONING_DISABLED = n: - multi_heap_poisoning (noflash) + multi_heap_poisoning:poison_allocated_region (noflash) + multi_heap_poisoning:verify_allocated_region (noflash) + multi_heap_poisoning:multi_heap_aligned_alloc (noflash) + multi_heap_poisoning:multi_heap_malloc (noflash) + multi_heap_poisoning:multi_heap_free (noflash) + multi_heap_poisoning:multi_heap_aligned_free (noflash) + multi_heap_poisoning:multi_heap_realloc (noflash) + multi_heap_poisoning:multi_heap_get_block_address (noflash) + multi_heap_poisoning:multi_heap_get_block_owner (noflash) + multi_heap_poisoning:multi_heap_get_allocated_size (noflash) + multi_heap_poisoning:multi_heap_internal_check_block_poisoning (noflash) + multi_heap_poisoning:multi_heap_internal_poison_fill_region (noflash) + + if HEAP_POISONING_COMPREHENSIVE = y: + multi_heap_poisoning:verify_fill_pattern (noflash) + multi_heap_poisoning:block_absorb_post_hook (noflash) diff --git a/components/heap/multi_heap.c b/components/heap/multi_heap.c index 09b29e6fc8c..aa14707631f 100644 --- a/components/heap/multi_heap.c +++ b/components/heap/multi_heap.c @@ -105,18 +105,6 @@ void multi_heap_in_rom_init(void) #else // CONFIG_HEAP_TLSF_USE_ROM_IMPL -/* Return true if this block is free. */ -static inline bool is_free(const block_header_t *block) -{ - return ((block->size & 0x01) != 0); -} - -/* Data size of the block (excludes this block's header) */ -static inline size_t block_data_size(const block_header_t *block) -{ - return (block->size & ~0x03); -} - /* Check a block is valid for this heap. Used to verify parameters. */ static void assert_valid_block(const heap_t *heap, const block_header_t *block) { @@ -130,8 +118,7 @@ static void assert_valid_block(const heap_t *heap, const block_header_t *block) void *multi_heap_get_block_address_impl(multi_heap_block_handle_t block) { - void *ptr = block_to_ptr(block); - return (ptr); + return block_to_ptr(block); } size_t multi_heap_get_allocated_size_impl(multi_heap_handle_t heap, void *p) @@ -195,7 +182,7 @@ multi_heap_block_handle_t multi_heap_get_next_block(multi_heap_handle_t heap, mu assert_valid_block(heap, block); block_header_t* next = block_next(block); - if(block_data_size(next) == 0) { + if(block_size(next) == 0) { //Last block: return NULL; } else { @@ -206,7 +193,7 @@ multi_heap_block_handle_t multi_heap_get_next_block(multi_heap_handle_t heap, mu bool multi_heap_is_free(multi_heap_block_handle_t block) { - return is_free(block); + return block_is_free(block); } void *multi_heap_malloc_impl(multi_heap_handle_t heap, size_t size) From 5dc0481d216b8683accf89dddc7989ff7cc9a95e Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Thu, 3 Nov 2022 12:38:36 +0100 Subject: [PATCH 2/6] heap: add documentation about the function placement in IRAM and its usage in ISR This commits adds a internal.md file in the heap directory to clarify the idea behind which functions is placed in IRAM or in flash. A section in mem_alloc.rst documentation is added to specify which functions from the heap component API can be used in interrupt handlers. --- components/heap/internals.md | 9 +++++++++ docs/en/api-reference/system/mem_alloc.rst | 23 +++++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 components/heap/internals.md diff --git a/components/heap/internals.md b/components/heap/internals.md new file mode 100644 index 00000000000..d007f1d4c83 --- /dev/null +++ b/components/heap/internals.md @@ -0,0 +1,9 @@ +# Function placement in IRAM section + +The heap component is compiled and linked in a way that minimizes the utilization of the IRAM section of memory without impacting the performance of its core functionalities. For this reason, the heap component API provided through [esp_heap_caps.h](./include/esp_heap_caps.h) and [esp_heap_caps_init.h](./include/esp_heap_caps_init.h) can be sorted into two sets of functions. + +1. The performance related functions placed into the IRAM by using the `IRAM_ATTR` defined in [esp_attr.h](./../../components/esp_common/include/esp_attr.h) (e.g., `heap_caps_malloc`, `heap_caps_free`, `heap_caps_realloc`, etc.) + +2. The functions that does not require the best of performance placed in the flash (e.g., `heap_caps_print_heap_info`, `heap_caps_dump`, `heap_caps_dump_all`, etc.) + +With that in mind, all the functions defined in [multi_heap.c](./multi_heap.c), [multi_heap_poisoning.c](./multi_heap_poisoning.c) and [tlsf.c](./tlsf/tlsf.c) that are directly or indirectly called from one of the heap component API functions placed in IRAM have to also be placed in IRAM. Symmetrically, the functions directly or indirectly called from one of the heap component API functions placed in flash will also be placed in flash. \ No newline at end of file diff --git a/docs/en/api-reference/system/mem_alloc.rst b/docs/en/api-reference/system/mem_alloc.rst index 8ee2bcb808f..7aa5a2635ff 100644 --- a/docs/en/api-reference/system/mem_alloc.rst +++ b/docs/en/api-reference/system/mem_alloc.rst @@ -129,7 +129,28 @@ Thread Safety Heap functions are thread safe, meaning they can be called from different tasks simultaneously without any limitations. -It is technically possible to call ``malloc``, ``free``, and related functions from interrupt handler (ISR) context. However this is not recommended, as heap function calls may delay other interrupts. It is strongly recommended to refactor applications so that any buffers used by an ISR are pre-allocated outside of the ISR. Support for calling heap functions from ISRs may be removed in a future update. +It is technically possible to call ``malloc``, ``free``, and related functions from interrupt handler (ISR) context (see :ref:`calling-heap-related-functions-from-isr`). However this is not recommended, as heap function calls may delay other interrupts. It is strongly recommended to refactor applications so that any buffers used by an ISR are pre-allocated outside of the ISR. Support for calling heap functions from ISRs may be removed in a future update. + +.. _calling-heap-related-functions-from-isr: + +Calling heap related functions from ISR +--------------------------------------- + +The following functions from the heap component can be called form interrupt handler (ISR): + +* :cpp:func:`heap_caps_malloc` +* :cpp:func:`heap_caps_malloc_default` +* :cpp:func:`heap_caps_realloc_default` +* :cpp:func:`heap_caps_malloc_prefer` +* :cpp:func:`heap_caps_realloc_prefer` +* :cpp:func:`heap_caps_calloc_prefer` +* :cpp:func:`heap_caps_free` +* :cpp:func:`heap_caps_realloc` +* :cpp:func:`heap_caps_calloc` +* :cpp:func:`heap_caps_aligned_alloc` +* :cpp:func:`heap_caps_aligned_free` + +Note however this practice is strongly discouraged. Heap Tracing & Debugging ------------------------ From 0376e6ec790f4a28a60f148b528675baee3318ee Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Fri, 4 Nov 2022 15:48:09 +0100 Subject: [PATCH 3/6] heap: add check for usage of flash content from iram this commits: - adds build-time test to check that no call to flash regions are done from IRAM functions - resolves problems related to IRAM function using content in flash memory - update heap_caps_alloc_failed to use a default function name in DRAM when necessary instead of creating a function name variable in DRAM for each call of heap_caps_alloc_failed. This allows to save some extra bytes in RAM. --- components/heap/CMakeLists.txt | 2 +- components/heap/heap_caps.c | 12 +++++++++--- components/heap/test_apps/CMakeLists.txt | 13 +++++++++++++ components/heap/test_apps/sdkconfig.defaults | 1 + 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/components/heap/CMakeLists.txt b/components/heap/CMakeLists.txt index 61cb81219a3..f0a691f4933 100644 --- a/components/heap/CMakeLists.txt +++ b/components/heap/CMakeLists.txt @@ -37,7 +37,7 @@ idf_component_register(SRCS "${srcs}" INCLUDE_DIRS ${includes} PRIV_INCLUDE_DIRS ${priv_includes} LDFRAGMENTS linker.lf - PRIV_REQUIRES soc) + PRIV_REQUIRES soc spi_flash) if(CONFIG_HEAP_TRACING) set(WRAP_FUNCTIONS diff --git a/components/heap/heap_caps.c b/components/heap/heap_caps.c index 9c35b56a810..50fe13763d0 100644 --- a/components/heap/heap_caps.c +++ b/components/heap/heap_caps.c @@ -14,7 +14,7 @@ #include "esp_log.h" #include "heap_private.h" #include "esp_system.h" - +#include "esp_private/cache_utils.h" /* Forward declaration for base function, put in IRAM. * These functions don't check for errors after trying to allocate memory. */ @@ -56,10 +56,16 @@ IRAM_ATTR static void *dram_alloc_to_iram_addr(void *addr, size_t len) } -static void heap_caps_alloc_failed(size_t requested_size, uint32_t caps, const char *function_name) +IRAM_ATTR static void heap_caps_alloc_failed(size_t requested_size, uint32_t caps, const char *function_name) { + static const DRAM_ATTR char *default_func_name = ""; if (alloc_failed_callback) { - alloc_failed_callback(requested_size, caps, function_name); + if (!spi_flash_cache_enabled() && !esp_ptr_internal(function_name)) { + alloc_failed_callback(requested_size, caps, default_func_name); + } + else { + alloc_failed_callback(requested_size, caps, function_name); + } } #ifdef CONFIG_HEAP_ABORT_WHEN_ALLOCATION_FAILS diff --git a/components/heap/test_apps/CMakeLists.txt b/components/heap/test_apps/CMakeLists.txt index df2d30eeb0c..5af129f397a 100644 --- a/components/heap/test_apps/CMakeLists.txt +++ b/components/heap/test_apps/CMakeLists.txt @@ -4,3 +4,16 @@ cmake_minimum_required(VERSION 3.16) include($ENV{IDF_PATH}/tools/cmake/project.cmake) project(test_heap) + +if(CONFIG_COMPILER_DUMP_RTL_FILES) + add_custom_target(check_test_app_sections ALL + COMMAND ${PYTHON} $ENV{IDF_PATH}/tools/ci/check_callgraph.py + --rtl-dir ${CMAKE_BINARY_DIR}/esp-idf/heap/ + --elf-file ${CMAKE_BINARY_DIR}/test_heap.elf + find-refs + --from-sections=.iram0.text + --to-sections=.flash.text,.flash.rodata + --exit-code + DEPENDS ${elf} + ) +endif() diff --git a/components/heap/test_apps/sdkconfig.defaults b/components/heap/test_apps/sdkconfig.defaults index 0e8a2c2557c..2fbcda0f36d 100644 --- a/components/heap/test_apps/sdkconfig.defaults +++ b/components/heap/test_apps/sdkconfig.defaults @@ -1,2 +1,3 @@ +CONFIG_COMPILER_DUMP_RTL_FILES=y CONFIG_ESP_TASK_WDT_CHECK_IDLE_TASK_CPU0=n CONFIG_ESP_SYSTEM_MEMPROT_FEATURE=n # memory protection needs to be disabled for certain tests From ab1bd08ab9530cb19c1db5153ba596ad16962c82 Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Wed, 9 Nov 2022 10:46:18 +0100 Subject: [PATCH 4/6] tools: update list of references to not include symbold used by __assert_func calls On xtensa architecture, the call to __assert_func uses a reference to __func__ that can sometimes be placed in flash. Since the __asert_func can be called from functions in IRAM the check_callgraph script can report an error when checking for invalid calls from IRAM to flash sections. However, the __asert_func prevents this scenario at runtime so the check_callgraph script reports a 'flas positive' situation. For this reasson, all references to __func__$x found prior to a call to __assert_func are droped in the parsing of the rtl files. --- components/heap/test_apps/CMakeLists.txt | 1 + tools/ci/check_callgraph.py | 32 +++++++++++++++++++++--- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/components/heap/test_apps/CMakeLists.txt b/components/heap/test_apps/CMakeLists.txt index 5af129f397a..b6174bb30a1 100644 --- a/components/heap/test_apps/CMakeLists.txt +++ b/components/heap/test_apps/CMakeLists.txt @@ -13,6 +13,7 @@ if(CONFIG_COMPILER_DUMP_RTL_FILES) find-refs --from-sections=.iram0.text --to-sections=.flash.text,.flash.rodata + --ignore-symbols=__func__/__assert_func,__func__/heap_caps_alloc_failed --exit-code DEPENDS ${elf} ) diff --git a/tools/ci/check_callgraph.py b/tools/ci/check_callgraph.py index 08b89cc2c2f..44b478c37fc 100755 --- a/tools/ci/check_callgraph.py +++ b/tools/ci/check_callgraph.py @@ -95,6 +95,11 @@ def __str__(self) -> str: ) +class IgnorePair(): + def __init__(self, pair: str) -> None: + self.symbol, self.function_call = pair.split('/') + + class ElfInfo(object): def __init__(self, elf_file: BinaryIO) -> None: self.elf_file = elf_file @@ -159,7 +164,7 @@ def section_for_addr(self, sym_addr: int) -> Optional[str]: return None -def load_rtl_file(rtl_filename: str, tu_filename: str, functions: List[RtlFunction]) -> None: +def load_rtl_file(rtl_filename: str, tu_filename: str, functions: List[RtlFunction], ignore_pairs: List[IgnorePair]) -> None: last_function: Optional[RtlFunction] = None for line in open(rtl_filename): # Find function definition @@ -175,6 +180,17 @@ def load_rtl_file(rtl_filename: str, tu_filename: str, functions: List[RtlFuncti match = re.match(CALL_REGEX, line) if match: target = match.group('target') + + # if target matches on of the IgnorePair function_call attributes, remove + # the last occurrence of the associated symbol from the last_function.refs list. + call_matching_pairs = [pair for pair in ignore_pairs if pair.function_call == target] + if call_matching_pairs and last_function and last_function.refs: + for pair in call_matching_pairs: + ignored_symbols = [ref for ref in last_function.refs if pair.symbol in ref] + if ignored_symbols: + last_ref = ignored_symbols.pop() + last_function.refs = [ref for ref in last_function.refs if last_ref != ref] + if target not in last_function.calls: last_function.calls.append(target) continue @@ -304,12 +320,12 @@ def match_rtl_funcs_to_symbols(rtl_functions: List[RtlFunction], elfinfo: ElfInf return symbols, refs -def get_symbols_and_refs(rtl_list: List[str], elf_file: BinaryIO) -> Tuple[List[Symbol], List[Reference]]: +def get_symbols_and_refs(rtl_list: List[str], elf_file: BinaryIO, ignore_pairs: List[IgnorePair]) -> Tuple[List[Symbol], List[Reference]]: elfinfo = ElfInfo(elf_file) rtl_functions: List[RtlFunction] = [] for file_name in rtl_list: - load_rtl_file(file_name, file_name, rtl_functions) + load_rtl_file(file_name, file_name, rtl_functions, ignore_pairs) return match_rtl_funcs_to_symbols(rtl_functions, elfinfo) @@ -361,6 +377,10 @@ def main() -> None: find_refs_parser.add_argument( '--to-sections', help='comma-separated list of target sections' ) + find_refs_parser.add_argument( + '--ignore-symbols', help='comma-separated list of symbol/function_name pairs. \ + This will force the parser to ignore the symbol preceding the call to function_name' + ) find_refs_parser.add_argument( '--exit-code', action='store_true', @@ -384,7 +404,11 @@ def main() -> None: if not rtl_list: raise RuntimeError('No RTL files specified') - _, refs = get_symbols_and_refs(rtl_list, args.elf_file) + ignore_pairs = [] + for pair in args.ignore_symbols.split(',') if args.ignore_symbols else []: + ignore_pairs.append(IgnorePair(pair)) + + _, refs = get_symbols_and_refs(rtl_list, args.elf_file, ignore_pairs) if args.action == 'find-refs': from_sections = args.from_sections.split(',') if args.from_sections else [] From e31e6d4de8acbd49ab98dc510449703d21670df5 Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Fri, 11 Nov 2022 08:44:07 +0100 Subject: [PATCH 5/6] feat: remove tlsf_fls and tlsf_ffs from linker as they are inlined. --- components/heap/linker.lf | 2 -- components/heap/tlsf | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/components/heap/linker.lf b/components/heap/linker.lf index 2d68361ad96..4f15783695a 100644 --- a/components/heap/linker.lf +++ b/components/heap/linker.lf @@ -2,8 +2,6 @@ archive: libheap.a entries: if HEAP_TLSF_USE_ROM_IMPL = n: - tlsf:tlsf_ffs (noflash) - tlsf:tlsf_fls (noflash) tlsf:tlsf_block_size (noflash) tlsf:tlsf_size (noflash) tlsf:tlsf_align_size (noflash) diff --git a/components/heap/tlsf b/components/heap/tlsf index 13da0fff7f5..8c9cd0517ad 160000 --- a/components/heap/tlsf +++ b/components/heap/tlsf @@ -1 +1 @@ -Subproject commit 13da0fff7f54623b20785dbeb466df420f52e510 +Subproject commit 8c9cd0517adf99e363812e9a295dfe3898fdd345 From b496bab0f609201075d9ebb7a568c7f1040e68f5 Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Mon, 14 Nov 2022 08:21:33 +0100 Subject: [PATCH 6/6] heap: fix linker issues and remove spi flash dependencies --- components/heap/CMakeLists.txt | 2 +- components/heap/heap_caps.c | 11 ++--------- components/heap/linker.lf | 10 ++++++---- components/heap/multi_heap.c | 10 +++++----- components/heap/multi_heap_internal.h | 8 ++++++++ components/heap/multi_heap_poisoning.c | 14 ++++++++++---- 6 files changed, 32 insertions(+), 23 deletions(-) diff --git a/components/heap/CMakeLists.txt b/components/heap/CMakeLists.txt index f0a691f4933..61cb81219a3 100644 --- a/components/heap/CMakeLists.txt +++ b/components/heap/CMakeLists.txt @@ -37,7 +37,7 @@ idf_component_register(SRCS "${srcs}" INCLUDE_DIRS ${includes} PRIV_INCLUDE_DIRS ${priv_includes} LDFRAGMENTS linker.lf - PRIV_REQUIRES soc spi_flash) + PRIV_REQUIRES soc) if(CONFIG_HEAP_TRACING) set(WRAP_FUNCTIONS diff --git a/components/heap/heap_caps.c b/components/heap/heap_caps.c index 50fe13763d0..e28653f69c8 100644 --- a/components/heap/heap_caps.c +++ b/components/heap/heap_caps.c @@ -14,7 +14,6 @@ #include "esp_log.h" #include "heap_private.h" #include "esp_system.h" -#include "esp_private/cache_utils.h" /* Forward declaration for base function, put in IRAM. * These functions don't check for errors after trying to allocate memory. */ @@ -56,16 +55,10 @@ IRAM_ATTR static void *dram_alloc_to_iram_addr(void *addr, size_t len) } -IRAM_ATTR static void heap_caps_alloc_failed(size_t requested_size, uint32_t caps, const char *function_name) +IRAM_ATTR NOINLINE_ATTR static void heap_caps_alloc_failed(size_t requested_size, uint32_t caps, const char *function_name) { - static const DRAM_ATTR char *default_func_name = ""; if (alloc_failed_callback) { - if (!spi_flash_cache_enabled() && !esp_ptr_internal(function_name)) { - alloc_failed_callback(requested_size, caps, default_func_name); - } - else { - alloc_failed_callback(requested_size, caps, function_name); - } + alloc_failed_callback(requested_size, caps, function_name); } #ifdef CONFIG_HEAP_ABORT_WHEN_ALLOCATION_FAILS diff --git a/components/heap/linker.lf b/components/heap/linker.lf index 4f15783695a..9f04259780a 100644 --- a/components/heap/linker.lf +++ b/components/heap/linker.lf @@ -15,7 +15,6 @@ entries: tlsf:tlsf_free (noflash) tlsf:tlsf_realloc (noflash) - multi_heap:assert_valid_block (noflash) multi_heap:multi_heap_get_block_address_impl (noflash) multi_heap:multi_heap_get_allocated_size_impl (noflash) multi_heap:multi_heap_set_lock (noflash) @@ -27,6 +26,9 @@ entries: multi_heap:multi_heap_realloc_impl (noflash) multi_heap:multi_heap_aligned_alloc_impl_offs (noflash) multi_heap:multi_heap_aligned_alloc_impl (noflash) + multi_heap:multi_heap_internal_lock (noflash) + multi_heap:multi_heap_internal_unlock (noflash) + multi_heap:assert_valid_block (noflash) if HEAP_TLSF_USE_ROM_IMPL = y: multi_heap:_multi_heap_lock (noflash) @@ -47,6 +49,6 @@ entries: multi_heap_poisoning:multi_heap_internal_check_block_poisoning (noflash) multi_heap_poisoning:multi_heap_internal_poison_fill_region (noflash) - if HEAP_POISONING_COMPREHENSIVE = y: - multi_heap_poisoning:verify_fill_pattern (noflash) - multi_heap_poisoning:block_absorb_post_hook (noflash) + if HEAP_POISONING_COMPREHENSIVE = y: + multi_heap_poisoning:verify_fill_pattern (noflash) + multi_heap_poisoning:block_absorb_post_hook (noflash) diff --git a/components/heap/multi_heap.c b/components/heap/multi_heap.c index aa14707631f..05477806d42 100644 --- a/components/heap/multi_heap.c +++ b/components/heap/multi_heap.c @@ -106,7 +106,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. */ -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 block_header_t *block) { pool_t pool = tlsf_get_pool(heap->heap_data); void *ptr = block_to_ptr(block); @@ -157,12 +157,12 @@ void multi_heap_set_lock(multi_heap_handle_t heap, void *lock) heap->lock = lock; } -void inline multi_heap_internal_lock(multi_heap_handle_t heap) +void multi_heap_internal_lock(multi_heap_handle_t heap) { MULTI_HEAP_LOCK(heap->lock); } -void inline multi_heap_internal_unlock(multi_heap_handle_t heap) +void multi_heap_internal_unlock(multi_heap_handle_t heap) { MULTI_HEAP_UNLOCK(heap->lock); } @@ -351,7 +351,7 @@ bool multi_heap_check(multi_heap_handle_t heap, bool print_errors) return valid; } -static void multi_heap_dump_tlsf(void* ptr, size_t size, int used, void* user) +__attribute__((noinline)) static void multi_heap_dump_tlsf(void* ptr, size_t size, int used, void* user) { (void)user; MULTI_HEAP_STDERR_PRINTF("Block %p data, size: %d bytes, Free: %s \n", @@ -388,7 +388,7 @@ size_t multi_heap_minimum_free_size_impl(multi_heap_handle_t heap) return heap->minimum_free_bytes; } -static void multi_heap_get_info_tlsf(void* ptr, size_t size, int used, void* user) +__attribute__((noinline)) static void multi_heap_get_info_tlsf(void* ptr, size_t size, int used, void* user) { multi_heap_info_t *info = user; diff --git a/components/heap/multi_heap_internal.h b/components/heap/multi_heap_internal.h index 00c1f23e917..b34c3393c61 100644 --- a/components/heap/multi_heap_internal.h +++ b/components/heap/multi_heap_internal.h @@ -5,6 +5,14 @@ */ #pragma once +/* Define a noclone attribute when compiled with GCC as certain functions + * in the heap component should not be cloned by the compiler */ +#if defined __has_attribute && __has_attribute(noclone) +#define NOCLONE_ATTR __attribute((noclone)) +#else +#define NOCLONE_ATTR +#endif + /* Define a structure that contains some function pointers that point to OS-related functions. An instance of this structure will be provided to the heap in ROM for use if needed. */ diff --git a/components/heap/multi_heap_poisoning.c b/components/heap/multi_heap_poisoning.c index 70809f38dcc..1e3afcf0fb3 100644 --- a/components/heap/multi_heap_poisoning.c +++ b/components/heap/multi_heap_poisoning.c @@ -66,7 +66,7 @@ typedef struct { Returns the pointer to the actual usable data buffer (ie after 'head') */ -static uint8_t *poison_allocated_region(poison_head_t *head, size_t alloc_size) +__attribute__((noinline)) static uint8_t *poison_allocated_region(poison_head_t *head, size_t alloc_size) { uint8_t *data = (uint8_t *)(&head[1]); /* start of data ie 'real' allocated buffer */ poison_tail_t *tail = (poison_tail_t *)(data + alloc_size); @@ -90,7 +90,7 @@ static uint8_t *poison_allocated_region(poison_head_t *head, size_t alloc_size) Returns a pointer to the poison header structure, or NULL if the poison structures are corrupt. */ -static poison_head_t *verify_allocated_region(void *data, bool print_errors) +__attribute__((noinline)) static poison_head_t *verify_allocated_region(void *data, bool print_errors) { poison_head_t *head = (poison_head_t *)((intptr_t)data - sizeof(poison_head_t)); poison_tail_t *tail = (poison_tail_t *)((intptr_t)data + head->alloc_size); @@ -132,8 +132,12 @@ static poison_head_t *verify_allocated_region(void *data, bool print_errors) if swap_pattern is true, swap patterns in the buffer (ie replace MALLOC_FILL_PATTERN with FREE_FILL_PATTERN, and vice versa.) Returns true if verification checks out. + + This function has the attribute noclone to prevent the compiler to create a clone on flash where expect_free is removed (as this + function is called only with expect_free == true throughout the component). */ -static bool verify_fill_pattern(void *data, size_t size, bool print_errors, bool expect_free, bool swap_pattern) +__attribute__((noinline)) NOCLONE_ATTR +static bool verify_fill_pattern(void *data, size_t size, const bool print_errors, const bool expect_free, bool swap_pattern) { const uint32_t FREE_FILL_WORD = (FREE_FILL_PATTERN << 24) | (FREE_FILL_PATTERN << 16) | (FREE_FILL_PATTERN << 8) | FREE_FILL_PATTERN; const uint32_t MALLOC_FILL_WORD = (MALLOC_FILL_PATTERN << 24) | (MALLOC_FILL_PATTERN << 16) | (MALLOC_FILL_PATTERN << 8) | MALLOC_FILL_PATTERN; @@ -259,7 +263,9 @@ void *multi_heap_malloc(multi_heap_handle_t heap, size_t size) return data; } -void multi_heap_free(multi_heap_handle_t heap, void *p) +/* This function has the noclone attribute to prevent the compiler to optimize out the + * check for p == NULL and create a clone function placed in flash. */ +NOCLONE_ATTR void multi_heap_free(multi_heap_handle_t heap, void *p) { if (p == NULL) { return;