Skip to content

Commit

Permalink
debug: refactoring & remove special case for PROGMEM
Browse files Browse the repository at this point in the history 
Don't use variable length array and remove `debugSend_P`, directly give the format to `vsnprintf_P`.

Common functions moved into namespace debug { ... }
Module functions moved into namespace { ... }

Provide len argument to most outputs, so it's calculated exactly once.
Early checks for nullptr and zero length.

Fix include order once again, try not to depend on config in header and
only use it in the actual .cpp code.

Still involves strlen for flash strings, but that needs to be addressed by using a custom type like:
```
struct PstrWithLength {
    const char* const ptr;
    size_t size;
};
```
And define PSTR macro to return:
```
PstrWithLength{&__pstr__[0], sizeof(__pstr__)};`
```
(and, probably, for any `const char (&fixed)[Size]` arrays as well)
  • Loading branch information
@mcspr
mcspr committed Aug 11, 2021
1 parent d3551ed commit b167d61
Show file tree
Hide file tree
Showing 6 changed files with 634 additions and 423 deletions.
249 changes: 162 additions & 87 deletions code/espurna/crash.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,109 +13,106 @@ Copyright (C) 2019-2020 by Maxim Prokhorov <prokhorov dot max at outlook dot com
// https://github.com/krzychb/EspSaveCrash
// -----------------------------------------------------------------------------

#include "crash.h"
#include "espurna.h"

#if DEBUG_SUPPORT

#include <stdio.h>
#include <stdarg.h>

#include "crash.h"
#include "system.h"
#include "rtcmem.h"
#include "storage_eeprom.h"

constexpr uint32_t EmptyTimestamp { 0xffffffff };

static bool _save_crash_enabled = true;

size_t crashReservedSize() {
if (!_save_crash_enabled) return 0;
return CrashReservedSize;
}
#include <cstdio>
#include <cstdarg>

/**
* 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
* Structure of the single crash data set
*
* 1. Crash time
* 2. Restart reason
* 3. Exception cause
* 4. epc1
* 5. epc2
* 6. epc3
* 7. excvaddr
* 8. depc
* 9. adress of stack start
* 10. adress of stack end
* 11. stack trace size
* 12. stack trace bytes
* ...
*/
extern "C" void custom_crash_callback(struct rst_info * rst_info, uint32_t stack_start, uint32_t stack_end ) {
#define SAVE_CRASH_CRASH_TIME 0x00 // 4 bytes
#define SAVE_CRASH_RESTART_REASON 0x04 // 1 byte
#define SAVE_CRASH_EXCEPTION_CAUSE 0x05 // 1 byte
#define SAVE_CRASH_EPC1 0x06 // 4 bytes
#define SAVE_CRASH_EPC2 0x0A // 4 bytes
#define SAVE_CRASH_EPC3 0x0E // 4 bytes
#define SAVE_CRASH_EXCVADDR 0x12 // 4 bytes
#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_SIZE 0x22 // 2 bytes
#define SAVE_CRASH_STACK_TRACE 0x24 // variable, 4 bytes per value

constexpr int EepromCrashBegin = EepromReservedSize;
constexpr int EepromCrashEnd = 256;

constexpr size_t CrashReservedSize = EepromCrashEnd - EepromCrashBegin;
constexpr size_t CrashTraceReservedSize = CrashReservedSize - SAVE_CRASH_STACK_TRACE;

// Small safeguard to protect from calling crash handler very early on boot.
if (!eepromReady()) {
return;
}
constexpr uint32_t EmptyTimestamp { 0xffffffff };

// If we crash more than once in a row, don't store (similar) crash log every time
if (systemStabilityCounter() > 1) {
return;
}
namespace debug {
namespace {
namespace crash {
namespace internal {

// Do not record crash data when doing a normal reboot or when crash trace was disabled
if (checkNeedsReset()) {
return;
}
bool enabled = true;

if (!_save_crash_enabled) {
return;
}
} // namespace internal

// We will use this later as a marker that there was a crash
uint32_t crash_time = millis();
eepromPut(EepromCrashBegin + SAVE_CRASH_CRASH_TIME, crash_time);
namespace build {

// XXX rst_info::reason and ::exccause are uint32_t, but are holding small values
// make sure we are using ::write() instead of ::put(), former tries to deduce the required size based on variable type
eepromWrite(EepromCrashBegin + SAVE_CRASH_RESTART_REASON,
static_cast<uint8_t>(rst_info->reason));
eepromWrite(EepromCrashBegin + SAVE_CRASH_EXCEPTION_CAUSE,
static_cast<uint8_t>(rst_info->exccause));
constexpr bool enabled() {
return 1 == SAVE_CRASH_ENABLED;
}

// write epc1, epc2, epc3, excvaddr and depc to EEPROM as uint32_t
eepromPut(EepromCrashBegin + SAVE_CRASH_EPC1, rst_info->epc1);
eepromPut(EepromCrashBegin + SAVE_CRASH_EPC2, rst_info->epc2);
eepromPut(EepromCrashBegin + SAVE_CRASH_EPC3, rst_info->epc3);
eepromPut(EepromCrashBegin + SAVE_CRASH_EXCVADDR, rst_info->excvaddr);
eepromPut(EepromCrashBegin + SAVE_CRASH_DEPC, rst_info->depc);
} // namespace build

// EEPROM size is limited, write as little as possible.
// we definitely want to avoid big stack traces, e.g. like when stack_end == 0x3fffffb0 and 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, CrashReservedSize);
eepromPut(EepromCrashBegin + SAVE_CRASH_STACK_START, stack_start);
eepromPut(EepromCrashBegin + SAVE_CRASH_STACK_END, stack_end);
eepromPut(EepromCrashBegin + SAVE_CRASH_STACK_SIZE, stack_size);
namespace settings {

// write stack trace to EEPROM and avoid overwriting settings and reserved data
// [EEPROM RESERVED SPACE] >>> ... CRASH DATA ... >>> [SETTINGS]
int eeprom_addr = EepromCrashBegin + SAVE_CRASH_STACK_TRACE;
bool enabled() {
return getSetting("sysCrashSave", build::enabled());
}

auto *addr = reinterpret_cast<uint32_t*>(stack_start);
while (EepromCrashEnd > eeprom_addr) {
eepromPut(eeprom_addr, *addr);
eeprom_addr += sizeof(uint32_t);
++addr;
} // namespace settings

bool enabled() {
return internal::enabled;
}

void enableFromSettings() {
internal::enabled = settings::enabled();
}

size_t reserved() {
if (enabled()) {
return CrashReservedSize;
}

eepromForceCommit();
return 0;
}

/**
* Clears crash info CRASH_TIME value, later checked in crashDump()
*/
void crashClear() {
// Simply reset the timestamp to stop dump() from printing the output more than once per crash.
void clear() {
eepromPut(EepromCrashBegin + SAVE_CRASH_CRASH_TIME, EmptyTimestamp);
eepromCommit();
}

namespace {

/**
* Print out crash information that has been previusly saved in EEPROM
* We can optionally check for the recorded crash time before proceeding.
*/
void _crashDump(Print& print, bool check) {

// Print out crash information that has been previusly saved in EEPROM
// Optionally, check whether the timestamp is erased / EEPROM contains no data.
void dump(Print& print, bool check) {
char buffer[256] = {0};

uint32_t crash_time;
Expand Down Expand Up @@ -210,26 +207,93 @@ void _crashDump(Print& print, bool check) {

snprintf_P(buffer, sizeof(buffer), PSTR("<<<stack<<<\n"));
print.print(buffer);

}

#if TERMINAL_SUPPORT

void _crashTerminalCommand(const terminal::CommandContext& ctx) {
crashForceDump(ctx.output);
terminalOK(ctx);
void forceDump(Print& print) {
dump(print, false);
}

#endif
void dump(Print& print) {
dump(print, true);
}

} // namespace crash
} // namespace
} // namespace debug

/**
* 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 ) {

// Small safeguard to protect from calling crash handler very early on boot.
if (!eepromReady()) {
return;
}

// If we crash more than once in a row, don't store (similar) crash log every time
if (systemStabilityCounter() > 1) {
return;
}

// Do not record crash data when doing a normal reboot or when crash trace was disabled
if (checkNeedsReset()) {
return;
}

if (!debug::crash::enabled()) {
return;
}

// We will use this later as a marker that there was a crash
uint32_t crash_time = millis();
eepromPut(EepromCrashBegin + SAVE_CRASH_CRASH_TIME, crash_time);

// XXX rst_info::reason and ::exccause are uint32_t, but are holding small values
// make sure we are using ::write() instead of ::put(), former tries to deduce the required size based on variable type
eepromWrite(EepromCrashBegin + SAVE_CRASH_RESTART_REASON,
static_cast<uint8_t>(rst_info->reason));
eepromWrite(EepromCrashBegin + SAVE_CRASH_EXCEPTION_CAUSE,
static_cast<uint8_t>(rst_info->exccause));

// write epc1, epc2, epc3, excvaddr and depc to EEPROM as uint32_t
eepromPut(EepromCrashBegin + SAVE_CRASH_EPC1, rst_info->epc1);
eepromPut(EepromCrashBegin + SAVE_CRASH_EPC2, rst_info->epc2);
eepromPut(EepromCrashBegin + SAVE_CRASH_EPC3, rst_info->epc3);
eepromPut(EepromCrashBegin + SAVE_CRASH_EXCVADDR, rst_info->excvaddr);
eepromPut(EepromCrashBegin + SAVE_CRASH_DEPC, rst_info->depc);

// EEPROM size is limited, write as little as possible.
// we definitely want to avoid big stack traces, e.g. like when stack_end == 0x3fffffb0 and 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, CrashReservedSize);
eepromPut(EepromCrashBegin + SAVE_CRASH_STACK_START, stack_start);
eepromPut(EepromCrashBegin + SAVE_CRASH_STACK_END, stack_end);
eepromPut(EepromCrashBegin + SAVE_CRASH_STACK_SIZE, stack_size);

// write stack trace to EEPROM and avoid overwriting settings and reserved data
// [EEPROM RESERVED SPACE] >>> ... CRASH DATA ... >>> [SETTINGS]
int eeprom_addr = EepromCrashBegin + SAVE_CRASH_STACK_TRACE;

auto *addr = reinterpret_cast<uint32_t*>(stack_start);
while (EepromCrashEnd > eeprom_addr) {
eepromPut(eeprom_addr, *addr);
eeprom_addr += sizeof(uint32_t);
++addr;
}

eepromForceCommit();
}

void crashForceDump(Print& print) {
_crashDump(print, false);
debug::crash::forceDump(print);
}

void crashDump(Print& print) {
_crashDump(print, true);
debug::crash::dump(print);
}

void crashResetReason(Print& print) {
Expand All @@ -246,16 +310,27 @@ void crashResetReason(Print& print) {
crashDump(print);
}

size_t crashReservedSize() {
return debug::crash::reserved();
}

void crashClear() {
debug::crash::clear();
}

void crashSetup() {
if (!rtcmemStatus()) {
crashClear();
debug::crash::clear();
}

#if TERMINAL_SUPPORT
terminalRegisterCommand(F("CRASH"), _crashTerminalCommand);
terminalRegisterCommand(F("CRASH"), [](const terminal::CommandContext& ctx) {
debug::crash::forceDump(ctx.output);
terminalOK(ctx);
});
#endif

_save_crash_enabled = getSetting("sysCrashSave", 1 == SAVE_CRASH_ENABLED);
debug::crash::enableFromSettings();
}

#endif // DEBUG_SUPPORT
39 changes: 0 additions & 39 deletions code/espurna/crash.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,46 +15,7 @@ Copyright (C) 2019-2020 by Maxim Prokhorov <prokhorov dot max at outlook dot com

#pragma once

#include "espurna.h"

#include <Arduino.h>
#include <cstdint>

/**
* Structure of the single crash data set
*
* 1. Crash time
* 2. Restart reason
* 3. Exception cause
* 4. epc1
* 5. epc2
* 6. epc3
* 7. excvaddr
* 8. depc
* 9. adress of stack start
* 10. adress of stack end
* 11. stack trace size
* 12. stack trace bytes
* ...
*/
#define SAVE_CRASH_CRASH_TIME 0x00 // 4 bytes
#define SAVE_CRASH_RESTART_REASON 0x04 // 1 byte
#define SAVE_CRASH_EXCEPTION_CAUSE 0x05 // 1 byte
#define SAVE_CRASH_EPC1 0x06 // 4 bytes
#define SAVE_CRASH_EPC2 0x0A // 4 bytes
#define SAVE_CRASH_EPC3 0x0E // 4 bytes
#define SAVE_CRASH_EXCVADDR 0x12 // 4 bytes
#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_SIZE 0x22 // 2 bytes
#define SAVE_CRASH_STACK_TRACE 0x24 // variable, 4 bytes per value

constexpr int EepromCrashBegin = EepromReservedSize;
constexpr int EepromCrashEnd = 256;

constexpr size_t CrashReservedSize = EepromCrashEnd - EepromCrashBegin;
constexpr size_t CrashTraceReservedSize = CrashReservedSize - SAVE_CRASH_STACK_TRACE;

size_t crashReservedSize();

Expand Down
Loading

0 comments on commit b167d61

Please sign in to comment.