Crash_Override/OVMS3-idf
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

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

Browse source 
New multi_heap code has proven effective at aborting when buffer overruns occur,
but it's currently hard to debug the stack traces from these failures.
This commit is contained in:
Angus Gratton 2017-09-18 14:51:51 +10:00 committed by Angus Gratton
parent 9a1ba5985b
commit 76d8190444
3 changed files with 63 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

53
components/heap/multi_heap.c
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. */ /* 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) 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) { if (heap < (const heap_t *)heap->last_block) {
const heap_block_t *next = get_next_block(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)) { 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 */ 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) { 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 (b->next_free == NULL || b->next_free >= block) {
if (is_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' */ 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; 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 */ bool free = is_free(a) && is_free(b); /* merging two free blocks creates a free block */
if (!free && (is_free(a) || is_free(b))) { 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 *free_block = is_free(a) ? a : b;
heap_block_t *prev_free = get_prev_free_block(heap, free_block); 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; prev_free->next_free = free_block->next_free;
heap->free_bytes -= block_data_size(free_block); heap->free_bytes -= block_data_size(free_block);
} }
a->header = b->header & NEXT_BLOCK_MASK; a->header = b->header & NEXT_BLOCK_MASK;
assert(a->header != 0); MULTI_HEAP_ASSERT(a->header != 0, a);
if (free) { if (free) {
a->header |= BLOCK_FREE_FLAG; a->header |= BLOCK_FREE_FLAG;
assert(b->next_free == NULL || b->next_free > a); if (b->next_free != NULL) {
assert(b->next_free == NULL || b->next_free > b); 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; a->next_free = b->next_free;
/* b's header can be put into the pool of free bytes */ /* 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) 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 */ MULTI_HEAP_ASSERT(!is_free(block), block); // split block shouldn't be free
assert(size <= block_data_size(block)); /* can't grow a block this way! */ MULTI_HEAP_ASSERT(size <= block_data_size(block), block); // size should be valid
size = ALIGN_UP(size); size = ALIGN_UP(size);
/* can't split the head or tail block */ /* 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) { if (prev_free_block == NULL) {
prev_free_block = get_prev_free_block(heap, block); 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; new_block->next_free = prev_free_block->next_free;
prev_free_block->next_free = new_block; prev_free_block->next_free = new_block;
heap->free_bytes += block_data_size(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); heap_block_t *pb = get_block(p);
assert_valid_block(heap, pb); 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); 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 */ /* Find best free block to perform the allocation in */
prev = &heap->first_block; prev = &heap->first_block;
for (heap_block_t *b = heap->first_block.next_free; b != NULL; b = b->next_free) { 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); size_t bs = block_data_size(b);
if (bs >= size && bs < best_size) { if (bs >= size && bs < best_size) {
best_block = b; best_block = b;
@ -384,15 +392,16 @@ void multi_heap_free_impl(multi_heap_handle_t heap, void *p)
MULTI_HEAP_LOCK(heap->lock); MULTI_HEAP_LOCK(heap->lock);
assert_valid_block(heap, pb); assert_valid_block(heap, pb);
assert(!is_free(pb)); MULTI_HEAP_ASSERT(!is_free(pb), pb); // block should not be free
assert(!is_last_block(pb)); MULTI_HEAP_ASSERT(!is_last_block(pb), pb); // block should not be last block
assert(pb != &heap->first_block); MULTI_HEAP_ASSERT(pb != &heap->first_block, pb); // block should not be first block
heap_block_t *next = get_next_block(pb); heap_block_t *next = get_next_block(pb);
/* Update freelist pointers */ /* Update freelist pointers */
heap_block_t *prev_free = get_prev_free_block(heap, pb); 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; pb->next_free = prev_free->next_free;
prev_free->next_free = pb; 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_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) { if (size == 0) {
/* note: calling multi_free_impl() here as we've already been /* 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; 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); MULTI_HEAP_UNLOCK(heap->lock);

28
components/heap/multi_heap_platform.h
View file

@ -18,6 +18,7 @@
#include <freertos/FreeRTOS.h> #include <freertos/FreeRTOS.h>
#include <freertos/task.h> #include <freertos/task.h>
#include <rom/ets_sys.h> #include <rom/ets_sys.h>
#include <assert.h>
/* Because malloc/free can happen inside an ISR context, /* Because malloc/free can happen inside an ISR context,
we need to use portmux spinlocks here not RTOS mutexes */ we need to use portmux spinlocks here not RTOS mutexes */
@ -40,11 +41,36 @@
#define MULTI_HEAP_PRINTF ets_printf #define MULTI_HEAP_PRINTF ets_printf
#define MULTI_HEAP_STDERR_PRINTF(MSG, ...) ets_printf(MSG, __VA_ARGS__) #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_PRINTF printf
#define MULTI_HEAP_STDERR_PRINTF(MSG, ...) fprintf(stderr, MSG, __VA_ARGS__) #define MULTI_HEAP_STDERR_PRINTF(MSG, ...) fprintf(stderr, MSG, __VA_ARGS__)
#define MULTI_HEAP_LOCK(PLOCK) #define MULTI_HEAP_LOCK(PLOCK)
#define MULTI_HEAP_UNLOCK(PLOCK) #define MULTI_HEAP_UNLOCK(PLOCK)
#define MULTI_HEAP_ASSERT(CONDITION, ADDRESS) assert((CONDITION) && "Heap corrupt")
#endif #endif

6
docs/api-reference/system/heap_debug.rst
View file

@ -38,7 +38,9 @@ Assertions
The heap implementation (``multi_heap.c``, etc.) includes a lot of assertions which will fail if the heap memory is corrupted. To detect heap corruption most effectively, ensure that assertions are enabled in ``make menuconfig`` under ``Compiler options``. The heap implementation (``multi_heap.c``, etc.) includes a lot of assertions which will fail if the heap memory is corrupted. To detect heap corruption most effectively, ensure that assertions are enabled in ``make menuconfig`` under ``Compiler options``.
It's also possible to manually check heap integrity by calling the ``heap_caps_check_integrity()`` function (see below). This function checks all of requested heap memory for integrity, and can be used even if assertions are disabled. If a heap integrity assertion fails, a line will be printed like ``CORRUPT HEAP: multi_heap.c:225 detected at 0x3ffbb71c``. The memory address which is printed is the address of the heap structure which has corrupt content.
It's also possible to manually check heap integrity by calling the ``heap_caps_check_integrity()`` function (see below). This function checks all of requested heap memory for integrity, and can be used even if assertions are disabled. If the integrity check prints an error, it will contain the address(es) of corrupt heap structures.
Configuration Configuration
^^^^^^^^^^^^^ ^^^^^^^^^^^^^
@ -81,7 +83,7 @@ Finding Heap Corruption
Memory corruption can be one of the hardest classes of bugs to find and fix, as one area of memory can be corrupted from a totally different place. Some tips: Memory corruption can be one of the hardest classes of bugs to find and fix, as one area of memory can be corrupted from a totally different place. Some tips:
- If you can find the address (in memory) which is being corrupted, you can set a watchpoint on this address via JTAG to have the CPU halt when it is written to. - Once you know the address (in memory) which is being corrupted, you can set a watchpoint on this address via JTAG to have the CPU halt when it is written to.
- If you don't have JTAG, but you do know roughly when the corruption happens, then you can set a watchpoint in software. A fatal exception will occur when the watchpoint triggers. For example ``esp_set_watchpoint(0, (void *)addr, 4, ESP_WATCHPOINT_STORE``. Note that the watchpoint is set on the current running CPU only, so if you don't know which CPU is corrupting memory then you will need to call this function on both CPUs. - If you don't have JTAG, but you do know roughly when the corruption happens, then you can set a watchpoint in software. A fatal exception will occur when the watchpoint triggers. For example ``esp_set_watchpoint(0, (void *)addr, 4, ESP_WATCHPOINT_STORE``. Note that the watchpoint is set on the current running CPU only, so if you don't know which CPU is corrupting memory then you will need to call this function on both CPUs.
- For buffer overflows, `heap tracing`_ in ``HEAP_TRACE_ALL`` mode lets you see which callers allocate the memory address(es) immediately before the address which is being corrupted. There is a strong chance this is the code which overflows the buffer. - For buffer overflows, `heap tracing`_ in ``HEAP_TRACE_ALL`` mode lets you see which callers allocate the memory address(es) immediately before the address which is being corrupted. There is a strong chance this is the code which overflows the buffer.