From d26ddaa644997a8b65dfa52b716e5d9502f69c9e Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Wed, 13 Nov 2019 09:24:08 +0800 Subject: [PATCH 01/10] heap/multi_heap: added initial implementation of aligned alloc function --- components/heap/include/multi_heap.h | 19 +++++++ components/heap/multi_heap_internal.h | 1 + components/heap/multi_heap_poisoning.c | 71 ++++++++++++++++++++++++++ 3 files changed, 91 insertions(+) diff --git a/components/heap/include/multi_heap.h b/components/heap/include/multi_heap.h index dbd0cae864..d07d418023 100644 --- a/components/heap/include/multi_heap.h +++ b/components/heap/include/multi_heap.h @@ -29,6 +29,17 @@ extern "C" { /** @brief Opaque handle to a registered heap */ typedef struct multi_heap_info *multi_heap_handle_t; +/** + * @brief allocate a chunk of memory with specific alignment + * + * @param heap Handle to a registered heap. + * @param size size in bytes of memory chunk + * @param alignment how the memory must be aligned + * + * @return pointer to the memory allocated, NULL on failure + */ +void *multi_heap_aligned_alloc(multi_heap_handle_t heap, size_t size, size_t alignment); + /** @brief malloc() a buffer in a given heap * * Semantics are the same as standard malloc(), only the returned buffer will be allocated in the specified heap. @@ -40,6 +51,14 @@ typedef struct multi_heap_info *multi_heap_handle_t; */ void *multi_heap_malloc(multi_heap_handle_t heap, size_t size); +/** @brief free() a buffer aligned in a given heap. + * + * @param heap Handle to a registered heap. + * @param p NULL, or a pointer previously returned from multi_heap_aligned_alloc() for the same heap. + */ +void multi_heap_aligned_free(multi_heap_handle_t heap, void *p); + + /** @brief free() a buffer in a given heap. * * Semantics are the same as standard free(), only the argument 'p' must be NULL or have been allocated in the specified heap. diff --git a/components/heap/multi_heap_internal.h b/components/heap/multi_heap_internal.h index a69a052590..0f6a520096 100644 --- a/components/heap/multi_heap_internal.h +++ b/components/heap/multi_heap_internal.h @@ -23,6 +23,7 @@ typedef const struct heap_block *multi_heap_block_handle_t; If heap poisoning is enabled, wrapper functions call each of these. */ + void *multi_heap_malloc_impl(multi_heap_handle_t heap, size_t size); void multi_heap_free_impl(multi_heap_handle_t heap, void *p); void *multi_heap_realloc_impl(multi_heap_handle_t heap, void *p, size_t size); diff --git a/components/heap/multi_heap_poisoning.c b/components/heap/multi_heap_poisoning.c index ffe885150d..9b86b5351e 100644 --- a/components/heap/multi_heap_poisoning.c +++ b/components/heap/multi_heap_poisoning.c @@ -45,6 +45,9 @@ #define HEAD_CANARY_PATTERN 0xABBA1234 #define TAIL_CANARY_PATTERN 0xBAAD5678 + +#define ALIGN_UP(num, align) (((num) + ((align) - 1)) & ~((align) - 1)) + typedef struct { uint32_t head_canary; MULTI_HEAP_BLOCK_OWNER @@ -182,6 +185,49 @@ static bool verify_fill_pattern(void *data, size_t size, bool print_errors, bool } #endif +void *multi_heap_aligned_alloc(multi_heap_handle_t heap, size_t size, size_t alignment) +{ + if(heap == NULL) { + return NULL; + } + + if(!size) { + return NULL; + } + + //Alignment must be a power of two... + if((alignment & (alignment - 1)) != 0) { + return NULL; + } + + if(size > SIZE_MAX - POISON_OVERHEAD) { + return NULL; + } + + uint32_t overhead = (sizeof(uint32_t) + (alignment - 1) + POISON_OVERHEAD); + + multi_heap_internal_lock(heap); + poison_head_t *head = multi_heap_malloc_impl(heap, size + overhead); + uint8_t *data = NULL; + if (head != NULL) { + data = poison_allocated_region(head, size); +#ifdef SLOW + /* check everything we got back is FREE_FILL_PATTERN & swap for MALLOC_FILL_PATTERN */ + bool ret = verify_fill_pattern(data, size, true, true, true); + assert( ret ); +#endif + } + + //Lets align our new obtained block address: + //and save the original heap pointer to allow deallocation + void *ptr = (void *)ALIGN_UP((uintptr_t)head + sizeof(uint32_t) + POISON_OVERHEAD, alignment); + *((uint32_t *)ptr - 1) = (uint32_t)((uintptr_t)ptr - (uintptr_t)head); + + multi_heap_internal_unlock(heap); + + return ptr; +} + void *multi_heap_malloc(multi_heap_handle_t heap, size_t size) { if(size > SIZE_MAX - POISON_OVERHEAD) { @@ -203,6 +249,31 @@ void *multi_heap_malloc(multi_heap_handle_t heap, size_t size) return data; } +void multi_heap_aligned_free(multi_heap_handle_t heap, void *p) +{ + if(p == NULL) { + return; + } + + multi_heap_internal_lock(heap); + + uint32_t offset = *((uint32_t *)p - 1); + void *block_head = (void *)((uint8_t *)p - (offset - POISON_OVERHEAD)); + + poison_head_t *head = verify_allocated_region(block_head, true); + assert(head != NULL); + +#ifdef SLOW + /* replace everything with FREE_FILL_PATTERN, including the poison head/tail */ + memset(head, FREE_FILL_PATTERN, + head->alloc_size + POISON_OVERHEAD); +#endif + + multi_heap_free_impl(heap, head); + + multi_heap_internal_unlock(heap); +} + void multi_heap_free(multi_heap_handle_t heap, void *p) { if (p == NULL) { From 8bd09429d34900e4612152d19ff0af463d554c33 Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Wed, 13 Nov 2019 10:37:23 +0800 Subject: [PATCH 02/10] heap/test_multi_heap_host: added initial tests for heap aligned alloc --- components/heap/multi_heap_poisoning.c | 30 +++++++++-------- .../test_multi_heap_host/test_multi_heap.cpp | 33 +++++++++++++++++++ 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/components/heap/multi_heap_poisoning.c b/components/heap/multi_heap_poisoning.c index 9b86b5351e..01e98f9b35 100644 --- a/components/heap/multi_heap_poisoning.c +++ b/components/heap/multi_heap_poisoning.c @@ -195,32 +195,36 @@ void *multi_heap_aligned_alloc(multi_heap_handle_t heap, size_t size, size_t ali return NULL; } + if(!alignment) { + return NULL; + } + //Alignment must be a power of two... if((alignment & (alignment - 1)) != 0) { return NULL; } - if(size > SIZE_MAX - POISON_OVERHEAD) { + if(size > SIZE_MAX /* - POISON_OVERHEAD*/) { return NULL; } - uint32_t overhead = (sizeof(uint32_t) + (alignment - 1) + POISON_OVERHEAD); + uint32_t overhead = (sizeof(uint32_t) + (alignment - 1) /*+ POISON_OVERHEAD*/); multi_heap_internal_lock(heap); poison_head_t *head = multi_heap_malloc_impl(heap, size + overhead); - uint8_t *data = NULL; + //uint8_t *data = NULL; if (head != NULL) { - data = poison_allocated_region(head, size); + //data = poison_allocated_region(head, size); #ifdef SLOW /* check everything we got back is FREE_FILL_PATTERN & swap for MALLOC_FILL_PATTERN */ - bool ret = verify_fill_pattern(data, size, true, true, true); - assert( ret ); + //bool ret = verify_fill_pattern(data, size, true, true, true); + //assert( ret ); #endif } //Lets align our new obtained block address: //and save the original heap pointer to allow deallocation - void *ptr = (void *)ALIGN_UP((uintptr_t)head + sizeof(uint32_t) + POISON_OVERHEAD, alignment); + void *ptr = (void *)ALIGN_UP((uintptr_t)head + sizeof(uint32_t) /* + POISON_OVERHEAD*/, alignment); *((uint32_t *)ptr - 1) = (uint32_t)((uintptr_t)ptr - (uintptr_t)head); multi_heap_internal_unlock(heap); @@ -258,19 +262,17 @@ void multi_heap_aligned_free(multi_heap_handle_t heap, void *p) multi_heap_internal_lock(heap); uint32_t offset = *((uint32_t *)p - 1); - void *block_head = (void *)((uint8_t *)p - (offset - POISON_OVERHEAD)); + void *block_head = (void *)((uint8_t *)p - (offset /*- POISON_OVERHEAD*/)); - poison_head_t *head = verify_allocated_region(block_head, true); - assert(head != NULL); + /* poison_head_t *head = verify_allocated_region(block_head, true); + assert(head != NULL); */ #ifdef SLOW /* replace everything with FREE_FILL_PATTERN, including the poison head/tail */ - memset(head, FREE_FILL_PATTERN, - head->alloc_size + POISON_OVERHEAD); + //memset(head, FREE_FILL_PATTERN, head->alloc_size + POISON_OVERHEAD); #endif - multi_heap_free_impl(heap, head); - + multi_heap_free_impl(heap, block_head); multi_heap_internal_unlock(heap); } 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 310ee9dcf1..5bfa74ee5f 100644 --- a/components/heap/test_multi_heap_host/test_multi_heap.cpp +++ b/components/heap/test_multi_heap_host/test_multi_heap.cpp @@ -494,3 +494,36 @@ TEST_CASE("unaligned heaps", "[multi_heap]") } } } + +TEST_CASE("multi_heap aligned allocations", "[multi_heap]") +{ + uint8_t test_heap[1024 * 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 + + printf("New heap:\n"); + multi_heap_dump(heap); + printf("*********************\n"); + + for(;aligments < 4096; aligments++) { + + //Use a non-sense size to test correct alignment even in strange + //memory layout objects: + uint8_t *buf = (uint8_t *)multi_heap_aligned_alloc(heap, (aligments + 117), aligments ); + if(((aligments & (aligments - 1)) != 0) || (!aligments)) { + REQUIRE( buf == NULL ); + //printf("[ALIGNED_ALLOC] alignment: %u is not a power of two, don't allow allocation \n", aligments); + } 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] allocated size: %d \n", multi_heap_get_allocated_size(heap, buf)); + printf("[ALIGNED_ALLOC] address of allocated memory: %p \n\n", (void *)buf); + + REQUIRE(((intptr_t)buf & (aligments - 1)) == 0); + multi_heap_aligned_free(heap, buf); + } + } +} From f31b8a8ab87b333ef41ad3c4571b1bec8106a6ad Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Wed, 13 Nov 2019 12:00:30 +0800 Subject: [PATCH 03/10] heap/multi_heap_poisoning: aligned alloc now working togheter with heap poisining code --- components/heap/multi_heap_poisoning.c | 30 +++++++++++-------- .../test_multi_heap_host/test_multi_heap.cpp | 19 +++++++++--- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/components/heap/multi_heap_poisoning.c b/components/heap/multi_heap_poisoning.c index 01e98f9b35..906c827d47 100644 --- a/components/heap/multi_heap_poisoning.c +++ b/components/heap/multi_heap_poisoning.c @@ -204,27 +204,30 @@ void *multi_heap_aligned_alloc(multi_heap_handle_t heap, size_t size, size_t ali return NULL; } - if(size > SIZE_MAX /* - POISON_OVERHEAD*/) { + if(size > SIZE_MAX - POISON_OVERHEAD) { return NULL; } - uint32_t overhead = (sizeof(uint32_t) + (alignment - 1) /*+ POISON_OVERHEAD*/); + uint32_t overhead = (sizeof(uint32_t) + (alignment - 1) + POISON_OVERHEAD); multi_heap_internal_lock(heap); poison_head_t *head = multi_heap_malloc_impl(heap, size + overhead); - //uint8_t *data = NULL; + uint8_t *data = NULL; if (head != NULL) { - //data = poison_allocated_region(head, size); + data = poison_allocated_region(head, size + (overhead - POISON_OVERHEAD)); #ifdef SLOW /* check everything we got back is FREE_FILL_PATTERN & swap for MALLOC_FILL_PATTERN */ - //bool ret = verify_fill_pattern(data, size, true, true, true); - //assert( ret ); + bool ret = verify_fill_pattern(data, size, true, true, true); + assert( ret ); +#else + (void)data; #endif } //Lets align our new obtained block address: - //and save the original heap pointer to allow deallocation - void *ptr = (void *)ALIGN_UP((uintptr_t)head + sizeof(uint32_t) /* + POISON_OVERHEAD*/, alignment); + //and save information to recover original block pointer + //to allow us to deallocate the memory when needed + void *ptr = (void *)ALIGN_UP((uintptr_t)head + sizeof(uint32_t) + sizeof(poison_head_t), alignment); *((uint32_t *)ptr - 1) = (uint32_t)((uintptr_t)ptr - (uintptr_t)head); multi_heap_internal_unlock(heap); @@ -262,14 +265,15 @@ void multi_heap_aligned_free(multi_heap_handle_t heap, void *p) multi_heap_internal_lock(heap); uint32_t offset = *((uint32_t *)p - 1); - void *block_head = (void *)((uint8_t *)p - (offset /*- POISON_OVERHEAD*/)); - - /* poison_head_t *head = verify_allocated_region(block_head, true); - assert(head != NULL); */ + void *block_head = (void *)((uint8_t *)p - offset); + block_head += sizeof(poison_head_t); + poison_head_t *head = verify_allocated_region(block_head, true); + assert(head != NULL); + block_head -= sizeof(poison_head_t); #ifdef SLOW /* replace everything with FREE_FILL_PATTERN, including the poison head/tail */ - //memset(head, FREE_FILL_PATTERN, head->alloc_size + POISON_OVERHEAD); + memset(block_head, FREE_FILL_PATTERN, head->alloc_size + POISON_OVERHEAD); #endif multi_heap_free_impl(heap, block_head); 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 5bfa74ee5f..6f00868803 100644 --- a/components/heap/test_multi_heap_host/test_multi_heap.cpp +++ b/components/heap/test_multi_heap_host/test_multi_heap.cpp @@ -500,16 +500,19 @@ TEST_CASE("multi_heap aligned allocations", "[multi_heap]") uint8_t test_heap[1024 * 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 + size_t old_size = multi_heap_free_size(heap); + size_t leakage = 1024; + printf("[ALIGNED_ALLOC] heap_size before: %d \n", old_size); printf("New heap:\n"); multi_heap_dump(heap); printf("*********************\n"); - for(;aligments < 4096; aligments++) { + for(;aligments < 500 * 1024; aligments++) { - //Use a non-sense size to test correct alignment even in strange + //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 + 117), aligments ); + uint8_t *buf = (uint8_t *)multi_heap_aligned_alloc(heap, (aligments + 137), aligments ); if(((aligments & (aligments - 1)) != 0) || (!aligments)) { REQUIRE( buf == NULL ); //printf("[ALIGNED_ALLOC] alignment: %u is not a power of two, don't allow allocation \n", aligments); @@ -521,9 +524,17 @@ TEST_CASE("multi_heap aligned allocations", "[multi_heap]") printf("[ALIGNED_ALLOC] alignment required: %u \n", aligments); //printf("[ALIGNED_ALLOC] allocated size: %d \n", multi_heap_get_allocated_size(heap, buf)); 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); + + //Write some data, if it corrupts memory probably the heap + //canary verification will fail: + memset(buf, 0xA5, (aligments + 137)); + multi_heap_aligned_free(heap, buf); } } + + printf("[ALIGNED_ALLOC] heap_size after: %d \n", multi_heap_free_size(heap)); + REQUIRE((old_size - multi_heap_free_size(heap)) <= leakage); } From 7fbf4c74d762079bd2ba010a8bf02c6d414fa6b2 Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Wed, 13 Nov 2019 12:49:57 +0800 Subject: [PATCH 04/10] heap/heap_caps: added initial, top level heap_caps_aligned_alloc and heap_caps_aligned_free --- components/heap/heap_caps.c | 91 +++++++++++++++++++++++++ components/heap/include/esp_heap_caps.h | 24 +++++++ 2 files changed, 115 insertions(+) diff --git a/components/heap/heap_caps.c b/components/heap/heap_caps.c index 54018b0dec..322ce23a89 100644 --- a/components/heap/heap_caps.c +++ b/components/heap/heap_caps.c @@ -502,3 +502,94 @@ size_t heap_caps_get_allocated_size( void *ptr ) size_t size = multi_heap_get_allocated_size(heap->heap, ptr); return size; } + +IRAM_ATTR void *heap_caps_aligned_alloc(size_t alignment, size_t size, int caps) +{ + void *ret = NULL; + + if(!alignment) { + return NULL; + } + + //Alignment must be a power of two: + if((alignment & (alignment - 1)) != 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 + return NULL; + } + + if (caps & MALLOC_CAP_EXEC) { + //MALLOC_CAP_EXEC forces an alloc from IRAM. There is a region which has both this as well as the following + //caps, but the following caps are not possible for IRAM. Thus, the combination is impossible and we return + //NULL directly, even although our heap capabilities (based on soc_memory_tags & soc_memory_regions) would + //indicate there is a tag for this. + if ((caps & MALLOC_CAP_8BIT) || (caps & MALLOC_CAP_DMA)) { + return NULL; + } + caps |= MALLOC_CAP_32BIT; // IRAM is 32-bit accessible RAM + } + + if (caps & MALLOC_CAP_32BIT) { + /* 32-bit accessible RAM should allocated in 4 byte aligned sizes + * (Future versions of ESP-IDF should possibly fail if an invalid size is requested) + */ + size = (size + 3) & (~3); // int overflow checked above + } + + for (int prio = 0; prio < SOC_MEMORY_TYPE_NO_PRIOS; prio++) { + //Iterate over heaps and check capabilities at this priority + heap_t *heap; + SLIST_FOREACH(heap, ®istered_heaps, next) { + if (heap->heap == NULL) { + continue; + } + if ((heap->caps[prio] & caps) != 0) { + //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) { + //This heap can satisfy all the requested capabilities. See if we can grab some memory using it. + if ((caps & MALLOC_CAP_EXEC) && esp_ptr_in_diram_dram((void *)heap->start)) { + //This is special, insofar that what we're going to get back is a DRAM address. If so, + //we need to 'invert' it (lowest address in DRAM == highest address in IRAM and vice-versa) and + //add a pointer to the DRAM equivalent before the address we're going to return. + ret = multi_heap_aligned_alloc(heap->heap, size + 4, alignment); // int overflow checked above + if (ret != NULL) { + return dram_alloc_to_iram_addr(ret, size + 4); // int overflow checked above + } + } else { + //Just try to alloc, nothing special. + ret = multi_heap_aligned_alloc(heap->heap, size, alignment); + if (ret != NULL) { + return ret; + } + } + } + } + } + } + //Nothing usable found. + return NULL; +} + +IRAM_ATTR void heap_caps_aligned_free(void *ptr) +{ + if (ptr == NULL) { + return; + } + + if (esp_ptr_in_diram_iram(ptr)) { + //Memory allocated here is actually allocated in the DRAM alias region and + //cannot be de-allocated as usual. dram_alloc_to_iram_addr stores a pointer to + //the equivalent DRAM address, though; free that. + uint32_t *dramAddrPtr = (uint32_t *)ptr; + ptr = (void *)dramAddrPtr[-1]; + } + + heap_t *heap = find_containing_heap(ptr); + assert(heap != NULL && "free() target pointer is outside heap areas"); + multi_heap_aligned_free(heap->heap, ptr); +} diff --git a/components/heap/include/esp_heap_caps.h b/components/heap/include/esp_heap_caps.h index cd91445227..81f62b19eb 100644 --- a/components/heap/include/esp_heap_caps.h +++ b/components/heap/include/esp_heap_caps.h @@ -85,6 +85,30 @@ void heap_caps_free( void *ptr); */ void *heap_caps_realloc( void *ptr, size_t size, int caps); +/** + * @brief Allocate a aligned chunk of memory which has the given capabilities + * + * Equivalent semantics to libc aligned_alloc(), for capability-aware memory. + * @param alignment How the pointer received needs to be aligned + * must be a power of two + * @param size Size, in bytes, of the amount of memory to allocate + * @param caps Bitwise OR of MALLOC_CAP_* flags indicating the type + * of memory to be returned + * + * @return A pointer to the memory allocated on success, NULL on failure + */ +void *heap_caps_aligned_alloc(size_t alignment, size_t size, int caps); + +/** + * @brief Used to deallocate memory previously allocated with heap_caps_aligned_alloc + * + * @param ptr Pointer to the memory allocated + * @note This function is aimed to deallocate only memory allocated with + * heap_caps_aligned_alloc, memory allocated with heap_caps_malloc + * MUST not be passed to this function + */ +void heap_caps_aligned_free(void *ptr); + /** * @brief Allocate a chunk of memory which has the given capabilities. The initialized value in the memory is set to zero. * From a2db437c46ded825f5d702c047304867b0c39050 Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Wed, 13 Nov 2019 16:37:07 +0800 Subject: [PATCH 05/10] heap/heap_caps: Added tests for align allocation on both internal and external ram --- components/heap/heap_caps.c | 40 +++-------- components/heap/multi_heap_poisoning.c | 3 + .../heap/test/test_aligned_alloc_caps.c | 70 +++++++++++++++++++ 3 files changed, 83 insertions(+), 30 deletions(-) create mode 100644 components/heap/test/test_aligned_alloc_caps.c diff --git a/components/heap/heap_caps.c b/components/heap/heap_caps.c index 322ce23a89..9cbe08a9e4 100644 --- a/components/heap/heap_caps.c +++ b/components/heap/heap_caps.c @@ -522,23 +522,14 @@ IRAM_ATTR void *heap_caps_aligned_alloc(size_t alignment, size_t size, int caps) return NULL; } - if (caps & MALLOC_CAP_EXEC) { - //MALLOC_CAP_EXEC forces an alloc from IRAM. There is a region which has both this as well as the following - //caps, but the following caps are not possible for IRAM. Thus, the combination is impossible and we return - //NULL directly, even although our heap capabilities (based on soc_memory_tags & soc_memory_regions) would - //indicate there is a tag for this. - if ((caps & MALLOC_CAP_8BIT) || (caps & MALLOC_CAP_DMA)) { - return NULL; - } - caps |= MALLOC_CAP_32BIT; // IRAM is 32-bit accessible RAM + //aligned alloc for now only supports default allocator or external + //allocator. + if((caps & (MALLOC_CAP_DEFAULT | MALLOC_CAP_SPIRAM)) == 0) { + return NULL; } - if (caps & MALLOC_CAP_32BIT) { - /* 32-bit accessible RAM should allocated in 4 byte aligned sizes - * (Future versions of ESP-IDF should possibly fail if an invalid size is requested) - */ - size = (size + 3) & (~3); // int overflow checked above - } + //if caps requested are supported, clear undesired others: + caps &= (MALLOC_CAP_DEFAULT | MALLOC_CAP_SPIRAM); for (int prio = 0; prio < SOC_MEMORY_TYPE_NO_PRIOS; prio++) { //Iterate over heaps and check capabilities at this priority @@ -551,21 +542,10 @@ IRAM_ATTR void *heap_caps_aligned_alloc(size_t alignment, size_t size, int caps) //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) { - //This heap can satisfy all the requested capabilities. See if we can grab some memory using it. - if ((caps & MALLOC_CAP_EXEC) && esp_ptr_in_diram_dram((void *)heap->start)) { - //This is special, insofar that what we're going to get back is a DRAM address. If so, - //we need to 'invert' it (lowest address in DRAM == highest address in IRAM and vice-versa) and - //add a pointer to the DRAM equivalent before the address we're going to return. - ret = multi_heap_aligned_alloc(heap->heap, size + 4, alignment); // int overflow checked above - if (ret != NULL) { - return dram_alloc_to_iram_addr(ret, size + 4); // int overflow checked above - } - } else { - //Just try to alloc, nothing special. - ret = multi_heap_aligned_alloc(heap->heap, size, alignment); - if (ret != NULL) { - return ret; - } + //Just try to alloc, nothing special. + ret = multi_heap_aligned_alloc(heap->heap, size, alignment); + if (ret != NULL) { + return ret; } } } diff --git a/components/heap/multi_heap_poisoning.c b/components/heap/multi_heap_poisoning.c index 906c827d47..8a356e6495 100644 --- a/components/heap/multi_heap_poisoning.c +++ b/components/heap/multi_heap_poisoning.c @@ -222,6 +222,9 @@ void *multi_heap_aligned_alloc(multi_heap_handle_t heap, size_t size, size_t ali #else (void)data; #endif + } else { + multi_heap_internal_unlock(heap); + return NULL; } //Lets align our new obtained block address: diff --git a/components/heap/test/test_aligned_alloc_caps.c b/components/heap/test/test_aligned_alloc_caps.c new file mode 100644 index 0000000000..909e654e79 --- /dev/null +++ b/components/heap/test/test_aligned_alloc_caps.c @@ -0,0 +1,70 @@ +/* + Tests for the capabilities-based memory allocator. +*/ + +#include +#include +#include "unity.h" +#include "esp_attr.h" +#include "esp_heap_caps.h" +#include "esp_spi_flash.h" +#include +#include +#include + +TEST_CASE("Capabilities aligned allocator test", "[heap]") +{ + uint32_t alignments = 0; + + printf("[ALIGNED_ALLOC] Allocating from default CAP: \n"); + + for(;alignments <= 1024; alignments++) { + uint8_t *buf = (uint8_t *)heap_caps_aligned_alloc(alignments, (alignments + 137), MALLOC_CAP_DEFAULT); + if(((alignments & (alignments - 1)) != 0) || (!alignments)) { + TEST_ASSERT( buf == NULL ); + //printf("[ALIGNED_ALLOC] alignment: %u is not a power of two, don't allow allocation \n", aligments); + } else { + TEST_ASSERT( buf != NULL ); + 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 + TEST_ASSERT(((intptr_t)buf & (alignments - 1)) == 0); + + //Write some data, if it corrupts memory probably the heap + //canary verification will fail: + memset(buf, 0xA5, (alignments + 137)); + + heap_caps_aligned_free(buf); + } + } + + //Alloc from a non permitted area: + uint32_t *not_permitted_buf = (uint32_t *)heap_caps_aligned_alloc(alignments, (alignments + 137), MALLOC_CAP_EXEC | MALLOC_CAP_32BIT); + TEST_ASSERT( not_permitted_buf == NULL ); + +#if CONFIG_ESP32_SPIRAM_SUPPORT || CONFIG_ESP32S2_SPIRAM_SUPPORT + alignments = 0; + printf("[ALIGNED_ALLOC] Allocating from external memory: \n"); + + for(;alignments <= 1024 * 1024; alignments++) { + //Now try to take aligned memory from IRAM: + uint8_t *buf = (uint8_t *)heap_caps_aligned_alloc(alignments, 10*1024, MALLOC_CAP_SPIRAM); + if(((alignments & (alignments - 1)) != 0) || (!alignments)) { + TEST_ASSERT( buf == NULL ); + //printf("[ALIGNED_ALLOC] alignment: %u is not a power of two, don't allow allocation \n", aligments); + } else { + TEST_ASSERT( buf != NULL ); + 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 + TEST_ASSERT(((intptr_t)buf & (alignments - 1)) == 0); + + //Write some data, if it corrupts memory probably the heap + //canary verification will fail: + memset(buf, 0xA5, (10*1024)); + heap_caps_aligned_free(buf); + } + } +#endif + +} \ No newline at end of file From aa100d2dfe2d1665a1b4d86c0f0eb597e2821cc1 Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Wed, 13 Nov 2019 17:12:04 +0800 Subject: [PATCH 06/10] newlib: added heap_caps_aligned_alloc on bottom of memalign --- components/newlib/heap.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/components/newlib/heap.c b/components/newlib/heap.c index 7203d81907..f2413f285f 100644 --- a/components/newlib/heap.c +++ b/components/newlib/heap.c @@ -91,9 +91,7 @@ void newlib_include_heap_impl(void) */ void* memalign(size_t alignment, size_t n) { - extern void memalign_function_was_linked_but_unsupported_in_esp_idf(void); - memalign_function_was_linked_but_unsupported_in_esp_idf(); - return NULL; + return heap_caps_aligned_alloc(alignment, n, MALLOC_CAP_DEFAULT); } int malloc_trim(size_t pad) From 45766daa4fe9053128322f447103826d66de159b Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Thu, 14 Nov 2019 09:34:41 +0800 Subject: [PATCH 07/10] test_multi_heap_host/test_multi_heap: fix undefined reference error when testing aligned_alloc with no heap poisoning --- components/heap/test_multi_heap_host/test_multi_heap.cpp | 4 ++++ 1 file changed, 4 insertions(+) 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 6f00868803..f668dd9533 100644 --- a/components/heap/test_multi_heap_host/test_multi_heap.cpp +++ b/components/heap/test_multi_heap_host/test_multi_heap.cpp @@ -495,6 +495,8 @@ TEST_CASE("unaligned heaps", "[multi_heap]") } } +#ifndef CONFIG_HEAP_POISONING_NONE + TEST_CASE("multi_heap aligned allocations", "[multi_heap]") { uint8_t test_heap[1024 * 1024]; @@ -538,3 +540,5 @@ TEST_CASE("multi_heap aligned allocations", "[multi_heap]") printf("[ALIGNED_ALLOC] heap_size after: %d \n", multi_heap_free_size(heap)); REQUIRE((old_size - multi_heap_free_size(heap)) <= leakage); } + +#endif \ No newline at end of file From 8e5ea171d3372cfd484cc73fa68b6ca876f3d0e4 Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Tue, 19 Nov 2019 10:36:58 -0300 Subject: [PATCH 08/10] newlib: reverted support of memalign function --- components/newlib/heap.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/components/newlib/heap.c b/components/newlib/heap.c index f2413f285f..9dbc635064 100644 --- a/components/newlib/heap.c +++ b/components/newlib/heap.c @@ -91,7 +91,10 @@ void newlib_include_heap_impl(void) */ void* memalign(size_t alignment, size_t n) { - return heap_caps_aligned_alloc(alignment, n, MALLOC_CAP_DEFAULT); + extern void memalign_function_was_linked_but_unsupported_in_esp_idf(void); + memalign_function_was_linked_but_unsupported_in_esp_idf(); + return NULL; + } int malloc_trim(size_t pad) From 0d8a5ebec792d33727bfb8ac6cc54f9fa4088a25 Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Fri, 20 Dec 2019 12:48:35 -0300 Subject: [PATCH 09/10] heap: added aligned calloc function plus tests --- components/heap/heap_caps.c | 15 ++++ components/heap/include/esp_heap_caps.h | 14 ++++ .../heap/test/test_aligned_alloc_caps.c | 69 +++++++++++++++++++ 3 files changed, 98 insertions(+) diff --git a/components/heap/heap_caps.c b/components/heap/heap_caps.c index 9cbe08a9e4..c73264458b 100644 --- a/components/heap/heap_caps.c +++ b/components/heap/heap_caps.c @@ -555,6 +555,21 @@ IRAM_ATTR void *heap_caps_aligned_alloc(size_t alignment, size_t size, int caps) return NULL; } +void *heap_caps_aligned_calloc(size_t alignment, size_t n, size_t size, uint32_t caps) +{ + size_t size_bytes; + if (__builtin_mul_overflow(n, size, &size_bytes)) { + return NULL; + } + + void *ptr = heap_caps_aligned_alloc(alignment,size_bytes, caps); + if(ptr != NULL) { + memset(ptr, 0, size_bytes); + } + + return ptr; +} + IRAM_ATTR void heap_caps_aligned_free(void *ptr) { if (ptr == NULL) { diff --git a/components/heap/include/esp_heap_caps.h b/components/heap/include/esp_heap_caps.h index 81f62b19eb..a0711d0b2d 100644 --- a/components/heap/include/esp_heap_caps.h +++ b/components/heap/include/esp_heap_caps.h @@ -99,6 +99,20 @@ void *heap_caps_realloc( void *ptr, size_t size, int caps); */ void *heap_caps_aligned_alloc(size_t alignment, size_t size, int caps); +/** + * @brief Allocate a aligned chunk of memory which has the given capabilities. The initialized value in the memory is set to zero. + * + * @param alignment How the pointer received needs to be aligned + * must be a power of two + * @param n Number of continuing chunks of memory to allocate + * @param size Size, in bytes, of a chunk of memory to allocate + * @param caps Bitwise OR of MALLOC_CAP_* flags indicating the type + * of memory to be returned + * + * @return A pointer to the memory allocated on success, NULL on failure + */ +void *heap_caps_aligned_calloc(size_t alignment, size_t n, size_t size, uint32_t caps); + /** * @brief Used to deallocate memory previously allocated with heap_caps_aligned_alloc * diff --git a/components/heap/test/test_aligned_alloc_caps.c b/components/heap/test/test_aligned_alloc_caps.c index 909e654e79..613eb1eb29 100644 --- a/components/heap/test/test_aligned_alloc_caps.c +++ b/components/heap/test/test_aligned_alloc_caps.c @@ -67,4 +67,73 @@ TEST_CASE("Capabilities aligned allocator test", "[heap]") } #endif +} + +TEST_CASE("Capabilities aligned calloc test", "[heap]") +{ + uint32_t alignments = 0; + + printf("[ALIGNED_ALLOC] Allocating from default CAP: \n"); + + for(;alignments <= 1024; alignments++) { + uint8_t *buf = (uint8_t *)heap_caps_aligned_calloc(alignments, 1, (alignments + 137), MALLOC_CAP_DEFAULT); + if(((alignments & (alignments - 1)) != 0) || (!alignments)) { + TEST_ASSERT( buf == NULL ); + //printf("[ALIGNED_ALLOC] alignment: %u is not a power of two, don't allow allocation \n", aligments); + } else { + TEST_ASSERT( buf != NULL ); + 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 + TEST_ASSERT(((intptr_t)buf & (alignments - 1)) == 0); + + //Write some data, if it corrupts memory probably the heap + //canary verification will fail: + memset(buf, 0xA5, (alignments + 137)); + + heap_caps_aligned_free(buf); + } + } + + //Check if memory is initialized with zero: + uint8_t byte_array[1024]; + memset(&byte_array, 0, sizeof(byte_array)); + uint8_t *buf = (uint8_t *)heap_caps_aligned_calloc(1024, 1, 1024, MALLOC_CAP_DEFAULT); + TEST_ASSERT(memcmp(byte_array, buf, sizeof(byte_array)) == 0); + heap_caps_aligned_free(buf); + + //Same size, but different chunk: + buf = (uint8_t *)heap_caps_aligned_calloc(1024, 1024, 1, MALLOC_CAP_DEFAULT); + TEST_ASSERT(memcmp(byte_array, buf, sizeof(byte_array)) == 0); + heap_caps_aligned_free(buf); + + //Alloc from a non permitted area: + uint32_t *not_permitted_buf = (uint32_t *)heap_caps_aligned_calloc(alignments, 1, (alignments + 137), MALLOC_CAP_32BIT); + TEST_ASSERT( not_permitted_buf == NULL ); + +#if CONFIG_ESP32_SPIRAM_SUPPORT || CONFIG_ESP32S2_SPIRAM_SUPPORT + alignments = 0; + printf("[ALIGNED_ALLOC] Allocating from external memory: \n"); + + for(;alignments <= 1024 * 1024; alignments++) { + //Now try to take aligned memory from IRAM: + uint8_t *buf = (uint8_t *)(uint8_t *)heap_caps_aligned_calloc(alignments, 1, 10*1024, MALLOC_CAP_SPIRAM);; + if(((alignments & (alignments - 1)) != 0) || (!alignments)) { + TEST_ASSERT( buf == NULL ); + //printf("[ALIGNED_ALLOC] alignment: %u is not a power of two, don't allow allocation \n", aligments); + } else { + TEST_ASSERT( buf != NULL ); + 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 + TEST_ASSERT(((intptr_t)buf & (alignments - 1)) == 0); + + //Write some data, if it corrupts memory probably the heap + //canary verification will fail: + memset(buf, 0xA5, (10*1024)); + heap_caps_aligned_free(buf); + } + } +#endif + } \ No newline at end of file From 6a307ee70fdb79ed871e87dec6ccf2de4a04b371 Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Fri, 20 Dec 2019 13:03:02 -0300 Subject: [PATCH 10/10] heap: removed ptr check in diram area since aligned allocator does not support data allocated from IRAM --- components/heap/heap_caps.c | 8 -------- components/heap/include/esp_heap_caps.h | 7 +++++++ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/components/heap/heap_caps.c b/components/heap/heap_caps.c index c73264458b..1d73b0087f 100644 --- a/components/heap/heap_caps.c +++ b/components/heap/heap_caps.c @@ -576,14 +576,6 @@ IRAM_ATTR void heap_caps_aligned_free(void *ptr) return; } - if (esp_ptr_in_diram_iram(ptr)) { - //Memory allocated here is actually allocated in the DRAM alias region and - //cannot be de-allocated as usual. dram_alloc_to_iram_addr stores a pointer to - //the equivalent DRAM address, though; free that. - uint32_t *dramAddrPtr = (uint32_t *)ptr; - ptr = (void *)dramAddrPtr[-1]; - } - heap_t *heap = find_containing_heap(ptr); assert(heap != NULL && "free() target pointer is outside heap areas"); multi_heap_aligned_free(heap->heap, ptr); diff --git a/components/heap/include/esp_heap_caps.h b/components/heap/include/esp_heap_caps.h index a0711d0b2d..61db74e1ce 100644 --- a/components/heap/include/esp_heap_caps.h +++ b/components/heap/include/esp_heap_caps.h @@ -96,6 +96,10 @@ void *heap_caps_realloc( void *ptr, size_t size, int caps); * of memory to be returned * * @return A pointer to the memory allocated on success, NULL on failure + * + * @note Any memory allocated with heaps_caps_aligned_alloc() MUST + * be freed with heap_caps_aligned_free() and CANNOT be passed to free() + * */ void *heap_caps_aligned_alloc(size_t alignment, size_t size, int caps); @@ -110,6 +114,9 @@ void *heap_caps_aligned_alloc(size_t alignment, size_t size, int caps); * of memory to be returned * * @return A pointer to the memory allocated on success, NULL on failure + * + * @note Any memory allocated with heap_caps_aligned_calloc() MUST + * be freed with heap_caps_aligned_free() and CANNOT be passed to free() */ void *heap_caps_aligned_calloc(size_t alignment, size_t n, size_t size, uint32_t caps);