Skip to content

Commit bd867cb

Browse files
rust-timerrust-timer
authored
Unrolled build for rust-lang#136147
Rollup merge of rust-lang#136147 - RalfJung:required-target-features-check-not-add, r=workingjubilee ABI-required target features: warn when they are missing in base CPU Part of rust-lang#135408: instead of adding ABI-required features to the target we build for LLVM, check that they are already there. Crucially we check this after applying `-Ctarget-cpu` and `-Ctarget-feature`, by reading `sess.unstable_target_features`. This means we can tweak the ABI target feature check without changing the behavior for any existing user; they will get warnings but the target features behave as before. The test changes here show that we are un-doing the "add all required target features" part. Without the full rust-lang#135408, there is no way to take a way an ABI-required target feature with `-Ctarget-cpu`, so we cannot yet test that part. Cc ``@workingjubilee``
2 parents 122fb29 + 3f6ffa1 commit bd867cb

16 files changed

+91
-140
lines changed

‎compiler/rustc_codegen_gcc/src/gcc_util.rs

+1-49
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
1-
use std::iter::FromIterator;
2-
31
#[cfg(feature = "master")]
42
use gccjit::Context;
53
use rustc_codegen_ssa::codegen_attrs::check_tied_features;
64
use rustc_codegen_ssa::errors::TargetFeatureDisableOrEnable;
7-
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
5+
use rustc_data_structures::fx::FxHashMap;
86
use rustc_data_structures::unord::UnordSet;
97
use rustc_session::Session;
108
use rustc_target::target_features::RUSTC_SPECIFIC_FEATURES;
@@ -45,12 +43,6 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
4543
let known_features = sess.target.rust_target_features();
4644
let mut featsmap = FxHashMap::default();
4745

48-
// Ensure that all ABI-required features are enabled, and the ABI-forbidden ones
49-
// are disabled.
50-
let abi_feature_constraints = sess.target.abi_required_features();
51-
let abi_incompatible_set =
52-
FxHashSet::from_iter(abi_feature_constraints.incompatible.iter().copied());
53-
5446
// Compute implied features
5547
let mut all_rust_features = vec![];
5648
for feature in sess.opts.cg.target_feature.split(',') {
@@ -117,51 +109,11 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
117109
}
118110
}
119111

120-
// Ensure that the features we enable/disable are compatible with the ABI.
121-
if enable {
122-
if abi_incompatible_set.contains(feature) {
123-
sess.dcx().emit_warn(ForbiddenCTargetFeature {
124-
feature,
125-
enabled: "enabled",
126-
reason: "this feature is incompatible with the target ABI",
127-
});
128-
}
129-
} else {
130-
// FIXME: we have to request implied features here since
131-
// negative features do not handle implied features above.
132-
for &required in abi_feature_constraints.required.iter() {
133-
let implied = sess.target.implied_target_features(std::iter::once(required));
134-
if implied.contains(feature) {
135-
sess.dcx().emit_warn(ForbiddenCTargetFeature {
136-
feature,
137-
enabled: "disabled",
138-
reason: "this feature is required by the target ABI",
139-
});
140-
}
141-
}
142-
}
143-
144112
// FIXME(nagisa): figure out how to not allocate a full hashset here.
145113
featsmap.insert(feature, enable);
146114
}
147115
}
148116

