Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

FrameLayoutChange-based .eh_frame support #902

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cranelift-faerie/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ cranelift-module = { path = "../cranelift-module", version = "0.54.0" }
faerie = "0.14.0"
goblin = "0.1.0"
anyhow = "1.0"
byteorder = "1.2"
gimli = "0.19.0"
target-lexicon = "0.10"

[dependencies.cranelift-codegen]
Expand Down
132 changes: 130 additions & 2 deletions cranelift-faerie/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@ use cranelift_codegen::binemit::{
use cranelift_codegen::isa::TargetIsa;
use cranelift_codegen::{self, binemit, ir};
use cranelift_module::{
Backend, DataContext, DataDescription, DataId, FuncId, Init, Linkage, ModuleError,
Backend, DataContext, DataDescription, DataId, FrameSink, FuncId, Init, Linkage, ModuleError,
ModuleNamespace, ModuleResult,
};
use faerie;
use std::fs::File;
use target_lexicon::Triple;

use gimli::write::{Address, FrameDescriptionEntry};
use gimli::{DW_EH_PE_pcrel, DW_EH_PE_sdata4};

#[derive(Debug)]
/// Setting to enable collection of traps. Setting this to `Enabled` in
/// `FaerieBuilder` means that a `FaerieTrapManifest` will be present
Expand Down Expand Up @@ -76,9 +79,76 @@ pub struct FaerieBackend {
isa: Box<dyn TargetIsa>,
artifact: faerie::Artifact,
trap_manifest: Option<FaerieTrapManifest>,
frame_sink: Option<FrameSink>,
libcall_names: Box<dyn Fn(ir::LibCall) -> String>,
}

struct FaerieDebugSink<'a> {
pub data: &'a mut Vec<u8>,
pub functions: &'a [String],
pub artifact: &'a mut faerie::Artifact,
}

impl<'a> gimli::write::Writer for FaerieDebugSink<'a> {
type Endian = gimli::LittleEndian;

fn endian(&self) -> Self::Endian {
gimli::LittleEndian
}
fn len(&self) -> usize {
self.data.len()
}
fn write(&mut self, bytes: &[u8]) -> gimli::write::Result<()> {
self.data.extend_from_slice(bytes);
Ok(())
}

fn write_at(&mut self, offset: usize, bytes: &[u8]) -> gimli::write::Result<()> {
if offset + bytes.len() > self.data.len() {
return Err(gimli::write::Error::LengthOutOfBounds);
}
self.data[offset..][..bytes.len()].copy_from_slice(bytes);
Ok(())
}

fn write_eh_pointer(
&mut self,
address: Address,
eh_pe: gimli::DwEhPe,
size: u8,
) -> gimli::write::Result<()> {
match address {
Address::Constant(val) => self.write_udata(val, size),
Address::Symbol { symbol, addend } => {
assert_eq!(addend, 0);

assert_eq!(eh_pe.format(), DW_EH_PE_sdata4, "faerie backend currently only supports PC-relative 4-byte offsets for DWARF pointers.");
assert_eq!(eh_pe.application(), DW_EH_PE_pcrel, "faerie backend currently only supports PC-relative 4-byte offsets for DWARF pointers.");

let name = self.functions[symbol].as_str();

let reloc = faerie::artifact::Reloc::Raw {
reloc: goblin::elf::reloc::R_X86_64_PC32,
addend: 0,
};

self.artifact
.link_with(
faerie::Link {
to: name,
from: ".eh_frame",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this __eh_frame for machO?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good question, I'll at least see what other toolchains name it. I also need to use a machO-appropriate relocation.

at: self.data.len() as u64,
},
reloc,
)
.map_err(|_link_err| gimli::write::Error::InvalidAddress)?;

self.write_udata(0, size)
}
}
}
}

