debug: add DEBUG_BREAKPOINT() macro, set breakpoint on failed assertion #19368

benpicco · 2023-03-09T12:55:13Z

Contribution description

This provides an architecture independent macro to set breakpoints. So far it has only been implemented for Cortex-M and native, but it degrades to a no-op if not defined, so the situation is the same as before.

To make use of this, I added a breakpoints to the assert() function in the case of failure.
This makes debugging failed asserts much easier.

Testing procedure

I simply added a assert(0) to main() in examples/hello-world and ran make debug:

master

main(): This is RIOT! (Version: 2023.04-devel-583-gb13ed2-DEBUG_BREAKPOINT)
Hello World!
You are running RIOT on a(n) native board.
This board features a(n) native MCU.
examples/hello-world/main.c:32 => *** RIOT kernel panic:
FAILED ASSERTION.

*** halted.


native: exiting
[Inferior 1 (process 17242) exited normally]

this PR

main(): This is RIOT! (Version: 2023.04-devel-583-gb13ed2-DEBUG_BREAKPOINT)
Hello World!
You are running RIOT on a(n) native board.
This board features a(n) native MCU.
examples/hello-world/main.c:32 => 
Program received signal SIGTRAP, Trace/breakpoint trap.
0xf7fc6549 in __kernel_vsyscall ()
(gdb) bt
#0  0xf7fc6549 in __kernel_vsyscall ()
#1  0xf7c89147 in __pthread_kill_implementation (threadid=threadid@entry=4160492864, 
    signo=signo@entry=5, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:43
#2  0xf7c891cf in __pthread_kill_internal (signo=5, threadid=4160492864) at ./nptl/pthread_kill.c:78
#3  0xf7c35b15 in __GI_raise (sig=5) at ../sysdeps/posix/raise.c:26
#4  0x08049740 in _assert_failure (file=0x804d018 "examples/hello-world/main.c", line=32)
    at /home/benpicco/dev/RIOT-BHT/core/lib/assert.c:34
#5  0x08049662 in main () at /home/benpicco/dev/RIOT-BHT/examples/hello-world/main.c:32

Issues/PRs references

riot-ci · 2023-03-09T12:58:38Z

Murdock results

✔️ PASSED

b2d54b8 core/assert: set breakpoint on failed assertion

Success	Failures	Total	Runtime
6931	0	6931	10m:47s

Artifacts

Documentation preview

sys/include/architecture.h

core/lib/include/debug.h

Teufelchen1

Looks good & works on my machine.
However, debugging RIOT is poorly documented. I initially wanted to complain that the quick example from the PR description would be a nice addition to the How-To-RIOT-Debugging section on doc.riot-os.org. But we don't have that. 🤷

benpicco · 2023-05-02T18:08:04Z

bors merge

bors · 2023-05-02T20:16:19Z

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

19766: core/lib: make the use of DEBUG_BREAKPOINT on assert optional r=maribu a=gschorcht ### Contribution description This PR makes the use of `DEBUG_BREAKPOINT` on failed assertion optional. The behavior of `assert` has been changed with PR #19368. Instead of printing some useful information, either a breakpoint is inserted and the execution of the MCU stops in debugger or a endless while loop is executed. Before PR #19368 the user got on failed assertion: ``` Starting ESP32x with ID: 7cdfa1e36a34 ESP-IDF SDK Version v4.4.1-0-g1329b19fe49 ... *** RIOT kernel panic: FAILED ASSERTION. *** halted. ``` This was very helpful during development, especially to identify quickly the cause of problems with `DEBUG_ASSERT_VERBOSE` enabled, e.g. when misconfiguration led to failed assertions. With PR #19368 the user gets an address in best case (or even `0` on platforms like ESP32), in worst case the MCU seems to stuck, e.g. ``` Starting ESP32x with ID: 7cdfa1e36a34 ESP-IDF SDK Version v4.4.1-0-g1329b19fe49 ... 0 ``` The problem with the new behavior is that - a user doesn't get a quick indication of what happened - there is not always an easy way to attach a debugger This PR therefore makes the use of `DEBUG_BREAKPOINT` optional using `DEBUG_ASSERT_BREAKPOINT` define. ### Testing procedure Add `assert(0)` in `examples/hello-world/main.c` and compile with and w/o `CFLAGS='-DDEBUG_ASSERT_BREAKPOINT'`. With `DEBUG_ASSERT_BREAKPOINT` the execution should stop in `assert_failue`. Without `DEBUG_ASSERT_BREAKPOINT`, the information as generated before PR #19368 and the execution should stop in `panic_arch`. ### Issues/PRs references Co-authored-by: Gunar Schorcht <gunar@schorcht.net>

