diff --git a/cranelift/codegen/src/binemit/memorysink.rs b/cranelift/codegen/src/binemit/memorysink.rs deleted file mode 100644 index 6635bff876..0000000000 --- a/cranelift/codegen/src/binemit/memorysink.rs +++ /dev/null @@ -1,189 +0,0 @@ -//! Code sink that writes binary machine code into contiguous memory. -//! -//! The `CodeSink` trait is the most general way of extracting binary machine code from Cranelift, -//! and it is implemented by things like the `test binemit` file test driver to generate -//! hexadecimal machine code. The `CodeSink` has some undesirable performance properties because of -//! the dual abstraction: `TargetIsa` is a trait object implemented by each supported ISA, so it -//! can't have any generic functions that could be specialized for each `CodeSink` implementation. -//! This results in many virtual function callbacks (one per `put*` call) when -//! `TargetIsa::emit_inst()` is used. -//! -//! The `MemoryCodeSink` type fixes the performance problem because it is a type known to -//! `TargetIsa` so it can specialize its machine code generation for the type. The trade-off is -//! that a `MemoryCodeSink` will always write binary machine code to raw memory. It forwards any -//! relocations to a `RelocSink` trait object. Relocations are less frequent than the -//! `CodeSink::put*` methods, so the performance impact of the virtual callbacks is less severe. -use super::{Addend, CodeInfo, CodeOffset, CodeSink, Reloc}; -use crate::binemit::stack_map::StackMap; -use crate::ir::{ExternalName, Opcode, SourceLoc, TrapCode}; -use core::ptr::write_unaligned; - -/// A `CodeSink` that writes binary machine code directly into memory. -/// -/// A `MemoryCodeSink` object should be used when emitting a Cranelift IR function into executable -/// memory. It writes machine code directly to a raw pointer without any bounds checking, so make -/// sure to allocate enough memory for the whole function. The number of bytes required is returned -/// by the `Context::compile()` function. -/// -/// Any relocations in the function are forwarded to the `RelocSink` trait object. -/// -/// Note that `MemoryCodeSink` writes multi-byte values in the native byte order of the host. This -/// is not the right thing to do for cross compilation. -pub struct MemoryCodeSink<'a> { - /// Pointer to start of sink's preallocated memory. - data: *mut u8, - /// Offset is isize because its major consumer needs it in that form. - offset: isize, - relocs: &'a mut dyn RelocSink, - traps: &'a mut dyn TrapSink, - /// Information about the generated code and read-only data. - pub info: CodeInfo, -} - -impl<'a> MemoryCodeSink<'a> { - /// Create a new memory code sink that writes a function to the memory pointed to by `data`. - /// - /// # Safety - /// - /// This function is unsafe since `MemoryCodeSink` does not perform bounds checking on the - /// memory buffer, and it can't guarantee that the `data` pointer is valid. - pub unsafe fn new( - data: *mut u8, - relocs: &'a mut dyn RelocSink, - traps: &'a mut dyn TrapSink, - ) -> Self { - Self { - data, - offset: 0, - info: CodeInfo { total_size: 0 }, - relocs, - traps, - } - } -} - -/// A trait for receiving relocations for code that is emitted directly into memory. -pub trait RelocSink { - /// Add a relocation referencing an external symbol at the current offset. - fn reloc_external( - &mut self, - _: CodeOffset, - _: SourceLoc, - _: Reloc, - _: &ExternalName, - _: Addend, - ); - - /// Track a call site whose return address is the given CodeOffset, for the given opcode. Does - /// nothing in general, only useful for certain embedders (SpiderMonkey). - fn add_call_site(&mut self, _: Opcode, _: CodeOffset, _: SourceLoc) {} -} - -/// A trait for receiving trap codes and offsets. -/// -/// If you don't need information about possible traps, you can use the -/// [`NullTrapSink`](NullTrapSink) implementation. -pub trait TrapSink { - /// Add trap information for a specific offset. - fn trap(&mut self, _: CodeOffset, _: SourceLoc, _: TrapCode); -} - -impl<'a> MemoryCodeSink<'a> { - fn write(&mut self, x: T) { - unsafe { - #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))] - write_unaligned(self.data.offset(self.offset) as *mut T, x); - self.offset += core::mem::size_of::() as isize; - } - } -} - -impl<'a> CodeSink for MemoryCodeSink<'a> { - fn offset(&self) -> CodeOffset { - self.offset as CodeOffset - } - - fn put1(&mut self, x: u8) { - self.write(x); - } - - fn put2(&mut self, x: u16) { - self.write(x); - } - - fn put4(&mut self, x: u32) { - self.write(x); - } - - fn put8(&mut self, x: u64) { - self.write(x); - } - - fn reloc_external( - &mut self, - srcloc: SourceLoc, - rel: Reloc, - name: &ExternalName, - addend: Addend, - ) { - let ofs = self.offset(); - self.relocs.reloc_external(ofs, srcloc, rel, name, addend); - } - - fn trap(&mut self, code: TrapCode, srcloc: SourceLoc) { - let ofs = self.offset(); - self.traps.trap(ofs, srcloc, code); - } - - fn end_codegen(&mut self) { - self.info.total_size = self.offset(); - } - - fn add_call_site(&mut self, opcode: Opcode, loc: SourceLoc) { - debug_assert!( - opcode.is_call(), - "adding call site info for a non-call instruction." - ); - let ret_addr = self.offset(); - self.relocs.add_call_site(opcode, ret_addr, loc); - } -} - -/// A `RelocSink` implementation that does nothing, which is convenient when -/// compiling code that does not relocate anything. -#[derive(Default)] -pub struct NullRelocSink {} - -impl RelocSink for NullRelocSink { - fn reloc_external( - &mut self, - _: CodeOffset, - _: SourceLoc, - _: Reloc, - _: &ExternalName, - _: Addend, - ) { - } -} - -/// A `TrapSink` implementation that does nothing, which is convenient when -/// compiling code that does not rely on trapping semantics. -#[derive(Default)] -pub struct NullTrapSink {} - -impl TrapSink for NullTrapSink { - fn trap(&mut self, _offset: CodeOffset, _srcloc: SourceLoc, _code: TrapCode) {} -} - -/// A trait for emitting stack maps. -pub trait StackMapSink { - /// Output a bitmap of the stack representing the live reference variables at this code offset. - fn add_stack_map(&mut self, _: CodeOffset, _: StackMap); -} - -/// Placeholder StackMapSink that does nothing. -pub struct NullStackMapSink {} - -impl StackMapSink for NullStackMapSink { - fn add_stack_map(&mut self, _: CodeOffset, _: StackMap) {} -} diff --git a/cranelift/codegen/src/binemit/mod.rs b/cranelift/codegen/src/binemit/mod.rs index 157a123d7e..750eaa2a21 100644 --- a/cranelift/codegen/src/binemit/mod.rs +++ b/cranelift/codegen/src/binemit/mod.rs @@ -3,15 +3,9 @@ //! The `binemit` module contains code for translating Cranelift's intermediate representation into //! binary machine code. -mod memorysink; mod stack_map; -pub use self::memorysink::{ - MemoryCodeSink, NullRelocSink, NullStackMapSink, NullTrapSink, RelocSink, StackMapSink, - TrapSink, -}; pub use self::stack_map::StackMap; -use crate::ir::{ExternalName, Opcode, SourceLoc, TrapCode}; use core::fmt; #[cfg(feature = "enable-serde")] use serde::{Deserialize, Serialize}; @@ -99,38 +93,3 @@ pub struct CodeInfo { /// Number of bytes in total. pub total_size: CodeOffset, } - -/// Abstract interface for adding bytes to the code segment. -/// -/// A `CodeSink` will receive all of the machine code for a function. It also accepts relocations -/// which are locations in the code section that need to be fixed up when linking. -pub trait CodeSink { - /// Get the current position. - fn offset(&self) -> CodeOffset; - - /// Add 1 byte to the code section. - fn put1(&mut self, _: u8); - - /// Add 2 bytes to the code section. - fn put2(&mut self, _: u16); - - /// Add 4 bytes to the code section. - fn put4(&mut self, _: u32); - - /// Add 8 bytes to the code section. - fn put8(&mut self, _: u64); - - /// Add a relocation referencing an external symbol plus the addend at the current offset. - fn reloc_external(&mut self, _: SourceLoc, _: Reloc, _: &ExternalName, _: Addend); - - /// Add trap information for the current offset. - fn trap(&mut self, _: TrapCode, _: SourceLoc); - - /// Read-only data output is complete, we're done. - fn end_codegen(&mut self); - - /// Add a call site for a call with the given opcode, returning at the current offset. - fn add_call_site(&mut self, _: Opcode, _: SourceLoc) { - // Default implementation doesn't need to do anything. - } -} diff --git a/cranelift/codegen/src/context.rs b/cranelift/codegen/src/context.rs index 67fc48d77c..91781d1895 100644 --- a/cranelift/codegen/src/context.rs +++ b/cranelift/codegen/src/context.rs @@ -9,7 +9,7 @@ //! contexts concurrently. Typically, you would have one context per compilation thread and only a //! single ISA instance. -use crate::binemit::{CodeInfo, MemoryCodeSink, RelocSink, StackMapSink, TrapSink}; +use crate::binemit::CodeInfo; use crate::dce::do_dce; use crate::dominator_tree::DominatorTree; use crate::flowgraph::ControlFlowGraph; @@ -18,7 +18,7 @@ use crate::isa::TargetIsa; use crate::legalizer::simple_legalize; use crate::licm::do_licm; use crate::loop_analysis::LoopAnalysis; -use crate::machinst::{MachCompileResult, MachStackMap}; +use crate::machinst::MachCompileResult; use crate::nan_canonicalization::do_nan_canonicalization; use crate::remove_constant_phis::do_remove_constant_phis; use crate::result::CodegenResult; @@ -111,16 +111,11 @@ impl Context { &mut self, isa: &dyn TargetIsa, mem: &mut Vec, - relocs: &mut dyn RelocSink, - traps: &mut dyn TrapSink, - stack_maps: &mut dyn StackMapSink, ) -> CodegenResult<()> { let info = self.compile(isa)?; let old_len = mem.len(); mem.resize(old_len + info.total_size as usize, 0); - let new_info = unsafe { - self.emit_to_memory(mem.as_mut_ptr().add(old_len), relocs, traps, stack_maps) - }; + let new_info = unsafe { self.emit_to_memory(mem.as_mut_ptr().add(old_len)) }; debug_assert!(new_info == info); Ok(()) } @@ -186,32 +181,18 @@ impl Context { /// and it can't guarantee that the `mem` pointer is valid. /// /// Returns information about the emitted code and data. - pub unsafe fn emit_to_memory( - &self, - mem: *mut u8, - relocs: &mut dyn RelocSink, - traps: &mut dyn TrapSink, - stack_maps: &mut dyn StackMapSink, - ) -> CodeInfo { + #[deny(unsafe_op_in_unsafe_fn)] + pub unsafe fn emit_to_memory(&self, mem: *mut u8) -> CodeInfo { let _tt = timing::binemit(); - let mut sink = MemoryCodeSink::new(mem, relocs, traps); let result = self .mach_compile_result .as_ref() .expect("only using mach backend now"); - result.buffer.emit(&mut sink); - let info = sink.info; - // New backends do not emit StackMaps through the `CodeSink` because its interface - // requires `Value`s; instead, the `StackMap` objects are directly accessible via - // `result.buffer.stack_maps()`. - for &MachStackMap { - offset_end, - ref stack_map, - .. - } in result.buffer.stack_maps() - { - stack_maps.add_stack_map(offset_end, stack_map.clone()); - } + let info = result.code_info(); + + let mem = unsafe { std::slice::from_raw_parts_mut(mem, info.total_size as usize) }; + mem.copy_from_slice(result.buffer.data()); + info } diff --git a/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs b/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs index 915657c2f5..3d7016af2d 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs @@ -1,7 +1,6 @@ use crate::ir::types::*; use crate::ir::TrapCode; use crate::isa::aarch64::inst::*; -use crate::isa::test_utils; use crate::isa::CallConv; use crate::settings; @@ -6523,12 +6522,10 @@ fn test_aarch64_binemit() { let actual_printing = insn.show_rru(Some(&rru)); assert_eq!(expected_printing, actual_printing); - let mut sink = test_utils::TestCodeSink::new(); let mut buffer = MachBuffer::new(); insn.emit(&mut buffer, &emit_info, &mut Default::default()); let buffer = buffer.finish(); - buffer.emit(&mut sink); - let actual_encoding = &sink.stringify(); + let actual_encoding = &buffer.stringify_code_bytes(); assert_eq!(expected_encoding, actual_encoding); } } diff --git a/cranelift/codegen/src/isa/aarch64/mod.rs b/cranelift/codegen/src/isa/aarch64/mod.rs index ae19034dd9..16e45cba00 100644 --- a/cranelift/codegen/src/isa/aarch64/mod.rs +++ b/cranelift/codegen/src/isa/aarch64/mod.rs @@ -130,7 +130,7 @@ impl TargetIsa for AArch64Backend { Some(UnwindInfo::SystemV( crate::isa::unwind::systemv::create_unwind_info_from_insts( &result.buffer.unwind_info[..], - result.buffer.data.len(), + result.buffer.data().len(), &mapper, )?, )) @@ -216,7 +216,7 @@ mod test { isa_flags, ); let buffer = backend.compile_function(&mut func, false).unwrap().buffer; - let code = &buffer.data[..]; + let code = buffer.data(); // mov x1, #0x1234 // add w0, w0, w1 @@ -271,7 +271,7 @@ mod test { let result = backend .compile_function(&mut func, /* want_disasm = */ false) .unwrap(); - let code = &result.buffer.data[..]; + let code = result.buffer.data(); // mov x1, #0x1234 // #4660 // add w0, w0, w1 diff --git a/cranelift/codegen/src/isa/arm32/inst/emit_tests.rs b/cranelift/codegen/src/isa/arm32/inst/emit_tests.rs index 73269be999..421cc2e548 100644 --- a/cranelift/codegen/src/isa/arm32/inst/emit_tests.rs +++ b/cranelift/codegen/src/isa/arm32/inst/emit_tests.rs @@ -1,5 +1,4 @@ use crate::isa::arm32::inst::*; -use crate::isa::test_utils; use crate::settings; use alloc::vec::Vec; @@ -1948,12 +1947,10 @@ fn test_arm32_emit() { // Check the printed text is as expected. let actual_printing = insn.show_rru(Some(&rru)); assert_eq!(expected_printing, actual_printing); - let mut sink = test_utils::TestCodeSink::new(); let mut buffer = MachBuffer::new(); insn.emit(&mut buffer, &flags, &mut Default::default()); let buffer = buffer.finish(); - buffer.emit(&mut sink); - let actual_encoding = &sink.stringify(); + let actual_encoding = &buffer.stringify_code_bytes(); assert_eq!(expected_encoding, actual_encoding, "{}", expected_printing); } } diff --git a/cranelift/codegen/src/isa/mod.rs b/cranelift/codegen/src/isa/mod.rs index e92dc21627..f50cb83f9d 100644 --- a/cranelift/codegen/src/isa/mod.rs +++ b/cranelift/codegen/src/isa/mod.rs @@ -76,9 +76,6 @@ pub mod unwind; mod call_conv; -#[cfg(test)] -mod test_utils; - /// Returns a builder that can create a corresponding `TargetIsa` /// or `Err(LookupError::SupportDisabled)` if not enabled. macro_rules! isa_builder { diff --git a/cranelift/codegen/src/isa/s390x/inst/emit_tests.rs b/cranelift/codegen/src/isa/s390x/inst/emit_tests.rs index 15f32bed74..9d60355fd1 100644 --- a/cranelift/codegen/src/isa/s390x/inst/emit_tests.rs +++ b/cranelift/codegen/src/isa/s390x/inst/emit_tests.rs @@ -1,7 +1,6 @@ use crate::ir::MemFlags; use crate::isa::s390x::inst::*; use crate::isa::s390x::settings as s390x_settings; -use crate::isa::test_utils; use crate::settings; use alloc::vec::Vec; @@ -8112,12 +8111,10 @@ fn test_s390x_binemit() { let actual_printing = insn.show_rru(Some(&rru)); assert_eq!(expected_printing, actual_printing); - let mut sink = test_utils::TestCodeSink::new(); let mut buffer = MachBuffer::new(); insn.emit(&mut buffer, &emit_info, &mut Default::default()); let buffer = buffer.finish(); - buffer.emit(&mut sink); - let actual_encoding = &sink.stringify(); + let actual_encoding = &buffer.stringify_code_bytes(); assert_eq!(expected_encoding, actual_encoding); } } diff --git a/cranelift/codegen/src/isa/s390x/mod.rs b/cranelift/codegen/src/isa/s390x/mod.rs index dbf8679924..046e077fcd 100644 --- a/cranelift/codegen/src/isa/s390x/mod.rs +++ b/cranelift/codegen/src/isa/s390x/mod.rs @@ -136,7 +136,7 @@ impl TargetIsa for S390xBackend { Some(UnwindInfo::SystemV( crate::isa::unwind::systemv::create_unwind_info_from_insts( &result.buffer.unwind_info[..], - result.buffer.data.len(), + result.buffer.data().len(), &mapper, )?, )) @@ -225,7 +225,7 @@ mod test { let result = backend .compile_function(&mut func, /* want_disasm = */ false) .unwrap(); - let code = &result.buffer.data[..]; + let code = result.buffer.data(); // ahi %r2, 0x1234 // br %r14 @@ -277,7 +277,7 @@ mod test { let result = backend .compile_function(&mut func, /* want_disasm = */ false) .unwrap(); - let code = &result.buffer.data[..]; + let code = result.buffer.data(); // FIXME: the branching logic should be optimized more diff --git a/cranelift/codegen/src/isa/test_utils.rs b/cranelift/codegen/src/isa/test_utils.rs deleted file mode 100644 index ed5eed09b4..0000000000 --- a/cranelift/codegen/src/isa/test_utils.rs +++ /dev/null @@ -1,74 +0,0 @@ -// This is unused when no platforms with the new backend are enabled. -#![allow(dead_code)] - -use crate::binemit::{Addend, CodeOffset, CodeSink, Reloc}; -use crate::ir::{ExternalName, Opcode, SourceLoc, TrapCode}; - -use alloc::vec::Vec; -use std::string::String; - -pub struct TestCodeSink { - bytes: Vec, -} - -impl TestCodeSink { - /// Create a new TestCodeSink. - pub fn new() -> TestCodeSink { - TestCodeSink { bytes: vec![] } - } - - /// Return the code emitted to this sink as a hex string. - pub fn stringify(&self) -> String { - // This is pretty lame, but whatever .. - use std::fmt::Write; - let mut s = String::with_capacity(self.bytes.len() * 2); - for b in &self.bytes { - write!(&mut s, "{:02X}", b).unwrap(); - } - s - } -} - -impl CodeSink for TestCodeSink { - fn offset(&self) -> CodeOffset { - self.bytes.len() as CodeOffset - } - - fn put1(&mut self, x: u8) { - self.bytes.push(x); - } - - fn put2(&mut self, x: u16) { - self.bytes.push((x >> 0) as u8); - self.bytes.push((x >> 8) as u8); - } - - fn put4(&mut self, mut x: u32) { - for _ in 0..4 { - self.bytes.push(x as u8); - x >>= 8; - } - } - - fn put8(&mut self, mut x: u64) { - for _ in 0..8 { - self.bytes.push(x as u8); - x >>= 8; - } - } - - fn reloc_external( - &mut self, - _srcloc: SourceLoc, - _rel: Reloc, - _name: &ExternalName, - _addend: Addend, - ) { - } - - fn trap(&mut self, _code: TrapCode, _srcloc: SourceLoc) {} - - fn end_codegen(&mut self) {} - - fn add_call_site(&mut self, _opcode: Opcode, _srcloc: SourceLoc) {} -} diff --git a/cranelift/codegen/src/isa/x64/inst/emit_tests.rs b/cranelift/codegen/src/isa/x64/inst/emit_tests.rs index acb335b80b..e8f905bcc5 100644 --- a/cranelift/codegen/src/isa/x64/inst/emit_tests.rs +++ b/cranelift/codegen/src/isa/x64/inst/emit_tests.rs @@ -13,7 +13,6 @@ //! -- isa::x64::inst::emit_tests::test_x64_emit use super::*; -use crate::isa::test_utils; use crate::isa::x64; use alloc::vec::Vec; @@ -4460,7 +4459,6 @@ fn test_x64_emit() { // Check the printed text is as expected. let actual_printing = insn.show_rru(Some(&rru)); assert_eq!(expected_printing, actual_printing); - let mut sink = test_utils::TestCodeSink::new(); let mut buffer = MachBuffer::new(); insn.emit(&mut buffer, &emit_info, &mut Default::default()); @@ -4470,8 +4468,7 @@ fn test_x64_emit() { buffer.bind_label(label); let buffer = buffer.finish(); - buffer.emit(&mut sink); - let actual_encoding = &sink.stringify(); + let actual_encoding = &buffer.stringify_code_bytes(); assert_eq!(expected_encoding, actual_encoding, "{}", expected_printing); } } diff --git a/cranelift/codegen/src/isa/x64/mod.rs b/cranelift/codegen/src/isa/x64/mod.rs index 7e4c4f277b..5fe00926a0 100644 --- a/cranelift/codegen/src/isa/x64/mod.rs +++ b/cranelift/codegen/src/isa/x64/mod.rs @@ -122,7 +122,7 @@ impl TargetIsa for X64Backend { Some(UnwindInfo::SystemV( crate::isa::unwind::systemv::create_unwind_info_from_insts( &result.buffer.unwind_info[..], - result.buffer.data.len(), + result.buffer.data().len(), &mapper, )?, )) diff --git a/cranelift/codegen/src/lib.rs b/cranelift/codegen/src/lib.rs index 426a9c0c4d..3d5ca20035 100644 --- a/cranelift/codegen/src/lib.rs +++ b/cranelift/codegen/src/lib.rs @@ -88,7 +88,7 @@ pub mod verifier; pub mod write; pub use crate::entity::packed_option; -pub use crate::machinst::buffer::MachSrcLoc; +pub use crate::machinst::buffer::{MachCallSite, MachReloc, MachSrcLoc, MachStackMap, MachTrap}; pub use crate::machinst::TextSectionBuilder; mod bitset; diff --git a/cranelift/codegen/src/machinst/buffer.rs b/cranelift/codegen/src/machinst/buffer.rs index 298ee90cd1..7091d880dc 100644 --- a/cranelift/codegen/src/machinst/buffer.rs +++ b/cranelift/codegen/src/machinst/buffer.rs @@ -140,7 +140,7 @@ //! Given these invariants, we argue why each optimization preserves execution //! semantics below (grep for "Preserves execution semantics"). -use crate::binemit::{Addend, CodeOffset, CodeSink, Reloc, StackMap}; +use crate::binemit::{Addend, CodeOffset, Reloc, StackMap}; use crate::ir::{ExternalName, Opcode, SourceLoc, TrapCode}; use crate::isa::unwind::UnwindInst; use crate::machinst::{ @@ -233,7 +233,7 @@ pub struct MachBuffer { /// without fixups. This allows the type to be independent of the backend. pub struct MachBufferFinalized { /// The buffer contents, as raw bytes. - pub data: SmallVec<[u8; 1024]>, + data: SmallVec<[u8; 1024]>, /// Any relocations referring to this code. Note that only *external* /// relocations are tracked here; references to labels within the buffer are /// resolved before emission. @@ -1350,6 +1350,10 @@ impl MachBuffer { /// Add a call-site record at the current offset. pub fn add_call_site(&mut self, srcloc: SourceLoc, opcode: Opcode) { + debug_assert!( + opcode.is_call(), + "adding call site info for a non-call instruction." + ); self.call_sites.push(MachCallSite { ret_addr: self.data.len() as CodeOffset, srcloc, @@ -1420,8 +1424,19 @@ impl MachBufferFinalized { self.data.len() as CodeOffset } - /// Emit this buffer to the given CodeSink. - pub fn emit(&self, sink: &mut CS) { + /// Return the code in this mach buffer as a hex string for testing purposes. + pub fn stringify_code_bytes(&self) -> String { + // This is pretty lame, but whatever .. + use std::fmt::Write; + let mut s = String::with_capacity(self.data.len() * 2); + for b in &self.data { + write!(&mut s, "{:02X}", b).unwrap(); + } + s + } + + /// Get the code bytes. + pub fn data(&self) -> &[u8] { // N.B.: we emit every section into the .text section as far as // the `CodeSink` is concerned; we do not bother to segregate // the contents into the actual program text, the jumptable and the @@ -1433,40 +1448,28 @@ impl MachBufferFinalized { // add this designation and segregate the output; take care, however, // to add the appropriate relocations in this case. - let mut next_reloc = 0; - let mut next_trap = 0; - let mut next_call_site = 0; - for (idx, byte) in self.data.iter().enumerate() { - while next_reloc < self.relocs.len() - && self.relocs[next_reloc].offset == idx as CodeOffset - { - let reloc = &self.relocs[next_reloc]; - sink.reloc_external(reloc.srcloc, reloc.kind, &reloc.name, reloc.addend); - next_reloc += 1; - } - while next_trap < self.traps.len() && self.traps[next_trap].offset == idx as CodeOffset - { - let trap = &self.traps[next_trap]; - sink.trap(trap.code, trap.srcloc); - next_trap += 1; - } - while next_call_site < self.call_sites.len() - && self.call_sites[next_call_site].ret_addr == idx as CodeOffset - { - let call_site = &self.call_sites[next_call_site]; - sink.add_call_site(call_site.opcode, call_site.srcloc); - next_call_site += 1; - } - sink.put1(*byte); - } + &self.data[..] + } - sink.end_codegen(); + /// Get the list of external relocations for this code. + pub fn relocs(&self) -> &[MachReloc] { + &self.relocs[..] + } + + /// Get the list of trap records for this code. + pub fn traps(&self) -> &[MachTrap] { + &self.traps[..] } /// Get the stack map metadata for this code. pub fn stack_maps(&self) -> &[MachStackMap] { &self.stack_maps[..] } + + /// Get the list of call sites for this code. + pub fn call_sites(&self) -> &[MachCallSite] { + &self.call_sites[..] + } } /// A constant that is deferred to the next constant-pool opportunity. @@ -1496,39 +1499,42 @@ struct MachLabelFixup { } /// A relocation resulting from a compilation. -struct MachReloc { +#[derive(Clone, Debug)] +pub struct MachReloc { /// The offset at which the relocation applies, *relative to the /// containing section*. - offset: CodeOffset, + pub offset: CodeOffset, /// The original source location. - srcloc: SourceLoc, + pub srcloc: SourceLoc, /// The kind of relocation. - kind: Reloc, + pub kind: Reloc, /// The external symbol / name to which this relocation refers. - name: ExternalName, + pub name: ExternalName, /// The addend to add to the symbol value. - addend: i64, + pub addend: i64, } /// A trap record resulting from a compilation. -struct MachTrap { +#[derive(Clone, Debug)] +pub struct MachTrap { /// The offset at which the trap instruction occurs, *relative to the /// containing section*. - offset: CodeOffset, + pub offset: CodeOffset, /// The original source location. - srcloc: SourceLoc, + pub srcloc: SourceLoc, /// The trap code. - code: TrapCode, + pub code: TrapCode, } /// A call site record resulting from a compilation. -struct MachCallSite { +#[derive(Clone, Debug)] +pub struct MachCallSite { /// The offset of the call's return address, *relative to the containing section*. - ret_addr: CodeOffset, + pub ret_addr: CodeOffset, /// The original source location. - srcloc: SourceLoc, + pub srcloc: SourceLoc, /// The call's opcode. - opcode: Opcode, + pub opcode: Opcode, } /// A source-location mapping resulting from a compilation. @@ -2060,54 +2066,31 @@ mod test { let buf = buf.finish(); - #[derive(Default)] - struct TestCodeSink { - offset: CodeOffset, - traps: Vec<(CodeOffset, TrapCode)>, - callsites: Vec<(CodeOffset, Opcode)>, - relocs: Vec<(CodeOffset, Reloc)>, - } - impl CodeSink for TestCodeSink { - fn offset(&self) -> CodeOffset { - self.offset - } - fn put1(&mut self, _: u8) { - self.offset += 1; - } - fn put2(&mut self, _: u16) { - self.offset += 2; - } - fn put4(&mut self, _: u32) { - self.offset += 4; - } - fn put8(&mut self, _: u64) { - self.offset += 8; - } - fn reloc_external(&mut self, _: SourceLoc, r: Reloc, _: &ExternalName, _: Addend) { - self.relocs.push((self.offset, r)); - } - fn trap(&mut self, t: TrapCode, _: SourceLoc) { - self.traps.push((self.offset, t)); - } - fn end_codegen(&mut self) {} - fn add_call_site(&mut self, op: Opcode, _: SourceLoc) { - self.callsites.push((self.offset, op)); - } - } - - let mut sink = TestCodeSink::default(); - buf.emit(&mut sink); - - assert_eq!(sink.offset, 4); + assert_eq!(buf.data(), &[1, 2, 3, 4]); assert_eq!( - sink.traps, + buf.traps() + .iter() + .map(|trap| (trap.offset, trap.code)) + .collect::>(), vec![ (1, TrapCode::HeapOutOfBounds), (2, TrapCode::IntegerOverflow), (2, TrapCode::IntegerDivisionByZero) ] ); - assert_eq!(sink.callsites, vec![(2, Opcode::Call),]); - assert_eq!(sink.relocs, vec![(2, Reloc::Abs4), (3, Reloc::Abs8)]); + assert_eq!( + buf.call_sites() + .iter() + .map(|call_site| (call_site.ret_addr, call_site.opcode)) + .collect::>(), + vec![(2, Opcode::Call)] + ); + assert_eq!( + buf.relocs() + .iter() + .map(|reloc| (reloc.offset, reloc.kind)) + .collect::>(), + vec![(2, Reloc::Abs4), (3, Reloc::Abs8)] + ); } } diff --git a/cranelift/filetests/src/function_runner.rs b/cranelift/filetests/src/function_runner.rs index 4046072a23..116dd73774 100644 --- a/cranelift/filetests/src/function_runner.rs +++ b/cranelift/filetests/src/function_runner.rs @@ -1,6 +1,5 @@ //! Provides functionality for compiling and running CLIF IR for `run` tests. use core::mem; -use cranelift_codegen::binemit::{NullRelocSink, NullStackMapSink, NullTrapSink}; use cranelift_codegen::data_value::DataValue; use cranelift_codegen::ir::{condcodes::IntCC, Function, InstBuilder, Signature}; use cranelift_codegen::isa::TargetIsa; @@ -241,14 +240,11 @@ fn compile(function: Function, isa: &dyn TargetIsa) -> Result binemit::CodeOffset { - self.offset - } - - fn put1(&mut self, _: u8) { - self.offset += 1; - } - - fn put2(&mut self, _: u16) { - self.offset += 2; - } - - fn put4(&mut self, _: u32) { - self.offset += 4; - } - - fn put8(&mut self, _: u64) { - self.offset += 8; - } - - fn reloc_external( - &mut self, - _srcloc: ir::SourceLoc, - _reloc: binemit::Reloc, - _name: &ir::ExternalName, - _addend: binemit::Addend, - ) { - } - fn trap(&mut self, _code: ir::TrapCode, _srcloc: ir::SourceLoc) {} - fn end_codegen(&mut self) {} -} - fn check_precise_output(text: &str, context: &Context) -> Result<()> { let actual = text.lines().collect::>(); diff --git a/cranelift/jit/examples/jit-minimal.rs b/cranelift/jit/examples/jit-minimal.rs index aaba5a7ea9..37ba2e228b 100644 --- a/cranelift/jit/examples/jit-minimal.rs +++ b/cranelift/jit/examples/jit-minimal.rs @@ -1,5 +1,4 @@ use cranelift::prelude::*; -use cranelift_codegen::binemit::{NullStackMapSink, NullTrapSink}; use cranelift_codegen::settings::{self, Configurable}; use cranelift_jit::{JITBuilder, JITModule}; use cranelift_module::{default_libcall_names, Linkage, Module}; @@ -48,11 +47,7 @@ fn main() { bcx.seal_all_blocks(); bcx.finalize(); } - let mut trap_sink = NullTrapSink {}; - let mut stack_map_sink = NullStackMapSink {}; - module - .define_function(func_a, &mut ctx, &mut trap_sink, &mut stack_map_sink) - .unwrap(); + module.define_function(func_a, &mut ctx).unwrap(); module.clear_context(&mut ctx); ctx.func.signature = sig_b; @@ -74,9 +69,7 @@ fn main() { bcx.seal_all_blocks(); bcx.finalize(); } - module - .define_function(func_b, &mut ctx, &mut trap_sink, &mut stack_map_sink) - .unwrap(); + module.define_function(func_b, &mut ctx).unwrap(); module.clear_context(&mut ctx); // Perform linking. diff --git a/cranelift/jit/src/backend.rs b/cranelift/jit/src/backend.rs index 90f308bf58..ed45734136 100644 --- a/cranelift/jit/src/backend.rs +++ b/cranelift/jit/src/backend.rs @@ -3,15 +3,15 @@ use crate::{compiled_blob::CompiledBlob, memory::Memory}; use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::settings::Configurable; -use cranelift_codegen::{self, ir, settings}; +use cranelift_codegen::{self, ir, settings, MachReloc}; use cranelift_codegen::{ - binemit::{Addend, CodeInfo, CodeOffset, Reloc, RelocSink, StackMapSink, TrapSink}, + binemit::{CodeInfo, Reloc}, CodegenError, }; use cranelift_entity::SecondaryMap; use cranelift_module::{ DataContext, DataDescription, DataId, FuncId, Init, Linkage, Module, ModuleCompiledFunction, - ModuleDeclarations, ModuleError, ModuleResult, RelocRecord, + ModuleDeclarations, ModuleError, ModuleResult, }; use log::info; use std::collections::HashMap; @@ -648,8 +648,6 @@ impl Module for JITModule { &mut self, id: FuncId, ctx: &mut cranelift_codegen::Context, - trap_sink: &mut dyn TrapSink, - stack_map_sink: &mut dyn StackMapSink, ) -> ModuleResult { info!("defining function {}: {}", id, ctx.func.display()); let decl = self.declarations.get_function_decl(id); @@ -673,15 +671,17 @@ impl Module for JITModule { .allocate(size, EXECUTABLE_DATA_ALIGNMENT) .expect("TODO: handle OOM etc."); - let mut reloc_sink = JITRelocSink::default(); - unsafe { ctx.emit_to_memory(ptr, &mut reloc_sink, trap_sink, stack_map_sink) }; + unsafe { ctx.emit_to_memory(ptr) }; + let relocs = ctx + .mach_compile_result + .as_ref() + .unwrap() + .buffer + .relocs() + .to_vec(); self.record_function_for_perf(ptr, size, &decl.name); - self.compiled_functions[id] = Some(CompiledBlob { - ptr, - size, - relocs: reloc_sink.relocs, - }); + self.compiled_functions[id] = Some(CompiledBlob { ptr, size, relocs }); if self.isa.flags().is_pic() { self.pending_got_updates.push(GotUpdate { @@ -721,7 +721,7 @@ impl Module for JITModule { &mut self, id: FuncId, bytes: &[u8], - relocs: &[RelocRecord], + relocs: &[MachReloc], ) -> ModuleResult { info!("defining function {} with bytes", id); let total_size: u32 = match bytes.len().try_into() { @@ -888,26 +888,3 @@ fn lookup_with_dlsym(name: &str) -> Option<*const u8> { None } } - -#[derive(Default)] -struct JITRelocSink { - relocs: Vec, -} - -impl RelocSink for JITRelocSink { - fn reloc_external( - &mut self, - offset: CodeOffset, - _srcloc: ir::SourceLoc, - reloc: Reloc, - name: &ir::ExternalName, - addend: Addend, - ) { - self.relocs.push(RelocRecord { - offset, - reloc, - name: name.clone(), - addend, - }); - } -} diff --git a/cranelift/jit/src/compiled_blob.rs b/cranelift/jit/src/compiled_blob.rs index 5af0067422..c5a62701f8 100644 --- a/cranelift/jit/src/compiled_blob.rs +++ b/cranelift/jit/src/compiled_blob.rs @@ -1,13 +1,13 @@ use cranelift_codegen::binemit::Reloc; use cranelift_codegen::ir::ExternalName; -use cranelift_module::RelocRecord; +use cranelift_codegen::MachReloc; use std::convert::TryFrom; #[derive(Clone)] pub(crate) struct CompiledBlob { pub(crate) ptr: *mut u8, pub(crate) size: usize, - pub(crate) relocs: Vec, + pub(crate) relocs: Vec, } impl CompiledBlob { @@ -19,16 +19,17 @@ impl CompiledBlob { ) { use std::ptr::write_unaligned; - for &RelocRecord { - reloc, + for &MachReloc { + kind, offset, + srcloc: _, ref name, addend, } in &self.relocs { debug_assert!((offset as usize) < self.size); let at = unsafe { self.ptr.offset(isize::try_from(offset).unwrap()) }; - match reloc { + match kind { Reloc::Abs4 => { let base = get_address(name); let what = unsafe { base.offset(isize::try_from(addend).unwrap()) }; diff --git a/cranelift/jit/tests/basic.rs b/cranelift/jit/tests/basic.rs index f53480702d..a1a038f6fe 100644 --- a/cranelift/jit/tests/basic.rs +++ b/cranelift/jit/tests/basic.rs @@ -1,4 +1,3 @@ -use cranelift_codegen::binemit::{NullStackMapSink, NullTrapSink}; use cranelift_codegen::ir::*; use cranelift_codegen::isa::CallConv; use cranelift_codegen::settings::{self, Configurable}; @@ -56,11 +55,7 @@ fn define_simple_function(module: &mut JITModule) -> FuncId { bcx.ins().return_(&[]); } - let mut trap_sink = NullTrapSink {}; - let mut stack_map_sink = NullStackMapSink {}; - module - .define_function(func_id, &mut ctx, &mut trap_sink, &mut stack_map_sink) - .unwrap(); + module.define_function(func_id, &mut ctx).unwrap(); func_id } @@ -205,11 +200,7 @@ fn libcall_function() { bcx.ins().return_(&[]); } - let mut trap_sink = NullTrapSink {}; - let mut stack_map_sink = NullStackMapSink {}; - module - .define_function(func_id, &mut ctx, &mut trap_sink, &mut stack_map_sink) - .unwrap(); + module.define_function(func_id, &mut ctx).unwrap(); module.finalize_definitions(); } diff --git a/cranelift/module/src/data_context.rs b/cranelift/module/src/data_context.rs index ce9536b1b0..d872819b9e 100644 --- a/cranelift/module/src/data_context.rs +++ b/cranelift/module/src/data_context.rs @@ -2,14 +2,13 @@ use cranelift_codegen::binemit::{Addend, CodeOffset, Reloc}; use cranelift_codegen::entity::PrimaryMap; -use cranelift_codegen::ir; +use cranelift_codegen::ir::{self, SourceLoc}; +use cranelift_codegen::MachReloc; use std::borrow::ToOwned; use std::boxed::Box; use std::string::String; use std::vec::Vec; -use crate::RelocRecord; - /// This specifies how data is to be initialized. #[derive(PartialEq, Eq, Debug)] pub enum Init { @@ -59,25 +58,24 @@ pub struct DataDescription { impl DataDescription { /// An iterator over all relocations of the data object. - pub fn all_relocs<'a>( - &'a self, - pointer_reloc: Reloc, - ) -> impl Iterator + 'a { + pub fn all_relocs<'a>(&'a self, pointer_reloc: Reloc) -> impl Iterator + 'a { let func_relocs = self .function_relocs .iter() - .map(move |&(offset, id)| RelocRecord { - reloc: pointer_reloc, + .map(move |&(offset, id)| MachReloc { + kind: pointer_reloc, offset, + srcloc: SourceLoc::default(), name: self.function_decls[id].clone(), addend: 0, }); let data_relocs = self .data_relocs .iter() - .map(move |&(offset, id, addend)| RelocRecord { - reloc: pointer_reloc, + .map(move |&(offset, id, addend)| MachReloc { + kind: pointer_reloc, offset, + srcloc: SourceLoc::default(), name: self.data_decls[id].clone(), addend, }); diff --git a/cranelift/module/src/lib.rs b/cranelift/module/src/lib.rs index 21d1c9df27..82562ae5df 100644 --- a/cranelift/module/src/lib.rs +++ b/cranelift/module/src/lib.rs @@ -43,7 +43,7 @@ mod traps; pub use crate::data_context::{DataContext, DataDescription, Init}; pub use crate::module::{ DataId, FuncId, FuncOrDataId, Linkage, Module, ModuleCompiledFunction, ModuleDeclarations, - ModuleError, ModuleResult, RelocRecord, + ModuleError, ModuleResult, }; pub use crate::traps::TrapSite; diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index 191d468ed7..336307a3a2 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -7,8 +7,8 @@ use super::HashMap; use crate::data_context::DataContext; -use cranelift_codegen::binemit; use cranelift_codegen::entity::{entity_impl, PrimaryMap}; +use cranelift_codegen::{binemit, MachReloc}; use cranelift_codegen::{ir, isa, CodegenError, Context}; use std::borrow::ToOwned; use std::string::String; @@ -416,19 +416,6 @@ pub struct ModuleCompiledFunction { pub size: binemit::CodeOffset, } -/// A record of a relocation to perform. -#[derive(Clone)] -pub struct RelocRecord { - /// Where in the generated code this relocation is to be applied. - pub offset: binemit::CodeOffset, - /// The kind of relocation this represents. - pub reloc: binemit::Reloc, - /// What symbol we're relocating against. - pub name: ir::ExternalName, - /// The offset to add to the relocation. - pub addend: binemit::Addend, -} - /// A `Module` is a utility for collecting functions and data objects, and linking them together. pub trait Module { /// Return the `TargetIsa` to compile for. @@ -554,8 +541,6 @@ pub trait Module { &mut self, func: FuncId, ctx: &mut Context, - trap_sink: &mut dyn binemit::TrapSink, - stack_map_sink: &mut dyn binemit::StackMapSink, ) -> ModuleResult; /// Define a function, taking the function body from the given `bytes`. @@ -569,7 +554,7 @@ pub trait Module { &mut self, func: FuncId, bytes: &[u8], - relocs: &[RelocRecord], + relocs: &[MachReloc], ) -> ModuleResult; /// Define a data object, producing the data contents from the given `DataContext`. @@ -656,17 +641,15 @@ impl Module for &mut M { &mut self, func: FuncId, ctx: &mut Context, - trap_sink: &mut dyn binemit::TrapSink, - stack_map_sink: &mut dyn binemit::StackMapSink, ) -> ModuleResult { - (**self).define_function(func, ctx, trap_sink, stack_map_sink) + (**self).define_function(func, ctx) } fn define_function_bytes( &mut self, func: FuncId, bytes: &[u8], - relocs: &[RelocRecord], + relocs: &[MachReloc], ) -> ModuleResult { (**self).define_function_bytes(func, bytes, relocs) } diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index 57c450de49..0998d829ee 100644 --- a/cranelift/object/src/backend.rs +++ b/cranelift/object/src/backend.rs @@ -3,14 +3,14 @@ use anyhow::anyhow; use cranelift_codegen::entity::SecondaryMap; use cranelift_codegen::isa::TargetIsa; -use cranelift_codegen::{self, ir}; +use cranelift_codegen::{self, ir, MachReloc}; use cranelift_codegen::{ - binemit::{Addend, CodeOffset, Reloc, RelocSink, StackMapSink, TrapSink}, + binemit::{Addend, CodeOffset, Reloc}, CodegenError, }; use cranelift_module::{ DataContext, DataDescription, DataId, FuncId, Init, Linkage, Module, ModuleCompiledFunction, - ModuleDeclarations, ModuleError, ModuleResult, RelocRecord, + ModuleDeclarations, ModuleError, ModuleResult, }; use log::info; use object::write::{ @@ -307,29 +307,24 @@ impl Module for ObjectModule { &mut self, func_id: FuncId, ctx: &mut cranelift_codegen::Context, - trap_sink: &mut dyn TrapSink, - stack_map_sink: &mut dyn StackMapSink, ) -> ModuleResult { info!("defining function {}: {}", func_id, ctx.func.display()); let mut code: Vec = Vec::new(); - let mut reloc_sink = ObjectRelocSink::default(); - ctx.compile_and_emit( - self.isa(), - &mut code, - &mut reloc_sink, - trap_sink, - stack_map_sink, - )?; + ctx.compile_and_emit(self.isa(), &mut code)?; - self.define_function_bytes(func_id, &code, &reloc_sink.relocs) + self.define_function_bytes( + func_id, + &code, + ctx.mach_compile_result.as_ref().unwrap().buffer.relocs(), + ) } fn define_function_bytes( &mut self, func_id: FuncId, bytes: &[u8], - relocs: &[RelocRecord], + relocs: &[MachReloc], ) -> ModuleResult { info!("defining function {} with bytes", func_id); let total_size: u32 = match bytes.len().try_into() { @@ -560,9 +555,9 @@ impl ObjectModule { } } - fn process_reloc(&self, record: &RelocRecord) -> ObjectRelocRecord { + fn process_reloc(&self, record: &MachReloc) -> ObjectRelocRecord { let mut addend = record.addend; - let (kind, encoding, size) = match record.reloc { + let (kind, encoding, size) = match record.kind { Reloc::Abs4 => (RelocationKind::Absolute, RelocationEncoding::Generic, 32), Reloc::Abs8 => (RelocationKind::Absolute, RelocationEncoding::Generic, 64), Reloc::X86PCRel4 => (RelocationKind::Relative, RelocationEncoding::Generic, 32), @@ -707,26 +702,3 @@ struct ObjectRelocRecord { size: u8, addend: Addend, } - -#[derive(Default)] -struct ObjectRelocSink { - relocs: Vec, -} - -impl RelocSink for ObjectRelocSink { - fn reloc_external( - &mut self, - offset: CodeOffset, - _srcloc: ir::SourceLoc, - reloc: Reloc, - name: &ir::ExternalName, - addend: Addend, - ) { - self.relocs.push(RelocRecord { - offset, - reloc, - addend, - name: name.clone(), - }) - } -} diff --git a/cranelift/object/tests/basic.rs b/cranelift/object/tests/basic.rs index a5add02295..8a3764f9b8 100644 --- a/cranelift/object/tests/basic.rs +++ b/cranelift/object/tests/basic.rs @@ -1,9 +1,6 @@ use cranelift_codegen::ir::*; use cranelift_codegen::isa::CallConv; -use cranelift_codegen::{ - binemit::{NullStackMapSink, NullTrapSink}, - settings, -}; +use cranelift_codegen::settings; use cranelift_codegen::{ir::types::I16, Context}; use cranelift_entity::EntityRef; use cranelift_frontend::*; @@ -53,11 +50,7 @@ fn define_simple_function(module: &mut ObjectModule) -> FuncId { bcx.ins().return_(&[]); } - let mut trap_sink = NullTrapSink {}; - let mut stack_map_sink = NullStackMapSink {}; - module - .define_function(func_id, &mut ctx, &mut trap_sink, &mut stack_map_sink) - .unwrap(); + module.define_function(func_id, &mut ctx).unwrap(); func_id } @@ -194,11 +187,7 @@ fn libcall_function() { bcx.ins().return_(&[]); } - let mut trap_sink = NullTrapSink {}; - let mut stack_map_sink = NullStackMapSink {}; - module - .define_function(func_id, &mut ctx, &mut trap_sink, &mut stack_map_sink) - .unwrap(); + module.define_function(func_id, &mut ctx).unwrap(); module.finish(); } diff --git a/cranelift/src/bugpoint.rs b/cranelift/src/bugpoint.rs index e16e91ae40..995e416764 100644 --- a/cranelift/src/bugpoint.rs +++ b/cranelift/src/bugpoint.rs @@ -1,6 +1,5 @@ //! CLI tool to reduce Cranelift IR files crashing during compilation. -use crate::disasm::{PrintRelocs, PrintStackMaps, PrintTraps}; use crate::utils::{parse_sets_and_triple, read_to_string}; use anyhow::{Context as _, Result}; use cranelift_codegen::cursor::{Cursor, FuncCursor}; @@ -1029,17 +1028,9 @@ impl<'a> CrashCheckContext<'a> { std::panic::set_hook(Box::new(|_| {})); // silence panics let res = match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - let mut relocs = PrintRelocs::new(false); - let mut traps = PrintTraps::new(false); - let mut stack_maps = PrintStackMaps::new(false); - - let _ = self.context.compile_and_emit( - self.isa, - &mut self.code_memory, - &mut relocs, - &mut traps, - &mut stack_maps, - ); + let _ = self + .context + .compile_and_emit(self.isa, &mut self.code_memory); })) { Ok(()) => CheckResult::Succeed, Err(err) => CheckResult::Crash(get_panic_string(err)), diff --git a/cranelift/src/compile.rs b/cranelift/src/compile.rs index c1993e8b65..be1d1f3d51 100644 --- a/cranelift/src/compile.rs +++ b/cranelift/src/compile.rs @@ -1,6 +1,6 @@ //! CLI tool to read Cranelift IR files and compile them into native code. -use crate::disasm::{print_all, PrintRelocs, PrintStackMaps, PrintTraps}; +use crate::disasm::print_all; use crate::utils::{parse_sets_and_triple, read_to_string}; use anyhow::{Context as _, Result}; use cranelift_codegen::print_errors::pretty_error; @@ -68,10 +68,6 @@ fn handle_module(options: &Options, path: &Path, name: &str, fisa: FlagsOrIsa) - }; for (func, _) in test_file.functions { - let mut relocs = PrintRelocs::new(options.print); - let mut traps = PrintTraps::new(options.print); - let mut stack_maps = PrintStackMaps::new(options.print); - if let Some(isa) = isa { let mut context = Context::new(); context.func = func; @@ -79,9 +75,10 @@ fn handle_module(options: &Options, path: &Path, name: &str, fisa: FlagsOrIsa) - // Compile and encode the result to machine code. context - .compile_and_emit(isa, &mut mem, &mut relocs, &mut traps, &mut stack_maps) + .compile_and_emit(isa, &mut mem) .map_err(|err| anyhow::anyhow!("{}", pretty_error(&context.func, err)))?; - let code_info = context.mach_compile_result.as_ref().unwrap().code_info(); + let result = context.mach_compile_result.as_ref().unwrap(); + let code_info = result.code_info(); if options.print { println!("{}", context.func.display()); @@ -92,9 +89,10 @@ fn handle_module(options: &Options, path: &Path, name: &str, fisa: FlagsOrIsa) - isa, &mem, code_info.total_size, - &relocs, - &traps, - &stack_maps, + options.print, + result.buffer.relocs(), + result.buffer.traps(), + result.buffer.stack_maps(), )?; } } diff --git a/cranelift/src/disasm.rs b/cranelift/src/disasm.rs index 7a108c5d80..174fb88651 100644 --- a/cranelift/src/disasm.rs +++ b/cranelift/src/disasm.rs @@ -1,85 +1,53 @@ use anyhow::Result; use cfg_if::cfg_if; use cranelift_codegen::isa::TargetIsa; -use cranelift_codegen::{binemit, ir}; +use cranelift_codegen::{MachReloc, MachStackMap, MachTrap}; use std::fmt::Write; -pub struct PrintRelocs { - pub flag_print: bool, - pub text: String, -} - -impl PrintRelocs { - pub fn new(flag_print: bool) -> Self { - Self { - flag_print, - text: String::new(), - } +pub fn print_relocs(relocs: &[MachReloc]) -> String { + let mut text = String::new(); + for &MachReloc { + kind, + offset, + srcloc: _, + ref name, + addend, + } in relocs + { + writeln!( + text, + "reloc_external: {} {} {} at {}", + kind, name, addend, offset + ) + .unwrap(); } + text } -impl binemit::RelocSink for PrintRelocs { - fn reloc_external( - &mut self, - where_: binemit::CodeOffset, - _srcloc: ir::SourceLoc, - r: binemit::Reloc, - name: &ir::ExternalName, - addend: binemit::Addend, - ) { - if self.flag_print { - writeln!( - &mut self.text, - "reloc_external: {} {} {} at {}", - r, name, addend, where_ - ) - .unwrap(); - } +pub fn print_traps(traps: &[MachTrap]) -> String { + let mut text = String::new(); + for &MachTrap { + offset, + srcloc: _, + code, + } in traps + { + writeln!(text, "trap: {} at {}", code, offset).unwrap(); } + text } -pub struct PrintTraps { - pub flag_print: bool, - pub text: String, -} - -impl PrintTraps { - pub fn new(flag_print: bool) -> Self { - Self { - flag_print, - text: String::new(), - } - } -} - -impl binemit::TrapSink for PrintTraps { - fn trap(&mut self, offset: binemit::CodeOffset, _srcloc: ir::SourceLoc, code: ir::TrapCode) { - if self.flag_print { - writeln!(&mut self.text, "trap: {} at {}", code, offset).unwrap(); - } - } -} - -pub struct PrintStackMaps { - pub flag_print: bool, - pub text: String, -} - -impl PrintStackMaps { - pub fn new(flag_print: bool) -> Self { - Self { - flag_print, - text: String::new(), - } - } -} - -impl binemit::StackMapSink for PrintStackMaps { - fn add_stack_map(&mut self, offset: binemit::CodeOffset, _: binemit::StackMap) { - if self.flag_print { - writeln!(&mut self.text, "add_stack_map at {}", offset).unwrap(); - } +pub fn print_stack_maps(traps: &[MachStackMap]) -> String { + let mut text = String::new(); + for &MachStackMap { + offset, + offset_end: _, + stack_map: _, + } in traps + { + writeln!(text, "add_stack_map at {}", offset).unwrap(); } + text } cfg_if! { @@ -193,13 +161,21 @@ pub fn print_all( isa: &dyn TargetIsa, mem: &[u8], code_size: u32, - relocs: &PrintRelocs, - traps: &PrintTraps, - stack_maps: &PrintStackMaps, + print: bool, + relocs: &[MachReloc], + traps: &[MachTrap], + stack_maps: &[MachStackMap], ) -> Result<()> { print_bytes(&mem); print_disassembly(isa, &mem[0..code_size as usize])?; - println!("\n{}\n{}\n{}", &relocs.text, &traps.text, &stack_maps.text); + if print { + println!( + "\n{}\n{}\n{}", + print_relocs(relocs), + print_traps(traps), + print_stack_maps(stack_maps) + ); + } Ok(()) } diff --git a/cranelift/src/wasm.rs b/cranelift/src/wasm.rs index bb3b93b098..4fd83992b7 100644 --- a/cranelift/src/wasm.rs +++ b/cranelift/src/wasm.rs @@ -7,7 +7,7 @@ allow(clippy::too_many_arguments, clippy::cognitive_complexity) )] -use crate::disasm::{print_all, PrintRelocs, PrintStackMaps, PrintTraps}; +use crate::disasm::print_all; use crate::utils::parse_sets_and_triple; use anyhow::{Context as _, Result}; use cranelift_codegen::ir::DisplayFunctionAnnotations; @@ -258,18 +258,17 @@ fn handle_module(options: &Options, path: &Path, name: &str, fisa: FlagsOrIsa) - let mut saved_size = None; let func_index = num_func_imports + def_index.index(); let mut mem = vec![]; - let mut relocs = PrintRelocs::new(options.print); - let mut traps = PrintTraps::new(options.print); - let mut stack_maps = PrintStackMaps::new(options.print); - if options.check_translation { + let (relocs, traps, stack_maps) = if options.check_translation { if let Err(errors) = context.verify(fisa) { anyhow::bail!("{}", pretty_verifier_error(&context.func, None, errors)); } + (vec![], vec![], vec![]) } else { context - .compile_and_emit(isa, &mut mem, &mut relocs, &mut traps, &mut stack_maps) + .compile_and_emit(isa, &mut mem) .map_err(|err| anyhow::anyhow!("{}", pretty_error(&context.func, err)))?; - let code_info = context.mach_compile_result.as_ref().unwrap().code_info(); + let result = context.mach_compile_result.as_ref().unwrap(); + let code_info = result.code_info(); if options.print_size { println!( @@ -287,7 +286,12 @@ fn handle_module(options: &Options, path: &Path, name: &str, fisa: FlagsOrIsa) - if options.disasm { saved_size = Some(code_info.total_size); } - } + ( + result.buffer.relocs().to_vec(), + result.buffer.traps().to_vec(), + result.buffer.stack_maps().to_vec(), + ) + }; if options.print { vprintln!(options.verbose, ""); @@ -323,7 +327,15 @@ fn handle_module(options: &Options, path: &Path, name: &str, fisa: FlagsOrIsa) - } if let Some(total_size) = saved_size { - print_all(isa, &mem, total_size, &relocs, &traps, &stack_maps)?; + print_all( + isa, + &mem, + total_size, + options.print, + &relocs, + &traps, + &stack_maps, + )?; } context.clear(); diff --git a/crates/cranelift/src/compiler.rs b/crates/cranelift/src/compiler.rs index f02fee01d2..05da81ff50 100644 --- a/crates/cranelift/src/compiler.rs +++ b/crates/cranelift/src/compiler.rs @@ -10,9 +10,9 @@ use anyhow::{Context as _, Result}; use cranelift_codegen::ir::{self, ExternalName, InstBuilder, MemFlags}; use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::print_errors::pretty_error; -use cranelift_codegen::settings; -use cranelift_codegen::MachSrcLoc; -use cranelift_codegen::{binemit, Context}; +use cranelift_codegen::Context; +use cranelift_codegen::{settings, MachReloc, MachTrap}; +use cranelift_codegen::{MachSrcLoc, MachStackMap}; use cranelift_entity::{EntityRef, PrimaryMap}; use cranelift_frontend::FunctionBuilder; use cranelift_wasm::{ @@ -166,19 +166,28 @@ impl wasmtime_environ::Compiler for Compiler { self.save_translator(func_translator); let mut code_buf: Vec = Vec::new(); - let mut reloc_sink = RelocSink::new(); - let mut trap_sink = TrapSink::new(); - let mut stack_map_sink = StackMapSink::default(); context - .compile_and_emit( - isa, - &mut code_buf, - &mut reloc_sink, - &mut trap_sink, - &mut stack_map_sink, - ) + .compile_and_emit(isa, &mut code_buf) .map_err(|error| CompileError::Codegen(pretty_error(&context.func, error)))?; + let result = context.mach_compile_result.as_ref().unwrap(); + + let func_relocs = result + .buffer + .relocs() + .into_iter() + .map(mach_reloc_to_reloc) + .collect::>(); + + let traps = result + .buffer + .traps() + .into_iter() + .map(mach_trap_to_trap) + .collect::>(); + + let stack_maps = mach_stack_maps_to_stack_maps(result.buffer.stack_maps()); + let unwind_info = context .create_unwind_info(isa) .map_err(|error| CompileError::Codegen(pretty_error(&context.func, error)))?; @@ -206,14 +215,14 @@ impl wasmtime_environ::Compiler for Compiler { let length = u32::try_from(code_buf.len()).unwrap(); Ok(Box::new(CompiledFunction { body: code_buf, - relocations: reloc_sink.func_relocs, + relocations: func_relocs, value_labels_ranges: ranges.unwrap_or(Default::default()), stack_slots: context.func.stack_slots, unwind_info, - traps: trap_sink.traps, + traps, info: FunctionInfo { start_srcloc: address_transform.start_srcloc, - stack_maps: stack_map_sink.finish(), + stack_maps, start: 0, length, }, @@ -534,19 +543,37 @@ impl Compiler { isa: &dyn TargetIsa, ) -> Result { let mut code_buf = Vec::new(); - let mut reloc_sink = TrampolineRelocSink::default(); - let mut trap_sink = binemit::NullTrapSink {}; - let mut stack_map_sink = binemit::NullStackMapSink {}; + let mut relocs = Vec::new(); context - .compile_and_emit( - isa, - &mut code_buf, - &mut reloc_sink, - &mut trap_sink, - &mut stack_map_sink, - ) + .compile_and_emit(isa, &mut code_buf) .map_err(|error| CompileError::Codegen(pretty_error(&context.func, error)))?; + for &MachReloc { + offset, + srcloc: _, + kind, + ref name, + addend, + } in context + .mach_compile_result + .as_ref() + .unwrap() + .buffer + .relocs() + { + let reloc_target = if let ir::ExternalName::LibCall(libcall) = *name { + RelocationTarget::LibCall(libcall) + } else { + panic!("unrecognized external name") + }; + relocs.push(Relocation { + reloc: kind, + reloc_target, + offset, + addend, + }); + } + let unwind_info = context .create_unwind_info(isa) .map_err(|error| CompileError::Codegen(pretty_error(&context.func, error)))?; @@ -554,7 +581,7 @@ impl Compiler { Ok(CompiledFunction { body: code_buf, unwind_info, - relocations: reloc_sink.relocs, + relocations: relocs, stack_slots: Default::default(), value_labels_ranges: Default::default(), info: Default::default(), @@ -627,145 +654,77 @@ fn collect_address_maps( } } -/// Implementation of a relocation sink that just saves all the information for later -struct RelocSink { - /// Relocations recorded for the function. - func_relocs: Vec, -} - -impl binemit::RelocSink for RelocSink { - fn reloc_external( - &mut self, - offset: binemit::CodeOffset, - _srcloc: ir::SourceLoc, - reloc: binemit::Reloc, - name: &ExternalName, - addend: binemit::Addend, - ) { - let reloc_target = if let ExternalName::User { namespace, index } = *name { - debug_assert_eq!(namespace, 0); - RelocationTarget::UserFunc(FuncIndex::from_u32(index)) - } else if let ExternalName::LibCall(libcall) = *name { - RelocationTarget::LibCall(libcall) - } else { - panic!("unrecognized external name") - }; - self.func_relocs.push(Relocation { - reloc, - reloc_target, - offset, - addend, - }); +fn mach_reloc_to_reloc(reloc: &MachReloc) -> Relocation { + let &MachReloc { + offset, + srcloc: _, + kind, + ref name, + addend, + } = reloc; + let reloc_target = if let ExternalName::User { namespace, index } = *name { + debug_assert_eq!(namespace, 0); + RelocationTarget::UserFunc(FuncIndex::from_u32(index)) + } else if let ExternalName::LibCall(libcall) = *name { + RelocationTarget::LibCall(libcall) + } else { + panic!("unrecognized external name") + }; + Relocation { + reloc: kind, + reloc_target, + offset, + addend, } } -impl RelocSink { - /// Return a new `RelocSink` instance. - fn new() -> Self { - Self { - func_relocs: Vec::new(), - } +fn mach_trap_to_trap(trap: &MachTrap) -> TrapInformation { + let &MachTrap { + offset, + srcloc: _, + code, + } = trap; + TrapInformation { + code_offset: offset, + trap_code: match code { + ir::TrapCode::StackOverflow => TrapCode::StackOverflow, + ir::TrapCode::HeapOutOfBounds => TrapCode::HeapOutOfBounds, + ir::TrapCode::HeapMisaligned => TrapCode::HeapMisaligned, + ir::TrapCode::TableOutOfBounds => TrapCode::TableOutOfBounds, + ir::TrapCode::IndirectCallToNull => TrapCode::IndirectCallToNull, + ir::TrapCode::BadSignature => TrapCode::BadSignature, + ir::TrapCode::IntegerOverflow => TrapCode::IntegerOverflow, + ir::TrapCode::IntegerDivisionByZero => TrapCode::IntegerDivisionByZero, + ir::TrapCode::BadConversionToInteger => TrapCode::BadConversionToInteger, + ir::TrapCode::UnreachableCodeReached => TrapCode::UnreachableCodeReached, + ir::TrapCode::Interrupt => TrapCode::Interrupt, + + // these should never be emitted by wasmtime-cranelift + ir::TrapCode::User(_) => unreachable!(), + }, } } -/// Implementation of a trap sink that simply stores all trap info in-memory -#[derive(Default)] -struct TrapSink { - /// The in-memory vector of trap info - traps: Vec, -} - -impl TrapSink { - /// Create a new `TrapSink` - fn new() -> Self { - Self::default() - } -} - -impl binemit::TrapSink for TrapSink { - fn trap( - &mut self, - code_offset: binemit::CodeOffset, - _source_loc: ir::SourceLoc, - trap_code: ir::TrapCode, - ) { - self.traps.push(TrapInformation { - code_offset, - trap_code: match trap_code { - ir::TrapCode::StackOverflow => TrapCode::StackOverflow, - ir::TrapCode::HeapOutOfBounds => TrapCode::HeapOutOfBounds, - ir::TrapCode::HeapMisaligned => TrapCode::HeapMisaligned, - ir::TrapCode::TableOutOfBounds => TrapCode::TableOutOfBounds, - ir::TrapCode::IndirectCallToNull => TrapCode::IndirectCallToNull, - ir::TrapCode::BadSignature => TrapCode::BadSignature, - ir::TrapCode::IntegerOverflow => TrapCode::IntegerOverflow, - ir::TrapCode::IntegerDivisionByZero => TrapCode::IntegerDivisionByZero, - ir::TrapCode::BadConversionToInteger => TrapCode::BadConversionToInteger, - ir::TrapCode::UnreachableCodeReached => TrapCode::UnreachableCodeReached, - ir::TrapCode::Interrupt => TrapCode::Interrupt, - - // these should never be emitted by wasmtime-cranelift - ir::TrapCode::User(_) => unreachable!(), - }, - }); - } -} - -#[derive(Default)] -struct StackMapSink { - infos: Vec, -} - -impl binemit::StackMapSink for StackMapSink { - fn add_stack_map(&mut self, code_offset: binemit::CodeOffset, stack_map: binemit::StackMap) { - // This is converting from Cranelift's representation of a stack map to - // Wasmtime's representation. They happen to align today but that may - // not always be true in the future. +fn mach_stack_maps_to_stack_maps(mach_stack_maps: &[MachStackMap]) -> Vec { + // This is converting from Cranelift's representation of a stack map to + // Wasmtime's representation. They happen to align today but that may + // not always be true in the future. + let mut stack_maps = Vec::new(); + for &MachStackMap { + offset_end, + ref stack_map, + .. + } in mach_stack_maps + { let stack_map = wasmtime_environ::StackMap::new( stack_map.mapped_words(), stack_map.as_slice().iter().map(|a| a.0), ); - self.infos.push(StackMapInformation { - code_offset, + stack_maps.push(StackMapInformation { + code_offset: offset_end, stack_map, }); } -} - -impl StackMapSink { - fn finish(mut self) -> Vec { - self.infos.sort_unstable_by_key(|info| info.code_offset); - self.infos - } -} - -/// We don't expect trampoline compilation to produce many relocations, so -/// this `RelocSink` just asserts that it doesn't recieve most of them, but -/// handles libcall ones. -#[derive(Default)] -struct TrampolineRelocSink { - relocs: Vec, -} - -impl binemit::RelocSink for TrampolineRelocSink { - fn reloc_external( - &mut self, - offset: binemit::CodeOffset, - _srcloc: ir::SourceLoc, - reloc: binemit::Reloc, - name: &ir::ExternalName, - addend: binemit::Addend, - ) { - let reloc_target = if let ir::ExternalName::LibCall(libcall) = *name { - RelocationTarget::LibCall(libcall) - } else { - panic!("unrecognized external name") - }; - self.relocs.push(Relocation { - reloc, - reloc_target, - offset, - addend, - }); - } + stack_maps.sort_unstable_by_key(|info| info.code_offset); + stack_maps }