Merge branch 'feature/multi_heap_assert' into 'master'

multi_heap: Print the problem address when aborting due to heap corruption

See merge request !1277
This commit is contained in:
Angus Gratton
2017-09-25 09:51:14 +08:00
4 changed files with 67 additions and 28 deletions
+32 -21
View File
@@ -144,12 +144,14 @@ static inline size_t block_data_size(const heap_block_t *block)
/* Check a block is valid for this heap. Used to verify parameters. */
static void assert_valid_block(const heap_t *heap, const heap_block_t *block)
{
assert(block >= &heap->first_block && block <= heap->last_block); /* block should be in heap */
MULTI_HEAP_ASSERT(block >= &heap->first_block && block <= heap->last_block,
block); // block not in heap
if (heap < (const heap_t *)heap->last_block) {
const heap_block_t *next = get_next_block(block);
assert(next >= &heap->first_block && next <= heap->last_block);
MULTI_HEAP_ASSERT(next >= &heap->first_block && next <= heap->last_block, block); // Next block not in heap
if (is_free(block)) {
assert(block->next_free >= &heap->first_block && block->next_free <= heap->last_block);
// Check block->next_free is valid
MULTI_HEAP_ASSERT(block->next_free >= &heap->first_block && block->next_free <= heap->last_block, &block->next_free);
}
}
}
@@ -168,10 +170,11 @@ static heap_block_t *get_prev_free_block(heap_t *heap, const heap_block_t *block
assert(block != &heap->first_block); /* can't look for a block before first_block */
for (heap_block_t *b = &heap->first_block; b != NULL && b < block; b = b->next_free) {
assert(is_free(b));
MULTI_HEAP_ASSERT(is_free(b), b); // Block should be free
if (b->next_free == NULL || b->next_free >= block) {
if (is_free(block)) {
assert(b->next_free == block); /* if block is on freelist, 'b' should be the item before it. */
/* if block is on freelist, 'b' should be the item before it. */
MULTI_HEAP_ASSERT(b->next_free == block, &b->next_free);
}
return b; /* b is the last free block before 'block' */
}
@@ -199,7 +202,7 @@ static heap_block_t *merge_adjacent(heap_t *heap, heap_block_t *a, heap_block_t
return b;
}
assert(get_next_block(a) == b);
MULTI_HEAP_ASSERT(get_next_block(a) == b, a); // Blocks should be in order
bool free = is_free(a) && is_free(b); /* merging two free blocks creates a free block */
if (!free && (is_free(a) || is_free(b))) {
@@ -208,18 +211,20 @@ static heap_block_t *merge_adjacent(heap_t *heap, heap_block_t *a, heap_block_t
*/
heap_block_t *free_block = is_free(a) ? a : b;
heap_block_t *prev_free = get_prev_free_block(heap, free_block);
assert(free_block->next_free > prev_free);
MULTI_HEAP_ASSERT(free_block->next_free > prev_free, &free_block->next_free); // Next free block should be after prev one
prev_free->next_free = free_block->next_free;
heap->free_bytes -= block_data_size(free_block);
}
a->header = b->header & NEXT_BLOCK_MASK;
assert(a->header != 0);
MULTI_HEAP_ASSERT(a->header != 0, a);
if (free) {
a->header |= BLOCK_FREE_FLAG;
assert(b->next_free == NULL || b->next_free > a);
assert(b->next_free == NULL || b->next_free > b);
if (b->next_free != NULL) {
MULTI_HEAP_ASSERT(b->next_free > a, &b->next_free);
MULTI_HEAP_ASSERT(b->next_free > b, &b->next_free);
}
a->next_free = b->next_free;
/* b's header can be put into the pool of free bytes */
@@ -245,8 +250,8 @@ static heap_block_t *merge_adjacent(heap_t *heap, heap_block_t *a, heap_block_t
*/
static void split_if_necessary(heap_t *heap, heap_block_t *block, size_t size, heap_block_t *prev_free_block)
{
assert(!is_free(block)); /* split_if_necessary doesn't expect a free block */
assert(size <= block_data_size(block)); /* can't grow a block this way! */
MULTI_HEAP_ASSERT(!is_free(block), block); // split block shouldn't be free
MULTI_HEAP_ASSERT(size <= block_data_size(block), block); // size should be valid
size = ALIGN_UP(size);
/* can't split the head or tail block */
@@ -266,7 +271,9 @@ static void split_if_necessary(heap_t *heap, heap_block_t *block, size_t size, h
if (prev_free_block == NULL) {
prev_free_block = get_prev_free_block(heap, block);
}
assert(prev_free_block->next_free > new_block); /* prev_free_block should point to a free block after new_block */
/* prev_free_block should point to a free block after new_block */
MULTI_HEAP_ASSERT(prev_free_block->next_free > new_block,
&prev_free_block->next_free); // free blocks should be in order
new_block->next_free = prev_free_block->next_free;
prev_free_block->next_free = new_block;
heap->free_bytes += block_data_size(new_block);
@@ -277,7 +284,7 @@ size_t multi_heap_get_allocated_size_impl(multi_heap_handle_t heap, void *p)
heap_block_t *pb = get_block(p);
assert_valid_block(heap, pb);
assert(!is_free(pb));
MULTI_HEAP_ASSERT(!is_free(pb), pb); // block should be free
return block_data_size(pb);
}
@@ -339,7 +346,8 @@ void *multi_heap_malloc_impl(multi_heap_handle_t heap, size_t size)
/* Find best free block to perform the allocation in */
prev = &heap->first_block;
for (heap_block_t *b = heap->first_block.next_free; b != NULL; b = b->next_free) {
assert(is_free(b));
MULTI_HEAP_ASSERT(b > prev, &prev->next_free); // free blocks should be ascending in address
MULTI_HEAP_ASSERT(is_free(b), b); // block should be free
size_t bs = block_data_size(b);
if (bs >= size && bs < best_size) {
best_block = b;
@@ -384,15 +392,16 @@ void multi_heap_free_impl(multi_heap_handle_t heap, void *p)
MULTI_HEAP_LOCK(heap->lock);
assert_valid_block(heap, pb);
assert(!is_free(pb));
assert(!is_last_block(pb));
assert(pb != &heap->first_block);
MULTI_HEAP_ASSERT(!is_free(pb), pb); // block should not be free
MULTI_HEAP_ASSERT(!is_last_block(pb), pb); // block should not be last block
MULTI_HEAP_ASSERT(pb != &heap->first_block, pb); // block should not be first block
heap_block_t *next = get_next_block(pb);
/* Update freelist pointers */
heap_block_t *prev_free = get_prev_free_block(heap, pb);
assert(prev_free->next_free == NULL || prev_free->next_free > pb);
// freelist validity check
MULTI_HEAP_ASSERT(prev_free->next_free == NULL || prev_free->next_free > pb, &prev_free->next_free);
pb->next_free = prev_free->next_free;
prev_free->next_free = pb;
@@ -428,7 +437,8 @@ void *multi_heap_realloc_impl(multi_heap_handle_t heap, void *p, size_t size)
}
assert_valid_block(heap, pb);
assert(!is_free(pb) && "realloc arg should be allocated");
// non-null realloc arg should be allocated
MULTI_HEAP_ASSERT(!is_free(pb), pb);
if (size == 0) {
/* note: calling multi_free_impl() here as we've already been
@@ -646,7 +656,8 @@ void multi_heap_get_info_impl(multi_heap_handle_t heap, multi_heap_info_t *info)
}
info->minimum_free_bytes = heap->minimum_free_bytes;
assert(info->total_free_bytes == heap->free_bytes);
// heap has wrong total size (address printed here is not indicative of the real error)
MULTI_HEAP_ASSERT(info->total_free_bytes == heap->free_bytes, heap);
MULTI_HEAP_UNLOCK(heap->lock);
+27 -1
View File
@@ -18,6 +18,7 @@
#include <freertos/FreeRTOS.h>
#include <freertos/task.h>
#include <rom/ets_sys.h>
#include <assert.h>
/* Because malloc/free can happen inside an ISR context,
we need to use portmux spinlocks here not RTOS mutexes */
@@ -40,11 +41,36 @@
#define MULTI_HEAP_PRINTF ets_printf
#define MULTI_HEAP_STDERR_PRINTF(MSG, ...) ets_printf(MSG, __VA_ARGS__)
#else
inline static void multi_heap_assert(bool condition, const char *format, int line, intptr_t address)
{
/* Can't use libc assert() here as it calls printf() which can cause another malloc() for a newlib lock.
Also, it's useful to be able to print the memory address where corruption was detected.
*/
#ifndef NDEBUG
if(!condition) {
#ifndef CONFIG_OPTIMIZATION_ASSERTIONS_SILENT
ets_printf(format, line, address);
#endif // CONFIG_OPTIMIZATION_ASSERTIONS_SILENT
abort();
}
#else // NDEBUG
(void) condition;
#endif // NDEBUG
}
#define MULTI_HEAP_ASSERT(CONDITION, ADDRESS) \
multi_heap_assert((CONDITION), "CORRUPT HEAP: multi_heap.c:%d detected at 0x%08x\n", \
__LINE__, (intptr_t)(ADDRESS))
#else // ESP_PLATFORM
#include <assert.h>
#define MULTI_HEAP_PRINTF printf
#define MULTI_HEAP_STDERR_PRINTF(MSG, ...) fprintf(stderr, MSG, __VA_ARGS__)
#define MULTI_HEAP_LOCK(PLOCK)
#define MULTI_HEAP_UNLOCK(PLOCK)
#define MULTI_HEAP_ASSERT(CONDITION, ADDRESS) assert((CONDITION) && "Heap corrupt")
#endif
+4 -4
View File
@@ -92,7 +92,7 @@ static poison_head_t *verify_allocated_region(void *data, bool print_errors)
/* check if the beginning of the data was overwritten */
if (head->head_canary != HEAD_CANARY_PATTERN) {
if (print_errors) {
printf("CORRUPT HEAP: Bad head at %p. Expected 0x%08x got 0x%08x\n", &head->head_canary,
MULTI_HEAP_STDERR_PRINTF("CORRUPT HEAP: Bad head at %p. Expected 0x%08x got 0x%08x\n", &head->head_canary,
HEAD_CANARY_PATTERN, head->head_canary);
}
return NULL;
@@ -142,7 +142,7 @@ static bool verify_fill_pattern(void *data, size_t size, bool print_errors, bool
while (size >= 4) {
if (*p != EXPECT_WORD) {
if (print_errors) {
printf("Invalid data at %p. Expected 0x%08x got 0x%08x\n", p, EXPECT_WORD, *p);
MULTI_HEAP_STDERR_PRINTF("CORRUPT HEAP: Invalid data at %p. Expected 0x%08x got 0x%08x\n", p, EXPECT_WORD, *p);
}
valid = false;
}
@@ -159,7 +159,7 @@ static bool verify_fill_pattern(void *data, size_t size, bool print_errors, bool
for (int i = 0; i < size; i++) {
if (p[i] != (uint8_t)EXPECT_WORD) {
if (print_errors) {
printf("Invalid data at %p. Expected 0x%02x got 0x%02x\n", p, (uint8_t)EXPECT_WORD, *p);
MULTI_HEAP_STDERR_PRINTF("CORRUPT HEAP: Invalid data at %p. Expected 0x%02x got 0x%02x\n", p, (uint8_t)EXPECT_WORD, *p);
}
valid = false;
}
@@ -315,7 +315,7 @@ bool multi_heap_internal_check_block_poisoning(void *start, size_t size, bool is
/* block can be bigger than alloc_size, for reasons of alignment & fragmentation,
but block can never be smaller than head->alloc_size... */
if (print_errors) {
printf("CORRUPT HEAP: Size at %p expected <=0x%08x got 0x%08x\n", &head->alloc_size,
MULTI_HEAP_STDERR_PRINTF("CORRUPT HEAP: Size at %p expected <=0x%08x got 0x%08x\n", &head->alloc_size,
size - POISON_OVERHEAD, head->alloc_size);
}
return false;