Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust 1.77 will clash harder with GNU binutils on Darwin Nix #299606

Open
workingjubilee opened this issue Mar 27, 2024 · 27 comments
Open

Rust 1.77 will clash harder with GNU binutils on Darwin Nix #299606

workingjubilee opened this issue Mar 27, 2024 · 27 comments
Labels
0.kind: bug 6.topic: darwin Running or building packages on Darwin 6.topic: rust

Comments

@workingjubilee
Copy link

workingjubilee commented Mar 27, 2024

Describe the bug

cargo 1.77 started stripping debuginfo by default for cargo build --release in order to make Rust binaries smaller by default, but on macOS, if somehow the "wrong" (GNU binutils ~2.41, I believe?) strip gets into the PATH, the strip call will result in a segfaulting binary when executed. nixpkgs does try to work around this already, and things work fine when the "right" strip is the path.

based on bug reports from macOS users, this is often not enough for many actually-extant user configurations. macOS users that use Nix are running into this anyways, due to their PATH rearranging (possibly after entering a nix shell? flake? no idea...). it is possible some specific Nix-based tooling is causing the problem, as a result, and not nixpkgs per se, but we have not gotten enough information from reports to narrow this down... it's simultaneously pervasive and elusive, despite the known reasons.

I decided to file this bug, even though it is incredibly vague, because you may wish to figure out a position on this issue before you land an upgrade to the Rust 1.77 toolchain for nixpkgs, and because people seem to prefer opening the issue against rust-lang as it regresses on the latest version of the toolchain, despite being a preexisting issue (i.e. the "wrong strip" could already be a problem if someone enabled it in their Cargo.toml's build settings, or used it "by hand"). please feel free to close it whenever.

Steps To Reproduce

steps to reproduce the behavior:

  1. put the GNU strip first in the PATH while on macOS
  2. set up the rust 1.77 toolchain
  3. cargo run --release

technically this also happens with compiling a C program and stripping it "manually", etc.

Expected behavior

printing "Hello world!", probably.

Screenshots

nah.

Additional context

apparently homebrew also sometimes causes this problem.

Notify maintainers

idk who should be bothered over this.

Metadata

not applicable.

Add a 👍 reaction to issues you find important.

@n8henrie
Copy link
Contributor

Thanks for the issue -- if anyone is interested I'm happy to help on the nix side.

Here's the issue I started at rust-lang: rust-lang/rust#123114

TBF the reason I started the issue at rust-lang is because cargo is calling strip as an external binary by PATH lookup without any other mechanisms for specifying the absolute path to the proper binary. This means that if I happened to make a personal tool named strip that were early on my PATH, it would break things as well, without an obvious remedy other than "don't put that early on your PATH". It would be nice for cargo to provide an escape hatch here!

But yes, I agree that it seems that nix's strip breaks rust binaries, which is a problem!

@eclairevoyant eclairevoyant added the 6.topic: darwin Running or building packages on Darwin label Mar 27, 2024
@winterqt
Copy link
Member

cc @toonn as we were recently discussing a different strip issue

@workingjubilee
Copy link
Author

someone putting novel utilities in their path that have the same name as an ancient piece of Unix tooling that is sufficiently commonplace as to have a POSIX spec since at least 15 years ago seems like an invitation for trouble in general, akin to making a tool for closed captioning and naming it cc. nonetheless perhaps we should be referencing some env var, much like we, if memory serve, respect $CC. I think the only reason we don't already is that there's no equally-hoary common standard for an env var that names "path to the strip binary that should be used" afaik, so we'd be making something up.

@reckenrode
Copy link
Contributor

reckenrode commented Mar 27, 2024

To clarify, is it only strip from GNU binutils or also the one from darwin.cctools-port and llvmPackages.bintools? Those are all different implementations with different quirks and issues. Darwin currently uses llvm-strip by default.

nixpkgs does try to work around this already, and things work fine when the "right" strip is the path.

I wonder if darwin.binutils should symlink everything from LLVM and not bother with GNU binutils or cctools (except for ld64 for now). Darwin will go useLLVM at some point. (I’d like to try it with the LLVM 18 update, but I’m skeptical about it.)

@n8henrie
Copy link
Contributor

@workingjubilee That's totally fair, though your argument sounds a lot like "users ignorant enough to poorly name a personal script should know better." Also, the inscrutable failure modes (just a (signal: 9, SIGKILL: kill)) make it pretty tough for a lowly hobbyist like myself to figure out what's going on or what to do about it. It seems that there are a few ways that rust could make this much easier -- shelling out to an external binary without any knobs to turn makes it tough for me to decide how I should work around this issue.

@reckenrode does this answer your question?

$ readlink -f $(type -p strip)
/nix/store/j0w1bxkg0lyw39dwsh3zpqzac2l05dkp-binutils-wrapper-2.40/bin/strip

@n8henrie
Copy link
Contributor

n8henrie commented Mar 27, 2024

$ cat run.c
#include <stdio.h>

int main() {
    printf("hello there\n");
    return 0;
}
$ clang run.c
$ ./a.out
hello there
$ /nix/store/j0w1bxkg0lyw39dwsh3zpqzac2l05dkp-binutils-wrapper-2.40/bin/strip ./a.out
$ ./a.out
Killed: 9
$ sw_vers
ProductName:            macOS
ProductVersion:         14.3.1
BuildVersion:           23D60
$ nix-info -m
 - system: `"aarch64-darwin"`
 - host os: `Darwin 23.3.0, macOS 14.3.1`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.19.3`
 - channels(n8henrie): `""`
 - channels(root): `""`
 - nixpkgs: `/nix/store/46bkbw4lb5qkn2a3nz3smmwbf8nlhykd-source`

@workingjubilee
Copy link
Author

@n8henrie I don't want to digress too far since this is about how to solve this issue, but just to explain, my position isn't "users should know better" and more "in the absence of an explicit alternative, $PATH is the directive where we should be looking for the strip tool"... I am afraid that if we made something up but got it wrong and misjudged what we should be doing, as we have before, we'd have made it virtually impossible for you to debug rather than merely pretty tough.

@reckenrode
Copy link
Contributor

reckenrode commented Mar 28, 2024

@reckenrode does this answer your question?

$ readlink -f $(type -p strip)
/nix/store/j0w1bxkg0lyw39dwsh3zpqzac2l05dkp-binutils-wrapper-2.40/bin/strip

How did that get in the path? The strip in the Darwin stdenv is llvm-strip. The only way to get strip from GNU binutils is by adding the top-level binutils as a native build input, to environment.systemPackages in nix-darwin, nix-env -i, etc.

I agree this is an issue that should be solved because users could have some particular strip on their path, and it would be bad if that resulted in a broken binary. However, I’d also like to understand how GNU binutils is being used in nixpkgs on Darwin. There’s not really a good reason to use it, especially if strip still causes problems with Mach-O binaries.

(If Rust can’t be made to invoke an absolute-pathed strip, can rustc or cargo in nixpkgs be wrapped with a known-working strip first on its path?)

@workingjubilee
Copy link
Author

workingjubilee commented Mar 28, 2024

I believe Cargo is the one that demands that we strip but rustc is the actually caller of strip, and there's already a rustc-wrapper in tree:

exec @prog@ "${extraBefore[@]}" "$@" "${extraAfter[@]}"

@alyssais
Copy link
Member

How does rustc determine which strip to use when cross compiling?

@n8henrie
Copy link
Contributor

@reckenrode

How did that get in the path? ... The only way to get strip from GNU binutils is by adding the top-level binutils as a native build input

Correct, because I have binutils in my environment.systemPackages. I'm a relatively recent (1-2 years?) convert from homebrew*, and one of my first steps to ease the transition was just putting into systemPackages most anything I had installed system-wide with homebrew. I use tools like nm, objdump, and strings from time to time, and it's inconvenient to have to enter a nix shell for a quick one-off command (startup time, etc.), so I included binutils in the list of "things I always want installed." I assumed that using the nix copy would help me keep behavior consistent between MacOS and Linux, as it's a right pain to have to remember the flags that MacOS does / doesn't have.

* Of note, I previously ran into the same issue with rust / strip / SIGKILL on homebrew

I’d also like to understand how GNU binutils is being used in nixpkgs on Darwin. There’s not really a good reason to use it, especially if strip still causes problems with Mach-O binaries.

As noted, I'm using other tools in the package, not strip specifically.

If Rust can’t be made to invoke an absolute-pathed strip

My suggestion at rust-lang/rust#123114 was simply to prefix PATH with /usr/bin: (on MacOS specifically) (likely here). As strip is the only binary that has given me a problem multiple times over multiple years across multiple system setups (and AFAIK the only tool that's being shelled out like this), it seems like this would be a simple (if inelegant) solution.

@reckenrode
Copy link
Contributor

reckenrode commented Mar 28, 2024

@n8henrie Thanks for the explanation. If flag-compatibility between platforms is important to you, then using darwin.cctools instead wouldn’t be a solution (since while it is closer to what the platform ships, the flags are different).

My concern regarding nixpkgs was if any packages were using GNU binutils on Darwin. Nothing should be, especially if Mach-O support hasn’t gotten much better — that linked workaround was committed almost a decade ago.

As an aside, I’m probably going to open a PR to rework darwin.binutils to symlink everything from llvmPackages.bintools. The only GNU binutils program it links is c++filt, which LLVM provides (and the LLVM version is what Apple ships anyway). That should cut down on build time since llvmPackages.bintools has to be built anyway.

My suggestion at rust-lang/rust#123114 was simply to prefix PATH with /usr/bin: (on MacOS specifically) (likely here). As strip is the only binary that has given me a problem multiple times over multiple years across multiple system setups (and AFAIK the only tool that's being shelled out like this), it seems like this would be a simple (if inelegant) solution.

Forcing the path in rustc would be a problem for nixpkgs because it would require patching Rust to use a path from nixpkgs instead. While the in-tree derivation could do that, there are popular overlays that provide upstream’s binaries. Those couldn’t be patched. They would try to invoke strip at /usr/bin/strip, which would fail if Xcode or the command-line tools are not installed or if Rust is being used in a sandboxed build. Adding it to the wrapper, which appears to have been a recent addition, would avoid patching and work with the overlays (provide they are updated to use the wrapper).

@workingjubilee
Copy link
Author

How does rustc determine which strip to use when cross compiling?

It seems we benignly assumed people would build strip with --enable-targets=all.

@n8henrie
Copy link
Contributor

I appreciate the explanation.

it would require patching Rust to use a path from nixpkgs instead

Just to clarify -- do you mean "require" for purity? Or for functionality? The specific function I'm referring to is only for calling strip (not any other external tool), which is based on a runtime PATH lookup already. It seems to me that both purity and functionality are somewhat compromised in the current situation:

$ type -a cargo
cargo is /run/current-system/sw/bin/cargo
$ cargo +nightly --version
cargo 1.78.0-nightly (2fe739fcf 2024-03-15)
$ cargo +nightly install -f cargo-espflash
    Updating crates.io index
  Installing cargo-espflash v3.0.0
    Updating crates.io index
   Compiling proc-macro2 v1.0.79
   Compiling unicode-ident v1.0.12
   Compiling libc v0.2.153
   Compiling cfg-if v1.0.0
   Compiling serde v1.0.197
   Compiling pkg-config v0.3.30
   Compiling vcpkg v0.2.15
   Compiling memchr v2.7.2
   Compiling thiserror v1.0.58
error: failed to run custom build command for `proc-macro2 v1.0.79`

Caused by:
  process didn't exit successfully: `/var/folders/kb/tw_lp_xd2_bbv0hqk4m0bvt80000gn/T/cargo-installYvXyxp/release/build/proc-macro2-a08fba1383bb57dd/build-script-build` (signal: 9, SIGKILL: kill)
warning: build failed, waiting for other jobs to finish...
error: failed to run custom build command for `serde v1.0.197`

Caused by:
  process didn't exit successfully: `/var/folders/kb/tw_lp_xd2_bbv0hqk4m0bvt80000gn/T/cargo-installYvXyxp/release/build/serde-27272797f677f9c1/build-script-build` (signal: 9, SIGKILL: kill)
error: failed to compile `cargo-espflash v3.0.0`, intermediate artifacts can be found at `/var/folders/kb/tw_lp_xd2_bbv0hqk4m0bvt80000gn/T/cargo-installYvXyxp`.
To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path.

Yes, I know cargo-espflash is packaged in nipxkgs, but it's an old version, and I just don't always have the time or energy to repackage something for a one-off command. As a workaround I can PATH=/usr/bin:$PATH cargo +nightly install ....

$ nix eval nixpkgs#cargo-espflash.version
"2.1.0"

Perhaps I had originally installed GNU binutils for some microcontroller work? Trying to remember. I guess the obvious solution is just to not have GNU binutils installed system-wide, since nix is much better at this than homebrew it shouldn't be a problem.

@workingjubilee
Copy link
Author

Just to clarify -- do you mean "require" for purity? Or for functionality? The specific function I'm referring to is only for calling strip (not any other external tool), which is based on a runtime PATH lookup already.

Purity is functionality if someone is trying to run rustc in a container. It is very common for rustc to be invoked in a situation where it does not have privileges sufficient to access /usr/bin, but rather is given a PATH that includes a directory that has the specific set of things it should have access to. This is a very sensible move because building Rust code, like most build systems, involves arbitary code execution, both in the forms of build.rs and proc-macros.

@alyssais
Copy link
Member

It seems we benignly assumed people would build strip with --enable-targets=all.

This is not the case in Nixpkgs, because it makes the binutils package huge. So stripping by default sounds like it's going to break in the cross case without some better configuration mechanism, even if there is a strip in PATH?

@workingjubilee
Copy link
Author

...how huge, exactly?

@alyssais
Copy link
Member

It's an extra ~60 MiB. (Default binutils it's 84.8 MiB, and then the wrapper adds 18.1 MiB.) The comment says "WARN: Enabling all targets increases output size to a multiple.", which makes me think the difference used to be bigger.

@n8henrie
Copy link
Contributor

n8henrie commented Mar 28, 2024

Purity is functionality

Sorry, I was trying to clarify in what sense @reckenrode was using "require" (break the build vs not best practice vs other) as the current situation with nix on darwin still does require reaching out to some extra-nix paths at times (libsystem, oxalica/rust-overlay#149 -- someday I'll perhaps understand 10% of @reckenrode's work on this!).

Currently, rust is reaching out to "whatever strip is first on PATH". I had thought that having it look for strip (not affecting anything else) at "/usr/bin/strip if (cfg(target_os = "macos") and this path exists), otherwise fall back to the current behavior" seemed like an easy solution that would resolve the issue for myself and at least a few other MacOS + nix / homebrew users.

I suppose this would potentially break cross compilation through nix though (presuming rust goes in nativeBuildInputs and therefore hostSystem == aarch64-darwin and targetSystem == ...-linux -- is that right?).

And regardless, I am starting to get the feeling that people with much more knowledge and experience think my suggestion is suboptimal, so I'll lay off 😆 Thanks for your patience!

@reckenrode
Copy link
Contributor

reckenrode commented Mar 28, 2024

Purity is functionality

Sorry, I was trying to clarify in what sense @reckenrode was using "require" (break the build vs not best practice vs other) as the current situation with nix on darwin still does require reaching out to some extra-nix paths at times (libsystem, oxalica/rust-overlay#149 -- someday I'll perhaps understand 10% of @reckenrode's work on this!).

By “require”, I mean the package function has binutils as an argument, which callPackage fills with pkgs.binutils (i.e., GNU binutils). Darwin is supported by GNU binutils (and mostly works), so there is no reason to mark it broken on that platform, but I would consider using it in nixpkgs an anti-pattern if not a mistake.

The situation in that PR is because they’re using binaries built outside of nixpkgs that link to system libraries, which expect to access other system resources and configuration files. That’s going to require sandbox exceptions or other adjustments occasionally.

Edit: I saw the other comment regarding “require”, but I’ll keep the above to clarify what I meant in nixpkgs. I’ll respond to the other separately.

@reckenrode
Copy link
Contributor

Just to clarify -- do you mean "require" for purity? Or for functionality? The specific function I'm referring to is only for calling strip (not any other external tool), which is based on a runtime PATH lookup already.

A hardcoded reference to /usr/bin would introduce an impurity. Instead of picking up the strip from the stdenv or nix-shell, it would always try to use the system one first. It would also result in different behavior depending on whether a build is done with sandboxing enabled.

Typically, those kinds of references are patched out in when the derivation builds the package. That’s easy for source builds, but it’s not really possible for binary overlays. It would be better for nixpkgs if both could use a wrapper than ensured a compatible strip was always available first in the PATH.

@workingjubilee
Copy link
Author

@alyssais Hmm. By default, binutils supports some very esoteric targets. What about if you enable binutils support for "just" all architectures that Nix and rustc support, by default? Or optimize for size?

@alyssais
Copy link
Member

@vcunat what do you think about the binutils all targets size? I think from looking at the commit logs it was you that set it up not to build all of them. Is the current size saving worth it?

@vcunat
Copy link
Member

vcunat commented Mar 28, 2024

I don't recall anything around that. binutils probably shouldn't be in normal closures, but it will be wherever building happens, which is most NixOS machines I expect. Probably don't mind me in this 🤷🏽

@workingjubilee
Copy link
Author

llvm-strip (which is just llvm-objcopy, but often strip is objcopy, so that's nothing new) should serve most of our needs and afaict costs less than a megabyte of additional weight on the system. 175kb it seems, perhaps less. If Nix would prefer, we could potentially just ship that with our toolchain, so that we take ownership of making sure it works with Rust binaries.

@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/darwin-updates-news/42249/10

@tie
Copy link
Member

tie commented May 30, 2024

This might be fixed by #314920 (or at least to a certain degree) that sets STRIP environment variable to an absolute path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: bug 6.topic: darwin Running or building packages on Darwin 6.topic: rust
Projects
None yet
Development

No branches or pull requests

9 participants