bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #117616 - RalfJung:unstable-target-features, r=compiler-errors

Browse source 
warn when using an unstable feature with -Ctarget-feature

Setting or unsetting the wrong target features can cause ABI incompatibility (https://github.com/rust-lang/rust/issues/116344, https://github.com/rust-lang/rust/issues/116558). We need to carefully audit features for their ABI impact before stabilization. I just learned that we currently accept arbitrary unstable features on stable and if they are in the list of Rust target features, even unstable, then we don't even warn about that!1 That doesn't seem great, so I propose we introduce a warning here.

This has an obvious loophole via `-Ctarget-cpu`. I'm not sure how to best deal with that, but it seems better to fix what we can and think about the other cases later, maybe once we have a better idea for how to resolve the general mess that are ABI-affecting target features.
This commit is contained in:
Matthias Krüger 2023-11-07 19:29:56 +01:00 • committed by GitHub
commit f8c67704f2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 70 additions and 23 deletions
Download patch file Download diff file Expand all files Collapse all files

8
compiler/rustc_codegen_llvm/messages.ftl
View file

@ -76,8 +76,8 @@ codegen_llvm_target_machine = could not create LLVM TargetMachine for triple: {$
codegen_llvm_target_machine_with_llvm_err = could not create LLVM TargetMachine for triple: {$triple}: {$llvm_err}
codegen_llvm_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
@ -87,6 +87,10 @@ codegen_llvm_unknown_ctarget_feature_prefix =
codegen_llvm_unknown_debuginfo_compression = unknown debuginfo compression algorithm {$algorithm} - will fall back to uncompressed debuginfo
codegen_llvm_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_llvm_write_bytecode = failed to write bytecode to {$path}: {$err}
codegen_llvm_write_ir = failed to write LLVM IR to {$path}

7
compiler/rustc_codegen_llvm/src/errors.rs
View file

@ -26,6 +26,13 @@ pub(crate) struct UnknownCTargetFeature<'a> {
pub rust_feature: PossibleFeature<'a>,
}
#[derive(Diagnostic)]
#[diag(codegen_llvm_unstable_ctarget_feature)]
#[note]
pub(crate) struct UnstableCTargetFeature<'a> {
pub feature: &'a str,
}
#[derive(Subdiagnostic)]
pub(crate) enum PossibleFeature<'a> {
#[help(codegen_llvm_possible_feature)]

47
compiler/rustc_codegen_llvm/src/llvm_util.rs
View file

@ -1,7 +1,7 @@
use crate::back::write::create_informational_target_machine;
use crate::errors::{
PossibleFeature, TargetFeatureDisableOrEnable, UnknownCTargetFeature,
UnknownCTargetFeaturePrefix,
UnknownCTargetFeaturePrefix, UnstableCTargetFeature,
};
use crate::llvm;
use libc::c_int;
@ -531,25 +531,34 @@ pub(crate) fn global_llvm_features(sess: &Session, diagnostics: bool) -> Vec<Str
};
let feature = backend_feature_name(s)?;
// Warn against use of LLVM 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 llvm_features = to_llvm_features(sess, rust_feature);
if llvm_features.contains(&feature) && !llvm_features.contains(&rust_feature) {
Some(rust_feature)
// Warn against use of LLVM specific feature names and unstable features on the CLI.
if diagnostics {
let feature_state = supported_features.iter().find(|&&(v, _)| v == feature);
if feature_state.is_none() {
let rust_feature = supported_features.iter().find_map(|&(rust_feature, _)| {
let llvm_features = to_llvm_features(sess, rust_feature);
if llvm_features.contains(&feature)
&& !llvm_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 {
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.emit_warning(unknown_feature);
UnknownCTargetFeature { feature, rust_feature: PossibleFeature::None }
};
sess.emit_warning(unknown_feature);
} else if feature_state.is_some_and(|(_name, feature_gate)| feature_gate.is_some())
{
// An unstable feature. Warn about using it.
sess.emit_warning(UnstableCTargetFeature { feature });
}
}
if diagnostics {

9
compiler/rustc_codegen_ssa/src/target_features.rs
View file

@ -23,6 +23,15 @@ pub const RUSTC_SPECIFIC_FEATURES: &[&str] = &["crt-static"];
// check whether they're named already elsewhere in rust
// e.g. in stdarch and whether the given name matches LLVM's
// if it doesn't, to_llvm_feature in llvm_util in rustc_codegen_llvm needs to be adapted
//
// When adding a new feature, be particularly mindful of features that affect function ABIs. Those
// need to be treated very carefully to avoid introducing unsoundness! This often affects features
// that enable/disable hardfloat support (see https://github.com/rust-lang/rust/issues/116344 for an
// example of this going wrong), but features enabling new SIMD registers are also a concern (see
// https://github.com/rust-lang/rust/issues/116558 for an example of this going wrong).
//
// Stabilizing a target feature (setting the 2nd component of the pair to `None`) requires t-lang
// approval.
const ARM_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[
// tidy-alphabetical-start

6
tests/ui/numbers-arithmetic/saturating-float-casts-wasm.stderr Normal file
View file

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

4
tests/ui/target-feature/similar-feature-suggestion.stderr
View file

@ -1,6 +1,6 @@
warning: unknown feature specified for `-Ctarget-feature`: `rdrnd`
warning: unknown and unstable feature specified for `-Ctarget-feature`: `rdrnd`
|
= note: it is still passed through to the codegen backend
= 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
= help: you might have meant: `rdrand`
warning: 1 warning emitted

6
tests/ui/target-feature/unstable-feature.rs Normal file
View file

@ -0,0 +1,6 @@
// compile-flags: -Ctarget-feature=+vaes --crate-type=rlib --target=x86_64-unknown-linux-gnu
// build-pass
// needs-llvm-components: x86
#![feature(no_core)]
#![no_core]

6
tests/ui/target-feature/unstable-feature.stderr Normal file
View file

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