Skip to content
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

Pluggable panic implementations (tracking issue for RFC 1513) #32837

Open
nikomatsakis opened this issue Apr 8, 2016 · 34 comments
Open

Pluggable panic implementations (tracking issue for RFC 1513) #32837

nikomatsakis opened this issue Apr 8, 2016 · 34 comments
Labels
A-error-handling Area: Error handling B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Tracked Libs issues that are tracked on the team's project board. P-low Low priority S-tracking-perma-unstable Status: The feature will stay unstable indefinitely. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Apr 8, 2016

Tracking issue for rust-lang/rfcs#1513.

-C panic=abort is now stable, but the ability to create customized panic implementations is still unstable.

@nikomatsakis nikomatsakis added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-lang Relevant to the language team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. labels Apr 8, 2016
@alexcrichton alexcrichton self-assigned this Apr 8, 2016
@alexcrichton
Copy link
Member

I'm working on an implementation.

@mrpollo
Copy link

mrpollo commented Apr 9, 2016

is there a branch we can track? @alexcrichton, thanks!

@alexcrichton
Copy link
Member

Sure! -- https://github.com/alexcrichton/rust/tree/panic2abort

@ketsuban
Copy link
Contributor

ketsuban commented Apr 9, 2016

Will I still need a dummy eh_personality lang item after this goes through? Some googling suggests -Z no-landing-pads was supposed to remove the need for it (for example, this regression report led to this issue which was resolved by this PR) but I only started using Rust for OSdev since it went stable and I've always needed eh_personality even with -Z no-landing-pads.

@alexcrichton
Copy link
Member

@ketsuban it depends. If you use any library compiled with -C panic=unwind (like the binaries we ship) then you'll need to define eh_personality. If, however, you compile everything with -C panic=abort then you shouldn't need to define a personality.

@alexcrichton
Copy link
Member

I've opened a PR for this.

@alexcrichton alexcrichton removed their assignment Apr 12, 2016
@alexcrichton alexcrichton removed the B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. label Apr 12, 2016
bors added a commit that referenced this issue May 5, 2016
rustc: Implement custom panic runtimes

This commit is an implementation of [RFC 1513] which allows applications to
alter the behavior of panics at compile time. A new compiler flag, `-C panic`,
is added and accepts the values `unwind` or `panic`, with the default being
`unwind`. This model affects how code is generated for the local crate, skipping
generation of landing pads with `-C panic=abort`.

[RFC 1513]: https://github.com/rust-lang/rfcs/blob/master/text/1513-less-unwinding.md

Panic implementations are then provided by crates tagged with
`#![panic_runtime]` and lazily required by crates with
`#![needs_panic_runtime]`. The panic strategy (`-C panic` value) of the panic
runtime must match the final product, and if the panic strategy is not `abort`
then the entire DAG must have the same panic strategy.

With the `-C panic=abort` strategy, users can expect a stable method to disable
generation of landing pads, improving optimization in niche scenarios,
decreasing compile time, and decreasing output binary size. With the `-C
panic=unwind` strategy users can expect the existing ability to isolate failure
in Rust code from the outside world.

Organizationally, this commit dismantles the `sys_common::unwind` module in
favor of some bits moving part of it to `libpanic_unwind` and the rest into the
`panicking` module in libstd. The custom panic runtime support is pretty similar
to the custom allocator support with the only major difference being how the
panic runtime is injected (takes the `-C panic` flag into account).

Closes #32837
bors added a commit that referenced this issue May 6, 2016
rustc: Implement custom panic runtimes

This commit is an implementation of [RFC 1513] which allows applications to
alter the behavior of panics at compile time. A new compiler flag, `-C panic`,
is added and accepts the values `unwind` or `panic`, with the default being
`unwind`. This model affects how code is generated for the local crate, skipping
generation of landing pads with `-C panic=abort`.

[RFC 1513]: https://github.com/rust-lang/rfcs/blob/master/text/1513-less-unwinding.md

Panic implementations are then provided by crates tagged with
`#![panic_runtime]` and lazily required by crates with
`#![needs_panic_runtime]`. The panic strategy (`-C panic` value) of the panic
runtime must match the final product, and if the panic strategy is not `abort`
then the entire DAG must have the same panic strategy.

With the `-C panic=abort` strategy, users can expect a stable method to disable
generation of landing pads, improving optimization in niche scenarios,
decreasing compile time, and decreasing output binary size. With the `-C
panic=unwind` strategy users can expect the existing ability to isolate failure
in Rust code from the outside world.

Organizationally, this commit dismantles the `sys_common::unwind` module in
favor of some bits moving part of it to `libpanic_unwind` and the rest into the
`panicking` module in libstd. The custom panic runtime support is pretty similar
to the custom allocator support with the only major difference being how the
panic runtime is injected (takes the `-C panic` flag into account).

