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

self-contained linker: retry linking without -fuse-ld=lld on CCs that don't support it #125417

Merged
merged 1 commit into from
May 23, 2024

Conversation

lqd
Copy link
Member

@lqd lqd commented May 22, 2024

For the self-contained linker, this PR applies the strategy of retrying the linking step when the driver doesn't support -fuse-ld=lld, but with the option removed. This is the same strategy we already use of retrying when e.g. -no-pie is not supported.

Fixes #125330
r? @petrochenkov

I have no idea how we could add a test here, much like we don't have one for -no-pie or -static-pie -- let me know if you have ideas -- but I tested on a CentOS7 image:

[root@d25b38376ede tmp]# ../build/host/stage1/bin/rustc helloworld.rs
 WARN rustc_codegen_ssa::back::link The linker driver does not support `-fuse-ld=lld`. Retrying without it.

[root@d25b38376ede tmp]# readelf -p .comment helloworld

String dump of section '.comment':
  [     0]  GCC: (GNU) 4.8.5 20150623 (Red Hat 4.8.5-44)
  [    2d]  rustc version 1.80.0-dev

I wasn't able to test with cross as the issue describes: I wasn't able to reproduce that behavior locally.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 22, 2024
@lqd
Copy link
Member Author

lqd commented May 22, 2024

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2024
self-contained linker: retry linking without `-fuse-ld=lld` on CCs that don't support it

