Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpu/cortexm_common: remove breakpoint from hard_fault_handler #17781

Merged
merged 1 commit into from
Jun 7, 2022

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Mar 9, 2022

Contribution description

The hard fault handler would normally call core_panic() which in turn calls ps() - this already helps debugging a lot of crashes caused by a thread stack overflow.

With the breakpoint in place, the code will not advance to this, leaving users unknown of this feature in the dark.

On top, the breakpoint is not very helpful if thread stacks are already corrupted.

Let's just drop it for simplicity's sake - it's not there for any other architectures.

Testing procedure

Issues/PRs references

The hard fault handler would normally call core_panic() which in turn
calls ps() - this already helps debugging a lot of crashes caused by
a thread stack overflow.

With the breakpoint in place, the code will not advance to this, leaving
users unknown of this feature in the dark.

On top, the breakpoint is not very helpful if thread stacks are already
corrupted.

Let's just drop it for simplicity's sake - it's not there for any other
architectures.
@benpicco benpicco requested a review from maribu March 9, 2022 22:36
@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Mar 9, 2022
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK. I agree that this is very much unexpected and without a debugger attached it will cause a lockup.

@maribu maribu added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 2, 2022
@fjmolinas fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 2, 2022
@benpicco benpicco merged commit a64f722 into RIOT-OS:master Jun 7, 2022
@benpicco benpicco deleted the hard_fault-BKPT branch June 7, 2022 13:28
@chrysn chrysn added this to the Release 2022.07 milestone Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants