Skip to content

Commit

Permalink
Update crash handler (#1796)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
mcspr authored Jun 29, 2019
1 parent 746ad70 commit 08a748e
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 27 deletions.
73 changes: 55 additions & 18 deletions code/espurna/crash.ino
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 ) {

Expand All @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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
4 changes: 4 additions & 0 deletions code/espurna/espurna.ino
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
4 changes: 3 additions & 1 deletion code/espurna/system.ino
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 0 additions & 8 deletions code/espurna/terminal.ino
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

1 comment on commit 08a748e

@reaper7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DEBUG_SUPPORT (debug.h) depends on:

  • DEBUG_SERIAL_SUPPORT
  • DEBUG_UDP_SUPPORT
  • DEBUG_TELNET_SUPPORT
  • DEBUG_WEB_SUPPORT

so, if no one is set and finally DEBUG_SUPPORT is disabled
then all crash.ino code is disabled too
but crashSetup(); from espurna.ino->setup() is still called,
which causes a compile error:

C:\Programy\arduino_projekty\espurna_dev\code\espurna\espurna.ino: In function 'void setup()':

espurna:87:16: error: 'crashSetup' was not declared in this scope

     crashSetup();

I think the call should look like this:

    // Init crash recorder
    #if DEBUG_SUPPORT
        crashSetup();
    #endif

Please sign in to comment.