diff --git a/cranelift/codegen/src/binemit/memorysink.rs b/cranelift/codegen/src/binemit/memorysink.rs index 651921609f..149fed67bb 100644 --- a/cranelift/codegen/src/binemit/memorysink.rs +++ b/cranelift/codegen/src/binemit/memorysink.rs @@ -13,47 +13,9 @@ //! 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 super::{Addend, CodeOffset, Reloc}; use crate::binemit::stack_map::StackMap; use crate::ir::{ExternalName, 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 { - /// 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, -} - -impl<'a> MemoryCodeSink { - /// 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) -> Self { - Self { data, offset: 0 } - } - - /// Information about the generated code and read-only data. - pub fn info(&self) -> CodeInfo { - CodeInfo { - total_size: self.offset as CodeOffset, - } - } -} /// A trait for receiving relocations for code that is emitted directly into memory. pub trait RelocSink { @@ -77,15 +39,6 @@ pub trait TrapSink { fn trap(&mut self, _: CodeOffset, _: SourceLoc, _: TrapCode); } -impl CodeSink for MemoryCodeSink { - fn put1(&mut self, x: u8) { - unsafe { - write_unaligned(self.data.offset(self.offset), x); - self.offset += 1; - } - } -} - /// A `RelocSink` implementation that does nothing, which is convenient when /// compiling code that does not relocate anything. #[derive(Default)] diff --git a/cranelift/codegen/src/binemit/mod.rs b/cranelift/codegen/src/binemit/mod.rs index 2bb800976f..0c634b27af 100644 --- a/cranelift/codegen/src/binemit/mod.rs +++ b/cranelift/codegen/src/binemit/mod.rs @@ -7,8 +7,7 @@ mod memorysink; mod stack_map; pub use self::memorysink::{ - MemoryCodeSink, NullRelocSink, NullStackMapSink, NullTrapSink, RelocSink, StackMapSink, - TrapSink, + NullRelocSink, NullStackMapSink, NullTrapSink, RelocSink, StackMapSink, TrapSink, }; pub use self::stack_map::StackMap; use core::fmt; @@ -98,12 +97,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 { - /// Add 1 byte to the code section. - fn put1(&mut self, _: u8); -} diff --git a/cranelift/codegen/src/context.rs b/cranelift/codegen/src/context.rs index c896c8e737..2cebe0d3c3 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, RelocSink, StackMapSink, TrapSink}; use crate::dce::do_dce; use crate::dominator_tree::DominatorTree; use crate::flowgraph::ControlFlowGraph; @@ -186,6 +186,7 @@ impl Context { /// and it can't guarantee that the `mem` pointer is valid. /// /// Returns information about the emitted code and data. + #[deny(unsafe_op_in_unsafe_fn)] pub unsafe fn emit_to_memory( &self, mem: *mut u8, @@ -194,13 +195,15 @@ impl Context { stack_maps: &mut dyn StackMapSink, ) -> CodeInfo { let _tt = timing::binemit(); - let mut sink = MemoryCodeSink::new(mem); let result = self .mach_compile_result .as_ref() .expect("only using mach backend now"); - result.buffer.emit(&mut sink); - let info = sink.info(); + 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()); + for &MachReloc { offset, srcloc, 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/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/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/machinst/buffer.rs b/cranelift/codegen/src/machinst/buffer.rs index 6c38297a33..2eeab23f33 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. @@ -1435,8 +1435,8 @@ impl MachBufferFinalized { s } - /// Emit this buffer to the given CodeSink. - pub fn emit(&self, sink: &mut CS) { + /// 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 @@ -1448,9 +1448,7 @@ impl MachBufferFinalized { // add this designation and segregate the output; take care, however, // to add the appropriate relocations in this case. - for &byte in self.data.iter() { - sink.put1(byte); - } + &self.data[..] } /// Get the list of external relocations for this code. @@ -2066,20 +2064,7 @@ mod test { let buf = buf.finish(); - #[derive(Default)] - struct TestCodeSink { - offset: CodeOffset, - } - impl CodeSink for TestCodeSink { - fn put1(&mut self, _: u8) { - self.offset += 1; - } - } - - let mut sink = TestCodeSink::default(); - buf.emit(&mut sink); - - assert_eq!(sink.offset, 4); + assert_eq!(buf.data(), &[1, 2, 3, 4]); assert_eq!( buf.traps() .iter() diff --git a/cranelift/filetests/src/test_compile.rs b/cranelift/filetests/src/test_compile.rs index cadc016dc1..265d6c02be 100644 --- a/cranelift/filetests/src/test_compile.rs +++ b/cranelift/filetests/src/test_compile.rs @@ -4,7 +4,7 @@ use crate::subtest::{run_filecheck, Context, SubTest}; use anyhow::{bail, Result}; -use cranelift_codegen::binemit::{self, CodeInfo}; +use cranelift_codegen::binemit::CodeInfo; use cranelift_codegen::ir; use cranelift_reader::{TestCommand, TestOption}; use log::info; @@ -75,17 +75,6 @@ impl SubTest for TestCompile { } } -/// Code sink that simply counts bytes. -struct SizeSink { - offset: binemit::CodeOffset, -} - -impl binemit::CodeSink for SizeSink { - fn put1(&mut self, _: u8) { - self.offset += 1; - } -} - fn check_precise_output(text: &str, context: &Context) -> Result<()> { let actual = text.lines().collect::>();