[Stack Mirror][v4.4] make corrupt backtraces not happen nearly as often

CMakeLists.txt

@@ -28,6 +28,10 @@ if(NOT BOOTLOADER_BUILD)
         list(APPEND compile_options "-O2")
     endif()
 
+    if(CONFIG_COMPILER_STACK_MIRROR)
+        list(APPEND compile_options "-finstrument-functions")
+    endif()
+
 else()  # BOOTLOADER_BUILD
 
     if(CONFIG_BOOTLOADER_COMPILER_OPTIMIZATION_SIZE)

Kconfig

@@ -380,6 +380,22 @@ mainmenu "Espressif IoT Development Framework Configuration"
             help
                 Stack smashing protection.
 
+        config COMPILER_STACK_MIRROR
+            prompt "Enable Stack Mirror"
+            bool
+            default "n"
+            help
+                Make corrupted backtraces much less likely by creating a dupliate copy of stack return 
+                addresses in heap memory. Requires about 100 bytes of memory per thread.
+
+        config COMPILER_STACK_MIRROR_DEPTH
+            prompt "Stack Mirror Depth"
+            int
+            default 26
+            depends on COMPILER_STACK_MIRROR
+            help
+                The maximum depth of the stack mirror. 4 bytes are needed per depth.
+
         config COMPILER_WARN_WRITE_STRINGS
             bool "Enable -Wwrite-strings warning flag"
             default "n"

components/esp_hw_support/include/soc/spinlock.h

