Auto merge of #135160 - RalfJung:aarch64-softfloat-not-neon, r=Noratrieb

aarch64-softfloat: forbid enabling the neon target feature

This fixes https://github.com/rust-lang/rust/issues/134375 in a rather crude way, by making [the example](https://godbolt.org/z/r56xWo8nT) not build any more on aarch64-unknown-none-softfloat. That is a breaking change since the "neon" aarch64 target feature is stable, but this is justified as a soundness fix. Note that it's not "neon" which is problematic but "fp-armv8"; however, the two are tied together by rustc.

`-Ctarget-feature=+neon` still works, it just causes a warning (but one that we do hope to make a hard error eventually). Only `#[target_feature="neon"]` becomes a hard error with this PR.

More work on the LLVM side will be needed before we can let people use neon without impacting the ABI of float values (and, in particular, the ABI used by automatically inserted calls to libm functions, e.g. for int-to-float casts, which rustc has no control over).

Nominating for `@rust-lang/lang` since it is a breaking change. As-is this PR doesn't have a warning cycle; the hope is that the aarch64-unknown-none-softfloat target is sufficiently niche that there's no huge fallout and we can easily revert if it causes trouble. A warning cycle could be added but would need some dedicated rather hacky check in the target_feature attribute handling logic.

try-job:  dist-various-1

8 files changed, 127 insertions, 18 deletions

diff --git a/compiler/rustc_codegen_ssa/messages.ftl b/compiler/rustc_codegen_ssa/messages.ftl
index 2621935eecf..acb4cbaa13f 100644
--- a/compiler/rustc_codegen_ssa/messages.ftl
+++ b/compiler/rustc_codegen_ssa/messages.ftl
@@ -1,5 +1,7 @@
 codegen_ssa_L4Bender_exporting_symbols_unimplemented = exporting symbols not implemented yet for L4Bender
 
+codegen_ssa_aarch64_softfloat_neon = enabling the `neon` target feature on the current target is unsound due to ABI issues
+
 codegen_ssa_add_native_library = failed to add native library {$library_path}: {$error}
 
 codegen_ssa_aix_strip_not_used = using host's `strip` binary to cross-compile to AIX which is not guaranteed to work
diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs
index aa55a0e0f14..b3bda784d43 100644
--- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs
+++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs
@@ -299,6 +299,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
                 }
                 from_target_feature_attr(
                     tcx,
+                    did,
                     attr,
                     rust_target_features,
                     &mut codegen_fn_attrs.target_features,
diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs
index d49aac75d05..572d7b1e06a 100644
--- a/compiler/rustc_codegen_ssa/src/errors.rs
+++ b/compiler/rustc_codegen_ssa/src/errors.rs
@@ -1316,3 +1316,7 @@ pub(crate) struct XcrunSdkPathWarning {
     pub sdk_name: &'static str,
     pub stderr: String,
 }
+
+#[derive(LintDiagnostic)]
+#[diag(codegen_ssa_aarch64_softfloat_neon)]
+pub(crate) struct Aarch64SoftfloatNeon;
diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs
index 3ecea522837..6bb3150c1c5 100644
--- a/compiler/rustc_codegen_ssa/src/target_features.rs
+++ b/compiler/rustc_codegen_ssa/src/target_features.rs
@@ -8,6 +8,7 @@ use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId};
 use rustc_middle::middle::codegen_fn_attrs::TargetFeature;
 use rustc_middle::query::Providers;
 use rustc_middle::ty::TyCtxt;
+use rustc_session::lint::builtin::AARCH64_SOFTFLOAT_NEON;
 use rustc_session::parse::feature_err;
 use rustc_span::{Span, Symbol, sym};
 use rustc_target::target_features::{self, Stability};
