Skip to content

Commit 1481721

Browse files
authored
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.
1 parent 57dca93 commit 1481721

File tree

9 files changed

+58
-23
lines changed

9 files changed

+58
-23
lines changed

cranelift/codegen/src/isa/aarch64/inst.isle

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,11 @@
780780
(Pacisp
781781
(key APIKey))
782782

783+
;; Strip pointer authentication code from instruction address in LR;
784+
;; equivalent to a no-op if Pointer authentication (FEAT_PAuth) is not
785+
;; supported.
786+
(Xpaclri)
787+
783788
;; Marker, no-op in generated code: SP "virtual offset" is adjusted. This
784789
;; controls how AMode::NominalSPOffset args are lowered.
785790
(VirtualSPOffsetAdj
@@ -1356,6 +1361,9 @@
13561361
))
13571362

13581363
;; Extractors for target features ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
1364+
(decl pure sign_return_address_disabled () Unit)
1365+
(extern constructor sign_return_address_disabled sign_return_address_disabled)
1366+
13591367
(decl use_lse () Inst)
13601368
(extern extractor use_lse use_lse)
13611369

@@ -2577,4 +2585,11 @@
25772585

25782586
(decl aarch64_link () Reg)
25792587
(rule (aarch64_link)
2588+
(if (sign_return_address_disabled))
25802589
(mov_preg (preg_link)))
2590+
2591+
(rule (aarch64_link)
2592+
;; This constructor is always used for non-leaf functions, so it is safe
2593+
;; to clobber LR.
2594+
(let ((_ Unit (emit (MInst.Xpaclri))))
2595+
(mov_preg (preg_link))))

