Skip to content

Commit

Permalink
Return error for register mapping failure.
Browse files Browse the repository at this point in the history
This commit removes a panic when a register mapping fails and instead returns
an error from creating the unwind information.
  • Loading branch information
peterhuene committed Apr 15, 2020
1 parent ab4e793 commit 0604841
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 27 deletions.
2 changes: 1 addition & 1 deletion cranelift/codegen/src/isa/unwind/systemv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ type Expression = Vec<u8>;

/// Enumerate the errors possible in mapping Cranelift registers to their DWARF equivalent.
#[allow(missing_docs)]
#[derive(Error, Debug)]
#[derive(Error, Debug, PartialEq, Eq)]
pub enum RegisterMappingError {
#[error("unable to find bank for register info")]
MissingBank,
Expand Down
54 changes: 28 additions & 26 deletions cranelift/codegen/src/isa/x86/unwind/systemv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::isa::{
x86::registers::RU,
CallConv, RegUnit, TargetIsa,
};
use crate::result::CodegenResult;
use crate::result::{CodegenError, CodegenResult};
use alloc::vec::Vec;
use gimli::{write::CommonInformationEntry, Encoding, Format, Register, X86_64};

Expand Down Expand Up @@ -117,7 +117,7 @@ impl<'a> InstructionBuilder<'a> {
}
}

fn push_reg(&mut self, offset: u32, arg: Value) {
fn push_reg(&mut self, offset: u32, arg: Value) -> Result<(), RegisterMappingError> {
self.cfa_offset += 8;

let reg = self.func.locations[arg].unwrap_reg();
Expand All @@ -135,13 +135,10 @@ impl<'a> InstructionBuilder<'a> {
// Pushes in the prologue are register saves, so record an offset of the save
self.instructions.push((
offset,
CallFrameInstruction::Offset(
map_reg(self.isa, reg)
.expect("a register mapping from cranelift to gimli")
.0,
-self.cfa_offset,
),
CallFrameInstruction::Offset(map_reg(self.isa, reg)?.0, -self.cfa_offset),
));

Ok(())
}

fn adjust_sp_down(&mut self, offset: u32) {
Expand Down Expand Up @@ -183,20 +180,23 @@ impl<'a> InstructionBuilder<'a> {
.push((offset, CallFrameInstruction::CfaOffset(self.cfa_offset)));
}

fn move_reg(&mut self, offset: u32, src: RegUnit, dst: RegUnit) {
fn move_reg(
&mut self,
offset: u32,
src: RegUnit,
dst: RegUnit,
) -> Result<(), RegisterMappingError> {
if let Some(fp) = self.frame_register {
// Check for change in CFA register (RSP is always the starting CFA)
if src == (RU::rsp as RegUnit) && dst == fp {
self.instructions.push((
offset,
CallFrameInstruction::CfaRegister(
map_reg(self.isa, dst)
.expect("a register mapping from cranelift to gimli")
.0,
),
CallFrameInstruction::CfaRegister(map_reg(self.isa, dst)?.0),
));
}
}

Ok(())
}

fn prologue_imm_const(&mut self, imm: i64) {
Expand All @@ -210,7 +210,7 @@ impl<'a> InstructionBuilder<'a> {
self.stack_size = Some(imm as i32);
}

fn ret(&mut self, inst: Inst) {
fn ret(&mut self, inst: Inst) -> Result<(), RegisterMappingError> {
let args = self.func.dfg.inst_args(inst);

for (i, arg) in args.iter().rev().enumerate() {
Expand All @@ -229,9 +229,7 @@ impl<'a> InstructionBuilder<'a> {
self.instructions.push((
self.epilogue_pop_offsets[i],
CallFrameInstruction::Cfa(
map_reg(self.isa, RU::rsp as RegUnit)
.expect("a register mapping from cranelift to gimli")
.0,
map_reg(self.isa, RU::rsp as RegUnit)?.0,
self.cfa_offset,
),
));
Expand All @@ -247,17 +245,15 @@ impl<'a> InstructionBuilder<'a> {
// This isn't necessary when using a frame pointer as the CFA doesn't change for CSR restores
self.instructions.push((
self.epilogue_pop_offsets[i],
CallFrameInstruction::SameValue(
map_reg(self.isa, reg)
.expect("a register mapping from cranelift to gimli")
.0,
),
CallFrameInstruction::SameValue(map_reg(self.isa, reg)?.0),
));
}
};
}

self.epilogue_pop_offsets.clear();

Ok(())
}

fn insert_pop_offset(&mut self, offset: u32) {
Expand Down Expand Up @@ -332,15 +328,19 @@ pub(crate) fn create_unwind_info(
match builder.func.dfg[inst] {
InstructionData::Unary { opcode, arg } => match opcode {
Opcode::X86Push => {
builder.push_reg(offset, arg);
builder
.push_reg(offset, arg)
.map_err(CodegenError::RegisterMappingError)?;
}
Opcode::AdjustSpDown => {
builder.adjust_sp_down(offset);
}
_ => {}
},
InstructionData::CopySpecial { src, dst, .. } => {
builder.move_reg(offset, src, dst);
builder
.move_reg(offset, src, dst)
.map_err(CodegenError::RegisterMappingError)?;
}
InstructionData::NullAry { opcode } => match opcode {
Opcode::X86Pop => {
Expand All @@ -362,7 +362,9 @@ pub(crate) fn create_unwind_info(
},
InstructionData::MultiAry { opcode, .. } => match opcode {
Opcode::Return => {
builder.ret(inst);
builder
.ret(inst)
.map_err(CodegenError::RegisterMappingError)?;

if !is_last_block {
builder.restore_state(offset);
Expand Down
5 changes: 5 additions & 0 deletions cranelift/codegen/src/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ pub enum CodegenError {
/// is exceeded, compilation fails.
#[error("Code for function is too large")]
CodeTooLarge,

/// A failure to map Cranelift register representation to a DWARF register representation.
#[cfg(feature = "unwind")]
#[error("Register mapping error")]
RegisterMappingError(crate::isa::unwind::systemv::RegisterMappingError),
}

/// A convenient alias for a `Result` that uses `CodegenError` as the error type.
Expand Down

0 comments on commit 0604841

Please sign in to comment.