forbid toggling x87 and fpregs on hard-float targets

Author: RalfJung

compiler/rustc_feature/src/unstable.rs

@@ -340,6 +340,7 @@ declare_features! (
     (unstable, sse4a_target_feature, "1.27.0", Some(44839)),
     (unstable, tbm_target_feature, "1.27.0", Some(44839)),
     (unstable, wasm_target_feature, "1.30.0", Some(44839)),
+    (unstable, x87_target_feature, "CURRENT_RUSTC_VERSION", Some(44839)),
     // !!!!    !!!!    !!!!    !!!!   !!!!    !!!!    !!!!    !!!!    !!!!    !!!!    !!!!
     // Features are listed in alphabetical order. Tidy will fail if you don't keep it this way.
     // !!!!    !!!!    !!!!    !!!!   !!!!    !!!!    !!!!    !!!!    !!!!    !!!!    !!!!

compiler/rustc_span/src/symbol.rs

@@ -2171,6 +2171,7 @@ symbols! {
         writeln_macro,
         x86_amx_intrinsics,
         x87_reg,
+        x87_target_feature,
         xer,
         xmm_reg,
         xop_target_feature,

compiler/rustc_target/src/spec/mod.rs

@@ -2594,6 +2594,18 @@ impl TargetOptions {
                 .collect();
         }
     }
+
+    pub(crate) fn has_feature(&self, search_feature: &str) -> bool {
+        self.features.split(',').any(|f| {
+            if let Some(f) = f.strip_prefix('+')
+                && f == search_feature
+            {
+                true
+            } else {
+                false
+            }
+        })
+    }
 }
 
 impl Default for TargetOptions {

compiler/rustc_target/src/target_features.rs

@@ -32,6 +32,8 @@ pub enum Stability<AllowToggle> {
     /// This target feature is unstable. It is only present in `#[cfg(target_feature)]` on
     /// nightly and using it in `#[target_feature]` requires enabling the given nightly feature.
     Unstable {
+        /// This must be a *language* feature, or else rustc will ICE when reporting a missing
+        /// feature gate!
         nightly_feature: Symbol,
         /// See `Stable::allow_toggle` comment above.
         allow_toggle: AllowToggle,
@@ -168,6 +170,22 @@ const ARM_FEATURES: &[(&str, StabilityUncomputed, ImpliedFeatures)] = &[
     ("dotprod", unstable(sym::arm_target_feature), &["neon"]),
     ("dsp", unstable(sym::arm_target_feature), &[]),
     ("fp-armv8", unstable(sym::arm_target_feature), &["vfp4"]),
+    (
+        "fpregs",
+        Stability::Unstable {
+            nightly_feature: sym::arm_target_feature,
+            allow_toggle: |target: &Target| {
+                // Only allow toggling this if the target has `soft-float` set. With `soft-float`,
+                // `fpregs` isn't needed so changing it cannot affect the ABI.
+                if target.has_feature("soft-float") {
+                    Ok(())
+                } else {
+                    Err("unsound on hard-float targets because it changes float ABI")
+                }
+            },
+        },
+        &[],
+    ),
     ("i8mm", unstable(sym::arm_target_feature), &["neon"]),
     ("mclass", unstable(sym::arm_target_feature), &[]),
     ("neon", unstable(sym::arm_target_feature), &["vfp3"]),
@@ -191,7 +209,6 @@ const ARM_FEATURES: &[(&str, StabilityUncomputed, 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_FEATURES: &[(&str, StabilityUncomputed, ImpliedFeatures)] = &[
@@ -444,13 +461,28 @@ const X86_FEATURES: &[(&str, StabilityUncomputed, ImpliedFeatures)] = &[
     ("tbm", unstable(sym::tbm_target_feature), &[]),
     ("vaes", unstable(sym::avx512_target_feature), &["avx2", "aes"]),
     ("vpclmulqdq", unstable(sym::avx512_target_feature), &["avx", "pclmulqdq"]),
+    (
+        "x87",
+        Stability::Unstable {
+            nightly_feature: sym::x87_target_feature,
+            allow_toggle: |target: &Target| {
+                // Only allow toggling this if the target has `soft-float` set. With `soft-float`,
+                // `fpregs` isn't needed so changing it cannot affect the ABI.
+                if target.has_feature("soft-float") {
+                    Ok(())
+                } else {
+                    Err("unsound on hard-float targets because it changes float ABI")
+                }
+            },
+        },
+        &[],
+    ),
     ("xop", unstable(sym::xop_target_feature), &[/*"fma4", */ "avx", "sse4a"]),
     ("xsave", STABLE, &[]),
     ("xsavec", STABLE, &["xsave"]),
     ("xsaveopt", STABLE, &["xsave"]),
     ("xsaves", STABLE, &["xsave"]),
     // tidy-alphabetical-end
-    // FIXME: need to also forbid turning off `x87` on hardfloat targets
 ];
 
 const HEXAGON_FEATURES: &[(&str, StabilityUncomputed, ImpliedFeatures)] = &[

tests/ui/target-feature/allowed-softfloat-target-feature-attribute.rs

@@ -0,0 +1,12 @@
+//@ compile-flags: --target=x86_64-unknown-none --crate-type=lib
+//@ needs-llvm-components: x86
+//@ build-pass
+#![feature(no_core, lang_items, x87_target_feature)]
+#![no_std]
+#![no_core]
+
+#[lang = "sized"]
+pub trait Sized {}
+
+#[target_feature(enable = "x87")]
+pub unsafe fn my_fun() {}

tests/ui/target-feature/allowed-softfloat-target-feature-flag-disable.rs

@@ -0,0 +1,10 @@
+//@ compile-flags: --target=x86_64-unknown-none --crate-type=lib
+//@ needs-llvm-components: x86
+//@ compile-flags: -Ctarget-feature=-x87
+//@ build-pass
+#![feature(no_core, lang_items)]
+#![no_std]
+#![no_core]
+
+#[lang = "sized"]
+pub trait Sized {}

tests/ui/target-feature/allowed-softfloat-target-feature-flag-disable.stderr

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

tests/ui/target-feature/forbidden-hardfloat-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 = "x87")]
+//~^ERROR: cannot be toggled with
+pub unsafe fn my_fun() {}

tests/ui/target-feature/forbidden-hardfloat-target-feature-attribute.stderr

@@ -0,0 +1,8 @@
+error: target feature `x87` cannot be toggled with `#[target_feature]`: unsound on hard-float targets because it changes float ABI
+  --> $DIR/forbidden-hardfloat-target-feature-attribute.rs:10:18
+   |
+LL | #[target_feature(enable = "x87")]
+   |                  ^^^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+

tests/ui/target-feature/forbidden-hardfloat-target-feature-cfg.rs

@@ -0,0 +1,15 @@
+//@ compile-flags: --target=x86_64-unknown-linux-gnu --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(not(target_feature = "x87"))]
+compile_error!("the x87 feature *should* be exposed in `cfg`");

tests/ui/target-feature/forbidden-hardfloat-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=-x87
+// For now this is just a warning.
+//@ build-pass
+#![feature(no_core, lang_items)]
+#![no_std]
+#![no_core]
+
+#[lang = "sized"]
+pub trait Sized {}

tests/ui/target-feature/forbidden-hardfloat-target-feature-flag-disable.stderr

@@ -0,0 +1,7 @@
+warning: target feature `x87` cannot be toggled with `-Ctarget-feature`: unsound on hard-float targets 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 <https://github.com/rust-lang/rust/issues/116344>
+
+warning: 1 warning emitted
+