From 9f61331187ec58b9720c31ebabda6bc6e5de527d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Mar 2026 10:46:49 -1000 Subject: [PATCH] Address Copilot review: static linkage + keep valid flag - Make s_raw_crash_data static with inline .noinit definition (no extern needed) - Remove valid=false clearing from crash_handler_log() so both serial (boot) and API (subscribe) paths can emit the crash data --- esphome/components/esp32/crash_handler.cpp | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/esphome/components/esp32/crash_handler.cpp b/esphome/components/esp32/crash_handler.cpp index 4bebef045f2..cdd5139235b 100644 --- a/esphome/components/esp32/crash_handler.cpp +++ b/esphome/components/esp32/crash_handler.cpp @@ -26,16 +26,14 @@ static inline bool IRAM_ATTR is_code_addr(uint32_t addr) { } // Raw crash data written by the panic handler wrapper. -// Lives in .noinit so it survives software reset. -// Defined at file scope (outside any namespace) because both the namespace -// functions and the extern "C" panic handler wrapper need to access it. -struct RawCrashData { +// Lives in .noinit so it survives software reset but contains garbage after power cycle. +// Validated by magic marker. Static linkage since it's only used within this file. +static struct { uint32_t magic; uint32_t pc; uint32_t backtrace[MAX_BACKTRACE]; uint8_t backtrace_count; -}; -extern RawCrashData s_raw_crash_data; +} __attribute__((section(".noinit"))) s_raw_crash_data; namespace esphome::esp32 { @@ -89,8 +87,6 @@ void crash_handler_log() { pos += snprintf(hint + pos, sizeof(hint) - pos, " 0x%08" PRIX32, s_crash_data.backtrace[i]); } ESP_LOGE(TAG, "%s", hint); - // Clear so we don't re-log on subsequent API reconnects - s_crash_data.valid = false; } } // namespace esphome::esp32 @@ -99,11 +95,6 @@ void crash_handler_log() { // Intercepts esp_panic_handler() via --wrap linker flag to capture crash data // into NOINIT memory before the normal panic handler runs. // -// The raw crash data struct must be separate from the read-side struct to avoid -// BSS initialization conflicts. It lives in .noinit so it survives software reset. - -RawCrashData __attribute__((section(".noinit"))) s_raw_crash_data; - extern "C" { extern void __real_esp_panic_handler(panic_info_t *info);