-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
stable mechanism to specify the behavior of panic! in no-std applications #2070
Conversation
👍
I would say the former. I imagine that most people don't want custom panic formatting (I struggle to think of a single use case), and it can be easily constructed via the other getters anyway.
Sure is. You only need to add an attribute to define the personality lang item, and then make sure the libpanic_unwind crate has the proper
I've successfully used the LLVM libunwind on a bare-metal target. The only issue with it is that libunwind expects you to provide ELF program headers, which you may not have in your firmware anymore, but that's a trivial patch to libunwind. |
The current issue I have with panic_fmt, is that if the panic handler does not print the message (or use it in any other way) and just goes into infinite loop, the message text is still present in the firmware and llvm can't optimize it away. |
I'd say we can add that backwards compatibly via a future RFC which allows using |
I'd say the latter. Adding location information wastes a lot of space, especially for embedded applications, without bringing any benefit whatsoever for explicit |
Maybe we could allow any type as the argument to the |
text/0000-panic-implementation.md
Outdated
|
||
When using `core::panic!` with formatting, e.g. `panic!("{}", 42)`, the payload | ||
will be uninspectable: it won't be downcastable to any known type. This is where | ||
`core::panic!` diverges from `std::panic!`. The latter returns a `String`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is that important?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to require allocations for no_std
targets
I disagree. The absolute worst offenders in embedded development are implicit panics (which are manageable) and methods like A proper solution here would be rustc saving location information elsewhere and only storing an index into that table in the executable itself. But that would be a completely different RFC (that I'm very interested in). |
@whitequark Maybe I'm not too deep into Rust yet to fully understand this but I do find the information carried along by implicit panics quite useless, even with unwinding support, because it only tells you where the panic happened but not the why. Hence I find it more important to focus on the efficiency of explicit |
I think we need to print the file and line information by default. The only case against them is firmware size, but in that case you can manually print the parts you need. @whitequark Do you propose to print the error code to UART and then have a txt file that is generated during build with full error messages? |
All you need is to give a section name to the globals generated by |
I can't repro. Are you using the right This is what I tested: fn main() {
#[cfg(A)]
panic!("The quick brown fox jumps over the lazy dog.");
#[cfg(B)]
Err::<(), _>("The quick brown fox jumps over the lazy dog.").unwrap();
#[cfg(C)]
panic!("The answer is {}", 42);
}
#[lang = "panic_fmt"]
#[no_mangle]
pub unsafe extern "C" fn rust_begin_unwind(
_msg: ::core::fmt::Arguments,
_file: &'static str,
_line: u32,
_col: u32, // <- new-ish
) -> ! {
// test::black_box
#[cfg(X)]
unsafe { asm!("" : : "r"(&_msg)) }
#[cfg(any(X, Y))]
loop {}
#[cfg(Z)]
::core::intrinsics::abort()
} # A, B or C with either Y or Z
$ arm-none-eabi-size -Ax app
target/thumbv7m-none-eabi/release/app :
section size addr
.vector_table 304 134217728
.text 516 134218032
.ARM.attributes 46 0
Total 866
$ arm-none-eabi-objcopy -O binary app app.bin
$ arm-none-eabi-strings app.bin && echo nothing
nothing # A, B or C with X
$ arm-none-eabi-size -Ax app
section size addr
.vector_table 304 134217728
.text 610 134218032
.rodata 44 134218656
.ARM.attributes 46 0
Total 1004
$ arm-none-eabi-objcopy -O binary app app.bin
$ arm-none-eabi-strings app.bin
The quick brown fox jumps over the lazy dog. Only option X keeps the strings in because the |
That sounds a bit like what stlog, a Symbol Table logger, does. The executable It seems that such functionality could be added in a backward compatible way, panic!(42);
// expands into
{
// same as today
static _FILE_LINE_COL: (&str, u32, u32) = (file!(), line!(), col!());
// new
#[link_section = ".panic.location"] // not allocated in the executable
static _PANIC_LOCATION: (&str, u32, u32) = (file!(), line!(), col!());
core::panicking::panic(&42, &_FILE_LINE_COL, &_PANIC_LOCATION);
}
// and PanicInfo gains a new method
impl PanicInfo {
fn panic_loc(&self) -> usize {
// the address of the _PANIC_LOCATION variable from before
&self.panic_loc as *const _ as usize
}
}
// usage would look like this
#[panic_implementation]
fn panic(pi: &PanicInfo) -> ! {
let panic_loc: usize = pi.panic_loc();
// (.. report the `panic_loc` address somehow ..)
// then abort or diverge in some other way
} Then a tool can map the address back into the location by looking into the EDIT: EDIT 2: Actually, it make require some rustc help to make sure the strings that |
I need to read this more thoroughly later, but right now, I hate to say I want to wait longer for stabilization. The issue is panicking, allocating, shims around main, logging, and other things I am forgetting are special cases of the cyclic modules/crates. The MLs have had a proper solution for this for decades, and anything less than that in Rust will be a severe disappointment to me. I don't want to hold the no-std ecosystem hostage to this, but I think that the fact that a lot of these have a somewhat low-level feel is no coincidence. The need for module-level cycles vs tying the knot with traits correlates with the need for statics/singletons/ambient authority. These things are all major warts warts that can and should be avoided in higher-level code, but are often unavoidable freestanding. OTOH, when #1974 was merged, it was acknowledged that the |
I propose no particular mechanism. For example, it would work if one of two
Yes, absolutely. |
@japaric strange, now I can't reproduce it either. Maybe i did have a wrong signature at some point. Either way it works now, so I have no objections. |
Some copypasta from this thread: https://internals.rust-lang.org/t/stabilization-of-items-needed-for-no-std-applications/5236/8 What would be really nice here are some default strategies for You could imagine something like:
...which goes into an infinite loop. Or if you got the abort intrinsic working:
One of the
|
I really dislike the idea of adding intrinsic support of libc into core Rust. |
@tarcieri With this proposal those "default strategies" can be just crates on crates.io. See below Application: // src/main.rs
extern crate panic_loop; // instead of #[lang_default(panic_strategy, loop)]
// .. panic-loop v1.0.0: // src/lib.rs
// ..
#[panic_implementation]
fn panic(_: &PanicInfo) -> ! {
loop {}
} No need to bake them in the language. |
Perhaps I'm missing something critical, but what exactly is the purpose of payloads? Can you give an example use case or a motivation for why they were included in this rfc? |
@mark-i-m There're plenty of cases (especially in the embedded world) where you might want to panic but don't have the ability to render the cause for the panic in a human readable text, e.g. because the device does not a display and/or serial output. What you might have though is the ability to blink a LED or write a byte to flash and the custom payload looks like a reasonable and lightweight way to convey this information to a panic handler. |
@therealprof Hmm... I see... That does sound like a useful feature, but I kind of would like to see that get its own RFC, rather than being tacked onto this one... |
I say that because, I can understand why it would be convenient, but I also think it can be done for with just the plain panicking mechanism proposed; you can put whatever you want into the panic implementation... |
@mark-i-m I'm not sure I understand - the payload is what allows the libstd implementation of the panic handler to use panic hooks (the default one of which prints the standard |
Actually, the confusion was stemming from my ignorance that payloads already exist in libstd; it seemed like a random extra feature being lumped. So now my question is why does the RFC say this:
Is it a side effect of how |
@mark-i-m from the beginning of the RFC:
Therefore |
Right now I think that goes through a formatting-specific API that libstd implements and does become a |
This looks great, and I'm excited to see |
@eddyb I think you're pointing out two separate issues.
I'd think that by explicitly returning Any we have not committed to anything in particular here?
This sounds like it might be a problem with the proposed |
We're returning |
I suppose we can't make it such that when you try to downcast a payload of edit: yeah I'm not sure what amount of magic would be able to turn a |
Well, the payload won't be |
The |
I've switched this over to @rfcbot fcp merge |
Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
Back in #2070 (comment) I was weary about the immanence of stabilization implied throughout, but I can tackle that in the tracking issue---this plan is certainly better than the status quo, good work @japaric. The std vs core inconsistency is somewhat worrisome to me, in that |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The final comment period is now complete. |
This RFC has been merged! Tracking issue. |
The
#![no_std]
attribute was stabilized some time ago and it made possible tobuild no-std libraries on stable. However, to this day no-std applications
still require a nightly compiler to be built. The main cause of this is that
the behavior of
panic!
is left undefined in no-std context, and the only wayto specify a panicking behavior is through the unstable
panic_fmt
languageitem.
This document proposes a stable mechanism to specify the behavior of
panic!
inno-std context. This would be a step towards enabling development of no-std
applications like device firmware, kernels and operating systems on the stable
channel.
Rendered
cc @alexcrichton @brson @Ericson2314 @jackpot51 @whitequark