Skip to content

Commit 27ae2b3

Browse files
borsbors
committed
Auto merge of #135160 - RalfJung:aarch64-softfloat-not-neon, r=<try>
aarch64-softfloat: forbid enabling the neon target feature This fixes #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
2 parents a8e4c68 + ceef31c commit 27ae2b3

File tree

9 files changed

+128
-19
lines changed

9 files changed

+128
-19
lines changed

‎compiler/rustc_codegen_ssa/messages.ftl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
codegen_ssa_L4Bender_exporting_symbols_unimplemented = exporting symbols not implemented yet for L4Bender
22
3+
codegen_ssa_aarch64_softfloat_neon = enabling the `neon` target feature on the current target is unsound due to ABI issues
4+
35
codegen_ssa_add_native_library = failed to add native library {$library_path}: {$error}
46
57
codegen_ssa_aix_strip_not_used = using host's `strip` binary to cross-compile to AIX which is not guaranteed to work

‎compiler/rustc_codegen_ssa/src/codegen_attrs.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
299299
}
300300
from_target_feature_attr(
301301
tcx,
302+
did,
302303
attr,
303304
rust_target_features,
304305
&mut codegen_fn_attrs.target_features,

‎compiler/rustc_codegen_ssa/src/errors.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,3 +1316,7 @@ pub(crate) struct XcrunSdkPathWarning {
13161316
pub sdk_name: &'static str,
13171317
pub stderr: String,
13181318
}
1319+
1320+
#[derive(LintDiagnostic)]
1321+
#[diag(codegen_ssa_aarch64_softfloat_neon)]
1322+
pub(crate) struct Aarch64SoftfloatNeon;

‎compiler/rustc_codegen_ssa/src/target_features.rs

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId};
88
use rustc_middle::middle::codegen_fn_attrs::TargetFeature;
99
use rustc_middle::query::Providers;
1010
use rustc_middle::ty::TyCtxt;
11+
use rustc_session::lint::builtin::AARCH64_SOFTFLOAT_NEON;
1112
use rustc_session::parse::feature_err;
1213
use rustc_span::{Span, Symbol, sym};
1314
use rustc_target::target_features::{self, Stability};
@@ -18,6 +19,7 @@ use crate::errors;
1819
/// Enabled target features are added to `target_features`.
1920
pub(crate) fn from_target_feature_attr(
2021
tcx: TyCtxt<'_>,
22+
did: LocalDefId,
2123
attr: &hir::Attribute,
2224
rust_target_features: &UnordMap<String, target_features::Stability>,
2325
target_features: &mut Vec<TargetFeature>,
@@ -92,11 +94,22 @@ pub(crate) fn from_target_feature_attr(
9294
// generating code so "it's fine".
9395
if !tcx.sess.opts.actually_rustdoc {
9496
if abi_feature_constraints.incompatible.contains(&name.as_str()) {
95-
tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr {
96-
span: item.span(),
97-
feature: name.as_str(),
98-
reason: "this feature is incompatible with the target ABI",
99-
});
97+
// For "neon" specifically, we emit an FCW instead of a hard error.
98+
// See <https://github.com/rust-lang/rust/issues/134375>.
99+
if tcx.sess.target.arch == "aarch64" && name.as_str() == "neon" {
100+
tcx.emit_node_span_lint(
101+
AARCH64_SOFTFLOAT_NEON,
102+
tcx.local_def_id_to_hir_id(did),
103+
item.span(),
104+
errors::Aarch64SoftfloatNeon,
105+
);
106+
} else {
107+
tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr {
108+
span: item.span(),
109+
feature: name.as_str(),
110+
reason: "this feature is incompatible with the target ABI",
111+
});
112+
}
100113
}
101114
}
102115
target_features.push(TargetFeature { name, implied: name != feature_sym })

