Skip to content

Commit e0a3793

Browse files
committed
Make unwind info instruction-precise within prologues and epilogues, and remove dead code.
This change ensures that at every instruction step when single-stepping through a function, a debugger knows the correct backtrace. The unwind op for the "push FP" in the prologue was one instruction too late, and we did not have a redefinition of the frame in terms of SP in the epilogue after the old FP was popped. This was because we were only trying to provide correct backtraces at explicit locations we expected traps/GCs. But it is not much more work to make the metadata instruction-precise, so we now have that. (Note that Windows SEH does not record information for epilogues, so that specific bit works only on System V platforms.) This change also removes some significant dead code I had not removed previously; the removed code was the prior fairly brittle "reverse-engineer the prologue" code for the new aarch64 and x64 backends. Also s/no_unwind_info/unwind_info/ in settings from review feedback.
1 parent 208e837 commit e0a3793

76 files changed

Lines changed: 346 additions & 530 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

cranelift/codegen/meta/src/shared/settings.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -236,15 +236,15 @@ pub(crate) fn define() -> SettingGroup {
236236
);
237237

238238
settings.add_bool(
239-
"no_unwind_info",
239+
"unwind_info",
240240
r#"
241-
Do not generate unwind info. This reduces metadata size
242-
and improves compile time, but may result in the inability
243-
for the debugger to trace frames, will break GC tracing
244-
that relies on libunwind (such as in Wasmtime), and may
245-
break platforms that require unwind info (such as Windows).
241+
Generate unwind info. This increases metadata size and compile time,
242+
but allows for the debugger to trace frames, is needed for GC tracing
243+
that relies on libunwind (such as in Wasmtime), and is
244+
unconditionally needed on certain platforms (such as Windows) that
245+
must always be able to unwind.
246246
"#,
247-
false,
247+
true,
248248
);
249249

250250
// BaldrMonkey requires that not-yet-relocated function addresses be encoded

cranelift/codegen/src/isa/aarch64/abi.rs

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,15 @@ impl ABIMachineSpec for AArch64MachineDeps {
486486
),
487487
flags: MemFlags::trusted(),
488488
});
489+
490+
if flags.unwind_info() {
491+
insts.push(Inst::Unwind {
492+
inst: UnwindInst::PushOldFP {
493+
sp_offset_to_caller: 16,
494+
},
495+
});
496+
}
497+
489498
// mov fp (x29), sp. This uses the ADDI rd, rs, 0 form of `MOV` because
490499
// the usual encoding (`ORR`) does not work with SP.
491500
insts.push(Inst::AluRRImm12 {
@@ -497,15 +506,10 @@ impl ABIMachineSpec for AArch64MachineDeps {
497506
shift12: false,
498507
},
499508
});
500-
if !flags.no_unwind_info() {
501-
insts.push(Inst::Unwind {
502-
inst: UnwindInst::PushOldFP,
503-
});
504-
}
505509
insts
506510
}
507511

508-
fn gen_epilogue_frame_restore() -> SmallInstVec<Inst> {
512+
fn gen_epilogue_frame_restore(flags: &settings::Flags) -> SmallInstVec<Inst> {
509513
let mut insts = SmallVec::new();
510514

511515
// N.B.: sp is already adjusted to the appropriate place by the
@@ -523,6 +527,16 @@ impl ABIMachineSpec for AArch64MachineDeps {
523527
flags: MemFlags::trusted(),
524528
});
525529

530+
if flags.unwind_info() {
531+
// Pop caller's FP off of the stack. CFA is now defined in terms of
532+
// SP again.
533+
insts.push(Inst::Unwind {
534+
inst: UnwindInst::PopOldFP {
535+
sp_offset_to_caller: 0,
536+
},
537+
});
538+
}
539+
526540
insts
527541
}
528542

@@ -547,7 +561,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
547561
let total_save_bytes = int_save_bytes + vec_save_bytes;
548562
let clobber_size = total_save_bytes as i32;
549563

