From 1481721c9d80e5fe61de4b1aa35090191da5144a Mon Sep 17 00:00:00 2001 From: Anton Kirilov Date: Wed, 17 Aug 2022 21:06:20 +0100 Subject: [PATCH] Enable back-edge CFI by default on macOS (#4720) Also, adjust the tests that are executed on that platform. Finally, fix a bug with obtaining backtraces when back-edge CFI is enabled. Copyright (c) 2022, Arm Limited. --- cranelift/codegen/src/isa/aarch64/inst.isle | 15 ++++++++++ .../codegen/src/isa/aarch64/inst/emit.rs | 1 + .../src/isa/aarch64/inst/emit_tests.rs | 1 + cranelift/codegen/src/isa/aarch64/inst/mod.rs | 13 ++++----- .../codegen/src/isa/aarch64/lower/isle.rs | 8 ++++++ cranelift/native/src/lib.rs | 3 ++ crates/fiber/src/lib.rs | 2 ++ crates/fiber/src/unix/aarch64.rs | 28 ++++++++----------- .../src/generators/codegen_settings.rs | 10 +++++++ 9 files changed, 58 insertions(+), 23 deletions(-) diff --git a/cranelift/codegen/src/isa/aarch64/inst.isle b/cranelift/codegen/src/isa/aarch64/inst.isle index 1f8122188d..61f164a3ac 100644 --- a/cranelift/codegen/src/isa/aarch64/inst.isle +++ b/cranelift/codegen/src/isa/aarch64/inst.isle @@ -780,6 +780,11 @@ (Pacisp (key APIKey)) + ;; Strip pointer authentication code from instruction address in LR; + ;; equivalent to a no-op if Pointer authentication (FEAT_PAuth) is not + ;; supported. + (Xpaclri) + ;; Marker, no-op in generated code: SP "virtual offset" is adjusted. This ;; controls how AMode::NominalSPOffset args are lowered. (VirtualSPOffsetAdj @@ -1356,6 +1361,9 @@ )) ;; Extractors for target features ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +(decl pure sign_return_address_disabled () Unit) +(extern constructor sign_return_address_disabled sign_return_address_disabled) + (decl use_lse () Inst) (extern extractor use_lse use_lse) @@ -2577,4 +2585,11 @@ (decl aarch64_link () Reg) (rule (aarch64_link) + (if (sign_return_address_disabled)) (mov_preg (preg_link))) + +(rule (aarch64_link) + ;; This constructor is always used for non-leaf functions, so it is safe + ;; to clobber LR. + (let ((_ Unit (emit (MInst.Xpaclri)))) + (mov_preg (preg_link)))) diff --git a/cranelift/codegen/src/isa/aarch64/inst/emit.rs b/cranelift/codegen/src/isa/aarch64/inst/emit.rs index 3c4baa63f9..22a6473bf8 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/emit.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/emit.rs @@ -3121,6 +3121,7 @@ impl MachInstEmit for Inst { sink.put4(0xd503233f | key << 6); } + &Inst::Xpaclri => sink.put4(0xd50320ff), &Inst::VirtualSPOffsetAdj { offset } => { trace!( "virtual sp offset adjusted by {} -> {}", diff --git a/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs b/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs index daa33fed46..2db4a70fa7 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs @@ -57,6 +57,7 @@ fn test_aarch64_binemit() { "retab", )); insns.push((Inst::Pacisp { key: APIKey::B }, "7F2303D5", "pacibsp")); + insns.push((Inst::Xpaclri, "FF2003D5", "xpaclri")); insns.push((Inst::Nop0, "", "nop-zero-len")); insns.push((Inst::Nop4, "1F2003D5", "nop")); insns.push((Inst::Csdb, "9F2203D5", "csdb")); diff --git a/cranelift/codegen/src/isa/aarch64/inst/mod.rs b/cranelift/codegen/src/isa/aarch64/inst/mod.rs index fb4f4e3945..e9e66af7a0 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/mod.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/mod.rs @@ -981,12 +981,7 @@ fn aarch64_get_operands VReg>(inst: &Inst, collector: &mut Operan collector.reg_def(rd); collector.reg_use(rn); } - &Inst::Ret { ref rets } => { - for &ret in rets { - collector.reg_use(ret); - } - } - &Inst::AuthenticatedRet { ref rets, .. } => { + &Inst::Ret { ref rets } | &Inst::AuthenticatedRet { ref rets, .. } => { for &ret in rets { collector.reg_use(ret); } @@ -1039,7 +1034,10 @@ fn aarch64_get_operands VReg>(inst: &Inst, collector: &mut Operan collector.reg_def(rd); memarg_operands(mem, collector); } - &Inst::Pacisp { .. } => {} + &Inst::Pacisp { .. } | &Inst::Xpaclri => { + // Neither LR nor SP is an allocatable register, so there is no need + // to do anything. + } &Inst::VirtualSPOffsetAdj { .. } => {} &Inst::ElfTlsGetAddr { .. } => { @@ -2715,6 +2713,7 @@ impl Inst { "paci".to_string() + key + "sp" } + &Inst::Xpaclri => "xpaclri".to_string(), &Inst::VirtualSPOffsetAdj { offset } => { state.virtual_sp_offset += offset; format!("virtual_sp_offset_adjust {}", offset) diff --git a/cranelift/codegen/src/isa/aarch64/lower/isle.rs b/cranelift/codegen/src/isa/aarch64/lower/isle.rs index 49ea91e108..a0252e5dbe 100644 --- a/cranelift/codegen/src/isa/aarch64/lower/isle.rs +++ b/cranelift/codegen/src/isa/aarch64/lower/isle.rs @@ -71,6 +71,14 @@ pub struct SinkableAtomicLoad { impl generated_code::Context for IsleContext<'_, '_, MInst, Flags, IsaFlags, 6> { isle_prelude_methods!(); + fn sign_return_address_disabled(&mut self) -> Option<()> { + if self.isa_flags.sign_return_address() { + None + } else { + Some(()) + } + } + fn use_lse(&mut self, _: Inst) -> Option<()> { if self.isa_flags.use_lse() { Some(()) diff --git a/cranelift/native/src/lib.rs b/cranelift/native/src/lib.rs index a21dcb450c..c9ca7a2882 100644 --- a/cranelift/native/src/lib.rs +++ b/cranelift/native/src/lib.rs @@ -138,6 +138,9 @@ pub fn builder_with_options(infer_native_flags: bool) -> Result (""); } - macro_rules! pacia1716 { () => (""); } - macro_rules! paciasp { () => (""); } - macro_rules! autiasp { () => (""); } + macro_rules! paci1716 { () => ("pacib1716\n"); } + macro_rules! pacisp { () => ("pacibsp\n"); } + macro_rules! autisp { () => ("autibsp\n"); } macro_rules! sym_adrp { ($s:tt) => (concat!("_", $s, "@PAGE")); } macro_rules! sym_add { ($s:tt) => (concat!("_", $s, "@PAGEOFF")); } } else { - macro_rules! cfi_window_save { () => (".cfi_window_save\n"); } - macro_rules! pacia1716 { () => ("pacia1716\n"); } - macro_rules! paciasp { () => ("paciasp\n"); } - macro_rules! autiasp { () => ("autiasp\n"); } + macro_rules! paci1716 { () => ("pacia1716\n"); } + macro_rules! pacisp { () => ("paciasp\n"); } + macro_rules! autisp { () => ("autiasp\n"); } macro_rules! sym_adrp { ($s:tt) => (concat!($s, "")); } macro_rules! sym_add { ($s:tt) => (concat!(":lo12:", $s)); } } @@ -44,9 +42,9 @@ asm_func!( " .cfi_startproc ", - paciasp!(), - cfi_window_save!(), + pacisp!(), " + .cfi_window_save // Save all callee-saved registers on the stack since we're // assuming they're clobbered as a result of the stack switch. stp x29, x30, [sp, -16]! @@ -81,9 +79,9 @@ asm_func!( ldp x20, x19, [sp], 16 ldp x29, x30, [sp], 16 ", - autiasp!(), - cfi_window_save!(), + autisp!(), " + .cfi_window_save ret .cfi_endproc ", @@ -121,7 +119,7 @@ asm_func!( adrp x17, ", sym_adrp!("wasmtime_fiber_start"), " add x17, x17, ", sym_add!("wasmtime_fiber_start"), " ", - pacia1716!(), + paci1716!(), " str x17, [x16, -0x8] // x17 => lr str x0, [x16, -0x18] // x0 => x19 @@ -151,9 +149,7 @@ asm_func!( 0x23, 0xa0, 0x1 /* DW_OP_plus_uconst 0xa0 */ .cfi_rel_offset x29, -0x10 .cfi_rel_offset x30, -0x08 - ", - cfi_window_save!(), - " + .cfi_window_save .cfi_rel_offset x19, -0x18 .cfi_rel_offset x20, -0x20 .cfi_rel_offset x21, -0x28 diff --git a/crates/fuzzing/src/generators/codegen_settings.rs b/crates/fuzzing/src/generators/codegen_settings.rs index 7678006871..8c2ef4476e 100644 --- a/crates/fuzzing/src/generators/codegen_settings.rs +++ b/crates/fuzzing/src/generators/codegen_settings.rs @@ -128,6 +128,16 @@ impl<'a> Arbitrary<'a> for CodegenSettings { test: is_aarch64_feature_detected, std: "lse" => clif: "has_lse", + // even though the natural correspondence seems to be + // between "paca" and "has_pauth", the latter has no effect + // in isolation, so we actually use the setting that affects + // code generation + std: "paca" => clif: "sign_return_address", + // "paca" and "pacg" check for the same underlying + // architectural feature, so we use the latter to cover more + // code generation settings, of which we have chosen the one + // with the most significant effect + std: "pacg" => clif: "sign_return_address_all" ratio: 1 in 2, }, }; return Ok(CodegenSettings::Target {