diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs index 69487d2039c31..e8060fac6c9a1 100644 --- a/compiler/rustc_codegen_ssa/src/target_features.rs +++ b/compiler/rustc_codegen_ssa/src/target_features.rs @@ -359,7 +359,7 @@ pub fn internal_target_features<'a, const N: usize>( } // Check feature stability. - if let Stability::InternalOnly { reason, hard_error } = stability { + if let Stability::InternalOnly { reason, hard_error, .. } = stability { let diag = diagnostics::InternalOnlyCTargetFeature { feature: base_feature, enabled: if enable { "enabled" } else { "disabled" }, diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 654a782262e22..4f1cf818e8ad2 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -22,7 +22,8 @@ use rustc_middle::dep_graph::WorkProductMap; use rustc_middle::ty::{CurrentGcx, TyCtxt}; use rustc_query_impl::{CollectActiveJobsKind, collect_active_query_jobs}; use rustc_session::config::{ - Cfg, CrateType, Jobs, OutFileName, OutputFilenames, OutputTypes, Sysroot, host_tuple, + Cfg, CrateType, Jobs, OptionsTargetModifiers, OutFileName, OutputFilenames, OutputTypes, + Sysroot, host_tuple, }; use rustc_session::{EarlyDiagCtxt, IncrCompSession, Session, filesearch}; use rustc_span::edition::Edition; @@ -55,10 +56,10 @@ pub(crate) fn add_configuration( sess.target .rust_target_features() .iter() - .filter_map(|(feature, gate, _)| { - if gate.in_cfg() + .filter_map(|(feature, stab, _)| { + if stab.in_cfg() && (sess.is_nightly_build() - || gate.requires_nightly(/* in_cfg */ true).is_none()) + || stab.requires_nightly(/* in_cfg */ true).is_none()) { Some(Symbol::intern(feature)) } else { @@ -69,6 +70,21 @@ pub(crate) fn add_configuration( .map(|feature| (sym::target_feature, Some(feature))), ); + // Record relevant target features as target modifier. + sess.opts.target_modifiers.insert( + OptionsTargetModifiers::TargetFeatures, + // Join all target-modifier target features into a single comma-separated string, + // sorted by the order they appear in in `rust_target_features`. + sess.target + .rust_target_features() + .iter() + .filter(|(_, stab, _)| stab.is_target_modifier()) + .map(|(feature, ..)| *feature) + .filter(|feature| tf_cfg.internal_target_features.contains(&Symbol::intern(feature))) + .collect::>() + .join(","), + ); + // Store all of them in the session. sess.internal_target_features.extend(tf_cfg.internal_target_features.into_sorted_stable_ord()); diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 8879c175da2f9..a01c9ad0316f6 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -11,7 +11,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::owned_slice::OwnedSlice; use rustc_data_structures::svh::Svh; use rustc_data_structures::sync::{self, FreezeReadGuard, FreezeWriteGuard}; -use rustc_data_structures::unord::UnordMap; +use rustc_data_structures::unord::{UnordMap, UnordSet}; use rustc_expand::base::SyntaxExtension; use rustc_hir as hir; use rustc_hir::def_id::{CrateNum, LOCAL_CRATE, LocalDefId, StableCrateId}; @@ -361,6 +361,61 @@ impl CStore { return; } let extern_crate = data.name(); + + if prefix.is_empty() { + // A synthetic target modifier, does not correspond to an actual flag. + match opt_name.as_str() { + "target-feature" => { + // Compute the features we have locally that don't exist in the other crate, + // and the features the other crate has that are missing locally. + let mut local_features = FxHashSet::from_iter( + flag_local_value.unwrap().split(",").filter(|s| !s.is_empty()), + ); + let mut extern_features = FxHashSet::from_iter( + flag_extern_value.unwrap().split(",").filter(|s| !s.is_empty()), + ); + #[allow(rustc::potential_query_instability)] // we are sorting below + let both_features = FxHashSet::from_iter( + local_features.intersection(&extern_features).copied(), + ); + #[allow(rustc::potential_query_instability)] // we are sorting below + for feature in both_features { + local_features.remove(feature); + extern_features.remove(feature); + } + let local_features = + UnordSet::from(local_features).into_sorted_stable_ord(); + let extern_features = + UnordSet::from(extern_features).into_sorted_stable_ord(); + assert!(local_features.len() > 0 || extern_features.len() > 0); + + let diag = format!( + "mixing target features will cause an ABI mismatch in crate `{local_crate}`" + ); + let mut diag = tcx.dcx().struct_warn(diag); + diag.help("some target features modify the ABI so Rust crates compiled with different values for these target features cannot be used together safely"); + if local_features.len() > 0 { + let features = local_features.join(", "); + let before = + if local_features.len() == 1 { "feature" } else { "features" }; + let after = if local_features.len() == 1 { "is" } else { "are" }; + diag.help(format!("the target {before} {features} {after} enabled in this crate but disabled in `{extern_crate}`")); + } + if extern_features.len() > 0 { + let features = extern_features.join(", "); + let before = + if extern_features.len() == 1 { "feature" } else { "features" }; + let after = if extern_features.len() == 1 { "is" } else { "are" }; + diag.help(format!("the target {before} {features} {after} enabled in `{extern_crate}` but disabled in this crate")); + } + diag.help("if you are sure this will not cause problems, you may use `-Cunsafe-allow-abi-mismatch=target-feature` to silence this warning"); + diag.emit(); + } + _ => panic!("unhandled synthetic target feature {opt_name}"), + } + return; + } + let flag_name = opt_name.clone(); let flag_name_prefixed = format!("-{}{}", prefix, opt_name); diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 785abc46e61b6..15b9dad7c97eb 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -68,9 +68,11 @@ macro_rules! hash_substruct { /// Target modifier enum value + user value ('2') from external crate /// is converted into description: prefix ('Z'), name ('regparm'), tech value ('Some(2)'). pub struct ExtendedTargetModifierInfo { - /// Flag prefix (usually, 'C' for codegen flags or 'Z' for unstable flags) + /// Flag prefix (usually, 'C' for codegen flags or 'Z' for unstable flags). And empty string + /// indicates a synthetic target modifier, which does not correspond directly to a flag. pub prefix: String, - /// Flag name + /// Flag name. For synthetic features, this is still relevant as the name used to suppress + /// the error for mismatches involving this target modifier. pub name: String, /// Flag parsed technical value pub tech_value: String, @@ -180,6 +182,7 @@ impl TargetModifier { return target_modifier_consistency_check::target_cpu(sess, self, other); } }, + _ => {} }; match other { Some(other) => self.extend().tech_value == other.extend().tech_value, @@ -218,6 +221,9 @@ macro_rules! top_level_options { $tmod_variant($tmod_enum), )? )* + // Synthetic target modifiers, manually computed. + /// List of target features that are actually target modifiers. + TargetFeatures, } impl OptionsTargetModifiers { @@ -228,6 +234,13 @@ macro_rules! top_level_options { Self::$tmod_variant(v) => v.reparse(user_value), )? )* + Self::TargetFeatures => { + ExtendedTargetModifierInfo { + prefix: String::new(), + name: "target-feature".into(), + tech_value: format!("{:?}", user_value), + } + } #[allow(unreachable_patterns)] _ => panic!("unknown target modifier option: {self:?}"), } @@ -252,6 +265,8 @@ macro_rules! top_level_options { $(#[$attr])* pub $opt: $t, )* + /// Store values for target modifiers. `gather_target_modifiers` takes those values + /// to decide what to store in metadata. pub target_modifiers: BTreeMap, pub mitigation_coverage_map: mitigation_coverage::MitigationCoverageMap, } @@ -290,6 +305,7 @@ macro_rules! top_level_options { pub fn gather_target_modifiers(&self) -> Vec { let mut mods = Vec::::new(); + // Forward values from `self.target_modifiers` into `mods`. $( $( // Only expand for flags that have `TARGET_MODIFIER`. @@ -297,6 +313,7 @@ macro_rules! top_level_options { self.$opt.gather_target_modifiers(&mut mods, &self.target_modifiers); )? )* + tmod_push_impl(OptionsTargetModifiers::TargetFeatures, &self.target_modifiers, &mut mods); mods.sort_by(|a, b| a.opt.cmp(&b.opt)); mods } diff --git a/compiler/rustc_target/src/target_features.rs b/compiler/rustc_target/src/target_features.rs index f1dd2d8191985..04e35e4883044 100644 --- a/compiler/rustc_target/src/target_features.rs +++ b/compiler/rustc_target/src/target_features.rs @@ -77,12 +77,15 @@ pub enum Stability { /// features are actually ABI configuration flags (such as "soft-float" on many targets). /// /// However, "internal" target features can still sometimes be enabled or disabled via - /// `-Ctarget-cpu` or Rust/LLVM target feature implications. Make sure nothing implies this - /// target feature and nothing is implied by this target feature (except for other internal-only - /// features). Ideally, ABI-relevant target features are pinned down (marked as required or - /// incompatible) in [`Target::abi_required_features`]. + /// `-Ctarget-cpu` or Rust/LLVM target feature implications. See `target_modifier` below for how + /// to protect ABI-relevant target features. InternalOnly { reason: &'static str, + /// Whether this target feature must be consistent across all crates in a crate graph. If + /// this is false, then either it should be fine for the target feature to differ across + /// crates, or the target feature is set by another flag that's already a target modifier, + /// or it needs to be pinned down via [`Target::abi_required_features`]. + target_modifier: bool, /// True if this is always an error, false if this can be reported as a warning when set via /// `-Ctarget-feature` (and a hard error when set via `#[target_feature]`). hard_error: bool, @@ -103,6 +106,11 @@ impl Stability { ) } + /// Returns whether this target feature is to be treated as a target modifier. + pub fn is_target_modifier(&self) -> bool { + matches!(self, Stability::InternalOnly { target_modifier: true, .. }) + } + /// Returns the nightly feature that is required to toggle this target feature via /// `#[target_feature]`/`-Ctarget-feature` or to test it via `cfg(target_feature)`. /// (For `cfg` we only care whether the feature is nightly or not, we don't require @@ -145,7 +153,7 @@ impl Stability { Stability::Unstable(_) | Stability::CfgStableToggleUnstable(_) | Stability::Stable { .. } => Ok(()), - Stability::InternalOnly { reason, hard_error: _ } => Err(reason), + Stability::InternalOnly { reason, .. } => Err(reason), } } } @@ -164,9 +172,9 @@ static ARM_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ ("aes", Unstable(sym::arm_target_feature), &["neon"]), ( "atomics-32", - // Not implied by any CPU model or other feature. Stability::InternalOnly { reason: "unsound because it changes the ABI of atomic operations", + target_modifier: true, hard_error: false, }, &[], @@ -252,8 +260,11 @@ static AARCH64_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // We forbid directly toggling just `fp-armv8`; it must be toggled with `neon`. ( "fp-armv8", - // Pinned down by [`Target::abi_required_features`] when needed. - Stability::InternalOnly { reason: "Rust ties `fp-armv8` to `neon`", hard_error: false }, + Stability::InternalOnly { + reason: "unsound because it changes the ABI of float types", + target_modifier: true, + hard_error: false, + }, &[], ), // FEAT_FP8 @@ -320,8 +331,13 @@ static AARCH64_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ ("rdm", Stable, &["neon"]), ( "reserve-x18", - // Not implied by any CPU model or other feature; the compiler flag is a target modifier. - InternalOnly { reason: "use `-Zfixed-x18` compiler flag instead", hard_error: false }, + InternalOnly { + reason: "use `-Zfixed-x18` compiler flag instead", + // The compiler flag is already a target modifier so we don't need to track this again. + // Cannot be implicitly toggled via implications or CPU models. + target_modifier: false, + hard_error: false, + }, &[], ), // FEAT_SB @@ -502,27 +518,33 @@ static X86_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ ("rdseed", Stable, &[]), ( "retpoline-external-thunk", - // Not implied by any CPU model or other feature; the compiler flag is a target modifier. Stability::InternalOnly { reason: "use `-Zretpoline-external-thunk` compiler flag instead", + // The compiler flag is already a target modifier so we don't need to track this again. + // Cannot be implicitly toggled via implications or CPU models. + target_modifier: false, hard_error: false, }, &[], ), ( "retpoline-indirect-branches", - // Not implied by any CPU model or other feature; the compiler flag is a target modifier. Stability::InternalOnly { reason: "use `-Zretpoline` compiler flag instead", + // The compiler flag is already a target modifier so we don't need to track this again. + // Cannot be implicitly toggled via implications or CPU models. + target_modifier: false, hard_error: false, }, &[], ), ( "retpoline-indirect-calls", - // Not implied by any CPU model or other feature; the compiler flag is a target modifier. Stability::InternalOnly { reason: "use `-Zretpoline` compiler flag instead", + // The compiler flag is already a target modifier so we don't need to track this again. + // Cannot be implicitly toggled via implications or CPU models. + target_modifier: false, hard_error: false, }, &[], @@ -534,8 +556,11 @@ static X86_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ ("sm4", Stable, &["avx2"]), ( "soft-float", - // Pinned down by [`Target::abi_required_features`]. - Stability::InternalOnly { reason: "use a soft-float target instead", hard_error: false }, + Stability::InternalOnly { + reason: "use a soft-float target instead", + target_modifier: true, + hard_error: false, + }, &[], ), ("sse", Stable, &[]), @@ -599,8 +624,11 @@ static POWERPC_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ ("altivec", Unstable(sym::powerpc_target_feature), &[]), ( "hard-float", - // Pinned down by [`Target::abi_required_features`]. - InternalOnly { reason: "unsupported ABI-configuration feature", hard_error: false }, + InternalOnly { + reason: "unsupported ABI-configuration feature", + target_modifier: true, + hard_error: false, + }, &[], ), ("msync", Unstable(sym::powerpc_target_feature), &[]), @@ -614,8 +642,11 @@ static POWERPC_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ ("quadword-atomics", Unstable(sym::powerpc_target_feature), &[]), ( "spe", - // Pinned down by [`Target::abi_required_features`]. - InternalOnly { reason: "unsupported ABI-configuration feature", hard_error: false }, + InternalOnly { + reason: "unsupported ABI-configuration feature", + target_modifier: true, + hard_error: false, + }, &[], ), ("vsx", Unstable(sym::powerpc_target_feature), &["altivec"]), @@ -681,9 +712,9 @@ static RISCV_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ ("f", CfgStableToggleUnstable(sym::riscv_target_feature), &["zicsr"]), ( "forced-atomics", - // Not implied by any CPU model or other feature. Stability::InternalOnly { reason: "unsound because it changes the ABI of atomic operations", + target_modifier: true, hard_error: false, }, &[], @@ -942,8 +973,11 @@ const IBMZ_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ ("miscellaneous-extensions-3", Stable, &[]), ("miscellaneous-extensions-4", Stable, &[]), ("nnp-assist", Stable, &["vector"]), - // Pinned down by [`Target::abi_required_features`]. - ("soft-float", InternalOnly { reason: "unsupported ABI-configuration feature", hard_error: false }, &[]), + ( + "soft-float", + InternalOnly { reason: "unsupported ABI-configuration feature", target_modifier: true, hard_error: false }, + &[], + ), ("transactional-execution", Unstable(sym::s390x_target_feature), &[]), ("vector", Stable, &[]), ("vector-enhancements-1", Stable, &["vector"]), @@ -997,8 +1031,12 @@ static AVR_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ ("spmx", Unstable(sym::avr_target_feature), &[]), ( "sram", - // Pinned down by [`Target::abi_required_features`]. - InternalOnly { reason: "devices that have no SRAM are unsupported", hard_error: false }, + InternalOnly { + reason: "devices that have no SRAM are unsupported", + // Always required by [`Target::abi_required_features`]. + target_modifier: false, + hard_error: false, + }, &[], ), ("tinyencoding", Unstable(sym::avr_target_feature), &[]), @@ -1013,9 +1051,10 @@ const XTENSA_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ ("interrupt", Unstable(sym::xtensa_target_feature), &["exception"]), ( "windowed", - // Pinned down by [`Target::abi_required_features`]. InternalOnly { reason: "windowed changes the Xtensa calling convention", + // Always required by [`Target::abi_required_features`]. + target_modifier: false, hard_error: false, }, &["exception"], diff --git a/tests/ui/target-feature/abi-relevant-target-modifier.rs b/tests/ui/target-feature/abi-relevant-target-modifier.rs new file mode 100644 index 0000000000000..611ead19ed33a --- /dev/null +++ b/tests/ui/target-feature/abi-relevant-target-modifier.rs @@ -0,0 +1,12 @@ +// Currently still just a warning. +//@ build-pass +//@ compile-flags: --crate-type=rlib --target=armv7-unknown-linux-gnueabihf +//@ needs-llvm-components: arm +//@ aux-build: using-atomics-32.rs +//@ ignore-backends: gcc +#![feature(no_core)] +#![no_core] + +extern crate using_atomics_32; + +//~? WARN mixing target features will cause an ABI mismatch diff --git a/tests/ui/target-feature/abi-relevant-target-modifier.stderr b/tests/ui/target-feature/abi-relevant-target-modifier.stderr new file mode 100644 index 0000000000000..fbf2ac5bff976 --- /dev/null +++ b/tests/ui/target-feature/abi-relevant-target-modifier.stderr @@ -0,0 +1,8 @@ +warning: mixing target features will cause an ABI mismatch in crate `abi_relevant_target_modifier` + | + = help: some target features modify the ABI so Rust crates compiled with different values for these target features cannot be used together safely + = help: the target feature atomics-32 is enabled in `using_atomics_32` but disabled in this crate + = help: if you are sure this will not cause problems, you may use `-Cunsafe-allow-abi-mismatch=target-feature` to silence this warning + +warning: 1 warning emitted + diff --git a/tests/ui/target-feature/auxiliary/using-atomics-32.rs b/tests/ui/target-feature/auxiliary/using-atomics-32.rs new file mode 100644 index 0000000000000..33dd8ebeb6592 --- /dev/null +++ b/tests/ui/target-feature/auxiliary/using-atomics-32.rs @@ -0,0 +1,5 @@ +//@ compile-flags: --crate-type=rlib --target=armv7-unknown-linux-gnueabihf --emit=metadata +//@ needs-llvm-components: arm +//@ compile-flags: -Ctarget-feature=+atomics-32 +#![feature(no_core)] +#![no_core]