Skip to content

Commit c822e0a

Browse files
Nicolas Pitrenashif
authored andcommitted
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 <[email protected]>
1 parent 4690b8d commit c822e0a

File tree

4 files changed

+60
-16
lines changed

4 files changed

+60
-16
lines changed

include/sys/sys_heap.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,15 @@ void sys_heap_free(struct sys_heap *h, void *mem);
137137
*
138138
* @param heap Heap from which to allocate
139139
* @param ptr Original pointer returned from a previous allocation
140+
* @param align Alignment in bytes, must be a power of two
140141
* @param bytes Number of bytes requested for the new block
141142
* @return Pointer to memory the caller can now use, or NULL
142143
*/
143-
void *sys_heap_realloc(struct sys_heap *heap, void *ptr, size_t bytes);
144+
void *sys_heap_aligned_realloc(struct sys_heap *heap, void *ptr,
145+
size_t align, size_t bytes);
146+
147+
#define sys_heap_realloc(heap, ptr, bytes) \
148+
sys_heap_aligned_realloc(heap, ptr, 0, bytes)
144149

145150
/** @brief Validate heap integrity
146151
*

lib/libc/minimal/source/stdlib/malloc.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@ static int malloc_prepare(const struct device *unused)
5656

5757
void *realloc(void *ptr, size_t requested_size)
5858
{
59-
void *ret = sys_heap_realloc(&z_malloc_heap, ptr, requested_size);
59+
void *ret = sys_heap_aligned_realloc(&z_malloc_heap, ptr,
60+
__alignof__(z_max_align_t),
61+
requested_size);
6062

6163
return ret == NULL ? ptr : ret;
6264
}

lib/os/heap.c

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -312,28 +312,34 @@ void *sys_heap_aligned_alloc(struct sys_heap *heap, size_t align, size_t bytes)
312312
return mem;
313313
}
314314

315-
void *sys_heap_realloc(struct sys_heap *heap, void *ptr, size_t bytes)
315+
void *sys_heap_aligned_realloc(struct sys_heap *heap, void *ptr,
316+
size_t align, size_t bytes)
316317
{
317318
struct z_heap *h = heap->heap;
318319

319320
/* special realloc semantics */
320321
if (ptr == NULL) {
321-
return sys_heap_alloc(heap, bytes);
322+
return sys_heap_aligned_alloc(heap, align, bytes);
322323
}
323324
if (bytes == 0) {
324325
sys_heap_free(heap, ptr);
325326
return NULL;
326327
}
327328

329+
__ASSERT((align & (align - 1)) == 0, "align must be a power of 2");
330+
328331
if (size_too_big(h, bytes)) {
329332
return NULL;
330333
}
331334

332335
chunkid_t c = mem_to_chunkid(h, ptr);
333336
chunkid_t rc = right_chunk(h, c);
334-
size_t chunks_need = bytes_to_chunksz(h, bytes);
337+
size_t align_gap = (uint8_t *)ptr - (uint8_t *)chunk_mem(h, c);
338+
size_t chunks_need = bytes_to_chunksz(h, bytes + align_gap);
335339

336-
if (chunk_size(h, c) == chunks_need) {
340+
if (align && ((uintptr_t)ptr & (align - 1))) {
341+
/* ptr is not sufficiently aligned */
342+
} else if (chunk_size(h, c) == chunks_need) {
337343
/* We're good already */
338344
return ptr;
339345
} 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)
357363
merge_chunks(h, c, rc);
358364
set_chunk_used(h, c, true);
359365
return ptr;
360-
} else {
361-
/* Reallocate and copy */
362-
void *ptr2 = sys_heap_alloc(heap, bytes);
366+
}
363367

364-
if (ptr2 == NULL) {
365-
return NULL;
366-
}
368+
/* Fallback: allocate and copy */
369+
void *ptr2 = sys_heap_aligned_alloc(heap, align, bytes);
367370

368-
memcpy(ptr2, ptr, bytes);
369-
sys_heap_free(heap, ptr);
370-
return ptr2;
371+
if (ptr2 == NULL) {
372+
return NULL;
371373
}
374+
375+
memcpy(ptr2, ptr, bytes);
376+
sys_heap_free(heap, ptr);
377+
return ptr2;
372378
}
373379

374380
void sys_heap_init(struct sys_heap *heap, void *mem, size_t bytes)

tests/lib/heap/src/main.c

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ static void test_realloc(void)
261261

262262
zassert_true(sys_heap_validate(&heap), "invalid heap");
263263
zassert_true(p1 != p2,
264-
"Realloc should must have moved %p", p1);
264+
"Realloc should have moved %p", p1);
265265
zassert_true(realloc_check_block(p2, p2, 64), "data changed");
266266
zassert_true(realloc_check_block(p3, p1, 64), "data changed");
267267

@@ -290,6 +290,37 @@ static void test_realloc(void)
290290
"Realloc should have expanded in place %p -> %p",
291291
p1, p3);
292292
zassert_true(realloc_check_block(p3, p1, 61), "data changed");
293+
294+
/* Corner case with sys_heap_aligned_realloc() on 32-bit targets
295+
* where actual memory doesn't match with given pointer
296+
* (align_gap != 0).
297+
*/
298+
p1 = sys_heap_aligned_alloc(&heap, 8, 32);
299+
realloc_fill_block(p1, 32);
300+
p2 = sys_heap_alloc(&heap, 32);
301+
realloc_fill_block(p2, 32);
302+
p3 = sys_heap_aligned_realloc(&heap, p1, 8, 36);
303+
304+
zassert_true(sys_heap_validate(&heap), "invalid heap");
305+
zassert_true(realloc_check_block(p3, p1, 32), "data changed");
306+
zassert_true(realloc_check_block(p2, p2, 32), "data changed");
307+
realloc_fill_block(p3, 36);
308+
zassert_true(sys_heap_validate(&heap), "invalid heap");
309+
zassert_true(p1 != p3,
310+
"Realloc should have moved %p", p1);
311+
312+
/* Test realloc with increasing alignment */
313+
p1 = sys_heap_aligned_alloc(&heap, 32, 32);
314+
p2 = sys_heap_aligned_alloc(&heap, 8, 32);
315+
p3 = sys_heap_aligned_realloc(&heap, p2, 8, 16);
316+
zassert_true(sys_heap_validate(&heap), "invalid heap");
317+
zassert_true(p2 == p3,
318+
"Realloc should have expanded in place %p -> %p",
319+
p2, p3);
320+
p3 = sys_heap_aligned_alloc(&heap, 32, 8);
321+
zassert_true(sys_heap_validate(&heap), "invalid heap");
322+
zassert_true(p2 != p3,
323+
"Realloc should have moved %p", p2);
293324
}
294325

295326
void test_main(void)

0 commit comments

Comments
 (0)