@@ -18,6 +19,7 @@ use crate::errors;
 /// Enabled target features are added to `target_features`.
 pub(crate) fn from_target_feature_attr(
     tcx: TyCtxt<'_>,
+    did: LocalDefId,
     attr: &hir::Attribute,
     rust_target_features: &UnordMap<String, target_features::Stability>,
     target_features: &mut Vec<TargetFeature>,
@@ -92,11 +94,22 @@ pub(crate) fn from_target_feature_attr(
                     // generating code so "it's fine".
                     if !tcx.sess.opts.actually_rustdoc {
                         if abi_feature_constraints.incompatible.contains(&name.as_str()) {
-                            tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr {
-                                span: item.span(),
-                                feature: name.as_str(),
-                                reason: "this feature is incompatible with the target ABI",
-                            });
+                            // For "neon" specifically, we emit an FCW instead of a hard error.
+                            // See <https://github.com/rust-lang/rust/issues/134375>.
+                            if tcx.sess.target.arch == "aarch64" && name.as_str() == "neon" {
+                                tcx.emit_node_span_lint(
+                                    AARCH64_SOFTFLOAT_NEON,
+                                    tcx.local_def_id_to_hir_id(did),
+                                    item.span(),
+                                    errors::Aarch64SoftfloatNeon,
+                                );
+                            } else {
+                                tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr {
+                                    span: item.span(),
+                                    feature: name.as_str(),
+                                    reason: "this feature is incompatible with the target ABI",
+                                });
+                            }
                         }
                     }
                     target_features.push(TargetFeature { name, implied: name != feature_sym })
diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs
index 3cea24634fe..b8d242bad86 100644
--- a/compiler/rustc_lint_defs/src/builtin.rs
+++ b/compiler/rustc_lint_defs/src/builtin.rs
@@ -16,6 +16,7 @@ declare_lint_pass! {
     /// that are used by other parts of the compiler.
     HardwiredLints => [
         // tidy-alphabetical-start
+        AARCH64_SOFTFLOAT_NEON,
         ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE,
         AMBIGUOUS_ASSOCIATED_ITEMS,
         AMBIGUOUS_GLOB_IMPORTS,
@@ -5043,14 +5044,14 @@ declare_lint! {
     ///
     /// ```text
     /// error: this function function definition is affected by the wasm ABI transition: it passes an argument of non-scalar type `MyType`
-    /// --> $DIR/wasm_c_abi_transition.rs:17:1
-    ///  |
-    ///  | pub extern "C" fn my_fun(_x: MyType) {}
-    ///  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-    ///  |
-    ///  = warning: 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 #138762 <https://github.com/rust-lang/rust/issues/138762>
-    ///  = help: the "C" ABI Rust uses on wasm32-unknown-unknown will change to align with the standard "C" ABI for this target
+    ///   --> $DIR/wasm_c_abi_transition.rs:17:1
+    ///    |
+    ///    | pub extern "C" fn my_fun(_x: MyType) {}
+    ///    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    ///    |
+    ///    = warning: 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 #138762 <https://github.com/rust-lang/rust/issues/138762>
+    ///    = help: the "C" ABI Rust uses on wasm32-unknown-unknown will change to align with the standard "C" ABI for this target
     /// ```
     ///
     /// ### Explanation
@@ -5067,3 +5068,44 @@ declare_lint! {
         reference: "issue #138762 <https://github.com/rust-lang/rust/issues/138762>",
     };
 }
+
+declare_lint! {
+    /// The `aarch64_softfloat_neon` lint detects usage of `#[target_feature(enable = "neon")]` on
+    /// softfloat aarch64 targets. Enabling this target feature causes LLVM to alter the ABI of
+    /// function calls, making this attribute unsound to use.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,ignore (needs aarch64-unknown-none-softfloat)
+    /// #[target_feature(enable = "neon")]
+    /// fn with_neon() {}
+    /// ```
+    ///
+    /// This will produce:
+    ///
+    /// ```text
+    /// error: enabling the `neon` target feature on the current target is unsound due to ABI issues
+    ///   --> $DIR/abi-incompatible-target-feature-attribute-fcw.rs:11:18
+    ///    |
+    ///    | #[target_feature(enable = "neon")]
+    ///    |                  ^^^^^^^^^^^^^^^
+    ///    |
+    ///    = warning: 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 #134375 <https://github.com/rust-lang/rust/issues/134375>
+    /// ```
+    ///
+    /// ### Explanation
+    ///
+    /// If a function like `with_neon` above ends up containing calls to LLVM builtins, those will
+    /// not use the correct ABI. This is caused by a lack of support in LLVM for mixing code with
+    /// and without the `neon` target feature. The target feature should never have been stabilized
+    /// on this target due to this issue, but the problem was not known at the time of
+    /// stabilization.
+    pub AARCH64_SOFTFLOAT_NEON,
+    Warn,
+    "detects code that could be affected by ABI issues on aarch64 softfloat targets",
+    @future_incompatible = FutureIncompatibleInfo {
+        reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
+        reference: "issue #134375 <https://github.com/rust-lang/rust/issues/134375>",
+    };
+}
diff --git a/compiler/rustc_target/src/target_features.rs b/compiler/rustc_target/src/target_features.rs
index 99b04ac2720..576c9bd6b57 100644
--- a/compiler/rustc_target/src/target_features.rs
+++ b/compiler/rustc_target/src/target_features.rs
@@ -980,14 +980,16 @@ impl Target {
                 // the use of soft-float, so all we can do here is some crude hacks.
                 match &*self.abi {
                     "softfloat" => {
-                        // This is not fully correct, LLVM actually doesn't let us enforce the softfloat
-                        // ABI properly... see <https://github.com/rust-lang/rust/issues/134375>.
-                        // FIXME: should we forbid "neon" here? But that would be a breaking change.
-                        NOTHING
+                        // LLVM will use float registers when `fp-armv8` is available, e.g. for
+                        // calls to built-ins. The only way to ensure a consistent softfloat ABI
+                        // on aarch64 is to never enable `fp-armv8`, so we enforce that.
+                        // In Rust we tie `neon` and `fp-armv8` together, therefore `neon` is the
+                        // feature we have to mark as incompatible.
+                        FeatureConstraints { required: &[], incompatible: &["neon"] }
                     }
                     _ => {
                         // Everything else is assumed to use a hardfloat ABI. neon and fp-armv8 must be enabled.
-                        // These are Rust feature names and we use "neon" to control both of them.
+                        // `FeatureConstraints` uses Rust feature names, hence only "neon" shows up.
                         FeatureConstraints { required: &["neon"], incompatible: &[] }
                     }
                 }
diff --git a/tests/ui/target-feature/abi-incompatible-target-feature-attribute-fcw.rs b/tests/ui/target-feature/abi-incompatible-target-feature-attribute-fcw.rs
new file mode 100644
index 00000000000..270874a9f58
--- /dev/null
+++ b/tests/ui/target-feature/abi-incompatible-target-feature-attribute-fcw.rs
@@ -0,0 +1,14 @@
+//@ compile-flags: --crate-type=lib
+//@ compile-flags: --target=aarch64-unknown-none-softfloat
+//@ needs-llvm-components: aarch64
+#![feature(no_core, lang_items)]
+#![no_core]
+#![deny(aarch64_softfloat_neon)]
+
+#[lang = "sized"]
+pub trait Sized {}
+
+#[target_feature(enable = "neon")]
+//~^ERROR: enabling the `neon` target feature on the current target is unsound
+//~|WARN: previously accepted
+pub unsafe fn my_fun() {}
diff --git a/tests/ui/target-feature/abi-incompatible-target-feature-attribute-fcw.stderr b/tests/ui/target-feature/abi-incompatible-target-feature-attribute-fcw.stderr
new file mode 100644
index 00000000000..bf745291a5a
--- /dev/null
+++ b/tests/ui/target-feature/abi-incompatible-target-feature-attribute-fcw.stderr
@@ -0,0 +1,31 @@
+error: enabling the `neon` target feature on the current target is unsound due to ABI issues
+  --> $DIR/abi-incompatible-target-feature-attribute-fcw.rs:11:18
+   |
+LL | #[target_feature(enable = "neon")]
+   |                  ^^^^^^^^^^^^^^^
+   |
+   = warning: 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 #134375 <https://github.com/rust-lang/rust/issues/134375>
+note: the lint level is defined here
+  --> $DIR/abi-incompatible-target-feature-attribute-fcw.rs:6:9
+   |
+LL | #![deny(aarch64_softfloat_neon)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+
+Future incompatibility report: Future breakage diagnostic:
+error: enabling the `neon` target feature on the current target is unsound due to ABI issues
+  --> $DIR/abi-incompatible-target-feature-attribute-fcw.rs:11:18
+   |
+LL | #[target_feature(enable = "neon")]
+   |                  ^^^^^^^^^^^^^^^
+   |
+   = warning: 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 #134375 <https://github.com/rust-lang/rust/issues/134375>
+note: the lint level is defined here
+  --> $DIR/abi-incompatible-target-feature-attribute-fcw.rs:6:9
+   |
+LL | #![deny(aarch64_softfloat_neon)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^
+