@@ -64,6 +64,7 @@ static inline void __attribute__((always_inline)) spinlock_initialize(spinlock_t
  * @param lock - target spinlock object
  * @param timeout - cycles to wait, passing SPINLOCK_WAIT_FOREVER blocs indefinitely
  */
+
 static inline bool __attribute__((always_inline)) spinlock_acquire(spinlock_t *lock, int32_t timeout)
 {
 #if !CONFIG_FREERTOS_UNICORE && !BOOTLOADER_BUILD

components/esp_system/CMakeLists.txt

@@ -20,6 +20,7 @@ else()
             "startup.c"
             "system_time.c"
             "stack_check.c"
+            "stack_mirror.c"
             "task_wdt.c"
             "ubsan.c"
             "xt_wdt.c"

components/esp_system/include/esp_private/stack_mirror.h

@@ -0,0 +1,10 @@
+/*
+ * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#if CONFIG_COMPILER_STACK_MIRROR
+void stack_mirror_enable();
+void stack_mirror_print_backtrace(bool panic);
+#endif

components/esp_system/port/arch/xtensa/debug_helpers.c

@@ -22,6 +22,7 @@
 #include "soc/soc_memory_layout.h"
 #include "soc/cpu.h"
 #include "esp_private/panic_internal.h"
+#include "esp_private/stack_mirror.h"
 
 #include "xtensa/xtensa_context.h"
 
@@ -94,6 +95,9 @@ esp_err_t IRAM_ATTR esp_backtrace_print_from_frame(int depth, const esp_backtrac
     esp_err_t ret = ESP_OK;
     if (corrupted) {
         print_str(" |<-CORRUPTED", panic);
+#ifndef CONFIG_COMPILER_STACK_MIRROR
+        print_str("\r\nTurn on Stack Mirroring to avoid corruption", panic);
+#endif
         ret =  ESP_FAIL;
     } else if (stk_frame.next_pc != 0) {    //Backtrace continues
         print_str(" |<-CONTINUES", panic);
@@ -107,5 +111,12 @@ esp_err_t IRAM_ATTR esp_backtrace_print(int depth)
     //Initialize stk_frame with first frame of stack
     esp_backtrace_frame_t start = { 0 };
     esp_backtrace_get_start(&(start.pc), &(start.sp), &(start.next_pc));
-    return esp_backtrace_print_from_frame(depth, &start, false);
+    esp_err_t err = esp_backtrace_print_from_frame(depth, &start, false);
+#if CONFIG_COMPILER_STACK_MIRROR
+    if (err != ESP_OK) {
+        stack_mirror_print_backtrace(false);
+    }
+#endif
+    return err;
 }
+

components/esp_system/port/arch/xtensa/panic_arch.c

@@ -11,6 +11,7 @@
 #include "esp_debug_helpers.h"
 
 #include "esp_private/panic_internal.h"
+#include "esp_private/stack_mirror.h"
 #include "esp_private/panic_reason.h"
 #include "soc/soc.h"
 
@@ -467,5 +468,9 @@ void panic_print_backtrace(const void *f, int core)
 {
     XtExcFrame *xt_frame = (XtExcFrame *) f;
     esp_backtrace_frame_t frame = {.pc = xt_frame->pc, .sp = xt_frame->a1, .next_pc = xt_frame->a0, .exc_frame = xt_frame};
-    esp_backtrace_print_from_frame(100, &frame, true);
+    if (esp_backtrace_print_from_frame(100, &frame, true) != ESP_OK) {
+#if CONFIG_COMPILER_STACK_MIRROR
+        stack_mirror_print_backtrace(true);
+#endif
+    }
 }

components/esp_system/stack_mirror.c

@@ -0,0 +1,128 @@
+/*
+ * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <stdlib.h>
+#include "freertos/FreeRTOS.h"
+#include "freertos/task.h"
+#include "esp_heap_caps.h"
+#include "esp_private/panic_internal.h"
+#include "soc/soc_memory_types.h"
+
+#if CONFIG_COMPILER_STACK_MIRROR
+
+#define STACK_MIRROR_MAGIC 139871234
+
+/* __cyg_profile_func_enter - gcc inserts a call to this function at the start of every function
+ * __cyg_profile_func_exit  - gcc inserts a call to this function at the end of every function
+ * no_instrument_function   - This tells GCC to not insert instrumentation calls (would cause an infinite loop) */
+void IRAM_ATTR __cyg_profile_func_enter(void *func, void *caller) __attribute__((no_instrument_function));
+void IRAM_ATTR __cyg_profile_func_exit(void *func, void *caller) __attribute__((no_instrument_function));
+static void IRAM_ATTR print_str(const char* str, bool panic) __attribute__((no_instrument_function));
+static void IRAM_ATTR print_entry(uint32_t pc, bool panic) __attribute__((no_instrument_function));
+
+static uint32_t enabled;
+
+static void IRAM_ATTR print_str(const char* str, bool panic)
+{
+    if (panic) {
+        panic_print_str(str);
+    } else {
+        esp_rom_printf(str);
+    }
+}
+
+static void IRAM_ATTR print_entry(uint32_t pc, bool panic)
+{
+    if (panic) {
+        panic_print_str(" 0x");
+        panic_print_hex(pc);
+    } else {
+        esp_rom_printf(" 0x%08X", pc);
+    }
+}
+
+extern void xt_unhandled_exception(XtExcFrame *frame);
+
+void IRAM_ATTR __cyg_profile_func_enter(void *func, void *callsite)
+{
+    if (enabled == STACK_MIRROR_MAGIC) {
+        esp_stack_mirror_t *m = pvTaskGetStackMirrorPointer();
+        if (func == xt_unhandled_exception) {
+            // dont add to the stack trace after we already hit panic.
+            m->panicing = true;
+        }
+        if (!m->panicing) {
+            if (m->depth < CONFIG_COMPILER_STACK_MIRROR_DEPTH) {
+                m->backtrace[m->depth] = (uint32_t) func;
+            }
+            m->depth++;
+        }
+    }
+}
+
+void IRAM_ATTR __cyg_profile_func_exit(void *func, void *callsite)
+{
+    if (enabled == STACK_MIRROR_MAGIC) {
+        esp_stack_mirror_t *m = pvTaskGetStackMirrorPointer();
+        if (!m->panicing && m->depth > 0) {
+            m->depth--;
+        }
+    }
+}
+
+void IRAM_ATTR stack_mirror_print_backtrace(bool panic)
+{
+    esp_stack_mirror_t *m = pvTaskGetStackMirrorPointer();
+
+    print_str("\r\nBacktrace (Mirror):", panic);
+
+    if (m == NULL) {
+
+        print_str("\r\nNot Set", panic);
+
+    } else if (esp_ptr_internal(m) == false) {
+
+        print_str("\r\nInvalid Ptr", panic);
+
+    } else if (m->depth == 0) {
+
+        print_str("\r\nEmpty", panic);
+
+    } else {
+
+        uint32_t depth = m->depth;
+
+        if (depth >= CONFIG_COMPILER_STACK_MIRROR_DEPTH) {
+            print_str("\r\nBACKTRACE INCOMPLETE", panic);
+            depth = CONFIG_COMPILER_STACK_MIRROR_DEPTH;
+        }
+
+        for (int i = depth - 1; i >= 0; i--) {
+
+            uint32_t pc = m->backtrace[i];
+
+            print_entry(pc, panic);
+
+            bool corrupted = !esp_ptr_executable((void *)esp_cpu_process_stack_pc(pc));
+
+            if (corrupted) {
+                print_str(" |<-CORRUPTED", panic);
+                break;
+            }
+        }
+    }
+
+    print_str("\r\n\r\n", panic);
+
+    return;
+}
+
+void stack_mirror_enable()
+{
+    enabled = STACK_MIRROR_MAGIC;
+}
+
+#endif // CONFIG_COMPILER_STACK_MIRROR

components/freertos/Kconfig

@@ -172,8 +172,10 @@ menu "FreeRTOS"
             FreeRTOS has the ability to store per-thread pointers in the task
             control block. This controls the number of pointers available.
 
-            This value must be at least 1. Index 0 is reserved for use by the pthreads API
+            This value must be at least 1. Index 0 is reserved for use by the pthreads API thread-local-storage. 
-            thread-local-storage. Other indexes can be used for any desired purpose.
+            
+            Other indexes can be used for any desired purpose.
+
 
     choice FREERTOS_ASSERT
         prompt "FreeRTOS assertions"

components/freertos/include/freertos/FreeRTOS.h

@@ -1254,6 +1254,12 @@ typedef struct xSTATIC_TCB
     #if ( configUSE_POSIX_ERRNO == 1 )
         int iDummy22;
     #endif
+    #if CONFIG_COMPILER_STACK_MIRROR
+        uint32_t uDummy23;
+        uint32_t uDummy24;
+        uint32_t uDummy25[CONFIG_COMPILER_STACK_MIRROR_DEPTH];
+    #endif
+
 } StaticTask_t;
 
 /*

components/freertos/include/freertos/task.h

@@ -1818,6 +1818,16 @@ configSTACK_DEPTH_TYPE uxTaskGetStackHighWaterMark2( TaskHandle_t xTask ) PRIVIL
  */
 uint8_t* pxTaskGetStackStart( TaskHandle_t xTask) PRIVILEGED_FUNCTION;
 
+#if CONFIG_COMPILER_STACK_MIRROR
+typedef struct {
+    uint32_t panicing;
+    uint32_t depth;
+    uint32_t backtrace[CONFIG_COMPILER_STACK_MIRROR_DEPTH];
+} esp_stack_mirror_t;
+
+esp_stack_mirror_t *pvTaskGetStackMirrorPointer();
+#endif // CONFIG_COMPILER_STACK_MIRROR
+
 /* When using trace macros it is sometimes necessary to include task.h before
  * FreeRTOS.h.  When this is done TaskHookFunction_t will not yet have been defined,
  * so the following two prototypes will cause a compilation error.  This can be

components/freertos/port/port_common.c

@@ -15,6 +15,7 @@
 #include "esp_task.h"
 #include "esp_private/crosscore_int.h"
 #include "esp_private/startup_internal.h"    /* Required by g_spiram_ok. [refactor-todo] for g_spiram_ok */
+#include "esp_private/stack_mirror.h"
 #include "esp_log.h"
 #include "soc/soc_memory_types.h"
 #include "soc/dport_access.h"
@@ -138,6 +139,11 @@ static void main_task(void* args)
     }
 #endif
 
