Skip to content

Commit

Permalink
Merge pull request #1983 from peterhuene/fix-unwind-info
Browse files Browse the repository at this point in the history 
Remove 'set frame pointer' unwind code from Windows x64 unwind.
  • Loading branch information
@peterhuene
peterhuene authored Jul 7, 2020
2 parents 62530e4 + 92864ba commit d6ae72a
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 149 deletions.
19 changes: 5 additions & 14 deletions cranelift/codegen/src/isa/unwind/winx64.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! System V ABI unwind information.
//! Windows x64 ABI unwind information.

use alloc::vec::Vec;
use byteorder::{ByteOrder, LittleEndian};
Expand Down Expand Up @@ -57,10 +57,6 @@ pub(crate) enum UnwindCode {
offset: u8,
size: u32,
},
SetFramePointer {
offset: u8,
sp_offset: u8,
},
}

impl UnwindCode {
Expand All @@ -69,7 +65,6 @@ impl UnwindCode {
PushNonvolatileRegister = 0,
LargeStackAlloc = 1,
SmallStackAlloc = 2,
SetFramePointer = 3,
SaveXmm128 = 8,
SaveXmm128Far = 9,
}
Expand All @@ -85,13 +80,13 @@ impl UnwindCode {
stack_offset,
} => {
writer.write_u8(*offset);
let stack_offset = stack_offset / 16;
if stack_offset <= core::u16::MAX as u32 {
let scaled_stack_offset = stack_offset / 16;
if scaled_stack_offset <= core::u16::MAX as u32 {
writer.write_u8((*reg << 4) | (UnwindOperation::SaveXmm128 as u8));
writer.write_u16::<LittleEndian>(stack_offset as u16);
writer.write_u16::<LittleEndian>(scaled_stack_offset as u16);
} else {
writer.write_u8((*reg << 4) | (UnwindOperation::SaveXmm128Far as u8));
writer.write_u16::<LittleEndian>(stack_offset as u16);
writer.write_u16::<LittleEndian>(*stack_offset as u16);
writer.write_u16::<LittleEndian>((stack_offset >> 16) as u16);
}
}
Expand All @@ -113,10 +108,6 @@ impl UnwindCode {
writer.write_u32::<LittleEndian>(*size);
}
}
Self::SetFramePointer { offset, sp_offset } => {
writer.write_u8(*offset);
writer.write_u8((*sp_offset << 4) | (UnwindOperation::SetFramePointer as u8));
}
};
}

