Add libstd Cargo feature "panic_immediate_abort" #55011

vi · 2018-10-12T13:56:54Z

It stop asserts and panics from libstd to automatically
include string output and formatting code.

Use case: developing static executables smaller than 50 kilobytes,
where usual formatting code is excessive while keeping debuggability
in debug mode.

May resolve #54981.

rust-highfive · 2018-10-12T13:56:57Z

r? @KodrAus

(rust_highfive has picked a reviewer for you, use r? to override)

Mark-Simulacrum · 2018-10-12T13:59:55Z

I suspect that if you're willing to compile std etc yourself using -Cpanic=abort would be the preferred approach, but I'm not sure.

vi · 2018-10-12T14:01:13Z

This is the next step after -Cpanic=abort and force_alloc_system and opt-level="z".

Even with -Cpanic=abort it places all those messages and formatting code into executable.

For info: stripped file size of a {simple program that checks password against sha512 hash and executes supplied program on success} after each step, for arm-unknown-linux-musleabi:

Step	Size
Usual debug	506K
Release (no LTO)	470K
Release (with LTO, codegen-units=1)	466K
All above + opt-level=z	438K
All above + panic=abort	426K
All above + crate-type staticlib _{no jemalloc}	211K
All above + Xargo _{force_alloc_system}	69K
All above + `panic_immediate_abort`	29K

bors · 2018-10-13T17:07:03Z

☔ The latest upstream changes (presumably #54951) made this pull request unmergeable. Please resolve the merge conflicts.

estebank · 2018-10-15T18:52:29Z

Sidenote: @vi do not merge master when merge conflicts arise, rebase against master on your PRs.

KodrAus · 2018-10-21T22:57:11Z

Thanks for your patience @vi!

Hmm, am I reading right that the case you've got is where you've been careful not to interact with the formatting machinery at all because it's heavy, but it's still finding its way in through panic, even though you just want those panics to abort and so there's no need to do any formatting?

On the surface adding a knob to patch up this case seems like it would be brittle, but I don't work in environments where this is really important so don't really understand your workflow.

Even with -Cpanic=abort it places all those messages and formatting code into executable.

This seems a little surprising to me. Is that what we expect?

cc @alexcrichton @japaric who would understand this much better than I do.

alexcrichton · 2018-10-22T07:00:05Z

Thanks for the PR here! I think, though, that for this problem we probably want to tackle this a slightly different way. I've been thinking that if we want to avoid panicking/formatting infrastructure then we should generate binaries that, at the codegen level, never emit the information. As written this still relies on LLVM's LTO to get rid of formatting infrastructure, although this does get rid of the panicking infrastructure.

Is there a way we could perhaps guarantee both get eliminated?

TimNN · 2018-11-06T18:40:50Z

Ping from triage @everyone! How do you think this PR can move forward? Does the suggested approach seem feasible at all?

vi · 2018-11-06T18:48:30Z

S-waiting-on-author

Shall I try doing something myself? Shall the patch change more things, but avoid relying on ~~TCO~~LTO?

I also expect formatting to be still available if one explicitly uses some format!, println! or log (without release_max_level_off), so relying on some form of dead code removal seems to be good idea.

Also simple and easy-ish approach like this patch can be made available now, with proper solution available someday.

TimNN · 2018-11-06T18:51:46Z

S-waiting-on-author

Shall I try doing something myself? Shall the patch change more things, but avoid relying on TCO?

I added this label last week, IIRC mainly because I had seen @alexcrichton's question in his last comment.

alexcrichton · 2018-11-07T16:43:38Z

I would personally either prefer a solution which adds this feature at the source, not even passing panic information into libstd, or one which is a little less invasive than sprinkling a few #[cfg] throughout libstd.

For the former solution I think it's more work to get done because it would require changing the definition of the panic! macro itself. This would probably compile out a number of modules in core/std.

For the latter solution I think this can probably get by with:

if cfg!(feature = "panic_immediate_abort") {
    unsafe { ::intrinsics::abort() }
}

in a few locations.

Dylan-DPC-zz · 2018-11-19T16:51:59Z

Ping from triage, @alexcrichton @vi what's the status on this?

vi · 2018-11-19T17:12:30Z

Idling, not sure what to do next.

I though about changing panic! itself when I was experimenting, but didn't find it in libstd itself and considered that it's too tricky to dig in further into rustc.

bjorn3 · 2018-11-14T18:18:03Z

src/libstd/panicking.rs

+           reason = "used by the panic! macro",
+           issue = "0")]
+#[cfg(feature="panic_immediate_abort")]
+#[inline(never)] #[cold] // avoid code bloat at the call sites as much as possible


Shouldn't this be #[inline]. A call may be longer than ud2.

Probably. Inline line is just a copy-paste from the other variant of the function.