+// Todo: this could be initialized sooner, but not sure where exactly...
+#if CONFIG_COMPILER_STACK_MIRROR
+    stack_mirror_enable();
+#endif
+
     app_main();
     vTaskDelete(NULL);
 }

components/freertos/port/xtensa/include/freertos/portmacro.h

@@ -558,6 +558,9 @@ static inline void __attribute__((always_inline)) uxPortCompareSetExtram(volatil
 
 // --------------------- Interrupts ------------------------
 
+#if CONFIG_COMPILER_STACK_MIRROR
+    IRAM_ATTR __attribute__((no_instrument_function))
+#endif
 static inline UBaseType_t __attribute__((always_inline)) xPortSetInterruptMaskFromISR(void)
 {
     UBaseType_t prev_int_level = XTOS_SET_INTLEVEL(XCHAL_EXCM_LEVEL);
@@ -565,6 +568,9 @@ static inline UBaseType_t __attribute__((always_inline)) xPortSetInterruptMaskFr
     return prev_int_level;
 }
 
+#if CONFIG_COMPILER_STACK_MIRROR
+    IRAM_ATTR __attribute__((no_instrument_function))
+#endif
 static inline void __attribute__((always_inline)) vPortClearInterruptMaskFromISR(UBaseType_t prev_level)
 {
     portbenchmarkINTERRUPT_RESTORE(prev_level);
@@ -629,6 +635,9 @@ static inline bool IRAM_ATTR xPortCanYield(void)
 
 // ----------------------- System --------------------------
 
+#if CONFIG_COMPILER_STACK_MIRROR
+    __attribute__((no_instrument_function))
+#endif
 static inline BaseType_t IRAM_ATTR xPortGetCoreID(void)
 {
     return (uint32_t) cpu_hal_get_core_id();

components/freertos/tasks.c

@@ -270,6 +270,7 @@
     #define taskEVENT_LIST_ITEM_VALUE_IN_USE    0x80000000UL
 #endif
 
+
 /*
  * Task control block.  A task control block (TCB) is allocated for each task,
  * and stores task state information, including a pointer to the task's context
@@ -356,6 +357,11 @@ typedef struct tskTaskControlBlock       /* The old naming convention is used to
     #if ( configUSE_POSIX_ERRNO == 1 )
         int iTaskErrno;
     #endif
+
+    #if CONFIG_COMPILER_STACK_MIRROR
+        esp_stack_mirror_t stackMirror;
+    #endif
+
 } tskTCB;
 
 /* The old tskTCB name is maintained above then typedefed to the new TCB_t name
@@ -991,6 +997,11 @@ static void prvInitialiseNewTask( TaskFunction_t pxTaskCode,
         }
     #endif /* portSTACK_GROWTH */
 
+    #if CONFIG_COMPILER_STACK_MIRROR
+        pxNewTCB->stackMirror.panicing = false;
+        pxNewTCB->stackMirror.depth = 0;
+    #endif
+
     /* Store the task name in the TCB. */
     if( pcName != NULL )
     {
@@ -4097,6 +4108,16 @@ static portTASK_FUNCTION( prvIdleTask, pvParameters )
 #endif /* configUSE_TICKLESS_IDLE */
 /*-----------------------------------------------------------*/
 
+
+#if CONFIG_COMPILER_STACK_MIRROR
+    IRAM_ATTR __attribute__((no_instrument_function))
+    esp_stack_mirror_t * pvTaskGetStackMirrorPointer()
+    {
+        TCB_t *pxTCB = xTaskGetCurrentTaskHandle();
+        return &pxTCB->stackMirror;
+    }
+#endif
+
 #if ( configNUM_THREAD_LOCAL_STORAGE_POINTERS != 0 )
 
 #if ( configTHREAD_LOCAL_STORAGE_DELETE_CALLBACKS )
@@ -4646,6 +4667,9 @@ static void prvResetNextTaskUnblockTime( void )
 
 #if ( ( INCLUDE_xTaskGetCurrentTaskHandle == 1 ) || ( configUSE_MUTEXES == 1 ) || (configNUM_CORES > 1) )
 
+#if CONFIG_COMPILER_STACK_MIRROR
+    IRAM_ATTR __attribute__((no_instrument_function))
+#endif
     TaskHandle_t xTaskGetCurrentTaskHandle( void )
     {
         TaskHandle_t xReturn;

components/hal/esp32/include/hal/cpu_ll.h

@@ -29,6 +29,9 @@
 extern "C" {
 #endif
 
+#if CONFIG_COMPILER_STACK_MIRROR
+    IRAM_ATTR __attribute__((no_instrument_function))
+#endif
 static inline uint32_t IRAM_ATTR cpu_ll_get_core_id(void)
 {
     uint32_t id;

components/hal/esp32s3/include/hal/cpu_ll.h

@@ -28,6 +28,9 @@
 extern "C" {
 #endif
 
+#if CONFIG_COMPILER_STACK_MIRROR
+    __attribute__((no_instrument_function))
+#endif
 static inline uint32_t IRAM_ATTR cpu_ll_get_core_id(void)
 {
     uint32_t id;