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

rustc can't find the linker when --sysroot is set to a local build of the sysroot #125246

Closed
RalfJung opened this issue May 18, 2024 · 21 comments · Fixed by #125263
Closed

rustc can't find the linker when --sysroot is set to a local build of the sysroot #125246

RalfJung opened this issue May 18, 2024 · 21 comments · Fixed by #125263
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented May 18, 2024

This is a recent regression, starting with #124129: when --sysroot is set, in my case to /home/r/.cache/cargo-careful where I have prepared a locally built sysroot (that's https://github.com/RalfJung/cargo-careful), then rustc now assumes it can find the linker in the sysroot, which does not work. See RalfJung/cargo-careful#31 for some more details.

It is new to me that the sysroot is supposed to contain the linker as well, is that expected behavior?

Also the error message in that case is really unhelpful.^^

  = note: collect2: fatal error: cannot find 'ld'
          compilation terminated.

Maybe rustc should check that the path it sets for -B actually exists before starting cc?

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 18, 2024
@ChrisDenton
Copy link
Member

It is new to me that the sysroot is supposed to contain the linker as well, is that expected behavior?

Yes. The blog post has a summary of that PR https://blog.rust-lang.org/2024/05/17/enabling-rust-lld-on-linux.html

@RalfJung
Copy link
Member Author

That blog post doesn't mention the term "sysroot" so it doesn't help with my issue.

@RalfJung
Copy link
Member Author

FWIW what I would have expected is that rustc searches for the linker binary somewhere around current_exe(). Isn't that how we find some other things as well that are usually installed with rustc by rustup?

@ChrisDenton
Copy link
Member

How do you handle windows-gnu? Imo this also uses a self-contained linker by default.

@RalfJung
Copy link
Member Author

No idea. CI passes on Windows, whatever github gives us when we say windows-latest.

@lqd
Copy link
Member

lqd commented May 18, 2024

Yeah the rust-lld linker has been in the sysroot since #85961, is used by default in e.g. some of the wasm and aarch64 targets, and rustc had flags to opt-into using it since that PR. What is new is that it is now used by x86_64-unknown-linux-gnu nightlies we distribute.

