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

Implement unstable -Clinker-flavor=gcc:lld for MCP 510 #96827

Closed
wants to merge 12 commits into from
Closed
143 changes: 114 additions & 29 deletions compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ use rustc_hir::def_id::CrateNum;
use rustc_metadata::fs::{emit_metadata, METADATA_FILENAME};
use rustc_middle::middle::dependency_format::Linkage;
use rustc_middle::middle::exported_symbols::SymbolExportKind;
use rustc_session::config::{self, CFGuard, CrateType, DebugInfo, LdImpl, Strip};
use rustc_session::config::{self, CFGuard, CrateType, DebugInfo, LdImpl, LinkerFlavorCli, Strip};
use rustc_session::config::{OutputFilenames, OutputType, PrintRequest, SplitDwarfKind};
use rustc_session::cstore::DllImport;
use rustc_session::output::{check_file_is_writeable, invalid_output_for_target, out_filename};
use rustc_session::search_paths::PathKind;
use rustc_session::utils::NativeLibKind;
/// For all the linkers we support, and information they might
/// need out of the shared crate context before we get rid of it.
use rustc_session::{filesearch, Session};
use rustc_session::{config::InstrumentCoverage, filesearch, Session};
use rustc_span::symbol::Symbol;
use rustc_span::DebuggerVisualizerFile;
use rustc_target::spec::crt_objects::{CrtObjects, CrtObjectsFallback};
Expand Down Expand Up @@ -1130,7 +1130,7 @@ pub fn ignored_for_lto(sess: &Session, info: &CrateInfo, cnum: CrateNum) -> bool
&& (info.compiler_builtins == Some(cnum) || info.is_no_builtins.contains(&cnum))
}

// This functions tries to determine the appropriate linker (and corresponding LinkerFlavor) to use
/// This function tries to determine the appropriate linker (and corresponding LinkerFlavor) to use
pub fn linker_and_flavor(sess: &Session) -> (PathBuf, LinkerFlavor) {
fn infer_from(
sess: &Session,
Expand Down Expand Up @@ -1203,9 +1203,13 @@ pub fn linker_and_flavor(sess: &Session) -> (PathBuf, LinkerFlavor) {
}
}

// linker and linker flavor specified via command line have precedence over what the target
// specification specifies
if let Some(ret) = infer_from(sess, sess.opts.cg.linker.clone(), sess.opts.cg.linker_flavor) {
// Lower the potential `-C linker-flavor` CLI flag to its principal linker-flavor
let linker_flavor =
sess.opts.cg.linker_flavor.as_ref().map(|surface_flavor| surface_flavor.to_flavor());

// The `-C linker` and `-C linker-flavor` CLI flags have higher priority than what the target
// specification declares.
if let Some(ret) = infer_from(sess, sess.opts.cg.linker.clone(), linker_flavor) {
return ret;
}

Expand Down Expand Up @@ -2012,7 +2016,7 @@ fn add_order_independent_options(
out_filename: &Path,
tmpdir: &Path,
) {
add_gcc_ld_path(cmd, sess, flavor);
handle_cli_linker_flavors(cmd, sess, flavor, crt_objects_fallback);

add_apple_sdk(cmd, sess, flavor);

Expand Down Expand Up @@ -2690,29 +2694,110 @@ fn get_apple_sdk_root(sdk_name: &str) -> Result<String, String> {
}
}

fn add_gcc_ld_path(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) {
if let Some(ld_impl) = sess.opts.unstable_opts.gcc_ld {
if let LinkerFlavor::Gcc = flavor {
match ld_impl {
LdImpl::Lld => {
let tools_path = sess.get_tools_search_paths(false);
let gcc_ld_dir = tools_path
.into_iter()
.map(|p| p.join("gcc-ld"))
.find(|p| {
p.join(if sess.host.is_like_windows { "ld.exe" } else { "ld" }).exists()
})
.unwrap_or_else(|| sess.fatal("rust-lld (as ld) not found"));
cmd.arg({
let mut arg = OsString::from("-B");
arg.push(gcc_ld_dir);
arg
});
cmd.arg(format!("-Wl,-rustc-lld-flavor={}", sess.target.lld_flavor.as_str()));
}
/// This takes care of the various possible enrichments to the linking that can be requested on the
/// CLI (and emitting errors and warnings when applicable):
/// - the unstable `-Zgcc-ld=lld` flag to use `rust-lld`
/// - the shortcut to `-fuse-ld=lld` with the `gcc` flavor, to eventually use `rust-lld` when
/// `-Clink-self-contained=linker` is introduced.
fn handle_cli_linker_flavors(
cmd: &mut dyn Linker,
sess: &Session,
flavor: LinkerFlavor,
crt_objects_fallback: bool,
) {
let unstable_gcc_lld = sess.opts.unstable_opts.gcc_ld == Some(LdImpl::Lld);

// Sanity check: ensure `gcc` is the currently selected flavor.
if unstable_gcc_lld && LinkerFlavor::Gcc != flavor {
sess.fatal("`-Zgcc-ld` is used even though the linker flavor is not `gcc`");
}

let cg = &sess.opts.cg;

// The `-C linker-flavor` CLI flag can optionally enrich linker-flavors. Check whether that's
// applicable, and emit errors if sanity checks fail. There's currently only one enrichment:
// adding an argument to the `cc` invocation to use `lld`.
match &cg.linker_flavor {
Some(LinkerFlavorCli::Gcc { use_ld }) => {
// Ensure `gcc` is the currently selected flavor.
//
// This should be the case as long as this piece of code stays downstream from the CLI
// linker-flavor lowering, and the actual `LinkerFlavor` is not changed or overriden by
// the time the order-independent options are added to the command args.
//
// If that changes by mistake (or intentionally) in the future, we panic.
if LinkerFlavor::Gcc != flavor {
bug!(
"`-Clinker-flavor=gcc:*` flag is used even though the \
linker flavor is not `gcc`",
);
lqd marked this conversation as resolved.
Show resolved Hide resolved
}

if *use_ld != LdImpl::Lld {
// We're not in a situation needing enrichments.
return;
}
}

Some(LinkerFlavorCli::WellKnown(_)) | None => {
if !unstable_gcc_lld {
// We're not in a situation needing enrichments.
return;
}
} else {
sess.fatal("option `-Z gcc-ld` is used even though linker flavor is not gcc");
}
}

// From now on in this function, we handle the `gcc` linker-flavor enrichment to use `lld`.

// Start by checking if we're in the context of a known issue that users might hit when
// using `lld`:
//
// 1. when requesting self-contained CRT linking (or on a target that does it
// automatically), and coverage/profile generation: point at #79555 "Coverage is not
// generated when using rust-lld as linker"
let instrument_coverage =
cg.instrument_coverage.is_some() && cg.instrument_coverage != Some(InstrumentCoverage::Off);
let generate_profile = cg.profile_generate.enabled();
if crt_objects_fallback && (instrument_coverage || generate_profile) {
sess.warn(
"Using `lld`, self-contained linking, and coverage or profile generation has known \
issues. See issue #79555 for more details, at \
https://github.com/rust-lang/rust/issues/79555",
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Ugh.
Is it really a compiler's job to do this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Same about other cases of copypasting issue tracker into the compiler's source code.

}

// 2. Maybe point at https://github.com/flamegraph-rs/flamegraph/pull/157 or the
// corresponding rust/LLVM issue when/if it's tracked, depending on whether we use the
// workaround argument `--no-rosegment` by default when invoking `lld`.
//
// 3. If in the future, other linker flavors and targets are eligible to a `rust-lld`
// enrichment, maybe also point at target-specific issues like:
// - MSVC + ThinLTO blocker https://github.com/rust-lang/rust/issues/81408
// - the "lld on MSVC" tracking issue https://github.com/rust-lang/rust/issues/71520
// containing a list of blocking issues

// Now, handle `rust-lld`. If the `-Zgcc-ld=lld` flag was provided, we use `rust-lld`, the
// rustup-distributed version of `lld` (when applicable, i.e. not in distro-builds) by:
// - checking the `lld-wrapper`s exist in the sysroot
// - adding their folder as a search path, or requesting to use a wrapper directly
if unstable_gcc_lld {
// A `gcc-ld` folder (containing the `lld-wrapper`s that will run `rust-lld`) is present in
// the sysroot's target-specific tool binaries folder.
let tools_path = sess.get_tools_search_paths(false);
let gcc_ld_dir = tools_path
.into_iter()
.map(|p| p.join("gcc-ld"))
.find(|p| p.join(if sess.host.is_like_windows { "ld.exe" } else { "ld" }).exists())
.unwrap_or_else(|| sess.fatal("rust-lld (as ld) not found"));

cmd.arg({
let mut arg = OsString::from("-B");
arg.push(gcc_ld_dir);
arg
});
cmd.arg(format!("-Wl,-rustc-lld-flavor={}", sess.target.lld_flavor.as_str()));
} else {
// Otherwise, we were asked to use `lld` but not `rust-lld`.
cmd.arg("-fuse-ld=lld");
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this PR should wait for #100200, in addition to fixing the regression it basically makes the "self-contained" and "lld" aspects of -Zgcc-ld orthogonal and removes useless checks like

    // - checking the `lld-wrapper`s exist in the sysroot

above, that check something that is not necessarily true, and will prevent finding system lld instead.

}
}
3 changes: 2 additions & 1 deletion compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::interface::parse_cfgspecs;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{emitter::HumanReadableErrorType, registry, ColorConfig};
use rustc_session::config::InstrumentCoverage;
use rustc_session::config::LinkerFlavorCli;
use rustc_session::config::Strip;
use rustc_session::config::{build_configuration, build_session_options, to_crate_config};
use rustc_session::config::{
Expand Down Expand Up @@ -551,7 +552,7 @@ fn test_codegen_options_tracking_hash() {
untracked!(link_args, vec![String::from("abc"), String::from("def")]);
untracked!(link_self_contained, Some(true));
untracked!(linker, Some(PathBuf::from("linker")));
untracked!(linker_flavor, Some(LinkerFlavor::Gcc));
untracked!(linker_flavor, Some(LinkerFlavorCli::WellKnown(LinkerFlavor::Gcc)));
untracked!(no_stack_check, true);
untracked!(remark, Passes::Some(vec![String::from("pass1"), String::from("pass2")]));
untracked!(rpath, true);
Expand Down
64 changes: 63 additions & 1 deletion compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,56 @@ impl LinkerPluginLto {
}
}

/// The possible values `-C linker-flavor` can take: either one of the well-known linker flavors, or
/// an enrichment to one of them, for example representing additional arguments to its principal
/// linker flavor.
///
/// This a surface enum for the CLI syntax, so that target specs don't have to deal with the
/// specifics of the CLI: they will always use the well-known principal linker flavor via the
/// `to_flavor` method, and never the enrichment variants.
///
/// Currently used to represent finer-grained uses of `gcc`, to improve ease-of-use for wrapping a
/// given linker (currently: only `lld`).
#[derive(Clone, Debug, PartialEq)]
pub enum LinkerFlavorCli {
/// When the CLI option is one of the well-known linker-flavors that don't need special
/// handling.
WellKnown(LinkerFlavor),

/// Enrichments to the `LinkerFlavor::Gcc` flavor, to specify the linker via `-fuse-ld`.
Gcc { use_ld: LdImpl },
}

impl LinkerFlavorCli {
/// Returns the principal linker flavor that this CLI option represents.
pub fn to_flavor(&self) -> LinkerFlavor {
match *self {
LinkerFlavorCli::WellKnown(flavor) => flavor,
LinkerFlavorCli::Gcc { .. } => LinkerFlavor::Gcc,
}
}
}

/// Parses a `-C linker-flavor` option
impl FromStr for LinkerFlavorCli {
type Err = ();

fn from_str(s: &str) -> Result<Self, ()> {
// If the value is one of the existing flavor mappings, return that.
if let Some(flavor) = LinkerFlavor::from_str(s) {
return Ok(LinkerFlavorCli::WellKnown(flavor));
}

// Otherwise, it should be the enrichments to the gcc/cc flavor: wrapping a given linker
// separated by a colon like `gcc:lld`.
if s != "gcc:lld" {
return Err(());
}

Ok(LinkerFlavorCli::Gcc { use_ld: LdImpl::Lld })
}
}

/// The different settings that can be enabled via the `-Z location-detail` flag.
#[derive(Clone, PartialEq, Hash, Debug)]
pub struct LocationDetail {
Expand Down Expand Up @@ -2376,7 +2426,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
}
}

if cg.linker_flavor == Some(LinkerFlavor::L4Bender)
if cg.linker_flavor == Some(LinkerFlavorCli::WellKnown(LinkerFlavor::L4Bender))
&& !nightly_options::is_unstable_enabled(matches)
{
early_error(
Expand All @@ -2386,6 +2436,18 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
);
}

if let Some(LinkerFlavorCli::Gcc { .. }) = &cg.linker_flavor {
// For testing purposes, until we have more feedback about these options: ensure `-Z
// unstable-options` is enabled when using the `gcc` linker flavor enrichments.
if !unstable_opts.unstable_options {
early_error(
error_format,
"the `gcc:lld` linker flavor is unstable, the `-Z unstable-options` \
flag must also be passed to use it",
);
}
}

let prints = collect_print_requests(&mut cg, &mut unstable_opts, matches, error_format);

let cg = cg;
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_session/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ pub mod output;

pub use getopts;

#[cfg(test)]
mod tests;

/// Requirements for a `StableHashingContext` to be used in this crate.
/// This is a hack to allow using the `HashStable_Generic` derive macro
/// instead of implementing everything in `rustc_middle`.
Expand Down
13 changes: 7 additions & 6 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::lint;
use crate::search_paths::SearchPath;
use crate::utils::NativeLib;
use rustc_errors::LanguageIdentifier;
use rustc_target::spec::{CodeModel, LinkerFlavor, MergeFunctions, PanicStrategy, SanitizerSet};
use rustc_target::spec::{CodeModel, MergeFunctions, PanicStrategy, SanitizerSet};
use rustc_target::spec::{
RelocModel, RelroLevel, SplitDebuginfo, StackProtector, TargetTriple, TlsModel,
};
Expand Down Expand Up @@ -383,7 +383,8 @@ mod desc {
"either a boolean (`yes`, `no`, `on`, `off`, etc), `checks`, or `nochecks`";
pub const parse_cfprotection: &str = "`none`|`no`|`n` (default), `branch`, `return`, or `full`|`yes`|`y` (equivalent to `branch` and `return`)";
pub const parse_strip: &str = "either `none`, `debuginfo`, or `symbols`";
pub const parse_linker_flavor: &str = ::rustc_target::spec::LinkerFlavor::one_of();
pub const parse_linker_flavor: &str = "one of: `em`, `gcc`, `l4-bender`, `ld`, `msvc`, \
`ptx-linker`, `bpf-linker`, `wasm-ld`, `ld64.lld`, `ld.lld`, `lld-link`, or `gcc:lld`";
pub const parse_optimization_fuel: &str = "crate=integer";
pub const parse_mir_spanview: &str = "`statement` (default), `terminator`, or `block`";
pub const parse_instrument_coverage: &str =
Expand Down Expand Up @@ -760,8 +761,8 @@ mod parse {
true
}

pub(crate) fn parse_linker_flavor(slot: &mut Option<LinkerFlavor>, v: Option<&str>) -> bool {
match v.and_then(LinkerFlavor::from_str) {
pub(crate) fn parse_linker_flavor(slot: &mut Option<LinkerFlavorCli>, v: Option<&str>) -> bool {
match v.and_then(|s| LinkerFlavorCli::from_str(s).ok()) {
Some(lf) => *slot = Some(lf),
_ => return false,
}
Expand Down Expand Up @@ -1120,7 +1121,7 @@ options! {
on C toolchain installed in the system"),
linker: Option<PathBuf> = (None, parse_opt_pathbuf, [UNTRACKED],
"system linker to link outputs with"),
linker_flavor: Option<LinkerFlavor> = (None, parse_linker_flavor, [UNTRACKED],
linker_flavor: Option<LinkerFlavorCli> = (None, parse_linker_flavor, [UNTRACKED],
"linker flavor"),
linker_plugin_lto: LinkerPluginLto = (LinkerPluginLto::Disabled,
parse_linker_plugin_lto, [TRACKED],
Expand Down Expand Up @@ -1616,7 +1617,7 @@ pub enum WasiExecModel {
Reactor,
}

#[derive(Clone, Copy, Hash)]
#[derive(Clone, Copy, Hash, PartialEq, Debug)]
pub enum LdImpl {
Lld,
}
53 changes: 53 additions & 0 deletions compiler/rustc_session/src/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
use std::str::FromStr;

use crate::config::*;
use rustc_target::spec::{LinkerFlavor, LldFlavor};

/// When adding enrichments to `-C linker-flavor`, we want to ensure the existing `rustc_target`
/// `LinkerFlavor`s are still supported as-is: they are option values that can be used on
/// stable.
#[test]
pub fn parse_well_known_linker_flavor() {
// All `LinkerFlavor`s are wrapped as a whole, so there's no particular need to be
// exhaustive here.
assert_eq!(LinkerFlavorCli::from_str("gcc"), Ok(LinkerFlavorCli::WellKnown(LinkerFlavor::Gcc)));
assert_eq!(
LinkerFlavorCli::from_str("msvc"),
Ok(LinkerFlavorCli::WellKnown(LinkerFlavor::Msvc))
);
assert_eq!(
LinkerFlavorCli::from_str("bpf-linker"),
Ok(LinkerFlavorCli::WellKnown(LinkerFlavor::BpfLinker))
);
assert_eq!(
LinkerFlavorCli::from_str("lld-link"),
Ok(LinkerFlavorCli::WellKnown(LinkerFlavor::Lld(LldFlavor::Link)))
);
assert_eq!(
LinkerFlavorCli::from_str("ld64.lld"),
Ok(LinkerFlavorCli::WellKnown(LinkerFlavor::Lld(LldFlavor::Ld64)))
);

// While other invalid values for well-known flavors are already errors
assert_eq!(LinkerFlavorCli::from_str("unknown_linker"), Err(()));
}

/// Enrichments can currently allow the `gcc` flavor to use `lld`, much like you'd use `-fuse-ld` as
/// a link arg.
#[test]
pub fn parse_gcc_enrichment_linker_flavor() {
assert_eq!(
LinkerFlavorCli::from_str("gcc:lld"),
Ok(LinkerFlavorCli::Gcc { use_ld: LdImpl::Lld })
);

// Only `gcc:lld` is supported for now
assert_eq!(LinkerFlavorCli::from_str("gcc:gold"), Err(()));

// No linker actually mentioned
assert_eq!(LinkerFlavorCli::from_str("gcc:"), Err(()));

// Only one `gcc:` separator allowed
assert_eq!(LinkerFlavorCli::from_str("gcc:gcc:"), Err(()));
assert_eq!(LinkerFlavorCli::from_str("gcc:gcc:linker"), Err(()));
}
Loading