Make some things more consistent between define_function and define_function_bytes

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),

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()));

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<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() {
-            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 })
     }