rust-embedded / wg

Coordination repository of the embedded devices Working Group
1.87k stars 96 forks source link

Panic handler binary size bloat #41

Open japaric opened 6 years ago

japaric commented 6 years ago

There are two issues here. The first one I consider a (compiler) bug and the second one a missing feature.

Panic strings kept in the binary

Even when panic_fmt (the panic handler) is defined to simply abort the compiler still produces machine code that passes the (unused) arguments (panic message, file name, line number and column number) to that function at runtime. This causes the arguments to be stored in the program which takes up a sizable amount of non volatile memory (.rodata).

Here's @nagisa's description of the issue.

@nagisa was looking into this and they may have found the root of the problem. ~~They'll open an issue in the rust-lang/rust repo with more details.~~ Here's @nagisa proposed solution.

This is also a problem on WASM land; see rust-lang-nursery/rust-wasm#19. If this can't be fixed in the compiler then this RFC (rust-lang/rfcs#2305) for optionally removing the arguments of panic_fmt could be a solution.

Reduce footprint of panic messages

When panic_fmt is defined to print / log panic messages complete strings are stored in the binary. This takes up precious space in non volatile memory (.rodata).

It would be great to be able to store the strings, along with file position data, in a table in debuginfo, which is not flashed into the device, and store only the address to each table entry in the binary. A external tool would be required to turn that address back into a panic location.

A related RFC has been proposed: rust-lang/rfcs#2154

cc @cr1901 @pftbest

nagisa commented 6 years ago

I wrote down my ideas on how we should fix the underlying issue. I still am going to write a separate issue for the underlying issue, of course.

cr1901 commented 6 years ago

Relevant commit for my use case: https://github.com/cr1901/AT2XT/commit/b6e299208dd90cf8ec1bb054ebfbaa87797eb93a

Notably, embedding unused strings does not happen with Result, only Option.

fitzgen commented 6 years ago

See also https://github.com/rust-lang-nursery/rust-wasm/issues/19

japaric commented 6 years ago

Update: we discussed this during Rust All Hands. Here's a summary by @nagisa (originally posted in https://github.com/rust-lang/rust/issues/44489#issuecomment-378058031)

We did discuss various approaches to handling the issue of panic size, and figured out that we’d love to have something that works in all scenarios such as non-lto builds and even debug builds. We didn’t arrive at any particular design, but we managed to reject a number of designs that wouldn’t work and also realised that the only real way to properly implement any design would rely on MIR-only rlibs.

With that in mind, this design seems to be as good as any other and, as long as we stabilise #[panic_implementation] carefully (i.e. the signature and behaviour, but not the exact expansion), we should be in a good place to implement a solution for panic sizes as well.

Where "this design" = RFC 2070

therealprof commented 6 years ago

I'm hopeful this can be sorted out. Panic handling is a bit painful at the moment.

johnthagen commented 5 years ago

Related: panic_immediate_abort libstd feature.

eldruin commented 3 years ago

Is there a solution for this now that RFC-2070 has been closed?

Dirbaio commented 1 month ago

panic-immediate-abort is the solution in nightly. See https://github.com/rust-embedded/wg/issues/551#issuecomment-821477413.

No solution in stable yet.