rust-embedded / cortex-m-rt

Minimal startup / runtime for Cortex-M microcontrollers
https://rust-embedded.github.io/cortex-m-rt/
Apache License 2.0
359 stars 85 forks source link

Better defaults for DefaultHander and Hardfault #253

Closed korken89 closed 4 years ago

korken89 commented 4 years ago

@jamesmunns and I had a discussion on Matrix about the current defaults that leave HardFault_ and DefaultHander_ implemented as infinite loops. This should most likely be replaced with panic in the default case to help the implementors detect issues and if nothing is done the panic crate takes care of the default course of action. This might increase code size a bit, but for those that do not want this can override it and provide their own implementation.

From an API standpoint it is not a breaking change, however the documentation states that we use infinite loops as the default, so it is a breaking change.

jonas-schievink commented 4 years ago

An alternative would be to panic when #[cfg(debug_assertions)] is on, and use infinite loops otherwise. That way users won't have to go through the hassle of overriding handlers to get small executables.

jamesmunns commented 4 years ago

Could do, but honestly I'd prefer exceptions to reset if I've selected panic-reset or panic-persist as my panic handler, even in release mode.

If you have a panic handler that disregards the panicinfo, it should still be largely optimized out (branch to panic handler/SCB::sys_reset(), instead of infinite branch loop)

jonas-schievink commented 4 years ago

This should probably have been closed by #257, but that has since been reverted, with the decision to make overriding non-maskable interrupt handlers unsafe (https://github.com/rust-embedded/cortex-m-rt/pull/289), so closing as wontfix instead.