Improve caller address reporting to indicate address in

mhightower83 · mhightower83 · commit 608814706cb1 · 2022-11-21T10:14:05.000-08:00
sketch or library.

Enhanced DEBUG_ESP_OOM to track OOM events when
C++ Exceptions: "enabled" is selected. Report more interesting
higher level code address as caller instead of GCC C++ module.

Enhanced poison neighbor reporting to distinguish between allocation
vs. neighbor failing poison check.

For draft version, added debug macro HEAP_DEBUG_PROBE_PSFLC_CB
to aid in evaluating caller address results.
diff --git a/cores/esp8266/abi.cpp b/cores/esp8266/abi.cpp
@@ -24,12 +24,46 @@
 using __cxxabiv1::__guard;
 
 // Debugging helper, last allocation which returned NULL
-extern "C" void *_heap_abi_malloc(size_t size, bool unhandle, const void* const caller);
+extern "C" void *_heap_abi_malloc(size_t size, bool unhandled, const void* const caller);
 
 extern "C" void __cxa_pure_virtual(void) __attribute__ ((__noreturn__));
 extern "C" void __cxa_deleted_virtual(void) __attribute__ ((__noreturn__));
 
-#if !defined(__cpp_exceptions)
+#if defined(__cpp_exceptions) && (defined(DEBUG_ESP_OOM) || defined(DEBUG_ESP_PORT))
+/*
+  When built with C++ Exceptions: "enabled", track caller address of Last OOM.
+  * For debug build, force enable Last OOM tracking.
+  * With the option "DEBUG_ESP_OOM," always do Last OOM tracking.
+  * Otherwise, disable Last OOM tracking. The build relies on the weak link to
+    the default C++ exception handler.
+*/
+
+// Debug replacement adaptation from ".../new_op.cc".
+using std::new_handler;
+using std::bad_alloc;
+
+void * operator new (std::size_t size)
+{
+    void *p;
+
+    /* malloc (0) is unpredictable; avoid it.  */
+    if (__builtin_expect(size == 0, false)) {
+        size = 1;
+    }
+
+    while (0 == (p = _heap_abi_malloc(size, false, __builtin_return_address(0)))) {
+        new_handler handler = std::get_new_handler();
+        if (!handler) {
+            throw(bad_alloc());
+        }
+        handler();
+    }
+
+    return p;
+}
+#elif !defined(__cpp_exceptions)
+// When doing builds with C++ Exceptions "disabled", always save details of
+// the last OOM event.
 
 // overwrite weak operators new/new[] definitions
 
diff --git a/cores/esp8266/heap.cpp b/cores/esp8266/heap.cpp
@@ -45,7 +45,8 @@
  *    tested. This option is enabled when Tools->Debug: Serial is selected or
  *    Tools->Debug level: "CORE" is selected. While coverage is not 100%, a
  *    sketch is less likely to have strange behavior from heavy heap access with
- *    interrupts disabled.
+ *    interrupts disabled. If needed, continue reading "UMM_POISON_CHECK" for
+ *    more details.
  *
  *  * UMM_POISON_CHECK - Adds and presets 4 bytes of poison at the beginning and
  *    end of each allocation. For each Heap API call, a complete sweep through
@@ -101,6 +102,8 @@ extern "C" void z2EapFree(void *ptr, const char* file, int line) __attribute__((
 #include <sys/reent.h>
 #include <user_interface.h>
 
+#include "heap_cb.h"
+
 extern "C" {
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -133,7 +136,7 @@ extern "C" {
 #undef realloc
 #undef free
 
-#elif defined(DEBUG_ESP_OOM) || defined(UMM_INTEGRITY_CHECK)
+#elif defined(DEBUG_ESP_OOM) || defined(UMM_INTEGRITY_CHECK) || defined(HEAP_DEBUG_PROBE_PSFLC_CB)
 // All other debug wrappers that do not require handling poison
 #define UMM_MALLOC_FL(s,f,l,c)    umm_malloc(s)
 #define UMM_CALLOC_FL(n,s,f,l,c)  umm_calloc(n,s)
@@ -197,10 +200,11 @@ extern "C" {
 
 
 ///////////////////////////////////////////////////////////////////////////////
-// OOM - this structure variable is always in use by abi.cpp
-//
-// Always track last failed caller and size requested
+// OOM - this structure variable is always in use by abi.cpp - except for
+// C++ Exceptions "enabled"  builds.
 //
+// When building with C++ Exceptions "disabled" or debug build,
+// always track last failed caller and size requested
 #if defined(DEBUG_ESP_OOM)
 struct umm_last_fail_alloc {
     const void *addr;
@@ -210,6 +214,8 @@ struct umm_last_fail_alloc {
 } _umm_last_fail_alloc = {NULL, 0, NULL, 0};
 
 #else
+// Note for the least used case "(defined(__cpp_exceptions) &&
+// !defined(DEBUG_ESP_OOM))", we only capture details for LIBC calls.
 struct umm_last_fail_alloc {
     const void *addr;
     size_t size;
@@ -268,6 +274,7 @@ static bool IRAM_ATTR oom_check__log_last_fail_atomic_psflc(void *ptr, size_t si
         _umm_last_fail_alloc.line = line;
         print_loc(size, file, line, caller);
         xt_wsr_ps(saved_ps);
+        _HEAP_DEBUG_PROBE_PSFLC_CB(heap_oom_cb_id, ptr, size, file, line, caller);
         return false;
     }
     return true;
@@ -276,18 +283,19 @@ static bool IRAM_ATTR oom_check__log_last_fail_atomic_psflc(void *ptr, size_t si
 #define OOM_CHECK__LOG_LAST_FAIL_LITE_FL(p, s, f, l, c) ({ (void)p, (void)s, (void)f; (void)l; (void)c; true; })
 
 #elif defined(ENABLE_THICK_DEBUG_WRAPPERS)
-static bool IRAM_ATTR oom_check__log_last_fail_psc(void *ptr, size_t size, const void* caller) {
+static bool IRAM_ATTR oom_check__log_last_fail_atomic_psc(void *ptr, size_t size, const void* caller) {
     if (0 != (size) && 0 == ptr) {
         // Need to ensure changes to umm_last_fail_alloc are atomic.
         uint32_t saved_ps = xt_rsil(DEFAULT_CRITICAL_SECTION_INTLEVEL);
         _umm_last_fail_alloc.addr = caller;
         _umm_last_fail_alloc.size = size;
         xt_wsr_ps(saved_ps);
+        _HEAP_DEBUG_PROBE_PSFLC_CB(heap_oom_cb_id, ptr, size, NULL, 0, caller);
         return false;
     }
     return true;
 }
-#define OOM_CHECK__LOG_LAST_FAIL_FL(p, s, f, l, c) oom_check__log_last_fail_psc(p, s, c)
+#define OOM_CHECK__LOG_LAST_FAIL_FL(p, s, f, l, c) oom_check__log_last_fail_atomic_psc(p, s, c)
 #define OOM_CHECK__LOG_LAST_FAIL_LITE_FL(p, s, f, l, c) ({ (void)p, (void)s, (void)f; (void)l; (void)c; true; })
 
 #else
@@ -298,6 +306,7 @@ static bool oom_check__log_last_fail_psc(void *ptr, size_t size, const void* cal
     if (0 != (size) && 0 == ptr) {
         _umm_last_fail_alloc.addr = caller;
         _umm_last_fail_alloc.size = size;
+        _HEAP_DEBUG_PROBE_PSFLC_CB(heap_oom_cb_id, ptr, size, NULL, 0, caller);
         return false;
     }
     return true;
@@ -356,6 +365,7 @@ void* IRAM_ATTR _heap_pvPortMalloc(size_t size, const char* file, int line, cons
     INTEGRITY_CHECK__PANIC_FL(file, line, caller);
     POISON_CHECK__PANIC_FL(file, line, caller);
     void* ret = UMM_MALLOC_FL(size, file, line, caller);
+    ret = _HEAP_DEBUG_PROBE_PSFLC_CB(heap_malloc_cb_id, ret, size, file, line, caller);
     OOM_CHECK__LOG_LAST_FAIL_FL(ret, size, file, line, caller);
     return ret;
 }
@@ -366,14 +376,17 @@ void* IRAM_ATTR _heap_pvPortCalloc(size_t count, size_t size, const char* file,
     POISON_CHECK__PANIC_FL(file, line, caller);
     size_t total_size = umm_umul_sat(count, size);
     void* ret = UMM_CALLOC_FL(1, total_size, file, line, caller);
+    ret = _HEAP_DEBUG_PROBE_PSFLC_CB(heap_calloc_cb_id, ret, size, file, line, caller);
     OOM_CHECK__LOG_LAST_FAIL_FL(ret, total_size, file, line, caller);
     return ret;
 }
 
 void* IRAM_ATTR _heap_pvPortRealloc(void *ptr, size_t size, const char* file, int line, const void *caller)
 {
     INTEGRITY_CHECK__PANIC_FL(file, line, caller);
+    ptr = _HEAP_DEBUG_PROBE_PSFLC_CB(heap_realloc_in_cb_id, ptr, size, file, line, caller);
     void* ret = UMM_REALLOC_FL(ptr, size, file, line, caller);
+    ret = _HEAP_DEBUG_PROBE_PSFLC_CB(heap_realloc_out_cb_id, ret, size, file, line, caller);
     POISON_CHECK__PANIC_FL(file, line, caller);
     OOM_CHECK__LOG_LAST_FAIL_FL(ret, size, file, line, caller);
     return ret;
@@ -382,6 +395,7 @@ void* IRAM_ATTR _heap_pvPortRealloc(void *ptr, size_t size, const char* file, in
 void IRAM_ATTR _heap_vPortFree(void *ptr, const char* file, int line, [[maybe_unused]] const void *caller)
 {
     INTEGRITY_CHECK__PANIC_FL(file, line, caller);
+    ptr = _HEAP_DEBUG_PROBE_PSFLC_CB(heap_free_cb_id, ptr, 0, file, line, caller);
     UMM_FREE_FL(ptr, file, line, caller);
     POISON_CHECK__PANIC_FL(file, line, caller);
 }
@@ -579,10 +593,10 @@ void system_show_malloc(void)
 #endif
 }
 
-#if !defined(__cpp_exceptions)
+#if defined(DEBUG_ESP_OOM) || !defined(__cpp_exceptions) || defined(DEBUG_ESP_PORT)
 ///////////////////////////////////////////////////////////////////////////////
 // heap allocator for "new" (ABI) - To support collecting OOM info, always defined
-void* _heap_abi_malloc(size_t size, bool unhandle, const void* caller)
+void* _heap_abi_malloc(size_t size, bool unhandled, const void* caller)
 {
     [[maybe_unused]] const char *file = NULL;
     [[maybe_unused]] const int line = 0;
@@ -591,16 +605,15 @@ void* _heap_abi_malloc(size_t size, bool unhandle, const void* caller)
     INTEGRITY_CHECK__PANIC_FL(file, line, caller);
     POISON_CHECK__PANIC_FL(file, line, caller);
     void* ret = UMM_MALLOC_FL(size, file, line, caller);
-    if (!OOM_CHECK__LOG_LAST_FAIL_FL(ret, size, file, line, caller) && unhandle) {
-        __unhandled_exception(PSTR("OOM"));
-    }
+    bool ok = OOM_CHECK__LOG_LAST_FAIL_FL(ret, size, file, line, caller);
     #else
     void* ret = UMM_MALLOC(size);
-    // Always do some level of OOM check
-    if (!OOM_CHECK__LOG_LAST_FAIL_LITE_FL(ret, size, file, line, caller) && unhandle) {
+    // minimum OOM check
+    bool ok = OOM_CHECK__LOG_LAST_FAIL_LITE_FL(ret, size, file, line, caller)
+    #endif
+    if (!ok && unhandled) {
         __unhandled_exception(PSTR("OOM"));
     }
-    #endif
     return ret;
 }
 #endif
@@ -617,23 +630,7 @@ void* _heap_abi_malloc(size_t size, bool unhandle, const void* caller)
 // Replacement C++ delete operator to capture callers address
 //
 #include <bits/c++config.h>
-
-#if !_GLIBCXX_HOSTED
-// A freestanding C runtime may not provide "free" -- but there is no
-// other reasonable way to implement "operator delete".
-namespace std
-{
-_GLIBCXX_BEGIN_NAMESPACE_VERSION
-  extern "C" void free(void*);
-_GLIBCXX_END_NAMESPACE_VERSION
-} // namespace
-#pragma message("!_GLIBCXX_HOSTED")
-#else
-// #pragma message("_GLIBCXX_HOSTED")
-// This is the path taken
 #include <cstdlib>
-#endif
-
 #include "new"
 
 // The sized deletes are defined in other files.
diff --git a/cores/esp8266/umm_malloc/umm_local.c b/cores/esp8266/umm_malloc/umm_local.c
@@ -86,6 +86,7 @@ static bool check_poison_neighbors(umm_heap_context_t *_context, uint16_t cur) {
 #if defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE)
 
 /* ------------------------------------------------------------------------ */
+#include "heap_cb.h"
 
 static void *get_unpoisoned_check_neighbors(const void *vptr, const char *file, int line, const void *caller) {
     uintptr_t ptr = (uintptr_t)vptr;
@@ -97,24 +98,34 @@ static void *get_unpoisoned_check_neighbors(const void *vptr, const char *file,
         #if defined(UMM_POISON_CHECK_LITE)
         UMM_CRITICAL_DECL(id_poison);
         uint16_t c;
-        bool poison = false;
+        bool poison = true;
         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);
+            if (! check_poison_block(&UMM_BLOCK(c))) {
+                DBGLOG_ERROR("Allocation address %p\n", vptr);
+                size_t size = *(size_t *)ptr;
+                _HEAP_DEBUG_PROBE_PSFLC_CB(heap_poison_lite_cb_id, (void *)ptr, size, file, line, caller);
+                poison = false;
+            } else
+            if (! check_poison_neighbors(_context, c)) {
+                DBGLOG_ERROR("This bad block is in a neighbor allocation near: %p\n", vptr);
+                _HEAP_DEBUG_PROBE_PSFLC_CB(heap_poison_lite_neighbor_cb_id, (void *)ptr, 0, file, line, caller);
+                poison = false;
+            }
             UMM_CRITICAL_EXIT(id_poison);
         } else {
             DBGLOG_ERROR("\nPointer %p is not a Heap address.\n", vptr);
+            _HEAP_DEBUG_PROBE_PSFLC_CB(heap_poison_lite_addr_cb_id, (void *)ptr, 0, file, line, caller);
+            poison = false;
         }
 
         if (!poison) {
-            DBGLOG_ERROR("called from %p\n", caller);
+            DBGLOG_ERROR("Called from %p\n", caller);
             if (file) {
                 __panic_func(file, line, "");
             } else {
diff --git a/cores/esp8266/umm_malloc/umm_local.h b/cores/esp8266/umm_malloc/umm_local.h
@@ -34,9 +34,10 @@ static size_t umm_uadd_sat(const size_t a, const size_t b);
 // #define DBGLOG_FORCE(force, format, ...) {if(force) {::printf(PSTR(format), ## __VA_ARGS__);}}
 
 
-#if defined(DEBUG_ESP_OOM) || defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE) || defined(UMM_INTEGRITY_CHECK)
+#if defined(DEBUG_ESP_OOM) || defined(UMM_POISON_CHECK) \
+|| defined(UMM_POISON_CHECK_LITE) || defined(UMM_INTEGRITY_CHECK) \
+|| defined(HEAP_DEBUG_PROBE_PSFLC_CB) || defined(_HEAP_DEBUG_PROBE_PSFLC_CB)
 #else
-
 #define umm_malloc(s)    malloc(s)
 #define umm_calloc(n,s)  calloc(n,s)
 #define umm_realloc(p,s) realloc(p,s)
