From 05575cc246f5564d4c681677375f12796f75c634 Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Wed, 8 Jun 2022 14:33:11 +0800 Subject: [PATCH 1/3] Heap: heap_caps_*_prefer functions now properly call alloc_failed callback heap_caps_*_prefer functions will now only call heaps_caps_alloc_failed callback if all attempts to allocation memory fail (and not after each attempt anymore). * Closes https://github.com/espressif/esp-idf/issues/9086 --- components/heap/heap_caps.c | 54 +++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/components/heap/heap_caps.c b/components/heap/heap_caps.c index 962fcd1a90..016ea520b2 100644 --- a/components/heap/heap_caps.c +++ b/components/heap/heap_caps.c @@ -24,8 +24,11 @@ #include "esp_system.h" -// forward declaration -IRAM_ATTR static void *heap_caps_realloc_base( void *ptr, size_t size, uint32_t caps); +/* Forward declaration for base function, put in IRAM. + * These functions don't check for errors after trying to allocate memory. */ +static void *heap_caps_realloc_base( void *ptr, size_t size, uint32_t caps ); +static void *heap_caps_calloc_base( size_t n, size_t size, uint32_t caps ); +static void *heap_caps_malloc_base( size_t size, uint32_t caps ); /* This file, combined with a region allocator that supports multiple heaps, solves the problem that the ESP32 has RAM @@ -256,13 +259,17 @@ IRAM_ATTR void *heap_caps_malloc_prefer( size_t size, size_t num, ... ) va_list argp; va_start( argp, num ); void *r = NULL; + uint32_t caps = MALLOC_CAP_DEFAULT; while (num--) { - uint32_t caps = va_arg( argp, uint32_t ); - r = heap_caps_malloc( size, caps ); + caps = va_arg( argp, uint32_t ); + r = heap_caps_malloc_base( size, caps ); if (r != NULL) { break; } } + if (r == NULL){ + heap_caps_alloc_failed(size, caps, __func__); + } va_end( argp ); return r; } @@ -275,13 +282,17 @@ IRAM_ATTR void *heap_caps_realloc_prefer( void *ptr, size_t size, size_t num, .. va_list argp; va_start( argp, num ); void *r = NULL; + uint32_t caps = MALLOC_CAP_DEFAULT; while (num--) { - uint32_t caps = va_arg( argp, uint32_t ); - r = heap_caps_realloc( ptr, size, caps ); + caps = va_arg( argp, uint32_t ); + r = heap_caps_realloc_base( ptr, size, caps ); if (r != NULL || size == 0) { break; } } + if (r == NULL){ + heap_caps_alloc_failed(size, caps, __func__); + } va_end( argp ); return r; } @@ -294,10 +305,16 @@ IRAM_ATTR void *heap_caps_calloc_prefer( size_t n, size_t size, size_t num, ... va_list argp; va_start( argp, num ); void *r = NULL; + uint32_t caps = MALLOC_CAP_DEFAULT; while (num--) { - uint32_t caps = va_arg( argp, uint32_t ); - r = heap_caps_calloc( n, size, caps ); - if (r != NULL) break; + caps = va_arg( argp, uint32_t ); + r = heap_caps_calloc_base( n, size, caps ); + if (r != NULL){ + break; + } + } + if (r == NULL){ + heap_caps_alloc_failed(size, caps, __func__); } va_end( argp ); return r; @@ -429,7 +446,11 @@ IRAM_ATTR void *heap_caps_realloc( void *ptr, size_t size, uint32_t caps) return ptr; } -IRAM_ATTR void *heap_caps_calloc( size_t n, size_t size, uint32_t caps) +/* +This function should not be called directly as it does not +check for failure / call heap_caps_alloc_failed() +*/ +IRAM_ATTR static void *heap_caps_calloc_base( size_t n, size_t size, uint32_t caps) { void *result; size_t size_bytes; @@ -438,13 +459,24 @@ IRAM_ATTR void *heap_caps_calloc( size_t n, size_t size, uint32_t caps) return NULL; } - result = heap_caps_malloc(size_bytes, caps); + result = heap_caps_malloc_base(size_bytes, caps); if (result != NULL) { bzero(result, size_bytes); } return result; } +IRAM_ATTR void *heap_caps_calloc( size_t n, size_t size, uint32_t caps) +{ + void* ptr = heap_caps_calloc_base(n, size, caps); + + if (!ptr){ + heap_caps_alloc_failed(size, caps, __func__); + } + + return ptr; +} + size_t heap_caps_get_total_size(uint32_t caps) { size_t total_size = 0; From b43e777e0a25fecd4652d5282427366fadae8b1d Mon Sep 17 00:00:00 2001 From: tgotic Date: Sun, 7 Aug 2022 16:21:22 +0200 Subject: [PATCH 2/3] fix malloc(0) and heap_caps_alloc_failed() Don't call heap_caps_alloc_failed() for malloc(0) and calloc(0), because it is not an error. Improve handling of malloc(0) and calloc(0). Merges https://github.com/espressif/esp-idf/pull/9517 --- components/heap/heap_caps.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/components/heap/heap_caps.c b/components/heap/heap_caps.c index 016ea520b2..19e45cfa6e 100644 --- a/components/heap/heap_caps.c +++ b/components/heap/heap_caps.c @@ -70,9 +70,9 @@ static void heap_caps_alloc_failed(size_t requested_size, uint32_t caps, const c alloc_failed_callback(requested_size, caps, function_name); } - #ifdef CONFIG_HEAP_ABORT_WHEN_ALLOCATION_FAILS +#ifdef CONFIG_HEAP_ABORT_WHEN_ALLOCATION_FAILS esp_system_abort("Memory allocation failed"); - #endif +#endif } esp_err_t heap_caps_register_failed_alloc_callback(esp_alloc_failed_hook_t callback) @@ -100,6 +100,10 @@ IRAM_ATTR static void *heap_caps_malloc_base( size_t size, uint32_t caps) { void *ret = NULL; + if (size == 0) { + return NULL; + } + if (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 @@ -169,7 +173,7 @@ IRAM_ATTR void *heap_caps_malloc( size_t size, uint32_t caps){ void* ptr = heap_caps_malloc_base(size, caps); - if (!ptr){ + if (!ptr && size > 0){ heap_caps_alloc_failed(size, caps, __func__); } @@ -204,13 +208,13 @@ IRAM_ATTR void *heap_caps_malloc_default( size_t size ) } else { r=heap_caps_malloc_base( size, MALLOC_CAP_DEFAULT | MALLOC_CAP_SPIRAM ); } - if (r==NULL) { + if (r==NULL && size > 0) { //try again while being less picky r=heap_caps_malloc_base( size, MALLOC_CAP_DEFAULT ); } // allocation failure? - if (r==NULL){ + if (r==NULL && size > 0){ heap_caps_alloc_failed(size, MALLOC_CAP_DEFAULT, __func__); } @@ -267,7 +271,7 @@ IRAM_ATTR void *heap_caps_malloc_prefer( size_t size, size_t num, ... ) break; } } - if (r == NULL){ + if (r == NULL && size > 0){ heap_caps_alloc_failed(size, caps, __func__); } va_end( argp ); @@ -290,7 +294,7 @@ IRAM_ATTR void *heap_caps_realloc_prefer( void *ptr, size_t size, size_t num, .. break; } } - if (r == NULL){ + if (r == NULL && size > 0){ heap_caps_alloc_failed(size, caps, __func__); } va_end( argp ); @@ -309,11 +313,11 @@ IRAM_ATTR void *heap_caps_calloc_prefer( size_t n, size_t size, size_t num, ... while (num--) { caps = va_arg( argp, uint32_t ); r = heap_caps_calloc_base( n, size, caps ); - if (r != NULL){ + if (r != NULL || size == 0){ break; } } - if (r == NULL){ + if (r == NULL && size > 0){ heap_caps_alloc_failed(size, caps, __func__); } va_end( argp ); @@ -470,7 +474,7 @@ IRAM_ATTR void *heap_caps_calloc( size_t n, size_t size, uint32_t caps) { void* ptr = heap_caps_calloc_base(n, size, caps); - if (!ptr){ + if (!ptr && size > 0){ heap_caps_alloc_failed(size, caps, __func__); } @@ -631,6 +635,10 @@ IRAM_ATTR void *heap_caps_aligned_alloc(size_t alignment, size_t size, uint32_t return NULL; } + if (size == 0) { + return NULL; + } + if (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 From 6906c3431988978802ccfe2daef093a3ba31e9a8 Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Mon, 8 Aug 2022 15:39:25 +0800 Subject: [PATCH 3/3] heap: add a unit test for malloc(0) and slightly optimize heap_caps_malloc_prefer --- components/heap/heap_caps.c | 2 +- components/heap/test/test_malloc.c | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/components/heap/heap_caps.c b/components/heap/heap_caps.c index 19e45cfa6e..f3ac9460ee 100644 --- a/components/heap/heap_caps.c +++ b/components/heap/heap_caps.c @@ -267,7 +267,7 @@ IRAM_ATTR void *heap_caps_malloc_prefer( size_t size, size_t num, ... ) while (num--) { caps = va_arg( argp, uint32_t ); r = heap_caps_malloc_base( size, caps ); - if (r != NULL) { + if (r != NULL || size == 0) { break; } } diff --git a/components/heap/test/test_malloc.c b/components/heap/test/test_malloc.c index 6f4bbd8728..995a64da33 100644 --- a/components/heap/test/test_malloc.c +++ b/components/heap/test/test_malloc.c @@ -132,3 +132,25 @@ TEST_CASE("malloc(0) should return a NULL pointer", "[heap]") p = malloc(0); TEST_ASSERT(p == NULL); } + +static bool failure_occured = false; + +static void test_alloc_failure_callback(size_t size, uint32_t caps, const char * function_name) +{ + failure_occured = true; +} + +TEST_CASE("malloc/calloc(0) should not call failure callback", "[heap]") +{ + void* ptr = NULL; + esp_err_t ret = heap_caps_register_failed_alloc_callback(test_alloc_failure_callback); + TEST_ASSERT(ret == ESP_OK); + ptr = malloc(0); + TEST_ASSERT_NULL(ptr); + /* Check that our callback was NOT called */ + TEST_ASSERT_FALSE(failure_occured); + /* Do the same thing for calloc */ + ptr = calloc(0, 0); + TEST_ASSERT_NULL(ptr); + TEST_ASSERT_FALSE(failure_occured); +}