Browse Source

xfdump: simplify stack debugging (sample pc+sp)

Instead of having to guess the PC where the SP was sampled, always take
both. This allows "seamless" stack decoding for both serial and xflash
dumps, since we don't have to guess which function generated the dump.

Make the core functions (doing the sampling) be ``noinline`` as well,
so that they always have valid frame.
Yuri D'Elia 2 years ago
parent
commit
dd8c6c064c

+ 19 - 8
Firmware/Dcodes.cpp

@@ -934,11 +934,11 @@ void dcode_9125()
 void dcode_20()
 {
     if(code_seen('E'))
-        xfdump_full_dump_and_reset(SP);
+        xfdump_full_dump_and_reset();
     else
     {
         unsigned long ts = _millis();
-        xfdump_dump(SP);
+        xfdump_dump();
         ts = _millis() - ts;
         DBG(_N("dump completed in %lums\n"), ts);
     }
@@ -969,28 +969,39 @@ void dcode_22()
 #endif
 
 #ifdef EMERGENCY_SERIAL_DUMP
+#include "asm.h"
 #include "xflash_dump.h"
 
 bool emergency_serial_dump = false;
 
-void serial_dump_and_reset(uint16_t sp, dump_crash_reason reason)
+void __attribute__((noinline)) serial_dump_and_reset(dump_crash_reason reason)
 {
+    uint16_t sp;
+    uint32_t pc;
+
+    // we're being called from a live state, so shut off interrupts ...
     cli();
 
+    // sample SP/PC
+    sp = SP;
+    GETPC(&pc);
+
     // extend WDT long enough to allow writing the entire stream
     wdt_enable(WDTO_8S);
 
-    // we're being called from a live state, so shut off interrupts and heaters
+    // ... and heaters
     WRITE(FAN_PIN, HIGH);
     disable_heater();
 
     // this function can also be called from within a corrupted state, so not use
     // printf family of functions that use the heap or grow the stack.
     SERIAL_ECHOLNPGM("D23 - emergency serial dump");
-    SERIAL_ECHOPGM("exception: ");
-    SERIAL_ECHO(sp);
-    SERIAL_ECHO(" ");
-    SERIAL_ECHOLN((unsigned)reason);
+    SERIAL_ECHOPGM("error: ");
+    MYSERIAL.print((uint8_t)reason, DEC);
+    MYSERIAL.print(" ");
+    MYSERIAL.print(pc, HEX);
+    MYSERIAL.print(" ");
+    MYSERIAL.println(sp, HEX);
 
     print_mem(0, RAMEND+1, dcode_mem_t::sram);
     SERIAL_ECHOLNRPGM(MSG_OK);

+ 1 - 1
Firmware/Dcodes.h

@@ -38,7 +38,7 @@ extern void dcode_22(); //D22 - Clear crash dump state
 #ifdef EMERGENCY_SERIAL_DUMP
 #include "xflash_dump.h"
 extern bool emergency_serial_dump;
-extern void serial_dump_and_reset(uint16_t sp, dump_crash_reason);
+extern void serial_dump_and_reset(dump_crash_reason);
 #endif
 
 #ifdef HEATBED_ANALYSIS

+ 6 - 6
Firmware/Marlin_main.cpp

@@ -1716,15 +1716,15 @@ void setup()
 }
 
 
-static inline void crash_and_burn(uint16_t sp, dump_crash_reason reason)
+static inline void crash_and_burn(dump_crash_reason reason)
 {
     WRITE(BEEPER, HIGH);
     eeprom_update_byte((uint8_t*)EEPROM_FW_CRASH_FLAG, (uint8_t)reason);
 #ifdef EMERGENCY_DUMP
-    xfdump_full_dump_and_reset(sp, reason);
+    xfdump_full_dump_and_reset(reason);
 #elif defined(EMERGENCY_SERIAL_DUMP)
     if(emergency_serial_dump)
-        serial_dump_and_reset(sp, reason);
+        serial_dump_and_reset(reason);
 #endif
     softReset();
 }
@@ -1733,18 +1733,18 @@ static inline void crash_and_burn(uint16_t sp, dump_crash_reason reason)
 #ifdef WATCHDOG
 ISR(WDT_vect)
 {
-    crash_and_burn(SP, dump_crash_reason::watchdog);
+    crash_and_burn(dump_crash_reason::watchdog);
 }
 #endif
 
 ISR(BADISR_vect)
 {
-    crash_and_burn(SP, dump_crash_reason::bad_isr);
+    crash_and_burn(dump_crash_reason::bad_isr);
 }
 #endif //EMERGENCY_HANDLERS
 
 void stack_error() {
-    crash_and_burn(SP, dump_crash_reason::stack_error);
+    crash_and_burn(dump_crash_reason::stack_error);
 }
 
 

+ 27 - 0
Firmware/asm.h

@@ -0,0 +1,27 @@
+#pragma once
+#include <stdint.h>
+
+#ifdef __AVR_ATmega2560__
+
+// return the current PC (on AVRs with 22bit PC)
+static inline void GETPC(uint32_t* v)
+{
+  uint8_t a, b, c;
+  asm
+  (
+      "rcall .\n"
+      "pop %2\n"
+      "pop %1\n"
+      "pop %0\n"
+      : "=r" (a), "=r" (b), "=r" (c)
+  );
+  ((uint8_t*)v)[0] = a;
+  ((uint8_t*)v)[1] = b;
+  ((uint8_t*)v)[2] = c;
+  ((uint8_t*)v)[3] = 0;
+
+  // go back 1 instruction before rcall
+  *v = (*v - 2) * 2;
+}
+
+#endif

+ 2 - 2
Firmware/ultralcd.cpp

@@ -1807,7 +1807,7 @@ static void lcd_preheat_menu()
 static void lcd_dump_memory()
 {
     lcd_beeper_quick_feedback();
-    xfdump_dump(SP);
+    xfdump_dump();
     lcd_return_to_status();
 }
 #endif //MENU_DUMP
@@ -1816,7 +1816,7 @@ static void lcd_dump_memory()
 
 static void lcd_serial_dump()
 {
-    serial_dump_and_reset(SP, dump_crash_reason::manual);
+    serial_dump_and_reset(dump_crash_reason::manual);
 }
 #endif //MENU_SERIAL_DUMP
 

+ 8 - 5
Firmware/xflash_dump.cpp

@@ -5,6 +5,7 @@
 
 #include "xflash_dump.h"
 #ifdef XFLASH_DUMP
+#include "asm.h"
 #include "xflash.h"
 #include "Marlin.h" // for softReset
 
@@ -49,13 +50,17 @@ static void xfdump_erase()
 }
 
 
