Auto merge of rust-lang#117616 - RalfJung:unstable-target-features, r=compiler-errors

warn when using an unstable feature with -Ctarget-feature

Setting or unsetting the wrong target features can cause ABI incompatibility (rust-lang#116344, rust-lang#116558). We need to carefully audit features for their ABI impact before stabilization. I just learned that we currently accept arbitrary unstable features on stable and if they are in the list of Rust target features, even unstable, then we don't even warn about that!1 That doesn't seem great, so I propose we introduce a warning here.

This has an obvious loophole via `-Ctarget-cpu`. I'm not sure how to best deal with that, but it seems better to fix what we can and think about the other cases later, maybe once we have a better idea for how to resolve the general mess that are ABI-affecting target features.

Author: bors

compiler/rustc_codegen_llvm/messages.ftl

@@ -76,8 +76,8 @@ codegen_llvm_target_machine = could not create LLVM TargetMachine for triple: {$
 codegen_llvm_target_machine_with_llvm_err = could not create LLVM TargetMachine for triple: {$triple}: {$llvm_err}
 
 codegen_llvm_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
 
@@ -87,6 +87,10 @@ codegen_llvm_unknown_ctarget_feature_prefix =
 
 codegen_llvm_unknown_debuginfo_compression = unknown debuginfo compression algorithm {$algorithm} - will fall back to uncompressed debuginfo
 
+codegen_llvm_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_llvm_write_bytecode = failed to write bytecode to {$path}: {$err}
 
 codegen_llvm_write_ir = failed to write LLVM IR to {$path}

compiler/rustc_codegen_llvm/src/errors.rs

@@ -26,6 +26,13 @@ pub(crate) struct UnknownCTargetFeature<'a> {
     pub rust_feature: PossibleFeature<'a>,
 }
 
+#[derive(Diagnostic)]
+#[diag(codegen_llvm_unstable_ctarget_feature)]
+#[note]
+pub(crate) struct UnstableCTargetFeature<'a> {
+    pub feature: &'a str,
+}
+
 #[derive(Subdiagnostic)]
 pub(crate) enum PossibleFeature<'a> {
     #[help(codegen_llvm_possible_feature)]

compiler/rustc_codegen_llvm/src/llvm_util.rs

@@ -1,7 +1,7 @@
 use crate::back::write::create_informational_target_machine;
 use crate::errors::{
     PossibleFeature, TargetFeatureDisableOrEnable, UnknownCTargetFeature,
-    UnknownCTargetFeaturePrefix,
+    UnknownCTargetFeaturePrefix, UnstableCTargetFeature,
 };
 use crate::llvm;
 use libc::c_int;
@@ -531,25 +531,34 @@ pub(crate) fn global_llvm_features(sess: &Session, diagnostics: bool) -> Vec<Str
             };
 
             let feature = backend_feature_name(s)?;
-            // Warn against use of LLVM 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 llvm_features = to_llvm_features(sess, rust_feature);
-                    if llvm_features.contains(&feature) && !llvm_features.contains(&rust_feature) {
-                        Some(rust_feature)
+            // 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 unknown_feature = if let Some(rust_feature) = rust_feature {
+                        UnknownCTargetFeature {
+                            feature,
+                            rust_feature: PossibleFeature::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.emit_warning(unknown_feature);
+                        UnknownCTargetFeature { feature, rust_feature: PossibleFeature::None }
+                    };
+                    sess.emit_warning(unknown_feature);
+                } else if feature_state.is_some_and(|(_name, feature_gate)| feature_gate.is_some())
+                {
+                    // An unstable feature. Warn about using it.
+                    sess.emit_warning(UnstableCTargetFeature { feature });
+                }
             }
 
             if diagnostics {

compiler/rustc_codegen_ssa/src/target_features.rs

@@ -23,6 +23,15 @@ pub const RUSTC_SPECIFIC_FEATURES: &[&str] = &["crt-static"];
 // check whether they're named already elsewhere in rust
 // e.g. in stdarch and whether the given name matches LLVM's
 // if it doesn't, to_llvm_feature in llvm_util in rustc_codegen_llvm needs to be adapted
+//
+// 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).
+//
+// Stabilizing a target feature (setting the 2nd component of the pair to `None`) requires t-lang
+// approval.
 
 const ARM_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[
     // tidy-alphabetical-start

tests/ui/target-feature/similar-feature-suggestion.stderr

@@ -1,6 +1,6 @@
-warning: unknown feature specified for `-Ctarget-feature`: `rdrnd`
+warning: unknown and unstable feature specified for `-Ctarget-feature`: `rdrnd`
    |
-   = note: it is still passed through to the codegen backend
+   = 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
    = help: you might have meant: `rdrand`
 
 warning: 1 warning emitted

tests/ui/target-feature/unstable-feature.rs

@@ -0,0 +1,6 @@
+// compile-flags: -Ctarget-feature=+vaes --crate-type=rlib --target=x86_64-unknown-linux-gnu
+// build-pass
+// needs-llvm-components: x86
+
+#![feature(no_core)]
+#![no_core]

tests/ui/target-feature/unstable-feature.stderr

@@ -0,0 +1,6 @@
+warning: unstable feature specified for `-Ctarget-feature`: `vaes`
+   |
+   = note: this feature is not stably supported; its behavior can change in the future
+
+warning: 1 warning emitted
+