From 856f799adef59bfa3cfacab70f316da4088149ad Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 9 Nov 2020 10:07:18 +0100 Subject: [PATCH] Make some things more consistent between define_function and define_function_bytes --- cranelift/module/src/module.rs | 3 --- cranelift/object/src/backend.rs | 16 ++++++++-------- cranelift/simplejit/src/backend.rs | 28 ++++++++++++++-------------- 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index b1ef1024a2..7112c1e86c 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -162,9 +162,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), diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index 6c18b2f22e..e27ebcde04 100644 --- a/cranelift/object/src/backend.rs +++ b/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 { 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())); diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index 8d2b5dd0c0..eb037154dc 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -1,12 +1,13 @@ //! Defines `SimpleJITModule`. 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::{Addend, CodeInfo, CodeOffset, Reloc, RelocSink, StackMap, StackMapSink, TrapSink}, + CodegenError, +}; use cranelift_entity::SecondaryMap; use cranelift_module::{ DataContext, DataDescription, DataId, FuncId, FuncOrDataId, Init, Linkage, Module, @@ -394,7 +395,6 @@ impl<'simple_jit_backend> Module for SimpleJITModule { return Err(ModuleError::DuplicateDefinition(decl.name.to_owned())); } - self.functions_to_finalize.push(id); let size = code_size as usize; let ptr = self .memory @@ -402,8 +402,6 @@ 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(); unsafe { @@ -416,11 +414,13 @@ impl<'simple_jit_backend> Module for SimpleJITModule { ) }; + self.record_function_for_perf(ptr, size, &decl.name); self.functions[id] = Some(CompiledBlob { ptr, size, relocs: reloc_sink.relocs, }); + self.functions_to_finalize.push(id); Ok(ModuleCompiledFunction { size: code_size }) } @@ -431,21 +431,21 @@ impl<'simple_jit_backend> Module for SimpleJITModule { bytes: &[u8], relocs: &[RelocRecord], ) -> ModuleResult { + 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() { - 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 @@ -453,17 +453,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.record_function_for_perf(ptr, size, &decl.name); self.functions[id] = Some(CompiledBlob { ptr, size, relocs: relocs.to_vec(), }); + self.functions_to_finalize.push(id); Ok(ModuleCompiledFunction { size: total_size }) }