Merge pull request #2390 from bjorn3/more_simplejit_refactors

More SimpleJIT refactorings

cranelift/codegen/src/binemit/memorysink.rs

@@ -74,9 +74,6 @@ impl<'a> MemoryCodeSink<'a> {
 
 /// A trait for receiving relocations for code that is emitted directly into memory.
 pub trait RelocSink {
-    /// Add a relocation referencing a block at the current offset.
-    fn reloc_block(&mut self, _: CodeOffset, _: Reloc, _: CodeOffset);
-
     /// Add a relocation referencing an external symbol at the current offset.
     fn reloc_external(
         &mut self,
@@ -138,11 +135,6 @@ impl<'a> CodeSink for MemoryCodeSink<'a> {
         self.write(x);
     }
 
-    fn reloc_block(&mut self, rel: Reloc, block_offset: CodeOffset) {
-        let ofs = self.offset();
-        self.relocs.reloc_block(ofs, rel, block_offset);
-    }
-
     fn reloc_external(
         &mut self,
         srcloc: SourceLoc,
@@ -204,7 +196,6 @@ impl<'a> CodeSink for MemoryCodeSink<'a> {
 pub struct NullRelocSink {}
 
 impl RelocSink for NullRelocSink {
-    fn reloc_block(&mut self, _: CodeOffset, _: Reloc, _: CodeOffset) {}
     fn reloc_external(
         &mut self,
         _: CodeOffset,

cranelift/codegen/src/binemit/mod.rs

@@ -140,9 +140,6 @@ pub trait CodeSink {
     /// Add 8 bytes to the code section.
     fn put8(&mut self, _: u64);
 
-    /// Add a relocation referencing a block at the current offset.
-    fn reloc_block(&mut self, _: Reloc, _: CodeOffset);
-
     /// Add a relocation referencing an external symbol plus the addend at the current offset.
     fn reloc_external(&mut self, _: SourceLoc, _: Reloc, _: &ExternalName, _: Addend);
 

cranelift/codegen/src/isa/test_utils.rs

@@ -59,8 +59,6 @@ impl CodeSink for TestCodeSink {
         }
     }
 
-    fn reloc_block(&mut self, _rel: Reloc, _block_offset: CodeOffset) {}
-
     fn reloc_external(
         &mut self,
         _srcloc: SourceLoc,

cranelift/filetests/src/test_binemit.rs

@@ -72,10 +72,6 @@ impl binemit::CodeSink for TextSink {
         self.offset += 8;
     }
 
-    fn reloc_block(&mut self, reloc: binemit::Reloc, block_offset: binemit::CodeOffset) {
-        write!(self.text, "{}({}) ", reloc, block_offset).unwrap();
-    }
-
     fn reloc_external(
         &mut self,
         _srcloc: ir::SourceLoc,

cranelift/filetests/src/test_compile.rs

@@ -109,7 +109,6 @@ impl binemit::CodeSink for SizeSink {
         self.offset += 8;
     }
 
-    fn reloc_block(&mut self, _reloc: binemit::Reloc, _block_offset: binemit::CodeOffset) {}
     fn reloc_external(
         &mut self,
         _srcloc: ir::SourceLoc,

cranelift/filetests/src/test_rodata.rs

@@ -106,7 +106,6 @@ impl binemit::CodeSink for RodataSink {
         }
     }
 
-    fn reloc_block(&mut self, _reloc: binemit::Reloc, _block_offset: binemit::CodeOffset) {}
     fn reloc_external(
         &mut self,
         _: ir::SourceLoc,

cranelift/filetests/src/test_stack_maps.rs

@@ -79,7 +79,6 @@ impl CodeSink for TestStackMapsSink {
         self.offset += 8;
     }
 
-    fn reloc_block(&mut self, _: Reloc, _: CodeOffset) {}
     fn reloc_external(&mut self, _: SourceLoc, _: Reloc, _: &ExternalName, _: Addend) {}
     fn reloc_constant(&mut self, _: Reloc, _: ConstantOffset) {}
     fn reloc_jt(&mut self, _: Reloc, _: JumpTable) {}

cranelift/module/src/data_context.rs

@@ -1,6 +1,6 @@
 //! Defines `DataContext`.
 
-use cranelift_codegen::binemit::{Addend, CodeOffset};
+use cranelift_codegen::binemit::{Addend, CodeOffset, Reloc};
 use cranelift_codegen::entity::PrimaryMap;
 use cranelift_codegen::ir;
 use std::borrow::ToOwned;
@@ -8,6 +8,8 @@ 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 {
@@ -55,6 +57,34 @@ pub struct DataDescription {
     pub align: Option<u64>,
 }
 
+impl DataDescription {
+    /// An iterator over all relocations of the data object.
+    pub fn all_relocs<'a>(
+        &'a self,
+        pointer_reloc: Reloc,
+    ) -> impl Iterator<Item = RelocRecord> + 'a {
+        let func_relocs = self
+            .function_relocs
+            .iter()
+            .map(move |&(offset, id)| RelocRecord {
+                reloc: pointer_reloc,
+                offset,
+                name: self.function_decls[id].clone(),
+                addend: 0,
+            });
+        let data_relocs = self
+            .data_relocs
+            .iter()
+            .map(move |&(offset, id, addend)| RelocRecord {
+                reloc: pointer_reloc,
+                offset,
+                name: self.data_decls[id].clone(),
+                addend,
+            });
+        func_relocs.chain(data_relocs)
+    }
+}
+
 /// This is to data objects what cranelift_codegen::Context is to functions.
 pub struct DataContext {
     description: DataDescription,

cranelift/module/src/module.rs

@@ -29,6 +29,18 @@ impl From<FuncId> for ir::ExternalName {
     }
 }
 
+impl FuncId {
+    /// Get the `FuncId` for the function named by `name`.
+    pub fn from_name(name: &ir::ExternalName) -> FuncId {
+        if let ir::ExternalName::User { namespace, index } = *name {
+            debug_assert_eq!(namespace, 0);
+            FuncId::from_u32(index)
+        } else {
+            panic!("unexpected ExternalName kind {}", name)
+        }
+    }
+}
+
 /// A data object identifier for use in the `Module` interface.
 #[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct DataId(u32);
@@ -44,6 +56,18 @@ impl From<DataId> for ir::ExternalName {
     }
 }
 
+impl DataId {
+    /// Get the `DataId` for the data object named by `name`.
+    pub fn from_name(name: &ir::ExternalName) -> DataId {
+        if let ir::ExternalName::User { namespace, index } = *name {
+            debug_assert_eq!(namespace, 1);
+            DataId::from_u32(index)
+        } else {
+            panic!("unexpected ExternalName kind {}", name)
+        }
+    }
+}
+
 /// Linkage refers to where an entity is defined and who can see it.
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub enum Linkage {
@@ -162,9 +186,6 @@ pub enum ModuleError {
     /// Indicates an identifier was defined, but was declared as an import
     #[error("Invalid to define identifier declared as an import: {0}")]
     InvalidImportDefinition(String),
-    /// Indicates a too-long function was defined
-    #[error("Function {0} exceeds the maximum function size")]
-    FunctionTooLarge(String),
     /// Wraps a `cranelift-codegen` error
     #[error("Compilation error: {0}")]
     Compilation(#[from] CodegenError),
@@ -217,21 +238,10 @@ impl ModuleDeclarations {
         self.functions.iter()
     }
 
-    /// Get the `FuncId` for the function named by `name`.
+    /// Return whether `name` names a function, rather than a data object.
-    pub fn get_function_id(&self, name: &ir::ExternalName) -> FuncId {
+    pub fn is_function(name: &ir::ExternalName) -> bool {
-        if let ir::ExternalName::User { namespace, index } = *name {
+        if let ir::ExternalName::User { namespace, .. } = *name {
-            debug_assert_eq!(namespace, 0);
+            namespace == 0
-            FuncId::from_u32(index)
-        } else {
-            panic!("unexpected ExternalName kind {}", name)
-        }
-    }
-
-    /// Get the `DataId` for the data object named by `name`.
-    pub fn get_data_id(&self, name: &ir::ExternalName) -> DataId {
-        if let ir::ExternalName::User { namespace, index } = *name {
-            debug_assert_eq!(namespace, 1);
-            DataId::from_u32(index)
         } else {
             panic!("unexpected ExternalName kind {}", name)
         }
@@ -252,15 +262,6 @@ impl ModuleDeclarations {
         &self.data_objects[data_id]
     }
 
-    /// Return whether `name` names a function, rather than a data object.
-    pub fn is_function(&self, name: &ir::ExternalName) -> bool {
-        if let ir::ExternalName::User { namespace, .. } = *name {
-            namespace == 0
-        } else {
-            panic!("unexpected ExternalName kind {}", name)
-        }
-    }
-
     /// Declare a function in this module.
     pub fn declare_function(
         &mut self,

cranelift/object/src/backend.rs

@@ -1,12 +1,13 @@
 //! Defines `ObjectModule`.
 
 use anyhow::anyhow;
-use cranelift_codegen::binemit::{
-    Addend, CodeInfo, CodeOffset, NullStackMapSink, Reloc, RelocSink, TrapSink,
-};
 use cranelift_codegen::entity::SecondaryMap;
 use cranelift_codegen::isa::TargetIsa;
 use cranelift_codegen::{self, ir};
+use cranelift_codegen::{
+    binemit::{Addend, CodeInfo, CodeOffset, NullStackMapSink, Reloc, RelocSink, TrapSink},
+    CodegenError,
+};
 use cranelift_module::{
     DataContext, DataDescription, DataId, FuncId, Init, Linkage, Module, ModuleCompiledFunction,
     ModuleDeclarations, ModuleError, ModuleResult, RelocRecord,
@@ -268,17 +269,16 @@ impl Module for ObjectModule {
         relocs: &[RelocRecord],
     ) -> ModuleResult<ModuleCompiledFunction> {
         info!("defining function {} with bytes", func_id);
+        let total_size: u32 = match bytes.len().try_into() {
+            Ok(total_size) => total_size,
+            _ => Err(CodegenError::CodeTooLarge)?,
+        };
 
         let decl = self.declarations.get_function_decl(func_id);
         if !decl.linkage.is_definable() {
             return Err(ModuleError::InvalidImportDefinition(decl.name.clone()));
         }
 
-        let total_size: u32 = match bytes.len().try_into() {
-            Ok(total_size) => total_size,
-            _ => Err(ModuleError::FunctionTooLarge(decl.name.clone()))?,
-        };
-
         let &mut (symbol, ref mut defined) = self.functions[func_id].as_mut().unwrap();
         if *defined {
             return Err(ModuleError::DuplicateDefinition(decl.name.clone()));
@@ -305,7 +305,10 @@ impl Module for ObjectModule {
         };
 
         if !relocs.is_empty() {
-            let relocs = self.process_relocs(relocs);
+            let relocs = relocs
+                .iter()
+                .map(|record| self.process_reloc(record))
+                .collect();
             self.relocs.push(SymbolRelocs {
                 section,
                 offset,
@@ -330,40 +333,24 @@ impl Module for ObjectModule {
 
         let &DataDescription {
             ref init,
-            ref function_decls,
+            function_decls: _,
-            ref data_decls,
+            data_decls: _,
-            ref function_relocs,
+            function_relocs: _,
-            ref data_relocs,
+            data_relocs: _,
             ref custom_segment_section,
             align,
         } = data_ctx.description();
 
-        let reloc_size = match self.isa.triple().pointer_width().unwrap() {
+        let pointer_reloc = match self.isa.triple().pointer_width().unwrap() {
-            PointerWidth::U16 => 16,
+            PointerWidth::U16 => unimplemented!("16bit pointers"),
-            PointerWidth::U32 => 32,
+            PointerWidth::U32 => Reloc::Abs4,
-            PointerWidth::U64 => 64,
+            PointerWidth::U64 => Reloc::Abs8,
         };
-        let mut relocs = Vec::new();
+        let relocs = data_ctx
-        for &(offset, id) in function_relocs {
+            .description()
-            relocs.push(ObjectRelocRecord {
+            .all_relocs(pointer_reloc)
-                offset,
+            .map(|record| self.process_reloc(&record))
-                name: function_decls[id].clone(),
+            .collect::<Vec<_>>();
-                kind: RelocationKind::Absolute,
-                encoding: RelocationEncoding::Generic,
-                size: reloc_size,
-                addend: 0,
-            });
-        }
-        for &(offset, id, addend) in data_relocs {
-            relocs.push(ObjectRelocRecord {
-                offset,
-                name: data_decls[id].clone(),
-                kind: RelocationKind::Absolute,
-                encoding: RelocationEncoding::Generic,
-                size: reloc_size,
-                addend,
-            });
-        }
 
         let section = if custom_segment_section.is_none() {
             let section_kind = if let Init::Zeros { .. } = *init {
@@ -477,11 +464,11 @@ impl ObjectModule {
     fn get_symbol(&mut self, name: &ir::ExternalName) -> SymbolId {
         match *name {
             ir::ExternalName::User { .. } => {
-                if self.declarations.is_function(name) {
+                if ModuleDeclarations::is_function(name) {
-                    let id = self.declarations.get_function_id(name);
+                    let id = FuncId::from_name(name);
                     self.functions[id].unwrap().0
                 } else {
-                    let id = self.declarations.get_data_id(name);
+                    let id = DataId::from_name(name);
                     self.data_objects[id].unwrap().0
                 }
             }
@@ -510,18 +497,13 @@ impl ObjectModule {
         }
     }
 
-    fn process_relocs(&self, relocs: &[RelocRecord]) -> Vec<ObjectRelocRecord> {
+    fn process_reloc(&self, record: &RelocRecord) -> ObjectRelocRecord {
-        relocs
-            .iter()
-            .map(|record| {
         let mut addend = record.addend;
         let (kind, encoding, size) = match record.reloc {
             Reloc::Abs4 => (RelocationKind::Absolute, RelocationEncoding::Generic, 32),
             Reloc::Abs8 => (RelocationKind::Absolute, RelocationEncoding::Generic, 64),
             Reloc::X86PCRel4 => (RelocationKind::Relative, RelocationEncoding::Generic, 32),
-                    Reloc::X86CallPCRel4 => {
+            Reloc::X86CallPCRel4 => (RelocationKind::Relative, RelocationEncoding::X86Branch, 32),
-                        (RelocationKind::Relative, RelocationEncoding::X86Branch, 32)
-                    }
             // TODO: Get Cranelift to tell us when we can use
             // R_X86_64_GOTPCRELX/R_X86_64_REX_GOTPCRELX.
             Reloc::X86CallPLTRel4 => (
@@ -529,9 +511,7 @@ impl ObjectModule {
                 RelocationEncoding::X86Branch,
                 32,
             ),
-                    Reloc::X86GOTPCRel4 => {
+            Reloc::X86GOTPCRel4 => (RelocationKind::GotRelative, RelocationEncoding::Generic, 32),
-                        (RelocationKind::GotRelative, RelocationEncoding::Generic, 32)
-                    }
             Reloc::ElfX86_64TlsGd => {
                 assert_eq!(
                     self.object.format(),
@@ -571,8 +551,6 @@ impl ObjectModule {
             size,
             addend,
         }
-            })
-            .collect()
     }
 }
 
@@ -644,10 +622,6 @@ struct ObjectRelocSink {
 }
 
 impl RelocSink for ObjectRelocSink {
-    fn reloc_block(&mut self, _offset: CodeOffset, _reloc: Reloc, _block_offset: CodeOffset) {
-        unimplemented!();
-    }
-
     fn reloc_external(
         &mut self,
         offset: CodeOffset,

cranelift/simplejit/examples/simplejit-minimal.rs

@@ -70,10 +70,10 @@ fn main() {
     module.clear_context(&mut ctx);
 
     // Perform linking.
-    let product = module.finish();
+    module.finalize_definitions();
 
     // Get a raw pointer to the generated code.
-    let code_b = product.lookup_func(func_b);
+    let code_b = module.get_finalized_function(func_b);
 
     // Cast it to a rust function pointer type.
     let ptr_b = unsafe { mem::transmute::<_, fn() -> u32>(code_b) };

cranelift/simplejit/src/backend.rs

@@ -1,16 +1,17 @@
 //! Defines `SimpleJITModule`.
 
-use crate::memory::Memory;
+use crate::{compiled_blob::CompiledBlob, memory::Memory};
-use cranelift_codegen::binemit::{
-    Addend, CodeInfo, CodeOffset, Reloc, RelocSink, StackMap, StackMapSink, TrapSink,
-};
 use cranelift_codegen::isa::TargetIsa;
 use cranelift_codegen::settings::Configurable;
 use cranelift_codegen::{self, ir, settings};
+use cranelift_codegen::{
+    binemit::{self, Addend, CodeInfo, CodeOffset, Reloc, RelocSink, TrapSink},
+    CodegenError,
+};
 use cranelift_entity::SecondaryMap;
 use cranelift_module::{
-    DataContext, DataDescription, DataId, FuncId, FuncOrDataId, Init, Linkage, Module,
+    DataContext, DataDescription, DataId, FuncId, Init, Linkage, Module, ModuleCompiledFunction,
-    ModuleCompiledFunction, ModuleDeclarations, ModuleError, ModuleResult, RelocRecord,
+    ModuleDeclarations, ModuleError, ModuleResult, RelocRecord,
 };
 use cranelift_native;
 #[cfg(not(windows))]
@@ -128,26 +129,12 @@ pub struct SimpleJITModule {
     libcall_names: Box<dyn Fn(ir::LibCall) -> String>,
     memory: MemoryHandle,
     declarations: ModuleDeclarations,
-    functions: SecondaryMap<FuncId, Option<CompiledBlob>>,
+    compiled_functions: SecondaryMap<FuncId, Option<CompiledBlob>>,
-    data_objects: SecondaryMap<DataId, Option<CompiledBlob>>,
+    compiled_data_objects: SecondaryMap<DataId, Option<CompiledBlob>>,
     functions_to_finalize: Vec<FuncId>,
     data_objects_to_finalize: Vec<DataId>,
 }
 
-struct StackMapRecord {
-    #[allow(dead_code)]
-    offset: CodeOffset,
-    #[allow(dead_code)]
-    stack_map: StackMap,
-}
-
-#[derive(Clone)]
-struct CompiledBlob {
-    ptr: *mut u8,
-    size: usize,
-    relocs: Vec<RelocRecord>,
-}
-
 /// A handle to allow freeing memory allocated by the `Module`.
 struct MemoryHandle {
     code: Memory,
@@ -155,16 +142,7 @@ struct MemoryHandle {
     writable: Memory,
 }
 
-/// A `SimpleJITProduct` allows looking up the addresses of all functions and data objects
+impl SimpleJITModule {
-/// defined in the original module.
-pub struct SimpleJITProduct {
-    memory: MemoryHandle,
-    declarations: ModuleDeclarations,
-    functions: SecondaryMap<FuncId, Option<CompiledBlob>>,
-    data_objects: SecondaryMap<DataId, Option<CompiledBlob>>,
-}
-
-impl SimpleJITProduct {
     /// Free memory allocated for code and data segments of compiled functions.
     ///
     /// # Safety
@@ -179,29 +157,6 @@ impl SimpleJITProduct {
         self.memory.writable.free_memory();
     }
 
-    /// Get the `FuncOrDataId` associated with the given name.
-    pub fn func_or_data_for_func(&self, name: &str) -> Option<FuncOrDataId> {
-        self.declarations.get_name(name)
-    }
-
-    /// Return the address of a function.
-    pub fn lookup_func(&self, func_id: FuncId) -> *const u8 {
-        self.functions[func_id]
-            .as_ref()
-            .unwrap_or_else(|| panic!("{} is not defined", func_id))
-            .ptr
-    }
-
-    /// Return the address and size of a data object.
-    pub fn lookup_data(&self, data_id: DataId) -> (*const u8, usize) {
-        let data = self.data_objects[data_id]
-            .as_ref()
-            .unwrap_or_else(|| panic!("{} is not defined", data_id));
-        (data.ptr, data.size)
-    }
-}
-
-impl SimpleJITModule {
     fn lookup_symbol(&self, name: &str) -> Option<*const u8> {
         self.symbols
             .get(name)
@@ -212,9 +167,9 @@ impl SimpleJITModule {
     fn get_definition(&self, name: &ir::ExternalName) -> *const u8 {
         match *name {
             ir::ExternalName::User { .. } => {
-                let (name, linkage) = if self.declarations.is_function(name) {
+                let (name, linkage) = if ModuleDeclarations::is_function(name) {
-                    let func_id = self.declarations.get_function_id(name);
+                    let func_id = FuncId::from_name(name);
-                    match &self.functions[func_id] {
+                    match &self.compiled_functions[func_id] {
                         Some(compiled) => return compiled.ptr,
                         None => {
                             let decl = self.declarations.get_function_decl(func_id);
@@ -222,8 +177,8 @@ impl SimpleJITModule {
                         }
                     }
                 } else {
-                    let data_id = self.declarations.get_data_id(name);
+                    let data_id = DataId::from_name(name);
-                    match &self.data_objects[data_id] {
+                    match &self.compiled_data_objects[data_id] {
                         Some(compiled) => return compiled.ptr,
                         None => {
                             let decl = self.declarations.get_data_decl(data_id);
@@ -250,7 +205,7 @@ impl SimpleJITModule {
 
     /// Returns the address of a finalized function.
     pub fn get_finalized_function(&self, func_id: FuncId) -> *const u8 {
-        let info = &self.functions[func_id];
+        let info = &self.compiled_functions[func_id];
         debug_assert!(
             !self.functions_to_finalize.iter().any(|x| *x == func_id),
             "function not yet finalized"
@@ -262,7 +217,7 @@ impl SimpleJITModule {
 
     /// Returns the address and size of a finalized data object.
     pub fn get_finalized_data(&self, data_id: DataId) -> (*const u8, usize) {
-        let info = &self.data_objects[data_id];
+        let info = &self.compiled_data_objects[data_id];
         debug_assert!(
             !self.data_objects_to_finalize.iter().any(|x| *x == data_id),
             "data object not yet finalized"
@@ -291,95 +246,6 @@ impl SimpleJITModule {
         }
     }
 
-    fn finalize_function(&mut self, id: FuncId) {
-        use std::ptr::write_unaligned;
-
-        let func = self.functions[id]
-            .as_ref()
-            .expect("function must be compiled before it can be finalized");
-
-        for &RelocRecord {
-            reloc,
-            offset,
-            ref name,
-            addend,
-        } in &func.relocs
-        {
-            debug_assert!((offset as usize) < func.size);
-            let at = unsafe { func.ptr.offset(offset as isize) };
-            let base = self.get_definition(name);
-            // TODO: Handle overflow.
-            let what = unsafe { base.offset(addend as isize) };
-            match reloc {
-                Reloc::Abs4 => {
-                    // TODO: Handle overflow.
-                    #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))]
-                    unsafe {
-                        write_unaligned(at as *mut u32, what as u32)
-                    };
-                }
-                Reloc::Abs8 => {
-                    #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))]
-                    unsafe {
-                        write_unaligned(at as *mut u64, what as u64)
-                    };
-                }
-                Reloc::X86PCRel4 | Reloc::X86CallPCRel4 => {
-                    // TODO: Handle overflow.
-                    let pcrel = ((what as isize) - (at as isize)) as i32;
-                    #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))]
-                    unsafe {
-                        write_unaligned(at as *mut i32, pcrel)
-                    };
-                }
-                Reloc::X86GOTPCRel4 | Reloc::X86CallPLTRel4 => panic!("unexpected PIC relocation"),
-                _ => unimplemented!(),
-            }
-        }
-    }
-
-    fn finalize_data(&mut self, id: DataId) {
-        use std::ptr::write_unaligned;
-
-        let data = self.data_objects[id]
-            .as_ref()
-            .expect("data object must be compiled before it can be finalized");
-
-        for &RelocRecord {
-            reloc,
-            offset,
-            ref name,
-            addend,
-        } in &data.relocs
-        {
-            debug_assert!((offset as usize) < data.size);
-            let at = unsafe { data.ptr.offset(offset as isize) };
-            let base = self.get_definition(name);
-            // TODO: Handle overflow.
-            let what = unsafe { base.offset(addend as isize) };
-            match reloc {
-                Reloc::Abs4 => {
-                    // TODO: Handle overflow.
-                    #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))]
-                    unsafe {
-                        write_unaligned(at as *mut u32, what as u32)
-                    };
-                }
-                Reloc::Abs8 => {
-                    #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))]
-                    unsafe {
-                        write_unaligned(at as *mut u64, what as u64)
-                    };
-                }
-                Reloc::X86PCRel4
-                | Reloc::X86CallPCRel4
-                | Reloc::X86GOTPCRel4
-                | Reloc::X86CallPLTRel4 => panic!("unexpected text relocation in data"),
-                _ => unimplemented!(),
-            }
-        }
-    }
-
     /// Finalize all functions and data objects that are defined but not yet finalized.
     /// All symbols referenced in their bodies that are declared as needing a definition
     /// must be defined by this point.
@@ -390,12 +256,18 @@ impl SimpleJITModule {
         for func in std::mem::take(&mut self.functions_to_finalize) {
             let decl = self.declarations.get_function_decl(func);
             debug_assert!(decl.linkage.is_definable());
-            self.finalize_function(func);
+            let func = self.compiled_functions[func]
+                .as_ref()
+                .expect("function must be compiled before it can be finalized");
+            func.perform_relocations(|name| self.get_definition(name));
         }
         for data in std::mem::take(&mut self.data_objects_to_finalize) {
             let decl = self.declarations.get_data_decl(data);
             debug_assert!(decl.linkage.is_definable());
-            self.finalize_data(data);
+            let data = self.compiled_data_objects[data]
+                .as_ref()
+                .expect("data object must be compiled before it can be finalized");
+            data.perform_relocations(|name| self.get_definition(name));
         }
 
         // Now that we're done patching, prepare the memory for execution!
@@ -417,8 +289,8 @@ impl SimpleJITModule {
             libcall_names: builder.libcall_names,
             memory,
             declarations: ModuleDeclarations::default(),
-            functions: SecondaryMap::new(),
+            compiled_functions: SecondaryMap::new(),
-            data_objects: SecondaryMap::new(),
+            compiled_data_objects: SecondaryMap::new(),
             functions_to_finalize: Vec::new(),
             data_objects_to_finalize: Vec::new(),
         }
@@ -480,11 +352,10 @@ impl<'simple_jit_backend> Module for SimpleJITModule {
             return Err(ModuleError::InvalidImportDefinition(decl.name.clone()));
         }
 
-        if !self.functions[id].is_none() {
+        if !self.compiled_functions[id].is_none() {
             return Err(ModuleError::DuplicateDefinition(decl.name.to_owned()));
         }
 
-        self.functions_to_finalize.push(id);
         let size = code_size as usize;
         let ptr = self
             .memory
@@ -492,10 +363,8 @@ impl<'simple_jit_backend> Module for SimpleJITModule {
             .allocate(size, EXECUTABLE_DATA_ALIGNMENT)
             .expect("TODO: handle OOM etc.");
 
-        self.record_function_for_perf(ptr, size, &decl.name);
-
         let mut reloc_sink = SimpleJITRelocSink::default();
-        let mut stack_map_sink = SimpleJITStackMapSink::default();
+        let mut stack_map_sink = binemit::NullStackMapSink {};
         unsafe {
             ctx.emit_to_memory(
                 &*self.isa,
@@ -506,11 +375,13 @@ impl<'simple_jit_backend> Module for SimpleJITModule {
             )
         };
 
-        self.functions[id] = Some(CompiledBlob {
+        self.record_function_for_perf(ptr, size, &decl.name);
+        self.compiled_functions[id] = Some(CompiledBlob {
             ptr,
             size,
             relocs: reloc_sink.relocs,
         });
+        self.functions_to_finalize.push(id);
 
         Ok(ModuleCompiledFunction { size: code_size })
     }
@@ -521,21 +392,21 @@ impl<'simple_jit_backend> Module for SimpleJITModule {
         bytes: &[u8],
         relocs: &[RelocRecord],
     ) -> ModuleResult<ModuleCompiledFunction> {
+        info!("defining function {} with bytes", id);
+        let total_size: u32 = match bytes.len().try_into() {
+            Ok(total_size) => total_size,
+            _ => Err(CodegenError::CodeTooLarge)?,
+        };
+
         let decl = self.declarations.get_function_decl(id);
         if !decl.linkage.is_definable() {
             return Err(ModuleError::InvalidImportDefinition(decl.name.clone()));
         }
 
-        let total_size: u32 = match bytes.len().try_into() {
+        if !self.compiled_functions[id].is_none() {
-            Ok(total_size) => total_size,
-            _ => Err(ModuleError::FunctionTooLarge(decl.name.clone()))?,
-        };
-
-        if !self.functions[id].is_none() {
             return Err(ModuleError::DuplicateDefinition(decl.name.to_owned()));
         }
 
-        self.functions_to_finalize.push(id);
         let size = bytes.len();
         let ptr = self
             .memory
@@ -543,17 +414,17 @@ impl<'simple_jit_backend> Module for SimpleJITModule {
             .allocate(size, EXECUTABLE_DATA_ALIGNMENT)
             .expect("TODO: handle OOM etc.");
 
-        self.record_function_for_perf(ptr, size, &decl.name);
-
         unsafe {
             ptr::copy_nonoverlapping(bytes.as_ptr(), ptr, size);
         }
 
-        self.functions[id] = Some(CompiledBlob {
+        self.record_function_for_perf(ptr, size, &decl.name);
+        self.compiled_functions[id] = Some(CompiledBlob {
             ptr,
             size,
             relocs: relocs.to_vec(),
         });
+        self.functions_to_finalize.push(id);
 
         Ok(ModuleCompiledFunction { size: total_size })
     }
@@ -564,20 +435,18 @@ impl<'simple_jit_backend> Module for SimpleJITModule {
             return Err(ModuleError::InvalidImportDefinition(decl.name.clone()));
         }
 
-        if !self.data_objects[id].is_none() {
+        if !self.compiled_data_objects[id].is_none() {
             return Err(ModuleError::DuplicateDefinition(decl.name.to_owned()));
         }
 
         assert!(!decl.tls, "SimpleJIT doesn't yet support TLS");
 
-        self.data_objects_to_finalize.push(id);
-
         let &DataDescription {
             ref init,
-            ref function_decls,
+            function_decls: _,
-            ref data_decls,
+            data_decls: _,
-            ref function_relocs,
+            function_relocs: _,
-            ref data_relocs,
+            data_relocs: _,
             custom_segment_section: _,
             align,
         } = data.description();
@@ -608,55 +477,23 @@ impl<'simple_jit_backend> Module for SimpleJITModule {
             }
         }
 
-        let reloc = match self.isa.triple().pointer_width().unwrap() {
+        let pointer_reloc = match self.isa.triple().pointer_width().unwrap() {
             PointerWidth::U16 => panic!(),
             PointerWidth::U32 => Reloc::Abs4,
             PointerWidth::U64 => Reloc::Abs8,
         };
-        let mut relocs = Vec::new();
+        let relocs = data
-        for &(offset, id) in function_relocs {
+            .description()
-            relocs.push(RelocRecord {
+            .all_relocs(pointer_reloc)
-                reloc,
+            .collect::<Vec<_>>();
-                offset,
-                name: function_decls[id].clone(),
-                addend: 0,
-            });
-        }
-        for &(offset, id, addend) in data_relocs {
-            relocs.push(RelocRecord {
-                reloc,
-                offset,
-                name: data_decls[id].clone(),
-                addend,
-            });
-        }
 
-        self.data_objects[id] = Some(CompiledBlob { ptr, size, relocs });
+        self.compiled_data_objects[id] = Some(CompiledBlob { ptr, size, relocs });
+        self.data_objects_to_finalize.push(id);
 
         Ok(())
     }
 }
 
-impl SimpleJITModule {
-    /// SimpleJIT emits code and data into memory as it processes them. This
-    /// method performs no additional processing, but returns a handle which
-    /// allows freeing the allocated memory. Otherwise said memory is leaked
-    /// to enable safe handling of the resulting pointers.
-    ///
-    /// This method does not need to be called when access to the memory
-    /// handle is not required.
-    pub fn finish(mut self) -> SimpleJITProduct {
-        self.finalize_definitions();
-
-        SimpleJITProduct {
-            memory: self.memory,
-            declarations: self.declarations,
-            functions: self.functions,
-            data_objects: self.data_objects,
-        }
-    }
-}
-
 #[cfg(not(windows))]
 fn lookup_with_dlsym(name: &str) -> Option<*const u8> {
     let c_str = CString::new(name).unwrap();
@@ -702,10 +539,6 @@ struct SimpleJITRelocSink {
 }
 
 impl RelocSink for SimpleJITRelocSink {
-    fn reloc_block(&mut self, _offset: CodeOffset, _reloc: Reloc, _block_offset: CodeOffset) {
-        unimplemented!();
-    }
-
     fn reloc_external(
         &mut self,
         offset: CodeOffset,
@@ -746,14 +579,3 @@ impl RelocSink for SimpleJITRelocSink {
         }
     }
 }
-
-#[derive(Default)]
-struct SimpleJITStackMapSink {
-    stack_maps: Vec<StackMapRecord>,
-}
-
-impl StackMapSink for SimpleJITStackMapSink {
-    fn add_stack_map(&mut self, offset: CodeOffset, stack_map: StackMap) {
-        self.stack_maps.push(StackMapRecord { offset, stack_map });
-    }
-}

cranelift/simplejit/src/compiled_blob.rs

@@ -0,0 +1,55 @@
+use cranelift_codegen::binemit::Reloc;
+use cranelift_codegen::ir::ExternalName;
+use cranelift_module::RelocRecord;
+
+#[derive(Clone)]
+pub(crate) struct CompiledBlob {
+    pub(crate) ptr: *mut u8,
+    pub(crate) size: usize,
+    pub(crate) relocs: Vec<RelocRecord>,
+}
+
+impl CompiledBlob {
+    pub(crate) fn perform_relocations(&self, get_definition: impl Fn(&ExternalName) -> *const u8) {
+        use std::ptr::write_unaligned;
+
+        for &RelocRecord {
+            reloc,
+            offset,
+            ref name,
+            addend,
+        } in &self.relocs
+        {
+            debug_assert!((offset as usize) < self.size);
+            let at = unsafe { self.ptr.offset(offset as isize) };
+            let base = get_definition(name);
+            // TODO: Handle overflow.
+            let what = unsafe { base.offset(addend as isize) };
+            match reloc {
+                Reloc::Abs4 => {
+                    // TODO: Handle overflow.
+                    #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))]
+                    unsafe {
+                        write_unaligned(at as *mut u32, what as u32)
+                    };
+                }
+                Reloc::Abs8 => {
+                    #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))]
+                    unsafe {
+                        write_unaligned(at as *mut u64, what as u64)
+                    };
+                }
+                Reloc::X86PCRel4 | Reloc::X86CallPCRel4 => {
+                    // TODO: Handle overflow.
+                    let pcrel = ((what as isize) - (at as isize)) as i32;
+                    #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))]
+                    unsafe {
+                        write_unaligned(at as *mut i32, pcrel)
+                    };
+                }
+                Reloc::X86GOTPCRel4 | Reloc::X86CallPLTRel4 => panic!("unexpected PIC relocation"),
+                _ => unimplemented!(),
+            }
+        }
+    }
+}

cranelift/simplejit/src/lib.rs

@@ -4,7 +4,8 @@
     missing_docs,
     trivial_numeric_casts,
     unused_extern_crates,
-    unstable_features
+    unstable_features,
+    unreachable_pub
 )]
 #![warn(unused_import_braces)]
 #![cfg_attr(feature = "clippy", plugin(clippy(conf_file = "../../clippy.toml")))]
@@ -23,9 +24,10 @@
 )]
 
 mod backend;
+mod compiled_blob;
 mod memory;
 
-pub use crate::backend::{SimpleJITBuilder, SimpleJITModule, SimpleJITProduct};
+pub use crate::backend::{SimpleJITBuilder, SimpleJITModule};
 
 /// Version number of this crate.
 pub const VERSION: &str = env!("CARGO_PKG_VERSION");

cranelift/simplejit/src/memory.rs

@@ -126,7 +126,7 @@ impl Drop for PtrLen {
 /// accessible memory. Memory will be leaked by default to have
 /// function pointers remain valid for the remainder of the
 /// program's life.
-pub struct Memory {
+pub(crate) struct Memory {
     allocations: Vec<PtrLen>,
     executable: usize,
     current: PtrLen,
@@ -134,7 +134,7 @@ pub struct Memory {
 }
 
 impl Memory {
-    pub fn new() -> Self {
+    pub(crate) fn new() -> Self {
         Self {
             allocations: Vec::new(),
             executable: 0,
@@ -150,7 +150,7 @@ impl Memory {
     }
 
     /// TODO: Use a proper error type.
-    pub fn allocate(&mut self, size: usize, align: u64) -> Result<*mut u8, String> {
+    pub(crate) fn allocate(&mut self, size: usize, align: u64) -> Result<*mut u8, String> {
         let align = usize::try_from(align).expect("alignment too big");
         if self.position % align != 0 {
             self.position += align - self.position % align;
@@ -173,7 +173,7 @@ impl Memory {
     }
 
     /// Set all memory allocated in this `Memory` up to now as readable and executable.
-    pub fn set_readable_and_executable(&mut self) {
+    pub(crate) fn set_readable_and_executable(&mut self) {
         self.finish_current();
 
         #[cfg(feature = "selinux-fix")]
@@ -202,7 +202,7 @@ impl Memory {
     }
 
     /// Set all memory allocated in this `Memory` up to now as readonly.
-    pub fn set_readonly(&mut self) {
+    pub(crate) fn set_readonly(&mut self) {
         self.finish_current();
 
         #[cfg(feature = "selinux-fix")]
@@ -232,7 +232,7 @@ impl Memory {
 
     /// Frees all allocated memory regions that would be leaked otherwise.
     /// Likely to invalidate existing function pointers, causing unsafety.
-    pub unsafe fn free_memory(&mut self) {
+    pub(crate) unsafe fn free_memory(&mut self) {
         self.allocations.clear();
     }
 }

cranelift/simplejit/tests/basic.rs

@@ -186,5 +186,5 @@ fn libcall_function() {
         .define_function(func_id, &mut ctx, &mut trap_sink)
         .unwrap();
 
-    module.finish();
+    module.finalize_definitions();
 }

cranelift/src/disasm.rs

@@ -19,22 +19,6 @@ impl PrintRelocs {
 }
 
 impl binemit::RelocSink for PrintRelocs {
-    fn reloc_block(
-        &mut self,
-        where_: binemit::CodeOffset,
-        r: binemit::Reloc,
-        offset: binemit::CodeOffset,
-    ) {
-        if self.flag_print {
-            writeln!(
-                &mut self.text,
-                "reloc_block: {} {} at {}",
-                r, offset, where_
-            )
-            .unwrap();
-        }
-    }
-
     fn reloc_external(
         &mut self,
         where_: binemit::CodeOffset,

crates/cranelift/src/lib.rs

@@ -114,15 +114,6 @@ struct RelocSink {
 }
 
 impl binemit::RelocSink for RelocSink {
-    fn reloc_block(
-        &mut self,
-        _offset: binemit::CodeOffset,
-        _reloc: binemit::Reloc,
-        _block_offset: binemit::CodeOffset,
-    ) {
-        // This should use the `offsets` field of `ir::Function`.
-        panic!("block headers not yet implemented");
-    }
     fn reloc_external(
         &mut self,
         offset: binemit::CodeOffset,

crates/jit/src/trampoline.rs

@@ -184,14 +184,6 @@ impl TrampolineRelocSink {
 }
 
 impl binemit::RelocSink for TrampolineRelocSink {
-    fn reloc_block(
-        &mut self,
-        _offset: binemit::CodeOffset,
-        _reloc: binemit::Reloc,
-        _block_offset: binemit::CodeOffset,
-    ) {
-        panic!("trampoline compilation should not produce block relocs");
-    }
     fn reloc_external(
         &mut self,
         offset: binemit::CodeOffset,

crates/lightbeam/src/translate_sections.rs

@@ -83,10 +83,6 @@ pub fn element(elements: ElementSectionReader) -> Result<(), Error> {
 struct UnimplementedRelocSink;
 
 impl binemit::RelocSink for UnimplementedRelocSink {
-    fn reloc_block(&mut self, _: binemit::CodeOffset, _: binemit::Reloc, _: binemit::CodeOffset) {
-        unimplemented!()
-    }
-
     fn reloc_external(
         &mut self,
         _: binemit::CodeOffset,

crates/lightbeam/wasmtime/src/lib.rs

@@ -86,15 +86,6 @@ struct RelocSink {
 }
 
 impl binemit::RelocSink for RelocSink {
-    fn reloc_block(
-        &mut self,
-        _offset: binemit::CodeOffset,
-        _reloc: binemit::Reloc,
-        _block_offset: binemit::CodeOffset,
-    ) {
-        // This should use the `offsets` field of `ir::Function`.
-        panic!("block headers not yet implemented");
-    }
     fn reloc_external(
         &mut self,
         offset: binemit::CodeOffset,