Allow serializing all cranelift-module data structures (#6172)

* Remove ModuleCompiledFunction

The same information can be retrieved using

ctx.compiled_code().unwrap().code_info().total_size

In addition for Module implementations that don't immediately compile the
given function there is no correct value that can be returned.

* Don't give anonymous functions and data objects an internal name

This internal name can conflict if a module is serialized and then
deserialized into another module. It also wasn't used by any of the
Module implementations anyway.

* Allow serializing all cranelift-module data structures

This allows a Module implementation to serialize it's internal state and
deserialize it in another compilation session. For example to implement
LTO or to load the module into cranelift-interpreter.

* Use expect

Cargo.lock

@@ -747,6 +747,7 @@ dependencies = [
  "cranelift-codegen",
  "cranelift-control",
  "hashbrown 0.13.2",
+ "serde",
 ]
 
 [[package]]

cranelift/codegen/src/ir/function.rs

@@ -31,7 +31,7 @@ use super::{RelSourceLoc, SourceLoc, UserExternalName};
 
 /// A version marker used to ensure that serialized clif ir is never deserialized with a
 /// different version of Cranelift.
-#[derive(Copy, Clone, Debug, PartialEq, Hash)]
+#[derive(Default, Copy, Clone, Debug, PartialEq, Hash)]
 pub struct VersionMarker;
 
 #[cfg(feature = "enable-serde")]

cranelift/jit/src/backend.rs

@@ -1,15 +1,15 @@
 //! Defines `JITModule`.
 
 use crate::{compiled_blob::CompiledBlob, memory::BranchProtection, memory::Memory};
+use cranelift_codegen::binemit::Reloc;
 use cranelift_codegen::isa::{OwnedTargetIsa, TargetIsa};
 use cranelift_codegen::settings::Configurable;
 use cranelift_codegen::{self, ir, settings, MachReloc};
-use cranelift_codegen::{binemit::Reloc, CodegenError};
 use cranelift_control::ControlPlane;
 use cranelift_entity::SecondaryMap;
 use cranelift_module::{
-    DataDescription, DataId, FuncId, Init, Linkage, Module, ModuleCompiledFunction,
+    DataDescription, DataId, FuncId, Init, Linkage, Module, ModuleDeclarations, ModuleError,
-    ModuleDeclarations, ModuleError, ModuleExtName, ModuleReloc, ModuleResult,
+    ModuleExtName, ModuleReloc, ModuleResult,
 };
 use log::info;
 use std::cell::RefCell;
@@ -272,7 +272,10 @@ impl JITModule {
         self.record_function_for_perf(
             plt_entry.as_ptr().cast(),
             std::mem::size_of::<[u8; 16]>(),
-            &format!("{}@plt", self.declarations.get_function_decl(id).name),
+            &format!(
+                "{}@plt",
+                self.declarations.get_function_decl(id).linkage_name(id)
+            ),
         );
         self.function_plt_entries[id] = Some(plt_entry);
     }
@@ -323,6 +326,9 @@ impl JITModule {
                         }
                     }
                 };