19766: core/lib: make the use of DEBUG_BREAKPOINT on assert optional r=gschorcht a=gschorcht ### Contribution description This PR makes the use of `DEBUG_BREAKPOINT` on failed assertion optional. The behavior of `assert` has been changed with PR #19368. Instead of printing some useful information, either a breakpoint is inserted and the execution of the MCU stops in debugger or a endless while loop is executed. Before PR #19368 the user got on failed assertion: ``` Starting ESP32x with ID: 7cdfa1e36a34 ESP-IDF SDK Version v4.4.1-0-g1329b19fe49 ... *** RIOT kernel panic: FAILED ASSERTION. *** halted. ``` This was very helpful during development, especially to identify quickly the cause of problems with `DEBUG_ASSERT_VERBOSE` enabled, e.g. when misconfiguration led to failed assertions. With PR #19368 the user gets an address in best case (or even `0` on platforms like ESP32), in worst case the MCU seems to stuck, e.g. ``` Starting ESP32x with ID: 7cdfa1e36a34 ESP-IDF SDK Version v4.4.1-0-g1329b19fe49 ... 0 ``` The problem with the new behavior is that - a user doesn't get a quick indication of what happened - there is not always an easy way to attach a debugger This PR therefore makes the use of `DEBUG_BREAKPOINT` optional using `DEBUG_ASSERT_BREAKPOINT` define. ### Testing procedure Add `assert(0)` in `examples/hello-world/main.c` and compile with and w/o `CFLAGS='-DDEBUG_ASSERT_BREAKPOINT'`. With `DEBUG_ASSERT_BREAKPOINT` the execution should stop in `assert_failue`. Without `DEBUG_ASSERT_BREAKPOINT`, the information as generated before PR #19368 and the execution should stop in `panic_arch`. ### Issues/PRs references Co-authored-by: Gunar Schorcht <gunar@schorcht.net>

benpicco requested review from kaspar030 and bergzand as code owners March 9, 2023 12:55

github-actions bot added Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: cpu Area: CPU/MCU ports Area: sys Area: System Platform: ARM Platform: This PR/issue effects ARM-based platforms Platform: native Platform: This PR/issue effects the native platform labels Mar 9, 2023

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 9, 2023

benpicco requested a review from maribu March 9, 2023 12:56

benpicco force-pushed the DEBUG_BREAKPOINT branch from 5978b84 to fba4fdf Compare March 9, 2023 13:05

maribu reviewed Mar 9, 2023

View reviewed changes

sys/include/architecture.h Outdated Show resolved Hide resolved

maribu reviewed Mar 9, 2023

View reviewed changes

core/lib/include/debug.h Show resolved Hide resolved

core/lib/include/debug.h Outdated Show resolved Hide resolved

benpicco added the Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation label Mar 22, 2023

benpicco requested review from aabadie and Teufelchen1 April 24, 2023 10:08

Teufelchen1 approved these changes Apr 27, 2023

View reviewed changes

benpicco added 4 commits April 27, 2023 13:19

debug: add DEBUG_BREAKPOINT() macro

b88c9c5

cpu/cortexm_common: define ARCHITECTURE_BREAKPOINT()

e850dcd

cpu/native: define ARCHITECTURE_BREAKPOINT()

fe4cc91

core/assert: set breakpoint on failed assertion

b2d54b8

benpicco force-pushed the DEBUG_BREAKPOINT branch from 65e0858 to b2d54b8 Compare April 27, 2023 11:19

bors bot merged commit 718a451 into RIOT-OS:master May 2, 2023

benpicco deleted the DEBUG_BREAKPOINT branch May 2, 2023 22:05

gschorcht mentioned this pull request Jun 27, 2023

core/lib: make the use of DEBUG_BREAKPOINT on assert optional #19766

Merged

benpicco added this to the Release 2023.07 milestone Aug 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

debug: add DEBUG_BREAKPOINT() macro, set breakpoint on failed assertion #19368

debug: add DEBUG_BREAKPOINT() macro, set breakpoint on failed assertion #19368

benpicco commented Mar 9, 2023 •

edited

Loading

riot-ci commented Mar 9, 2023 •

edited

Loading

Teufelchen1 left a comment

benpicco commented May 2, 2023

bors bot commented May 2, 2023

debug: add DEBUG_BREAKPOINT() macro, set breakpoint on failed assertion #19368

debug: add DEBUG_BREAKPOINT() macro, set breakpoint on failed assertion #19368

Conversation

benpicco commented Mar 9, 2023 • edited Loading

Contribution description

Testing procedure

master

this PR

Issues/PRs references

riot-ci commented Mar 9, 2023 • edited Loading

Murdock results

Artifacts

Teufelchen1 left a comment

Choose a reason for hiding this comment

benpicco commented May 2, 2023

bors bot commented May 2, 2023

benpicco commented Mar 9, 2023 •

edited

Loading

riot-ci commented Mar 9, 2023 •

edited

Loading