rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.72k stars 12.5k forks source link

Consider using `llvm-strip` #123151

Open workingjubilee opened 5 months ago

workingjubilee commented 5 months ago

Historically, when handling issues like "the tools on the system we are delivering the toolchain to, for a particular target arch, have lots of individual quirks", Rust has had two primary moves:

On macOS various systems, we are encountering reports in the wild of people having bad inadequately-built strip binaries in their PATH that cannot support all Rust compiler use-cases[^0][^1] (e.g. GNU binutils that is built with only host platform support).

In particular, this means:

We can try to work with Nix, Homebrew, and apparently illumos and most Linux distros to solve this problem, but if this is a persistent issue, and considering the somewhat mysterious errors that result, it may be most productive to simply ship our own copy of strip on these hosts. I believe llvm-strip gets everything right, as far as we are concerned, and would be the choice to "bless" accordingly.

Even if it has its own problems, it would be far easier to ship desired patches to binary support we control, or invite people to push fixes to upstream: we'd just need to cooperate with LLVM (or whoever else supplies this), rather than relying on every single distro cooperating perfectly with us. And I think that it's probably worth it: llvm-strip costs only about 175KB on my system, and probably a little bit of build infra, but pays for itself several times over, obviously: every Rust binary built with the toolchain, thanks to enabling strip, is a few MB lighter for its --release mode.

[^0]: On macOS this is especially bad. The local toolchain comes with llvm-strip which supports mach-o correctly, but in practice, no one uses the local toolchain exclusively. Yeah, XCode is the standard for merely building something, but in order to make a really complete development environment on macOS, you have to add something, like Homebrew, MacPorts, or Nix. So people on macOS are constantly injecting odd strip binaries into their PATH that can't even support the host, leading to us using one that is flawed. These tools try to compensate for the inevitable odd system configurations that result but do so imperfectly, resulting in erroneous behavior. [^1]: By default, it seems e.g. Nix only builds with host support, because the cost of all-targets support is about 60 more MB. I'm sure they're not the only one. 'tis a very "don't pay for what you don't need" mindset to make you install every single cross-build target individually and pay a few dozen MB each time, instead of paying double, once.

workingjubilee commented 5 months ago

This obviously was exacerbated by the recent decision to strip in --release but I should note it was very much preexisting. And I'm not sure who exactly would be responsible for this decision, seems like a shared one.

workingjubilee commented 5 months ago

There are other options, of course, like trying to respect e.g. a $STRIP environment variable, I just don't know if other compilers already respect any such variable and thus if there would be any precedent/intuition/pattern we would find useful. These choices are also not mutually exclusive, either.

It may also be the case that this is sufficiently easy-enough to fix that the package management tools in question will sort this out and we should just twiddle our thumbs for a couple weeks. That would be mildly preferable to macOS hosts having Yet Another strip binary, even if the waste is trivial. I'm mostly just trying to collect the information that we have floating around.

taiki-e commented 5 months ago

