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.

Sign up for GitHub

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

Jump to bottom

[WIP] Linker flavors: next steps #119906

Closed
wants to merge 3 commits into from
Closed

[WIP] Linker flavors: next steps #119906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
76fbc12
rustc_target: Add clang linker flavors
petrochenkov Jan 12, 2024
9b9bee4
Employ clang linker flavor for supporting cross-linking automatically
petrochenkov Jan 12, 2024
364f36d
Generic linker flavors
petrochenkov Jan 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 15 additions & 30 deletions compiler/rustc_codegen_ssa/src/back/link.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +788,7 @@ fn link_natively<'a>(
// then it should not default to linking executables as pie. Different
// versions of gcc seem to use different quotes in the error message so
// don't check for them.
if matches!(flavor, LinkerFlavor::Gnu(Cc::Yes, _))
if matches!(flavor, LinkerFlavor::Gnu(Cc::Yes | Cc::Clang, _))
&& unknown_arg_regex.is_match(&out)
&& out.contains("-no-pie")
&& cmd.get_args().iter().any(|e| e.to_string_lossy() == "-no-pie")
Expand All @@ -806,7 +806,7 @@ fn link_natively<'a>(

// Detect '-static-pie' used with an older version of gcc or clang not supporting it.
// Fallback from '-static-pie' to '-static' in that case.
if matches!(flavor, LinkerFlavor::Gnu(Cc::Yes, _))
if matches!(flavor, LinkerFlavor::Gnu(Cc::Yes | Cc::Clang, _))
&& unknown_arg_regex.is_match(&out)
&& (out.contains("-static-pie") || out.contains("--no-dynamic-linker"))
&& cmd.get_args().iter().any(|e| e.to_string_lossy() == "-static-pie")
Expand Down Expand Up @@ -1318,6 +1318,10 @@ pub fn linker_and_flavor(sess: &Session) -> (PathBuf, LinkerFlavor) {
"cc"
}
}
LinkerFlavor::Gnu(Cc::Clang, _)
| LinkerFlavor::Darwin(Cc::Clang, _)
| LinkerFlavor::WasmLld(Cc::Clang)
| LinkerFlavor::Unix(Cc::Clang) => "clang",
LinkerFlavor::Gnu(_, Lld::Yes)
| LinkerFlavor::Darwin(_, Lld::Yes)
| LinkerFlavor::WasmLld(..)
Expand Down Expand Up @@ -1778,7 +1782,9 @@ fn add_pre_link_objects(
let empty = Default::default();
let objects = if self_contained {
&opts.pre_link_objects_self_contained
} else if !(sess.target.os == "fuchsia" && matches!(flavor, LinkerFlavor::Gnu(Cc::Yes, _))) {
} else if !(sess.target.os == "fuchsia"
&& matches!(flavor, LinkerFlavor::Gnu(Cc::Yes | Cc::Clang, _)))
{
&opts.pre_link_objects
} else {
&empty
Expand Down Expand Up @@ -2278,6 +2284,10 @@ fn add_order_independent_options(
out_filename: &Path,
tmpdir: &Path,
) {
if flavor.uses_clang() && sess.target.linker_flavor != sess.host.linker_flavor {
cmd.arg(format!("--target={}", sess.target.llvm_target));
Comment on lines +2287 to +2288
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this condition sufficient for the general case of adding --target when cross-compiling? The linker flavor doesn't contain any information about the target triple (unless I'm missing something), so I don't see how this can reliably detect cross-compiling.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I know --target only affects which linker is looked up (ld.lld, or ld64.lld, etc) if clang is used for linking only.
So if the required flavor is the same as host, then clang without --target will already use the right linker.

On the other hand, there are some issues with crt object versions on macOS if --target is specified (fixed by #101792), maybe target takes priority over some relevant environment variables.
So always passing --target leads to issues.

We'll likely need to tweak this logic based on reported issues.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For lld this may not matter, since lld is also a cross-linker.
My main concern is if gnu ld is used (linker flavor (Cc::Clang, Lld::No) for both host and target), then --target is required for clang to select e.g. aarch64-linux-gnu-ld instead of just ld.

}

// Take care of the flavors and CLI options requesting the `lld` linker.
add_lld_args(cmd, sess, flavor, self_contained_components);

Expand All @@ -2287,7 +2297,7 @@ fn add_order_independent_options(

if sess.target.os == "fuchsia"
&& crate_type == CrateType::Executable
&& !matches!(flavor, LinkerFlavor::Gnu(Cc::Yes, _))
&& !matches!(flavor, LinkerFlavor::Gnu(Cc::Yes | Cc::Clang, _))
{
let prefix = if sess.opts.unstable_opts.sanitizer.contains(SanitizerSet::ADDRESS) {
"asan/"
Expand Down Expand Up @@ -2923,7 +2933,7 @@ fn add_apple_sdk(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) {
};

match flavor {
LinkerFlavor::Darwin(Cc::Yes, _) => {
LinkerFlavor::Darwin(Cc::Yes | Cc::Clang, _) => {
cmd.args(&["-isysroot", &sdk_root, "-Wl,-syslibroot", &sdk_root]);
}
LinkerFlavor::Darwin(Cc::No, _) => {
Expand Down Expand Up @@ -3047,29 +3057,4 @@ fn add_lld_args(
// 2. Implement the "linker flavor" part of this feature by asking `cc` to use some kind of
// `lld` as the linker.
cmd.arg("-fuse-ld=lld");

if !flavor.is_gnu() {
// Tell clang to use a non-default LLD flavor.
// Gcc doesn't understand the target option, but we currently assume
// that gcc is not used for Apple and Wasm targets (#97402).
//
// Note that we don't want to do that by default on macOS: e.g. passing a
// 10.7 target to LLVM works, but not to recent versions of clang/macOS, as
// shown in issue #101653 and the discussion in PR #101792.
//
// It could be required in some cases of cross-compiling with
// LLD, but this is generally unspecified, and we don't know
// which specific versions of clang, macOS SDK, host and target OS
// combinations impact us here.
//
// So we do a simple first-approximation until we know more of what the
// Apple targets require (and which would be handled prior to hitting this
// LLD codepath anyway), but the expectation is that until then
// this should be manually passed if needed. We specify the target when
// targeting a different linker flavor on macOS, and that's also always
// the case when targeting WASM.
if sess.target.linker_flavor != sess.host.linker_flavor {
cmd.arg(format!("--target={}", sess.target.llvm_target));
}
}
}
90 changes: 74 additions & 16 deletions compiler/rustc_target/src/spec/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use rustc_fs_util::try_canonicalize;
use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
use rustc_span::symbol::{kw, sym, Symbol};
use serde_json::Value;
use std::assert_matches::assert_matches;
use std::borrow::Cow;
use std::collections::BTreeMap;
use std::hash::{Hash, Hasher};
Expand All @@ -65,6 +66,7 @@ pub use base::avr_gnu::ef_avr_arch;
/// Linker is called through a C/C++ compiler.
#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)]
pub enum Cc {
Clang,
Yes,
No,
}
Expand Down Expand Up @@ -142,6 +144,9 @@ pub enum LinkerFlavorCli {
Bpf,
Ptx,

// Generic (unstable) flavors.
Any(Cc, Lld),

// Legacy stable values
Gcc,
Ld,
Expand All @@ -160,7 +165,8 @@ impl LinkerFlavorCli {
| LinkerFlavorCli::Msvc(Lld::Yes)
| LinkerFlavorCli::EmCc
| LinkerFlavorCli::Bpf
| LinkerFlavorCli::Ptx => true,
| LinkerFlavorCli::Ptx
| LinkerFlavorCli::Any(..) => true,
LinkerFlavorCli::Gcc
| LinkerFlavorCli::Ld
| LinkerFlavorCli::Lld(..)
Expand Down Expand Up @@ -221,6 +227,15 @@ impl LinkerFlavor {
LinkerFlavorCli::Bpf => LinkerFlavor::Bpf,
LinkerFlavorCli::Ptx => LinkerFlavor::Ptx,

// Generic flavors
LinkerFlavorCli::Any(cc, lld) => match lld_flavor {
LldFlavor::Ld if is_gnu => LinkerFlavor::Gnu(cc, lld),
LldFlavor::Ld64 => LinkerFlavor::Darwin(cc, lld),
LldFlavor::Wasm => LinkerFlavor::WasmLld(cc),
LldFlavor::Ld => LinkerFlavor::Unix(cc),
LldFlavor::Link => LinkerFlavor::Msvc(lld),
},

// Below: legacy stable values
LinkerFlavorCli::Gcc => match lld_flavor {
LldFlavor::Ld if is_gnu => LinkerFlavor::Gnu(Cc::Yes, Lld::No),
Expand All @@ -244,10 +259,10 @@ impl LinkerFlavor {
/// Returns the corresponding backwards-compatible CLI flavor.
fn to_cli(self) -> LinkerFlavorCli {
match self {
LinkerFlavor::Gnu(Cc::Yes, _)
| LinkerFlavor::Darwin(Cc::Yes, _)
| LinkerFlavor::WasmLld(Cc::Yes)
| LinkerFlavor::Unix(Cc::Yes) => LinkerFlavorCli::Gcc,
LinkerFlavor::Gnu(Cc::Yes | Cc::Clang, _)
| LinkerFlavor::Darwin(Cc::Yes | Cc::Clang, _)
| LinkerFlavor::WasmLld(Cc::Yes | Cc::Clang)
| LinkerFlavor::Unix(Cc::Yes | Cc::Clang) => LinkerFlavorCli::Gcc,
LinkerFlavor::Gnu(_, Lld::Yes) => LinkerFlavorCli::Lld(LldFlavor::Ld),
LinkerFlavor::Darwin(_, Lld::Yes) => LinkerFlavorCli::Lld(LldFlavor::Ld64),
LinkerFlavor::WasmLld(..) => LinkerFlavorCli::Lld(LldFlavor::Wasm),
Expand Down Expand Up @@ -287,6 +302,9 @@ impl LinkerFlavor {
LinkerFlavorCli::EmCc => (Some(Cc::Yes), Some(Lld::Yes)),
LinkerFlavorCli::Bpf | LinkerFlavorCli::Ptx => (None, None),

// Generic flavors
LinkerFlavorCli::Any(cc, lld) => (Some(cc), Some(lld)),

// Below: legacy stable values
LinkerFlavorCli::Gcc => (Some(Cc::Yes), None),
LinkerFlavorCli::Ld => (Some(Cc::No), Some(Lld::No)),
Expand All @@ -308,12 +326,14 @@ impl LinkerFlavor {
|| stem.ends_with("-gcc")
|| stem == "g++"
|| stem.ends_with("-g++")
|| stem == "clang"
{
(Some(Cc::Yes), Some(Lld::No))
} else if stem == "clang"
|| stem.ends_with("-clang")
|| stem == "clang++"
|| stem.ends_with("-clang++")
{
(Some(Cc::Yes), Some(Lld::No))
(Some(Cc::Clang), Some(Lld::No))
} else if stem == "wasm-ld"
|| stem.ends_with("-wasm-ld")
|| stem == "ld.lld"
Expand Down Expand Up @@ -420,10 +440,10 @@ impl LinkerFlavor {
pub fn uses_cc(self) -> bool {
// Exhaustive match in case new flavors are added in the future.
match self {
LinkerFlavor::Gnu(Cc::Yes, _)
| LinkerFlavor::Darwin(Cc::Yes, _)
| LinkerFlavor::WasmLld(Cc::Yes)
| LinkerFlavor::Unix(Cc::Yes)
LinkerFlavor::Gnu(Cc::Yes | Cc::Clang, _)
| LinkerFlavor::Darwin(Cc::Yes | Cc::Clang, _)
| LinkerFlavor::WasmLld(Cc::Yes | Cc::Clang)
| LinkerFlavor::Unix(Cc::Yes | Cc::Clang)
| LinkerFlavor::EmCc => true,
LinkerFlavor::Gnu(..)
| LinkerFlavor::Darwin(..)
Expand All @@ -434,6 +454,17 @@ impl LinkerFlavor {
| LinkerFlavor::Ptx => false,
}
}

pub fn uses_clang(self) -> bool {
match self {
LinkerFlavor::Gnu(Cc::Clang, _)
| LinkerFlavor::Darwin(Cc::Clang, _)
| LinkerFlavor::WasmLld(Cc::Clang)
| LinkerFlavor::Unix(Cc::Clang)
| LinkerFlavor::EmCc => true,
_ => false,
}
}
}

macro_rules! linker_flavor_cli_impls {
Expand Down Expand Up @@ -468,20 +499,34 @@ linker_flavor_cli_impls! {
(LinkerFlavorCli::Gnu(Cc::No, Lld::Yes)) "gnu-lld"
(LinkerFlavorCli::Gnu(Cc::Yes, Lld::No)) "gnu-cc"
(LinkerFlavorCli::Gnu(Cc::Yes, Lld::Yes)) "gnu-lld-cc"
(LinkerFlavorCli::Gnu(Cc::Clang, Lld::No)) "gnu-clang"
(LinkerFlavorCli::Gnu(Cc::Clang, Lld::Yes)) "gnu-lld-clang"
(LinkerFlavorCli::Darwin(Cc::No, Lld::No)) "darwin"
(LinkerFlavorCli::Darwin(Cc::No, Lld::Yes)) "darwin-lld"
(LinkerFlavorCli::Darwin(Cc::Yes, Lld::No)) "darwin-cc"
(LinkerFlavorCli::Darwin(Cc::Yes, Lld::Yes)) "darwin-lld-cc"
(LinkerFlavorCli::Darwin(Cc::Clang, Lld::No)) "darwin-clang"
(LinkerFlavorCli::Darwin(Cc::Clang, Lld::Yes)) "darwin-lld-clang"
(LinkerFlavorCli::WasmLld(Cc::No)) "wasm-lld"
(LinkerFlavorCli::WasmLld(Cc::Yes)) "wasm-lld-cc"
(LinkerFlavorCli::WasmLld(Cc::Clang)) "wasm-lld-clang"
(LinkerFlavorCli::Unix(Cc::No)) "unix"
(LinkerFlavorCli::Unix(Cc::Yes)) "unix-cc"
(LinkerFlavorCli::Unix(Cc::Clang)) "unix-clang"
(LinkerFlavorCli::Msvc(Lld::Yes)) "msvc-lld"
(LinkerFlavorCli::Msvc(Lld::No)) "msvc"
(LinkerFlavorCli::EmCc) "em-cc"
(LinkerFlavorCli::Bpf) "bpf"
(LinkerFlavorCli::Ptx) "ptx"

// Generic flavors
(LinkerFlavorCli::Any(Cc::No, Lld::No)) "*"
(LinkerFlavorCli::Any(Cc::No, Lld::Yes)) "*-lld"
(LinkerFlavorCli::Any(Cc::Yes, Lld::No)) "*-cc"
(LinkerFlavorCli::Any(Cc::Yes, Lld::Yes)) "*-lld-cc"
(LinkerFlavorCli::Any(Cc::Clang, Lld::No)) "*-clang"
(LinkerFlavorCli::Any(Cc::Clang, Lld::Yes)) "*-lld-clang"

// Legacy stable flavors
(LinkerFlavorCli::Gcc) "gcc"
(LinkerFlavorCli::Ld) "ld"
Expand Down Expand Up @@ -2176,21 +2221,34 @@ fn add_link_args_iter(
match flavor {
LinkerFlavor::Gnu(cc, lld) => {
assert_eq!(lld, Lld::No);
assert_matches!(cc, Cc::No | Cc::Yes);
insert(LinkerFlavor::Gnu(cc, Lld::Yes));
if cc == Cc::Yes {
insert(LinkerFlavor::Gnu(Cc::Clang, Lld::No));
insert(LinkerFlavor::Gnu(Cc::Clang, Lld::Yes));
}
}
LinkerFlavor::Darwin(cc, lld) => {
assert_eq!(lld, Lld::No);
assert_matches!(cc, Cc::No | Cc::Yes);
insert(LinkerFlavor::Darwin(cc, Lld::Yes));
if cc == Cc::Yes {
insert(LinkerFlavor::Darwin(Cc::Clang, Lld::No));
insert(LinkerFlavor::Darwin(Cc::Clang, Lld::Yes));
}
}
LinkerFlavor::Msvc(lld) => {
assert_eq!(lld, Lld::No);
insert(LinkerFlavor::Msvc(Lld::Yes));
}
LinkerFlavor::WasmLld(..)
| LinkerFlavor::Unix(..)
| LinkerFlavor::EmCc
| LinkerFlavor::Bpf
| LinkerFlavor::Ptx => {}
LinkerFlavor::WasmLld(cc) | LinkerFlavor::Unix(cc) => {
assert_matches!(cc, Cc::No | Cc::Yes);
if cc == Cc::Yes {
insert(LinkerFlavor::Gnu(Cc::Clang, Lld::No));
insert(LinkerFlavor::Gnu(Cc::Clang, Lld::Yes));
Comment on lines +2247 to +2248
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly still WIP: these linker args should have a WasmLld or Unix principal flavor here, rather than Gnu?

}
}
LinkerFlavor::EmCc | LinkerFlavor::Bpf | LinkerFlavor::Ptx => {}
}
}

Expand Down
10 changes: 1 addition & 9 deletions compiler/rustc_target/src/spec/targets/wasm32_unknown_unknown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,7 @@ pub fn target() -> Target {
"--no-entry",
],
);
options.add_pre_link_args(
LinkerFlavor::WasmLld(Cc::Yes),
&[
// Make sure clang uses LLD as its linker and is configured appropriately
// otherwise
"--target=wasm32-unknown-unknown",
"-Wl,--no-entry",
],
);
options.add_pre_link_args(LinkerFlavor::WasmLld(Cc::Yes), &["-Wl,--no-entry"]);

Target {
llvm_target: "wasm32-unknown-unknown".into(),
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_target/src/spec/targets/wasm32_wasi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,12 @@

use crate::spec::crt_objects;
use crate::spec::LinkSelfContainedDefault;
use crate::spec::{base, Cc, LinkerFlavor, Target};
use crate::spec::{base, Target};

pub fn target() -> Target {
let mut options = base::wasm::options();

options.os = "wasi".into();
options.add_pre_link_args(LinkerFlavor::WasmLld(Cc::Yes), &["--target=wasm32-wasi"]);

options.pre_link_objects_self_contained = crt_objects::pre_wasi_self_contained();
options.post_link_objects_self_contained = crt_objects::post_wasi_self_contained();
Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_target/src/spec/targets/wasm32_wasi_preview1_threads.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,7 @@ pub fn target() -> Target {
);
options.add_pre_link_args(
LinkerFlavor::WasmLld(Cc::Yes),
&[
"--target=wasm32-wasi-threads",
"-Wl,--import-memory",
"-Wl,--export-memory,",
"-Wl,--shared-memory",
],
&["-Wl,--import-memory", "-Wl,--export-memory,", "-Wl,--shared-memory"],
);

options.pre_link_objects_self_contained = crt_objects::pre_wasi_self_contained();
Expand Down
10 changes: 1 addition & 9 deletions compiler/rustc_target/src/spec/targets/wasm64_unknown_unknown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,7 @@ pub fn target() -> Target {
"-mwasm64",
],
);
options.add_pre_link_args(
LinkerFlavor::WasmLld(Cc::Yes),
&[
// Make sure clang uses LLD as its linker and is configured appropriately
// otherwise
"--target=wasm64-unknown-unknown",
"-Wl,--no-entry",
],
);
options.add_pre_link_args(LinkerFlavor::WasmLld(Cc::Yes), &["-Wl,--no-entry"]);

// Any engine that implements wasm64 will surely implement the rest of these
// features since they were all merged into the official spec by the time
Expand Down
14 changes: 10 additions & 4 deletions compiler/rustc_target/src/spec/tests/tests_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,21 @@ impl Target {
};

match self.linker_flavor {
LinkerFlavor::Gnu(Cc::Yes, lld) => check_noncc(LinkerFlavor::Gnu(Cc::No, lld)),
LinkerFlavor::WasmLld(Cc::Yes) => check_noncc(LinkerFlavor::WasmLld(Cc::No)),
LinkerFlavor::Unix(Cc::Yes) => check_noncc(LinkerFlavor::Unix(Cc::No)),
LinkerFlavor::Gnu(Cc::Yes | Cc::Clang, lld) => {
check_noncc(LinkerFlavor::Gnu(Cc::No, lld))
}
LinkerFlavor::WasmLld(Cc::Yes | Cc::Clang) => {
check_noncc(LinkerFlavor::WasmLld(Cc::No))
}
LinkerFlavor::Unix(Cc::Yes | Cc::Clang) => {
check_noncc(LinkerFlavor::Unix(Cc::No))
}
_ => {}
}
}

// Check that link args for lld and non-lld versions of flavors are consistent.
for cc in [Cc::No, Cc::Yes] {
for cc in [Cc::No, Cc::Yes, Cc::Clang] {
assert_eq!(
args.get(&LinkerFlavor::Gnu(cc, Lld::No)),
args.get(&LinkerFlavor::Gnu(cc, Lld::Yes)),
Expand Down
2 changes: 1 addition & 1 deletion tests/run-make/rust-lld/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ include ../tools.mk
# needs-rust-lld
# ignore-s390x lld does not yet support s390x as target
all:
RUSTC_LOG=rustc_codegen_ssa::back::link=info $(RUSTC) -Clink-self-contained=+linker -Clinker-flavor=gnu-lld-cc -Zunstable-options -Clink-args=-Wl,-v main.rs 2> $(TMPDIR)/output.txt
RUSTC_LOG=rustc_codegen_ssa::back::link=info $(RUSTC) -Clink-self-contained=+linker -Clinker-flavor=*-lld-cc -Zunstable-options -Clink-args=-Wl,-v main.rs 2> $(TMPDIR)/output.txt
$(CGREP) -e "^LLD [0-9]+\.[0-9]+\.[0-9]+" < $(TMPDIR)/output.txt
Loading
Loading