-static void xfdump_dump_core(dump_header_t& hdr, uint32_t addr, uint8_t* buf, uint16_t cnt)
+static void __attribute__((noinline)) xfdump_dump_core(dump_header_t& hdr, uint32_t addr, uint8_t* buf, uint16_t cnt)
 {
     XFLASH_SPI_ENTER();
 
     // start by clearing all sectors (we need all of them in any case)
     xfdump_erase();
 
+    // sample SP/PC
+    hdr.sp = SP;
+    GETPC(&hdr.pc);
+
     // write header
     static_assert(sizeof(hdr) <= 256, "header is larger than a single page write");
     xflash_enable_wr();
@@ -68,13 +73,12 @@ static void xfdump_dump_core(dump_header_t& hdr, uint32_t addr, uint8_t* buf, ui
 }
 
 
-void xfdump_dump(uint16_t sp)
+void xfdump_dump()
 {
     dump_header_t buf;
     buf.magic = DUMP_MAGIC;
     buf.regs_present = false;
     buf.crash_reason = (uint8_t)dump_crash_reason::manual;
-    buf.sp = sp;
 
     // write sram only
     xfdump_dump_core(buf, DUMP_OFFSET + offsetof(dump_t, data.sram),
@@ -82,13 +86,12 @@ void xfdump_dump(uint16_t sp)
 }
 
 
-void xfdump_full_dump_and_reset(uint16_t sp, dump_crash_reason reason)
+void xfdump_full_dump_and_reset(dump_crash_reason reason)
 {
     dump_header_t buf;
     buf.magic = DUMP_MAGIC;
     buf.regs_present = true;
     buf.crash_reason = (uint8_t)reason;
-    buf.sp = sp;
 
     // disable interrupts for a cleaner register dump
     cli();

+ 3 - 4
Firmware/xflash_dump.h

@@ -11,13 +11,12 @@ enum class dump_crash_reason : uint8_t
 };
 
 #ifdef XFLASH_DUMP
-void xfdump_reset();            // reset XFLASH dump state
-void xfdump_dump(uint16_t sp);  // create a new SRAM memory dump
+void xfdump_reset();    // reset XFLASH dump state
+void xfdump_dump();     // create a new SRAM memory dump
 
 // return true if a dump is present, save type in "reason" if provided
 bool xfdump_check_state(dump_crash_reason* reason = NULL);
 
 // create a new dump containing registers and SRAM, then reset
-void xfdump_full_dump_and_reset(
-        uint16_t sp, dump_crash_reason crash = dump_crash_reason::manual);
+void xfdump_full_dump_and_reset(dump_crash_reason crash = dump_crash_reason::manual);
 #endif

+ 3 - 1
Firmware/xflash_layout.h

@@ -21,7 +21,9 @@ struct dump_header_t
 
     uint8_t regs_present; // true when the lower segment containing registers is present
     uint8_t crash_reason; // uses values from dump_crash_source
-    uint16_t sp;          // SP closest to the crash location
+
+    uint32_t pc;          // PC nearby the crash location
+    uint16_t sp;          // SP nearby the crash location
 };
 
 struct dump_data_t