From b995dc944ce60087d77accd091e6ffb103030603 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Mon, 31 Jan 2022 21:47:07 +0200 Subject: [PATCH] No branch protection metadata unless enabled Even if we emit metadata disabling branch protection, this metadata may conflict with other modules (e.g. during LTO) that have different branch protection metadata set. This is an unstable flag and feature, so ideally the flag not being specified should act as if the feature wasn't implemented in the first place. Additionally this PR also ensures we emit an error if `-Zbranch-protection` is set on targets other than the supported aarch64. For now the error is being output from codegen, but ideally it should be moved to earlier in the pipeline before stabilization. --- compiler/rustc_codegen_llvm/src/context.rs | 60 +++++++++---------- compiler/rustc_interface/src/tests.rs | 5 +- compiler/rustc_session/src/lib.rs | 1 + compiler/rustc_session/src/options.rs | 5 +- src/test/codegen/branch-protection.rs | 47 ++++++++------- ...rotection-missing-pac-ret.BADFLAGS.stderr} | 0 ...rotection-missing-pac-ret.BADTARGET.stderr | 4 ++ .../branch-protection-missing-pac-ret.rs | 15 ++++- 8 files changed, 82 insertions(+), 55 deletions(-) rename src/test/ui/invalid-compile-flags/{branch-protection-missing-pac-ret.stderr => branch-protection-missing-pac-ret.BADFLAGS.stderr} (100%) create mode 100644 src/test/ui/invalid-compile-flags/branch-protection-missing-pac-ret.BADTARGET.stderr diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index 90ddf791450..ddc8d72e9bf 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -269,36 +269,36 @@ pub unsafe fn create_module<'ll>( } } - if sess.target.arch == "aarch64" { - let BranchProtection { bti, pac_ret: pac } = sess.opts.debugging_opts.branch_protection; - - llvm::LLVMRustAddModuleFlag( - llmod, - llvm::LLVMModFlagBehavior::Error, - "branch-target-enforcement\0".as_ptr().cast(), - bti.into(), - ); - - llvm::LLVMRustAddModuleFlag( - llmod, - llvm::LLVMModFlagBehavior::Error, - "sign-return-address\0".as_ptr().cast(), - pac.is_some().into(), - ); - let pac_opts = pac.unwrap_or(PacRet { leaf: false, key: PAuthKey::A }); - llvm::LLVMRustAddModuleFlag( - llmod, - llvm::LLVMModFlagBehavior::Error, - "sign-return-address-all\0".as_ptr().cast(), - pac_opts.leaf.into(), - ); - let is_bkey: bool = pac_opts.key != PAuthKey::A; - llvm::LLVMRustAddModuleFlag( - llmod, - llvm::LLVMModFlagBehavior::Error, - "sign-return-address-with-bkey\0".as_ptr().cast(), - is_bkey.into(), - ); + if let Some(BranchProtection { bti, pac_ret }) = sess.opts.debugging_opts.branch_protection { + if sess.target.arch != "aarch64" { + sess.err("-Zbranch-protection is only supported on aarch64"); + } else { + llvm::LLVMRustAddModuleFlag( + llmod, + llvm::LLVMModFlagBehavior::Error, + "branch-target-enforcement\0".as_ptr().cast(), + bti.into(), + ); + llvm::LLVMRustAddModuleFlag( + llmod, + llvm::LLVMModFlagBehavior::Error, + "sign-return-address\0".as_ptr().cast(), + pac_ret.is_some().into(), + ); + let pac_opts = pac_ret.unwrap_or(PacRet { leaf: false, key: PAuthKey::A }); + llvm::LLVMRustAddModuleFlag( + llmod, + llvm::LLVMModFlagBehavior::Error, + "sign-return-address-all\0".as_ptr().cast(), + pac_opts.leaf.into(), + ); + llvm::LLVMRustAddModuleFlag( + llmod, + llvm::LLVMModFlagBehavior::Error, + "sign-return-address-with-bkey\0".as_ptr().cast(), + u32::from(pac_opts.key == PAuthKey::B), + ); + } } // Pass on the control-flow protection flags to LLVM (equivalent to `-fcf-protection` in Clang). diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 9ab138c1b12..f9c39f7f83d 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -721,7 +721,10 @@ fn test_debugging_options_tracking_hash() { tracked!(binary_dep_depinfo, true); tracked!( branch_protection, - BranchProtection { bti: true, pac_ret: Some(PacRet { leaf: true, key: PAuthKey::B }) } + Some(BranchProtection { + bti: true, + pac_ret: Some(PacRet { leaf: true, key: PAuthKey::B }) + }) ); tracked!(chalk, true); tracked!(codegen_backend, Some("abc".to_string())); diff --git a/compiler/rustc_session/src/lib.rs b/compiler/rustc_session/src/lib.rs index 383250cd68f..c7a6ac1d4ee 100644 --- a/compiler/rustc_session/src/lib.rs +++ b/compiler/rustc_session/src/lib.rs @@ -2,6 +2,7 @@ #![feature(derive_default_enum)] #![feature(min_specialization)] #![feature(once_cell)] +#![feature(option_get_or_insert_default)] #![recursion_limit = "256"] #![cfg_attr(not(bootstrap), allow(rustc::potential_query_instability))] diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 0a4bd23937d..658b07cced8 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -984,9 +984,10 @@ mod parse { true } - crate fn parse_branch_protection(slot: &mut BranchProtection, v: Option<&str>) -> bool { + crate fn parse_branch_protection(slot: &mut Option, v: Option<&str>) -> bool { match v { Some(s) => { + let slot = slot.get_or_insert_default(); for opt in s.split(',') { match opt { "bti" => slot.bti = true, @@ -1161,7 +1162,7 @@ options! { (default: no)"), borrowck: String = ("migrate".to_string(), parse_string, [UNTRACKED], "select which borrowck is used (`mir` or `migrate`) (default: `migrate`)"), - branch_protection: BranchProtection = (BranchProtection::default(), parse_branch_protection, [TRACKED], + branch_protection: Option = (None, parse_branch_protection, [TRACKED], "set options for branch target identification and pointer authentication on AArch64"), cf_protection: CFProtection = (CFProtection::None, parse_cfprotection, [TRACKED], "instrument control-flow architecture protection"), diff --git a/src/test/codegen/branch-protection.rs b/src/test/codegen/branch-protection.rs index 106c9b148ee..b23073778c0 100644 --- a/src/test/codegen/branch-protection.rs +++ b/src/test/codegen/branch-protection.rs @@ -1,12 +1,12 @@ // Test that the correct module flags are emitted with different branch protection flags. -// revisions: bti pac-ret leaf b-key +// revisions: BTI PACRET LEAF BKEY NONE // min-llvm-version: 12.0.0 // needs-llvm-components: aarch64 -// [bti] compile-flags: -Z branch-protection=bti -// [pac-ret] compile-flags: -Z branch-protection=pac-ret -// [leaf] compile-flags: -Z branch-protection=pac-ret,leaf -// [b-key] compile-flags: -Z branch-protection=pac-ret,b-key +// [BTI] compile-flags: -Z branch-protection=bti +// [PACRET] compile-flags: -Z branch-protection=pac-ret +// [LEAF] compile-flags: -Z branch-protection=pac-ret,leaf +// [BKEY] compile-flags: -Z branch-protection=pac-ret,b-key // compile-flags: --target aarch64-unknown-linux-gnu #![crate_type = "lib"] @@ -20,22 +20,27 @@ trait Sized { } pub fn test() { } -// bti: !"branch-target-enforcement", i32 1 -// bti: !"sign-return-address", i32 0 -// bti: !"sign-return-address-all", i32 0 -// bti: !"sign-return-address-with-bkey", i32 0 +// BTI: !"branch-target-enforcement", i32 1 +// BTI: !"sign-return-address", i32 0 +// BTI: !"sign-return-address-all", i32 0 +// BTI: !"sign-return-address-with-bkey", i32 0 -// pac-ret: !"branch-target-enforcement", i32 0 -// pac-ret: !"sign-return-address", i32 1 -// pac-ret: !"sign-return-address-all", i32 0 -// pac-ret: !"sign-return-address-with-bkey", i32 0 +// PACRET: !"branch-target-enforcement", i32 0 +// PACRET: !"sign-return-address", i32 1 +// PACRET: !"sign-return-address-all", i32 0 +// PACRET: !"sign-return-address-with-bkey", i32 0 -// leaf: !"branch-target-enforcement", i32 0 -// leaf: !"sign-return-address", i32 1 -// leaf: !"sign-return-address-all", i32 1 -// leaf: !"sign-return-address-with-bkey", i32 0 +// LEAF: !"branch-target-enforcement", i32 0 +// LEAF: !"sign-return-address", i32 1 +// LEAF: !"sign-return-address-all", i32 1 +// LEAF: !"sign-return-address-with-bkey", i32 0 -// b-key: !"branch-target-enforcement", i32 0 -// b-key: !"sign-return-address", i32 1 -// b-key: !"sign-return-address-all", i32 0 -// b-key: !"sign-return-address-with-bkey", i32 1 +// BKEY: !"branch-target-enforcement", i32 0 +// BKEY: !"sign-return-address", i32 1 +// BKEY: !"sign-return-address-all", i32 0 +// BKEY: !"sign-return-address-with-bkey", i32 1 + +// NONE-NOT: branch-target-enforcement +// NONE-NOT: sign-return-address +// NONE-NOT: sign-return-address-all +// NONE-NOT: sign-return-address-with-bkey diff --git a/src/test/ui/invalid-compile-flags/branch-protection-missing-pac-ret.stderr b/src/test/ui/invalid-compile-flags/branch-protection-missing-pac-ret.BADFLAGS.stderr similarity index 100% rename from src/test/ui/invalid-compile-flags/branch-protection-missing-pac-ret.stderr rename to src/test/ui/invalid-compile-flags/branch-protection-missing-pac-ret.BADFLAGS.stderr diff --git a/src/test/ui/invalid-compile-flags/branch-protection-missing-pac-ret.BADTARGET.stderr b/src/test/ui/invalid-compile-flags/branch-protection-missing-pac-ret.BADTARGET.stderr new file mode 100644 index 00000000000..6bd9c6a0276 --- /dev/null +++ b/src/test/ui/invalid-compile-flags/branch-protection-missing-pac-ret.BADTARGET.stderr @@ -0,0 +1,4 @@ +error: -Zbranch-protection is only supported on aarch64 + +error: aborting due to previous error + diff --git a/src/test/ui/invalid-compile-flags/branch-protection-missing-pac-ret.rs b/src/test/ui/invalid-compile-flags/branch-protection-missing-pac-ret.rs index 4f39d223a2e..4bc4919bc93 100644 --- a/src/test/ui/invalid-compile-flags/branch-protection-missing-pac-ret.rs +++ b/src/test/ui/invalid-compile-flags/branch-protection-missing-pac-ret.rs @@ -1 +1,14 @@ -// compile-flags: -Z branch-protection=leaf +// revisions: BADFLAGS BADTARGET +// [BADFLAGS] compile-flags: --target=aarch64-unknown-linux-gnu -Zbranch-protection=leaf +// [BADFLAGS] check-fail +// [BADFLAGS] needs-llvm-components: aarch64 +// [BADTARGET] compile-flags: --target=x86_64-unknown-linux-gnu -Zbranch-protection=bti +// [BADTARGET] build-fail +// [BADTARGET] needs-llvm-components: x86 + +#![crate_type = "lib"] +#![feature(no_core, lang_items)] +#![no_core] + +#[lang="sized"] +trait Sized { }