rust-embedded / cortex-m-rt

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

RFC: Provide defaults for `HardFault` and `DefaultHandler`? #72

Closed japaric closed 6 years ago

japaric commented 6 years ago

Currently these two symbols must be defined somewhere in the dependency graph of an application -- though they are usually defined in the application crate itself.

We could provide defaults for these two symbols in this crate obviating the need to define them in some cases. The questions to answer are: should we? and if we do what should the defaults be?

Some thoughts to kick off the discussion

Should we do this?

There's a trade-off between ergonomics for the experienced used (as in knows what exceptions are and what the HardFault exception means) and learnability for people getting started on embedded development.

For the experienced user defaults are good because that just means "less boilerplate" for trivial applications, examples and programs that are being debugged; and they know when they should override these exception handlers: non trivial applications, production builds, etc.

On the other hand, defaults may make the beginner never learn about these handlers, think that they are not important and/or think that the defaults do the right thing in every possible situation ("sane defaults"). This can turn into a nasty surprise down road when they run their application detached from the debugger and end up experiencing a crash / lockup with no means of investigating the cause because they didn't override the handlers to log faults.

What should the defaults be?

Likely bad options:

What others do:

Other considerations:

cc @therealprof @jcsoo @thejpster

therealprof commented 6 years ago

On the other hand, defaults may make the beginner never learn about these handlers, think that they are not important and/or think that the defaults do the right thing in every possible situation ("sane defaults").

Should be rather easy to teach about Exceptions and Faults since they have other uses as well (like using SysTick for periodic triggering of some activity).

Hardware breakpoint (bkpt). bkpt triggers a HardFault exception. When used from the HardFault handler it results in infinite recursion.

I'm not sure that is correct but I find the Cortex-M user manual ambiguous about this point. In my reading it will simply enter debug mode if a debugger is attached or lock up if not. But the lock-up piece is a pain because it prevents post-mortem debugging which is pretty much the only mode I use.

thejpster commented 6 years ago

I think an empty loop as a default is fine. It's fewer steps to your first "success", and it's pretty easy to set your own once you know what you're doing (or you get to that chapter in the book). I don't think you can complain if your fault handler doesn't do anything by default, because given the almost infinite range of embedded device configurations, there's very little it could do by default.

japaric commented 6 years ago

@rust-embedded/cortex-m we need to take a decision here

korken89 commented 6 years ago

I would agree with @thejpster here, an infinite loop is simple and common in existing starting code for embedded.

therealprof commented 6 years ago

As far as I can see we're in agreement here.

adamgreig commented 6 years ago

+1 on infinite loop as default. It seems least surprising for existing embedded users and as good as anything for newcomers, plus much simpler than having to always define it yourself.

Perhaps we could make a crate like panic-semihosting to allow users to easily override the default (to e.g. panic in the handlers instead), or perhaps as a feature on cortex-m-rt (default-handler-panic?).

japaric commented 6 years ago

There seem to be consensus on using infinite loops for the default behavior.

Follow up question: do we want to give the user the option of overriding the default handler? Or should it only be possible to override each handler individually?

korken89 commented 6 years ago

I would have a way to override default handlers, it can help a lot when debugging to add some extra checks or instructions.

therealprof commented 6 years ago

@japaric If it comes for "free" I'd take it, if it adds complexity I can do without.

@korken89 There's not a lot you can do in there since any exception which is not explicitly defined will fire it. Being able to specify one handler for multiple exceptions would be a lot more useful I think. Bonus points apply if we could also easily use the "move to RAM" hack. 😏

korken89 commented 6 years ago

@therealprof The thing I always do is check the ISR number in the default handler to figure out why an unexpected interrupt is firing. I find it useful when porting large parts of projects (from my C/C++ works).

japaric commented 6 years ago

If it comes for "free" I'd take it, if it adds complexity I can do without.

I believe it shouldn't be hard to implement.

Being able to specify one handler for multiple exceptions would be a lot more useful I think

This on the other hand sounds more complicated. Off the top of my head can't think of a way to implement in Rust in a way that's guaranteed that the handlers won't be duplicated. It seems easier to "implement this" in the linker script but there may be some problems regarding the order in which the linker processes the linker scripts. If you add PROVIDE(some_handler = other_handler) to memory.x I think that ought to work.

Bonus points apply if we could also easily use the "move to RAM" hack.

(I totally forget how I implemented this last time so no comment :-)

therealprof commented 6 years ago

@japaric

I believe it shouldn't be hard to implement.

Well, I'm more concerned about usability.

(I totally forget how I implemented this last time so no comment :-)

Make the handler a veneer function and add

#[link_section = ".data"]
#[inline(never)]

to the real handler function.

japaric commented 6 years ago

Implementation PR (#85) is up for feedback.