Build musl dist artifacts with debuginfo enabled

[wesleywiser]

Since our musl targets link to a version of musl we build and bundle
with the targets, if users need to debug into musl or generate
backtraces which contain parts of the musl library, they will be unable
to do so unless we enable and ship the debug info.

This patch changes our dist builds so they enabled debug info when
building musl. This patch also includes a fix for CFI detection in
musl's configure script which has been posted upstream.

The net effect of this is that we now ship debug info for musl in those
targets. This adds ~90kb to those artifacts but running strip on
binaries produced removes all of that. For a "hello world" Rust binary
on x86_64, the numbers are:

	debug	release	release + strip
without musl debuginfo	507kb	495kb	410kb
with musl debuginfo	595kb	584kb	410kb

Once stripped, the final binaries are the same size (down to the byte).

Fixes #90103

r? @Mark-Simulacrum

[joshtriplett]

I think this strengthens the case for doing strip by default in release mode (cc @rust-lang/cargo). But on balance this seems reasonable.

[Mark-Simulacrum]

Perhaps unless debuginfo is explicitly enabled anywhere in the dep-graph (that Cargo can see). But anyway, this change seems reasonable to me; I think people who care about binary sizes probably already apply strip to get rid of std debuginfo.

I've marked as relnotes so we can note the potential (increased) need for applying strip, though.

@wesleywiser can you update this PR with a link to the upstream PR or similar where we're asking for the patch to get merged? I think with that (so it's easier to figure out "stop applying this"), r=me

It may also be worth checking if --enable-debug is enabling full debuginfo (=2) or just the line tables (roughly Rust's =1) or so -- for Rust, at least, that makes a big difference, and might be worth it here as well, since I presume for unwinding you only need line tables (or maybe even less than that?)

[bjorn3]

since I presume for unwinding you only need line tables (or maybe even less than that?)

You don't need line tables for unwinding. Only if you want to know the location inside a file, you need line tables. Without them you still get function names from the symbol table.

[Mark-Simulacrum]

Yeah -- line tables is the wrong term. But it seems likely, still, that --enable-debug may enable more than we need (e.g., line tables, or even finer granularity), so it may be worth finding out if something less 'big' is possible.

[bjorn3]

https://git.musl-libc.org/cgit/musl/tree/configure#n403

it pass -g to the compiler if --enable-debug is used which includes pretty much all debuginfo. I think you can use -g1 to only emit line tables. I think it can be passed using ./configure CFLAGS=-g1.

[Mark-Simulacrum]

It feels like we want something closer to -funwind-tables, right? In theory, for the purpose of getting backtraces through musl you shouldn't actually need to do more than be able to walk that stack -- you don't actually care about stepping through those functions, presumably.

[wesleywiser]

We also need the CFI directives since without those, gdb can't walk the stack out of the syscall function. The test for that though is gated on -g* so just passing -funwind-tables won't work. I believe CFLAGS=-g1 would work. I will test tomorrow and see if we can get away with that and what the impact on binary size is.

[wesleywiser]

With -g1 instead of -g, the debug experience seems the same but we see a significant reduction in the binary size impact:

	debug	release	release with strip
without musl debuginfo	507kb	495kb	410kb
with musl -g1 debuginfo	595kb	584kb	410kb
with musl -g debuginfo	874kb	862kb	410kb

Which looks great to me! I've updated the PR with this approach.

[Mark-Simulacrum]

@wesleywiser can you update this PR with a link to the upstream PR or similar where we're asking for the patch to get merged? I think with that (so it's easier to figure out "stop applying this"), r=me

I think this might've gotten missed -- once you update with the link or some reference, still r=me :)

[wesleywiser]

Oh, I put the link in the PR description ("posted upstream") but I'll update the commit message as well.

[wesleywiser]

@bors r=Mark-Simulacrum

Thanks for the reviews @Mark-Simulacrum, @bjorn3 and @joshtriplett!

[bors]

📌 Commit e97317c has been approved by Mark-Simulacrum

[bors]

⌛ Testing commit e97317c with merge 6de210700de3d511def5d460a5d1d7e768716853...

[rust-log-analyzer]

The job x86_64-gnu-aux failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

error: RPC failed; curl 56 GnuTLS recv error (-54): Error in the pull function.
fatal: The remote end hung up unexpectedly
fatal: early EOF
fatal: index-pack failed
thread 'main' panicked at 'assertion failed: status.success()', src/tools/cargotest/main.rs:125:13


command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/cargotest" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools-bin/cargo" "/checkout/obj/build/ct"
expected success, got: exit status: 101
expected success, got: exit status: 101


Build completed unsuccessfully in 0:28:12
make: *** [check-aux] Error 1
Makefile:44: recipe for target 'check-aux' failed

[bors]

💔 Test failed - checks-actions

[bjorn3]

error: RPC failed; curl 56 GnuTLS recv error (-54): Error in the pull function.
fatal: The remote end hung up unexpectedly
fatal: early EOF
fatal: index-pack failed

Seems spurious.

@bors retry