From 91d1d246cd2ee8d854bba220900ce6efd255b02b Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 21 Apr 2023 14:39:15 +0200 Subject: [PATCH] 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 | 1 + cranelift/codegen/src/ir/function.rs | 2 +- cranelift/jit/src/backend.rs | 65 +++--- cranelift/module/Cargo.toml | 4 + cranelift/module/src/data_context.rs | 2 + cranelift/module/src/lib.rs | 3 +- cranelift/module/src/module.rs | 315 ++++++++++++++++++++++++--- cranelift/object/src/backend.rs | 67 +++--- 8 files changed, 359 insertions(+), 100 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8e5c411371..9811cb8cb1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -747,6 +747,7 @@ dependencies = [ "cranelift-codegen", "cranelift-control", "hashbrown 0.13.2", + "serde", ] [[package]] diff --git a/cranelift/codegen/src/ir/function.rs b/cranelift/codegen/src/ir/function.rs index 74501f1fda..a7a8eee275 100644 --- a/cranelift/codegen/src/ir/function.rs +++ b/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")] diff --git a/cranelift/jit/src/backend.rs b/cranelift/jit/src/backend.rs index ce00451f46..a2dacf070d 100644 --- a/cranelift/jit/src/backend.rs +++ b/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, - ModuleDeclarations, ModuleError, ModuleExtName, ModuleReloc, ModuleResult, + DataDescription, DataId, FuncId, Init, Linkage, Module, ModuleDeclarations, ModuleError, + 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 { + ) -> 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 = code_size as usize; + let size = compiled_code.code_info().total_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 { + ) -> 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"); diff --git a/cranelift/module/Cargo.toml b/cranelift/module/Cargo.toml index 16419af867..542de4d797 100644 --- a/cranelift/module/Cargo.toml +++ b/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" } diff --git a/cranelift/module/src/data_context.rs b/cranelift/module/src/data_context.rs index 3188ad90c4..3e1e30a111 100644 --- a/cranelift/module/src/data_context.rs +++ b/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, diff --git a/cranelift/module/src/lib.rs b/cranelift/module/src/lib.rs index bf561083be..046204ecbf 100644 --- a/cranelift/module/src/lib.rs +++ b/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, - ModuleResult, + ModuleDeclarations, ModuleError, ModuleExtName, ModuleReloc, ModuleResult, }; pub use crate::traps::TrapSite; diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index 5eddbe07cb..1b524cad85 100644 --- a/cranelift/module/src/module.rs +++ b/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 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, #[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 = Result; /// 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, #[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, functions: PrimaryMap, data_objects: PrimaryMap, } +#[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( + functions: &PrimaryMap, + data_objects: &PrimaryMap, + ) -> Result, 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(&self, s: S) -> Result { + 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(self, val: u64) -> Result { + match val { + 0u64 => Ok(ModuleDeclarationsField::VersionMarker), + 1u64 => Ok(ModuleDeclarationsField::Functions), + 2u64 => Ok(ModuleDeclarationsField::DataObjects), + _ => Ok(ModuleDeclarationsField::Ignore), + } + } + + fn visit_str(self, val: &str) -> Result { + match val { + "_version_marker" => Ok(ModuleDeclarationsField::VersionMarker), + "functions" => Ok(ModuleDeclarationsField::Functions), + "data_objects" => Ok(ModuleDeclarationsField::DataObjects), + _ => Ok(ModuleDeclarationsField::Ignore), + } + } + + fn visit_bytes(self, val: &[u8]) -> Result { + 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: D) -> Result { + 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>(self, mut seq: A) -> Result { + 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>(self, mut map: A) -> Result { + let mut _version_marker: Option = None; + let mut functions: Option> = None; + let mut data_objects: Option> = None; + while let Some(key) = map.next_key::()? { + 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::()?; + } + } + } + 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: D) -> Result { + 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 { 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 { 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( - &mut self, - func: FuncId, - ctx: &mut Context, - ) -> ModuleResult { + fn define_function(&mut self, func: FuncId, ctx: &mut Context) -> ModuleResult<()> { 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; + ) -> 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; + ) -> 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 Module for &mut M { (**self).declare_data_in_data(data_id, data) } - fn define_function( - &mut self, - func: FuncId, - ctx: &mut Context, - ) -> ModuleResult { + fn define_function(&mut self, func: FuncId, ctx: &mut Context) -> ModuleResult<()> { (**self).define_function(func, ctx) } @@ -789,7 +1036,7 @@ impl Module for &mut M { func: FuncId, ctx: &mut Context, ctrl_plane: &mut ControlPlane, - ) -> ModuleResult { + ) -> ModuleResult<()> { (**self).define_function_with_control_plane(func, ctx, ctrl_plane) } @@ -800,7 +1047,7 @@ impl Module for &mut M { alignment: u64, bytes: &[u8], relocs: &[MachReloc], - ) -> ModuleResult { + ) -> ModuleResult<()> { (**self).define_function_bytes(func_id, func, alignment, bytes, relocs) } diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index 5a2421329a..d8bc2a69a1 100644 --- a/cranelift/object/src/backend.rs +++ b/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, - ModuleDeclarations, ModuleError, ModuleExtName, ModuleReloc, ModuleResult, + DataDescription, DataId, FuncId, Init, Linkage, Module, ModuleDeclarations, ModuleError, + 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 String + Send + Sync>, known_symbols: HashMap, 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 { - // 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 { - // 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 { + ) -> ModuleResult<()> { info!("defining function {}: {}", func_id, ctx.func.display()); let mut code: Vec = Vec::new(); @@ -344,21 +334,20 @@ impl Module for ObjectModule { alignment: u64, bytes: &[u8], relocs: &[MachReloc], - ) -> ModuleResult { + ) -> 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;