For the self-contained linker, this PR applies [the strategy](rust-lang#125330 (comment)) of retrying the linking step when the driver doesn't support `-fuse-ld=lld`, but with the option removed. This is the same strategy we already use of retrying when e.g. `-no-pie` is not supported.

Fixes rust-lang#125330
r? `@petrochenkov`

I have no idea how we could add a test here, much like we don't have one for `-no-pie` or `-static-pie` -- let me know if you have ideas -- but I tested on a CentOS7 image:

```console
[root@d25b38376ede tmp]# ../build/host/stage1/bin/rustc helloworld.rs
 WARN rustc_codegen_ssa::back::link The linker driver does not support `-fuse-ld=lld`. Retrying without it.

[root@d25b38376ede tmp]# readelf -p .comment helloworld

String dump of section '.comment':
  [     0]  GCC: (GNU) 4.8.5 20150623 (Red Hat 4.8.5-44)
  [    2d]  rustc version 1.80.0-dev
```

(I wasn't able to test with `cross` as the issue describes: I wasn't able to reproduce that behavior locally. So I'll ask for help from the OP with a try build)
@bors
Copy link
Contributor

bors commented May 22, 2024

⌛ Trying commit 5da41f5 with merge 5572f38...

@lqd
Copy link
Member Author

lqd commented May 22, 2024

@VorpalBlade I couldn't test with cross as you described in #125330, the issue doesn't reproduce for me locally. I'm not sure if cross' images may have changed in the last few days, but it doesn't look like it in the cross repository. From a quick look at their commits, it seemed there wasn't any that would help fix this issue either.

Here, cargo add syn && cross +nightly build --target aarch64-unknown-linux-gnu succeeds. The binary's .comment looks like it used at least 1 GCC in the docker image that supports -fuse-ld=lld, which could explain why it worked.

$ readelf -p .comment target/aarch64-unknown-linux-gnu/debug/helloworld

String dump of section '.comment':
  [     0]  GCC: (Ubuntu 9.4.0-1ubuntu1~20.04.2) 9.4.0
  [    2b]  rustc version 1.80.0-nightly (b92758a9a 2024-05-20)
  [    5f]  GCC: (crosstool-NG UNKNOWN) 8.5.0

I don't know if you have another way to reproduce this issue that I could try?

I've tested on a CentOS7 image with gcc 4.5 and a helloworld worked there, so I have high hopes it will fix your use-case but I'd like to make sure with an actual reproduction.


Otherwise, I've asked CI to do a try build, which is similar to a rustup toolchain for this PR, and maybe you would be able to test it where nightly fails?

With rustup-toolchain-install-master, we can install CI artifacts and use them as regular rustup toolchains.

When CI completes (the bot will post a message that the build was successful), rustup-toolchain-install-master 5572f385cb1b21326f0cf6672b3f74a172b5dcfa will install the toolchain, and you'd be able to use it where you encountered the issue: with +nightly replaced by +5572f385cb1b21326f0cf6672b3f74a172b5dcfa.

To be honest I have a suspicion this might also fail, for unrelated reasons: I don't believe CI will build other std targets here, unfortunately. That seemed necessary in the cross-compilation use-case, but maybe cargo's -Zbuild-std could work here and build std. That's why I'm still mentioning this possibility.

@petrochenkov
Copy link
Contributor

r=me after testing what you want to test.
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 22, 2024
@bors
Copy link
Contributor

bors commented May 22, 2024

☀️ Try build successful - checks-actions
Build commit: 5572f38 (5572f385cb1b21326f0cf6672b3f74a172b5dcfa)

@lqd
Copy link
Member Author

lqd commented May 22, 2024

Let me try to cook something up with CI: @bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2024
self-contained linker: retry linking without `-fuse-ld=lld` on CCs that don't support it

For the self-contained linker, this PR applies [the strategy](rust-lang#125330 (comment)) of retrying the linking step when the driver doesn't support `-fuse-ld=lld`, but with the option removed. This is the same strategy we already use of retrying when e.g. `-no-pie` is not supported.

Fixes rust-lang#125330
r? `@petrochenkov`

I have no idea how we could add a test here, much like we don't have one for `-no-pie` or `-static-pie` -- let me know if you have ideas -- but I tested on a CentOS7 image:

```console
[root@d25b38376ede tmp]# ../build/host/stage1/bin/rustc helloworld.rs
 WARN rustc_codegen_ssa::back::link The linker driver does not support `-fuse-ld=lld`. Retrying without it.

[root@d25b38376ede tmp]# readelf -p .comment helloworld

String dump of section '.comment':
  [     0]  GCC: (GNU) 4.8.5 20150623 (Red Hat 4.8.5-44)
  [    2d]  rustc version 1.80.0-dev
```

(I wasn't able to test with `cross` as the issue describes: I wasn't able to reproduce that behavior locally. So I'll ask for help from the OP with a try build)

try-job: dist-aarch64-linux
try-job: dist-x86_64-linux
@bors
Copy link
Contributor

bors commented May 22, 2024

⌛ Trying commit 5da41f5 with merge 78c8355...

@VorpalBlade
Copy link

VorpalBlade commented May 22, 2024

@lqd I will give this a try. As the build isn't done this evening and it is already pretty late, the next time I will have time to give this a try will be Friday evening, as tomorrow evening is DnD night (well actually not DnD, a different pen and paper RPG, but you get the point).

@lqd
Copy link
Member Author

lqd commented May 22, 2024

@VorpalBlade The previous build is done. I just queued a new one in case I'd be able to get enough artifacts for cross-compiling.

@VorpalBlade
Copy link

VorpalBlade commented May 22, 2024

@lqd Apparently I misread your bot, try build and try commit being two different things? That is slightly confusing to an outsider. Nope, I was even more confused and didn't notice there were two builds. My bad. It is late locally.

So:

rustup-toolchain-install-master 5572f385cb1b21326f0cf6672b3f74a172b5dcfa
detecting the channel of the `5572f385cb1b21326f0cf6672b3f74a172b5dcfa` toolchain...
downloading <https://ci-artifacts.rust-lang.org/rustc-builds/5572f385cb1b21326f0cf6672b3f74a172b5dcfa/rustc-nightly-x86_64-unknown-linux-gnu.tar.xz>...
74.12 MB / 74.12 MB [=====================================================================================================================================================] 100.00 % 14.73 MB/s 
downloading <https://ci-artifacts.rust-lang.org/rustc-builds/5572f385cb1b21326f0cf6672b3f74a172b5dcfa/rust-std-nightly-x86_64-unknown-linux-gnu.tar.xz>...
27.49 MB / 27.49 MB [======================================================================================================================================================] 100.00 % 2.99 MB/s 
toolchain `5572f385cb1b21326f0cf6672b3f74a172b5dcfa` is successfully installed!cross +5572f385cb1b21326f0cf6672b3f74a172b5dcfa build --target aarch64-unknown-linux-gnu
error: error: invalid value '5572f385cb1b21326f0cf6672b3f74a172b5dcfa-x86_64-unknown-linux-gnu' for '<toolchain>...': invalid toolchain name: '5572f385cb1b21326f0cf6672b3f74a172b5dcfa-x86_64-unknown-linux-gnu'

For more information, try '--help'.
: invalid toolchain name: '5572f385cb1b21326f0cf6672b3f74a172b5dcfa-x86_64-unknown-linux-gnu'
Error: 
   0: couldn't install toolchain `5572f385cb1b21326f0cf6672b3f74a172b5dcfa-x86_64-unknown-linux-gnu`
   1: `rustup toolchain add 5572f385cb1b21326f0cf6672b3f74a172b5dcfa-x86_64-unknown-linux-gnu --profile minimal` failed with exit status: 1rustup-toolchain-install-master 5572f385cb1b21326f0cf6672b3f74a172b5dcfa -t aarch64-unknown-linux-gnu
toolchain `5572f385cb1b21326f0cf6672b3f74a172b5dcfa` is already installed

Expected? Any ideas on how to proceed? Would you would need the toolchain inside the docker image cross builds with? I'm not actually sure how cross works, I just used it as a tool.

@lqd
Copy link
Member Author

lqd commented May 22, 2024

Would you would need the toolchain inside the docker image cross builds with?

I don't know cross, so I couldn't tell you.

It also seems to synthesize a toolchain name that makes no sense.

And the last thing you tried is what I mentioned would fail (but here, it just ignored your request, as the toolchain was already installed, one would add the target with rustup, not rustup-toolchain-install-master which you only use to install the initial toolchain), and am trying to finagle CI for it not to fail. I don't know if it'll work.

Trying to install the toolchain with a custom name like rustup-toolchain-install-master 5572f385cb1b21326f0cf6672b3f74a172b5dcfa -n 5572f385cb1b21326f0cf6672b3f74a172b5dcfa-x86_64-unknown-linux-gnu will probably fix the cross error though.

@VorpalBlade
Copy link

VorpalBlade commented May 22, 2024

cross +5572f385cb1b21326f0cf6672b3f74a172b5dcfa build --target aarch64-unknown-linux-gnu
Error: 
   0: `rustup target list --toolchain 5572f385cb1b21326f0cf6672b3f74a172b5dcfa-x86_64-unknown-linux-gnu` failed with exit status: 1

Stderr:
   error: error: invalid value '5572f385cb1b21326f0cf6672b3f74a172b5dcfa-x86_64-unknown-linux-gnu' for '--toolchain <toolchain>': invalid toolchain name: '5572f385cb1b21326f0cf6672b3f74a172b5dcfa-x86_64-unknown-linux-gnu'

   For more information, try '--help'.
   : invalid toolchain name: '5572f385cb1b21326f0cf6672b3f74a172b5dcfa-x86_64-unknown-linux-gnu'

Hm, not looking good with the custom name, and a different error too. We could try pinging the the developer involved in cross-rs/cross#1496 (the cross issue about the same thing), because a quick look through the wiki for cross didn't show any way to do this. @Emilgardis

@lqd
Copy link
Member Author

lqd commented May 22, 2024

I tested this on the same kind of gcc that the cross docker images should contain, so it's not the end of the world if you don't know how I can reproduce the issue, or if cross can't use the CI artifacts. We think it should work, and you'd be able to test once it lands on master as a real toolchain, with the expected std for cross compilation of course.

If it doesn't then you'd let us know :)

@lqd
Copy link
Member Author

lqd commented May 22, 2024

(The invalid toolchain name is not from cross but from rustup)

@bors
Copy link
Contributor

bors commented May 22, 2024

☀️ Try build successful - checks-actions
Build commit: 78c8355 (78c8355821161d05841b58cd7c27ff0bb679847d)

@Emilgardis
Copy link
Contributor

@VorpalBlade the way to use the custom toolchain would be CROSS_CUSTOM_TOOLCHAIN=1

@VorpalBlade
Copy link

VorpalBlade commented May 22, 2024

@lqd

rustup-toolchain-install-master 5572f385cb1b21326f0cf6672b3f74a172b5dcfa -t aarch64-unknown-linux-gnu
detecting the channel of the `5572f385cb1b21326f0cf6672b3f74a172b5dcfa` toolchain...
downloading <https://ci-artifacts.rust-lang.org/rustc-builds/5572f385cb1b21326f0cf6672b3f74a172b5dcfa/rustc-nightly-x86_64-unknown-linux-gnu.tar.xz>...
74.12 MB / 74.12 MB [=====================================================================================================================================================] 100.00 % 18.31 MB/s 
downloading <https://ci-artifacts.rust-lang.org/rustc-builds/5572f385cb1b21326f0cf6672b3f74a172b5dcfa/rust-std-nightly-aarch64-unknown-linux-gnu.tar.xz>...
error: missing component `rust-std` on toolchain `5572f385cb1b21326f0cf6672b3f74a172b5dcfa` on channel `nightly` for target `aarch64-unknown-linux-gnu`

Well that didn't seem to work either. I can either try again during Friday evening or when this has been merged to nightly.

@Emilgardis
Copy link
Contributor

nevermind, seems rustup-toolchain-master installs the toolchains a bit different to cargo-bisect-rusct, and doesn't really work with cross +toolchain handling. I'll try to make a fix for it.

Anyway, this pr should resolve the issue.

@lqd
Copy link
Member Author

lqd commented May 22, 2024

It seems rustup-toolchain-install-master installs toolchains that don't support components, so adding the target will fail anyways if cross does indeed do it this way:

$ rustup +78c8355821161d05841b58cd7c27ff0bb679847d target add aarch64-unknown-linux-gnu
error: toolchain '78c8355821161d05841b58cd7c27ff0bb679847d' does not support components

However, if it can accept a ready-made toolchain (and rustup doesn't fail when listing targets like we had above):
rustup-toolchain-install-master 78c8355821161d05841b58cd7c27ff0bb679847d -t x86_64-unknown-linux-gnu -t aarch64-unknown-linux-gnu will work.

Anyway, this pr should resolve the issue.

agreed

@lqd
Copy link
Member Author

lqd commented May 22, 2024

Alrighty. We can't test with cross until this lands, we know it works on CentOS7, and it should it fix it there too.

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented May 22, 2024

📌 Commit 5da41f5 has been approved by petrochenkov

It is now in the queue for this repository.

@bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 22, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request May 23, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#122665 (Add some tests for public-private dependencies.)
 - rust-lang#123623 (Fix OutsideLoop's error suggestion: adding label `'block` for `if` block.)
 - rust-lang#125054 (Handle `ReVar` in `note_and_explain_region`)
 - rust-lang#125156 (Expand `for_loops_over_fallibles` lint to lint on fallibles behind references.)
 - rust-lang#125222 (Migrate `run-make/issue-46239` to `rmake`)
 - rust-lang#125316 (Tweak `Spacing` use)
 - rust-lang#125392 (Wrap Context.ext in AssertUnwindSafe)
 - rust-lang#125417 (self-contained linker: retry linking without `-fuse-ld=lld` on CCs that don't support it)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 748647f into rust-lang:master May 23, 2024
7 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 23, 2024
Rollup merge of rust-lang#125417 - lqd:lld-retry, r=petrochenkov

self-contained linker: retry linking without `-fuse-ld=lld` on CCs that don't support it

For the self-contained linker, this PR applies [the strategy](rust-lang#125330 (comment)) of retrying the linking step when the driver doesn't support `-fuse-ld=lld`, but with the option removed. This is the same strategy we already use of retrying when e.g. `-no-pie` is not supported.

Fixes rust-lang#125330
r? `@petrochenkov`

I have no idea how we could add a test here, much like we don't have one for `-no-pie` or `-static-pie` -- let me know if you have ideas -- but I tested on a CentOS7 image:

```console
[root@d25b38376ede tmp]# ../build/host/stage1/bin/rustc helloworld.rs
 WARN rustc_codegen_ssa::back::link The linker driver does not support `-fuse-ld=lld`. Retrying without it.

[root@d25b38376ede tmp]# readelf -p .comment helloworld

String dump of section '.comment':
  [     0]  GCC: (GNU) 4.8.5 20150623 (Red Hat 4.8.5-44)
  [    2d]  rustc version 1.80.0-dev
```

I wasn't able to test with `cross` as the issue describes: I wasn't able to reproduce that behavior locally.
@rustbot rustbot added this to the 1.80.0 milestone May 23, 2024
@lqd lqd deleted the lld-retry branch May 23, 2024 09:01
@VorpalBlade
Copy link

Works fine with nightly and cross now. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cross compilation fails on Rust nightly when using proc macro crates due to GCC not understanding -fuse-ld=lld
7 participants