Merge branch 'refactor/duplicate_backtrace_code' into 'master'

esp_system: remove duplicate backtrace

Closes IDF-2783

See merge request espressif/esp-idf!12596

components/esp_system/include/esp_debug_helpers.h

@@ -87,6 +87,24 @@ extern void esp_backtrace_get_start(uint32_t *pc, uint32_t *sp, uint32_t *next_p
  */
 bool esp_backtrace_get_next_frame(esp_backtrace_frame_t *frame);
 
+/**
+ * @brief Print the backtrace from specified frame.
+ *
+ * @param depth The maximum number of stack frames to print (should be > 0)
+ * @param frame Starting frame to print from
+ * @param panic Indicator if backtrace print is during a system panic
+ *
+ * @note On the ESP32, users must call esp_backtrace_get_start() first to flush the stack.
+ * @note If a esp_backtrace_frame_t* frame is obtained though a call to esp_backtrace_get_start()
+ * from some example function func_a(), then frame is only valid within the frame/scope of func_a().
+ * Users should not attempt to pass/use frame other frames within the same stack of different stacks.
+ *
+ * @return
+ *      - ESP_OK    Backtrace successfully printed to completion or to depth limit
+ *      - ESP_FAIL  Backtrace is corrupted
+ */
+esp_err_t IRAM_ATTR esp_backtrace_print_from_frame(int depth, const esp_backtrace_frame_t* frame, bool panic);
+
 /**
  * @brief Print the backtrace of the current stack
  *

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

@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <string.h>
+
 #include "sdkconfig.h"
 #include "esp_types.h"
 #include "esp_attr.h"
@@ -19,6 +21,7 @@
 #include "esp_debug_helpers.h"
 #include "soc/soc_memory_layout.h"
 #include "soc/cpu.h"
+#include "esp_private/panic_internal.h"
 
 #include "sdkconfig.h"
 
@@ -36,7 +39,28 @@ bool IRAM_ATTR esp_backtrace_get_next_frame(esp_backtrace_frame_t *frame)
     return (esp_stack_ptr_is_sane(frame->sp) && esp_ptr_executable((void*)esp_cpu_process_stack_pc(frame->pc)));
 }
 
-esp_err_t IRAM_ATTR esp_backtrace_print(int depth)
+static void IRAM_ATTR print_entry(uint32_t pc, uint32_t sp, bool panic)
+{
+    if (panic) {
+        panic_print_str("0x");
+        panic_print_hex(pc);
+        panic_print_str(":0x");
+        panic_print_hex(sp);
+    } else {
+        esp_rom_printf("0x%08X:0x%08X", pc, sp);
+    }
+}
+
+static void IRAM_ATTR print_str(const char* str, bool panic)
+{
+    if (panic) {
+        panic_print_str(str);
+    } else {
+        esp_rom_printf(str);
+    }
+}
+
+esp_err_t IRAM_ATTR esp_backtrace_print_from_frame(int depth, const esp_backtrace_frame_t* frame, bool panic)
 {
     //Check arguments
     if (depth <= 0) {
@@ -44,11 +68,11 @@ esp_err_t IRAM_ATTR esp_backtrace_print(int depth)
     }
 
     //Initialize stk_frame with first frame of stack
-    esp_backtrace_frame_t stk_frame;
-    esp_backtrace_get_start(&(stk_frame.pc), &(stk_frame.sp), &(stk_frame.next_pc));
-    //esp_cpu_get_backtrace_start(&stk_frame);
-    esp_rom_printf("\r\n\r\nBacktrace:");
-    esp_rom_printf("0x%08X:0x%08X ", esp_cpu_process_stack_pc(stk_frame.pc), stk_frame.sp);
+    esp_backtrace_frame_t stk_frame = { 0 };
+    memcpy(&stk_frame, frame, sizeof(esp_backtrace_frame_t));
+
+    print_str("\r\n\r\nBacktrace:", panic);
+    print_entry(esp_cpu_process_stack_pc(stk_frame.pc), stk_frame.sp, panic);
 
     //Check if first frame is valid
     bool corrupted = (esp_stack_ptr_is_sane(stk_frame.sp) &&
@@ -60,17 +84,26 @@ esp_err_t IRAM_ATTR esp_backtrace_print(int depth)
         if (!esp_backtrace_get_next_frame(&stk_frame)) {    //Get previous stack frame
             corrupted = true;
         }
-        esp_rom_printf("0x%08X:0x%08X ", esp_cpu_process_stack_pc(stk_frame.pc), stk_frame.sp);
+        print_entry(esp_cpu_process_stack_pc(stk_frame.pc), stk_frame.sp, panic);
+        print_str(" ", panic);
     }
 
     //Print backtrace termination marker
     esp_err_t ret = ESP_OK;
     if (corrupted) {
-        esp_rom_printf(" |<-CORRUPTED");
+        print_str(" |<-CORRUPTED", panic);
         ret =  ESP_FAIL;
     } else if (stk_frame.next_pc != 0) {    //Backtrace continues
-        esp_rom_printf(" |<-CONTINUES");
+        print_str(" |<-CONTINUES", panic);
     }
-    esp_rom_printf("\r\n\r\n");
+    print_str("\r\n\r\n", panic);
     return ret;
 }
+
+esp_err_t IRAM_ATTR esp_backtrace_print(int depth)
+{
+    //Initialize stk_frame with first frame of stack
+    esp_backtrace_frame_t start;
+    esp_backtrace_get_start(&(start.pc), &(start.sp), &(start.next_pc));
+    return esp_backtrace_print_from_frame(depth, &start, false);
+}

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

@@ -444,14 +444,6 @@ void panic_soc_fill_info(void *f, panic_info_t *info)
 #endif
 }
 
-static void print_backtrace_entry(uint32_t pc, uint32_t sp)
-{
-    panic_print_str("0x");
-    panic_print_hex(pc);
-    panic_print_str(":0x");
-    panic_print_hex(sp);
-}
-
 uint32_t panic_get_address(const void* f)
 {
     return ((XtExcFrame*)f)->pc;
@@ -469,34 +461,7 @@ void panic_set_address(void *f, uint32_t addr)
 
 void panic_print_backtrace(const void *f, int core)
 {
-    // [refactor-todo] once debug helpers have support for both xtensa and riscv, move to
-    // common panic_handler.c
-    XtExcFrame *frame = (XtExcFrame *) f;
-    int depth = 100;
-    //Initialize stk_frame with first frame of stack
-    esp_backtrace_frame_t stk_frame = {.pc = frame->pc, .sp = frame->a1, .next_pc = frame->a0};
-    panic_print_str("\r\nBacktrace:");
-    print_backtrace_entry(esp_cpu_process_stack_pc(stk_frame.pc), stk_frame.sp);
-
-    //Check if first frame is valid
-    bool corrupted = !(esp_stack_ptr_is_sane(stk_frame.sp) &&
-                       (esp_ptr_executable((void *)esp_cpu_process_stack_pc(stk_frame.pc)) ||
-                        /* Ignore the first corrupted PC in case of InstrFetchProhibited */
-                        frame->exccause == EXCCAUSE_INSTR_PROHIBITED));
-
-    uint32_t i = ((depth <= 0) ? INT32_MAX : depth) - 1;    //Account for stack frame that's already printed
-    while (i-- > 0 && stk_frame.next_pc != 0 && !corrupted) {
-        if (!esp_backtrace_get_next_frame(&stk_frame)) {    //Get next stack frame
-            corrupted = true;
-        }
-        panic_print_str(" ");
-        print_backtrace_entry(esp_cpu_process_stack_pc(stk_frame.pc), stk_frame.sp);
-    }
-
-    //Print backtrace termination marker
-    if (corrupted) {
-        panic_print_str(" |<-CORRUPTED");
-    } else if (stk_frame.next_pc != 0) {    //Backtrace continues
-        panic_print_str(" |<-CONTINUES");
-    }
+    XtExcFrame *xt_frame = (XtExcFrame *) f;
+    esp_backtrace_frame_t frame = {.pc = xt_frame->pc, .sp = xt_frame->a1, .next_pc = xt_frame->a0};
+    esp_backtrace_print_from_frame(100, &frame, true);
 }