-
Notifications
You must be signed in to change notification settings - Fork 120
Fixes #18 -- call the kernel's BUG macro when we panic #19
Conversation
You can observe on #17 that we get something approximating a useful error message when this happens. |
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.
If only we could get the kernel unwinder to do C++ demangling! LGTM.
I wasn't convinced why this should work, so I did some digging into what exactly is going on: libcore and libstd have different panic implementations. libcore's calls the You can see this by looking at the tracebacks for a panic from inside libcore or a panic from The corollary here is that the panic=abort / panic=unwind selector from RFC 1513 doesn't directly do very much for programs without libstd. It is the One edge case: per the RFC, if you set The full spec for #[lang = "panic_fmt"]
extern fn panic_fmt(msg: fmt::Arguments, file: &'static str, line: u32, col: u32) -> !; We should attempt to print out the message, probably with printk. Also, at some point we'll also need to add Also I think the right thing is to go back to panic=unwind so that we appropriately drop things, poison mutexes, etc. during unwinding. Having a |
Can we move these follow up items into distinct tickets, so that we can
make sure they don't get lost?
…On Sun, May 27, 2018 at 1:56 PM Geoffrey Thomas ***@***.***> wrote:
I wasn't convinced why this should work, so I did some digging into what
exactly is going on:
libcore and libstd have *different* panic implementations. libcore's
<https://github.com/rust-lang/rust/blob/1.26.0/src/libcore/macros.rs#L15-L29>
calls the panic_fmt lang item, and only takes a format string and
Arguments. libstd's
<https://github.com/rust-lang/rust/blob/1.26.0/src/libstd/macros.rs#L64-L78>
can take either a format string and Arguments or a Box<Any>, and calls
__rust_start_panic, which is defined by one of the panic crates (
panic_abort or panic_unwind). Also, libstd *defines* the panic_fmt lang
item as entering its panic handling
<https://github.com/rust-lang/rust/blob/1.26.0/src/libstd/panicking.rs#L316-L326>,
so that panics from libcore take the expected flow. (The panic_fmt lang
item is only used for coordination between libcore and the implementation
of panicking; AFAICT it's not used by the compiler and #![no_core]
programs won't need it.)
You can see this by looking at the tracebacks for a panic from inside
libcore
<http://play.rust-lang.org/?gist=1f5a560cede143be7757818ed6ac6ba6&version=stable&mode=debug>
or a panic from panic! from std
<http://play.rust-lang.org/?gist=9fa833342dd8be73c022c7c666280bce&version=stable&mode=debug>
- the libcore panic has a few more calls on the stack to reach libstd.
The corollary here is that the panic=abort / panic=unwind selector from RFC
1513
<https://github.com/rust-lang/rfcs/blob/master/text/1513-less-unwinding.md>
doesn't directly do very much for programs without libstd. It is the
panic_abort crate that actually aborts
<https://github.com/rust-lang/rust/blob/1.26.0/src/libpanic_abort/lib.rs#L56-L71>,
and in particular it calls libc::abort() on UNIX! A program without
libstd won't link either panic_abort or panic_unwind and won't generate
aborts, unless we tell it to do that in our panic_fmt lang item. However,
it *also* has the purpose of disabling unwinding support in the compiler
(not telling LLVM to generate code to handle exceptions, etc.), so you get
a smaller binary.
One edge case: per the RFC, if you set panic=abort in *any* crate in your
dependency tree and you use libstd, it will use panic_abort. But you need
to set panic=abort in *every* crate in your dependency tree to avoid
setting eh_personality. The intention of the former is to support people
like us before we were using cargo xbuild; we could use precompiled
libcore with panic_unwind and a dummy eh_personality. You'd need
something to resolve libcore's eh_personality relocations against, but it
wouldn't get used (cf. rust-lang/rust#32837 (comment)
<rust-lang/rust#32837 (comment)>).
However, we should now be able to drop eh_personality now that we're
recompiling everything (which was the intention of adding panic=abort as
a target spec option, rust-lang/rust#36647
<rust-lang/rust#36647>).
The full spec for panic_fmt (from libcore's declaration, see also
libstd's definition) is
#[lang = "panic_fmt"]extern fn panic_fmt(msg: fmt::Arguments, file: &'static str, line: u32, col: u32) -> !;
We should attempt to print out the message, probably with printk.
Also, at some point we'll also need to add #[unwind] or #[unwind(allowed)]
to that prototype to mark it as an extern fn that's allowed to unwind (see
rust-lang/rust#46833 <rust-lang/rust#46833>, and
rust-lang/rust#48251 <rust-lang/rust#48251>
which reverted that temporarily because of a regression with Lua, which
uses setjmp/longjmp for its implementation which is also what Windows
unwinding uses).
Also I *think* the right thing is to go back to panic=unwind so that we
appropriately drop things, poison mutexes, etc. during unwinding. Having a
panic_fmt that doesn't terminate the entire program (i.e., kernel panic
for us) under panic=abort seems sort of unsound.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#19 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAADBJRCbvkWb8r3xqZcIu2y_X30ojsWks5t2ujlgaJpZM4UPCqT>
.
--
"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: D1B3 ADC0 E023 8CA6
|
See #19 (comment) for why we can do this - basically, since every crate is compiled with panic=abort (using cargo xbuild), the compiler should not generate any code that needs a personality function.
See #19 (comment) for why we can do this - basically, since every crate is compiled with panic=abort (using cargo xbuild), the compiler should not generate any code that needs a personality function.
No description provided.