550-
if !flags.no_unwind_info() {
564+
if flags.unwind_info() {
551565
// The *unwind* frame (but not the actual frame) starts at the
552566
// clobbers, just below the saved FP/LR pair.
553567
insts.push(Inst::Unwind {
@@ -589,7 +603,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
589603
),
590604
flags: MemFlags::trusted(),
591605
});
592-
if !flags.no_unwind_info() {
606+
if flags.unwind_info() {
593607
frame_offset -= 8;
594608
if r2 != zero_reg() {
595609
insts.push(Inst::Unwind {
@@ -615,7 +629,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
615629
mem: AMode::PreIndexed(writable_stack_reg(), SImm9::maybe_from_i64(-16).unwrap()),
616630
flags: MemFlags::trusted(),
617631
});
618-
if !flags.no_unwind_info() {
632+
if flags.unwind_info() {
619633
frame_offset -= 16;
620634
insts.push(Inst::Unwind {
621635
inst: UnwindInst::SaveReg {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,6 @@ impl MachInstEmitInfo for EmitInfo {
542542
impl MachInstEmit for Inst {
543543
type State = EmitState;
544544
type Info = EmitInfo;
545-
type UnwindInfo = super::unwind::AArch64UnwindInfo;
546545

547546
fn emit(&self, sink: &mut MachBuffer<Inst>, emit_info: &Self::Info, state: &mut EmitState) {
548547
// N.B.: we *must* not exceed the "worst-case size" used to compute
@@ -2096,6 +2095,7 @@ impl MachInstEmit for Inst {
20962095
}
20972096
&Inst::Ret => {
20982097
sink.put4(0xd65f03c0);
2098+
sink.add_unwind(UnwindInst::Ret);
20992099
}
21002100
&Inst::EpiloguePlaceholder => {
21012101
// Noop; this is just a placeholder for epilogues.
Lines changed: 0 additions & 199 deletions
Original file line numberDiff line numberDiff line change
@@ -1,201 +1,2 @@
1-
use super::*;
2-
use crate::isa::aarch64::inst::{args::PairAMode, imms::Imm12, regs, ALUOp, Inst};
3-
use crate::isa::unwind::input::{UnwindCode, UnwindInfo};
4-
use crate::machinst::UnwindInfoContext;
5-
use crate::result::CodegenResult;
6-
use alloc::vec::Vec;
7-
use regalloc::Reg;
8-
91
#[cfg(feature = "unwind")]
102
pub(crate) mod systemv;
11-
12-
pub struct AArch64UnwindInfo;
13-
14-
impl UnwindInfoGenerator<Inst> for AArch64UnwindInfo {
15-
fn create_unwind_info(
16-
context: UnwindInfoContext<Inst>,
17-
) -> CodegenResult<Option<UnwindInfo<Reg>>> {
18-
let word_size = 8u8;
19-
let pair_size = word_size * 2;
20-
let mut codes = Vec::new();
21-
22-
for i in context.prologue.clone() {
23-
let i = i as usize;
24-
let inst = &context.insts[i];
25-
let offset = context.insts_layout[i];
26-
27-
match inst {
28-
Inst::StoreP64 {
29-
rt,
30-
rt2,
31-
mem: PairAMode::PreIndexed(rn, imm7),
32-
..
33-
} if *rt == regs::fp_reg()
34-
&& *rt2 == regs::link_reg()
35-
&& *rn == regs::writable_stack_reg()
36-
&& imm7.value == -(pair_size as i16) =>
37-
{
38-
// stp fp (x29), lr (x30), [sp, #-16]!
39-
codes.push((
40-
offset,
41-
UnwindCode::StackAlloc {
42-
size: pair_size as u32,
43-
},
44-
));
45-
codes.push((
46-
offset,
47-
UnwindCode::SaveRegister {
48-
reg: *rt,
49-
stack_offset: 0,
50-
},
51-
));
52-
codes.push((
53-
offset,
54-
UnwindCode::SaveRegister {
55-
reg: *rt2,
56-
stack_offset: word_size as u32,
57-
},
58-
));
59-
}
60-
Inst::StoreP64 {
61-
rt,
62-
rt2,
63-
mem: PairAMode::PreIndexed(rn, imm7),
64-
..
65-
} if rn.to_reg() == regs::stack_reg() && imm7.value % (pair_size as i16) == 0 => {
66-
// stp r1, r2, [sp, #(i * #16)]
67-
let stack_offset = imm7.value as u32;
68-
codes.push((
69-
offset,
70-
UnwindCode::SaveRegister {
71-
reg: *rt,
72-
stack_offset,
73-
},
74-
));
75-
if *rt2 != regs::zero_reg() {
76-
codes.push((
77-
offset,
78-
UnwindCode::SaveRegister {
79-
reg: *rt2,
80-
stack_offset: stack_offset + word_size as u32,
81-
},
82-
));
83-
}
84-
}
85-
Inst::AluRRImm12 {
86-
alu_op: ALUOp::Add64,
87-
rd,
88-
rn,
89-
imm12:
90-
Imm12 {
91-
bits: 0,
92-
shift12: false,
93-
},
94-
} if *rd == regs::writable_fp_reg() && *rn == regs::stack_reg() => {
95-
// mov fp (x29), sp.
96-
codes.push((offset, UnwindCode::SetFramePointer { reg: rd.to_reg() }));
97-
}
98-
Inst::VirtualSPOffsetAdj { offset: adj } if offset > 0 => {
99-
codes.push((offset, UnwindCode::StackAlloc { size: *adj as u32 }));
100-
}
101-
_ => {}
102-
}
103-
}
104-
105-
// TODO epilogues
106-
107-
let prologue_size = if context.prologue.len() == 0 {
108-
0
109-
} else {
110-
context.insts_layout[context.prologue.end as usize - 1]
111-
};
112-
113-
Ok(Some(UnwindInfo {
114-
prologue_size,
115-
prologue_unwind_codes: codes,
116-
epilogues_unwind_codes: vec![],
117-
function_size: context.len,
118-
word_size,
119-
initial_sp_offset: 0,
120-
}))
121-
}
122-
}
123-
124-
#[cfg(test)]
125-
mod tests {
126-
use super::*;
127-
use crate::cursor::{Cursor, FuncCursor};
128-
use crate::ir::{ExternalName, Function, InstBuilder, Signature, StackSlotData, StackSlotKind};
129-
use crate::isa::{lookup, CallConv};
130-
use crate::settings::{builder, Flags};
131-
use crate::Context;
132-
use std::str::FromStr;
133-
use target_lexicon::triple;
134-
135-
#[test]
136-
fn test_simple_func() {
137-
let isa = lookup(triple!("aarch64"))
138-
.expect("expect aarch64 ISA")
139-
.finish(Flags::new(builder()));
140-
141-
let mut context = Context::for_function(create_function(
142-
CallConv::SystemV,
143-
Some(StackSlotData::new(StackSlotKind::ExplicitSlot, 64)),
144-
));
145-
146-
context.compile(&*isa).expect("expected compilation");
147-
148-
let result = context.mach_compile_result.unwrap();
149-
let unwind_info = result.unwind_info.unwrap();
150-
151-
assert_eq!(
152-
unwind_info,
153-
UnwindInfo {
154-
prologue_size: 12,
155-
prologue_unwind_codes: vec![
156-
(4, UnwindCode::StackAlloc { size: 16 }),
157-
(
158-
4,
159-
UnwindCode::SaveRegister {
160-
reg: regs::fp_reg(),
161-
stack_offset: 0
162-
}
163-
),
164-
(
165-
4,
166-
UnwindCode::SaveRegister {
167-
reg: regs::link_reg(),
168-
stack_offset: 8
169-
}
170-
),
171-
(
172-
8,
173-
UnwindCode::SetFramePointer {
174-
reg: regs::fp_reg()
175-
}
176-
)
177-
],
178-
epilogues_unwind_codes: vec![],
179-
function_size: 24,
180-
word_size: 8,
181-
initial_sp_offset: 0,
182-
}
183-
);
184-
}
185-
186-
fn create_function(call_conv: CallConv, stack_slot: Option<StackSlotData>) -> Function {
187-
let mut func =
188-
Function::with_name_signature(ExternalName::user(0, 0), Signature::new(call_conv));
189-
190-
let block0 = func.dfg.make_block();
191-
let mut pos = FuncCursor::new(&mut func);
192-
pos.insert_block(block0);
193-
pos.ins().return_(&[]);
194-
195-
if let Some(stack_slot) = stack_slot {
196-
func.stack_slots.push(stack_slot);
197-
}
198-
199-
func
200-
}
201-
}

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ impl MachBackend for AArch64Backend {
6565

6666
let buffer = vcode.emit();
6767
let frame_size = vcode.frame_size();
68-
let unwind_info = vcode.unwind_info()?;
6968
let stackslot_offsets = vcode.stackslot_offsets().clone();
7069

7170
let disasm = if want_disasm {
@@ -80,7 +79,6 @@ impl MachBackend for AArch64Backend {
8079
buffer,
8180
frame_size,
8281
disasm,
83-
unwind_info,
8482
value_labels_ranges: Default::default(),
8583
stackslot_offsets,
8684
})

cranelift/codegen/src/isa/arm32/abi.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ impl ABIMachineSpec for Arm32MachineDeps {
295295
ret
296296
}
297297

298-
fn gen_epilogue_frame_restore() -> SmallInstVec<Inst> {
298+
fn gen_epilogue_frame_restore(_: &settings::Flags) -> SmallInstVec<Inst> {
299299
let mut ret = SmallVec::new();
300300
ret.push(Inst::Mov {
301301
rd: writable_sp_reg(),

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,6 @@ impl MachInstEmitInfo for EmitInfo {
286286
impl MachInstEmit for Inst {
287287
type Info = EmitInfo;
288288
type State = EmitState;
289-
type UnwindInfo = super::unwind::Arm32UnwindInfo;
290289

291290
fn emit(&self, sink: &mut MachBuffer<Inst>, emit_info: &Self::Info, state: &mut EmitState) {
292291
let start_off = sink.cur_offset();

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ mod emit;
2222
pub use self::emit::*;
2323
mod regs;
2424
pub use self::regs::*;
25-
pub mod unwind;
2625

2726
#[cfg(test)]
2827
mod emit_tests;

cranelift/codegen/src/isa/arm32/inst/unwind.rs

Lines changed: 0 additions & 14 deletions
This file was deleted.

cranelift/codegen/src/isa/arm32/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ impl MachBackend for Arm32Backend {
7575
buffer,
7676
frame_size,
7777
disasm,
78-
unwind_info: None,
7978
value_labels_ranges: Default::default(),
8079
stackslot_offsets,
8180
})

0 commit comments

Comments
 (0)