cranelift/codegen/src/isa/aarch64/inst/emit.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3121,6 +3121,7 @@ impl MachInstEmit for Inst {
31213121

31223122
sink.put4(0xd503233f | key << 6);
31233123
}
3124+
&Inst::Xpaclri => sink.put4(0xd50320ff),
31243125
&Inst::VirtualSPOffsetAdj { offset } => {
31253126
trace!(
31263127
"virtual sp offset adjusted by {} -> {}",

cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ fn test_aarch64_binemit() {
5757
"retab",
5858
));
5959
insns.push((Inst::Pacisp { key: APIKey::B }, "7F2303D5", "pacibsp"));
60+
insns.push((Inst::Xpaclri, "FF2003D5", "xpaclri"));
6061
insns.push((Inst::Nop0, "", "nop-zero-len"));
6162
insns.push((Inst::Nop4, "1F2003D5", "nop"));
6263
insns.push((Inst::Csdb, "9F2203D5", "csdb"));

cranelift/codegen/src/isa/aarch64/inst/mod.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -981,12 +981,7 @@ fn aarch64_get_operands<F: Fn(VReg) -> VReg>(inst: &Inst, collector: &mut Operan
981981
collector.reg_def(rd);
982982
collector.reg_use(rn);
983983
}
984-
&Inst::Ret { ref rets } => {
985-
for &ret in rets {
986-
collector.reg_use(ret);
987-
}
988-
}
989-
&Inst::AuthenticatedRet { ref rets, .. } => {
984+
&Inst::Ret { ref rets } | &Inst::AuthenticatedRet { ref rets, .. } => {
990985
for &ret in rets {
991986
collector.reg_use(ret);
992987
}
@@ -1039,7 +1034,10 @@ fn aarch64_get_operands<F: Fn(VReg) -> VReg>(inst: &Inst, collector: &mut Operan
10391034
collector.reg_def(rd);
10401035
memarg_operands(mem, collector);
10411036
}
1042-
&Inst::Pacisp { .. } => {}
1037+
&Inst::Pacisp { .. } | &Inst::Xpaclri => {
1038+
// Neither LR nor SP is an allocatable register, so there is no need
1039+
// to do anything.
1040+
}
10431041
&Inst::VirtualSPOffsetAdj { .. } => {}
10441042

10451043
&Inst::ElfTlsGetAddr { .. } => {
@@ -2715,6 +2713,7 @@ impl Inst {
27152713

27162714
"paci".to_string() + key + "sp"
27172715
}
2716+
&Inst::Xpaclri => "xpaclri".to_string(),
27182717
&Inst::VirtualSPOffsetAdj { offset } => {
27192718
state.virtual_sp_offset += offset;
27202719
format!("virtual_sp_offset_adjust {}", offset)

cranelift/codegen/src/isa/aarch64/lower/isle.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,14 @@ pub struct SinkableAtomicLoad {
7171
impl generated_code::Context for IsleContext<'_, '_, MInst, Flags, IsaFlags, 6> {
7272
isle_prelude_methods!();
7373

74+
fn sign_return_address_disabled(&mut self) -> Option<()> {
75+
if self.isa_flags.sign_return_address() {
76+
None
77+
} else {
78+
Some(())
79+
}
80+
}
81+
7482
fn use_lse(&mut self, _: Inst) -> Option<()> {
7583
if self.isa_flags.use_lse() {
7684
Some(())

cranelift/native/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,9 @@ pub fn builder_with_options(infer_native_flags: bool) -> Result<isa::Builder, &'
138138
}
139139

140140
if cfg!(target_os = "macos") {
141+
// Pointer authentication is always available on Apple Silicon.
142+
isa_builder.enable("sign_return_address").unwrap();
143+
// macOS enforces the use of the B key for return addresses.
141144
isa_builder.enable("sign_return_address_with_bkey").unwrap();
142145
}
143146
}

crates/fiber/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,8 @@ mod tests {
234234
.any(|s| s.contains("look_for_me"))
235235
// TODO: apparently windows unwind routines don't unwind through fibers, so this will always fail. Is there a way we can fix that?
236236
|| cfg!(windows)
237+
// TODO: the system libunwind is broken (#2808)
238+
|| cfg!(all(target_os = "macos", target_arch = "aarch64"))
237239
);
238240
}
239241

crates/fiber/src/unix/aarch64.rs

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,15 @@ use wasmtime_asm_macros::asm_func;
2222

2323
cfg_if::cfg_if! {
2424
if #[cfg(target_os = "macos")] {
25-
macro_rules! cfi_window_save { () => (""); }
26-
macro_rules! pacia1716 { () => (""); }
27-
macro_rules! paciasp { () => (""); }
28-
macro_rules! autiasp { () => (""); }
25+
macro_rules! paci1716 { () => ("pacib1716\n"); }
26+
macro_rules! pacisp { () => ("pacibsp\n"); }
27+
macro_rules! autisp { () => ("autibsp\n"); }
2928
macro_rules! sym_adrp { ($s:tt) => (concat!("_", $s, "@PAGE")); }
3029
macro_rules! sym_add { ($s:tt) => (concat!("_", $s, "@PAGEOFF")); }
3130
} else {
32-
macro_rules! cfi_window_save { () => (".cfi_window_save\n"); }
33-
macro_rules! pacia1716 { () => ("pacia1716\n"); }
34-
macro_rules! paciasp { () => ("paciasp\n"); }
35-
macro_rules! autiasp { () => ("autiasp\n"); }
31+
macro_rules! paci1716 { () => ("pacia1716\n"); }
32+
macro_rules! pacisp { () => ("paciasp\n"); }
33+
macro_rules! autisp { () => ("autiasp\n"); }
3634
macro_rules! sym_adrp { ($s:tt) => (concat!($s, "")); }
3735
macro_rules! sym_add { ($s:tt) => (concat!(":lo12:", $s)); }
3836
}
@@ -44,9 +42,9 @@ asm_func!(
4442
"
4543
.cfi_startproc
4644
",
47-
paciasp!(),
48-
cfi_window_save!(),
45+
pacisp!(),
4946
"
47+
.cfi_window_save
5048
// Save all callee-saved registers on the stack since we're
5149
// assuming they're clobbered as a result of the stack switch.
5250
stp x29, x30, [sp, -16]!
@@ -81,9 +79,9 @@ asm_func!(
8179
ldp x20, x19, [sp], 16
8280
ldp x29, x30, [sp], 16
8381
",
84-
autiasp!(),
85-
cfi_window_save!(),
82+
autisp!(),
8683
"
84+
.cfi_window_save
8785
ret
8886
.cfi_endproc
8987
",
@@ -121,7 +119,7 @@ asm_func!(
121119
adrp x17, ", sym_adrp!("wasmtime_fiber_start"), "
122120
add x17, x17, ", sym_add!("wasmtime_fiber_start"), "
123121
",
124-
pacia1716!(),
122+
paci1716!(),
125123
"
126124
str x17, [x16, -0x8] // x17 => lr
127125
str x0, [x16, -0x18] // x0 => x19
@@ -151,9 +149,7 @@ asm_func!(
151149
0x23, 0xa0, 0x1 /* DW_OP_plus_uconst 0xa0 */
152150
.cfi_rel_offset x29, -0x10
153151
.cfi_rel_offset x30, -0x08
154-
",
155-
cfi_window_save!(),
156-
"
152+
.cfi_window_save
157153
.cfi_rel_offset x19, -0x18
158154
.cfi_rel_offset x20, -0x20
159155
.cfi_rel_offset x21, -0x28

crates/fuzzing/src/generators/codegen_settings.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,16 @@ impl<'a> Arbitrary<'a> for CodegenSettings {
128128
test: is_aarch64_feature_detected,
129129

130130
std: "lse" => clif: "has_lse",
131+
// even though the natural correspondence seems to be
132+
// between "paca" and "has_pauth", the latter has no effect
133+
// in isolation, so we actually use the setting that affects
134+
// code generation
135+
std: "paca" => clif: "sign_return_address",
136+
// "paca" and "pacg" check for the same underlying
137+
// architectural feature, so we use the latter to cover more
138+
// code generation settings, of which we have chosen the one
139+
// with the most significant effect
140+
std: "pacg" => clif: "sign_return_address_all" ratio: 1 in 2,
131141
},
132142
};
133143
return Ok(CodegenSettings::Target {

0 commit comments

Comments
 (0)