Expand Down
3 changes: 1 addition & 2 deletions cranelift/codegen/src/isa/x86/abi.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1071,8 +1071,7 @@ pub fn create_unwind_info(
.map(|u| UnwindInfo::SystemV(u))
}
CallConv::WindowsFastcall => {
super::unwind::winx64::create_unwind_info(func, isa, Some(RU::rbp.into()))?
.map(|u| UnwindInfo::WindowsX64(u))
super::unwind::winx64::create_unwind_info(func, isa)?.map(|u| UnwindInfo::WindowsX64(u))
}
_ => None,
})
Expand Down
111 changes: 21 additions & 90 deletions cranelift/codegen/src/isa/x86/unwind/winx64.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use log::warn;
pub(crate) fn create_unwind_info(
func: &Function,
isa: &dyn TargetIsa,
frame_register: Option<RegUnit>,
) -> CodegenResult<Option<UnwindInfo>> {
// Only Windows fastcall is supported for unwind information
if func.signature.call_conv != CallConv::WindowsFastcall || func.prologue_end.is_none() {
Expand All @@ -28,7 +27,6 @@ pub(crate) fn create_unwind_info(
let mut prologue_size = 0;
let mut unwind_codes = Vec::new();
let mut found_end = false;
let mut xmm_save_count: u8 = 0;

for (offset, inst, size) in func.inst_offsets(entry_block, &isa.encoding_info()) {
// x64 ABI prologues cannot exceed 255 bytes in length
Expand Down Expand Up @@ -64,16 +62,6 @@ pub(crate) fn create_unwind_info(
_ => {}
}
}
InstructionData::CopySpecial { src, dst, .. } => {
if let Some(frame_register) = frame_register {
if src == (RU::rsp as RegUnit) && dst == frame_register {
unwind_codes.push(UnwindCode::SetFramePointer {
offset: unwind_offset,
sp_offset: 0,
});
}
}
}
InstructionData::UnaryImm { opcode, imm } => {
match opcode {
Opcode::Iconst => {
Expand Down Expand Up @@ -112,16 +100,13 @@ pub(crate) fn create_unwind_info(
{
// If this is a save of an FPR, record an unwind operation
// Note: the stack_offset here is relative to an adjusted SP
// This will be fixed up later to be based on the frame pointer offset
if dst == (RU::rsp as RegUnit) && FPR.contains(src) {
let offset: i32 = offset.into();
unwind_codes.push(UnwindCode::SaveXmm {
offset: unwind_offset,
reg: src as u8,
stack_offset: offset as u32,
});

xmm_save_count += 1;
}
}
}
Expand All @@ -136,45 +121,11 @@ pub(crate) fn create_unwind_info(

assert!(found_end);

// When using a frame register, certain unwind operations, such as XMM saves, are relative to the frame
// register minus some offset, forming a "base address". This attempts to calculate the frame register offset
// while updating the XMM save offsets to be relative from this "base address" rather than RSP.
let mut frame_register_offset = 0;
if frame_register.is_some() && xmm_save_count > 0 {
// Determine the number of 16-byte slots used for all CSRs (including GPRs)
// The "frame register offset" will point at the last slot used (i.e. the last saved FPR)
// Assumption: each FPR is stored at a lower address than the previous one
let mut last_stack_offset = None;
let mut fpr_save_count: u8 = 0;
let mut gpr_push_count: u8 = 0;
for code in unwind_codes.iter_mut() {
match code {
UnwindCode::SaveXmm { stack_offset, .. } => {
if let Some(last) = last_stack_offset {
assert!(last > *stack_offset);
}
last_stack_offset = Some(*stack_offset);
fpr_save_count += 1;
*stack_offset = (xmm_save_count - fpr_save_count) as u32 * 16;
}
UnwindCode::PushRegister { .. } => {
gpr_push_count += 1;
}
_ => {}
}
}
assert_eq!(fpr_save_count, xmm_save_count);

// Account for alignment space when there's an odd number of GPR pushes
// Assumption: an FPR (16 bytes) is twice the size of a GPR (8 bytes), hence the (rounded-up) integer division
frame_register_offset = fpr_save_count + ((gpr_push_count + 1) / 2);
}

Ok(Some(UnwindInfo {
flags: 0, // this assumes cranelift functions have no SEH handlers
prologue_size: prologue_size as u8,
frame_register: frame_register.map(|r| GPR.index_of(r) as u8),
frame_register_offset,
frame_register: None,
frame_register_offset: 0,
unwind_codes,
}))
}
Expand All @@ -201,7 +152,7 @@ mod tests {
context.compile(&*isa).expect("expected compilation");

assert_eq!(
create_unwind_info(&context.func, &*isa, None).expect("can create unwind info"),
create_unwind_info(&context.func, &*isa).expect("can create unwind info"),
None
);
}
Expand All @@ -219,7 +170,7 @@ mod tests {

context.compile(&*isa).expect("expected compilation");

let unwind = create_unwind_info(&context.func, &*isa, Some(RU::rbp.into()))
let unwind = create_unwind_info(&context.func, &*isa)
.expect("can create unwind info")
.expect("expected unwind info");

Expand All @@ -228,17 +179,13 @@ mod tests {
UnwindInfo {
flags: 0,
prologue_size: 9,
frame_register: Some(GPR.index_of(RU::rbp.into()) as u8),
frame_register: None,
frame_register_offset: 0,
unwind_codes: vec![
UnwindCode::PushRegister {
offset: 2,
reg: GPR.index_of(RU::rbp.into()) as u8
},
UnwindCode::SetFramePointer {
offset: 5,
sp_offset: 0
},
UnwindCode::StackAlloc {
offset: 9,
size: 64
Expand All @@ -247,26 +194,22 @@ mod tests {
}
);

assert_eq!(unwind.emit_size(), 12);
assert_eq!(unwind.emit_size(), 8);

let mut buf = [0u8; 12];
let mut buf = [0u8; 8];
unwind.emit(&mut buf);

assert_eq!(
buf,
[
0x01, // Version and flags (version 1, no flags)
0x09, // Prologue size
0x03, // Unwind code count (1 for stack alloc, 1 for save frame reg, 1 for push reg)
0x05, // Frame register + offset (RBP with 0 offset)
0x02, // Unwind code count (1 for stack alloc, 1 for push reg)
0x00, // Frame register + offset (no frame register)
0x09, // Prolog offset
0x72, // Operation 2 (small stack alloc), size = 0xB slots (e.g. (0x7 * 8) + 8 = 64 bytes)
0x05, // Prolog offset
0x03, // Operation 3 (save frame register), stack pointer offset = 0
0x02, // Prolog offset
0x50, // Operation 0 (save nonvolatile register), reg = 5 (RBP)
0x00, // Padding byte
0x00, // Padding byte
]
);
}
Expand All @@ -284,7 +227,7 @@ mod tests {

context.compile(&*isa).expect("expected compilation");

let unwind = create_unwind_info(&context.func, &*isa, Some(RU::rbp.into()))
let unwind = create_unwind_info(&context.func, &*isa)
.expect("can create unwind info")
.expect("expected unwind info");

Expand All @@ -293,17 +236,13 @@ mod tests {
UnwindInfo {
flags: 0,
prologue_size: 27,
frame_register: Some(GPR.index_of(RU::rbp.into()) as u8),
frame_register: None,
frame_register_offset: 0,
unwind_codes: vec![
UnwindCode::PushRegister {
offset: 2,
reg: GPR.index_of(RU::rbp.into()) as u8
},
UnwindCode::SetFramePointer {
offset: 5,
sp_offset: 0
},
UnwindCode::StackAlloc {
offset: 27,
size: 10000
Expand All @@ -322,16 +261,16 @@ mod tests {
[
0x01, // Version and flags (version 1, no flags)
0x1B, // Prologue size
0x04, // Unwind code count (2 for stack alloc, 1 for save frame reg, 1 for push reg)
0x05, // Frame register + offset (RBP with 0 offset)
0x03, // Unwind code count (2 for stack alloc, 1 for push reg)
0x00, // Frame register + offset (no frame register)
0x1B, // Prolog offset
0x01, // Operation 1 (large stack alloc), size is scaled 16-bits (info = 0)
0xE2, // Low size byte
0x04, // High size byte (e.g. 0x04E2 * 8 = 10000 bytes)
0x05, // Prolog offset
0x03, // Operation 3 (save frame register), stack pointer offset = 0
0x02, // Prolog offset
0x50, // Operation 0 (push nonvolatile register), reg = 5 (RBP)
0x00, // Padding
0x00, // Padding
]
);
}
Expand All @@ -349,7 +288,7 @@ mod tests {

context.compile(&*isa).expect("expected compilation");

let unwind = create_unwind_info(&context.func, &*isa, Some(RU::rbp.into()))
let unwind = create_unwind_info(&context.func, &*isa)
.expect("can create unwind info")
.expect("expected unwind info");

Expand All @@ -358,17 +297,13 @@ mod tests {
UnwindInfo {
flags: 0,
prologue_size: 27,
frame_register: Some(GPR.index_of(RU::rbp.into()) as u8),
frame_register: None,
frame_register_offset: 0,
unwind_codes: vec![
UnwindCode::PushRegister {
offset: 2,
reg: GPR.index_of(RU::rbp.into()) as u8
},
UnwindCode::SetFramePointer {
offset: 5,
sp_offset: 0
},
UnwindCode::StackAlloc {
offset: 27,
size: 1000000
Expand All @@ -377,30 +312,26 @@ mod tests {
}
);

assert_eq!(unwind.emit_size(), 16);
assert_eq!(unwind.emit_size(), 12);

let mut buf = [0u8; 16];
let mut buf = [0u8; 12];
unwind.emit(&mut buf);

assert_eq!(
buf,
[
0x01, // Version and flags (version 1, no flags)
0x1B, // Prologue size
0x05, // Unwind code count (3 for stack alloc, 1 for save frame reg, 1 for push reg)
0x05, // Frame register + offset (RBP with 0 offset)
0x04, // Unwind code count (3 for stack alloc, 1 for push reg)
0x00, // Frame register + offset (no frame register)
0x1B, // Prolog offset
0x11, // Operation 1 (large stack alloc), size is unscaled 32-bits (info = 1)
0x40, // Byte 1 of size
0x42, // Byte 2 of size
0x0F, // Byte 3 of size
0x00, // Byte 4 of size (size is 0xF4240 = 1000000 bytes)
0x05, // Prolog offset
0x03, // Operation 3 (save frame register), stack pointer offset = 0
0x02, // Prolog offset
0x50, // Operation 0 (push nonvolatile register), reg = 5 (RBP)
0x00, // Padding byte
0x00, // Padding byte
]
);
}
Expand Down
Loading

0 comments on commit d6ae72a

Please sign in to comment.