Skip to content

Commit

Permalink
Rollup merge of #93782 - adamgemmell:dev/adagem01/split-pauth, r=Amanieu
Browse files Browse the repository at this point in the history
Split `pauth` target feature

Per discussion on #86941 we'd like to split `pauth` into `paca` and `pacg` in order to better support possible future environments that only have the keys available for address or generic authentication. At the moment LLVM has the one `pauth` target_feature while Linux presents separate `paca` and `pacg` flags for feature detection.

Because the use of [target_feature](https://rust-lang.github.io/rfcs/2045-target-feature.html) will "allow the compiler to generate code under the assumption that this code will only be reached in hosts that support the feature", it does not make sense to simply translate `paca` into the LLVM feature `pauth`, as it will generate code as if `pacg` is available.

To accommodate this we error if only one of the two features is present. If LLVM splits them in the future we can remove this restriction without making a breaking change.

r? ```@Amanieu```
  • Loading branch information
matthiaskrgr authored Feb 11, 2022
2 parents ffa8d6b + d39a637 commit 13d636d
Show file tree
Hide file tree
Showing 9 changed files with 148 additions and 15 deletions.
31 changes: 26 additions & 5 deletions compiler/rustc_codegen_llvm/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,12 +322,33 @@ pub fn from_fn_attrs<'ll, 'tcx>(
// The target doesn't care; the subtarget reads our attribute.
apply_tune_cpu_attr(cx, llfn);

let mut function_features = codegen_fn_attrs
.target_features
let function_features =
codegen_fn_attrs.target_features.iter().map(|f| f.as_str()).collect::<Vec<&str>>();

if let Some(f) = llvm_util::check_tied_features(
cx.tcx.sess,
&function_features.iter().map(|f| (*f, true)).collect(),
) {
let span = cx
.tcx
.get_attrs(instance.def_id())
.iter()
.find(|a| a.has_name(rustc_span::sym::target_feature))
.map_or_else(|| cx.tcx.def_span(instance.def_id()), |a| a.span);
let msg = format!(
"the target features {} must all be either enabled or disabled together",
f.join(", ")
);
let mut err = cx.tcx.sess.struct_span_err(span, &msg);
err.help("add the missing features in a `target_feature` attribute");
err.emit();
return;
}

let mut function_features = function_features
.iter()
.flat_map(|f| {
let feature = f.as_str();
llvm_util::to_llvm_feature(cx.tcx.sess, feature)
.flat_map(|feat| {
llvm_util::to_llvm_feature(cx.tcx.sess, feat)
.into_iter()
.map(|f| format!("+{}", f))
.collect::<Vec<String>>()
Expand Down
49 changes: 41 additions & 8 deletions compiler/rustc_codegen_llvm/src/llvm_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use crate::back::write::create_informational_target_machine;
use crate::{llvm, llvm_util};
use libc::c_int;
use libloading::Library;
use rustc_codegen_ssa::target_features::supported_target_features;
use rustc_data_structures::fx::FxHashSet;
use rustc_codegen_ssa::target_features::{supported_target_features, tied_target_features};
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_fs_util::path_to_c_string;
use rustc_middle::bug;
use rustc_session::config::PrintRequest;
Expand Down Expand Up @@ -191,10 +191,30 @@ pub fn to_llvm_feature<'a>(sess: &Session, s: &'a str) -> Vec<&'a str> {
("aarch64", "frintts") => vec!["fptoint"],
("aarch64", "fcma") => vec!["complxnum"],
("aarch64", "pmuv3") => vec!["perfmon"],
("aarch64", "paca") => vec!["pauth"],
("aarch64", "pacg") => vec!["pauth"],
(_, s) => vec![s],
}
}

// Given a map from target_features to whether they are enabled or disabled,
// ensure only valid combinations are allowed.
pub fn check_tied_features(
sess: &Session,
features: &FxHashMap<&str, bool>,
) -> Option<&'static [&'static str]> {
for tied in tied_target_features(sess) {
// Tied features must be set to the same value, or not set at all
let mut tied_iter = tied.iter();
let enabled = features.get(tied_iter.next().unwrap());

if tied_iter.any(|f| enabled != features.get(f)) {
return Some(tied);
}
}
None
}

pub fn target_features(sess: &Session) -> Vec<Symbol> {
let target_machine = create_informational_target_machine(sess);
supported_target_features(sess)
Expand Down Expand Up @@ -395,15 +415,19 @@ pub fn llvm_global_features(sess: &Session) -> Vec<String> {
Some(_) | None => {}
};

fn strip(s: &str) -> &str {
s.strip_prefix(&['+', '-']).unwrap_or(s)
}

let filter = |s: &str| {
if s.is_empty() {
return vec![];
}
let feature = if s.starts_with('+') || s.starts_with('-') {
&s[1..]
} else {
let feature = strip(s);
if feature == s {
return vec![s.to_string()];
};
}

// Rustc-specific feature requests like `+crt-static` or `-crt-static`
// are not passed down to LLVM.
if RUSTC_SPECIFIC_FEATURES.contains(&feature) {
Expand All @@ -420,8 +444,17 @@ pub fn llvm_global_features(sess: &Session) -> Vec<String> {
features.extend(sess.target.features.split(',').flat_map(&filter));

// -Ctarget-features
features.extend(sess.opts.cg.target_feature.split(',').flat_map(&filter));

let feats: Vec<&str> = sess.opts.cg.target_feature.split(',').collect();
// LLVM enables based on the last occurence of a feature
if let Some(f) =
check_tied_features(sess, &feats.iter().map(|f| (strip(f), !f.starts_with("-"))).collect())
{
sess.err(&format!(
"Target features {} must all be enabled or disabled together",
f.join(", ")
));
}
features.extend(feats.iter().flat_map(|&f| filter(f)));
features
}

Expand Down
15 changes: 13 additions & 2 deletions compiler/rustc_codegen_ssa/src/target_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,10 @@ const AARCH64_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[
("ssbs", Some(sym::aarch64_target_feature)),
// FEAT_SB
("sb", Some(sym::aarch64_target_feature)),
// FEAT_PAUTH
("pauth", Some(sym::aarch64_target_feature)),
// FEAT_PAUTH (address authentication)
("paca", Some(sym::aarch64_target_feature)),
// FEAT_PAUTH (generic authentication)
("pacg", Some(sym::aarch64_target_feature)),
// FEAT_DPB
("dpb", Some(sym::aarch64_target_feature)),
// FEAT_DPB2
Expand Down Expand Up @@ -137,6 +139,8 @@ const AARCH64_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[
("v8.7a", Some(sym::aarch64_target_feature)),
];

const AARCH64_TIED_FEATURES: &[&[&str]] = &[&["paca", "pacg"]];

const X86_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[
("adx", Some(sym::adx_target_feature)),
("aes", None),
Expand Down Expand Up @@ -256,6 +260,13 @@ pub fn supported_target_features(sess: &Session) -> &'static [(&'static str, Opt
}
}

pub fn tied_target_features(sess: &Session) -> &'static [&'static [&'static str]] {
match &*sess.target.arch {
"aarch64" => AARCH64_TIED_FEATURES,
_ => &[],
}
}

pub(crate) fn provide(providers: &mut Providers) {
providers.supported_target_features = |tcx, cnum| {
assert_eq!(cnum, LOCAL_CRATE);
Expand Down
4 changes: 4 additions & 0 deletions src/test/ui/target-feature/tied-features-cli.one.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
error: Target features paca, pacg must all be enabled or disabled together

error: aborting due to previous error

9 changes: 9 additions & 0 deletions src/test/ui/target-feature/tied-features-cli.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// only-aarch64
// revisions: one two three four
//[one] compile-flags: -C target-feature=+paca
//[two] compile-flags: -C target-feature=-pacg,+pacg
//[three] compile-flags: -C target-feature=+paca,+pacg,-paca
//[four] check-pass
//[four] compile-flags: -C target-feature=-paca,+pacg -C target-feature=+paca

fn main() {}
4 changes: 4 additions & 0 deletions src/test/ui/target-feature/tied-features-cli.three.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
error: Target features paca, pacg must all be enabled or disabled together

error: aborting due to previous error

4 changes: 4 additions & 0 deletions src/test/ui/target-feature/tied-features-cli.two.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
error: Target features paca, pacg must all be enabled or disabled together

error: aborting due to previous error

29 changes: 29 additions & 0 deletions src/test/ui/target-feature/tied-features.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// only-aarch64
// build-fail

#![feature(aarch64_target_feature, target_feature_11)]

fn main() {
#[target_feature(enable = "pacg")]
//~^ ERROR must all be either enabled or disabled together
unsafe fn inner() {}

unsafe {
foo();
bar();
baz();
inner();
}
}

#[target_feature(enable = "paca")]
//~^ ERROR must all be either enabled or disabled together
unsafe fn foo() {}


#[target_feature(enable = "paca,pacg")]
unsafe fn bar() {}

#[target_feature(enable = "paca")]
#[target_feature(enable = "pacg")]
unsafe fn baz() {}
18 changes: 18 additions & 0 deletions src/test/ui/target-feature/tied-features.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error: the target features paca, pacg must all be either enabled or disabled together
--> $DIR/tied-features.rs:7:5
|
LL | #[target_feature(enable = "pacg")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: add the missing features in a `target_feature` attribute

error: the target features paca, pacg must all be either enabled or disabled together
--> $DIR/tied-features.rs:19:1
|
LL | #[target_feature(enable = "paca")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: add the missing features in a `target_feature` attribute

error: aborting due to 2 previous errors

0 comments on commit 13d636d

Please sign in to comment.