bjorn3 · 2018-11-19T17:21:41Z

I though about changing panic! itself when I was experimenting, but didn't find it in libstd itself

Its in libcore.

alexcrichton · 2018-11-20T21:39:48Z

I've already outlined my thoughts on this. @vi do you have questions about my thinking?

vi · 2018-11-20T23:35:48Z

(two solutions)

Is it worth to do the second, easier approach first, postponing the first, complexier approach to possible further pull requests?

For the latter solution I think this can probably get by with (if cfg!/abort) in a few locations.

Isn't it what this pull request attempts to do? Or do you mean the pull request should be changed to use cfg! instead of #[cfg]?

in a few locations

Or maybe I got those locations wrong and abort should be somewhere else?

alexcrichton · 2018-11-21T18:31:55Z

I think it's fine to start here, but yes I would prefer to switch to cfg! instead of #[cfg] so this can be tested on CI if it's the "quick and dirty" approach

vi · 2018-11-22T17:46:56Z

Patching panic! macro itself in libstd and libcore seems to be cleaner solution.

But if cfg! does not work there (it checks for features of current crate using panic! instead of features of libstd), so duplication may still be necessary.

vi · 2018-11-23T13:22:12Z

@alexcrichton Prepared second version of the easy approach.

Primary decision switching point is shifted to the panic! macro itself;
libcore is now also involved;
Macro is duplicated (cfg vs no cfg) because of if cfg! inside macro_rules checks target crate's feature, not libstd/libcore's feature.
Some core::panicking:: functions are hacked if if cfg! because of cases like let q = [1,2,3]; q[4] triggering panic without involvement of panic! macro. I'm not sure if it is enough (no missing cases) and necessary (no excessive unneeded if cfg!s).

Currently recommended Xargo.toml to take advantage of the feature:

[dependencies]
std = {default-features=false, features=["panic_immediate_abort"]}
core = {default-features=false, features=["panic_immediate_abort"]}

and #[no_main] (otherwise it includes formatting even when fn main() -> !).

Question: should there be non-default panic_immediate_abort feature to turn off usual Rust panic handling or default feature panic_handling that turns on usual Rust panic handling?

dlrobertson · 2018-11-30T14:14:58Z

That seems related to #49878, actually.

Yeah, seems to be an issue with the test.

But why was it not caught there?

No idea why it wasn't caught in the many tests I ran.

kennytm · 2018-11-30T15:30:46Z

let's retry @bors

alexcrichton · 2018-11-30T15:51:00Z

cc @dlrobertson this failure looks like there may be a spurious bug in the va_list tests added maybe?

dlrobertson · 2018-11-30T16:36:22Z

@alexcrichton Thanks, I've got some tests running just to make sure it is a issue with the tests. If it is, I'll have a fix up shortly.

…chton Add libstd Cargo feature "panic_immediate_abort" It stop asserts and panics from libstd to automatically include string output and formatting code. Use case: developing static executables smaller than 50 kilobytes, where usual formatting code is excessive while keeping debuggability in debug mode. May resolve rust-lang#54981.

Rollup of 19 pull requests Successful merges: - #55011 (Add libstd Cargo feature "panic_immediate_abort") - #55821 (Use sort_by_cached_key when the key function is not trivial/free) - #56014 (add test for issue #21335) - #56131 (Assorted tweaks) - #56214 (Implement chalk unification routines) - #56216 (Add TryFrom<&[T]> for [T; $N] where T: Copy) - #56268 (Reuse the `P` in `InvocationCollector::fold_{,opt_}expr`.) - #56324 (Use raw_entry for more efficient interning) - #56336 (Clean up and streamline the pretty-printer) - #56337 (Fix const_fn ICE with non-const function pointer) - #56339 (Remove not used option) - #56341 (Rename conversion util; remove duplicate util in librustc_codegen_llvm.) - #56349 (rustc 1.30.0's linker flavor inference is a non-backwards compat change to -Clinker) - #56355 (Add inline attributes and add unit to CommonTypes) - #56360 (Optimize local linkchecker program) - #56364 (Fix panic with outlives in existential type) - #56365 (Stabilize self_struct_ctor feature.) - #56367 (Moved some feature gate tests to correct location) - #56373 (Update books)

tests: Simplify VaList run-make test The va_list tests were too complex and were causing some spurious test failures on Windows. Example: #55011 (comment)

johnthagen · 2018-12-11T22:05:10Z

@vi / @alexcrichton Is there an example somewhere that shows how to use panic_immediate_abort? And is Xargo required? I'd like to add this (and any other techniques I've missed) to my min-sized-rust repo so that there is an up to date reference on this topic people can reference.

alexcrichton · 2018-12-11T22:20:06Z

@johnthagen I don't know of examples myself, but yeah Xargo would be needed and would be used to enable this feature

