From 08a748e05d356209963aadb2768d19e0a40d9713 Mon Sep 17 00:00:00 2001 From: Max Prokhorov Date: Sun, 30 Jun 2019 01:25:59 +0300 Subject: [PATCH] Update crash handler (#1796) * Update crash handler - limit recorder stack size by 128 bytes - offset settings a bit more - add possibility to disable crash crash recoder completely * nicer setting names, comment layout change --- code/espurna/crash.ino | 73 +++++++++++++++++++++++++++++---------- code/espurna/espurna.ino | 4 +++ code/espurna/system.ino | 4 ++- code/espurna/terminal.ino | 8 ----- 4 files changed, 62 insertions(+), 27 deletions(-) diff --git a/code/espurna/crash.ino b/code/espurna/crash.ino index 3a440171..5a39dfb1 100644 --- a/code/espurna/crash.ino +++ b/code/espurna/crash.ino @@ -29,7 +29,8 @@ extern "C" { * 8. depc * 9. adress of stack start * 10. adress of stack end - * 11. stack trace bytes + * 11. stack trace size + * 12. stack trace bytes * ... */ #define SAVE_CRASH_CRASH_TIME 0x00 // 4 bytes @@ -42,12 +43,19 @@ extern "C" { #define SAVE_CRASH_DEPC 0x16 // 4 bytes #define SAVE_CRASH_STACK_START 0x1A // 4 bytes #define SAVE_CRASH_STACK_END 0x1E // 4 bytes -#define SAVE_CRASH_STACK_TRACE 0x22 // variable +#define SAVE_CRASH_STACK_SIZE 0x22 // 2 bytes +#define SAVE_CRASH_STACK_TRACE 0x24 // variable + +#define SAVE_CRASH_STACK_TRACE_MAX 0x80 // limit at 128 bytes (increment/decrement by 16) + +uint16_t _save_crash_stack_trace_max = SAVE_CRASH_STACK_TRACE_MAX; +uint16_t _save_crash_enabled = true; /** * Save crash information in EEPROM * This function is called automatically if ESP8266 suffers an exception * It should be kept quick / consise to be able to execute before hardware wdt may kick in + * This method assumes EEPROM has already been initialized, which is the first thing ESPurna does */ extern "C" void custom_crash_callback(struct rst_info * rst_info, uint32_t stack_start, uint32_t stack_end ) { @@ -56,37 +64,44 @@ extern "C" void custom_crash_callback(struct rst_info * rst_info, uint32_t stack return; } - // This method assumes EEPROM has already been initialized - // which is the first thing ESPurna does + // Check if runtime setting disabled this callback + if (!_save_crash_enabled) { + return; + } - // write crash time to EEPROM + // write crash time to EEPROM, which we will later use as a marker that there was a crash uint32_t crash_time = millis(); EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_CRASH_TIME, crash_time); - // write reset info to EEPROM + // rst_info::reason and ::exccause are uint32_t, but are holding small values EEPROMr.write(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_RESTART_REASON, rst_info->reason); EEPROMr.write(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_EXCEPTION_CAUSE, rst_info->exccause); - // write epc1, epc2, epc3, excvaddr and depc to EEPROM + // write epc1, epc2, epc3, excvaddr and depc to EEPROM as uint32_t EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_EPC1, rst_info->epc1); EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_EPC2, rst_info->epc2); EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_EPC3, rst_info->epc3); EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_EXCVADDR, rst_info->excvaddr); EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_DEPC, rst_info->depc); - // write stack start and end address to EEPROM + // EEPROM size is limited, write as little as possible. + // we sometimes want to avoid big stack traces, e.g. if stack_end == 0x3fffffb0, we are in SYS context. + // but still should get enough relevant info and it is possible to set needed size at build/runtime + const uint16_t stack_size = constrain((stack_end - stack_start), 0, _save_crash_stack_trace_max); EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_START, stack_start); EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_END, stack_end); + EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_SIZE, stack_size); - // starting address of Embedis data plus reserve - const uint16_t settings_start = SPI_FLASH_SEC_SIZE - settingsSize() - 0x10; + // starting EEPROM address of Embedis data plus reserve + const uint16_t settings_start = ( + ((SPI_FLASH_SEC_SIZE - settingsSize() + 31) & -32) - 0x20); // write stack trace to EEPROM and avoid overwriting settings - int16_t current_address = SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_TRACE; - for (uint32_t i = stack_start; i < stack_end; i++) { - if (current_address >= settings_start) break; - byte* byteValue = (byte*) i; - EEPROMr.write(current_address++, *byteValue); + int16_t eeprom_addr = SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_TRACE; + for (uint32_t* addr = (uint32_t*)stack_start; addr < (uint32_t*)(stack_start + stack_size); addr++) { + if (eeprom_addr >= settings_start) break; + EEPROMr.put(eeprom_addr, *addr); + eeprom_addr += sizeof(uint32_t); } EEPROMr.commit(); @@ -129,19 +144,22 @@ void crashDump() { DEBUG_MSG_P(PSTR("[DEBUG] excvaddr=0x%08x depc=0x%08x\n"), excvaddr, depc); uint32_t stack_start, stack_end; + uint16_t stack_size; + EEPROMr.get(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_START, stack_start); EEPROMr.get(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_END, stack_end); + EEPROMr.get(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_SIZE, stack_size); - DEBUG_MSG_P(PSTR("[DEBUG] sp=0x%08x end=0x%08x\n"), stack_start, stack_end); + DEBUG_MSG_P(PSTR("sp=0x%08x end=0x%08x saved=0x%04x\n\n"), stack_start, stack_end, stack_size); + if (0xFFFF == stack_size) return; int16_t current_address = SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_TRACE; - int16_t stack_len = stack_end - stack_start; uint32_t stack_trace; DEBUG_MSG_P(PSTR("[DEBUG] >>>stack>>>\n[DEBUG] ")); - for (int16_t i = 0; i < stack_len; i += 0x10) { + for (int16_t i = 0; i < stack_size; i += 0x10) { DEBUG_MSG_P(PSTR("%08x: "), stack_start + i); for (byte j = 0; j < 4; j++) { EEPROMr.get(current_address, stack_trace); @@ -154,4 +172,23 @@ void crashDump() { } +void crashSetup() { + + #if TERMINAL_SUPPORT + terminalRegisterCommand(F("CRASH"), [](Embedis* e) { + crashDump(); + crashClear(); + terminalOK(); + }); + #endif + + // Minumum of 16 and align for column formatter in crashDump() + _save_crash_stack_trace_max = getSetting("sysTraceMax", SAVE_CRASH_STACK_TRACE_MAX).toInt(); + _save_crash_stack_trace_max = (_save_crash_stack_trace_max + 15) & -16; + setSetting("sysScTraceMax", _save_crash_stack_trace_max); + + _save_crash_enabled = getSetting("sysCrashSave", 1).toInt() == 1; + +} + #endif // DEBUG_SUPPORT diff --git a/code/espurna/espurna.ino b/code/espurna/espurna.ino index 6992e216..ae528d77 100644 --- a/code/espurna/espurna.ino +++ b/code/espurna/espurna.ino @@ -83,9 +83,13 @@ void setup() { // Init persistance settingsSetup(); + // Init crash recorder + crashSetup(); + // Return bogus free heap value for broken devices // XXX: device is likely to trigger other bugs! tread carefuly wtfHeap(getSetting("wtfHeap", 0).toInt()); + // Init Serial, SPIFFS and system check systemSetup(); diff --git a/code/espurna/system.ino b/code/espurna/system.ino index 5853fb6d..e0de98b4 100644 --- a/code/espurna/system.ino +++ b/code/espurna/system.ino @@ -164,7 +164,9 @@ void _systemSetupHeartbeat() { #if WEB_SUPPORT bool _systemWebSocketOnReceive(const char * key, JsonVariant& value) { - return (strncmp(key, "hb", 2) == 0); + if (strncmp(key, "sys", 3) == 0) return true; + if (strncmp(key, "hb", 2) == 0) return true; + return false; } #endif diff --git a/code/espurna/terminal.ino b/code/espurna/terminal.ino index 5dd5ffb3..b92698aa 100644 --- a/code/espurna/terminal.ino +++ b/code/espurna/terminal.ino @@ -81,14 +81,6 @@ void _terminalKeysCommand() { void _terminalInitCommand() { - #if DEBUG_SUPPORT - terminalRegisterCommand(F("CRASH"), [](Embedis* e) { - crashDump(); - crashClear(); - terminalOK(); - }); - #endif - terminalRegisterCommand(F("COMMANDS"), [](Embedis* e) { _terminalHelpCommand(); terminalOK();