Closes #32837
@sfackler sfackler changed the title Panic as abort (tracking issue for RFC 1513) Pluggable panic implementations (tracking issue for RFC 1513) Sep 7, 2016
@Ericson2314
Copy link
Contributor

Once we have transparently rebuildable std, I'd like to remove the Cargo option from profile. The other profile options do not (to my knowledge) change ABI or semantics so it is inconsistent to put the panic strategy in there. The target spec as per #36647 is a better home for the time being.

@japaric
Copy link
Member

japaric commented Oct 5, 2016

Once we have transparently rebuildable std, I'd like to remove the Cargo option from profile.

What would be the ideal way to pick the panic strategy then? Would it be something like this? (assuming abort is the default and you want to switch to panic=unwind):

  • executable that depends on std. List std as dependency but enable std's "unwind" Cargo feature
  • executable that doesn't depends on std. Either depend on the panic_unwind crate or some other crate that provides an unwinder.
  • library that wants to indicate that relies on unwinding: depend on std with the "unwind" feature enabled or depend on panic_unwind.

Plus some magic attribute in panic_unwind that causes Cargo to compile everything (std included) with landing pads.

@hanna-kruppe
Copy link
Contributor

@Ericson2314

Once we have transparently rebuildable std, I'd like to remove the Cargo option from profile.

How would that work? Leaving aside the issue that it would be a breaking change, per-profile panic strategies are useful: For unit tests you need panic=unwind if you have #[should_panic] tests, but having unit tests is certainly not mutually exclusive with wanting panic=abort in release builds.

@Ericson2314
Copy link
Contributor

Ericson2314 commented Oct 5, 2016

Here are some ideas:

For the low-level panic ABI stuff (abort/unwinding)

I've been thinking that crates should be able to assert a cfg/scenario formula, and that this formula can be used during dependency solving. [lib]/[bin]/[test]/whatever can each have their own formulas. The panic-abi stuff, since it is part of the target, can be mentioned in those formulas.

Additionally, [bin],[test], etc, since they form a depenency root, could be allowed to specify a default target inline, which could be a diff on one of the built-in defaults.

For high-level strategies

@japaric that's the basic idea, but a proper-needs provides solution should make that slicker. https://github.com/ezyang/ghc-proposals/blob/backpack/proposals/0000-backpack.rst is a proposal for Haskell that basically gives it parameterized packages. For "global/singleton" things panic strategies and log, we need to make a "needer" package for them that must be instantiated exactly 1 way in the install plan.

It's a huge undertaking (Backpack is a PHD thesis), but would be the pinnacle of everything we are trying to do build-system-wise for exotic platform development.

@Centril
Copy link
Contributor

Centril commented Sep 15, 2018

cc @japaric is this fully done now that #[panic_handler] is stabilized? should we close?

@japaric
Copy link
Member

japaric commented Sep 15, 2018

@Centril That depends on whether we want to use this issue to track a stable mechanism for implementing unwinding in no_std (right now you have to use the unstable eh_personality lang item), or if we want to do that in another issue, perhaps in a rust-lang/rfcs issue.

@Centril
Copy link
Contributor

Centril commented Sep 15, 2018

@japaric can you raise this on internals perhaps (and close this issue after...)? I'd like to avoid reusing tracking issues and rather keep their scope tightly delimited.

@Dylan-DPC-zz
Copy link

@japaric any update on this?

@KodrAus KodrAus added Libs-Tracked Libs issues that are tracked on the team's project board. A-error-handling Area: Error handling labels Jul 31, 2020
@joshtriplett joshtriplett added the S-tracking-perma-unstable Status: The feature will stay unstable indefinitely. label Jan 19, 2022
@Mark-Simulacrum
Copy link
Member

Lang discussed this today in a backlog bonanza meeting, and generally those in the room were not aware of desire for replacement panic implementations (beyond switching between abort/unwind). It also seemed likely that it may make sense to expect a new RFC if such a desire exists. Please chime in on this issue if you do have a use case!

In the meantime, before such interest and/or design work, it seems like the implementation details that let us have two runtimes (panic=unwind and panic=abort) are roughly perma-unstable.

@Amanieu
Copy link
Member

Amanieu commented Jan 19, 2022

Having worked on unwinding-related things recently, I see the panic runtimes as an implementation detail of std to support -C panic=XXX. While there are use cases for supporting unwinding on no_std, this is outside the scope of this RFC/tracking issue. I think this issue should be closed to indicate that it will remain permanently unstable: there is nothing left to track.

@nikomatsakis
Copy link
Contributor Author

I think we often keep the tracking issue open, but we did tag it as "perma-unstable".

@bjorn3
Copy link
Member

