search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.87k stars 12.67k forks source link

Double-panics recurse in `panic = "abort"` mode #97146

Open jamesmunns opened 2 years ago

jamesmunns commented 2 years ago

Today @diondokter noticed that in a no_std embedded binary, which is configured to use panic = "abort" as a default, a panic during the panic handler will recurse, rather than abort.

Here is an embedded reproduction case (you'd need an nRF52840-DK, but I've included the output in the README):

https://github.com/jamesmunns/multi-panic

The docs mention 'abort on double panic', but prior to this - I didn't realize this was unique to the "unwind" handler, rather than panics in general. This was a surprise to myself and a number of other experienced embedded-rust folks.

This was tested with stable rust 1.59, but I wanted to check (with the lang/libs teams?) whether this is:

At this point, I'm also not sure if this generalizes to desktop programs in panic = "abort", but in that case, it may be a soundness issue if folks had not considered their panic handlers to be re-entrant safe (though that's probably just the std panic handler in almost every case anyway).

jamesmunns commented 2 years ago

For the record, if the unstable build-std-features=panic_immediate_abort flag is set, the compiler will generate a UDF instruction (basically a hard abort instruction) for these platforms, so it could be possible to re-use that behavior on embedded.

adamgreig commented 2 years ago

It also seems some users currently rely on this behaviour; they use a static atomic to detect the double panic and respond differently (for example, initially in the first panic they attempt to communicate the panic using interfaces that might also panic, then in a second panic they perform only non-panicking handling/abort).

jamesmunns commented 2 years ago

Hey @joshtriplett (picking you as a member of both lang and libs), do you know offhand if this would be a "T-lang" or "T-libs" sort of issue to "rule on"? Would it be possible to get this triaged into one of those piles to get a 'ruling' on this, so we can decide whether to update our wg-embedded docs?

It would be much appreciated!