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

Handling of linker flags appears to have changed from 1.60 to 1.61 #97196

Closed
jonhoo opened this issue May 19, 2022 · 7 comments
Closed

Handling of linker flags appears to have changed from 1.60 to 1.61 #97196

jonhoo opened this issue May 19, 2022 · 7 comments
Labels
C-bug Category: This is a bug.

Comments

@jonhoo
Copy link
Contributor

jonhoo commented May 19, 2022

rustc appears to have changed the flags it passes to the linker, which has made the order of -l flags significant in a way that they were not before.

Code

First, steps to set up a Cargo project to replicate the problem:

$ cargo new link
$ cd link
$ cat >Cargo.toml <<EOF
[package]
name = "link"
version = "0.1.0"
edition = "2021"
links = "test"

[build-dependencies]
cc = "1"
EOF
$ cat >build.rs <<EOF
fn main() {
    println!("cargo:rustc-link-lib=static=test");
    cc::Build::new().file("src/foo.c").compile("foo");
}
EOF
$ cat >src/main.rs <<EOF
extern "C" {
    fn bar();
}
fn main() {
    unsafe { bar() };
}
EOF
$ cat >src/foo.c <<EOF
#include "test.h"
void bar(void) {
    version();
}
EOF
$ mkdir .cargo
$ cat >.cargo/config.toml <<EOF
[build]
rustflags = ['-Clink-arg=-Lc/']

[env.LDFLAGS]
value = "-Lc/"

[env.CFLAGS]
value = "-Ic/"
EOF
$ mkdir c
$ echo 'void version();' > c/test.h
$ cat >c/test.c <<EOF
#include "test.h"
void version() {}
EOF
$ cc -c -o c/test.o c/test.c
$ ar rcs c/libtest.a c/test.o

With Rust 1.60.0 installed, this builds and runs fine (cargo b; cargo r).

With Rust 1.61.0 installed, the build errors with:

$ cargo b
   Compiling link v0.1.0
error: linking with `cc` failed: exit status: 1
  |
  = note: "cc" ...
  = note: target/debug/build/link-3971cbc394518864/out/libfoo.a(foo.o): In function `bar':
          src/foo.c:3: undefined reference to `version'
          collect2: error: ld returned 1 exit status

  = help: some `extern` functions couldn't be found; some native libraries may need to be installed or have their path specified
  = note: use the `-l` flag to specify native libraries to link
  = note: use the `cargo:rustc-link-lib` directive to specify the native libraries to link with Cargo (see https://doc.rust-lang.org/cargo/reference/build-scripts.html#cargorustc-link-libkindname)

Flipping the order of the cc invocation and the println! in build.rs fixes the build.

Looking at the output with --verbose, the rustc command that cargo invokes appears to be identical (except for different hashes). Using RUSTC_LOG=rustc_codegen_ssa::back::link=info to get the underlying linker archive, it appears that rustc changed the arguments it passes to the linker under the hood:

-"-Wl,-Bstatic" "-Wl,--whole-archive" "-ltest" "-Wl,--no-whole-archive" "-Wl,--whole-archive" "-lfoo" "-Wl,--no-whole-archive" "-Wl,--start-group"
+"-Wl,-Bstatic" "-ltest" "-lfoo" "-Wl,--start-group"

Why exactly this causes the crate to stop compiling when it did previously, I'm not entirely sure, but it is a regression. Flipping the two invocations in build.rs has the effect of flipping the order of the -l static=test and -l static=foo arguments that Cargo passes to rustc, which in turn makes the linker invocation succeed.

Version it worked on

It most recently worked on: 1.60.0

Version with regression

rustc --version --verbose:

rustc 1.61.0 (fe5b13d68 2022-05-18)
binary: rustc
commit-hash: fe5b13d681f25ee6474be29d748c65adcd91f69e
commit-date: 2022-05-18
host: aarch64-unknown-linux-gnu
release: 1.61.0
LLVM version: 14.0.0

The most likely culprit for this appears to be #93901, so cc @petrochenkov.

@rustbot modify labels: +regression-from-stable-to-stable -regression-untriaged

@jonhoo jonhoo added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels May 19, 2022
@rustbot rustbot added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. I-prioritize Issue: Indicates that prioritization has been requested for this issue. and removed regression-untriaged Untriaged performance or correctness regression. labels May 19, 2022
@nbdd0121
Copy link
Contributor

I believe this is expected. The order of the two lines is indeed wrong; the problem is just masked before 1.61.

@jonhoo
Copy link
Contributor Author

jonhoo commented May 20, 2022

That seems reasonable, though it is a regression. May be worth updating the release notes to mention this change in behavior if it's expected and desired?

@tmfink
Copy link
Contributor

tmfink commented May 20, 2022

This is potentially related other issues related to order of link arguments in #96328 and rust-lang/cargo#10600.

@BGR360
Copy link
Contributor

BGR360 commented May 20, 2022

Any chance this is causing the issue I just encountered in #97198?

@petrochenkov
Copy link
Contributor

@jonhoo

May be worth updating the release notes to mention this change in behavior if it's expected and desired?

This is already in release notes, the first compatibility note in https://github.com/rust-lang/rust/blob/stable/RELEASES.md#compatibility-notes.

@petrochenkov
Copy link
Contributor

@BGR360
This is likely the last compatibility note in https://github.com/rust-lang/rust/blob/stable/RELEASES.md#compatibility-notes

bootstrap: static-libstdcpp is now enabled by default, and can now be disabled when llvm-tools is enabled

@jonhoo
Copy link
Contributor Author

jonhoo commented May 20, 2022

@jonhoo

May be worth updating the release notes to mention this change in behavior if it's expected and desired?

This is already in release notes, the first compatibility note in https://github.com/rust-lang/rust/blob/stable/RELEASES.md#compatibility-notes.

Clearly I'm bad at a reading! Sorry for the noise 😅

@jonhoo jonhoo closed this as completed May 20, 2022
@apiraino apiraino removed regression-from-stable-to-stable Performance or correctness regression from one stable version to another. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

7 participants