Auto merge of rust-lang#129884 - RalfJung:forbidden-target-features, …

…r=workingjubilee

mark some target features as 'forbidden' so they cannot be (un)set with -Ctarget-feature

The context for this is rust-lang#116344: some target features change the way floats are passed between functions. Changing those target features is unsound as code compiled for the same target may now use different ABIs.

So this introduces a new concept of "forbidden" target features (on top of the existing "stable " and "unstable" categories), and makes it a hard error to (un)set such a target feature. For now, the x86 and ARM feature `soft-float` is on that list. We'll have to make some effort to collect more relevant features, and similar features from other targets, but that can happen after the basic infrastructure for this landed. (These features are being collected in rust-lang#131799.)

I've made this a warning for now to give people some time to speak up if this would break something.

MCP: rust-lang/compiler-team#780

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
 

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)]

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<Stri
     );
 
     // -Ctarget-features
-    let supported_features = sess.target.supported_target_features();
+    let known_features = sess.target.rust_target_features();
     let mut featsmap = FxHashMap::default();
     let feats = sess
         .opts
@@ -62,37 +65,49 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
                 }
             };
 
+            // Get the backend feature name, if any.
+            // This excludes rustc-specific features, that do not get passed down to GCC.
             let feature = backend_feature_name(s)?;
             // Warn against use of GCC specific feature names on the CLI.
-            if diagnostics && !supported_features.iter().any(|&(v, _, _)| v == feature) {
-                let rust_feature = supported_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
+            if diagnostics {
+                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 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

compiler/rustc_codegen_gcc/src/lib.rs

@@ -491,8 +491,9 @@ pub fn target_features(
 ) -> Vec<Symbol> {
     // 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)

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 <https://github.com/rust-lang/rust/issues/116344>
+
 codegen_llvm_from_llvm_diag = {$message}
 
 codegen_llvm_from_llvm_optimization_diag = {$filename}:{$line}:{$column} {$pass_name} ({$kind}): {$message}

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)]

compiler/rustc_codegen_llvm/src/llvm_util.rs

@@ -17,12 +17,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;
 
@@ -281,19 +281,29 @@ pub(crate) fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> Option<LLVMFea
     }
 }
 
-/// Used to generate cfg variables and apply features
-/// Must express features in the way Rust understands them
+/// Used to generate cfg variables and apply features.
+/// Must express features in the way Rust understands them.
+///
+/// We do not have to worry about RUSTC_SPECIFIC_FEATURES here, those are handled outside codegen.
 pub fn target_features(sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
-    let mut features = vec![];
-
-    // Add base features for the target
+    let mut features: FxHashSet<Symbol> = 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;
                 }
@@ -324,16 +334,22 @@ pub fn target_features(sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
         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)
             });
         }
     }
 
     // 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)
@@ -451,9 +467,13 @@ fn print_target_features(sess: &Session, tm: &llvm::TargetMachine, out: &mut Str
     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;
@@ -605,7 +625,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
@@ -639,50 +659,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.

compiler/rustc_codegen_ssa/messages.ftl

@@ -64,6 +64,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