about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2022-02-11 21:48:48 +0100
committerGitHub <noreply@github.com>2022-02-11 21:48:48 +0100
commit13d636dff2a25b64ec8b7d661ef1ca5b9956a946 (patch)
treeed940d5318610d3b8fa592224226604012679424
parentffa8d6b47d2b3f7e8931eae7d07f87e3585597b8 (diff)
parentd39a6377e9cfa108eb181ea892366e6b01fd753f (diff)
downloadrust-13d636dff2a25b64ec8b7d661ef1ca5b9956a946.tar.gz
rust-13d636dff2a25b64ec8b7d661ef1ca5b9956a946.zip
Rollup merge of #93782 - adamgemmell:dev/adagem01/split-pauth, r=Amanieu
Split `pauth` target feature

Per discussion on https://github.com/rust-lang/rust/issues/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```
-rw-r--r--compiler/rustc_codegen_llvm/src/attributes.rs31
-rw-r--r--compiler/rustc_codegen_llvm/src/llvm_util.rs49
-rw-r--r--compiler/rustc_codegen_ssa/src/target_features.rs15
-rw-r--r--src/test/ui/target-feature/tied-features-cli.one.stderr4
-rw-r--r--src/test/ui/target-feature/tied-features-cli.rs9
-rw-r--r--src/test/ui/target-feature/tied-features-cli.three.stderr4
-rw-r--r--src/test/ui/target-feature/tied-features-cli.two.stderr4
-rw-r--r--src/test/ui/target-feature/tied-features.rs29
-rw-r--r--src/test/ui/target-feature/tied-features.stderr18
9 files changed, 148 insertions, 15 deletions
diff --git a/compiler/rustc_codegen_llvm/src/attributes.rs b/compiler/rustc_codegen_llvm/src/attributes.rs
index 7f82ce307d5..2472789601e 100644
--- a/compiler/rustc_codegen_llvm/src/attributes.rs
+++ b/compiler/rustc_codegen_llvm/src/attributes.rs
@@ -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>>()
diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs
index f91fad2d9c9..727d079d83d 100644
--- a/compiler/rustc_codegen_llvm/src/llvm_util.rs
+++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs
@@ -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;
@@ -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)
@@ -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) {
@@ -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
 }
 
diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs
index 63cc6faf9ec..f31b0ee592e 100644
--- a/compiler/rustc_codegen_ssa/src/target_features.rs
+++ b/compiler/rustc_codegen_ssa/src/target_features.rs
@@ -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
@@ -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),
@@ -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);
diff --git a/src/test/ui/target-feature/tied-features-cli.one.stderr b/src/test/ui/target-feature/tied-features-cli.one.stderr
new file mode 100644
index 00000000000..2bc64a76aae
--- /dev/null
+++ b/src/test/ui/target-feature/tied-features-cli.one.stderr
@@ -0,0 +1,4 @@
+error: Target features paca, pacg must all be enabled or disabled together
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/target-feature/tied-features-cli.rs b/src/test/ui/target-feature/tied-features-cli.rs
new file mode 100644
index 00000000000..ea09d4fc460
--- /dev/null
+++ b/src/test/ui/target-feature/tied-features-cli.rs
@@ -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() {}
diff --git a/src/test/ui/target-feature/tied-features-cli.three.stderr b/src/test/ui/target-feature/tied-features-cli.three.stderr
new file mode 100644
index 00000000000..2bc64a76aae
--- /dev/null
+++ b/src/test/ui/target-feature/tied-features-cli.three.stderr
@@ -0,0 +1,4 @@
+error: Target features paca, pacg must all be enabled or disabled together
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/target-feature/tied-features-cli.two.stderr b/src/test/ui/target-feature/tied-features-cli.two.stderr
new file mode 100644
index 00000000000..2bc64a76aae
--- /dev/null
+++ b/src/test/ui/target-feature/tied-features-cli.two.stderr
@@ -0,0 +1,4 @@
+error: Target features paca, pacg must all be enabled or disabled together
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/target-feature/tied-features.rs b/src/test/ui/target-feature/tied-features.rs
new file mode 100644
index 00000000000..86400361db3
--- /dev/null
+++ b/src/test/ui/target-feature/tied-features.rs
@@ -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() {}
diff --git a/src/test/ui/target-feature/tied-features.stderr b/src/test/ui/target-feature/tied-features.stderr
new file mode 100644
index 00000000000..0b1460e0b75
--- /dev/null
+++ b/src/test/ui/target-feature/tied-features.stderr
@@ -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
+