Remove Function argument from define_function_bytes (#10346)

* Remove Function argument from define_function_bytes

define_function_bytes is supposed to be used when the code was not
compiled by Cranelift and thus wouldn't have any associated Function.

* Avoid collecting the relocations into a Vec twice

Author: bjorn3

cranelift/jit/src/backend.rs

@@ -4,7 +4,7 @@ use crate::{compiled_blob::CompiledBlob, memory::BranchProtection, memory::Memor
 use cranelift_codegen::binemit::Reloc;
 use cranelift_codegen::isa::{OwnedTargetIsa, TargetIsa};
 use cranelift_codegen::settings::Configurable;
-use cranelift_codegen::{ir, settings, FinalizedMachReloc};
+use cranelift_codegen::{ir, settings};
 use cranelift_control::ControlPlane;
 use cranelift_entity::SecondaryMap;
 use cranelift_module::{
@@ -772,10 +772,9 @@ impl Module for JITModule {
     fn define_function_bytes(
         &mut self,
         id: FuncId,
-        func: &ir::Function,
         alignment: u64,
         bytes: &[u8],
-        relocs: &[FinalizedMachReloc],
+        relocs: &[ModuleReloc],
     ) -> ModuleResult<()> {
         info!("defining function {} with bytes", id);
         let decl = self.declarations.get_function_decl(id);
@@ -812,10 +811,7 @@ impl Module for JITModule {
         self.compiled_functions[id] = Some(CompiledBlob {
             ptr,
             size,
-            relocs: relocs
-                .iter()
-                .map(|reloc| ModuleReloc::from_mach_reloc(reloc, func, id))
-                .collect(),
+            relocs: relocs.to_owned(),
         });
 
         if self.isa.flags().is_pic() {

cranelift/module/src/module.rs

@@ -974,10 +974,9 @@ pub trait Module {
     fn define_function_bytes(
         &mut self,
         func_id: FuncId,
-        func: &ir::Function,
         alignment: u64,
         bytes: &[u8],
-        relocs: &[FinalizedMachReloc],
+        relocs: &[ModuleReloc],
     ) -> ModuleResult<()>;
 
     /// Define a data object, producing the data contents from the given `DataContext`.
@@ -1076,12 +1075,11 @@ impl<M: Module + ?Sized> Module for &mut M {
     fn define_function_bytes(
         &mut self,
         func_id: FuncId,
-        func: &ir::Function,
         alignment: u64,
         bytes: &[u8],
-        relocs: &[FinalizedMachReloc],
+        relocs: &[ModuleReloc],
     ) -> ModuleResult<()> {
-        (**self).define_function_bytes(func_id, func, alignment, bytes, relocs)
+        (**self).define_function_bytes(func_id, alignment, bytes, relocs)
     }
 
     fn define_data(&mut self, data_id: DataId, data: &DataDescription) -> ModuleResult<()> {
@@ -1181,12 +1179,11 @@ impl<M: Module + ?Sized> Module for Box<M> {
     fn define_function_bytes(
         &mut self,
         func_id: FuncId,
-        func: &ir::Function,
         alignment: u64,
         bytes: &[u8],
-        relocs: &[FinalizedMachReloc],
+        relocs: &[ModuleReloc],
     ) -> ModuleResult<()> {
-        (**self).define_function_bytes(func_id, func, alignment, bytes, relocs)
+        (**self).define_function_bytes(func_id, alignment, bytes, relocs)
     }
 
     fn define_data(&mut self, data_id: DataId, data: &DataDescription) -> ModuleResult<()> {

cranelift/object/src/backend.rs

@@ -3,8 +3,8 @@
 use anyhow::anyhow;
 use cranelift_codegen::binemit::{Addend, CodeOffset, Reloc};
 use cranelift_codegen::entity::SecondaryMap;
+use cranelift_codegen::ir;
 use cranelift_codegen::isa::{OwnedTargetIsa, TargetIsa};
-use cranelift_codegen::{ir, FinalizedMachReloc};
 use cranelift_control::ControlPlane;
 use cranelift_module::{
     DataDescription, DataId, FuncId, Init, Linkage, Module, ModuleDeclarations, ModuleError,
@@ -345,65 +345,29 @@ impl Module for ObjectModule {
         let res = ctx.compile(self.isa(), ctrl_plane)?;
         let alignment = res.buffer.alignment as u64;
 
-        self.define_function_bytes(
-            func_id,
-            &ctx.func,
-            alignment,
-            ctx.compiled_code().unwrap().code_buffer(),
-            ctx.compiled_code().unwrap().buffer.relocs(),
-        )
+        let buffer = &ctx.compiled_code().unwrap().buffer;
+        let relocs = buffer
+            .relocs()
+            .iter()
+            .map(|reloc| {
+                self.process_reloc(&ModuleReloc::from_mach_reloc(&reloc, &ctx.func, func_id))
+            })
+            .collect::<Vec<_>>();
+        self.define_function_inner(func_id, alignment, buffer.data(), relocs)
     }
 
     fn define_function_bytes(
         &mut self,
         func_id: FuncId,
-        func: &ir::Function,
         alignment: u64,
         bytes: &[u8],
-        relocs: &[FinalizedMachReloc],
+        relocs: &[ModuleReloc],
     ) -> ModuleResult<()> {
-        info!("defining function {} with bytes", func_id);
-        let decl = self.declarations.get_function_decl(func_id);
-        let decl_name = decl.linkage_name(func_id);
-        if !decl.linkage.is_definable() {
-            return Err(ModuleError::InvalidImportDefinition(decl_name.into_owned()));
-        }
-
-        let &mut (symbol, ref mut defined) = self.functions[func_id].as_mut().unwrap();
-        if *defined {
-            return Err(ModuleError::DuplicateDefinition(decl_name.into_owned()));
-        }
-        *defined = true;
-
-        let align = alignment
-            .max(self.isa.function_alignment().minimum.into())
-            .max(self.isa.symbol_alignment());
-        let section = if self.per_function_section {
-            // FIXME pass empty symbol name once add_subsection produces `.text` as section name
-            // instead of `.text.` when passed an empty symbol name. (object#748) Until then pass
-            // `subsection` to produce `.text.subsection` as section name to reduce confusion.
-            self.object
-                .add_subsection(StandardSection::Text, b"subsection")
-        } else {
-            self.object.section_id(StandardSection::Text)
-        };
-        let offset = self.object.add_symbol_data(symbol, section, bytes, align);
-
-        if !relocs.is_empty() {
-            let relocs = relocs
-                .iter()
-                .map(|record| {
-                    self.process_reloc(&ModuleReloc::from_mach_reloc(&record, func, func_id))
-                })
-                .collect();
-            self.relocs.push(SymbolRelocs {
-                section,
-                offset,
-                relocs,
-            });
-        }
-
-        Ok(())
+        let relocs = relocs
+            .iter()
+            .map(|reloc| self.process_reloc(reloc))
+            .collect();
+        self.define_function_inner(func_id, alignment, bytes, relocs)
     }
 
     fn define_data(&mut self, data_id: DataId, data: &DataDescription) -> ModuleResult<()> {
@@ -511,6 +475,51 @@ impl Module for ObjectModule {
 }
 
 impl ObjectModule {
+    fn define_function_inner(
+        &mut self,
+        func_id: FuncId,
+        alignment: u64,
+        bytes: &[u8],
+        relocs: Vec<ObjectRelocRecord>,
+    ) -> Result<(), ModuleError> {
+        info!("defining function {} with bytes", func_id);
+        let decl = self.declarations.get_function_decl(func_id);
+        let decl_name = decl.linkage_name(func_id);
+        if !decl.linkage.is_definable() {
+            return Err(ModuleError::InvalidImportDefinition(decl_name.into_owned()));
+        }
+
+        let &mut (symbol, ref mut defined) = self.functions[func_id].as_mut().unwrap();
+        if *defined {
+            return Err(ModuleError::DuplicateDefinition(decl_name.into_owned()));
+        }
+        *defined = true;
+
+        let align = alignment
+            .max(self.isa.function_alignment().minimum.into())
+            .max(self.isa.symbol_alignment());
+        let section = if self.per_function_section {
+            // FIXME pass empty symbol name once add_subsection produces `.text` as section name
+            // instead of `.text.` when passed an empty symbol name. (object#748) Until then pass
+            // `subsection` to produce `.text.subsection` as section name to reduce confusion.
+            self.object
+                .add_subsection(StandardSection::Text, b"subsection")
+        } else {
+            self.object.section_id(StandardSection::Text)
+        };
+        let offset = self.object.add_symbol_data(symbol, section, bytes, align);
+
+        if !relocs.is_empty() {
+            self.relocs.push(SymbolRelocs {
+                section,
+                offset,
+                relocs,
+            });
+        }
+
+        Ok(())
+    }
+
     /// Finalize all relocations and output an object.
     pub fn finish(mut self) -> ObjectProduct {
         let symbol_relocs = mem::take(&mut self.relocs);