Strip debuginfo when debuginfo is not requested #13257

Kobzol · 2024-01-06T16:48:56Z

What does this PR try to resolve?

This PR implements this proposal. It contains a detailed description of the change.

As a summary, this PR modifies Cargo so that if the user doesn't set strip explicitly, and debuginfo is not enabled for any package being compiled, Cargo will implicitly set strip = "debuginfo", to strip pre-existing debuginfo coming from the standard library. This reduces the default size of release binaries considerably (~4.5 MiB => ~450 KiB for helloworld on Linux x64).

Perhaps we could only add the -Cstrip option for the leaf/root target (i.e., a binary), but cargo already passes -Cstrip to libraries if it's set by [profile.<...>.package.<lib>], so it seems harmless.

Fixes: #4122

How should we test and review this PR?

Best reviewed commit by commit. There is one commit that fixes an existing related test that was using wrong assertion.

Additional information

The implementation of the deferred option was inspired by DebugInfo, which already uses a similar concept.

Unresolved questions

Should we also do this on macOS by default? It seems that there can be some issues with strip there. If it doesn't work, it basically inhibits compilation in release mode, with no easy way to opt out (unless the user explicitly requests debuginfo, but that's not the same as the previous state).

rustbot · 2024-01-06T16:49:01Z

r? @ehuss

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

Kobzol · 2024-01-12T10:16:09Z

src/cargo/core/profiles.rs