I think this would also help to fix the problem with strip in cross-compilation (https://github.com/rust-lang/rust/issues/114411, and if my guess is correct illumos has the same issue not only macOS).

workingjubilee commented 5 months ago

y'all really out here shipping binutils with only one arch supported when ./configure --enable-targets=all is right there, huh

workingjubilee commented 5 months ago

At least one prominent Linux distribution has informed me that their strip does not have cross-compilation support.

the8472 commented 5 months ago

despite the local toolchain having an applicable, useful strip

Another option would be looking at all the available strips on the PATH and picking a preferred one / skipping bad ones. Assuming they can be easily identified.

At least one prominent Linux distribution has informed me that their strip does not have cross-compilation support.

Does that result in an error or in a corrupted binary like on macos? The former doesn't seem so bad since we can turn that into a warning or error.

workingjubilee commented 5 months ago

Some relevant outputs. --version is the usual, --info lists the supported architectures of this binutils.

$ strip --version
GNU strip (GNU Binutils) 2.42.0
Copyright (C) 2024 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) any later version.
This program has absolutely no warranty.
$ llvm-strip --version
llvm-strip, compatible with GNU strip
LLVM (http://llvm.org/):
  LLVM version 17.0.6
  Optimized build.
$ strip --info
BFD header file version (GNU Binutils) 2.42.0
elf64-x86-64
 (header little endian, data little endian)
  i386
elf32-i386
 (header little endian, data little endian)
  i386
elf32-iamcu
 (header little endian, data little endian)
  iamcu
elf32-x86-64
 (header little endian, data little endian)
  i386
pei-i386
 (header little endian, data little endian)
  i386
pe-x86-64
 (header little endian, data little endian)
  i386
pei-x86-64
 (header little endian, data little endian)
  i386
elf64-little
 (header little endian, data little endian)
  i386
  iamcu
  bpf
elf64-big
 (header big endian, data big endian)
  i386
  iamcu
  bpf
elf32-little
 (header little endian, data little endian)
  i386
  iamcu
  bpf
elf32-big
 (header big endian, data big endian)
  i386
  iamcu
  bpf
pe-bigobj-x86-64
 (header little endian, data little endian)
  i386
pe-i386
 (header little endian, data little endian)
  i386
pdb
 (header little endian, data little endian)
elf64-bpfle
 (header little endian, data little endian)
  bpf
elf64-bpfbe
 (header big endian, data big endian)
  bpf
srec
 (header endianness unknown, data endianness unknown)
  i386
  iamcu
  bpf
symbolsrec
 (header endianness unknown, data endianness unknown)
  i386
  iamcu
  bpf
verilog
 (header endianness unknown, data endianness unknown)
  i386
  iamcu
  bpf
tekhex
 (header endianness unknown, data endianness unknown)
  i386
  iamcu
  bpf
binary
 (header endianness unknown, data endianness unknown)
  i386
  iamcu
  bpf
ihex
 (header endianness unknown, data endianness unknown)
  i386
  iamcu
  bpf
plugin
 (header little endian, data little endian)

         elf64-x86-64 elf32-i386 elf32-iamcu elf32-x86-64 pei-i386 pe-x86-64 
    i386 elf64-x86-64 elf32-i386 ----------- elf32-x86-64 pei-i386 pe-x86-64
   iamcu ------------ ---------- elf32-iamcu ------------ -------- ---------
     bpf ------------ ---------- ----------- ------------ -------- ---------

         pei-x86-64 elf64-little elf64-big elf32-little elf32-big 
    i386 pei-x86-64 elf64-little elf64-big elf32-little elf32-big
   iamcu ---------- elf64-little elf64-big elf32-little elf32-big
     bpf ---------- elf64-little elf64-big elf32-little elf32-big

         pe-bigobj-x86-64 pe-i386 pdb elf64-bpfle elf64-bpfbe srec symbolsrec 
    i386 pe-bigobj-x86-64 pe-i386 --- ----------- ----------- srec symbolsrec
   iamcu ---------------- ------- --- ----------- ----------- srec symbolsrec
     bpf ---------------- ------- --- elf64-bpfle elf64-bpfbe srec symbolsrec

         verilog tekhex binary ihex plugin 
    i386 verilog tekhex binary ihex ------
   iamcu verilog tekhex binary ihex ------
     bpf verilog tekhex binary ihex ------
$ llvm-strip --info
llvm-strip: error: unknown argument '--info'
workingjubilee commented 5 months ago

I'm going to push this into T-compiler's agenda for next week as the possible solutions all involve affecting rustc's behavior at least mildly.

workingjubilee commented 5 months ago

Hmm. I spent some time dinking about and it seems we do in fact ship llvm-tools as a rustup component and they are no longer a "preview" anything, so the question would be if they should be considered effectively mandatory or not.

ChrisDenton commented 5 months ago

There was a discussion on zulip about llvm-tools.

wesleywiser commented 5 months ago

I think using llvm-strip is entirely reasonable here especially given the extremely small size of the tool. If we don't want to rely on llvm-tools being installed, I think it would be reasonable to ship it in the sysroot (possibly duplicating it with one in llvm-tools).

Mark-Simulacrum commented 5 months ago

Please don't rely on llvm-tools for this purpose, it's lack of -preview was just an accident (not an intentional stabilization).

It's also a larger component and we shouldn't make it downloaded by default for users, IMO, and as such the experience of rustc/cargo shouldn't degrade without it. I think duplicating llvm-strip as a sibling to rust-lld makes a lot of sense to give a uniform experience for users.

workingjubilee commented 4 months ago

Okay cool! Then it sounds like folks are largely on-board with "ship llvm-strip, but not the full llvm-tools component, just llvm-strip", and it's mostly down to PRing the changes. Will try to take a look at that in a few days.