bjorn3 commented Jul 15, 2023

With extern "C-unwind" stabilized you can now create your own #[panic_handler] implementation that does unwinding. This doesn't apply to std programs though as libstd defines #[panic_handler] already.

@juntyr
Copy link
Contributor

juntyr commented Dec 14, 2023

The unstable core::panicking module also contains the very helpful panic_nounwind_fmt function which calls the custom panic handler without unwinding, which seems quite useful to get the panic handlers pretty message without the code size cost for unwinding.

Could -Cpanic perhaps be expanded in the future from abort and unwind to a middle-ground no-unwind option that uses the panic_nounwind* functions to expose this behaviour to end users?

@bjorn3
Copy link
Member

bjorn3 commented Dec 14, 2023

-Cpanic=no-unwind would be -Cpanic=abort except without producing a backtrace, right? The same panic handler is used between -Cpanic=unwind and -Cpanic=abort and without recompiling libstd (or avoiding libstd entirely and defining your own panic handler), it is not possible to get rid of the backtrace functionality. panic_nounwind_fmt forces the -Cpanic=abort behavior for that particular panic call. This will still produce a backtrace when using libstd's panic handler. By the way there is currently no cfg in libstd to disable just the backtrace, we do have panic_immediate_abort, but that also skips the panic message and panic hook and instead codegens every panic as an abort instruction.

@juntyr
Copy link
Contributor

juntyr commented Dec 14, 2023

Thank you for your helpful and detailed reply!

-Cpanic=no-unwind would be -Cpanic=abort except without producing a backtrace, right?

Yes, the behaviour I’m looking for is that a panic would immediately call the panic handler (without unwinding the stack), assuming that the panic handler calls abort/exit when it completes. If another panic is invoked while running the panic handler, we would now abort immediately.

Perhaps I misunderstood -Cpanic=abort so far - is that what it does already? I thought it worked like panic_immediate_abort and produced a call to abort without going through the panic handler first.

without recompiling libstd (or avoiding libstd entirely and defining your own panic handler), it is not possible to get rid of the backtrace functionality.

My particular use case is a NVPTX kernel, where I was experimenting with getting nice print and panic messages to work without pulling in the entire unwinding machinery. Thankfully I’m already defining a custom panic handler here, which just display formats the panic info.

@bjorn3
Copy link
Member

bjorn3 commented Dec 14, 2023

Perhaps I misunderstood -Cpanic=abort so far - is that what it does already?

Correct. -Cpanic=abort will still call the panic handler. It merely stops the panic handler from unwinding.

Thankfully I’m already defining a custom panic handler here, which just display formats the panic info.

In that case panic=abort should be enough.

@juntyr
Copy link
Contributor

juntyr commented Dec 16, 2023

In that case panic=abort should be enough.

In my .cargo/config.toml file, I have the following:

[target.nvptx64-nvidia-cuda]
rustflags = ["-Cpanic=abort", "-Clink-args=--arch sm_35"]

[unstable]
build-std = ["core", "alloc", "std", "panic_abort"]
build-std-features = []

The inclusion of std inside build-std is necessary to not produce many "duplicate lang item in crate core" errors (the same code is compiled twice, once for the host and once for the kernel, where the kernel is no-std but the host uses std).

When I invoke panic!("hi") inside my kernel, the generated PTX explodes, mostly because core::panicking::panic_fmt now calls rust_begin_unwind, which is a huge function by itself and seems to pull in some Vec growth code. While rust_begin_unwind eventually calls my panic handler, this extra code seems to do something which goes against my intuition that "panic=abort means a panic!() will immediately call the panic handler". Am I not configuring my compilation correctly or am I still misunderstanding something?

Thank you again so much for your help <3

@bjorn3
Copy link
Member

bjorn3 commented Dec 16, 2023

The function marked as #[panic_handler] will get the symbol name rust_begin_unwind. Are you sure you defined this function yourself and didn't pull in libstd for the binary compiled for nvptx?

@juntyr
Copy link
Contributor

juntyr commented Dec 16, 2023

I finally figured out what was going on - I was simultaneously experimenting with inlining inside the kernel as well, which pulled some string formatting code into the rust_begin_unwind function. Once I got forceful inlining to work, the optimiser can now look through the panic!("hi") expansion and see that no dynamic formatting (with format!) is needed inside the panic handler to print the message. This eliminates all string formatting code from rust_begin_unwind, which I originally mistook for being part of some unwinding routine. So finally the PTX assembly code is as clean as expected!

Thank you so much @bjorn3 for your patient help in figuring out rust was already doing what I wanted all along :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-error-handling Area: Error handling B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Tracked Libs issues that are tracked on the team's project board. P-low Low priority S-tracking-perma-unstable Status: The feature will stay unstable indefinitely. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests