From bb03c04debd87302e3ac5690fd7002f1294f8f3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Mon, 25 Apr 2022 19:15:04 +0200 Subject: [PATCH 1/6] add dedicated `-C link-self-contained` structures --- compiler/rustc_session/src/config.rs | 136 +++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 997f361737b34..d506115a0a1a5 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -199,6 +199,142 @@ impl LinkerPluginLto { } } +/// The different values `-C link-self-contained` can take. +/// +/// They are fine-grained, and control the behavior of: +/// - linking to our own CRT objects (the historical default) +/// - using a linker we distribute +/// +/// Since this flag historically targeted the CRT use-case, the defaults haven't changed, but there +/// are new values so that users can choose that behavior in combination with the linker use-case. +/// +/// For example: +/// - the absence of the `-C link-self-contained` was the historical default, and therefore this +/// default value still targets `auto` (`LinkSelfContainedCrt::Auto`). +/// - `-C link-self-contained=linker` turns on the linker, while keeping the default CRT behavior. +/// - explicitly turning on the CRT linking can be done as the historical `-C link-self-contained=y`, +/// or the fine-grained `-C link-self-contained=crt` +/// - turning both on can be done with `-C link-self-contained=all` +/// +#[derive(Clone, Copy, PartialEq, Debug)] +pub struct LinkSelfContained { + pub crt: LinkSelfContainedCrt, + pub linker: LinkSelfContainedLinker, +} + +impl LinkSelfContained { + /// Explicitly turns off the self-contained linker facet, becoming an opt-out that is the + /// historically stable `-C link-self-contained=y` behavior. + fn crt_only() -> Self { + LinkSelfContained { crt: LinkSelfContainedCrt::On, linker: LinkSelfContainedLinker::Off } + } + + /// Keeps stable behavior only turning the self-contained CRT linking on. + fn on() -> Self { + LinkSelfContained::crt_only() + } + + fn off() -> Self { + LinkSelfContained { crt: LinkSelfContainedCrt::Off, linker: LinkSelfContainedLinker::Off } + } + + /// Explicitly turns off the self-contained linker facet, becoming an opt-out that is the + /// historically stable default, target-defined, behavior. + /// + /// The current default. + fn auto() -> Self { + LinkSelfContained { crt: LinkSelfContainedCrt::Auto, linker: LinkSelfContainedLinker::Off } + } + + fn linker() -> Self { + LinkSelfContained { crt: LinkSelfContainedCrt::Auto, linker: LinkSelfContainedLinker::On } + } + + fn all() -> Self { + LinkSelfContained { crt: LinkSelfContainedCrt::On, linker: LinkSelfContainedLinker::On } + } +} + +impl Default for LinkSelfContained { + fn default() -> Self { + LinkSelfContained::auto() + } +} + +impl FromStr for LinkSelfContained { + type Err = (); + + fn from_str(s: &str) -> Result { + // TMP: do we also need "linker_only", crt:off and linker:on ? Except in rare targets, this + // use-case should be taken care of by "linker" for crt:auto linker:on. + + Ok(match s { + // Historical value parsing: a bool option + "no" | "n" | "off" => LinkSelfContained::off(), + "yes" | "y" | "on" => LinkSelfContained::on(), + + // New fine-grained facets + "crt" => { + // This is the same as `-C link-self-contained=y`, but allows to possibly change + // that while keeping the current only turning on self-contained CRT linking. Also + // makes an explicit value to opt into only a single facet. + LinkSelfContained::crt_only() + } + "auto" => LinkSelfContained::auto(), + "linker" => LinkSelfContained::linker(), + + // Composite of both + "all" => LinkSelfContained::all(), + _ => return Err(()), + }) + } +} + +/// Historically, `-C link-self-contained` was mainly about linking to our own CRT objects. This has +/// been extracted to a few dedicated values, while keeping the existing stable values as-is: the +/// default value, or opt-in for the whole flag are still these CRT-related values. +#[derive(Clone, Copy, PartialEq, Debug)] +pub enum LinkSelfContainedCrt { + /// The historical default value, when `-C link-self-contained` was not specified.The + /// self-contained linking behavior is target-defined in this case. + Auto, + + /// The historical value for `-C link-self-contained=y`. + On, + + /// When self-contained linking is explicitly turned off, with `-C link-self-contained=n` + Off, +} + +impl LinkSelfContainedCrt { + pub fn is_explicitly_set(&self) -> Option { + match self { + LinkSelfContainedCrt::On => Some(true), + LinkSelfContainedCrt::Off => Some(false), + LinkSelfContainedCrt::Auto => None, + } + } +} + +/// The different linker-related options for `-C link-self-contained`, to choose between +/// using system-installed linkers, or one present in the rust distribution. +#[derive(Clone, Copy, PartialEq, Debug)] +pub enum LinkSelfContainedLinker { + /// Whenever `-C link-self-contained=linker` is present. This opts in to using + /// a linker we distribute (e.g. `rust-lld`). + On, + + /// The default case: when `-C link-self-contained=linker` is absent, and when self-contained + /// linking is explicitly turned off, with `-C link-self-contained=n`. + Off, +} + +impl LinkSelfContainedLinker { + pub fn is_on(&self) -> bool { + *self == LinkSelfContainedLinker::On + } +} + /// The different settings that can be enabled via the `-Z location-detail` flag. #[derive(Clone, PartialEq, Hash, Debug)] pub struct LocationDetail { From f989d5a1bebeabab39164bfd977433cd49525561 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Mon, 25 Apr 2022 19:19:21 +0200 Subject: [PATCH 2/6] add tests for `LinkSelfContained` options parsing --- compiler/rustc_session/src/lib.rs | 3 ++ compiler/rustc_session/src/tests.rs | 71 +++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 compiler/rustc_session/src/tests.rs diff --git a/compiler/rustc_session/src/lib.rs b/compiler/rustc_session/src/lib.rs index f84a154950f64..b7e57f741ccd2 100644 --- a/compiler/rustc_session/src/lib.rs +++ b/compiler/rustc_session/src/lib.rs @@ -33,6 +33,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`. diff --git a/compiler/rustc_session/src/tests.rs b/compiler/rustc_session/src/tests.rs new file mode 100644 index 0000000000000..9f31db8396b9d --- /dev/null +++ b/compiler/rustc_session/src/tests.rs @@ -0,0 +1,71 @@ +use std::str::FromStr; + +use crate::config::{LinkSelfContained, LinkSelfContainedCrt, LinkSelfContainedLinker}; + +/// When adding support for `-C link-self-contained=linker`, we want to ensure the existing +/// values are still supported as-is: they are option values that can be used on stable. +#[test] +pub fn parse_stable_self_contained() { + // The existing default is when the argument is not used, the behavior depends on the + // target. + assert_eq!( + LinkSelfContained::default(), + LinkSelfContained { crt: LinkSelfContainedCrt::Auto, linker: LinkSelfContainedLinker::Off } + ); + + // Turning the flag `on` should currently only enable the default on stable. + assert_eq!( + LinkSelfContained::from_str("on"), + Ok(LinkSelfContained { + crt: LinkSelfContainedCrt::On, + linker: LinkSelfContainedLinker::Off + }) + ); + + // Turning the flag `off` applies to both facets. + assert_eq!( + LinkSelfContained::from_str("off"), + Ok(LinkSelfContained { + crt: LinkSelfContainedCrt::Off, + linker: LinkSelfContainedLinker::Off + }) + ); + + assert_eq!( + LinkSelfContained::from_str("crt"), + Ok(LinkSelfContained { + crt: LinkSelfContainedCrt::On, + linker: LinkSelfContainedLinker::Off + }) + ); +} + +#[test] +pub fn parse_self_contained_with_linker() { + // Turning the linker on doesn't change the CRT behavior + assert_eq!( + LinkSelfContained::from_str("linker"), + Ok(LinkSelfContained { + crt: LinkSelfContainedCrt::Auto, + linker: LinkSelfContainedLinker::On + }) + ); + + assert_eq!( + LinkSelfContained::from_str("all"), + Ok(LinkSelfContained { + crt: LinkSelfContainedCrt::On, + linker: LinkSelfContainedLinker::On + }) + ); + + // If `linker` is turned on by default someday, we need to be able to go back to the current + // default. + assert_eq!( + LinkSelfContained::from_str("auto"), + Ok(LinkSelfContained { + crt: LinkSelfContainedCrt::Auto, + linker: LinkSelfContainedLinker::Off + }) + ); +} From 72ec7a3ccbc379f92912d60adf7dfdd757a992a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Mon, 25 Apr 2022 19:43:32 +0200 Subject: [PATCH 3/6] use `LinkSelfContained` for `-C link-self-contained` --- compiler/rustc_codegen_ssa/src/back/link.rs | 2 +- compiler/rustc_interface/src/tests.rs | 3 ++- compiler/rustc_session/src/config.rs | 2 +- compiler/rustc_session/src/options.rs | 18 +++++++++++++++++- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 04ec1e7f3c1af..06df9485c5220 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -1534,7 +1534,7 @@ fn detect_self_contained_mingw(sess: &Session) -> bool { /// Whether we link to our own CRT objects instead of relying on gcc to pull them. /// We only provide such support for a very limited number of targets. fn crt_objects_fallback(sess: &Session, crate_type: CrateType) -> bool { - if let Some(self_contained) = sess.opts.cg.link_self_contained { + if let Some(self_contained) = sess.opts.cg.link_self_contained.crt.is_explicitly_set() { return self_contained; } diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 1327bf6fcd427..0b83e18aad7d9 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -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::LinkSelfContained; use rustc_session::config::Strip; use rustc_session::config::{build_configuration, build_session_options, to_crate_config}; use rustc_session::config::{ @@ -549,7 +550,7 @@ fn test_codegen_options_tracking_hash() { untracked!(incremental, Some(String::from("abc"))); // `link_arg` is omitted because it just forwards to `link_args`. untracked!(link_args, vec![String::from("abc"), String::from("def")]); - untracked!(link_self_contained, Some(true)); + untracked!(link_self_contained, LinkSelfContained::on()); untracked!(linker, Some(PathBuf::from("linker"))); untracked!(linker_flavor, Some(LinkerFlavor::Gcc)); untracked!(no_stack_check, true); diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index d506115a0a1a5..7b9a0b0ad544c 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -230,7 +230,7 @@ impl LinkSelfContained { } /// Keeps stable behavior only turning the self-contained CRT linking on. - fn on() -> Self { + pub fn on() -> Self { LinkSelfContained::crt_only() } diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 12e00ef51140b..5193341d6ee9f 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -414,6 +414,8 @@ mod desc { pub const parse_split_dwarf_kind: &str = "one of supported split dwarf modes (`split` or `single`)"; pub const parse_gcc_ld: &str = "one of: no value, `lld`"; + pub const parse_link_self_contained: &str = + "one of: `y`, `yes`, `on`, `n`, `no`, `off`, `auto`, `crt`, `linker`, `all`"; pub const parse_stack_protector: &str = "one of (`none` (default), `basic`, `strong`, or `all`)"; pub const parse_branch_protection: &str = @@ -1027,6 +1029,20 @@ mod parse { true } + /// Parses `-C link-self-contained`: it used to be a boolean without a static default, but now + /// also accepts some strings, in addition to the regular boolean values. + pub(crate) fn parse_link_self_contained(slot: &mut LinkSelfContained, v: Option<&str>) -> bool { + // Whenever `-C link-self-contained` is passed without a value, it's an opt-in + // just like `parse_opt_bool`. + let s = v.unwrap_or("y"); + + match LinkSelfContained::from_str(s).ok() { + Some(value) => *slot = value, + None => return false, + } + true + } + pub(crate) fn parse_stack_protector(slot: &mut StackProtector, v: Option<&str>) -> bool { match v.and_then(|s| StackProtector::from_str(s).ok()) { Some(ssp) => *slot = ssp, @@ -1116,7 +1132,7 @@ options! { "extra arguments to append to the linker invocation (space separated)"), link_dead_code: Option = (None, parse_opt_bool, [TRACKED], "keep dead code at link time (useful for code coverage) (default: no)"), - link_self_contained: Option = (None, parse_opt_bool, [UNTRACKED], + link_self_contained: LinkSelfContained = (LinkSelfContained::default(), parse_link_self_contained, [UNTRACKED], "control whether to link Rust provided C objects/libraries or rely on C toolchain installed in the system"), linker: Option = (None, parse_opt_pathbuf, [UNTRACKED], From 3368b62b32f7ffd5904cd90cc702701908382bcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Mon, 25 Apr 2022 20:49:18 +0200 Subject: [PATCH 4/6] add fixme to improve `-Clink-self-contained` doc --- compiler/rustc_session/src/options.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 5193341d6ee9f..c4631979f8f07 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -414,6 +414,8 @@ mod desc { pub const parse_split_dwarf_kind: &str = "one of supported split dwarf modes (`split` or `single`)"; pub const parse_gcc_ld: &str = "one of: no value, `lld`"; + // FIXME: add link to reference or other documentation, where the `-Clink-self-contained` + // options will be explained in more detail. pub const parse_link_self_contained: &str = "one of: `y`, `yes`, `on`, `n`, `no`, `off`, `auto`, `crt`, `linker`, `all`"; pub const parse_stack_protector: &str = From 271a11a492c4bde2ee8f813a62589f21e62b63fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Tue, 26 Apr 2022 23:45:23 +0200 Subject: [PATCH 5/6] require `-Z unstable-options` to use the link-self-contained new flag values only the stable values for `-Clink-self-contained` can be used on stable until we have more feedback on the interface --- compiler/rustc_session/src/config.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 7b9a0b0ad544c..1e964272d0999 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -2543,6 +2543,26 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options { ); } + // For testing purposes, until we have more feedback about these options: ensure `-Z + // unstable-options` is enabled when using the unstable `-C link-self-contained` options. + if !debugging_opts.unstable_options { + let uses_unstable_self_contained_option = + matches.opt_strs("C").iter().any(|option| match option.as_str() { + "link-self-contained=crt" + | "link-self-contained=auto" + | "link-self-contained=linker" + | "link-self-contained=all" => true, + _ => false, + }); + if uses_unstable_self_contained_option { + early_error( + error_format, + "only `-C link-self-contained` values `y`/`yes`/`on`/`n`/`no`/`off` are stable, \ + the `-Z unstable-options` flag must also be passed to use the unstable values", + ); + } + } + let prints = collect_print_requests(&mut cg, &mut debugging_opts, matches, error_format); let cg = cg; From d477d45896d25f97be28f02e646f70c0583f8dc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Wed, 27 Apr 2022 19:15:28 +0200 Subject: [PATCH 6/6] test unstable values of `-C link-self-contained` These values require using `-Z unstable options` --- .../linkers/unstable_link_self_contained.all.stderr | 2 ++ .../linkers/unstable_link_self_contained.auto.stderr | 2 ++ .../linkers/unstable_link_self_contained.crt.stderr | 2 ++ .../unstable_link_self_contained.linker.stderr | 2 ++ src/test/ui/linkers/unstable_link_self_contained.rs | 11 +++++++++++ 5 files changed, 19 insertions(+) create mode 100644 src/test/ui/linkers/unstable_link_self_contained.all.stderr create mode 100644 src/test/ui/linkers/unstable_link_self_contained.auto.stderr create mode 100644 src/test/ui/linkers/unstable_link_self_contained.crt.stderr create mode 100644 src/test/ui/linkers/unstable_link_self_contained.linker.stderr create mode 100644 src/test/ui/linkers/unstable_link_self_contained.rs diff --git a/src/test/ui/linkers/unstable_link_self_contained.all.stderr b/src/test/ui/linkers/unstable_link_self_contained.all.stderr new file mode 100644 index 0000000000000..33c37c7119e6e --- /dev/null +++ b/src/test/ui/linkers/unstable_link_self_contained.all.stderr @@ -0,0 +1,2 @@ +error: only `-C link-self-contained` values `y`/`yes`/`on`/`n`/`no`/`off` are stable, the `-Z unstable-options` flag must also be passed to use the unstable values + diff --git a/src/test/ui/linkers/unstable_link_self_contained.auto.stderr b/src/test/ui/linkers/unstable_link_self_contained.auto.stderr new file mode 100644 index 0000000000000..33c37c7119e6e --- /dev/null +++ b/src/test/ui/linkers/unstable_link_self_contained.auto.stderr @@ -0,0 +1,2 @@ +error: only `-C link-self-contained` values `y`/`yes`/`on`/`n`/`no`/`off` are stable, the `-Z unstable-options` flag must also be passed to use the unstable values + diff --git a/src/test/ui/linkers/unstable_link_self_contained.crt.stderr b/src/test/ui/linkers/unstable_link_self_contained.crt.stderr new file mode 100644 index 0000000000000..33c37c7119e6e --- /dev/null +++ b/src/test/ui/linkers/unstable_link_self_contained.crt.stderr @@ -0,0 +1,2 @@ +error: only `-C link-self-contained` values `y`/`yes`/`on`/`n`/`no`/`off` are stable, the `-Z unstable-options` flag must also be passed to use the unstable values + diff --git a/src/test/ui/linkers/unstable_link_self_contained.linker.stderr b/src/test/ui/linkers/unstable_link_self_contained.linker.stderr new file mode 100644 index 0000000000000..33c37c7119e6e --- /dev/null +++ b/src/test/ui/linkers/unstable_link_self_contained.linker.stderr @@ -0,0 +1,2 @@ +error: only `-C link-self-contained` values `y`/`yes`/`on`/`n`/`no`/`off` are stable, the `-Z unstable-options` flag must also be passed to use the unstable values + diff --git a/src/test/ui/linkers/unstable_link_self_contained.rs b/src/test/ui/linkers/unstable_link_self_contained.rs new file mode 100644 index 0000000000000..c4e134f3ac44a --- /dev/null +++ b/src/test/ui/linkers/unstable_link_self_contained.rs @@ -0,0 +1,11 @@ +// check-fail +// revisions: crt auto linker all +// [crt] compile-flags: -C link-self-contained=crt +// [auto] compile-flags: -C link-self-contained=auto +// [linker] compile-flags: -C link-self-contained=linker +// [all] compile-flags: -C link-self-contained=all + +// Test ensuring that the unstable values of the stable `-C link-self-contained` flag +// require using `-Z unstable options` + +fn main() {}