+#[serde(rename_all = "lowercase")]
+pub enum Strip {
+    /// A strip option that is fixed and will not change.
+    Resolved(StripInner),


Maybe it would make sense to introduce some kind of generic Resolved/Deferred wrapper and use it both here and in DebugInfo?

Kobzol · 2024-01-15T08:47:47Z

The FCP has been accepted.

ehuss · 2024-01-15T17:19:19Z

Thanks!

@bors r+

bors · 2024-01-15T17:19:22Z

📌 Commit e954099 has been approved by ehuss

It is now in the queue for this repository.

bors · 2024-01-15T17:20:30Z

⌛ Testing commit e954099 with merge fbf9251...

bors · 2024-01-15T17:56:44Z

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing fbf9251 to master...

Update cargo 10 commits in 84976cd699f4aea56cb3a90ce3eedeed9e20d5a5..1cff2ee6b92e0ad3f87c44b70b28f788b2528b3c 2024-01-12 15:55:43 +0000 to 2024-01-16 16:56:57 +0000 - doc: add a heading to highlight "How to find features enabled on dependencies" (rust-lang/cargo#13305) - fix(cargo-update): `--precise` accept arbitrary git revisions (rust-lang/cargo#13250) - Strip debuginfo when debuginfo is not requested (rust-lang/cargo#13257) - Update ahash dependency to 0.8.7 (rust-lang/cargo#13301) - docs: add more links to pkgid spec chapter (rust-lang/cargo#13298) - fix(metadata): Stabilize id format as PackageIDSpec (rust-lang/cargo#12914) - Introduce `-Zprecise-pre-release` unstable flag (rust-lang/cargo#13296) - Delete sentence about parentheses being unsupported in license (rust-lang/cargo#13292) - Add guidance on setting homepage in manifest (rust-lang/cargo#13293) - Clarify the function of the test options (rust-lang/cargo#13236) r? ghost

tests/testsuite/profiles.rs

utkarshgupta137 · 2024-03-21T16:24:19Z

I think this is worth mentioning in the docs too?

Kobzol · 2024-03-21T16:25:44Z

I would say that the docs already mentioned this (debug = 0 claimed that there will be no debuginfo in the target binary). It just wasn't upheld before, and it is now. OTOH, we could maybe document the exact mechanism that is used to achieve this.

Fix debuginfo strip when using `--target` This fixes an issue where automatic `strip` of debuginfo added in #13257 wasn't working when the `--target` flag was used. The problem is that the adjustment code was only running in the optimization pass that is done when `--target` is *not* specified. The solution is to just always run the unit graph rebuild. I believe it should be safe to do so, since the adjustments it makes should be conditional on just the scenarios that matter when `--target` is not specified. The downside is that this might be a small performance hit when `--target` is used. Trying to avoid that I think would be quite challenging. Fixes #13617

Do not strip debuginfo by default for MSVC This PR disables the logic which enables debuginfo stripping by default in release mode (#13257) for MSVC, since it causes problems there (rust-lang/rust#122857). I'm not sure if this is the correct way to gate the logic on a specific target triple. The root of the issue is that `-Cstrip=debuginfo` currently behaves like `-Cstrip=symbols` on MSVC, which causes the original optimization to break backtraces on Windows. Ultimately, this should probably be fixed in `rustc`, but that might take some further design work, therefore a faster solution would be to just special case this in Cargo for now, and remove the special case later, once it gets fixed on the `rustc` side. Related issue: rust-lang/rust#122857

Pkgsrc changes: * Adapt checksums and patches. Upstream chnages: Version 1.77.0 (2024-03-21) ========================== - [Reveal opaque types within the defining body for exhaustiveness checking.] (rust-lang/rust#116821) - [Stabilize C-string literals.] (rust-lang/rust#117472) - [Stabilize THIR unsafeck.] (rust-lang/rust#117673) - [Add lint `static_mut_refs` to warn on references to mutable statics.] (rust-lang/rust#117556) - [Support async recursive calls (as long as they have indirection).] (rust-lang/rust#117703) - [Undeprecate lint `unstable_features` and make use of it in the compiler.] (rust-lang/rust#118639) - [Make inductive cycles in coherence ambiguous always.] (rust-lang/rust#118649) - [Get rid of type-driven traversal in const-eval interning] (rust-lang/rust#119044), only as a [future compatiblity lint] (rust-lang/rust#122204) for now. - [Deny braced macro invocations in let-else.] (rust-lang/rust#119062) Compiler -------- - [Include lint `soft_unstable` in future breakage reports.] (rust-lang/rust#116274) - [Make `i128` and `u128` 16-byte aligned on x86-based targets.] (rust-lang/rust#116672) - [Use `--verbose` in diagnostic output.] (rust-lang/rust#119129) - [Improve spacing between printed tokens.] (rust-lang/rust#120227) - [Merge the `unused_tuple_struct_fields` lint into `dead_code`.] (rust-lang/rust#118297) - [Error on incorrect implied bounds in well-formedness check] (rust-lang/rust#118553), with a temporary exception for Bevy. - [Fix coverage instrumentation/reports for non-ASCII source code.] (rust-lang/rust#119033) - [Fix `fn`/`const` items implied bounds and well-formedness check.] (rust-lang/rust#120019) - [Promote `riscv32{im|imafc}-unknown-none-elf` targets to tier 2.] (rust-lang/rust#118704) - Add several new tier 3 targets: - [`aarch64-unknown-illumos`] (rust-lang/rust#112936) - [`hexagon-unknown-none-elf`] (rust-lang/rust#117601) - [`riscv32imafc-esp-espidf`] (rust-lang/rust#119738) - [`riscv32im-risc0-zkvm-elf`] (rust-lang/rust#117958) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Implement `From<&[T; N]>` for `Cow<[T]>`.] (rust-lang/rust#113489) - [Remove special-case handling of `vec.split_off (0)`.](rust-lang/rust#119917) Stabilized APIs --------------- - [`array::each_ref`] (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_ref) - [`array::each_mut`] (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_mut) - [`core::net`] (https://doc.rust-lang.org/stable/core/net/index.html) - [`f32::round_ties_even`] (https://doc.rust-lang.org/stable/std/primitive.f32.html#method.round_ties_even) - [`f64::round_ties_even`] (https://doc.rust-lang.org/stable/std/primitive.f64.html#method.round_ties_even) - [`mem::offset_of!`] (https://doc.rust-lang.org/stable/std/mem/macro.offset_of.html) - [`slice::first_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk) - [`slice::first_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk_mut) - [`slice::split_first_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk) - [`slice::split_first_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk_mut) - [`slice::last_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk) - [`slice::last_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk_mut) - [`slice::split_last_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk) - [`slice::split_last_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk_mut) - [`slice::chunk_by`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by) - [`slice::chunk_by_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by_mut) - [`Bound::map`] (https://doc.rust-lang.org/stable/std/ops/enum.Bound.html#method.map) - [`File::create_new`] (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.create_new) - [`Mutex::clear_poison`] (https://doc.rust-lang.org/stable/std/sync/struct.Mutex.html#method.clear_poison) - [`RwLock::clear_poison`] (https://doc.rust-lang.org/stable/std/sync/struct.RwLock.html#method.clear_poison) Cargo ----- - [Extend the build directive syntax with `cargo::`.] (rust-lang/cargo#12201) - [Stabilize metadata `id` format as `PackageIDSpec`.] (rust-lang/cargo#12914) - [Pull out as `cargo-util-schemas` as a crate.] (rust-lang/cargo#13178) - [Strip all debuginfo when debuginfo is not requested.] (rust-lang/cargo#13257) - [Inherit jobserver from env for all kinds of runners.] (rust-lang/cargo#12776) - [Deprecate rustc plugin support in cargo.] (rust-lang/cargo#13248) Rustdoc ----- - [Allows links in markdown headings.] (rust-lang/rust#117662) - [Search for tuples and unit by type with `()`.] (rust-lang/rust#118194) - [Clean up the source sidebar's hide button.] (rust-lang/rust#119066) - [Prevent JS injection from `localStorage`.] (rust-lang/rust#120250) Misc ---- - [Recommend version-sorting for all sorting in style guide.] (rust-lang/rust#115046) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Add more weirdness to `weird-exprs.rs`.] (rust-lang/rust#119028)

The debuginfo is used by the "bcachefs debug" and "bcachefs list_bkeys" commands. Rust 1.77 [1] changed Cargo's release profile to strip debuginfo by default, but we always want it included. [1] rust-lang/cargo#13257 Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>

This seems to have changed in rust-lang/cargo#13257. Also do not install libusb-1.0 during setup. This doesn't seem necessary anymore.

rustbot assigned ehuss Jan 6, 2024

rustbot added A-build-execution Area: anything dealing with executing the compiler A-profiles Area: profiles S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 6, 2024

Kobzol force-pushed the profile-strip-debuginfo branch from c50fa19 to ef67dfb Compare January 6, 2024 16:50

Kobzol added 5 commits January 7, 2024 09:48

Store the Strip option in Deferred form

58da1ce

Strip debuginfo if no debuginfo was requested

bc53451

Fix existing test

872aa8f

Add tests

af46bbd

Update existing tests

e954099

Kobzol force-pushed the profile-strip-debuginfo branch from ef67dfb to e954099 Compare January 7, 2024 08:48

bjorn3 mentioned this pull request Jan 10, 2024

PoC: add Rust submodule as libbasic_rust systemd/systemd#19598

Open

Kobzol commented Jan 12, 2024

View reviewed changes

Kobzol mentioned this pull request Jan 15, 2024

Enable strip in release mode by default #4122

Closed

ehuss added the relnotes Release-note worthy label Jan 15, 2024

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 Jan 15, 2024

bors merged commit fbf9251 into rust-lang:master Jan 15, 2024
20 checks passed

Kobzol deleted the profile-strip-debuginfo branch January 15, 2024 18:00

weihanglo mentioned this pull request Jan 16, 2024

Update cargo rust-lang/rust#120036

Merged

rustbot added this to the 1.77.0 milestone Jan 17, 2024

$afraca$

afraca reviewed Jan 24, 2024

View reviewed changes

tests/testsuite/profiles.rs Show resolved Hide resolved

Kobzol mentioned this pull request Jan 24, 2024

Fix typo in test #13342

Merged

taiki-e mentioned this pull request Feb 21, 2024

Warning: arm-linux-gnueabihf-strip not found, skip stripping taiki-e/upload-rust-binary-action#59

Closed

daxpedda mentioned this pull request Mar 8, 2024

Deprecate --keep-debug flag rustwasm/wasm-bindgen#3876

Draft

ehuss mentioned this pull request Mar 21, 2024

Fix debuginfo strip when using --target #13618

Merged

ChrisDenton mentioned this pull request Mar 22, 2024

Whitespace trigger CI rust-lang/backtrace-rs#594

Closed

workingjubilee mentioned this pull request Mar 22, 2024

No backtrace on windows with current rustc stable rust-lang/rust#122857

Closed

devjgm mentioned this pull request Mar 23, 2024

1.77 produced release binaries die w/ SIGKILL on macos 14.4 rust-lang/rust#122902

Closed

Kobzol mentioned this pull request Mar 23, 2024

Do not strip debuginfo by default for MSVC #13630

Merged

filnet mentioned this pull request Mar 28, 2024

rust: update to 1.77.1 msys2/MINGW-packages#20397

Merged

ehuss mentioned this pull request Apr 1, 2024

Document variance in strip #13677

Open

Calbabreaker mentioned this pull request Apr 16, 2024

docs(profiles): change strip = "none" to strip = "debuginfo" in r… #13763

Closed

ia0 added a commit to ia0/wasefire that referenced this pull request May 30, 2024

Do not strip debuginfo in debug build

de05a82

This seems to have changed in rust-lang/cargo#13257. Also do not install libusb-1.0 during setup. This doesn't seem necessary anymore.

ia0 mentioned this pull request May 30, 2024

Do not strip debuginfo in debug build google/wasefire#499

Merged

ia0 added a commit to google/wasefire that referenced this pull request May 30, 2024

Do not strip debuginfo in debug build (#499)

6fe24cd

This seems to have changed in rust-lang/cargo#13257. Also do not install libusb-1.0 during setup. This doesn't seem necessary anymore.

ia0 mentioned this pull request May 30, 2024

Insufficient DWARF info with recent nightly probe-rs/probe-rs#2516

Closed

mhnap mentioned this pull request May 31, 2024

Update book: Rust 1.77 changed the default behaviour on debug symbol stripping #13638

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Strip debuginfo when debuginfo is not requested #13257

Strip debuginfo when debuginfo is not requested #13257

Kobzol commented Jan 6, 2024 •

edited

rustbot commented Jan 6, 2024

Kobzol Jan 12, 2024

Kobzol commented Jan 15, 2024

ehuss commented Jan 15, 2024

bors commented Jan 15, 2024

bors commented Jan 15, 2024

bors commented Jan 15, 2024

utkarshgupta137 commented Mar 21, 2024

Kobzol commented Mar 21, 2024

Strip debuginfo when debuginfo is not requested #13257

Strip debuginfo when debuginfo is not requested #13257

Conversation

Kobzol commented Jan 6, 2024 • edited

What does this PR try to resolve?

How should we test and review this PR?

Additional information

Unresolved questions

rustbot commented Jan 6, 2024

Kobzol Jan 12, 2024

Choose a reason for hiding this comment

Kobzol commented Jan 15, 2024

ehuss commented Jan 15, 2024

bors commented Jan 15, 2024

bors commented Jan 15, 2024

bors commented Jan 15, 2024

utkarshgupta137 commented Mar 21, 2024

Kobzol commented Mar 21, 2024

Kobzol commented Jan 6, 2024 •

edited