fix(heap): Fix bugs in heap task tracking

Update task tracking feature to fix bugs introduced when
decoupling task tracking from heap poisoning.

Closes https://github.com/espressif/esp-idf/issues/12498
Closes https://github.com/espressif/esp-idf/issues/12493
This commit is contained in:
Guillaume Souchere 2023-10-27 15:06:58 +02:00 committed by BOT
parent 7bbe4eae46
commit 4824325fe4
6 changed files with 62 additions and 13 deletions

View File

@ -122,7 +122,9 @@ HEAP_IRAM_ATTR static void *heap_caps_malloc_base( size_t size, uint32_t caps)
{
void *ret = NULL;
if (size == 0 || MULTI_HEAP_ADD_BLOCK_OWNER_SIZE(size) > HEAP_SIZE_MAX ) {
// remove block owner size to HEAP_SIZE_MAX rather than adding the block owner size
// to size to prevent overflows.
if (size == 0 || size > MULTI_HEAP_REMOVE_BLOCK_OWNER_SIZE(HEAP_SIZE_MAX) ) {
// Avoids int overflow when adding small numbers to size, or
// calculating 'end' from start+size, by limiting 'size' to the possible range
return NULL;
@ -412,7 +414,9 @@ HEAP_IRAM_ATTR static void *heap_caps_realloc_base( void *ptr, size_t size, uint
return NULL;
}
if (MULTI_HEAP_ADD_BLOCK_OWNER_SIZE(size) > HEAP_SIZE_MAX) {
// remove block owner size to HEAP_SIZE_MAX rather than adding the block owner size
// to size to prevent overflows.
if (size > MULTI_HEAP_REMOVE_BLOCK_OWNER_SIZE(HEAP_SIZE_MAX)) {
return NULL;
}
@ -421,6 +425,7 @@ HEAP_IRAM_ATTR static void *heap_caps_realloc_base( void *ptr, size_t size, uint
if(esp_ptr_in_diram_iram((void *)ptr)) {
uint32_t *dram_addr = (uint32_t *)ptr;
dram_ptr = (void *)dram_addr[-1];
dram_ptr = MULTI_HEAP_REMOVE_BLOCK_OWNER_OFFSET(dram_ptr);
heap = find_containing_heap(dram_ptr);
assert(heap != NULL && "realloc() pointer is outside heap areas");
@ -435,6 +440,12 @@ HEAP_IRAM_ATTR static void *heap_caps_realloc_base( void *ptr, size_t size, uint
assert(heap != NULL && "realloc() pointer is outside heap areas");
}
// shift ptr by block owner offset. Since the ptr returned to the user
// does not include the block owner bytes (that are located at the
// beginning of the allocated memory) we have to add them back before
// processing the realloc.
ptr = MULTI_HEAP_REMOVE_BLOCK_OWNER_OFFSET(ptr);
// are the existing heap's capabilities compatible with the
// requested ones?
bool compatible_caps = (caps & get_all_caps(heap)) == caps;
@ -466,8 +477,10 @@ HEAP_IRAM_ATTR static void *heap_caps_realloc_base( void *ptr, size_t size, uint
}
assert(old_size > 0);
memcpy(new_p, ptr, MIN(size, old_size));
heap_caps_free(ptr);
// do not copy the block owner bytes
memcpy(new_p, MULTI_HEAP_ADD_BLOCK_OWNER_OFFSET(ptr), MIN(size, old_size));
// add the block owner bytes to ptr since they are removed in heap_caps_free
heap_caps_free(MULTI_HEAP_ADD_BLOCK_OWNER_OFFSET(ptr));
return new_p;
}
@ -571,11 +584,13 @@ void heap_caps_get_info( multi_heap_info_t *info, uint32_t caps )
multi_heap_info_t hinfo;
multi_heap_get_info(heap->heap, &hinfo);
info->total_free_bytes += hinfo.total_free_bytes;
info->total_allocated_bytes += hinfo.total_allocated_bytes;
info->total_free_bytes += hinfo.total_free_bytes - MULTI_HEAP_ADD_BLOCK_OWNER_SIZE(0);
info->total_allocated_bytes += (hinfo.total_allocated_bytes -
hinfo.allocated_blocks * MULTI_HEAP_ADD_BLOCK_OWNER_SIZE(0));
info->largest_free_block = MAX(info->largest_free_block,
hinfo.largest_free_block);
info->minimum_free_bytes += hinfo.minimum_free_bytes;
info->largest_free_block -= info->largest_free_block ? MULTI_HEAP_ADD_BLOCK_OWNER_SIZE(0) : 0;
info->minimum_free_bytes += hinfo.minimum_free_bytes - MULTI_HEAP_ADD_BLOCK_OWNER_SIZE(0);
info->allocated_blocks += hinfo.allocated_blocks;
info->free_blocks += hinfo.free_blocks;
info->total_blocks += hinfo.total_blocks;
@ -654,6 +669,9 @@ void heap_caps_dump_all(void)
size_t heap_caps_get_allocated_size( void *ptr )
{
// add the block owner bytes back to ptr before handing over
// to multi heap layer.
ptr = MULTI_HEAP_REMOVE_BLOCK_OWNER_OFFSET(ptr);
heap_t *heap = find_containing_heap(ptr);
assert(heap);
size_t size = multi_heap_get_allocated_size(heap->heap, ptr);
@ -696,8 +714,9 @@ HEAP_IRAM_ATTR void *heap_caps_aligned_alloc(size_t alignment, size_t size, uint
//Heap has at least one of the caps requested. If caps has other bits set that this prio
//doesn't cover, see if they're available in other prios.
if ((get_all_caps(heap) & caps) == caps) {
//Just try to alloc, nothing special.
ret = multi_heap_aligned_alloc(heap->heap, MULTI_HEAP_ADD_BLOCK_OWNER_SIZE(size), alignment);
// Just try to alloc, nothing special. Provide the size of the block owner
// as an offset to prevent a miscalculation of the alignment.
ret = multi_heap_aligned_alloc_offs(heap->heap, MULTI_HEAP_ADD_BLOCK_OWNER_SIZE(size), alignment, MULTI_HEAP_BLOCK_OWNER_SIZE());
if (ret != NULL) {
MULTI_HEAP_SET_BLOCK_OWNER(ret);
ret = MULTI_HEAP_ADD_BLOCK_OWNER_OFFSET(ret);

View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -179,6 +179,17 @@ typedef struct {
*/
void multi_heap_get_info(multi_heap_handle_t heap, multi_heap_info_t *info);
/**
* @brief Perform an aligned allocation from the provided offset
*
* @param heap The heap in which to perform the allocation
* @param size The size of the allocation
* @param alignment How the memory must be aligned
* @param offset The offset at which the alignment should start
* @return void* The ptr to the allocated memory
*/
void *multi_heap_aligned_alloc_offs(multi_heap_handle_t heap, size_t size, size_t alignment, size_t offset);
#ifdef __cplusplus
}
#endif

View File

@ -48,6 +48,9 @@ entries:
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)
multi_heap_poisoning:multi_heap_aligned_alloc_offs (noflash)
else:
multi_heap:multi_heap_aligned_alloc_offs (noflash)
if HEAP_POISONING_COMPREHENSIVE = y:
multi_heap_poisoning:verify_fill_pattern (noflash)

View File

@ -26,7 +26,14 @@
/* Defines compile-time configuration macros */
#include "multi_heap_config.h"
#if (!defined MULTI_HEAP_POISONING) && (!defined CONFIG_HEAP_TLSF_USE_ROM_IMPL)
#if (!defined MULTI_HEAP_POISONING)
void *multi_heap_aligned_alloc_offs(multi_heap_handle_t heap, size_t size, size_t alignment, size_t offset)
{
return multi_heap_aligned_alloc_impl_offs(heap, size, alignment, offset);
}
#if (!defined CONFIG_HEAP_TLSF_USE_ROM_IMPL)
/* if no heap poisoning, public API aliases directly to these implementations */
void *multi_heap_malloc(multi_heap_handle_t heap, size_t size)
__attribute__((alias("multi_heap_malloc_impl")));
@ -60,7 +67,9 @@ size_t multi_heap_minimum_free_size(multi_heap_handle_t heap)
void *multi_heap_get_block_address(multi_heap_block_handle_t block)
__attribute__((alias("multi_heap_get_block_address_impl")));
#endif
#endif // !CONFIG_HEAP_TLSF_USE_ROM_IMPL
#endif // !MULTI_HEAP_POISONING
#define ALIGN(X) ((X) & ~(sizeof(void *)-1))
#define ALIGN_UP(X) ALIGN((X)+sizeof(void *)-1)

View File

@ -72,6 +72,7 @@ inline static void multi_heap_assert(bool condition, const char *format, int lin
#define MULTI_HEAP_REMOVE_BLOCK_OWNER_OFFSET(HEAD) ((TaskHandle_t*)(HEAD) - 1)
#define MULTI_HEAP_ADD_BLOCK_OWNER_SIZE(SIZE) ((SIZE) + sizeof(TaskHandle_t))
#define MULTI_HEAP_REMOVE_BLOCK_OWNER_SIZE(SIZE) ((SIZE) - sizeof(TaskHandle_t))
#define MULTI_HEAP_BLOCK_OWNER_SIZE() sizeof(TaskHandle_t)
#else
#define MULTI_HEAP_SET_BLOCK_OWNER(HEAD)
#define MULTI_HEAP_GET_BLOCK_OWNER(HEAD) (NULL)
@ -79,6 +80,7 @@ inline static void multi_heap_assert(bool condition, const char *format, int lin
#define MULTI_HEAP_REMOVE_BLOCK_OWNER_OFFSET(HEAD) (HEAD)
#define MULTI_HEAP_ADD_BLOCK_OWNER_SIZE(SIZE) (SIZE)
#define MULTI_HEAP_REMOVE_BLOCK_OWNER_SIZE(SIZE) (SIZE)
#define MULTI_HEAP_BLOCK_OWNER_SIZE() 0
#endif // CONFIG_HEAP_TASK_TRACKING
#else // MULTI_HEAP_FREERTOS

View File

@ -205,6 +205,11 @@ void block_absorb_post_hook(void *start, size_t size, bool is_free)
#endif
void *multi_heap_aligned_alloc(multi_heap_handle_t heap, size_t size, size_t alignment)
{
return multi_heap_aligned_alloc_offs(heap, size, alignment, 0);
}
void *multi_heap_aligned_alloc_offs(multi_heap_handle_t heap, size_t size, size_t alignment, size_t offset)
{
if (!size) {
return NULL;
@ -216,7 +221,7 @@ void *multi_heap_aligned_alloc(multi_heap_handle_t heap, size_t size, size_t ali
multi_heap_internal_lock(heap);
poison_head_t *head = multi_heap_aligned_alloc_impl_offs(heap, size + POISON_OVERHEAD,
alignment, sizeof(poison_head_t));
alignment, offset + sizeof(poison_head_t));
uint8_t *data = NULL;
if (head != NULL) {
data = poison_allocated_region(head, size);