bootstrap puts it in the sysroot when rust.lld is true (and it's now the default on x64 linux under certain conditions), so you can also set that when ./x build-ing if you want it. It works with or without download-ci-llvm.

@jieyouxu jieyouxu added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label May 18, 2024
@onur-ozkan
Copy link
Member

FWIW what I would have expected is that rustc searches for the linker binary somewhere around current_exe(). Isn't that how we find some other things as well that are usually installed with rustc by rustup?

We resolve the sysroot from rustc driver, not current executable.

@onur-ozkan onur-ozkan removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 18, 2024
@RalfJung
Copy link
Member Author

RalfJung commented May 18, 2024 via email

@ehuss
Copy link
Contributor

ehuss commented May 18, 2024

This broke cargo's CI since the build-std tests try to verify that nothing in the sysroot is required.

Would it be possible to change it so that it will only require rust-lld if it exists in the sysroot (similar to windows-gnu and other targets)?

@onur-ozkan onur-ozkan added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 18, 2024
bors added a commit to rust-lang/cargo that referenced this issue May 18, 2024
Temporarily fix standard_lib tests on linux.

This fixes the standard_lib tests which are broken in the latest nightly. The latest nightly now requires rust-lld to be in the sysroot for x86_64-unknown-linux-gnu. This broke these tests which were trying to verify that the standard library is not required. This temporarily removes this validation, but we should have some way of enforcing it (rust-lang/wg-cargo-std-aware#31).

cc rust-lang/rust#125246
@lqd
Copy link
Member

lqd commented May 18, 2024

I don't know what we do on windows-gnu but we can do something similar, yes. I'll open a PR.

@RalfJung
Copy link
Member Author

How do you handle windows-gnu? Imo this also uses a self-contained linker by default.

FWIW I managed to add tests for windows-gnu and they work like a charm. So no copying of files seems to be required there.
Though I do recall xargo copying some things for windows-gnu so maybe I am not testing enough... but my tests do involve building a small program against the sysroot and running it, so maybe those files are just not needed any more.

@lqd
Copy link
Member

lqd commented May 19, 2024

We actually hit a binutils linker bug last week on windows-gnu (not windows-gnullvm), and the linker flavor there is gnu-cc, so do these targets really use rust-lld by default?

@RalfJung
Copy link
Member Author

How does this work with cross-compilation -- I assume we are always taking the host sysroot even when building for another target?

To me that seems like a sign that this shouldn't be part of the sysroot, it should be part of the rustc distribution. The sysroot is for target-related things.

@RalfJung
Copy link
Member Author

RalfJung commented May 19, 2024

In particular, I would expect that if I set --sysroot to a folder that only contains a sysroot for some foreign target, then that should work if I also set --target appropriately (and use the right linker and everything). But currently that will then likely fail as it tries to search the host target in the --sysroot folder, and with #125263 it will instead silently fall back to the slow linker.

@petrochenkov
Copy link
Contributor

@lqd

We actually hit a binutils linker bug last week on windows-gnu (not windows-gnullvm), and the linker flavor there is gnu-cc, so do these targets really use rust-lld by default?

They do use a self-contained linker from sysroot by default, but not rust-lld.
We ship ld.exe from binutils for those targets.

@lqd
Copy link
Member

lqd commented May 22, 2024

Thanks for the clarification. That explains why there's no existence check (and your 2nd hack for older GCCs in that other issue :). Locally, it was just using mingw's ld probably because of $PATH differences.

@petrochenkov
Copy link
Contributor

@RalfJung

How does this work with cross-compilation -- I assume we are always taking the host sysroot even when building for another target?

To me that seems like a sign that this shouldn't be part of the sysroot, it should be part of the rustc distribution. The sysroot is for target-related things.

Linkers are generally target-dependent, so it's pretty reasonable to ship them in target sysroot, I guess.
rust-lld is just applicable to a wide range of targets by being a cross-linker.

Also there's no target vs host separation for sysroot in rustc right now, there's just one sysroot.
Maybe there's some space for improvements here, it's just not many people use --sysroot to notice any issues.

@petrochenkov
Copy link
Contributor

Thanks for the clarification. That explains why there's no existence check (and your 2nd hack for older GCCs in that other issue :). Locally, it was just using mingw's ld probably because of $PATH differences.

I still hope to migrate both rust-lld and mingw linker to the common directory layout scheme based on self-contained.

@RalfJung
Copy link
Member Author

RalfJung commented May 22, 2024

Linkers are generally target-dependent, so it's pretty reasonable to ship them in target sysroot, I guess.
rust-lld is just applicable to a wide range of targets by being a cross-linker.

If I'm on macOS and want to build for a Linux target, then the rust-lld binary shipped in the x86_64-unknown-linux-gnu sysroot is going to be completely useless to me. So in that sense shipping them in the target sysroot seems pointless. (Of course our cross-compile story is not great so such cross-building will hardly ever work, but it demonstrates the fundamental issue I was pointing out.) Even when I am on x86_64-unknown-linux-gnu and building for i686-unknown-linux-gnu I probably want to use a 64bit program for the linking, and not a binary shipped in the i686 sysroot -- this is e.g. how Firefox is (or was) built as 4GB of RAM (the maximum accessible to 32bit programs) are just not enough for linking.

Given that these are binaries that need to be run on the host (i.e. the machine where the build happens), IMO the only sensible location is together with the other host stuff, i.e., where rustc lives.

@lqd
Copy link
Member

lqd commented May 22, 2024

Ah I think I understand.

If I'm on macOS and want to build for a Linux target, then the rust-lld binary shipped in the x86_64-unknown-linux-gnu sysroot is going to be completely useless to me.

That's right, and on macOS we wouldn't look for rust-lld in the "x86_64-unknown-linux-gnu sysroot" 😅.

You can try this locally on a helloworld (look for the path to gcc-ld), from x86_64-unknown-linux-gnu with a x86_64-unknown-linux-musl target:

$ RUSTC_LOG=rustc_codegen_ssa::back::link RUSTFLAGS="-Clink-self-contained=+linker -Zunstable-options -Zlinker-features=+lld" cargo build --target=x86_64-unknown-linux-musl

(...a big linker command with the following of interest:)

"-B/home/lqd/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin/gcc-ld" "-fuse-ld=lld" ...
"-L" "/home/lqd/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-musl/lib" ...
"-o" "./target/x86_64-unknown-linux-musl/debug/deps/helloworld-e346cf08cdc1bbb0"

$ readelf -p .comment target/x86_64-unknown-linux-musl/debug/helloworld

String dump of section '.comment':
  [     0]  Linker: LLD 18.1.6
  [    14]  rustc version 1.80.0-nightly (b92758a9a 2024-05-20)
  [    48]  GCC: (GNU) 9.4.0

@RalfJung
Copy link
Member Author

Oh, so we always assume that --sysroot contains a sysroot for the host and the current target. That's news to me as well (and Miri certainly doesn't guarantee it).

But the fallback in your PR should solve that as well, I think.

@bors bors closed this as completed in d6a1f1d May 24, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue May 24, 2024
Rollup merge of rust-lang#125263 - lqd:lld-fallback, r=petrochenkov

rust-lld: fallback to rustc's sysroot if there's no path to the linker in the target sysroot

As seen in rust-lang#125246, some sysroots don't expect to contain `rust-lld` and want to keep it that way, so we fallback to the default rustc sysroot if there is no path to the linker in any of the sysroot tools search paths. This is how we locate codegen-backends' dylibs already.

People also have requested an error if none of these search paths contain the self-contained linker directory, so there's also an error in that case.

r? `@petrochenkov` cc `@ehuss` `@RalfJung`

I'm not sure where we check for `rust-lld`'s existence on the targets where we use it by default, and if we just ignore it when missing or emit a warning (as I assume we don't emit an error), so I just checked for the existence of `gcc-ld`, where `cc` will look for the lld-wrapper binaries.

<sub>*Feel free to point out better ways to do this, it's the middle of the night here.*</sub>

Fixes rust-lang#125246
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
8 participants