vi · 2018-12-11T22:55:35Z

@johnthagen, See this comment and this trivial example.

MauriceKayser · 2019-03-13T10:00:07Z

Sorry if I overlooked the answer to the following question somewhere: Why are the arguments not optimized out, if I don't use the PanicInfo in rust_begin_unwind? I would still like my handler to be called, not intrinsics::abort(). To realize this, I currently have to modify the function panic_fmt in libcore::panicking itself to achieve this, which is really hacky..

vi · 2019-03-13T10:48:01Z

@MauriceKayser , "panic means abort" is easy and simple to explain and is relatively reliable. Previous version of this pull request used some hackery in rust_begin_unwind or something like that, but it failed to remove core::fmt in some cases (or there was some other problem pointed by reviewers).

MauriceKayser · 2019-03-13T11:27:01Z

"panic means abort" is easy and simple to explain and is relatively reliable. Previous version of this pull request used some hackery in rust_begin_unwind or something like that, but it failed to remove core::fmt in some cases (or there was some other problem pointed by reviewers).

This is fine, if the user wishes for this behaviour. I do not, and I do not understand how that answers my question of optimizing out unused arguments. Is this optimization not possible because of the extern "Rust"? This optimization would allow the user to decide whether he wants to use the information, or discard them from the binary, while still being in control of what happens in a panic situation.
I currently have to modify fn panic_impl(pi: &PanicInfo) -> !; to fn panic_impl() -> !; in libcore::panicking::panic_fmt to discard the info, but still call my rust_begin_unwind function. Optimizing away the unused pi parameter would be ideal for me.

