From 2346fe7de64bf8632d576d7154120c7ae6379111 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sun, 11 Oct 2020 16:16:39 +0200 Subject: [PATCH 1/6] Make some things private --- cranelift/simplejit/src/backend.rs | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index 369561f06b..b5c5f1f87a 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -151,14 +151,14 @@ struct StackMapRecord { } #[derive(Clone)] -pub struct SimpleJITCompiledFunction { +struct SimpleJITCompiledFunction { code: *mut u8, size: usize, relocs: Vec, } #[derive(Clone)] -pub struct SimpleJITCompiledData { +struct SimpleJITCompiledData { storage: *mut u8, size: usize, relocs: Vec, @@ -451,8 +451,8 @@ impl<'simple_jit_backend> Module for SimpleJITModule { self.record_function_for_perf(ptr, size, &decl.name); - let mut reloc_sink = SimpleJITRelocSink::new(); - let mut stack_map_sink = SimpleJITStackMapSink::new(); + let mut reloc_sink = SimpleJITRelocSink::default(); + let mut stack_map_sink = SimpleJITStackMapSink::default(); unsafe { ctx.emit_to_memory( &*self.isa, @@ -673,14 +673,9 @@ fn lookup_with_dlsym(name: &str) -> *const u8 { } } +#[derive(Default)] struct SimpleJITRelocSink { - pub relocs: Vec, -} - -impl SimpleJITRelocSink { - pub fn new() -> Self { - Self { relocs: Vec::new() } - } + relocs: Vec, } impl RelocSink for SimpleJITRelocSink { @@ -729,16 +724,9 @@ impl RelocSink for SimpleJITRelocSink { } } +#[derive(Default)] struct SimpleJITStackMapSink { - pub stack_maps: Vec, -} - -impl SimpleJITStackMapSink { - pub fn new() -> Self { - Self { - stack_maps: Vec::new(), - } - } + stack_maps: Vec, } impl StackMapSink for SimpleJITStackMapSink { From 589f4a4405f605df95852c0b06995e18a38cb643 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sun, 11 Oct 2020 16:18:27 +0200 Subject: [PATCH 2/6] Re-introduce finalize_definitions and get_finalized_* for simplejit --- cranelift/simplejit/src/backend.rs | 64 +++++++++++++++++++++++------- 1 file changed, 50 insertions(+), 14 deletions(-) diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index b5c5f1f87a..4324817add 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -252,6 +252,32 @@ impl SimpleJITModule { } } + /// Returns the address of a finalized function. + pub fn get_finalized_function(&self, func_id: FuncId) -> *const u8 { + let info = &self.functions[func_id]; + debug_assert!( + !self.functions_to_finalize.iter().any(|x| *x == func_id), + "function not yet finalized" + ); + info.as_ref() + .expect("function must be compiled before it can be finalized") + .code + } + + /// Returns the address and size of a finalized data object. + pub fn get_finalized_data(&self, data_id: DataId) -> (*const u8, usize) { + let info = &self.data_objects[data_id]; + debug_assert!( + !self.data_objects_to_finalize.iter().any(|x| *x == data_id), + "data object not yet finalized" + ); + let compiled = info + .as_ref() + .expect("data object must be compiled before it can be finalized"); + + (compiled.storage, compiled.size) + } + fn record_function_for_perf(&self, ptr: *mut u8, size: usize, name: &str) { // The Linux perf tool supports JIT code via a /tmp/perf-$PID.map file, // which contains memory regions and their associated names. If we @@ -360,6 +386,29 @@ impl SimpleJITModule { } } + /// Finalize all functions and data objects that are defined but not yet finalized. + /// All symbols referenced in their bodies that are declared as needing a definition + /// must be defined by this point. + /// + /// Use `get_finalized_function` and `get_finalized_data` to obtain the final + /// artifacts. + pub fn finalize_definitions(&mut self) { + for func in std::mem::take(&mut self.functions_to_finalize) { + let decl = self.declarations.get_function_decl(func); + debug_assert!(decl.linkage.is_definable()); + self.finalize_function(func); + } + for data in std::mem::take(&mut self.data_objects_to_finalize) { + let decl = self.declarations.get_data_decl(data); + debug_assert!(decl.linkage.is_definable()); + self.finalize_data(data); + } + + // Now that we're done patching, prepare the memory for execution! + self.memory.readonly.set_readonly(); + self.memory.code.set_readable_and_executable(); + } + /// Create a new `SimpleJITModule`. pub fn new(builder: SimpleJITBuilder) -> Self { let memory = SimpleJITMemoryHandle { @@ -606,20 +655,7 @@ impl SimpleJITModule { /// This method does not need to be called when access to the memory /// handle is not required. pub fn finish(mut self) -> SimpleJITProduct { - for func in std::mem::take(&mut self.functions_to_finalize) { - let decl = self.declarations.get_function_decl(func); - debug_assert!(decl.linkage.is_definable()); - self.finalize_function(func); - } - for data in std::mem::take(&mut self.data_objects_to_finalize) { - let decl = self.declarations.get_data_decl(data); - debug_assert!(decl.linkage.is_definable()); - self.finalize_data(data); - } - - // Now that we're done patching, prepare the memory for execution! - self.memory.readonly.set_readonly(); - self.memory.code.set_readable_and_executable(); + self.finalize_definitions(); SimpleJITProduct { memory: self.memory, From 6b0d246b2480eca69da31a9ddcd80e2d5729acad Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sun, 11 Oct 2020 16:23:43 +0200 Subject: [PATCH 3/6] Implement Linkage::Preemptible for simplejit --- cranelift/simplejit/src/backend.rs | 50 +++++++++++++++++------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index 4324817add..67f775dfbc 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -218,35 +218,47 @@ impl SimpleJITProduct { } impl SimpleJITModule { - fn lookup_symbol(&self, name: &str) -> *const u8 { - match self.symbols.get(name) { - Some(&ptr) => ptr, - None => lookup_with_dlsym(name), - } + fn lookup_symbol(&self, name: &str) -> Option<*const u8> { + self.symbols + .get(name) + .copied() + .or_else(|| lookup_with_dlsym(name)) } fn get_definition(&self, name: &ir::ExternalName) -> *const u8 { match *name { ir::ExternalName::User { .. } => { - if self.declarations.is_function(name) { + let (name, linkage) = if self.declarations.is_function(name) { let func_id = self.declarations.get_function_id(name); match &self.functions[func_id] { - Some(compiled) => compiled.code, + Some(compiled) => return compiled.code, None => { - self.lookup_symbol(&self.declarations.get_function_decl(func_id).name) + let decl = self.declarations.get_function_decl(func_id); + (&decl.name, decl.linkage) } } } else { let data_id = self.declarations.get_data_id(name); match &self.data_objects[data_id] { - Some(compiled) => compiled.storage, - None => self.lookup_symbol(&self.declarations.get_data_decl(data_id).name), + Some(compiled) => return compiled.storage, + None => { + let decl = self.declarations.get_data_decl(data_id); + (&decl.name, decl.linkage) + } } + }; + if let Some(ptr) = self.lookup_symbol(&name) { + ptr + } else if linkage == Linkage::Preemptible { + 0 as *const u8 + } else { + panic!("can't resolve symbol {}", name); } } ir::ExternalName::LibCall(ref libcall) => { let sym = (self.libcall_names)(*libcall); self.lookup_symbol(&sym) + .unwrap_or_else(|| panic!("can't resolve libcall {}", sym)) } _ => panic!("invalid ExternalName {}", name), } @@ -667,18 +679,19 @@ impl SimpleJITModule { } #[cfg(not(windows))] -fn lookup_with_dlsym(name: &str) -> *const u8 { +fn lookup_with_dlsym(name: &str) -> Option<*const u8> { let c_str = CString::new(name).unwrap(); let c_str_ptr = c_str.as_ptr(); let sym = unsafe { libc::dlsym(libc::RTLD_DEFAULT, c_str_ptr) }; if sym.is_null() { - panic!("can't resolve symbol {}", name); + None + } else { + Some(sym as *const u8) } - sym as *const u8 } #[cfg(windows)] -fn lookup_with_dlsym(name: &str) -> *const u8 { +fn lookup_with_dlsym(name: &str) -> Option<*const u8> { const MSVCRT_DLL: &[u8] = b"msvcrt.dll\0"; let c_str = CString::new(name).unwrap(); @@ -697,15 +710,10 @@ fn lookup_with_dlsym(name: &str) -> *const u8 { if addr.is_null() { continue; } - return addr as *const u8; + return Some(addr as *const u8); } - let msg = if handles[1].is_null() { - "(msvcrt not loaded)" - } else { - "" - }; - panic!("cannot resolve address of symbol {} {}", name, msg); + None } } From 7a2a4bc9037ce6a22227296343ac13202fcd0022 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 12 Oct 2020 11:45:01 +0200 Subject: [PATCH 4/6] Add some debug derives --- cranelift/module/src/module.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index 58ddb2aa76..3b87876c11 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -122,6 +122,7 @@ impl From for ir::ExternalName { } /// Information about a function which can be called. +#[derive(Debug)] pub struct FunctionDeclaration { pub name: String, pub linkage: Linkage, @@ -176,6 +177,7 @@ pub enum ModuleError { pub type ModuleResult = Result; /// Information about a data object which can be accessed. +#[derive(Debug)] pub struct DataDeclaration { pub name: String, pub linkage: Linkage, @@ -196,7 +198,7 @@ impl DataDeclaration { /// This provides a view to the state of a module which allows `ir::ExternalName`s to be translated /// into `FunctionDeclaration`s and `DataDeclaration`s. -#[derive(Default)] +#[derive(Debug, Default)] pub struct ModuleDeclarations { names: HashMap, functions: PrimaryMap, From 69513ee85e3ca6c3cff7facf7040c8eedaa0d896 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 12 Oct 2020 14:02:37 +0200 Subject: [PATCH 5/6] Merge CompiledFunction and CompiledData --- cranelift/simplejit/src/backend.rs | 55 +++++++++++++----------------- 1 file changed, 23 insertions(+), 32 deletions(-) diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index 67f775dfbc..06d22dbf14 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -128,8 +128,8 @@ pub struct SimpleJITModule { libcall_names: Box String>, memory: SimpleJITMemoryHandle, declarations: ModuleDeclarations, - functions: SecondaryMap>, - data_objects: SecondaryMap>, + functions: SecondaryMap>, + data_objects: SecondaryMap>, functions_to_finalize: Vec, data_objects_to_finalize: Vec, } @@ -151,15 +151,8 @@ struct StackMapRecord { } #[derive(Clone)] -struct SimpleJITCompiledFunction { - code: *mut u8, - size: usize, - relocs: Vec, -} - -#[derive(Clone)] -struct SimpleJITCompiledData { - storage: *mut u8, +struct SimpleJITCompiledBlob { + ptr: *mut u8, size: usize, relocs: Vec, } @@ -176,8 +169,8 @@ struct SimpleJITMemoryHandle { pub struct SimpleJITProduct { memory: SimpleJITMemoryHandle, declarations: ModuleDeclarations, - functions: SecondaryMap>, - data_objects: SecondaryMap>, + functions: SecondaryMap>, + data_objects: SecondaryMap>, } impl SimpleJITProduct { @@ -205,7 +198,7 @@ impl SimpleJITProduct { self.functions[func_id] .as_ref() .unwrap_or_else(|| panic!("{} is not defined", func_id)) - .code + .ptr } /// Return the address and size of a data object. @@ -213,7 +206,7 @@ impl SimpleJITProduct { let data = self.data_objects[data_id] .as_ref() .unwrap_or_else(|| panic!("{} is not defined", data_id)); - (data.storage, data.size) + (data.ptr, data.size) } } @@ -231,7 +224,7 @@ impl SimpleJITModule { let (name, linkage) = if self.declarations.is_function(name) { let func_id = self.declarations.get_function_id(name); match &self.functions[func_id] { - Some(compiled) => return compiled.code, + Some(compiled) => return compiled.ptr, None => { let decl = self.declarations.get_function_decl(func_id); (&decl.name, decl.linkage) @@ -240,7 +233,7 @@ impl SimpleJITModule { } else { let data_id = self.declarations.get_data_id(name); match &self.data_objects[data_id] { - Some(compiled) => return compiled.storage, + Some(compiled) => return compiled.ptr, None => { let decl = self.declarations.get_data_decl(data_id); (&decl.name, decl.linkage) @@ -273,7 +266,7 @@ impl SimpleJITModule { ); info.as_ref() .expect("function must be compiled before it can be finalized") - .code + .ptr } /// Returns the address and size of a finalized data object. @@ -287,7 +280,7 @@ impl SimpleJITModule { .as_ref() .expect("data object must be compiled before it can be finalized"); - (compiled.storage, compiled.size) + (compiled.ptr, compiled.size) } fn record_function_for_perf(&self, ptr: *mut u8, size: usize, name: &str) { @@ -321,9 +314,8 @@ impl SimpleJITModule { addend, } in &func.relocs { - let ptr = func.code; debug_assert!((offset as usize) < func.size); - let at = unsafe { ptr.offset(offset as isize) }; + let at = unsafe { func.ptr.offset(offset as isize) }; let base = self.get_definition(name); // TODO: Handle overflow. let what = unsafe { base.offset(addend as isize) }; @@ -369,9 +361,8 @@ impl SimpleJITModule { addend, } in &data.relocs { - let ptr = data.storage; debug_assert!((offset as usize) < data.size); - let at = unsafe { ptr.offset(offset as isize) }; + let at = unsafe { data.ptr.offset(offset as isize) }; let base = self.get_definition(name); // TODO: Handle overflow. let what = unsafe { base.offset(addend as isize) }; @@ -524,8 +515,8 @@ impl<'simple_jit_backend> Module for SimpleJITModule { ) }; - self.functions[id] = Some(SimpleJITCompiledFunction { - code: ptr, + self.functions[id] = Some(SimpleJITCompiledBlob { + ptr, size, relocs: reloc_sink.relocs, }); @@ -566,8 +557,8 @@ impl<'simple_jit_backend> Module for SimpleJITModule { ptr::copy_nonoverlapping(bytes.as_ptr(), ptr, size); } - self.functions[id] = Some(SimpleJITCompiledFunction { - code: ptr, + self.functions[id] = Some(SimpleJITCompiledBlob { + ptr, size, relocs: vec![], }); @@ -600,7 +591,7 @@ impl<'simple_jit_backend> Module for SimpleJITModule { } = data.description(); let size = init.size(); - let storage = if decl.writable { + let ptr = if decl.writable { self.memory .writable .allocate(size, align.unwrap_or(WRITABLE_DATA_ALIGNMENT)) @@ -617,11 +608,11 @@ impl<'simple_jit_backend> Module for SimpleJITModule { panic!("data is not initialized yet"); } Init::Zeros { .. } => { - unsafe { ptr::write_bytes(storage, 0, size) }; + unsafe { ptr::write_bytes(ptr, 0, size) }; } Init::Bytes { ref contents } => { let src = contents.as_ptr(); - unsafe { ptr::copy_nonoverlapping(src, storage, size) }; + unsafe { ptr::copy_nonoverlapping(src, ptr, size) }; } } @@ -648,8 +639,8 @@ impl<'simple_jit_backend> Module for SimpleJITModule { }); } - self.data_objects[id] = Some(SimpleJITCompiledData { - storage, + self.data_objects[id] = Some(SimpleJITCompiledBlob { + ptr, size, relocs, }); From cb2d180f2a8266477ded2a7de0fc0e8a758f0dc4 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 12 Oct 2020 14:06:38 +0200 Subject: [PATCH 6/6] Remove SimpleJIT prefix from some private types --- cranelift/simplejit/src/backend.rs | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index 06d22dbf14..d14d2cc29c 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -126,10 +126,10 @@ pub struct SimpleJITModule { isa: Box, symbols: HashMap, libcall_names: Box String>, - memory: SimpleJITMemoryHandle, + memory: MemoryHandle, declarations: ModuleDeclarations, - functions: SecondaryMap>, - data_objects: SecondaryMap>, + functions: SecondaryMap>, + data_objects: SecondaryMap>, functions_to_finalize: Vec, data_objects_to_finalize: Vec, } @@ -151,14 +151,14 @@ struct StackMapRecord { } #[derive(Clone)] -struct SimpleJITCompiledBlob { +struct CompiledBlob { ptr: *mut u8, size: usize, relocs: Vec, } /// A handle to allow freeing memory allocated by the `Module`. -struct SimpleJITMemoryHandle { +struct MemoryHandle { code: Memory, readonly: Memory, writable: Memory, @@ -167,10 +167,10 @@ struct SimpleJITMemoryHandle { /// A `SimpleJITProduct` allows looking up the addresses of all functions and data objects /// defined in the original module. pub struct SimpleJITProduct { - memory: SimpleJITMemoryHandle, + memory: MemoryHandle, declarations: ModuleDeclarations, - functions: SecondaryMap>, - data_objects: SecondaryMap>, + functions: SecondaryMap>, + data_objects: SecondaryMap>, } impl SimpleJITProduct { @@ -414,7 +414,7 @@ impl SimpleJITModule { /// Create a new `SimpleJITModule`. pub fn new(builder: SimpleJITBuilder) -> Self { - let memory = SimpleJITMemoryHandle { + let memory = MemoryHandle { code: Memory::new(), readonly: Memory::new(), writable: Memory::new(), @@ -515,7 +515,7 @@ impl<'simple_jit_backend> Module for SimpleJITModule { ) }; - self.functions[id] = Some(SimpleJITCompiledBlob { + self.functions[id] = Some(CompiledBlob { ptr, size, relocs: reloc_sink.relocs, @@ -557,7 +557,7 @@ impl<'simple_jit_backend> Module for SimpleJITModule { ptr::copy_nonoverlapping(bytes.as_ptr(), ptr, size); } - self.functions[id] = Some(SimpleJITCompiledBlob { + self.functions[id] = Some(CompiledBlob { ptr, size, relocs: vec![], @@ -639,11 +639,7 @@ impl<'simple_jit_backend> Module for SimpleJITModule { }); } - self.data_objects[id] = Some(SimpleJITCompiledBlob { - ptr, - size, - relocs, - }); + self.data_objects[id] = Some(CompiledBlob { ptr, size, relocs }); Ok(()) }