From ffad9aac27ff8a78f5d751bf88250470e2e9d790 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 2 Sep 2024 11:45:59 +0200 Subject: [PATCH] mark some target features as 'forbidden' so they cannot be (un)set For now, this is just a warning, but should become a hard error in the future --- compiler/rustc_codegen_gcc/messages.ftl | 11 +- compiler/rustc_codegen_gcc/src/errors.rs | 13 ++ compiler/rustc_codegen_gcc/src/gcc_util.rs | 65 +++++--- compiler/rustc_codegen_gcc/src/lib.rs | 3 +- compiler/rustc_codegen_llvm/messages.ftl | 5 + compiler/rustc_codegen_llvm/src/errors.rs | 9 ++ compiler/rustc_codegen_llvm/src/llvm_util.rs | 119 +++++++++------ compiler/rustc_codegen_ssa/messages.ftl | 3 + .../rustc_codegen_ssa/src/codegen_attrs.rs | 14 +- compiler/rustc_codegen_ssa/src/errors.rs | 9 ++ .../rustc_codegen_ssa/src/target_features.rs | 54 ++++--- compiler/rustc_middle/src/query/mod.rs | 5 +- compiler/rustc_session/src/config/cfg.rs | 5 +- compiler/rustc_target/src/target_features.rs | 142 +++++++++++------- .../using-target-feature-unstable.rs | 0 .../forbidden-target-feature-attribute.rs | 12 ++ .../forbidden-target-feature-attribute.stderr | 8 + .../forbidden-target-feature-cfg.rs | 15 ++ .../forbidden-target-feature-flag-disable.rs | 11 ++ ...rbidden-target-feature-flag-disable.stderr | 7 + .../forbidden-target-feature-flag.rs | 11 ++ .../forbidden-target-feature-flag.stderr | 7 + .../using-target-feature-unstable.rs | 0 23 files changed, 371 insertions(+), 157 deletions(-) rename tests/ui/{ => target-feature}/auxiliary/using-target-feature-unstable.rs (100%) create mode 100644 tests/ui/target-feature/forbidden-target-feature-attribute.rs create mode 100644 tests/ui/target-feature/forbidden-target-feature-attribute.stderr create mode 100644 tests/ui/target-feature/forbidden-target-feature-cfg.rs create mode 100644 tests/ui/target-feature/forbidden-target-feature-flag-disable.rs create mode 100644 tests/ui/target-feature/forbidden-target-feature-flag-disable.stderr create mode 100644 tests/ui/target-feature/forbidden-target-feature-flag.rs create mode 100644 tests/ui/target-feature/forbidden-target-feature-flag.stderr rename tests/ui/{ => target-feature}/using-target-feature-unstable.rs (100%) diff --git a/compiler/rustc_codegen_gcc/messages.ftl b/compiler/rustc_codegen_gcc/messages.ftl index bbae59ea7a55a..26ddc5732dd0b 100644 --- a/compiler/rustc_codegen_gcc/messages.ftl +++ b/compiler/rustc_codegen_gcc/messages.ftl @@ -8,6 +8,9 @@ codegen_gcc_invalid_minimum_alignment = codegen_gcc_lto_not_supported = LTO is not supported. You may get a linker error. +codegen_gcc_forbidden_ctarget_feature = + target feature `{$feature}` cannot be toggled with `-Ctarget-feature` + codegen_gcc_unwinding_inline_asm = GCC backend does not support unwinding from inline asm @@ -24,11 +27,15 @@ codegen_gcc_lto_dylib = lto cannot be used for `dylib` crate type without `-Zdyl codegen_gcc_lto_bitcode_from_rlib = failed to get bitcode from object file for LTO ({$gcc_err}) codegen_gcc_unknown_ctarget_feature = - unknown feature specified for `-Ctarget-feature`: `{$feature}` - .note = it is still passed through to the codegen backend + unknown and unstable feature specified for `-Ctarget-feature`: `{$feature}` + .note = it is still passed through to the codegen backend, but use of this feature might be unsound and the behavior of this feature can change in the future .possible_feature = you might have meant: `{$rust_feature}` .consider_filing_feature_request = consider filing a feature request +codegen_gcc_unstable_ctarget_feature = + unstable feature specified for `-Ctarget-feature`: `{$feature}` + .note = this feature is not stably supported; its behavior can change in the future + codegen_gcc_missing_features = add the missing features in a `target_feature` attribute diff --git a/compiler/rustc_codegen_gcc/src/errors.rs b/compiler/rustc_codegen_gcc/src/errors.rs index dc1895f437b52..7a586b5b04c52 100644 --- a/compiler/rustc_codegen_gcc/src/errors.rs +++ b/compiler/rustc_codegen_gcc/src/errors.rs @@ -17,6 +17,19 @@ pub(crate) struct UnknownCTargetFeature<'a> { pub rust_feature: PossibleFeature<'a>, } +#[derive(Diagnostic)] +#[diag(codegen_gcc_unstable_ctarget_feature)] +#[note] +pub(crate) struct UnstableCTargetFeature<'a> { + pub feature: &'a str, +} + +#[derive(Diagnostic)] +#[diag(codegen_gcc_forbidden_ctarget_feature)] +pub(crate) struct ForbiddenCTargetFeature<'a> { + pub feature: &'a str, +} + #[derive(Subdiagnostic)] pub(crate) enum PossibleFeature<'a> { #[help(codegen_gcc_possible_feature)] diff --git a/compiler/rustc_codegen_gcc/src/gcc_util.rs b/compiler/rustc_codegen_gcc/src/gcc_util.rs index 3104088e0d5e9..65279c9495a3d 100644 --- a/compiler/rustc_codegen_gcc/src/gcc_util.rs +++ b/compiler/rustc_codegen_gcc/src/gcc_util.rs @@ -5,10 +5,13 @@ use rustc_codegen_ssa::errors::TargetFeatureDisableOrEnable; use rustc_data_structures::fx::FxHashMap; use rustc_middle::bug; use rustc_session::Session; -use rustc_target::target_features::RUSTC_SPECIFIC_FEATURES; +use rustc_target::target_features::{RUSTC_SPECIFIC_FEATURES, Stability}; use smallvec::{SmallVec, smallvec}; -use crate::errors::{PossibleFeature, UnknownCTargetFeature, UnknownCTargetFeaturePrefix}; +use crate::errors::{ + ForbiddenCTargetFeature, PossibleFeature, UnknownCTargetFeature, UnknownCTargetFeaturePrefix, + UnstableCTargetFeature, +}; /// The list of GCC features computed from CLI flags (`-Ctarget-cpu`, `-Ctarget-feature`, /// `--target` and similar). @@ -43,7 +46,7 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec Vec { + let rust_feature = + known_features.iter().find_map(|&(rust_feature, _, _)| { + let gcc_features = to_gcc_features(sess, rust_feature); + if gcc_features.contains(&feature) + && !gcc_features.contains(&rust_feature) + { + Some(rust_feature) + } else { + None + } + }); + let unknown_feature = if let Some(rust_feature) = rust_feature { + UnknownCTargetFeature { + feature, + rust_feature: PossibleFeature::Some { rust_feature }, + } + } else { + UnknownCTargetFeature { feature, rust_feature: PossibleFeature::None } + }; + sess.dcx().emit_warn(unknown_feature); } - }); - let unknown_feature = if let Some(rust_feature) = rust_feature { - UnknownCTargetFeature { - feature, - rust_feature: PossibleFeature::Some { rust_feature }, + Some((_, Stability::Stable, _)) => {} + Some((_, Stability::Unstable(_), _)) => { + // An unstable feature. Warn about using it. + sess.dcx().emit_warn(UnstableCTargetFeature { feature }); } - } else { - UnknownCTargetFeature { feature, rust_feature: PossibleFeature::None } - }; - sess.dcx().emit_warn(unknown_feature); - } + Some((_, Stability::Forbidden { .. }, _)) => { + sess.dcx().emit_err(ForbiddenCTargetFeature { feature }); + } + } - if diagnostics { // FIXME(nagisa): figure out how to not allocate a full hashset here. featsmap.insert(feature, enable_disable == '+'); } - // rustc-specific features do not get passed down to GCC… - if RUSTC_SPECIFIC_FEATURES.contains(&feature) { - return None; - } // ... otherwise though we run through `to_gcc_features` when // passing requests down to GCC. This means that all in-language // features also work on the command line instead of having two diff --git a/compiler/rustc_codegen_gcc/src/lib.rs b/compiler/rustc_codegen_gcc/src/lib.rs index 7486eefeb85a7..f70dc94b26741 100644 --- a/compiler/rustc_codegen_gcc/src/lib.rs +++ b/compiler/rustc_codegen_gcc/src/lib.rs @@ -491,8 +491,9 @@ pub fn target_features( ) -> Vec { // TODO(antoyo): use global_gcc_features. sess.target - .supported_target_features() + .rust_target_features() .iter() + .filter(|(_, gate, _)| gate.is_supported()) .filter_map(|&(feature, gate, _)| { if sess.is_nightly_build() || allow_unstable || gate.is_stable() { Some(feature) diff --git a/compiler/rustc_codegen_llvm/messages.ftl b/compiler/rustc_codegen_llvm/messages.ftl index 0950e4bb26bac..63c64269eb805 100644 --- a/compiler/rustc_codegen_llvm/messages.ftl +++ b/compiler/rustc_codegen_llvm/messages.ftl @@ -7,6 +7,11 @@ codegen_llvm_dynamic_linking_with_lto = codegen_llvm_fixed_x18_invalid_arch = the `-Zfixed-x18` flag is not supported on the `{$arch}` architecture +codegen_llvm_forbidden_ctarget_feature = + target feature `{$feature}` cannot be toggled with `-Ctarget-feature`: {$reason} + .note = this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! +codegen_llvm_forbidden_ctarget_feature_issue = for more information, see issue #116344 + codegen_llvm_from_llvm_diag = {$message} codegen_llvm_from_llvm_optimization_diag = {$filename}:{$line}:{$column} {$pass_name} ({$kind}): {$message} diff --git a/compiler/rustc_codegen_llvm/src/errors.rs b/compiler/rustc_codegen_llvm/src/errors.rs index 0d436e1891ece..3cdb5b971d908 100644 --- a/compiler/rustc_codegen_llvm/src/errors.rs +++ b/compiler/rustc_codegen_llvm/src/errors.rs @@ -31,6 +31,15 @@ pub(crate) struct UnstableCTargetFeature<'a> { pub feature: &'a str, } +#[derive(Diagnostic)] +#[diag(codegen_llvm_forbidden_ctarget_feature)] +#[note] +#[note(codegen_llvm_forbidden_ctarget_feature_issue)] +pub(crate) struct ForbiddenCTargetFeature<'a> { + pub feature: &'a str, + pub reason: &'a str, +} + #[derive(Subdiagnostic)] pub(crate) enum PossibleFeature<'a> { #[help(codegen_llvm_possible_feature)] diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index 9adb1299b3d31..8b27a6a66777b 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -16,12 +16,12 @@ use rustc_session::Session; use rustc_session::config::{PrintKind, PrintRequest}; use rustc_span::symbol::Symbol; use rustc_target::spec::{MergeFunctions, PanicStrategy, SmallDataThresholdSupport}; -use rustc_target::target_features::{RUSTC_SPECIAL_FEATURES, RUSTC_SPECIFIC_FEATURES}; +use rustc_target::target_features::{RUSTC_SPECIAL_FEATURES, RUSTC_SPECIFIC_FEATURES, Stability}; use crate::back::write::create_informational_target_machine; use crate::errors::{ - FixedX18InvalidArch, InvalidTargetFeaturePrefix, PossibleFeature, UnknownCTargetFeature, - UnknownCTargetFeaturePrefix, UnstableCTargetFeature, + FixedX18InvalidArch, ForbiddenCTargetFeature, InvalidTargetFeaturePrefix, PossibleFeature, + UnknownCTargetFeature, UnknownCTargetFeaturePrefix, UnstableCTargetFeature, }; use crate::llvm; @@ -280,19 +280,29 @@ pub(crate) fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> Option Vec { - let mut features = vec![]; - - // Add base features for the target + let mut features: FxHashSet = Default::default(); + + // Add base features for the target. + // We do *not* add the -Ctarget-features there, and instead duplicate the logic for that below. + // The reason is that if LLVM considers a feature implied but we do not, we don't want that to + // show up in `cfg`. That way, `cfg` is entirely under our control -- except for the handling of + // the target CPU, that is still expanded to target features (with all their implied features) by + // LLVM. let target_machine = create_informational_target_machine(sess, true); + // Compute which of the known target features are enabled in the 'base' target machine. + // We only consider "supported" features; "forbidden" features are not reflected in `cfg` as of now. features.extend( sess.target - .supported_target_features() + .rust_target_features() .iter() + .filter(|(_, gate, _)| gate.is_supported()) .filter(|(feature, _, _)| { - // skip checking special features, as LLVM may not understands them + // skip checking special features, as LLVM may not understand them if RUSTC_SPECIAL_FEATURES.contains(feature) { return true; } @@ -323,7 +333,12 @@ pub fn target_features(sess: &Session, allow_unstable: bool) -> Vec { if enabled { features.extend(sess.target.implied_target_features(std::iter::once(feature))); } else { + // We don't care about the order in `features` since the only thing we use it for is the + // `features.contains` below. + #[allow(rustc::potential_query_instability)] features.retain(|f| { + // Keep a feature if it does not imply `feature`. Or, equivalently, + // remove the reverse-dependencies of `feature`. !sess.target.implied_target_features(std::iter::once(*f)).contains(&feature) }); } @@ -331,8 +346,9 @@ pub fn target_features(sess: &Session, allow_unstable: bool) -> Vec { // Filter enabled features based on feature gates sess.target - .supported_target_features() + .rust_target_features() .iter() + .filter(|(_, gate, _)| gate.is_supported()) .filter_map(|&(feature, gate, _)| { if sess.is_nightly_build() || allow_unstable || gate.is_stable() { Some(feature) @@ -392,9 +408,13 @@ fn print_target_features(out: &mut String, sess: &Session, tm: &llvm::TargetMach let mut known_llvm_target_features = FxHashSet::<&'static str>::default(); let mut rustc_target_features = sess .target - .supported_target_features() + .rust_target_features() .iter() - .filter_map(|(feature, _gate, _implied)| { + .filter_map(|(feature, gate, _implied)| { + if !gate.is_supported() { + // Only list (experimentally) supported features. + return None; + } // LLVM asserts that these are sorted. LLVM and Rust both use byte comparison for these // strings. let llvm_feature = to_llvm_features(sess, *feature)?.llvm_feature_name; @@ -567,7 +587,7 @@ pub(crate) fn global_llvm_features( // -Ctarget-features if !only_base_features { - let supported_features = sess.target.supported_target_features(); + let known_features = sess.target.rust_target_features(); let mut featsmap = FxHashMap::default(); // insert implied features @@ -601,50 +621,53 @@ pub(crate) fn global_llvm_features( } }; + // Get the backend feature name, if any. + // This excludes rustc-specific features, which do not get passed to LLVM. let feature = backend_feature_name(sess, s)?; // Warn against use of LLVM specific feature names and unstable features on the CLI. if diagnostics { - let feature_state = supported_features.iter().find(|&&(v, _, _)| v == feature); - if feature_state.is_none() { - let rust_feature = - supported_features.iter().find_map(|&(rust_feature, _, _)| { - let llvm_features = to_llvm_features(sess, rust_feature)?; - if llvm_features.contains(feature) - && !llvm_features.contains(rust_feature) - { - Some(rust_feature) - } else { - None + let feature_state = known_features.iter().find(|&&(v, _, _)| v == feature); + match feature_state { + None => { + let rust_feature = + known_features.iter().find_map(|&(rust_feature, _, _)| { + let llvm_features = to_llvm_features(sess, rust_feature)?; + if llvm_features.contains(feature) + && !llvm_features.contains(rust_feature) + { + Some(rust_feature) + } else { + None + } + }); + let unknown_feature = if let Some(rust_feature) = rust_feature { + UnknownCTargetFeature { + feature, + rust_feature: PossibleFeature::Some { rust_feature }, } - }); - let unknown_feature = if let Some(rust_feature) = rust_feature { - UnknownCTargetFeature { - feature, - rust_feature: PossibleFeature::Some { rust_feature }, - } - } else { - UnknownCTargetFeature { feature, rust_feature: PossibleFeature::None } - }; - sess.dcx().emit_warn(unknown_feature); - } else if feature_state - .is_some_and(|(_name, feature_gate, _implied)| !feature_gate.is_stable()) - { - // An unstable feature. Warn about using it. - sess.dcx().emit_warn(UnstableCTargetFeature { feature }); + } else { + UnknownCTargetFeature { + feature, + rust_feature: PossibleFeature::None, + } + }; + sess.dcx().emit_warn(unknown_feature); + } + Some((_, Stability::Stable, _)) => {} + Some((_, Stability::Unstable(_), _)) => { + // An unstable feature. Warn about using it. + sess.dcx().emit_warn(UnstableCTargetFeature { feature }); + } + Some((_, Stability::Forbidden { reason }, _)) => { + sess.dcx().emit_warn(ForbiddenCTargetFeature { feature, reason }); + } } - } - if diagnostics { // FIXME(nagisa): figure out how to not allocate a full hashset here. featsmap.insert(feature, enable_disable == '+'); } - // rustc-specific features do not get passed down to LLVM… - if RUSTC_SPECIFIC_FEATURES.contains(&feature) { - return None; - } - - // ... otherwise though we run through `to_llvm_features` when + // We run through `to_llvm_features` when // passing requests down to LLVM. This means that all in-language // features also work on the command line instead of having two // different names when the LLVM name and the Rust name differ. diff --git a/compiler/rustc_codegen_ssa/messages.ftl b/compiler/rustc_codegen_ssa/messages.ftl index d07274920feaf..bb74698a060af 100644 --- a/compiler/rustc_codegen_ssa/messages.ftl +++ b/compiler/rustc_codegen_ssa/messages.ftl @@ -58,6 +58,9 @@ codegen_ssa_failed_to_write = failed to write {$path}: {$error} codegen_ssa_field_associated_value_expected = associated value expected for `{$name}` +codegen_ssa_forbidden_target_feature_attr = + target feature `{$feature}` cannot be toggled with `#[target_feature]`: {$reason} + codegen_ssa_ignoring_emit_path = ignoring emit path because multiple .{$extension} files were produced codegen_ssa_ignoring_output = ignoring -o because multiple .{$extension} files were produced diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index a5bd3adbcddc9..31f8d479f7eb8 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -20,8 +20,8 @@ use rustc_span::symbol::Ident; use rustc_span::{Span, sym}; use rustc_target::spec::{SanitizerSet, abi}; -use crate::errors::{self, MissingFeatures, TargetFeatureDisableOrEnable}; -use crate::target_features::{check_target_feature_trait_unsafe, from_target_feature}; +use crate::errors; +use crate::target_features::{check_target_feature_trait_unsafe, from_target_feature_attr}; fn linkage_by_name(tcx: TyCtxt<'_>, def_id: LocalDefId, name: &str) -> Linkage { use rustc_middle::mir::mono::Linkage::*; @@ -73,7 +73,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_BUILTINS; } - let supported_target_features = tcx.supported_target_features(LOCAL_CRATE); + let rust_target_features = tcx.rust_target_features(LOCAL_CRATE); let mut inline_span = None; let mut link_ordinal_span = None; @@ -281,10 +281,10 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { check_target_feature_trait_unsafe(tcx, did, attr.span); } } - from_target_feature( + from_target_feature_attr( tcx, attr, - supported_target_features, + rust_target_features, &mut codegen_fn_attrs.target_features, ); } @@ -676,10 +676,10 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { .next() .map_or_else(|| tcx.def_span(did), |a| a.span); tcx.dcx() - .create_err(TargetFeatureDisableOrEnable { + .create_err(errors::TargetFeatureDisableOrEnable { features, span: Some(span), - missing_features: Some(MissingFeatures), + missing_features: Some(errors::MissingFeatures), }) .emit(); } diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index d67cf0e3a6d5f..670ad39d811e6 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -1018,6 +1018,15 @@ pub(crate) struct TargetFeatureSafeTrait { pub def: Span, } +#[derive(Diagnostic)] +#[diag(codegen_ssa_forbidden_target_feature_attr)] +pub struct ForbiddenTargetFeatureAttr<'a> { + #[primary_span] + pub span: Span, + pub feature: &'a str, + pub reason: &'a str, +} + #[derive(Diagnostic)] #[diag(codegen_ssa_failed_to_get_layout)] pub struct FailedToGetLayout<'tcx> { diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs index 0845bcc5749f4..eee7cc75400d9 100644 --- a/compiler/rustc_codegen_ssa/src/target_features.rs +++ b/compiler/rustc_codegen_ssa/src/target_features.rs @@ -11,13 +11,16 @@ use rustc_middle::ty::TyCtxt; use rustc_session::parse::feature_err; use rustc_span::Span; use rustc_span::symbol::{Symbol, sym}; +use rustc_target::target_features::{self, Stability}; use crate::errors; -pub(crate) fn from_target_feature( +/// Compute the enabled target features from the `#[target_feature]` function attribute. +/// Enabled target features are added to `target_features`. +pub(crate) fn from_target_feature_attr( tcx: TyCtxt<'_>, attr: &ast::Attribute, - supported_target_features: &UnordMap>, + rust_target_features: &UnordMap, target_features: &mut Vec, ) { let Some(list) = attr.meta_item_list() else { return }; @@ -46,12 +49,12 @@ pub(crate) fn from_target_feature( // We allow comma separation to enable multiple features. added_target_features.extend(value.as_str().split(',').filter_map(|feature| { - let Some(feature_gate) = supported_target_features.get(feature) else { + let Some(stability) = rust_target_features.get(feature) else { let msg = format!("the feature named `{feature}` is not valid for this target"); let mut err = tcx.dcx().struct_span_err(item.span(), msg); err.span_label(item.span(), format!("`{feature}` is not valid for this target")); if let Some(stripped) = feature.strip_prefix('+') { - let valid = supported_target_features.contains_key(stripped); + let valid = rust_target_features.contains_key(stripped); if valid { err.help("consider removing the leading `+` in the feature name"); } @@ -61,18 +64,31 @@ pub(crate) fn from_target_feature( }; // Only allow target features whose feature gates have been enabled. - let allowed = match feature_gate.as_ref().copied() { - Some(name) => rust_features.enabled(name), - None => true, + let allowed = match stability { + Stability::Forbidden { .. } => false, + Stability::Stable => true, + Stability::Unstable(name) => rust_features.enabled(*name), }; if !allowed { - feature_err( - &tcx.sess, - feature_gate.unwrap(), - item.span(), - format!("the target feature `{feature}` is currently unstable"), - ) - .emit(); + match stability { + Stability::Stable => unreachable!(), + &Stability::Unstable(lang_feature_name) => { + feature_err( + &tcx.sess, + lang_feature_name, + item.span(), + format!("the target feature `{feature}` is currently unstable"), + ) + .emit(); + } + Stability::Forbidden { reason } => { + tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr { + span: item.span(), + feature, + reason, + }); + } + } } Some(Symbol::intern(feature)) })); @@ -138,20 +154,20 @@ pub(crate) fn check_target_feature_trait_unsafe(tcx: TyCtxt<'_>, id: LocalDefId, pub(crate) fn provide(providers: &mut Providers) { *providers = Providers { - supported_target_features: |tcx, cnum| { + rust_target_features: |tcx, cnum| { assert_eq!(cnum, LOCAL_CRATE); if tcx.sess.opts.actually_rustdoc { // rustdoc needs to be able to document functions that use all the features, so // whitelist them all - rustc_target::target_features::all_known_features() - .map(|(a, b)| (a.to_string(), b.as_feature_name())) + rustc_target::target_features::all_rust_features() + .map(|(a, b)| (a.to_string(), b)) .collect() } else { tcx.sess .target - .supported_target_features() + .rust_target_features() .iter() - .map(|&(a, b, _)| (a.to_string(), b.as_feature_name())) + .map(|&(a, b, _)| (a.to_string(), b)) .collect() } }, diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index d7a60a843b717..0f7f727212cf0 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -2185,10 +2185,11 @@ rustc_queries! { desc { "computing autoderef types for `{}`", goal.canonical.value.value } } - query supported_target_features(_: CrateNum) -> &'tcx UnordMap> { + /// Returns the Rust target features for the current target. These are not always the same as LLVM target features! + query rust_target_features(_: CrateNum) -> &'tcx UnordMap { arena_cache eval_always - desc { "looking up supported target features" } + desc { "looking up Rust target features" } } query implied_target_features(feature: Symbol) -> &'tcx Vec { diff --git a/compiler/rustc_session/src/config/cfg.rs b/compiler/rustc_session/src/config/cfg.rs index 31ef2bda4f1e5..f30da4fbfc64b 100644 --- a/compiler/rustc_session/src/config/cfg.rs +++ b/compiler/rustc_session/src/config/cfg.rs @@ -370,8 +370,9 @@ impl CheckCfg { ins!(sym::sanitizer_cfi_normalize_integers, no_values); ins!(sym::target_feature, empty_values).extend( - rustc_target::target_features::all_known_features() - .map(|(f, _sb)| f) + rustc_target::target_features::all_rust_features() + .filter(|(_, s)| s.is_supported()) + .map(|(f, _s)| f) .chain(rustc_target::target_features::RUSTC_SPECIFIC_FEATURES.iter().cloned()) .map(Symbol::intern), ); diff --git a/compiler/rustc_target/src/target_features.rs b/compiler/rustc_target/src/target_features.rs index 3df8f0590a354..eec07a8c3517e 100644 --- a/compiler/rustc_target/src/target_features.rs +++ b/compiler/rustc_target/src/target_features.rs @@ -1,10 +1,17 @@ +//! Declares Rust's target feature names for each target. +//! Note that these are similar to but not always identical to LLVM's feature names, +//! and Rust adds some features that do not correspond to LLVM features at all. use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_span::symbol::{Symbol, sym}; /// Features that control behaviour of rustc, rather than the codegen. +/// These exist globally and are not in the target-specific lists below. pub const RUSTC_SPECIFIC_FEATURES: &[&str] = &["crt-static"]; -/// Features that require special handling when passing to LLVM. +/// Features that require special handling when passing to LLVM: +/// these are target-specific (i.e., must also be listed in the target-specific list below) +/// but do not correspond to an LLVM target feature. pub const RUSTC_SPECIAL_FEATURES: &[&str] = &["backchain"]; /// Stability information for target features. @@ -16,26 +23,47 @@ pub enum Stability { /// This target feature is unstable; using it in `#[target_feature]` or `#[cfg(target_feature)]` /// requires enabling the given nightly feature. Unstable(Symbol), + /// This feature can not be set via `-Ctarget-feature` or `#[target_feature]`, it can only be set in the basic + /// target definition. Used in particular for features that change the floating-point ABI. + Forbidden { reason: &'static str }, } use Stability::*; -impl Stability { - pub fn as_feature_name(self) -> Option { +impl HashStable for Stability { + #[inline] + fn hash_stable(&self, hcx: &mut CTX, hasher: &mut StableHasher) { + std::mem::discriminant(self).hash_stable(hcx, hasher); match self { - Stable => None, - Unstable(s) => Some(s), + Stable => {} + Unstable(sym) => { + sym.hash_stable(hcx, hasher); + } + Forbidden { .. } => {} } } +} +impl Stability { pub fn is_stable(self) -> bool { matches!(self, Stable) } + + /// Forbidden features are not supported. + pub fn is_supported(self) -> bool { + !matches!(self, Forbidden { .. }) + } } // Here we list target features that rustc "understands": they can be used in `#[target_feature]` // and `#[cfg(target_feature)]`. They also do not trigger any warnings when used with // `-Ctarget-feature`. // +// Note that even unstable (and even entirely unlisted) features can be used with `-Ctarget-feature` +// on stable. Using a feature not on the list of Rust target features only emits a warning. +// Only `cfg(target_feature)` and `#[target_feature]` actually do any stability gating. +// `cfg(target_feature)` for unstable features just works on nightly without any feature gate. +// `#[target_feature]` requires a feature gate. +// // When adding features to the below lists // check whether they're named already elsewhere in rust // e.g. in stdarch and whether the given name matches LLVM's @@ -46,17 +74,27 @@ impl Stability { // per-function level, since we would then allow safe calls from functions with `+soft-float` to // functions without that feature! // -// When adding a new feature, be particularly mindful of features that affect function ABIs. Those -// need to be treated very carefully to avoid introducing unsoundness! This often affects features -// that enable/disable hardfloat support (see https://github.com/rust-lang/rust/issues/116344 for an -// example of this going wrong), but features enabling new SIMD registers are also a concern (see -// https://github.com/rust-lang/rust/issues/116558 for an example of this going wrong). +// It is important for soundness that features allowed here do *not* change the function call ABI. +// For example, disabling the `x87` feature on x86 changes how scalar floats are passed as +// arguments, so enabling toggling that feature would be unsound. In fact, since `-Ctarget-feature` +// will just allow unknown features (with a warning), we have to explicitly list features that change +// the ABI as `Forbidden` to ensure using them causes an error. Note that this is only effective if +// such features can never be toggled via `-Ctarget-cpu`! If that is ever a possibility, we will need +// extra checks ensuring that the LLVM-computed target features for a CPU did not (un)set a +// `Forbidden` feature. See https://github.com/rust-lang/rust/issues/116344 for some more context. +// FIXME: add such "forbidden" features for non-x86 targets. +// +// The one exception to features that change the ABI is features that enable larger vector +// registers. Those are permitted to be listed here. This is currently unsound (see +// https://github.com/rust-lang/rust/issues/116558); in the future we will have to ensure that +// functions can only use such vectors as arguments/return types if the corresponding target feature +// is enabled. // // Stabilizing a target feature requires t-lang approval. type ImpliedFeatures = &'static [&'static str]; -const ARM_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ +const ARM_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-start ("aclass", Unstable(sym::arm_target_feature), &[]), ("aes", Unstable(sym::arm_target_feature), &["neon"]), @@ -70,6 +108,7 @@ const ARM_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ ("neon", Unstable(sym::arm_target_feature), &["vfp3"]), ("rclass", Unstable(sym::arm_target_feature), &[]), ("sha2", Unstable(sym::arm_target_feature), &["neon"]), + ("soft-float", Forbidden { reason: "unsound because it changes float ABI" }, &[]), // This is needed for inline assembly, but shouldn't be stabilized as-is // since it should be enabled per-function using #[instruction_set], not // #[target_feature]. @@ -87,9 +126,10 @@ const ARM_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ ("vfp4", Unstable(sym::arm_target_feature), &["vfp3"]), ("virtualization", Unstable(sym::arm_target_feature), &[]), // tidy-alphabetical-end + // FIXME: need to also forbid turning off `fpregs` on hardfloat targets ]; -const AARCH64_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ +const AARCH64_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-start // FEAT_AES & FEAT_PMULL ("aes", Stable, &["neon"]), @@ -277,7 +317,7 @@ const AARCH64_TIED_FEATURES: &[&[&str]] = &[ &["paca", "pacg"], // Together these represent `pauth` in LLVM ]; -const X86_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ +const X86_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-start ("adx", Stable, &[]), ("aes", Stable, &["sse2"]), @@ -328,6 +368,7 @@ const X86_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ ("sha512", Unstable(sym::sha512_sm_x86), &["avx2"]), ("sm3", Unstable(sym::sha512_sm_x86), &["avx"]), ("sm4", Unstable(sym::sha512_sm_x86), &["avx2"]), + ("soft-float", Forbidden { reason: "unsound because it changes float ABI" }, &[]), ("sse", Stable, &[]), ("sse2", Stable, &["sse"]), ("sse3", Stable, &["sse2"]), @@ -344,16 +385,17 @@ const X86_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ ("xsaveopt", Stable, &["xsave"]), ("xsaves", Stable, &["xsave"]), // tidy-alphabetical-end + // FIXME: need to also forbid turning off `x87` on hardfloat targets ]; -const HEXAGON_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ +const HEXAGON_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-start ("hvx", Unstable(sym::hexagon_target_feature), &[]), ("hvx-length128b", Unstable(sym::hexagon_target_feature), &["hvx"]), // tidy-alphabetical-end ]; -const POWERPC_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ +const POWERPC_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-start ("altivec", Unstable(sym::powerpc_target_feature), &[]), ("partword-atomics", Unstable(sym::powerpc_target_feature), &[]), @@ -367,7 +409,7 @@ const POWERPC_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-end ]; -const MIPS_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ +const MIPS_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-start ("fp64", Unstable(sym::mips_target_feature), &[]), ("msa", Unstable(sym::mips_target_feature), &[]), @@ -375,7 +417,7 @@ const MIPS_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-end ]; -const RISCV_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ +const RISCV_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-start ("a", Stable, &["zaamo", "zalrsc"]), ("c", Stable, &[]), @@ -415,7 +457,7 @@ const RISCV_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-end ]; -const WASM_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ +const WASM_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-start ("atomics", Unstable(sym::wasm_target_feature), &[]), ("bulk-memory", Stable, &[]), @@ -431,10 +473,10 @@ const WASM_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-end ]; -const BPF_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = +const BPF_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[("alu32", Unstable(sym::bpf_target_feature), &[])]; -const CSKY_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ +const CSKY_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-start ("10e60", Unstable(sym::csky_target_feature), &["7e10"]), ("2e3", Unstable(sym::csky_target_feature), &["e2"]), @@ -481,7 +523,7 @@ const CSKY_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-end ]; -const LOONGARCH_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ +const LOONGARCH_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-start ("d", Unstable(sym::loongarch_target_feature), &["f"]), ("f", Unstable(sym::loongarch_target_feature), &[]), @@ -495,7 +537,7 @@ const LOONGARCH_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-end ]; -const IBMZ_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ +const IBMZ_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-start ("backchain", Unstable(sym::s390x_target_feature), &[]), ("vector", Unstable(sym::s390x_target_feature), &[]), @@ -506,41 +548,39 @@ const IBMZ_ALLOWED_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ /// primitives may be documented. /// /// IMPORTANT: If you're adding another feature list above, make sure to add it to this iterator! -pub fn all_known_features() -> impl Iterator { +pub fn all_rust_features() -> impl Iterator { std::iter::empty() - .chain(ARM_ALLOWED_FEATURES.iter()) - .chain(AARCH64_ALLOWED_FEATURES.iter()) - .chain(X86_ALLOWED_FEATURES.iter()) - .chain(HEXAGON_ALLOWED_FEATURES.iter()) - .chain(POWERPC_ALLOWED_FEATURES.iter()) - .chain(MIPS_ALLOWED_FEATURES.iter()) - .chain(RISCV_ALLOWED_FEATURES.iter()) - .chain(WASM_ALLOWED_FEATURES.iter()) - .chain(BPF_ALLOWED_FEATURES.iter()) - .chain(CSKY_ALLOWED_FEATURES) - .chain(LOONGARCH_ALLOWED_FEATURES) - .chain(IBMZ_ALLOWED_FEATURES) + .chain(ARM_FEATURES.iter()) + .chain(AARCH64_FEATURES.iter()) + .chain(X86_FEATURES.iter()) + .chain(HEXAGON_FEATURES.iter()) + .chain(POWERPC_FEATURES.iter()) + .chain(MIPS_FEATURES.iter()) + .chain(RISCV_FEATURES.iter()) + .chain(WASM_FEATURES.iter()) + .chain(BPF_FEATURES.iter()) + .chain(CSKY_FEATURES) + .chain(LOONGARCH_FEATURES) + .chain(IBMZ_FEATURES) .cloned() .map(|(f, s, _)| (f, s)) } impl super::spec::Target { - pub fn supported_target_features( - &self, - ) -> &'static [(&'static str, Stability, ImpliedFeatures)] { + pub fn rust_target_features(&self) -> &'static [(&'static str, Stability, ImpliedFeatures)] { match &*self.arch { - "arm" => ARM_ALLOWED_FEATURES, - "aarch64" | "arm64ec" => AARCH64_ALLOWED_FEATURES, - "x86" | "x86_64" => X86_ALLOWED_FEATURES, - "hexagon" => HEXAGON_ALLOWED_FEATURES, - "mips" | "mips32r6" | "mips64" | "mips64r6" => MIPS_ALLOWED_FEATURES, - "powerpc" | "powerpc64" => POWERPC_ALLOWED_FEATURES, - "riscv32" | "riscv64" => RISCV_ALLOWED_FEATURES, - "wasm32" | "wasm64" => WASM_ALLOWED_FEATURES, - "bpf" => BPF_ALLOWED_FEATURES, - "csky" => CSKY_ALLOWED_FEATURES, - "loongarch64" => LOONGARCH_ALLOWED_FEATURES, - "s390x" => IBMZ_ALLOWED_FEATURES, + "arm" => ARM_FEATURES, + "aarch64" | "arm64ec" => AARCH64_FEATURES, + "x86" | "x86_64" => X86_FEATURES, + "hexagon" => HEXAGON_FEATURES, + "mips" | "mips32r6" | "mips64" | "mips64r6" => MIPS_FEATURES, + "powerpc" | "powerpc64" => POWERPC_FEATURES, + "riscv32" | "riscv64" => RISCV_FEATURES, + "wasm32" | "wasm64" => WASM_FEATURES, + "bpf" => BPF_FEATURES, + "csky" => CSKY_FEATURES, + "loongarch64" => LOONGARCH_FEATURES, + "s390x" => IBMZ_FEATURES, _ => &[], } } @@ -557,7 +597,7 @@ impl super::spec::Target { base_features: impl Iterator, ) -> FxHashSet { let implied_features = self - .supported_target_features() + .rust_target_features() .iter() .map(|(f, _, i)| (Symbol::intern(f), i)) .collect::>(); diff --git a/tests/ui/auxiliary/using-target-feature-unstable.rs b/tests/ui/target-feature/auxiliary/using-target-feature-unstable.rs similarity index 100% rename from tests/ui/auxiliary/using-target-feature-unstable.rs rename to tests/ui/target-feature/auxiliary/using-target-feature-unstable.rs diff --git a/tests/ui/target-feature/forbidden-target-feature-attribute.rs b/tests/ui/target-feature/forbidden-target-feature-attribute.rs new file mode 100644 index 0000000000000..91c56b43689e1 --- /dev/null +++ b/tests/ui/target-feature/forbidden-target-feature-attribute.rs @@ -0,0 +1,12 @@ +//@ compile-flags: --target=x86_64-unknown-linux-gnu --crate-type=lib +//@ needs-llvm-components: x86 +#![feature(no_core, lang_items)] +#![no_std] +#![no_core] + +#[lang = "sized"] +pub trait Sized {} + +#[target_feature(enable = "soft-float")] +//~^ERROR: cannot be toggled with +pub unsafe fn my_fun() {} diff --git a/tests/ui/target-feature/forbidden-target-feature-attribute.stderr b/tests/ui/target-feature/forbidden-target-feature-attribute.stderr new file mode 100644 index 0000000000000..fb318531f7edc --- /dev/null +++ b/tests/ui/target-feature/forbidden-target-feature-attribute.stderr @@ -0,0 +1,8 @@ +error: target feature `soft-float` cannot be toggled with `#[target_feature]`: unsound because it changes float ABI + --> $DIR/forbidden-target-feature-attribute.rs:10:18 + | +LL | #[target_feature(enable = "soft-float")] + | ^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui/target-feature/forbidden-target-feature-cfg.rs b/tests/ui/target-feature/forbidden-target-feature-cfg.rs new file mode 100644 index 0000000000000..5df26e26793d9 --- /dev/null +++ b/tests/ui/target-feature/forbidden-target-feature-cfg.rs @@ -0,0 +1,15 @@ +//@ compile-flags: --target=x86_64-unknown-none --crate-type=lib +//@ needs-llvm-components: x86 +//@ check-pass +#![feature(no_core, lang_items)] +#![no_std] +#![no_core] +#![allow(unexpected_cfgs)] + +#[lang = "sized"] +pub trait Sized {} + +// The compile_error macro does not exist, so if the `cfg` evaluates to `true` this +// complains about the missing macro rather than showing the error... but that's good enough. +#[cfg(target_feature = "soft-float")] +compile_error!("the soft-float feature should not be exposed in `cfg`"); diff --git a/tests/ui/target-feature/forbidden-target-feature-flag-disable.rs b/tests/ui/target-feature/forbidden-target-feature-flag-disable.rs new file mode 100644 index 0000000000000..b27e8a10afee7 --- /dev/null +++ b/tests/ui/target-feature/forbidden-target-feature-flag-disable.rs @@ -0,0 +1,11 @@ +//@ compile-flags: --target=x86_64-unknown-linux-gnu --crate-type=lib +//@ needs-llvm-components: x86 +//@ compile-flags: -Ctarget-feature=-soft-float +// For now this is just a warning. +//@ build-pass +#![feature(no_core, lang_items)] +#![no_std] +#![no_core] + +#[lang = "sized"] +pub trait Sized {} diff --git a/tests/ui/target-feature/forbidden-target-feature-flag-disable.stderr b/tests/ui/target-feature/forbidden-target-feature-flag-disable.stderr new file mode 100644 index 0000000000000..508e1fe0cf476 --- /dev/null +++ b/tests/ui/target-feature/forbidden-target-feature-flag-disable.stderr @@ -0,0 +1,7 @@ +warning: target feature `soft-float` cannot be toggled with `-Ctarget-feature`: unsound because it changes float ABI + | + = note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #116344 + +warning: 1 warning emitted + diff --git a/tests/ui/target-feature/forbidden-target-feature-flag.rs b/tests/ui/target-feature/forbidden-target-feature-flag.rs new file mode 100644 index 0000000000000..93cebc6b53690 --- /dev/null +++ b/tests/ui/target-feature/forbidden-target-feature-flag.rs @@ -0,0 +1,11 @@ +//@ compile-flags: --target=x86_64-unknown-linux-gnu --crate-type=lib +//@ needs-llvm-components: x86 +//@ compile-flags: -Ctarget-feature=+soft-float +// For now this is just a warning. +//@ build-pass +#![feature(no_core, lang_items)] +#![no_std] +#![no_core] + +#[lang = "sized"] +pub trait Sized {} diff --git a/tests/ui/target-feature/forbidden-target-feature-flag.stderr b/tests/ui/target-feature/forbidden-target-feature-flag.stderr new file mode 100644 index 0000000000000..508e1fe0cf476 --- /dev/null +++ b/tests/ui/target-feature/forbidden-target-feature-flag.stderr @@ -0,0 +1,7 @@ +warning: target feature `soft-float` cannot be toggled with `-Ctarget-feature`: unsound because it changes float ABI + | + = note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #116344 + +warning: 1 warning emitted + diff --git a/tests/ui/using-target-feature-unstable.rs b/tests/ui/target-feature/using-target-feature-unstable.rs similarity index 100% rename from tests/ui/using-target-feature-unstable.rs rename to tests/ui/target-feature/using-target-feature-unstable.rs