149-
// To be sure the ABI-relevant features are all in the right state, we explicitly
150-
// (un)set them here. This means if the target spec sets those features wrong,
151-
// we will silently correct them rather than silently producing wrong code.
152-
// (The target sanity check tries to catch this, but we can't know which features are
153-
// enabled in GCC by default so we can't be fully sure about that check.)
154-
// We add these at the beginning of the list so that `-Ctarget-features` can
155-
// still override it... that's unsound, but more compatible with past behavior.
156-
all_rust_features.splice(
157-
0..0,
158-
abi_feature_constraints
159-
.required
160-
.iter()
161-
.map(|&f| (true, f))
162-
.chain(abi_feature_constraints.incompatible.iter().map(|&f| (false, f))),
163-
);
164-
165117
// Translate this into GCC features.
166118
let feats =
167119
all_rust_features.iter().flat_map(|&(enable, feature)| {

‎compiler/rustc_codegen_gcc/src/lib.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -493,9 +493,10 @@ fn target_features_cfg(
493493
sess.target
494494
.rust_target_features()
495495
.iter()
496-
.filter(|&&(_, gate, _)| gate.in_cfg())
497496
.filter_map(|&(feature, gate, _)| {
498-
if sess.is_nightly_build() || allow_unstable || gate.requires_nightly().is_none() {
497+
if allow_unstable
498+
|| (gate.in_cfg() && (sess.is_nightly_build() || gate.requires_nightly().is_none()))
499+
{
499500
Some(feature)
500501
} else {
501502
None

‎compiler/rustc_codegen_llvm/src/llvm_util.rs

+6-50
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,6 @@ pub fn target_features_cfg(sess: &Session, allow_unstable: bool) -> Vec<Symbol>
319319
sess.target
320320
.rust_target_features()
321321
.iter()
322-
.filter(|(_, gate, _)| gate.in_cfg())
323322
.filter(|(feature, _, _)| {
324323
// skip checking special features, as LLVM may not understand them
325324
if RUSTC_SPECIAL_FEATURES.contains(feature) {
@@ -388,9 +387,13 @@ pub fn target_features_cfg(sess: &Session, allow_unstable: bool) -> Vec<Symbol>
388387
sess.target
389388
.rust_target_features()
390389
.iter()
391-
.filter(|(_, gate, _)| gate.in_cfg())
392390
.filter_map(|(feature, gate, _)| {
393-
if sess.is_nightly_build() || allow_unstable || gate.requires_nightly().is_none() {
391+
// The `allow_unstable` set is used by rustc internally to determined which target
392+
// features are truly available, so we want to return even perma-unstable "forbidden"
393+
// features.
394+
if allow_unstable
395+
|| (gate.in_cfg() && (sess.is_nightly_build() || gate.requires_nightly().is_none()))
396+
{
394397
Some(*feature)
395398
} else {
396399
None
@@ -670,12 +673,6 @@ pub(crate) fn global_llvm_features(
670673
// Will only be filled when `diagnostics` is set!
671674
let mut featsmap = FxHashMap::default();
672675

673-
// Ensure that all ABI-required features are enabled, and the ABI-forbidden ones
674-
// are disabled.
675-
let abi_feature_constraints = sess.target.abi_required_features();
676-
let abi_incompatible_set =
677-
FxHashSet::from_iter(abi_feature_constraints.incompatible.iter().copied());
678-
679676
// Compute implied features
680677
let mut all_rust_features = vec![];
681678
for feature in sess.opts.cg.target_feature.split(',') {
@@ -746,52 +743,11 @@ pub(crate) fn global_llvm_features(
746743
}
747744
}
748745

749-
// Ensure that the features we enable/disable are compatible with the ABI.
750-
if enable {
751-
if abi_incompatible_set.contains(feature) {
752-
sess.dcx().emit_warn(ForbiddenCTargetFeature {
753-
feature,
754-
enabled: "enabled",
755-
reason: "this feature is incompatible with the target ABI",
756-
});
757-
}
758-
} else {
759-
// FIXME: we have to request implied features here since
760-
// negative features do not handle implied features above.
761-
for &required in abi_feature_constraints.required.iter() {
762-
let implied =
763-
sess.target.implied_target_features(std::iter::once(required));
764-
if implied.contains(feature) {
765-
sess.dcx().emit_warn(ForbiddenCTargetFeature {
766-
feature,
767-
enabled: "disabled",
768-
reason: "this feature is required by the target ABI",
769-
});
770-
}
771-
}
772-
}
773-
774746
// FIXME(nagisa): figure out how to not allocate a full hashset here.
775747
featsmap.insert(feature, enable);
776748
}
777749
}
778750

779-
// To be sure the ABI-relevant features are all in the right state, we explicitly
780-
// (un)set them here. This means if the target spec sets those features wrong,
781-
// we will silently correct them rather than silently producing wrong code.
782-
// (The target sanity check tries to catch this, but we can't know which features are
783-
// enabled in LLVM by default so we can't be fully sure about that check.)
784-
// We add these at the beginning of the list so that `-Ctarget-features` can
785-
// still override it... that's unsound, but more compatible with past behavior.
786-
all_rust_features.splice(
787-
0..0,
788-
abi_feature_constraints
789-
.required
790-
.iter()
791-
.map(|&f| (true, f))
792-
.chain(abi_feature_constraints.incompatible.iter().map(|&f| (false, f))),
793-
);
794-
795751
// Translate this into LLVM features.
796752
let feats = all_rust_features
797753
.iter()

‎compiler/rustc_interface/messages.ftl

+5
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
interface_abi_required_feature =
2+
target feature `{$feature}` must be {$enabled} to ensure that the ABI of the current target can be implemented correctly
3+
.note = this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
4+
interface_abi_required_feature_issue = for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>
5+
16
interface_cant_emit_mir =
27
could not emit MIR: {$error}
38

‎compiler/rustc_interface/src/errors.rs

+9
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,12 @@ pub struct IgnoringOutDir;
103103
#[derive(Diagnostic)]
104104
#[diag(interface_multiple_output_types_to_stdout)]
105105
pub struct MultipleOutputTypesToStdout;
106+
107+
#[derive(Diagnostic)]
108+
#[diag(interface_abi_required_feature)]
109+
#[note]
110+
#[note(interface_abi_required_feature_issue)]
111+
pub(crate) struct AbiRequiredTargetFeature<'a> {
112+
pub feature: &'a str,
113+
pub enabled: &'a str,
114+
}

‎compiler/rustc_interface/src/interface.rs

+2
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,8 @@ pub fn run_compiler<R: Send>(config: Config, f: impl FnOnce(&Compiler) -> R + Se
492492
}
493493
sess.lint_store = Some(Lrc::new(lint_store));
494494

495+
util::check_abi_required_features(&sess);
496+
495497
let compiler = Compiler {
496498
sess,
497499
codegen_backend,

‎compiler/rustc_interface/src/util.rs

+35-3
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,25 @@ use rustc_session::{EarlyDiagCtxt, Session, filesearch};
1818
use rustc_span::edit_distance::find_best_match_for_name;
1919
use rustc_span::edition::Edition;
2020
use rustc_span::source_map::SourceMapInputs;
21-
use rustc_span::sym;
21+
use rustc_span::{Symbol, sym};
2222
use rustc_target::spec::Target;
2323
use tracing::info;
2424

2525
use crate::errors;
2626

2727
/// Function pointer type that constructs a new CodegenBackend.
28-
pub type MakeBackendFn = fn() -> Box<dyn CodegenBackend>;
28+
type MakeBackendFn = fn() -> Box<dyn CodegenBackend>;
2929

3030
/// Adds `target_feature = "..."` cfgs for a variety of platform
3131
/// specific features (SSE, NEON etc.).
3232
///
3333
/// This is performed by checking whether a set of permitted features
3434
/// is available on the target machine, by querying the codegen backend.
35-
pub fn add_configuration(cfg: &mut Cfg, sess: &mut Session, codegen_backend: &dyn CodegenBackend) {
35+
pub(crate) fn add_configuration(
36+
cfg: &mut Cfg,
37+
sess: &mut Session,
38+
codegen_backend: &dyn CodegenBackend,
39+
) {
3640
let tf = sym::target_feature;
3741

3842
let unstable_target_features = codegen_backend.target_features_cfg(sess, true);
@@ -48,6 +52,34 @@ pub fn add_configuration(cfg: &mut Cfg, sess: &mut Session, codegen_backend: &dy
4852
}
4953
}
5054

55+
/// Ensures that all target features required by the ABI are present.
56+
/// Must be called after `unstable_target_features` has been populated!
57+
pub(crate) fn check_abi_required_features(sess: &Session) {
58+
let abi_feature_constraints = sess.target.abi_required_features();
59+
// We check this against `unstable_target_features` as that is conveniently already
60+
// back-translated to rustc feature names, taking into account `-Ctarget-cpu` and `-Ctarget-feature`.
61+
// Just double-check that the features we care about are actually on our list.
62+
for feature in
63+
abi_feature_constraints.required.iter().chain(abi_feature_constraints.incompatible.iter())
64+
{
65+
assert!(
66+
sess.target.rust_target_features().iter().any(|(name, ..)| feature == name),
67+
"target feature {feature} is required/incompatible for the current ABI but not a recognized feature for this target"
68+
);
69+
}
70+
71+
for feature in abi_feature_constraints.required {
72+
if !sess.unstable_target_features.contains(&Symbol::intern(feature)) {
73+
sess.dcx().emit_warn(errors::AbiRequiredTargetFeature { feature, enabled: "enabled" });
74+
}
75+
}
76+
for feature in abi_feature_constraints.incompatible {
77+
if sess.unstable_target_features.contains(&Symbol::intern(feature)) {
78+
sess.dcx().emit_warn(errors::AbiRequiredTargetFeature { feature, enabled: "disabled" });
79+
}
80+
}
81+
}
82+
5183
pub static STACK_SIZE: OnceLock<usize> = OnceLock::new();
5284
pub const DEFAULT_STACK_SIZE: usize = 8 * 1024 * 1024;
5385

‎compiler/rustc_target/src/target_features.rs

+11-13
Original file line numberDiff line numberDiff line change
@@ -108,21 +108,19 @@ impl Stability {
108108
// per-function level, since we would then allow safe calls from functions with `+soft-float` to
109109
// functions without that feature!
110110
//
111-
// It is important for soundness that features allowed here do *not* change the function call ABI.
112-
// For example, disabling the `x87` feature on x86 changes how scalar floats are passed as
113-
// arguments, so enabling toggling that feature would be unsound. In fact, since `-Ctarget-feature`
114-
// will just allow unknown features (with a warning), we have to explicitly list features that change
115-
// the ABI as `Forbidden` to ensure using them causes an error. Note that this is only effective if
116-
// such features can never be toggled via `-Ctarget-cpu`! If that is ever a possibility, we will need
117-
// extra checks ensuring that the LLVM-computed target features for a CPU did not (un)set a
118-
// `Forbidden` feature. See https://github.com/rust-lang/rust/issues/116344 for some more context.
119-
// FIXME: add such "forbidden" features for non-x86 targets.
111+
// It is important for soundness to consider the interaction of targets features and the function
112+
// call ABI. For example, disabling the `x87` feature on x86 changes how scalar floats are passed as
113+
// arguments, so letting people toggle that feature would be unsound. To this end, the
114+
// `abi_required_features` function computes which target features must and must not be enabled for
115+
// any given target, and individual features can also be marked as `Forbidden`.
116+
// See https://github.com/rust-lang/rust/issues/116344 for some more context.
120117
//
121118
// The one exception to features that change the ABI is features that enable larger vector
122-
// registers. Those are permitted to be listed here. This is currently unsound (see
123-
// https://github.com/rust-lang/rust/issues/116558); in the future we will have to ensure that
124-
// functions can only use such vectors as arguments/return types if the corresponding target feature
125-
// is enabled.
119+
// registers. Those are permitted to be listed here. The `*_FOR_CORRECT_VECTOR_ABI` arrays store
120+
// information about which target feature is ABI-required for which vector size; this is used to
121+
// ensure that vectors can only be passed via `extern "C"` when the right feature is enabled. (For
122+
// the "Rust" ABI we generally pass vectors by-ref exactly to avoid these issues.)
123+
// Also see https://github.com/rust-lang/rust/issues/116558.
126124
//
127125
// Stabilizing a target feature requires t-lang approval.
128126

‎src/tools/miri/tests/pass/shims/x86/intrinsics-x86-pause-without-sse2.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// We're testing x86 target specific features
2-
//@only-target: x86_64 i686
1+
// We're testing x86-32 target specific features. SSE always exists on x86-64.
2+
//@only-target: i686
33
//@compile-flags: -C target-feature=-sse2
44

55
#[cfg(target_arch = "x86")]

‎tests/codegen/target-feature-overrides.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ pub unsafe fn banana() -> u32 {
3939
}
4040

4141
// CHECK: attributes [[APPLEATTRS]]
42-
// COMPAT-SAME: "target-features"="+x87,+sse2,+avx,+avx2,{{.*}}"
43-
// INCOMPAT-SAME: "target-features"="+x87,+sse2,-avx2,-avx,+avx,{{.*}}"
42+
// COMPAT-SAME: "target-features"="+avx,+avx2,{{.*}}"
43+
// INCOMPAT-SAME: "target-features"="-avx2,-avx,+avx,{{.*}}"
4444
// CHECK: attributes [[BANANAATTRS]]
45-
// COMPAT-SAME: "target-features"="+x87,+sse2,+avx,+avx2,{{.*}}"
46-
// INCOMPAT-SAME: "target-features"="+x87,+sse2,-avx2,-avx"
45+
// COMPAT-SAME: "target-features"="+avx,+avx2,{{.*}}"
46+
// INCOMPAT-SAME: "target-features"="-avx2,-avx"

‎tests/codegen/tied-features-strength.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,10 @@
1111
// ENABLE_SVE: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(\+sve,?)|(\+neon,?)|(\+fp-armv8,?))*}}" }
1212

1313
//@ [DISABLE_SVE] compile-flags: -C target-feature=-sve -Copt-level=0
14-
// DISABLE_SVE: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(-sve,?)|(\+neon,?)|(\+fp-armv8,?))*}}" }
14+
// DISABLE_SVE: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(-sve,?)|(\+neon,?))*}}" }
1515

1616
//@ [DISABLE_NEON] compile-flags: -C target-feature=-neon -Copt-level=0
17-
// `neon` and `fp-armv8` get enabled as target base features, but then disabled again at the end of the list.
18-
// DISABLE_NEON: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fp-armv8,?)|(\+neon,?))*}},-neon,-fp-armv8{{(,\+fpmr)?}}" }
17+
// DISABLE_NEON: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(-fp-armv8,?)|(-neon,?))*}}" }
1918

2019
//@ [ENABLE_NEON] compile-flags: -C target-feature=+neon -Copt-level=0
2120
// ENABLE_NEON: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(\+fp-armv8,?)|(\+neon,?))*}}" }

‎tests/ui-fulldeps/codegen-backend/hotplug.rs

+4
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
//@ normalize-stdout: "libthe_backend.dylib" -> "libthe_backend.so"
77
//@ normalize-stdout: "the_backend.dll" -> "libthe_backend.so"
88

9+
// Pick a target that requires no target features, so that no warning is shown
10+
// about missing target features.
11+
//@ compile-flags: --target arm-unknown-linux-gnueabi
12+
//@ needs-llvm-components: arm
913
//@ revisions: normal dep bindep
1014
//@ compile-flags: --crate-type=lib
1115
//@ [normal] compile-flags: --emit=link=-

‎tests/ui/target-feature/feature-hierarchy.aarch64-sve2.stderr

-7
This file was deleted.

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
warning: target feature `sse` cannot be disabled with `-Ctarget-feature`: this feature is required by the target ABI
1+
warning: target feature `sse2` must be enabled to ensure that the ABI of the current target can be implemented correctly
22
|
33
= note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
44
= note: for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
warning: target feature `neon` cannot be disabled with `-Ctarget-feature`: this feature is required by the target ABI
1+
warning: target feature `neon` must be enabled to ensure that the ABI of the current target can be implemented correctly
22
|
33
= note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
44
= note: for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>

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

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
warning: unstable feature specified for `-Ctarget-feature`: `x87`
2-
|
3-
= note: this feature is not stably supported; its behavior can change in the future
4-
5-
warning: target feature `x87` cannot be disabled with `-Ctarget-feature`: this feature is required by the target ABI
1+
warning: target feature `x87` must be enabled to ensure that the ABI of the current target can be implemented correctly
62
|
73
= note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
84
= note: for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>
95

6+
warning: unstable feature specified for `-Ctarget-feature`: `x87`
7+
|
8+
= note: this feature is not stably supported; its behavior can change in the future
9+
1010
warning: 2 warnings emitted
1111

0 commit comments

Comments
 (0)