+                let name = name
+                    .as_ref()
+                    .expect("anonymous symbol must be defined locally");
                 if let Some(ptr) = self.lookup_symbol(name) {
                     ptr
                 } else if linkage == Linkage::Preemptible {
@@ -554,13 +560,15 @@ impl JITModule {
         assert!(self.hotswap_enabled, "Hotswap support is not enabled");
         let decl = self.declarations.get_function_decl(func_id);
         if !decl.linkage.is_definable() {
-            return Err(ModuleError::InvalidImportDefinition(decl.name.clone()));
+            return Err(ModuleError::InvalidImportDefinition(
+                decl.linkage_name(func_id).into_owned(),
+            ));
         }
 
         if self.compiled_functions[func_id].is_none() {
             return Err(ModuleError::Backend(anyhow::anyhow!(
                 "Tried to redefine not yet defined function {}",
-                decl.name
+                decl.linkage_name(func_id),
             )));
         }
 
@@ -647,15 +655,19 @@ impl Module for JITModule {
         id: FuncId,
         ctx: &mut cranelift_codegen::Context,
         ctrl_plane: &mut ControlPlane,
-    ) -> ModuleResult<ModuleCompiledFunction> {
+    ) -> ModuleResult<()> {
         info!("defining function {}: {}", id, ctx.func.display());
         let decl = self.declarations.get_function_decl(id);
         if !decl.linkage.is_definable() {
-            return Err(ModuleError::InvalidImportDefinition(decl.name.clone()));
+            return Err(ModuleError::InvalidImportDefinition(
+                decl.linkage_name(id).into_owned(),
+            ));
         }
 
         if !self.compiled_functions[id].is_none() {
-            return Err(ModuleError::DuplicateDefinition(decl.name.to_owned()));
+            return Err(ModuleError::DuplicateDefinition(
+                decl.linkage_name(id).into_owned(),
+            ));
         }
 
         if self.hotswap_enabled {
@@ -678,9 +690,7 @@ impl Module for JITModule {
         let alignment = res.alignment as u64;
         let compiled_code = ctx.compiled_code().unwrap();
 
-        let code_size = compiled_code.code_info().total_size;
+        let size = compiled_code.code_info().total_size as usize;
-
-        let size = code_size as usize;
         let align = alignment
             .max(self.isa.function_alignment() as u64)
             .max(self.isa.symbol_alignment());
@@ -705,7 +715,7 @@ impl Module for JITModule {
             .map(|reloc| ModuleReloc::from_mach_reloc(reloc, &ctx.func))
             .collect();
 
-        self.record_function_for_perf(ptr, size, &decl.name);
+        self.record_function_for_perf(ptr, size, &decl.linkage_name(id));
         self.compiled_functions[id] = Some(CompiledBlob { ptr, size, relocs });
 
         if self.isa.flags().is_pic() {
@@ -739,7 +749,7 @@ impl Module for JITModule {
             self.functions_to_finalize.push(id);
         }
 
-        Ok(ModuleCompiledFunction { size: code_size })
+        Ok(())
     }
 
     fn define_function_bytes(
@@ -749,20 +759,19 @@ impl Module for JITModule {
         alignment: u64,
         bytes: &[u8],
         relocs: &[MachReloc],
-    ) -> ModuleResult<ModuleCompiledFunction> {
+    ) -> 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()));
+            return Err(ModuleError::InvalidImportDefinition(
+                decl.linkage_name(id).into_owned(),
+            ));
         }
 
         if !self.compiled_functions[id].is_none() {
-            return Err(ModuleError::DuplicateDefinition(decl.name.to_owned()));
+            return Err(ModuleError::DuplicateDefinition(
+                decl.linkage_name(id).into_owned(),
+            ));
         }
 
         let size = bytes.len();
@@ -782,7 +791,7 @@ impl Module for JITModule {
             ptr::copy_nonoverlapping(bytes.as_ptr(), ptr, size);
         }
 
-        self.record_function_for_perf(ptr, size, &decl.name);
+        self.record_function_for_perf(ptr, size, &decl.linkage_name(id));
         self.compiled_functions[id] = Some(CompiledBlob {
             ptr,
             size,
@@ -812,17 +821,21 @@ impl Module for JITModule {
             self.functions_to_finalize.push(id);
         }
 
-        Ok(ModuleCompiledFunction { size: total_size })
+        Ok(())
     }
 
     fn define_data(&mut self, id: DataId, data: &DataDescription) -> ModuleResult<()> {
         let decl = self.declarations.get_data_decl(id);
         if !decl.linkage.is_definable() {
-            return Err(ModuleError::InvalidImportDefinition(decl.name.clone()));
+            return Err(ModuleError::InvalidImportDefinition(
+                decl.linkage_name(id).into_owned(),
+            ));
         }
 
         if !self.compiled_data_objects[id].is_none() {
-            return Err(ModuleError::DuplicateDefinition(decl.name.to_owned()));
+            return Err(ModuleError::DuplicateDefinition(
+                decl.linkage_name(id).into_owned(),
+            ));
         }
 
         assert!(!decl.tls, "JIT doesn't yet support TLS");

cranelift/module/Cargo.toml

@@ -15,11 +15,15 @@ cranelift-codegen = { workspace = true }
 cranelift-control = { workspace = true }
 hashbrown = { workspace = true, optional = true }
 anyhow = { workspace = true }
+serde = { version = "1.0.94", features = ["derive"], optional = true }
 
 [features]
 default = ["std"]
 std = ["cranelift-codegen/std"]
 core = ["hashbrown", "cranelift-codegen/core"]
 
+# For dependent crates that want to serialize some parts of cranelift
+enable-serde = ["serde", "cranelift-codegen/enable-serde"]
+
 [badges]
 maintenance = { status = "experimental" }

cranelift/module/src/data_context.rs

@@ -13,6 +13,7 @@ use crate::ModuleExtName;
 
 /// This specifies how data is to be initialized.
 #[derive(Clone, PartialEq, Eq, Debug)]
+#[cfg_attr(feature = "enable-serde", derive(serde::Serialize, serde::Deserialize))]
 pub enum Init {
     /// This indicates that no initialization has been specified yet.
     Uninitialized,
@@ -41,6 +42,7 @@ impl Init {
 
 /// A description of a data object.
 #[derive(Clone, Debug)]
+#[cfg_attr(feature = "enable-serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct DataDescription {
     /// How the data should be initialized.
     pub init: Init,

cranelift/module/src/lib.rs

@@ -43,8 +43,7 @@ mod traps;
 pub use crate::data_context::{DataDescription, Init};
 pub use crate::module::{
     DataDeclaration, DataId, FuncId, FuncOrDataId, FunctionDeclaration, Linkage, Module,
-    ModuleCompiledFunction, ModuleDeclarations, ModuleError, ModuleExtName, ModuleReloc,
+    ModuleDeclarations, ModuleError, ModuleExtName, ModuleReloc, ModuleResult,
-    ModuleResult,
 };
 pub use crate::traps::TrapSite;
 

cranelift/module/src/module.rs

@@ -10,12 +10,12 @@ use crate::data_context::DataDescription;
 use core::fmt::Display;
 use cranelift_codegen::binemit::{CodeOffset, Reloc};
 use cranelift_codegen::entity::{entity_impl, PrimaryMap};
-use cranelift_codegen::ir::Function;
+use cranelift_codegen::ir::function::{Function, VersionMarker};
 use cranelift_codegen::settings::SetError;
-use cranelift_codegen::{binemit, MachReloc};
+use cranelift_codegen::MachReloc;
 use cranelift_codegen::{ir, isa, CodegenError, CompileError, Context};
 use cranelift_control::ControlPlane;
-use std::borrow::ToOwned;
+use std::borrow::{Cow, ToOwned};
 use std::string::String;
 
 /// A module relocation.
@@ -55,6 +55,7 @@ impl ModuleReloc {
 
 /// A function identifier for use in the `Module` interface.
 #[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
+#[cfg_attr(feature = "enable-serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct FuncId(u32);
 entity_impl!(FuncId, "funcid");
 
@@ -82,6 +83,7 @@ impl FuncId {
 
 /// A data object identifier for use in the `Module` interface.
 #[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
+#[cfg_attr(feature = "enable-serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct DataId(u32);
 entity_impl!(DataId, "dataid");
 
@@ -109,6 +111,7 @@ impl DataId {
 
 /// Linkage refers to where an entity is defined and who can see it.
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
+#[cfg_attr(feature = "enable-serde", derive(serde::Serialize, serde::Deserialize))]
 pub enum Linkage {
     /// Defined outside of a module.
     Import,
@@ -167,6 +170,7 @@ impl Linkage {
 
 /// A declared name may refer to either a function or data declaration
 #[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Debug)]
+#[cfg_attr(feature = "enable-serde", derive(serde::Serialize, serde::Deserialize))]
 pub enum FuncOrDataId {
     /// When it's a FuncId
     Func(FuncId),
@@ -186,9 +190,10 @@ impl From<FuncOrDataId> for ModuleExtName {
 
 /// Information about a function which can be called.
 #[derive(Debug)]
+#[cfg_attr(feature = "enable-serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct FunctionDeclaration {
     #[allow(missing_docs)]
-    pub name: String,
+    pub name: Option<String>,
     #[allow(missing_docs)]
     pub linkage: Linkage,
     #[allow(missing_docs)]
@@ -196,11 +201,29 @@ pub struct FunctionDeclaration {
 }
 
 impl FunctionDeclaration {
-    fn merge(&mut self, linkage: Linkage, sig: &ir::Signature) -> Result<(), ModuleError> {
+    /// The linkage name of the function.
+    ///
+    /// Synthesized from the given function id if it is an anonymous function.
+    pub fn linkage_name(&self, id: FuncId) -> Cow<'_, str> {
+        match &self.name {
+            Some(name) => Cow::Borrowed(name),
+            // Symbols starting with .L are completely omitted from the symbol table after linking.
+            // Using hexadecimal instead of decimal for slightly smaller symbol names and often
+            // slightly faster linking.
+            None => Cow::Owned(format!(".Lfn{:x}", id.as_u32())),
+        }
+    }
+
+    fn merge(
+        &mut self,
+        id: FuncId,
+        linkage: Linkage,
+        sig: &ir::Signature,
+    ) -> Result<(), ModuleError> {
         self.linkage = Linkage::merge(self.linkage, linkage);
         if &self.signature != sig {
             return Err(ModuleError::IncompatibleSignature(
-                self.name.clone(),
+                self.linkage_name(id).into_owned(),
                 self.signature.clone(),
                 sig.clone(),
             ));
@@ -325,9 +348,10 @@ pub type ModuleResult<T> = Result<T, ModuleError>;
 
 /// Information about a data object which can be accessed.
 #[derive(Debug)]
+#[cfg_attr(feature = "enable-serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct DataDeclaration {
     #[allow(missing_docs)]
-    pub name: String,
+    pub name: Option<String>,
     #[allow(missing_docs)]
     pub linkage: Linkage,
     #[allow(missing_docs)]
@@ -337,6 +361,19 @@ pub struct DataDeclaration {
 }
 
 impl DataDeclaration {
+    /// The linkage name of the data object.
+    ///
+    /// Synthesized from the given data id if it is an anonymous function.
+    pub fn linkage_name(&self, id: DataId) -> Cow<'_, str> {
+        match &self.name {
+            Some(name) => Cow::Borrowed(name),
+            // Symbols starting with .L are completely omitted from the symbol table after linking.
+            // Using hexadecimal instead of decimal for slightly smaller symbol names and often
+            // slightly faster linking.
+            None => Cow::Owned(format!(".Ldata{:x}", id.as_u32())),
+        }
+    }
+
     fn merge(&mut self, linkage: Linkage, writable: bool, tls: bool) {
         self.linkage = Linkage::merge(self.linkage, linkage);
         self.writable = self.writable || writable;
@@ -349,6 +386,7 @@ impl DataDeclaration {
 
 /// A translated `ExternalName` into something global we can handle.
 #[derive(Clone, Debug)]
+#[cfg_attr(feature = "enable-serde", derive(serde::Serialize, serde::Deserialize))]
 pub enum ModuleExtName {
     /// User defined function, converted from `ExternalName::User`.
     User {
@@ -384,11 +422,236 @@ impl Display for ModuleExtName {
 /// into `FunctionDeclaration`s and `DataDeclaration`s.
 #[derive(Debug, Default)]
 pub struct ModuleDeclarations {
+    /// A version marker used to ensure that serialized clif ir is never deserialized with a
+    /// different version of Cranelift.
+    // Note: This must be the first field to ensure that Serde will deserialize it before
+    // attempting to deserialize other fields that are potentially changed between versions.
+    _version_marker: VersionMarker,
+
     names: HashMap<String, FuncOrDataId>,
     functions: PrimaryMap<FuncId, FunctionDeclaration>,
     data_objects: PrimaryMap<DataId, DataDeclaration>,
 }
 
+#[cfg(feature = "enable-serde")]
+mod serialize {
+    // This is manually implementing Serialize and Deserialize to avoid serializing the names field,
+    // which can be entirely reconstructed from the functions and data_objects fields, saving space.
+
+    use super::*;
+
+    use serde::de::{Deserialize, Deserializer, Error, MapAccess, SeqAccess, Unexpected, Visitor};
+    use serde::ser::{Serialize, SerializeStruct, Serializer};
+    use std::fmt;
+
+    fn get_names<E: Error>(
+        functions: &PrimaryMap<FuncId, FunctionDeclaration>,
+        data_objects: &PrimaryMap<DataId, DataDeclaration>,
+    ) -> Result<HashMap<String, FuncOrDataId>, E> {
+        let mut names = HashMap::new();
+        for (func_id, decl) in functions.iter() {
+            if let Some(name) = &decl.name {
+                let old = names.insert(name.clone(), FuncOrDataId::Func(func_id));
+                if old.is_some() {
+                    return Err(E::invalid_value(
+                        Unexpected::Other("duplicate name"),
+                        &"FunctionDeclaration's with no duplicate names",
+                    ));
+                }
+            }
+        }
+        for (data_id, decl) in data_objects.iter() {
+            if let Some(name) = &decl.name {
+                let old = names.insert(name.clone(), FuncOrDataId::Data(data_id));
+                if old.is_some() {
+                    return Err(E::invalid_value(
+                        Unexpected::Other("duplicate name"),
+                        &"DataDeclaration's with no duplicate names",
+                    ));
+                }
+            }
+        }
+        Ok(names)
+    }
+
+    impl Serialize for ModuleDeclarations {
+        fn serialize<S: Serializer>(&self, s: S) -> Result<S::Ok, S::Error> {
+            let ModuleDeclarations {
+                _version_marker,
+                functions,
+                data_objects,
+                names: _,
+            } = self;
+
+            let mut state = s.serialize_struct("ModuleDeclarations", 4)?;
+            state.serialize_field("_version_marker", _version_marker)?;
+            state.serialize_field("functions", functions)?;
+            state.serialize_field("data_objects", data_objects)?;
+            state.end()
+        }
+    }
+
+    enum ModuleDeclarationsField {
+        VersionMarker,
+        Functions,
+        DataObjects,
+        Ignore,
+    }
+
+    struct ModuleDeclarationsFieldVisitor;
+
+    impl<'de> serde::de::Visitor<'de> for ModuleDeclarationsFieldVisitor {
+        type Value = ModuleDeclarationsField;
+
+        fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result {
+            f.write_str("field identifier")
+        }
+
+        fn visit_u64<E: Error>(self, val: u64) -> Result<Self::Value, E> {
+            match val {
+                0u64 => Ok(ModuleDeclarationsField::VersionMarker),
+                1u64 => Ok(ModuleDeclarationsField::Functions),
+                2u64 => Ok(ModuleDeclarationsField::DataObjects),
+                _ => Ok(ModuleDeclarationsField::Ignore),
+            }
+        }
+
+        fn visit_str<E: Error>(self, val: &str) -> Result<Self::Value, E> {
+            match val {
+                "_version_marker" => Ok(ModuleDeclarationsField::VersionMarker),
+                "functions" => Ok(ModuleDeclarationsField::Functions),
+                "data_objects" => Ok(ModuleDeclarationsField::DataObjects),
+                _ => Ok(ModuleDeclarationsField::Ignore),
+            }
+        }
+
+        fn visit_bytes<E: Error>(self, val: &[u8]) -> Result<Self::Value, E> {
+            match val {
+                b"_version_marker" => Ok(ModuleDeclarationsField::VersionMarker),
+                b"functions" => Ok(ModuleDeclarationsField::Functions),
+                b"data_objects" => Ok(ModuleDeclarationsField::DataObjects),
+                _ => Ok(ModuleDeclarationsField::Ignore),
+            }
+        }
+    }
+
+    impl<'de> Deserialize<'de> for ModuleDeclarationsField {
+        #[inline]
+        fn deserialize<D: Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
+            d.deserialize_identifier(ModuleDeclarationsFieldVisitor)
+        }
+    }
+
+    struct ModuleDeclarationsVisitor;
+
+    impl<'de> Visitor<'de> for ModuleDeclarationsVisitor {
+        type Value = ModuleDeclarations;
+
+        fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result {
+            f.write_str("struct ModuleDeclarations")
+        }
+
+        #[inline]
+        fn visit_seq<A: SeqAccess<'de>>(self, mut seq: A) -> Result<Self::Value, A::Error> {
+            let _version_marker = match seq.next_element()? {
+                Some(val) => val,
+                None => {
+                    return Err(Error::invalid_length(
+                        0usize,
+                        &"struct ModuleDeclarations with 4 elements",
+                    ));
+                }
+            };
+            let functions = match seq.next_element()? {
+                Some(val) => val,
+                None => {
+                    return Err(Error::invalid_length(
+                        2usize,
+                        &"struct ModuleDeclarations with 4 elements",
+                    ));
+                }
+            };
+            let data_objects = match seq.next_element()? {
+                Some(val) => val,
+                None => {
+                    return Err(Error::invalid_length(
+                        3usize,
+                        &"struct ModuleDeclarations with 4 elements",
+                    ));
+                }
+            };
+            let names = get_names(&functions, &data_objects)?;
+            Ok(ModuleDeclarations {
+                _version_marker,
+                names,
+                functions,
+                data_objects,
+            })
+        }
+
+        #[inline]
+        fn visit_map<A: MapAccess<'de>>(self, mut map: A) -> Result<Self::Value, A::Error> {
+            let mut _version_marker: Option<VersionMarker> = None;
+            let mut functions: Option<PrimaryMap<FuncId, FunctionDeclaration>> = None;
+            let mut data_objects: Option<PrimaryMap<DataId, DataDeclaration>> = None;
+            while let Some(key) = map.next_key::<ModuleDeclarationsField>()? {
+                match key {
+                    ModuleDeclarationsField::VersionMarker => {
+                        if _version_marker.is_some() {
+                            return Err(Error::duplicate_field("_version_marker"));
+                        }
+                        _version_marker = Some(map.next_value()?);
+                    }
+                    ModuleDeclarationsField::Functions => {
+                        if functions.is_some() {
+                            return Err(Error::duplicate_field("functions"));
+                        }
+                        functions = Some(map.next_value()?);
+                    }
+                    ModuleDeclarationsField::DataObjects => {
+                        if data_objects.is_some() {
+                            return Err(Error::duplicate_field("data_objects"));
+                        }
+                        data_objects = Some(map.next_value()?);
+                    }
+                    _ => {
+                        map.next_value::<serde::de::IgnoredAny>()?;
+                    }
+                }
+            }
+            let _version_marker = match _version_marker {
+                Some(_version_marker) => _version_marker,
+                None => return Err(Error::missing_field("_version_marker")),
+            };
+            let functions = match functions {
+                Some(functions) => functions,
+                None => return Err(Error::missing_field("functions")),
+            };
+            let data_objects = match data_objects {
+                Some(data_objects) => data_objects,
+                None => return Err(Error::missing_field("data_objects")),
+            };
+            let names = get_names(&functions, &data_objects)?;
+            Ok(ModuleDeclarations {
+                _version_marker,
+                names,
+                functions,
+                data_objects,
+            })
+        }
+    }
+
+    impl<'de> Deserialize<'de> for ModuleDeclarations {
+        fn deserialize<D: Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
+            d.deserialize_struct(
+                "ModuleDeclarations",
+                &["_version_marker", "functions", "data_objects"],
+                ModuleDeclarationsVisitor,
+            )
+        }
+    }
+}
+
 impl ModuleDeclarations {
     /// Get the module identifier for a given name, if that name
     /// has been declared.
@@ -439,7 +702,7 @@ impl ModuleDeclarations {
             Occupied(entry) => match *entry.get() {
                 FuncOrDataId::Func(id) => {
                     let existing = &mut self.functions[id];
-                    existing.merge(linkage, signature)?;
+                    existing.merge(id, linkage, signature)?;
                     Ok((id, existing.linkage))
                 }
                 FuncOrDataId::Data(..) => {
@@ -448,7 +711,7 @@ impl ModuleDeclarations {
             },
             Vacant(entry) => {
                 let id = self.functions.push(FunctionDeclaration {
-                    name: name.to_owned(),
+                    name: Some(name.to_owned()),
                     linkage,
                     signature: signature.clone(),
                 });
@@ -464,11 +727,10 @@ impl ModuleDeclarations {
         signature: &ir::Signature,
     ) -> ModuleResult<FuncId> {
         let id = self.functions.push(FunctionDeclaration {
-            name: String::new(),
+            name: None,
             linkage: Linkage::Local,
             signature: signature.clone(),
         });
-        self.functions[id].name = format!(".L{:?}", id);
         Ok(id)
     }
 
@@ -496,7 +758,7 @@ impl ModuleDeclarations {
             },
             Vacant(entry) => {
                 let id = self.data_objects.push(DataDeclaration {
-                    name: name.to_owned(),
+                    name: Some(name.to_owned()),
                     linkage,
                     writable,
                     tls,
@@ -510,22 +772,15 @@ impl ModuleDeclarations {
     /// Declare an anonymous data object in this module.
     pub fn declare_anonymous_data(&mut self, writable: bool, tls: bool) -> ModuleResult<DataId> {
         let id = self.data_objects.push(DataDeclaration {
-            name: String::new(),
+            name: None,
             linkage: Linkage::Local,
             writable,
             tls,
         });
-        self.data_objects[id].name = format!(".L{:?}", id);
         Ok(id)
     }
 }
 
-/// Information about the compiled function.
-pub struct ModuleCompiledFunction {
-    /// The size of the compiled function.
-    pub size: binemit::CodeOffset,
-}
-
 /// A `Module` is a utility for collecting functions and data objects, and linking them together.
 pub trait Module {
     /// Return the `TargetIsa` to compile for.
@@ -660,11 +915,7 @@ pub trait Module {
     /// Note: After calling this function the given `Context` will contain the compiled function.
     ///
     /// [`define_function_with_control_plane`]: Self::define_function_with_control_plane
-    fn define_function(
+    fn define_function(&mut self, func: FuncId, ctx: &mut Context) -> ModuleResult<()> {
-        &mut self,
-        func: FuncId,
-        ctx: &mut Context,
-    ) -> ModuleResult<ModuleCompiledFunction> {
         self.define_function_with_control_plane(func, ctx, &mut ControlPlane::default())
     }
 
@@ -678,7 +929,7 @@ pub trait Module {
         func: FuncId,
         ctx: &mut Context,
         ctrl_plane: &mut ControlPlane,
-    ) -> ModuleResult<ModuleCompiledFunction>;
+    ) -> ModuleResult<()>;
 
     /// Define a function, taking the function body from the given `bytes`.
     ///
@@ -694,7 +945,7 @@ pub trait Module {
         alignment: u64,
         bytes: &[u8],
         relocs: &[MachReloc],
-    ) -> ModuleResult<ModuleCompiledFunction>;
+    ) -> ModuleResult<()>;
 
     /// Define a data object, producing the data contents from the given `DataContext`.
     fn define_data(&mut self, data_id: DataId, data: &DataDescription) -> ModuleResult<()>;
@@ -776,11 +1027,7 @@ impl<M: Module> Module for &mut M {
         (**self).declare_data_in_data(data_id, data)
     }
 
-    fn define_function(
+    fn define_function(&mut self, func: FuncId, ctx: &mut Context) -> ModuleResult<()> {
-        &mut self,
-        func: FuncId,
-        ctx: &mut Context,
-    ) -> ModuleResult<ModuleCompiledFunction> {
         (**self).define_function(func, ctx)
     }
 
@@ -789,7 +1036,7 @@ impl<M: Module> Module for &mut M {
         func: FuncId,
         ctx: &mut Context,
         ctrl_plane: &mut ControlPlane,
-    ) -> ModuleResult<ModuleCompiledFunction> {
+    ) -> ModuleResult<()> {
         (**self).define_function_with_control_plane(func, ctx, ctrl_plane)
     }
 
@@ -800,7 +1047,7 @@ impl<M: Module> Module for &mut M {
         alignment: u64,
         bytes: &[u8],
         relocs: &[MachReloc],
-    ) -> ModuleResult<ModuleCompiledFunction> {
+    ) -> ModuleResult<()> {
         (**self).define_function_bytes(func_id, func, alignment, bytes, relocs)
     }
 

cranelift/object/src/backend.rs

@@ -1,17 +1,14 @@
 //! Defines `ObjectModule`.
 
 use anyhow::anyhow;
+use cranelift_codegen::binemit::{Addend, CodeOffset, Reloc};
 use cranelift_codegen::entity::SecondaryMap;
 use cranelift_codegen::isa::{OwnedTargetIsa, TargetIsa};
 use cranelift_codegen::{self, ir, MachReloc};
-use cranelift_codegen::{
-    binemit::{Addend, CodeOffset, Reloc},
-    CodegenError,
-};
 use cranelift_control::ControlPlane;
 use cranelift_module::{
-    DataDescription, DataId, FuncId, Init, Linkage, Module, ModuleCompiledFunction,
+    DataDescription, DataId, FuncId, Init, Linkage, Module, ModuleDeclarations, ModuleError,
-    ModuleDeclarations, ModuleError, ModuleExtName, ModuleReloc, ModuleResult,
+    ModuleExtName, ModuleReloc, ModuleResult,
 };
 use log::info;
 use object::write::{
@@ -21,7 +18,6 @@ use object::{
     RelocationEncoding, RelocationKind, SectionKind, SymbolFlags, SymbolKind, SymbolScope,
 };
 use std::collections::HashMap;
-use std::convert::TryInto;
 use std::mem;
 use target_lexicon::PointerWidth;
 
@@ -135,8 +131,6 @@ pub struct ObjectModule {
     libcall_names: Box<dyn Fn(ir::LibCall) -> String + Send + Sync>,
     known_symbols: HashMap<ir::KnownSymbol, SymbolId>,
     per_function_section: bool,
-    anon_func_number: u64,
-    anon_data_number: u64,
 }
 
 impl ObjectModule {
@@ -156,8 +150,6 @@ impl ObjectModule {
             libcall_names: builder.libcall_names,
             known_symbols: HashMap::new(),
             per_function_section: builder.per_function_section,
-            anon_func_number: 0,
-            anon_data_number: 0,
         }
     }
 }
@@ -219,16 +211,15 @@ impl Module for ObjectModule {
     }
 
     fn declare_anonymous_function(&mut self, signature: &ir::Signature) -> ModuleResult<FuncId> {
-        // Symbols starting with .L are completely omitted from the symbol table after linking.
-        // Using hexadecimal instead of decimal for slightly smaller symbol names and often slightly
-        // faster linking.
-        let name = format!(".Lfn{:x}", self.anon_func_number);
-        self.anon_func_number += 1;
-
         let id = self.declarations.declare_anonymous_function(signature)?;
 
         let symbol_id = self.object.add_symbol(Symbol {
-            name: name.as_bytes().to_vec(),
+            name: self
+                .declarations
+                .get_function_decl(id)
+                .linkage_name(id)
+                .into_owned()
+                .into_bytes(),
             value: 0,
             size: 0,
             kind: SymbolKind::Text,
@@ -287,12 +278,6 @@ impl Module for ObjectModule {
     }
 
     fn declare_anonymous_data(&mut self, writable: bool, tls: bool) -> ModuleResult<DataId> {
-        // Symbols starting with .L are completely omitted from the symbol table after linking.
-        // Using hexadecimal instead of decimal for slightly smaller symbol names and often slightly
-        // faster linking.
-        let name = format!(".Ldata{:x}", self.anon_data_number);
-        self.anon_data_number += 1;
-
         let id = self.declarations.declare_anonymous_data(writable, tls)?;
 
         let kind = if tls {
@@ -302,7 +287,12 @@ impl Module for ObjectModule {
         };
 
         let symbol_id = self.object.add_symbol(Symbol {
-            name: name.as_bytes().to_vec(),
+            name: self
+                .declarations
+                .get_data_decl(id)
+                .linkage_name(id)
+                .into_owned()
+                .into_bytes(),
             value: 0,
             size: 0,
             kind,
@@ -321,7 +311,7 @@ impl Module for ObjectModule {
         func_id: FuncId,
         ctx: &mut cranelift_codegen::Context,
         ctrl_plane: &mut ControlPlane,
-    ) -> ModuleResult<ModuleCompiledFunction> {
+    ) -> ModuleResult<()> {
         info!("defining function {}: {}", func_id, ctx.func.display());
         let mut code: Vec<u8> = Vec::new();
 
@@ -344,21 +334,20 @@ impl Module for ObjectModule {
         alignment: u64,
         bytes: &[u8],
         relocs: &[MachReloc],
-    ) -> ModuleResult<ModuleCompiledFunction> {
+    ) -> 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()));
+            return Err(ModuleError::InvalidImportDefinition(
+                decl.linkage_name(func_id).into_owned(),
+            ));
         }
 
         let &mut (symbol, ref mut defined) = self.functions[func_id].as_mut().unwrap();
         if *defined {
-            return Err(ModuleError::DuplicateDefinition(decl.name.clone()));
+            return Err(ModuleError::DuplicateDefinition(
+                decl.linkage_name(func_id).into_owned(),
+            ));
         }
         *defined = true;
 
@@ -391,18 +380,22 @@ impl Module for ObjectModule {
             });
         }
 
-        Ok(ModuleCompiledFunction { size: total_size })
+        Ok(())
     }
 
     fn define_data(&mut self, data_id: DataId, data: &DataDescription) -> ModuleResult<()> {
         let decl = self.declarations.get_data_decl(data_id);
         if !decl.linkage.is_definable() {
-            return Err(ModuleError::InvalidImportDefinition(decl.name.clone()));
+            return Err(ModuleError::InvalidImportDefinition(
+                decl.linkage_name(data_id).into_owned(),
+            ));
         }
 
         let &mut (symbol, ref mut defined) = self.data_objects[data_id].as_mut().unwrap();
         if *defined {
-            return Err(ModuleError::DuplicateDefinition(decl.name.clone()));
+            return Err(ModuleError::DuplicateDefinition(
+                decl.linkage_name(data_id).into_owned(),
+            ));
         }
         *defined = true;