Skip to content

Commit

Permalink
Auto merge of #129884 - RalfJung:forbidden-target-features, r=working…
Browse files Browse the repository at this point in the history
…jubilee

mark some target features as 'forbidden' so they cannot be (un)set with -Ctarget-feature

The context for this is rust-lang/rust#116344: some target features change the way floats are passed between functions. Changing those target features is unsound as code compiled for the same target may now use different ABIs.

So this introduces a new concept of "forbidden" target features (on top of the existing "stable " and "unstable" categories), and makes it a hard error to (un)set such a target feature. For now, the x86 and ARM feature `soft-float` is on that list. We'll have to make some effort to collect more relevant features, and similar features from other targets, but that can happen after the basic infrastructure for this landed. (These features are being collected in rust-lang/rust#131799.)

I've made this a warning for now to give people some time to speak up if this would break something.

MCP: rust-lang/compiler-team#780
  • Loading branch information
bors committed Nov 5, 2024
2 parents b664093 + 4843153 commit 92c27e8
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 28 deletions.
11 changes: 9 additions & 2 deletions messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ codegen_gcc_invalid_minimum_alignment =
codegen_gcc_lto_not_supported =
LTO is not supported. You may get a linker error.
codegen_gcc_forbidden_ctarget_feature =
target feature `{$feature}` cannot be toggled with `-Ctarget-feature`
codegen_gcc_unwinding_inline_asm =
GCC backend does not support unwinding from inline asm
Expand All @@ -24,11 +27,15 @@ codegen_gcc_lto_dylib = lto cannot be used for `dylib` crate type without `-Zdyl
codegen_gcc_lto_bitcode_from_rlib = failed to get bitcode from object file for LTO ({$gcc_err})
codegen_gcc_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
codegen_gcc_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_gcc_missing_features =
add the missing features in a `target_feature` attribute
Expand Down
13 changes: 13 additions & 0 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,19 @@ pub(crate) struct UnknownCTargetFeature<'a> {
pub rust_feature: PossibleFeature<'a>,
}

#[derive(Diagnostic)]
#[diag(codegen_gcc_unstable_ctarget_feature)]
#[note]
pub(crate) struct UnstableCTargetFeature<'a> {
pub feature: &'a str,
}

#[derive(Diagnostic)]
#[diag(codegen_gcc_forbidden_ctarget_feature)]
pub(crate) struct ForbiddenCTargetFeature<'a> {
pub feature: &'a str,
}

#[derive(Subdiagnostic)]
pub(crate) enum PossibleFeature<'a> {
#[help(codegen_gcc_possible_feature)]
Expand Down
65 changes: 40 additions & 25 deletions src/gcc_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ use rustc_codegen_ssa::errors::TargetFeatureDisableOrEnable;
use rustc_data_structures::fx::FxHashMap;
use rustc_middle::bug;
use rustc_session::Session;
use rustc_target::target_features::RUSTC_SPECIFIC_FEATURES;
use rustc_target::target_features::{RUSTC_SPECIFIC_FEATURES, Stability};
use smallvec::{SmallVec, smallvec};

use crate::errors::{PossibleFeature, UnknownCTargetFeature, UnknownCTargetFeaturePrefix};
use crate::errors::{
ForbiddenCTargetFeature, PossibleFeature, UnknownCTargetFeature, UnknownCTargetFeaturePrefix,
UnstableCTargetFeature,
};

/// The list of GCC features computed from CLI flags (`-Ctarget-cpu`, `-Ctarget-feature`,
/// `--target` and similar).
Expand Down Expand Up @@ -43,7 +46,7 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
);

// -Ctarget-features
let supported_features = sess.target.supported_target_features();
let known_features = sess.target.rust_target_features();
let mut featsmap = FxHashMap::default();
let feats = sess
.opts
Expand All @@ -62,37 +65,49 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
}
};

// Get the backend feature name, if any.
// This excludes rustc-specific features, that do not get passed down to GCC.
let feature = backend_feature_name(s)?;
// Warn against use of GCC 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 gcc_features = to_gcc_features(sess, rust_feature);
if gcc_features.contains(&feature) && !gcc_features.contains(&rust_feature) {
Some(rust_feature)
} else {
None
if diagnostics {
let feature_state = known_features.iter().find(|&&(v, _, _)| v == feature);
match feature_state {
None => {
let rust_feature =
known_features.iter().find_map(|&(rust_feature, _, _)| {
let gcc_features = to_gcc_features(sess, rust_feature);
if gcc_features.contains(&feature)
&& !gcc_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 {
UnknownCTargetFeature { feature, rust_feature: PossibleFeature::None }
};
sess.dcx().emit_warn(unknown_feature);
}
});
let unknown_feature = if let Some(rust_feature) = rust_feature {
UnknownCTargetFeature {
feature,
rust_feature: PossibleFeature::Some { rust_feature },
Some((_, Stability::Stable, _)) => {}
Some((_, Stability::Unstable(_), _)) => {
// An unstable feature. Warn about using it.
sess.dcx().emit_warn(UnstableCTargetFeature { feature });
}
} else {
UnknownCTargetFeature { feature, rust_feature: PossibleFeature::None }
};
sess.dcx().emit_warn(unknown_feature);
}
Some((_, Stability::Forbidden { .. }, _)) => {
sess.dcx().emit_err(ForbiddenCTargetFeature { feature });
}
}

if diagnostics {
// FIXME(nagisa): figure out how to not allocate a full hashset here.
featsmap.insert(feature, enable_disable == '+');
}

// rustc-specific features do not get passed down to GCC…
if RUSTC_SPECIFIC_FEATURES.contains(&feature) {
return None;
}
// ... otherwise though we run through `to_gcc_features` when
// passing requests down to GCC. This means that all in-language
// features also work on the command line instead of having two
Expand Down
3 changes: 2 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,8 +491,9 @@ pub fn target_features(
) -> Vec<Symbol> {
// TODO(antoyo): use global_gcc_features.
sess.target
.supported_target_features()
.rust_target_features()
.iter()
.filter(|(_, gate, _)| gate.is_supported())
.filter_map(|&(feature, gate, _)| {
if sess.is_nightly_build() || allow_unstable || gate.is_stable() {
Some(feature)
Expand Down

0 comments on commit 92c27e8

Please sign in to comment.