pub struct FaerieCompiledFunction {
code_length: u32,
}
Expand Down Expand Up @@ -108,13 +178,15 @@ impl Backend for FaerieBackend {

/// Create a new `FaerieBackend` using the given Cranelift target.
fn new(builder: FaerieBuilder) -> Self {
let frame_sink = FrameSink::new(&builder.isa);
Self {
artifact: faerie::Artifact::new(builder.isa.triple().clone(), builder.name),
isa: builder.isa,
trap_manifest: match builder.collect_traps {
FaerieTrapCollection::Enabled => Some(FaerieTrapManifest::new()),
FaerieTrapCollection::Disabled => None,
},
frame_sink: Some(frame_sink),
libcall_names: builder.libcall_names,
}
}
Expand Down Expand Up @@ -193,6 +265,44 @@ impl Backend for FaerieBackend {
// because `define` will take ownership of code, this is our last chance
let code_length = code.len() as u32;

if let Some(ref mut frame_sink) = self.frame_sink {
if let Some(layout) = ctx.func.frame_layout.as_ref() {
let (cie, mut encoder) = frame_sink.cie_for(&layout.initial);

let mut fd_entry =
FrameDescriptionEntry::new(frame_sink.address_for(name), code_length);

let mut frame_changes = vec![];
for ebb in ctx.func.layout.ebbs() {
for (offset, inst, size) in
ctx.func.inst_offsets(ebb, &self.isa.encoding_info())
{
if let Some(changes) = layout.instructions.get(&inst) {
for change in changes.iter() {
frame_changes.push((offset + size, change.clone()));
}
}
}
}

frame_changes.sort_by(|a, b| a.0.cmp(&b.0));

let fde_insts = frame_changes
.into_iter()
.flat_map(|(addr, change)| encoder.translate(&change).map(|inst| (addr, inst)));

for (addr, inst) in fde_insts.into_iter() {
fd_entry.add_instruction(addr, inst);
}

frame_sink.add_fde(cie, fd_entry);
} else {
// we have a frame sink to write .eh_frames into, but are not collecting debug
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please panic here to make it easier to debug in that case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized this morning that there's a case where we will reach this arm but not be a bug: if the user of cranelift has a per-function nounwind-like attribute, a nounwind function wouldn't need unwind information collected, even though there still would be a frame sink for other functions. Right now it happens to be that Lucet is all-or-nothing on unwind info, but as a rustc backend for example I could see some function being opt-out.

Given that, would you rather a panic to identify the case for later discussion, or should I discard the else entirely?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but as a rustc backend for example I could see some function being opt-out.

I believe unwinding enabled/disabled is done per crate (using -Cpanic=abort), not per function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rustc has the unstable #[unwind(allowed/aborts)] to control per function, as well I think it has different requirements for extern "C", but I don't know the details of that and it is under ongoing discussion.

// information for at least the current function. This might be a bug in the code
// using cranelift-faerie?
}
}

self.artifact
.define(name, code)
.expect("inconsistent declaration");
Expand Down Expand Up @@ -308,7 +418,25 @@ impl Backend for FaerieBackend {
}

fn publish(&mut self) {
// Nothing to do.
if let Some(ref mut frame_sink) = self.frame_sink {
self.artifact
.declare(
".eh_frame",
faerie::Decl::section(faerie::SectionKind::Data),
)
.unwrap();

let mut eh_frame_bytes = Vec::new();

let mut eh_frame_writer = gimli::write::EhFrame(FaerieDebugSink {
data: &mut eh_frame_bytes,
functions: frame_sink.fn_names_slice(),
artifact: &mut self.artifact,
});
frame_sink.write_to(&mut eh_frame_writer).unwrap();

self.artifact.define(".eh_frame", eh_frame_bytes).unwrap();
}
}

fn finish(self) -> FaerieProduct {
Expand Down
1 change: 1 addition & 0 deletions cranelift-module/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ edition = "2018"
[dependencies]
cranelift-codegen = { path = "../cranelift-codegen", version = "0.54.0", default-features = false }
cranelift-entity = { path = "../cranelift-entity", version = "0.54.0" }
gimli = "0.19.0"
hashbrown = { version = "0.6", optional = true }
log = { version = "0.4.6", default-features = false }
thiserror = "1.0.4"
Expand Down
Loading