Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Heap panic / abort cleanup #8465

Merged
merged 5 commits into from
Mar 31, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions cores/esp8266/umm_malloc/umm_local.c
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,20 @@ static void *get_unpoisoned_check_neighbors(void *vptr, const char *file, int li
UMM_CRITICAL_DECL(id_poison);
uint16_t c;
bool poison = false;
umm_heap_context_t *_context = umm_get_ptr_context(vptr);
if (NULL == _context) {
panic();
return NULL;
umm_heap_context_t *_context = _umm_get_ptr_context((void *)ptr);
if (_context) {

/* Figure out which block we're in. Note the use of truncated division... */
c = (ptr - (uintptr_t)(&(_context->heap[0]))) / sizeof(umm_block);

UMM_CRITICAL_ENTRY(id_poison);
poison =
check_poison_block(&UMM_BLOCK(c)) &&
check_poison_neighbors(_context, c);
UMM_CRITICAL_EXIT(id_poison);
} else {
DBGLOG_ERROR("\nPointer %p is not a Heap address.\n", vptr);
}
/* Figure out which block we're in. Note the use of truncated division... */
c = (ptr - (uintptr_t)(&(_context->heap[0]))) / sizeof(umm_block);

UMM_CRITICAL_ENTRY(id_poison);
poison = check_poison_block(&UMM_BLOCK(c)) && check_poison_neighbors(_context, c);
UMM_CRITICAL_EXIT(id_poison);

if (!poison) {
if (file) {
Expand Down
56 changes: 40 additions & 16 deletions cores/esp8266/umm_malloc/umm_malloc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,19 @@ extern "C" {
// C extern void *UMM_MALLOC_CFG_HEAP_ADDR;
// C extern uint32_t UMM_MALLOC_CFG_HEAP_SIZE;

#if 0 // Must be zero at release
#warning "Macro NON_NULL_CONTEXT_ASSERT() is active!"
/*
* Keep for future debug/maintenance of umm_malloc. Not needed in a
* regular/debug build. Call paths that use NON_NULL_CONTEXT_ASSERT logically
* guard against returning NULL. This macro double-checks that assumption during
* development.
*/
#define NON_NULL_CONTEXT_ASSERT() assert((NULL != _context))
#else
#define NON_NULL_CONTEXT_ASSERT() (void)0
#endif

#include "umm_local.h" // target-dependent supplemental

/* ------------------------------------------------------------------------- */
Expand Down Expand Up @@ -212,21 +225,41 @@ int umm_get_heap_stack_index(void) {
* realloc or free since you may not be in the right heap to handle it.
*
*/
static bool test_ptr_context(size_t which, void *ptr) {
static bool test_ptr_context(const size_t which, const void *const ptr) {
return
heap_context[which].heap &&
ptr >= (void *)heap_context[which].heap &&
ptr < heap_context[which].heap_end;
}

static umm_heap_context_t *umm_get_ptr_context(void *ptr) {
/*
* Find Heap context by allocation address - may return NULL
*/
umm_heap_context_t *_umm_get_ptr_context(const void *const ptr) {
for (size_t i = 0; i < UMM_NUM_HEAPS; i++) {
if (test_ptr_context(i, ptr)) {
return umm_get_heap_by_id(i);
}
}

panic();
return NULL;
}

/*
* Find Heap context by allocation address - must either succeed or abort
*/
static umm_heap_context_t *umm_get_ptr_context(const void *const ptr) {
umm_heap_context_t *const _context = _umm_get_ptr_context(ptr);
if (_context) {
return _context;
}

[[maybe_unused]] uintptr_t sketch_ptr = (uintptr_t)ptr;
#if defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE)
sketch_ptr += sizeof(UMM_POISONED_BLOCK_LEN_TYPE) + UMM_POISON_SIZE_BEFORE;
#endif
DBGLOG_ERROR("\nPointer %p is not a Heap address.\n", (void *)sketch_ptr);
abort();
return NULL;
}

Expand Down Expand Up @@ -556,10 +589,7 @@ static void umm_free_core(umm_heap_context_t *_context, void *ptr) {

uint16_t c;

if (NULL == _context) {
panic();
return;
}
NON_NULL_CONTEXT_ASSERT();

STATS__FREE_REQUEST(id_free);
/*
Expand Down Expand Up @@ -649,12 +679,9 @@ static void *umm_malloc_core(umm_heap_context_t *_context, size_t size) {

uint16_t cf;

STATS__ALLOC_REQUEST(id_malloc, size);
NON_NULL_CONTEXT_ASSERT();

if (NULL == _context) {
panic();
return NULL;
}
STATS__ALLOC_REQUEST(id_malloc, size);

blocks = umm_blocks(size);

Expand Down Expand Up @@ -902,10 +929,7 @@ void *umm_realloc(void *ptr, size_t size) {

/* Need to be in the heap in which this block lives */
umm_heap_context_t *_context = umm_get_ptr_context(ptr);
if (NULL == _context) {
panic();
return NULL;
}
NON_NULL_CONTEXT_ASSERT();

if (0 == size) {
DBGLOG_DEBUG("realloc to 0 size, just free the block\n");
Expand Down
2 changes: 1 addition & 1 deletion cores/esp8266/umm_malloc/umm_malloc_cfg.h
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,7 @@ void IRAM_ATTR heap_vPortFree(void *ptr, const char *file, int line);
#define dbg_heap_free(p) free(p)
#endif

#elif defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE)
#elif defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE) // #elif for #ifdef DEBUG_ESP_OOM
#include <pgmspace.h>
void *IRAM_ATTR heap_pvPortRealloc(void *ptr, size_t size, const char *file, int line);
#define realloc(p,s) ({ static const char mem_debug_file[] PROGMEM STORE_ATTR = __FILE__; heap_pvPortRealloc(p, s, mem_debug_file, __LINE__); })
Expand Down
6 changes: 2 additions & 4 deletions cores/esp8266/umm_malloc/umm_poison.c
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,8 @@ static void *get_unpoisoned(void *vptr) {
ptr -= (sizeof(UMM_POISONED_BLOCK_LEN_TYPE) + UMM_POISON_SIZE_BEFORE);

umm_heap_context_t *_context = umm_get_ptr_context(vptr);
if (NULL == _context) {
panic();
return NULL;
}
NON_NULL_CONTEXT_ASSERT();

/* Figure out which block we're in. Note the use of truncated division... */
c = (ptr - (uintptr_t)(&(_context->heap[0]))) / sizeof(umm_block);

Expand Down