From 8d32b668ec25415954a9c70685eac55e8ff0f437 Mon Sep 17 00:00:00 2001 From: Lingkai Dong Date: Tue, 31 Aug 2021 14:04:31 +0100 Subject: [PATCH 1/2] Workaround for a bug in malloc() from newlib-nano 4.1.0 The commit 84d0689 "Nano-malloc: Fix for unwanted external heap fragmentation" from newlib 4.1.0 introduced several optimizations, one of which is as follows: When the last chunk in the free list is smaller than requested, nano_malloc() calls sbrk(0) to see if the heap's current head is adjacent to this chunk, and if so it asks sbrk() to allocate the difference in bytes only and expands the current chunk. This doesn't work if the heap consists of non-contiguous regions. sbrk(0) returns the the current region's head if the region has any remaining capacity. But if this capacity is not enough for the second (non-trivial) call to sbrk() described above, allocation will happen from the next region if available. Expanding the current chunk won't work and will result in a segmentation fault. So this optimization needs to be reverted in order to bring back compatibility with non-contiguous heaps. Before the next version of newlib becomes available and gets updated in the GCC Arm Embedded Toolchain, we work around this issue by including the fix in Mbed OS. The linker prioritizes malloc() from the project to the one from the toolchain. --- .codecheckignore | 1 + platform/source/CMakeLists.txt | 7 + .../source/newlib_nano_malloc_workaround.c | 314 ++++++++++++++++++ 3 files changed, 322 insertions(+) create mode 100644 platform/source/newlib_nano_malloc_workaround.c diff --git a/.codecheckignore b/.codecheckignore index 3b8d06abad..c75a49e7be 100644 --- a/.codecheckignore +++ b/.codecheckignore @@ -26,6 +26,7 @@ ^platform/FEATURE_EXPERIMENTAL_API/FEATURE_PSA/TARGET_TFM ^platform/mbed-trace ^platform/randlib +^platform/source/newlib_nano_malloc_workaround.c ^platform/tests/UNITTESTS ^events/tests/UNITTESTS ^rtos/source/TARGET_CORTEX/rtx4 diff --git a/platform/source/CMakeLists.txt b/platform/source/CMakeLists.txt index 0eef2eef45..2067646dc4 100644 --- a/platform/source/CMakeLists.txt +++ b/platform/source/CMakeLists.txt @@ -48,3 +48,10 @@ target_sources(mbed-core mbed_thread.cpp mbed_wait_api_no_rtos.c ) + +if(MBED_TOOLCHAIN STREQUAL "GCC_ARM" AND MBED_C_LIB STREQUAL "small") + target_sources(mbed-core + INTERFACE + newlib_nano_malloc_workaround.c + ) +endif() diff --git a/platform/source/newlib_nano_malloc_workaround.c b/platform/source/newlib_nano_malloc_workaround.c new file mode 100644 index 0000000000..46f51ee68d --- /dev/null +++ b/platform/source/newlib_nano_malloc_workaround.c @@ -0,0 +1,314 @@ +/* + * Copyright (c) 2012, 2013 ARM Ltd + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. The name of the company may not be used to endorse or promote + * products derived from this software without specific prior written + * permission. + * + * THIS SOFTWARE IS PROVIDED BY ARM LTD ``AS IS'' AND ANY EXPRESS OR IMPLIED + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. + * IN NO EVENT SHALL ARM LTD BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED + * TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR + * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING + * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +/** + * NOTE: This file is based on newlib/libc/stdlib/nano-mallocr.c + * from git://sourceware.org/git/newlib-cygwin.git. It contains a + * workaround for a bug in newlib 4.1.0: + * + * The commit 84d0689 "Nano-malloc: Fix for unwanted external heap + * fragmentation" from newlib 4.1.0 introduced several optimizations, + * one of which is as follows: + * + * When the last chunk in the free list is smaller than requested, + * nano_malloc() calls sbrk(0) to see if the heap's current head is + * adjacent to this chunk, and if so it asks sbrk() for the difference + * in bytes only and expands the current chunk. + * + * This doesn't work if the heap consists of non-contiguous regions. + * sbrk(0) returns the the current region's head if the latter has any + * remaining capacity. But if this capacity is not enough for the second + * (non-trivial) call to sbrk() described above, allocation will happen + * at the next region if available. Expanding the current chunk won't + * work and will result in a segmentation fault. + * + * So this optimization needs to be reverted in order to bring back + * compatibility with non-contiguous heaps. Before the next version + * of newlib becomes available and gets updated in the GCC Arm Embedded + * Toolchains, we work around this issue by including the fix in Mbed OS. + * The linker prioritizes malloc() from the project to the one from the + * toolchain. + */ + +#if defined(__NEWLIB_NANO) + +#include + +#if (__NEWLIB__ == 4) && (__NEWLIB_MINOR__ == 1) && (__NEWLIB_PATCHLEVEL__ == 0) + +/* Implementation of <> <> <> <>, optional + * as to be reenterable. + * + * Interface documentation refer to malloc.c. + */ + +#include +#include +#include +#include + +#if DEBUG +#include +#else +#define assert(x) ((void)0) +#endif + +#ifndef MAX +#define MAX(a,b) ((a) >= (b) ? (a) : (b)) +#endif + +#define _SBRK_R(X) _sbrk_r(X) + +#include +#include + +#define RARG struct _reent *reent_ptr, +#define RCALL reent_ptr, + +#define MALLOC_LOCK __malloc_lock(reent_ptr) +#define MALLOC_UNLOCK __malloc_unlock(reent_ptr) + +#define RERRNO reent_ptr->_errno + +#define nano_malloc _malloc_r + +/* Redefine names to avoid conflict with user names */ +#define free_list __malloc_free_list +#define sbrk_start __malloc_sbrk_start + +#define ALIGN_PTR(ptr, align) \ + (((ptr) + (align) - (intptr_t)1) & ~((align) - (intptr_t)1)) +#define ALIGN_SIZE(size, align) \ + (((size) + (align) - (size_t)1) & ~((align) - (size_t)1)) + +/* Alignment of allocated block */ +#define MALLOC_ALIGN (8U) +#define CHUNK_ALIGN (sizeof(void*)) +#define MALLOC_PADDING ((MAX(MALLOC_ALIGN, CHUNK_ALIGN)) - CHUNK_ALIGN) + +/* as well as the minimal allocation size + * to hold a free pointer */ +#define MALLOC_MINSIZE (sizeof(void *)) +#define MALLOC_PAGE_ALIGN (0x1000) +#define MAX_ALLOC_SIZE (0x80000000U) + +typedef size_t malloc_size_t; + +typedef struct malloc_chunk +{ + /* -------------------------------------- + * chunk->| size | + * -------------------------------------- + * | Padding for alignment | + * | This includes padding inserted by | + * | the compiler (to align fields) and | + * | explicit padding inserted by this | + * | implementation. If any explicit | + * | padding is being used then the | + * | sizeof (size) bytes at | + * | mem_ptr - CHUNK_OFFSET must be | + * | initialized with the negative | + * | offset to size. | + * -------------------------------------- + * mem_ptr->| When allocated: data | + * | When freed: pointer to next free | + * | chunk | + * -------------------------------------- + */ + /* size of the allocated payload area, including size before + CHUNK_OFFSET */ + long size; + + /* since here, the memory is either the next free block, or data load */ + struct malloc_chunk * next; +}chunk; + + +#define CHUNK_OFFSET ((malloc_size_t)(&(((struct malloc_chunk *)0)->next))) + +/* size of smallest possible chunk. A memory piece smaller than this size + * won't be able to create a chunk */ +#define MALLOC_MINCHUNK (CHUNK_OFFSET + MALLOC_PADDING + MALLOC_MINSIZE) + +/* List list header of free blocks */ +chunk * free_list = NULL; + +/* Starting point of memory allocated from system */ +char * sbrk_start = NULL; + +/** Function sbrk_aligned + * Algorithm: + * Use sbrk() to obtain more memory and ensure it is CHUNK_ALIGN aligned + * Optimise for the case that it is already aligned - only ask for extra + * padding after we know we need it + */ +static void* sbrk_aligned(RARG malloc_size_t s) +{ + char *p, *align_p; + + if (sbrk_start == NULL) sbrk_start = _SBRK_R(RCALL 0); + + p = _SBRK_R(RCALL s); + + /* sbrk returns -1 if fail to allocate */ + if (p == (void *)-1) + return p; + + align_p = (char*)ALIGN_PTR((uintptr_t)p, CHUNK_ALIGN); + if (align_p != p) + { + /* p is not aligned, ask for a few more bytes so that we have s + * bytes reserved from align_p. */ + p = _SBRK_R(RCALL align_p - p); + if (p == (void *)-1) + return p; + } + return align_p; +} + +/** Function nano_malloc + * Algorithm: + * Walk through the free list to find the first match. If fails to find + * one, call sbrk to allocate a new chunk. + */ +void * nano_malloc(RARG malloc_size_t s) +{ + chunk *p, *r; + char * ptr, * align_ptr; + int offset; + + malloc_size_t alloc_size; + + alloc_size = ALIGN_SIZE(s, CHUNK_ALIGN); /* size of aligned data load */ + alloc_size += MALLOC_PADDING; /* padding */ + alloc_size += CHUNK_OFFSET; /* size of chunk head */ + alloc_size = MAX(alloc_size, MALLOC_MINCHUNK); + + if (alloc_size >= MAX_ALLOC_SIZE || alloc_size < s) + { + RERRNO = ENOMEM; + return NULL; + } + + MALLOC_LOCK; + + p = free_list; + r = p; + + while (r) + { + int rem = r->size - alloc_size; + if (rem >= 0) + { + if (rem >= MALLOC_MINCHUNK) + { + if (p == r) + { + /* First item in the list, break it into two chunks + * and return the first one */ + r->size = alloc_size; + free_list = (chunk *)((char *)r + alloc_size); + free_list->size = rem; + free_list->next = r->next; + } else { + /* Any other item in the list. Split and return + * the first one */ + r->size = alloc_size; + p->next = (chunk *)((char *)r + alloc_size); + p->next->size = rem; + p->next->next = r->next; + } + } + /* Find a chunk that is exactly the size or slightly bigger + * than requested size, just return this chunk */ + else if (p == r) + { + /* Now it implies p==r==free_list. Move the free_list + * to next chunk */ + free_list = r->next; + } + else + { + /* Normal case. Remove it from free_list */ + p->next = r->next; + } + break; + } + p=r; + r=r->next; + } + + /* Failed to find a appropriate chunk. Ask for more memory */ + if (r == NULL) + { + r = sbrk_aligned(RCALL alloc_size); + + /* sbrk returns -1 if fail to allocate */ + if (r == (void *)-1) + { + RERRNO = ENOMEM; + MALLOC_UNLOCK; + return NULL; + } + else + { + r->size = alloc_size; + } + } + MALLOC_UNLOCK; + + ptr = (char *)r + CHUNK_OFFSET; + + align_ptr = (char *)ALIGN_PTR((uintptr_t)ptr, MALLOC_ALIGN); + offset = align_ptr - ptr; + + if (offset) + { + /* Initialize sizeof (malloc_chunk.size) bytes at + align_ptr - CHUNK_OFFSET with negative offset to the + size field (at the start of the chunk). + + The negative offset to size from align_ptr - CHUNK_OFFSET is + the size of any remaining padding minus CHUNK_OFFSET. This is + equivalent to the total size of the padding, because the size of + any remaining padding is the total size of the padding minus + CHUNK_OFFSET. + + Note that the size of the padding must be at least CHUNK_OFFSET. + + The rest of the padding is not initialized. */ + *(long *)((char *)r + offset) = -offset; + } + + assert(align_ptr + size <= (char *)r + alloc_size); + return align_ptr; +} + +#endif // (__NEWLIB__ == 4) && (__NEWLIB_MINOR__ == 1) && (__NEWLIB_PATCHLEVEL__ == 0) + +#endif // defined(__NEWLIB_NANO) From 8e412987b12d60d2b1012492f56f186fcccc4974 Mon Sep 17 00:00:00 2001 From: Lingkai Dong Date: Wed, 1 Sep 2021 15:05:11 +0100 Subject: [PATCH 2/2] Revert "TESTS: Reduce allocate memory size" This reverts commit 6649e9510663967606c69e618d5c2df7b72424df. In the malloc test, `ALLOC_ARRAY_SIZE` defines the *capacity* of array that stores pointers to `malloc`'d buffers. It is *not* the number of allocations. In an ideal scenario, the test makes as many allocations as possible until the heap runs out and malloc() returns NULL. So revert the capacity of the array of pointers from 50 back to 100 so this array is less likely to run out before the heap does. --- rtos/tests/TESTS/mbed_rtos/malloc/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rtos/tests/TESTS/mbed_rtos/malloc/main.cpp b/rtos/tests/TESTS/mbed_rtos/malloc/main.cpp index 70aece1946..a470547e2d 100644 --- a/rtos/tests/TESTS/mbed_rtos/malloc/main.cpp +++ b/rtos/tests/TESTS/mbed_rtos/malloc/main.cpp @@ -106,7 +106,7 @@ void test_multithread_allocation(void) #endif /** Test for multiple heap alloc and free calls */ -#define ALLOC_ARRAY_SIZE 50 +#define ALLOC_ARRAY_SIZE 100 #define ALLOC_LOOP 20 #define SIZE_INCREMENTS 1023 #define SIZE_MODULO 31