From eade923d19c2c547afdd017c1a02ff0ee6ce2dec Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Fri, 31 May 2024 06:47:29 -0700 Subject: [PATCH 1/3] Impl Send for JITModule Ref: https://bytecodealliance.zulipchat.com/#narrow/stream/206238-general/topic/Cross.20thread.20JITModule/near/441536899 --- cranelift/jit/src/backend.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cranelift/jit/src/backend.rs b/cranelift/jit/src/backend.rs index 226d3c9afba1..480c2c2b6c0b 100644 --- a/cranelift/jit/src/backend.rs +++ b/cranelift/jit/src/backend.rs @@ -28,7 +28,7 @@ const READONLY_DATA_ALIGNMENT: u64 = 0x1; pub struct JITBuilder { isa: OwnedTargetIsa, symbols: HashMap, - lookup_symbols: Vec Option<*const u8>>>, + lookup_symbols: Vec Option<*const u8> + Send>>, libcall_names: Box String + Send + Sync>, hotswap_enabled: bool, } @@ -140,7 +140,7 @@ impl JITBuilder { /// symbol table. Symbol lookup fn's are called in reverse of the order in which they were added. pub fn symbol_lookup_fn( &mut self, - symbol_lookup_fn: Box Option<*const u8>>, + symbol_lookup_fn: Box Option<*const u8> + Send>, ) -> &mut Self { self.lookup_symbols.push(symbol_lookup_fn); self @@ -173,8 +173,8 @@ pub struct JITModule { isa: OwnedTargetIsa, hotswap_enabled: bool, symbols: RefCell>, - lookup_symbols: Vec Option<*const u8>>>, - libcall_names: Box String>, + lookup_symbols: Vec Option<*const u8> + Send>>, + libcall_names: Box String + Send + Sync>, memory: MemoryHandle, declarations: ModuleDeclarations, function_got_entries: SecondaryMap>>>, @@ -191,6 +191,8 @@ pub struct JITModule { pending_got_updates: Vec, } +unsafe impl Send for JITModule {} + /// A handle to allow freeing memory allocated by the `Module`. struct MemoryHandle { code: Memory, From 31b1790c453cd95c82750ceaecfff1b888098e9d Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Fri, 31 May 2024 08:15:05 -0700 Subject: [PATCH 2/3] impl Send on Memory,CompiledBlob,GotUpdate and wrap remaining Sendable fields in JITModule with a SendWrapper --- cranelift/jit/src/backend.rs | 77 +++++++++++++++++++----------- cranelift/jit/src/compiled_blob.rs | 2 + cranelift/jit/src/memory.rs | 2 + 3 files changed, 53 insertions(+), 28 deletions(-) diff --git a/cranelift/jit/src/backend.rs b/cranelift/jit/src/backend.rs index 480c2c2b6c0b..adb21a2ab80b 100644 --- a/cranelift/jit/src/backend.rs +++ b/cranelift/jit/src/backend.rs @@ -27,7 +27,7 @@ const READONLY_DATA_ALIGNMENT: u64 = 0x1; /// A builder for `JITModule`. pub struct JITBuilder { isa: OwnedTargetIsa, - symbols: HashMap, + symbols: HashMap>, lookup_symbols: Vec Option<*const u8> + Send>>, libcall_names: Box String + Send + Sync>, hotswap_enabled: bool, @@ -116,7 +116,7 @@ impl JITBuilder { where K: Into, { - self.symbols.insert(name.into(), ptr); + self.symbols.insert(name.into(), SendWrapper(ptr)); self } @@ -129,7 +129,7 @@ impl JITBuilder { K: Into, { for (name, ptr) in symbols { - self.symbols.insert(name.into(), ptr); + self.symbols.insert(name.into(), SendWrapper(ptr)); } self } @@ -165,6 +165,20 @@ struct GotUpdate { ptr: *const u8, } +unsafe impl Send for GotUpdate {} + +/// A wrapper that impls Send for the contents. +/// +/// SAFETY: This must not be used for any types where it would be UB for them to be Send +struct SendWrapper(T); +unsafe impl Send for SendWrapper {} +impl Copy for SendWrapper {} +impl Clone for SendWrapper { + fn clone(&self) -> Self { + Self(self.0.clone()) + } +} + /// A `JITModule` implements `Module` and emits code and data into memory where it can be /// directly called and accessed. /// @@ -172,16 +186,16 @@ struct GotUpdate { pub struct JITModule { isa: OwnedTargetIsa, hotswap_enabled: bool, - symbols: RefCell>, + symbols: RefCell>>, lookup_symbols: Vec Option<*const u8> + Send>>, libcall_names: Box String + Send + Sync>, memory: MemoryHandle, declarations: ModuleDeclarations, - function_got_entries: SecondaryMap>>>, - function_plt_entries: SecondaryMap>>, - data_object_got_entries: SecondaryMap>>>, - libcall_got_entries: HashMap>>, - libcall_plt_entries: HashMap>, + function_got_entries: SecondaryMap>>>>, + function_plt_entries: SecondaryMap>>>, + data_object_got_entries: SecondaryMap>>>>, + libcall_got_entries: HashMap>>>, + libcall_plt_entries: HashMap>>, compiled_functions: SecondaryMap>, compiled_data_objects: SecondaryMap>, functions_to_finalize: Vec, @@ -191,8 +205,6 @@ pub struct JITModule { pending_got_updates: Vec, } -unsafe impl Send for JITModule {} - /// A handle to allow freeing memory allocated by the `Module`. struct MemoryHandle { code: Memory, @@ -217,7 +229,7 @@ impl JITModule { fn lookup_symbol(&self, name: &str) -> Option<*const u8> { match self.symbols.borrow_mut().entry(name.to_owned()) { - std::collections::hash_map::Entry::Occupied(occ) => Some(*occ.get()), + std::collections::hash_map::Entry::Occupied(occ) => Some(occ.get().0), std::collections::hash_map::Entry::Vacant(vac) => { let ptr = self .lookup_symbols @@ -225,7 +237,7 @@ impl JITModule { .rev() // Try last lookup function first .find_map(|lookup| lookup(name)); if let Some(ptr) = ptr { - vac.insert(ptr); + vac.insert(SendWrapper(ptr)); } ptr } @@ -268,7 +280,7 @@ impl JITModule { fn new_func_plt_entry(&mut self, id: FuncId, val: *const u8) { let got_entry = self.new_got_entry(val); - self.function_got_entries[id] = Some(got_entry); + self.function_got_entries[id] = Some(SendWrapper(got_entry)); let plt_entry = self.new_plt_entry(got_entry); self.record_function_for_perf( plt_entry.as_ptr().cast(), @@ -278,12 +290,12 @@ impl JITModule { self.declarations.get_function_decl(id).linkage_name(id) ), ); - self.function_plt_entries[id] = Some(plt_entry); + self.function_plt_entries[id] = Some(SendWrapper(plt_entry)); } fn new_data_got_entry(&mut self, id: DataId, val: *const u8) { let got_entry = self.new_got_entry(val); - self.data_object_got_entries[id] = Some(got_entry); + self.data_object_got_entries[id] = Some(SendWrapper(got_entry)); } unsafe fn write_plt_entry_bytes(plt_ptr: *mut [u8; 16], got_ptr: NonNull>) { @@ -352,7 +364,7 @@ impl JITModule { /// Panics if there's no entry in the table for the given function. pub fn read_got_entry(&self, func_id: FuncId) -> *const u8 { let got_entry = self.function_got_entries[func_id].unwrap(); - unsafe { got_entry.as_ref() }.load(Ordering::SeqCst) + unsafe { got_entry.0.as_ref() }.load(Ordering::SeqCst) } fn get_got_address(&self, name: &ModuleRelocTarget) -> NonNull> { @@ -360,16 +372,18 @@ impl JITModule { ModuleRelocTarget::User { .. } => { if ModuleDeclarations::is_function(name) { let func_id = FuncId::from_name(name); - self.function_got_entries[func_id].unwrap() + self.function_got_entries[func_id].unwrap().0 } else { let data_id = DataId::from_name(name); - self.data_object_got_entries[data_id].unwrap() + self.data_object_got_entries[data_id].unwrap().0 } } - ModuleRelocTarget::LibCall(ref libcall) => *self - .libcall_got_entries - .get(libcall) - .unwrap_or_else(|| panic!("can't resolve libcall {}", libcall)), + ModuleRelocTarget::LibCall(ref libcall) => { + self.libcall_got_entries + .get(libcall) + .unwrap_or_else(|| panic!("can't resolve libcall {}", libcall)) + .0 + } _ => panic!("invalid name"), } } @@ -381,6 +395,7 @@ impl JITModule { let func_id = FuncId::from_name(name); self.function_plt_entries[func_id] .unwrap() + .0 .as_ptr() .cast::() } else { @@ -391,6 +406,7 @@ impl JITModule { .libcall_plt_entries .get(libcall) .unwrap_or_else(|| panic!("can't resolve libcall {}", libcall)) + .0 .as_ptr() .cast::(), _ => panic!("invalid name"), @@ -545,9 +561,13 @@ impl JITModule { continue; }; let got_entry = module.new_got_entry(addr); - module.libcall_got_entries.insert(libcall, got_entry); + module + .libcall_got_entries + .insert(libcall, SendWrapper(got_entry)); let plt_entry = module.new_plt_entry(got_entry); - module.libcall_plt_entries.insert(libcall, plt_entry); + module + .libcall_plt_entries + .insert(libcall, SendWrapper(plt_entry)); } module @@ -721,7 +741,7 @@ impl Module for JITModule { if self.isa.flags().is_pic() { self.pending_got_updates.push(GotUpdate { - entry: self.function_got_entries[id].unwrap(), + entry: self.function_got_entries[id].unwrap().0, ptr, }) } @@ -739,6 +759,7 @@ impl Module for JITModule { .libcall_plt_entries .get(libcall) .unwrap_or_else(|| panic!("can't resolve libcall {}", libcall)) + .0 .as_ptr() .cast::(), _ => panic!("invalid name"), @@ -804,7 +825,7 @@ impl Module for JITModule { if self.isa.flags().is_pic() { self.pending_got_updates.push(GotUpdate { - entry: self.function_got_entries[id].unwrap(), + entry: self.function_got_entries[id].unwrap().0, ptr, }) } @@ -909,7 +930,7 @@ impl Module for JITModule { self.data_objects_to_finalize.push(id); if self.isa.flags().is_pic() { self.pending_got_updates.push(GotUpdate { - entry: self.data_object_got_entries[id].unwrap(), + entry: self.data_object_got_entries[id].unwrap().0, ptr, }) } diff --git a/cranelift/jit/src/compiled_blob.rs b/cranelift/jit/src/compiled_blob.rs index 84e10460e182..4eab207ff011 100644 --- a/cranelift/jit/src/compiled_blob.rs +++ b/cranelift/jit/src/compiled_blob.rs @@ -17,6 +17,8 @@ pub(crate) struct CompiledBlob { pub(crate) relocs: Vec, } +unsafe impl Send for CompiledBlob {} + impl CompiledBlob { pub(crate) fn perform_relocations( &self, diff --git a/cranelift/jit/src/memory.rs b/cranelift/jit/src/memory.rs index a4ceca3ac403..abbb513c663f 100644 --- a/cranelift/jit/src/memory.rs +++ b/cranelift/jit/src/memory.rs @@ -132,6 +132,8 @@ pub(crate) struct Memory { branch_protection: BranchProtection, } +unsafe impl Send for Memory {} + impl Memory { pub(crate) fn new(branch_protection: BranchProtection) -> Self { Self { From 42d6f3ac37e6595f9d74c07a56ffc9566cfd6487 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Fri, 31 May 2024 08:24:08 -0700 Subject: [PATCH 3/3] Derive Copy,Clone for SendWrapper --- cranelift/jit/src/backend.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/cranelift/jit/src/backend.rs b/cranelift/jit/src/backend.rs index adb21a2ab80b..903fecd6c406 100644 --- a/cranelift/jit/src/backend.rs +++ b/cranelift/jit/src/backend.rs @@ -170,14 +170,9 @@ unsafe impl Send for GotUpdate {} /// A wrapper that impls Send for the contents. /// /// SAFETY: This must not be used for any types where it would be UB for them to be Send +#[derive(Copy, Clone)] struct SendWrapper(T); unsafe impl Send for SendWrapper {} -impl Copy for SendWrapper {} -impl Clone for SendWrapper { - fn clone(&self) -> Self { - Self(self.0.clone()) - } -} /// A `JITModule` implements `Module` and emits code and data into memory where it can be /// directly called and accessed.