Hello,I would like to request comments for a change to _assert() behaviour in sched/misc/assert.c
I noticed PR #16443 and associated issue #16444 . I wanted to do some testing and noticed that it is somewhat difficult to terminate execution using PANIC()
Currently, the code only terminates execution by a call to reset_board() if the assert/PANIC is raised from kernel thread or interrupt context. In those cases, the program ends in a loop which blinks the panic LED (if the board has one.)
If you want to be able to always trigger a panic, it is needed to enable boardctl() method, define board_reset() and set BOARD_RESET_ON_ASSERT to 2. This method, however, overrides default reset_board() blinking LED behaviour with board_reset() call.
I am attaching a proposal patch to change this. It is adding PANIC_ALWAYS_RESET_BOARD configuration option which also always causes assert/PANIC to terminate execution. This method does not require boardctl() to be enabled, saving flash space.
The patch is currently not tested, I will do that after confirmation that this approach is feasible and can be merged.
If possible, I would also like to ask what is the meaning of PANIC() supposed to be. From the name and all-capital letters, I would expect it to always end the world so to speak. Also, some configuration (TTY_FORCE_PANIC) uses wording that panic is supposed to crash the system and other configuration (MM_PANIC_ON_FAILURE) gives a feeling that it should be doing the same. Nevertheless, that currently only happens based on context.
As a note to the PR #16443 itself, I would recommend trying to use KEEP(*(.vectors)) as seen in boards/avr/avrdx/breadxavr/scripts/breadxavr.ld - the default config should then not need the "# CONFIG_DEBUG_OPT_UNUSED_SECTIONS is not set" line
Best regards.
diff --git a/sched/Kconfig b/sched/Kconfig index 81f226c1cef..ace9406aed4 100644 --- a/sched/Kconfig +++ b/sched/Kconfig @@ -53,6 +53,25 @@ config DISABLE_IDLE_LOOP This option allows nx_start to return instead of entering the idle loop. +config PANIC_ALWAYS_RESET_BOARD + bool "Always call reset_board after PANIC/_assert" + default n + ---help--- + Enable this option if you want every call to PANIC/_assert + to call reset_board regardless of context. If not selected, + reset_board() will only be called if the assertion (PANIC) + happens in a kernel thread or in an interrupt context. + + Note that the same behaviour can also be configured + by setting BOARD_RESET_ON_ASSERT to 2 in Board Selection + menu. Using this method, however, requires boardctl() + to be enabled and board_reset() to be defined. + + This configuration option stops the execution by using + default reset_board function which blinks a LED, + if the board has one. This means that the board is not + actually reset. + menu "Clocks and Timers" config ARCH_HAVE_TICKLESS diff --git a/sched/misc/assert.c b/sched/misc/assert.c index 969fbf68fe2..2d33ab45ae3 100644 --- a/sched/misc/assert.c +++ b/sched/misc/assert.c @@ -843,7 +843,7 @@ void _assert(FAR const char *filename, int linenum, sched_lock(); } -#if CONFIG_BOARD_RESET_ON_ASSERT < 2 +#if !(defined CONFIG_PANIC_ALWAYS_RESET_BOARD) && (CONFIG_BOARD_RESET_ON_ASSERT < 2) if (up_interrupt_context() || (rtcb->flags & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_KERNEL) #endif