From c822e0abbdbdb44274497d80ac3a270f08d83959 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Mon, 25 Jan 2021 20:55:16 -0500 Subject: [PATCH] libc/minimal: fix realloc() allocated memory alignment The definition for realloc() says that it should return a pointer to the allocated memory which is suitably aligned for any built-in type. Turn sys_heap_realloc() into a sys_heap_aligned_realloc() and use it with __alignof__(z_max_align_t) to implement realloc() with proper memory alignment for any platform. Signed-off-by: Nicolas Pitre --- include/sys/sys_heap.h | 7 +++++- lib/libc/minimal/source/stdlib/malloc.c | 4 ++- lib/os/heap.c | 32 ++++++++++++++---------- tests/lib/heap/src/main.c | 33 ++++++++++++++++++++++++- 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/include/sys/sys_heap.h b/include/sys/sys_heap.h index 66bcb600210..975373a0f4d 100644 --- a/include/sys/sys_heap.h +++ b/include/sys/sys_heap.h @@ -137,10 +137,15 @@ void sys_heap_free(struct sys_heap *h, void *mem); * * @param heap Heap from which to allocate * @param ptr Original pointer returned from a previous allocation + * @param align Alignment in bytes, must be a power of two * @param bytes Number of bytes requested for the new block * @return Pointer to memory the caller can now use, or NULL */ -void *sys_heap_realloc(struct sys_heap *heap, void *ptr, size_t bytes); +void *sys_heap_aligned_realloc(struct sys_heap *heap, void *ptr, + size_t align, size_t bytes); + +#define sys_heap_realloc(heap, ptr, bytes) \ + sys_heap_aligned_realloc(heap, ptr, 0, bytes) /** @brief Validate heap integrity * diff --git a/lib/libc/minimal/source/stdlib/malloc.c b/lib/libc/minimal/source/stdlib/malloc.c index 5dbc03f9427..685ff20b108 100644 --- a/lib/libc/minimal/source/stdlib/malloc.c +++ b/lib/libc/minimal/source/stdlib/malloc.c @@ -56,7 +56,9 @@ static int malloc_prepare(const struct device *unused) void *realloc(void *ptr, size_t requested_size) { - void *ret = sys_heap_realloc(&z_malloc_heap, ptr, requested_size); + void *ret = sys_heap_aligned_realloc(&z_malloc_heap, ptr, + __alignof__(z_max_align_t), + requested_size); return ret == NULL ? ptr : ret; } diff --git a/lib/os/heap.c b/lib/os/heap.c index 27407d9f621..16678e8c01d 100644 --- a/lib/os/heap.c +++ b/lib/os/heap.c @@ -312,28 +312,34 @@ void *sys_heap_aligned_alloc(struct sys_heap *heap, size_t align, size_t bytes) return mem; } -void *sys_heap_realloc(struct sys_heap *heap, void *ptr, size_t bytes) +void *sys_heap_aligned_realloc(struct sys_heap *heap, void *ptr, + size_t align, size_t bytes) { struct z_heap *h = heap->heap; /* special realloc semantics */ if (ptr == NULL) { - return sys_heap_alloc(heap, bytes); + return sys_heap_aligned_alloc(heap, align, bytes); } if (bytes == 0) { sys_heap_free(heap, ptr); return NULL; } + __ASSERT((align & (align - 1)) == 0, "align must be a power of 2"); + if (size_too_big(h, bytes)) { return NULL; } chunkid_t c = mem_to_chunkid(h, ptr); chunkid_t rc = right_chunk(h, c); - size_t chunks_need = bytes_to_chunksz(h, bytes); + size_t align_gap = (uint8_t *)ptr - (uint8_t *)chunk_mem(h, c); + size_t chunks_need = bytes_to_chunksz(h, bytes + align_gap); - if (chunk_size(h, c) == chunks_need) { + if (align && ((uintptr_t)ptr & (align - 1))) { + /* ptr is not sufficiently aligned */ + } else if (chunk_size(h, c) == chunks_need) { /* We're good already */ return ptr; } else if (chunk_size(h, c) > chunks_need) { @@ -357,18 +363,18 @@ void *sys_heap_realloc(struct sys_heap *heap, void *ptr, size_t bytes) merge_chunks(h, c, rc); set_chunk_used(h, c, true); return ptr; - } else { - /* Reallocate and copy */ - void *ptr2 = sys_heap_alloc(heap, bytes); + } - if (ptr2 == NULL) { - return NULL; - } + /* Fallback: allocate and copy */ + void *ptr2 = sys_heap_aligned_alloc(heap, align, bytes); - memcpy(ptr2, ptr, bytes); - sys_heap_free(heap, ptr); - return ptr2; + if (ptr2 == NULL) { + return NULL; } + + memcpy(ptr2, ptr, bytes); + sys_heap_free(heap, ptr); + return ptr2; } void sys_heap_init(struct sys_heap *heap, void *mem, size_t bytes) diff --git a/tests/lib/heap/src/main.c b/tests/lib/heap/src/main.c index 81a3a509c42..28904c30000 100644 --- a/tests/lib/heap/src/main.c +++ b/tests/lib/heap/src/main.c @@ -261,7 +261,7 @@ static void test_realloc(void) zassert_true(sys_heap_validate(&heap), "invalid heap"); zassert_true(p1 != p2, - "Realloc should must have moved %p", p1); + "Realloc should have moved %p", p1); zassert_true(realloc_check_block(p2, p2, 64), "data changed"); zassert_true(realloc_check_block(p3, p1, 64), "data changed"); @@ -290,6 +290,37 @@ static void test_realloc(void) "Realloc should have expanded in place %p -> %p", p1, p3); zassert_true(realloc_check_block(p3, p1, 61), "data changed"); + + /* Corner case with sys_heap_aligned_realloc() on 32-bit targets + * where actual memory doesn't match with given pointer + * (align_gap != 0). + */ + p1 = sys_heap_aligned_alloc(&heap, 8, 32); + realloc_fill_block(p1, 32); + p2 = sys_heap_alloc(&heap, 32); + realloc_fill_block(p2, 32); + p3 = sys_heap_aligned_realloc(&heap, p1, 8, 36); + + zassert_true(sys_heap_validate(&heap), "invalid heap"); + zassert_true(realloc_check_block(p3, p1, 32), "data changed"); + zassert_true(realloc_check_block(p2, p2, 32), "data changed"); + realloc_fill_block(p3, 36); + zassert_true(sys_heap_validate(&heap), "invalid heap"); + zassert_true(p1 != p3, + "Realloc should have moved %p", p1); + + /* Test realloc with increasing alignment */ + p1 = sys_heap_aligned_alloc(&heap, 32, 32); + p2 = sys_heap_aligned_alloc(&heap, 8, 32); + p3 = sys_heap_aligned_realloc(&heap, p2, 8, 16); + zassert_true(sys_heap_validate(&heap), "invalid heap"); + zassert_true(p2 == p3, + "Realloc should have expanded in place %p -> %p", + p2, p3); + p3 = sys_heap_aligned_alloc(&heap, 32, 8); + zassert_true(sys_heap_validate(&heap), "invalid heap"); + zassert_true(p2 != p3, + "Realloc should have moved %p", p2); } void test_main(void)