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

Reply via email to