‎compiler/rustc_lint_defs/src/builtin.rs

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ declare_lint_pass! {
1616
/// that are used by other parts of the compiler.
1717
HardwiredLints => [
1818
// tidy-alphabetical-start
19+
AARCH64_SOFTFLOAT_NEON,
1920
ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE,
2021
AMBIGUOUS_ASSOCIATED_ITEMS,
2122
AMBIGUOUS_GLOB_IMPORTS,
@@ -5043,14 +5044,14 @@ declare_lint! {
50435044
///
50445045
/// ```text
50455046
/// error: this function function definition is affected by the wasm ABI transition: it passes an argument of non-scalar type `MyType`
5046-
/// --> $DIR/wasm_c_abi_transition.rs:17:1
5047-
/// |
5048-
/// | pub extern "C" fn my_fun(_x: MyType) {}
5049-
/// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5050-
/// |
5051-
/// = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
5052-
/// = note: for more information, see issue #138762 <https://github.com/rust-lang/rust/issues/138762>
5053-
/// = help: the "C" ABI Rust uses on wasm32-unknown-unknown will change to align with the standard "C" ABI for this target
5047+
/// --> $DIR/wasm_c_abi_transition.rs:17:1
5048+
/// |
5049+
/// | pub extern "C" fn my_fun(_x: MyType) {}
5050+
/// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5051+
/// |
5052+
/// = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
5053+
/// = note: for more information, see issue #138762 <https://github.com/rust-lang/rust/issues/138762>
5054+
/// = help: the "C" ABI Rust uses on wasm32-unknown-unknown will change to align with the standard "C" ABI for this target
50545055
/// ```
50555056
///
50565057
/// ### Explanation
@@ -5067,3 +5068,44 @@ declare_lint! {
50675068
reference: "issue #138762 <https://github.com/rust-lang/rust/issues/138762>",
50685069
};
50695070
}
5071+
5072+
declare_lint! {
5073+
/// The `aarch64_softfloat_neon` lint detects usage of `#[target_feature(enable = "neon")]` on
5074+
/// softfloat aarch64 targets. Enabling this target feature causes LLVM to alter the ABI of
5075+
/// function calls, making this attribute unsound to use.
5076+
///
5077+
/// ### Example
5078+
///
5079+
/// ```rust,ignore (needs aarch64-unknown-none-softfloat)
5080+
/// #[target_feature(enable = "neon")]
5081+
/// fn with_neon() {}
5082+
/// ```
5083+
///
5084+
/// This will produce:
5085+
///
5086+
/// ```text
5087+
/// error: enabling the `neon` target feature on the current target is unsound due to ABI issues
5088+
/// --> $DIR/abi-incompatible-target-feature-attribute-fcw.rs:11:18
5089+
/// |
5090+
/// | #[target_feature(enable = "neon")]
5091+
/// | ^^^^^^^^^^^^^^^
5092+
/// |
5093+
/// = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
5094+
/// = note: for more information, see issue #134375 <https://github.com/rust-lang/rust/issues/134375>
5095+
/// ```
5096+
///
5097+
/// ### Explanation
5098+
///
5099+
/// If a function like `with_neon` above ends up containing calls to LLVM builtins, those will
5100+
/// not use the correct ABI. This is caused by a lack of support in LLVM for mixing code with
5101+
/// and without the `neon` target feature. The target feature should never have been stabilized
5102+
/// on this target due to this issue, but the problem was not known at the time of
5103+
/// stabilization.
5104+
pub AARCH64_SOFTFLOAT_NEON,
5105+
Warn,
5106+
"detects code that could be affected by ABI issues on aarch64 softfloat targets",
5107+
@future_incompatible = FutureIncompatibleInfo {
5108+
reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
5109+
reference: "issue #134375 <https://github.com/rust-lang/rust/issues/134375>",
5110+
};
5111+
}

‎compiler/rustc_target/src/target_features.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -980,14 +980,16 @@ impl Target {
980980
// the use of soft-float, so all we can do here is some crude hacks.
981981
match &*self.abi {
982982
"softfloat" => {
983-
// This is not fully correct, LLVM actually doesn't let us enforce the softfloat
984-
// ABI properly... see <https://github.com/rust-lang/rust/issues/134375>.
985-
// FIXME: should we forbid "neon" here? But that would be a breaking change.
986-
NOTHING
983+
// LLVM will use float registers when `fp-armv8` is available, e.g. for
984+
// calls to built-ins. The only way to ensure a consistent softfloat ABI
985+
// on aarch64 is to never enable `fp-armv8`, so we enforce that.
986+
// In Rust we tie `neon` and `fp-armv8` together, therefore `neon` is the
987+
// feature we have to mark as incompatible.
988+
FeatureConstraints { required: &[], incompatible: &["neon"] }
987989
}
988990
_ => {
989991
// Everything else is assumed to use a hardfloat ABI. neon and fp-armv8 must be enabled.
990-
// These are Rust feature names and we use "neon" to control both of them.
992+
// `FeatureConstraints` uses Rust feature names, hence only "neon" shows up.
991993
FeatureConstraints { required: &["neon"], incompatible: &[] }
992994
}
993995
}

‎library/stdarch

‎tests/ui/target-feature/abi-incompatible-target-feature-attribute-fcw.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
//@ compile-flags: --crate-type=lib
2+
//@ compile-flags: --target=aarch64-unknown-none-softfloat
3+
//@ needs-llvm-components: aarch64
4+
#![feature(no_core, lang_items)]
5+
#![no_core]
6+
#![deny(aarch64_softfloat_neon)]
7+
8+
#[lang = "sized"]
9+
pub trait Sized {}
10+
11+
#[target_feature(enable = "neon")]
12+
//~^ERROR: enabling the `neon` target feature on the current target is unsound
13+
//~|WARN: previously accepted
14+
pub unsafe fn my_fun() {}

‎tests/ui/target-feature/abi-incompatible-target-feature-attribute-fcw.stderr

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
error: enabling the `neon` target feature on the current target is unsound due to ABI issues
2+
--> $DIR/abi-incompatible-target-feature-attribute-fcw.rs:11:18
3+
|
4+
LL | #[target_feature(enable = "neon")]
5+
| ^^^^^^^^^^^^^^^
6+
|
7+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
8+
= note: for more information, see issue #134375 <https://github.com/rust-lang/rust/issues/134375>
9+
note: the lint level is defined here
10+
--> $DIR/abi-incompatible-target-feature-attribute-fcw.rs:6:9
11+
|
12+
LL | #![deny(aarch64_softfloat_neon)]
13+
| ^^^^^^^^^^^^^^^^^^^^^^
14+
15+
error: aborting due to 1 previous error
16+
17+
Future incompatibility report: Future breakage diagnostic:
18+
error: enabling the `neon` target feature on the current target is unsound due to ABI issues
19+
--> $DIR/abi-incompatible-target-feature-attribute-fcw.rs:11:18
20+
|
21+
LL | #[target_feature(enable = "neon")]
22+
| ^^^^^^^^^^^^^^^
23+
|
24+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
25+
= note: for more information, see issue #134375 <https://github.com/rust-lang/rust/issues/134375>
26+
note: the lint level is defined here
27+
--> $DIR/abi-incompatible-target-feature-attribute-fcw.rs:6:9
28+
|
29+
LL | #![deny(aarch64_softfloat_neon)]
30+
| ^^^^^^^^^^^^^^^^^^^^^^
31+

0 commit comments

Comments
 (0)