From 36eaa2c4a11e844f98b2a13d480851a24e56fb40 Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Tue, 12 Aug 2025 11:30:15 +0200 Subject: [PATCH 1/2] fix(panic_handler): Fixed a issue where the system reboots before halt This commit fixes an issue where the panic handler may reboot even if it is configured to halt the CPU. Closes https://github.com/espressif/esp-idf/issues/17260 --- components/esp_system/panic.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/components/esp_system/panic.c b/components/esp_system/panic.c index 23cb2abfc2..38044b70d9 100644 --- a/components/esp_system/panic.c +++ b/components/esp_system/panic.c @@ -262,6 +262,14 @@ static inline void disable_all_wdts(void) wdt_hal_write_protect_enable(&rtc_wdt_ctx); } +/* IRAM-only halt stub: reset modules, then loop */ +void IRAM_ATTR esp_panic_handler_reset_modules_on_exit_and_halt(void) +{ + // Do not print or call non-IRAM functions beyond this point + esp_system_reset_modules_on_exit(); + ESP_INFINITE_LOOP(); +} + /********************** Panic handler functions **********************/ /* This function is called from the panic handler entry point to increment the panic entry count */ @@ -455,10 +463,10 @@ void esp_panic_handler(panic_info_t *info) panic_print_str("Rebooting...\r\n"); panic_restart(); #else /* CONFIG_ESP_SYSTEM_PANIC_PRINT_REBOOT || CONFIG_ESP_SYSTEM_PANIC_SILENT_REBOOT */ + esp_panic_handler_feed_wdts(); panic_print_str("CPU halted.\r\n"); - esp_system_reset_modules_on_exit(); disable_all_wdts(); - ESP_INFINITE_LOOP(); + esp_panic_handler_reset_modules_on_exit_and_halt(); #endif /* CONFIG_ESP_SYSTEM_PANIC_PRINT_REBOOT || CONFIG_ESP_SYSTEM_PANIC_SILENT_REBOOT */ #endif /* CONFIG_ESP_SYSTEM_PANIC_GDBSTUB */ } From f1ab53eda05238a38fe58bc6f2614e996a6b1b5d Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Wed, 13 Aug 2025 12:17:35 +0200 Subject: [PATCH 2/2] test(panic_handler): Added unit test to verify panic handler can halt This test verifies that the panic handler can indeed halt when configured to print and halt instead of rebboting. --- .../system/panic/main/include/test_panic.h | 4 +++ .../system/panic/main/test_app_main.c | 3 ++ .../test_apps/system/panic/main/test_panic.c | 9 ++++++ tools/test_apps/system/panic/pytest_panic.py | 28 +++++++++++-------- .../system/panic/sdkconfig.ci.panic_halt | 5 ++++ 5 files changed, 38 insertions(+), 11 deletions(-) create mode 100644 tools/test_apps/system/panic/sdkconfig.ci.panic_halt diff --git a/tools/test_apps/system/panic/main/include/test_panic.h b/tools/test_apps/system/panic/main/include/test_panic.h index eaa573ddfd..b61c70aa75 100644 --- a/tools/test_apps/system/panic/main/include/test_panic.h +++ b/tools/test_apps/system/panic/main/include/test_panic.h @@ -83,6 +83,10 @@ void test_coredump_summary(void); void test_panic_print_backtrace(void); +#if CONFIG_ESP_SYSTEM_PANIC_PRINT_HALT +void test_panic_halt(void); +#endif /* CONFIG_ESP_SYSTEM_PANIC_PRINT_HALT */ + #ifdef __cplusplus } #endif diff --git a/tools/test_apps/system/panic/main/test_app_main.c b/tools/test_apps/system/panic/main/test_app_main.c index 636717d9cd..34e637483c 100644 --- a/tools/test_apps/system/panic/main/test_app_main.c +++ b/tools/test_apps/system/panic/main/test_app_main.c @@ -126,6 +126,9 @@ void app_main(void) HANDLE_TEST(test_name, test_tcb_corrupted); HANDLE_TEST(test_name, test_panic_handler_stuck0); HANDLE_TEST(test_name, test_panic_handler_crash0); +#if CONFIG_ESP_SYSTEM_PANIC_PRINT_HALT + HANDLE_TEST(test_name, test_panic_halt); +#endif /* CONFIG_ESP_SYSTEM_PANIC_PRINT_HALT */ #if CONFIG_ESP_SYSTEM_USE_FRAME_POINTER HANDLE_TEST(test_name, test_panic_print_backtrace); #endif diff --git a/tools/test_apps/system/panic/main/test_panic.c b/tools/test_apps/system/panic/main/test_panic.c index 20015774bc..53d78cc826 100644 --- a/tools/test_apps/system/panic/main/test_panic.c +++ b/tools/test_apps/system/panic/main/test_panic.c @@ -445,3 +445,12 @@ void test_panic_print_backtrace(void) } #endif + +#if CONFIG_ESP_SYSTEM_PANIC_PRINT_HALT +void test_panic_halt(void) +{ + printf("Triggering panic. Device should print 'CPU halted.' and stop.\n"); + fflush(stdout); + assert(0); +} +#endif /* CONFIG_ESP_SYSTEM_PANIC_PRINT_HALT */ diff --git a/tools/test_apps/system/panic/pytest_panic.py b/tools/test_apps/system/panic/pytest_panic.py index 7b10874b42..1df2f49583 100644 --- a/tools/test_apps/system/panic/pytest_panic.py +++ b/tools/test_apps/system/panic/pytest_panic.py @@ -2,12 +2,9 @@ # SPDX-License-Identifier: CC0-1.0 import itertools import re +from collections.abc import Sequence +from re import Pattern from typing import Any -from typing import List -from typing import Optional -from typing import Pattern -from typing import Sequence -from typing import Union import pexpect import pytest @@ -48,6 +45,7 @@ CONFIGS = list( CONFIG_PANIC = list(itertools.chain(itertools.product(['panic'], ['supported_targets']))) CONFIG_PANIC_DUAL_CORE = list(itertools.chain(itertools.product(['panic'], TARGETS_DUAL_CORE))) +CONFIG_PANIC_HALT = list(itertools.chain(itertools.product(['panic_halt'], TARGETS_ALL))) CONFIGS_BACKTRACE = list( itertools.chain( @@ -105,11 +103,11 @@ CONFIG_COREDUMP_SUMMARY_FLASH_ENCRYPTED = list( PANIC_ABORT_PREFIX = 'Panic reason: ' -def get_default_backtrace(config: str) -> List[str]: +def get_default_backtrace(config: str) -> list[str]: return [config, 'app_main', 'main_task', 'vPortTaskWrapper'] -def expect_coredump_flash_write_logs(dut: PanicTestDut, config: str, check_cpu_reset: Optional[bool] = True) -> None: +def expect_coredump_flash_write_logs(dut: PanicTestDut, config: str, check_cpu_reset: bool | None = True) -> None: dut.expect_exact('Save core dump to flash...') if 'extram_stack' in config: dut.expect_exact('Backing up stack @') @@ -120,7 +118,7 @@ def expect_coredump_flash_write_logs(dut: PanicTestDut, config: str, check_cpu_r dut.expect_cpu_reset() -def expect_coredump_uart_write_logs(dut: PanicTestDut, check_cpu_reset: Optional[bool] = True) -> Any: +def expect_coredump_uart_write_logs(dut: PanicTestDut, check_cpu_reset: bool | None = True) -> Any: # ================= CORE DUMP START ================= # B8AAAMAEgAGAAAAXAEAAAAAAABkAAAA # ... @@ -144,9 +142,9 @@ def expect_coredump_uart_write_logs(dut: PanicTestDut, check_cpu_reset: Optional def common_test( dut: PanicTestDut, config: str, - expected_backtrace: Optional[List[str]] = None, - check_cpu_reset: Optional[bool] = True, - expected_coredump: Optional[Sequence[Union[str, Pattern[Any]]]] = None, + expected_backtrace: list[str] | None = None, + check_cpu_reset: bool | None = True, + expected_coredump: Sequence[str | Pattern[Any]] | None = None, ) -> None: if 'gdbstub' in config: if 'coredump' in config: @@ -1245,3 +1243,11 @@ def test_panic_print_backtrace(dut: PanicTestDut, config: str, test_func_name: s coredump_pattern = re.compile(PANIC_ABORT_PREFIX + regex_pattern.decode('utf-8')) common_test(dut, config, expected_backtrace=None, expected_coredump=[coredump_pattern]) + + +@pytest.mark.generic +@idf_parametrize('config, target', CONFIG_PANIC_HALT, indirect=['config', 'target']) +def test_panic_halt(dut: PanicTestDut) -> None: + dut.run_test_func('test_panic_halt') + dut.expect_exact('CPU halted.', timeout=30) + dut.expect_none(dut.REBOOT, timeout=3) diff --git a/tools/test_apps/system/panic/sdkconfig.ci.panic_halt b/tools/test_apps/system/panic/sdkconfig.ci.panic_halt new file mode 100644 index 0000000000..31714460cd --- /dev/null +++ b/tools/test_apps/system/panic/sdkconfig.ci.panic_halt @@ -0,0 +1,5 @@ +# Panic halt CI config +CONFIG_ESP_SYSTEM_PANIC_PRINT_HALT=y +CONFIG_ESP_SYSTEM_PANIC_PRINT_REBOOT=n +CONFIG_ESP_SYSTEM_PANIC_SILENT_REBOOT=n +CONFIG_ESP_SYSTEM_PANIC_GDBSTUB=n