Skip to content

Commit

Permalink
Enable back-edge CFI by default on macOS
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
akirilov-arm committed Aug 17, 2022
1 parent 0f94493 commit ef772b1
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 23 deletions.
15 changes: 15 additions & 0 deletions cranelift/codegen/src/isa/aarch64/inst.isle
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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))))
1 change: 1 addition & 0 deletions cranelift/codegen/src/isa/aarch64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {} -> {}",
Expand Down
1 change: 1 addition & 0 deletions cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down
13 changes: 6 additions & 7 deletions cranelift/codegen/src/isa/aarch64/inst/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -981,12 +981,7 @@ fn aarch64_get_operands<F: Fn(VReg) -> 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);
}
Expand Down Expand Up @@ -1039,7 +1034,10 @@ fn aarch64_get_operands<F: Fn(VReg) -> 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 { .. } => {
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions cranelift/codegen/src/isa/aarch64/lower/isle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
3 changes: 3 additions & 0 deletions cranelift/native/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ pub fn builder_with_options(infer_native_flags: bool) -> Result<isa::Builder, &'
}

if cfg!(target_os = "macos") {
// Pointer authentication is always available on Apple Silicon.
isa_builder.enable("sign_return_address").unwrap();
// macOS enforces the use of the B key for return addresses.
isa_builder.enable("sign_return_address_with_bkey").unwrap();
}
}
Expand Down
2 changes: 2 additions & 0 deletions crates/fiber/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,8 @@ mod tests {
.any(|s| s.contains("look_for_me"))
// TODO: apparently windows unwind routines don't unwind through fibers, so this will always fail. Is there a way we can fix that?
|| cfg!(windows)
// TODO: the system libunwind is broken (#2808)
|| cfg!(all(target_os = "macos", target_arch = "aarch64"))
);
}

Expand Down
28 changes: 12 additions & 16 deletions crates/fiber/src/unix/aarch64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,15 @@ use wasmtime_asm_macros::asm_func;

cfg_if::cfg_if! {
if #[cfg(target_os = "macos")] {
macro_rules! cfi_window_save { () => (""); }
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)); }
}
Expand All @@ -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]!
Expand Down Expand Up @@ -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
",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions crates/fuzzing/src/generators/codegen_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit ef772b1

Please sign in to comment.