… r=joshtriplett Make certain panicky stdlib functions behave better under panic_immediate_abort The stdlib has a `panic_immediate_abort` feature that turns panics into immediate aborts, without any formatting/display logic. This feature was [introduced](rust-lang#55011) primarily for codesize-constrained situations. Unfortunately, this win doesn't quite propagate to `Result::expect()` and `Result::unwrap()`, while the formatting machinery is reduced, `expect()` and `unwrap()` both call `unwrap_failed("msg", &err)` which has a signature of `fn unwrap_failed(msg: &str, error: &dyn fmt::Debug)` and is `#[inline(never)]`. This means that `unwrap_failed` will unconditionally construct a `dyn Debug` trait object even though the object is never used in the function. Constructing a trait object (even if you never call a method on it!) forces rust to include the vtable and any dependencies. This means that in `panic_immediate_abort` mode, calling expect/unwrap on a Result will pull in a whole bunch of formatting code for the error type even if it's completely unused. This PR swaps out the function with one that won't require a trait object such that it won't force the inclusion of vtables in the code. It also gates off `#[inline(never)]` in a bunch of other places where allowing the inlining of an abort may be useful (this kind of thing is already done elsewhere in the stdlib). I don't know how to write a test for this; we don't really seem to have any tests for `panic_immediate_abort` anyway so perhaps it's fine as is.

Builds with `-Z build-std-features=panic_immediate_abort` to ensure the `core::panicking` plumbing is stripped. See: rust-lang/rust#55011 Adds `-Cpanic=abort` to RUSTFLAGS so that build profiles do not need to specify `panic = "abort"` in Cargo.toml. This can potentially allow simulator builds to customize panics. (Maybe printing to the console and stopping the event loop, for instance. But this work is TBD.) Fixes pd-rs/crankstart#66

rust-highfive assigned KodrAus Oct 12, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 12, 2018

vi force-pushed the panic_immediate_abort branch from 44b0e23 to d48a11a Compare October 15, 2018 22:29

This comment has been minimized.

Sign in to view

TimNN added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 30, 2018

bjorn3 reviewed Nov 19, 2018

View reviewed changes

vi force-pushed the panic_immediate_abort branch from d48a11a to aae24d4 Compare November 23, 2018 08:37

Dylan-DPC-zz added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 26, 2018

RalfJung mentioned this pull request Nov 30, 2018

libcore: Add VaList and variadic arg handling intrinsics #49878

Merged

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 30, 2018

scalexm mentioned this pull request Nov 30, 2018

Implement chalk unification routines #56214

Merged

kennytm mentioned this pull request Nov 30, 2018

Rollup of 19 pull requests #56381

Merged

bors merged commit f18a8c6 into rust-lang:master Nov 30, 2018

dlrobertson mentioned this pull request Dec 1, 2018

tests: Simplify VaList run-make test #56396

Merged

bors added a commit that referenced this pull request Dec 2, 2018

Auto merge of #56396 - dlrobertson:fix_va_list_tests, r=nikic

af7554d

tests: Simplify VaList run-make test The va_list tests were too complex and were causing some spurious test failures on Windows. Example: #55011 (comment)

alexcrichton mentioned this pull request Dec 3, 2018

Large file size with fetch example? rustwasm/wasm-bindgen#1078

Closed

johnthagen mentioned this pull request Feb 5, 2019

Panic handler binary size bloat rust-embedded/wg#41

Open

jrvanwhy mentioned this pull request Aug 26, 2019

#[derive(Debug)] seems expensive tock/libtock-rs#84

Open

torfmaster mentioned this pull request Mar 13, 2020

Split libtock into core and high level library tock/libtock-rs#164

Merged

Manishearth mentioned this pull request Dec 10, 2021

Make certain panicky stdlib functions behave better under panic_immediate_abort #91737

Merged

parasyte mentioned this pull request Sep 15, 2023

Fix linker error: undefined symbol core::panicking::panic pd-rs/crank#52

Merged

AlexTMjugador mentioned this pull request Nov 14, 2023

Optimize file size of release binaries shssoichiro/oxipng#571

Closed

jharveyb mentioned this pull request Feb 28, 2024

-panic=trap still prints panic messages tinygo-org/tinygo#4161

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add libstd Cargo feature "panic_immediate_abort" #55011

Add libstd Cargo feature "panic_immediate_abort" #55011

vi commented Oct 12, 2018

rust-highfive commented Oct 12, 2018

Mark-Simulacrum commented Oct 12, 2018

vi commented Oct 12, 2018 •

edited

Loading

bors commented Oct 13, 2018

estebank commented Oct 15, 2018

KodrAus commented Oct 21, 2018

This comment has been minimized.

This comment has been minimized.

alexcrichton commented Oct 22, 2018

TimNN commented Nov 6, 2018

vi commented Nov 6, 2018 •

edited

Loading

TimNN commented Nov 6, 2018

alexcrichton commented Nov 7, 2018

Dylan-DPC-zz commented Nov 19, 2018

vi commented Nov 19, 2018 •

edited

Loading

bjorn3 Nov 14, 2018

vi Nov 19, 2018

bjorn3 commented Nov 19, 2018

alexcrichton commented Nov 20, 2018

vi commented Nov 20, 2018 •

edited

Loading

alexcrichton commented Nov 21, 2018

vi commented Nov 22, 2018

vi commented Nov 23, 2018 •

edited

Loading

dlrobertson commented Nov 30, 2018

kennytm commented Nov 30, 2018

alexcrichton commented Nov 30, 2018

dlrobertson commented Nov 30, 2018

johnthagen commented Dec 11, 2018

alexcrichton commented Dec 11, 2018

vi commented Dec 11, 2018

MauriceKayser commented Mar 13, 2019 •

edited

Loading

vi commented Mar 13, 2019

MauriceKayser commented Mar 13, 2019 •

edited

Loading

Add libstd Cargo feature "panic_immediate_abort" #55011

Add libstd Cargo feature "panic_immediate_abort" #55011

Conversation

vi commented Oct 12, 2018

rust-highfive commented Oct 12, 2018

Mark-Simulacrum commented Oct 12, 2018

vi commented Oct 12, 2018 • edited Loading

bors commented Oct 13, 2018

estebank commented Oct 15, 2018

KodrAus commented Oct 21, 2018

This comment has been minimized.

This comment has been minimized.

alexcrichton commented Oct 22, 2018

TimNN commented Nov 6, 2018

vi commented Nov 6, 2018 • edited Loading

TimNN commented Nov 6, 2018

alexcrichton commented Nov 7, 2018

Dylan-DPC-zz commented Nov 19, 2018

vi commented Nov 19, 2018 • edited Loading

bjorn3 Nov 14, 2018

Choose a reason for hiding this comment

vi Nov 19, 2018

Choose a reason for hiding this comment

bjorn3 commented Nov 19, 2018

alexcrichton commented Nov 20, 2018

vi commented Nov 20, 2018 • edited Loading

alexcrichton commented Nov 21, 2018

vi commented Nov 22, 2018

vi commented Nov 23, 2018 • edited Loading

dlrobertson commented Nov 30, 2018

kennytm commented Nov 30, 2018

alexcrichton commented Nov 30, 2018

dlrobertson commented Nov 30, 2018

johnthagen commented Dec 11, 2018

alexcrichton commented Dec 11, 2018

vi commented Dec 11, 2018

MauriceKayser commented Mar 13, 2019 • edited Loading

vi commented Mar 13, 2019

MauriceKayser commented Mar 13, 2019 • edited Loading

vi commented Oct 12, 2018 •

edited

Loading

vi commented Nov 6, 2018 •

edited

Loading

vi commented Nov 19, 2018 •

edited

Loading

vi commented Nov 20, 2018 •

edited

Loading

vi commented Nov 23, 2018 •

edited

Loading

MauriceKayser commented Mar 13, 2019 •

edited

Loading

MauriceKayser commented Mar 13, 2019 •

edited

Loading