diff --git a/cranelift/filetests/src/function_runner.rs b/cranelift/filetests/src/function_runner.rs index 51b9dbab0d..39f6079050 100644 --- a/cranelift/filetests/src/function_runner.rs +++ b/cranelift/filetests/src/function_runner.rs @@ -244,7 +244,7 @@ impl TestFileCompiler { // Finalize the functions which we just defined, which resolves any // outstanding relocations (patching in addresses, now that they're // available). - self.module.finalize_definitions(); + self.module.finalize_definitions()?; Ok(CompiledTestFile { module: Some(self.module), diff --git a/cranelift/jit/examples/jit-minimal.rs b/cranelift/jit/examples/jit-minimal.rs index f5e70e9b62..29d210825e 100644 --- a/cranelift/jit/examples/jit-minimal.rs +++ b/cranelift/jit/examples/jit-minimal.rs @@ -78,7 +78,7 @@ fn main() { module.clear_context(&mut ctx); // Perform linking. - module.finalize_definitions(); + module.finalize_definitions().unwrap(); // Get a raw pointer to the generated code. let code_b = module.get_finalized_function(func_b); diff --git a/cranelift/jit/src/backend.rs b/cranelift/jit/src/backend.rs index 441a878d4f..202dfe516a 100644 --- a/cranelift/jit/src/backend.rs +++ b/cranelift/jit/src/backend.rs @@ -427,7 +427,9 @@ impl JITModule { /// /// Use `get_finalized_function` and `get_finalized_data` to obtain the final /// artifacts. - pub fn finalize_definitions(&mut self) { + /// + /// Returns ModuleError in case of allocation or syscall failure + pub fn finalize_definitions(&mut self) -> ModuleResult<()> { for func in std::mem::take(&mut self.functions_to_finalize) { let decl = self.declarations.get_function_decl(func); assert!(decl.linkage.is_definable()); @@ -455,12 +457,13 @@ impl JITModule { } // Now that we're done patching, prepare the memory for execution! - self.memory.readonly.set_readonly(); - self.memory.code.set_readable_and_executable(); + self.memory.readonly.set_readonly()?; + self.memory.code.set_readable_and_executable()?; for update in self.pending_got_updates.drain(..) { unsafe { update.entry.as_ref() }.store(update.ptr as *mut _, Ordering::SeqCst); } + Ok(()) } /// Create a new `JITModule`. @@ -688,7 +691,10 @@ impl Module for JITModule { .memory .code .allocate(size, align) - .expect("TODO: handle OOM etc."); + .map_err(|e| ModuleError::Allocation { + message: "unable to alloc function", + err: e, + })?; { let mem = unsafe { std::slice::from_raw_parts_mut(ptr, size) }; @@ -770,7 +776,10 @@ impl Module for JITModule { .memory .code .allocate(size, align) - .expect("TODO: handle OOM etc."); + .map_err(|e| ModuleError::Allocation { + message: "unable to alloc function bytes", + err: e, + })?; unsafe { ptr::copy_nonoverlapping(bytes.as_ptr(), ptr, size); @@ -836,12 +845,18 @@ impl Module for JITModule { self.memory .writable .allocate(size, align.unwrap_or(WRITABLE_DATA_ALIGNMENT)) - .expect("TODO: handle OOM etc.") + .map_err(|e| ModuleError::Allocation { + message: "unable to alloc writable data", + err: e, + })? } else { self.memory .readonly .allocate(size, align.unwrap_or(READONLY_DATA_ALIGNMENT)) - .expect("TODO: handle OOM etc.") + .map_err(|e| ModuleError::Allocation { + message: "unable to alloc readonly data", + err: e, + })? }; match *init { diff --git a/cranelift/jit/src/memory.rs b/cranelift/jit/src/memory.rs index 9187131f88..08518b3c8e 100644 --- a/cranelift/jit/src/memory.rs +++ b/cranelift/jit/src/memory.rs @@ -1,3 +1,5 @@ +use cranelift_module::{ModuleError, ModuleResult}; + #[cfg(feature = "selinux-fix")] use memmap2::MmapMut; @@ -56,10 +58,14 @@ impl PtrLen { // Safety: We assert that the size is non-zero above. let ptr = unsafe { alloc::alloc(layout) }; - Ok(Self { - ptr, - len: alloc_size, - }) + if !ptr.is_null() { + Ok(Self { + ptr, + len: alloc_size, + }) + } else { + Err(io::Error::from(io::ErrorKind::OutOfMemory)) + } } #[cfg(target_os = "windows")] @@ -168,7 +174,7 @@ impl Memory { } /// Set all memory allocated in this `Memory` up to now as readable and executable. - pub(crate) fn set_readable_and_executable(&mut self) { + pub(crate) fn set_readable_and_executable(&mut self) -> ModuleResult<()> { self.finish_current(); // Clear all the newly allocated code from cache if the processor requires it @@ -182,7 +188,7 @@ impl Memory { }; } - let set_region_readable_and_executable = |ptr, len| { + let set_region_readable_and_executable = |ptr, len| -> ModuleResult<()> { if self.branch_protection == BranchProtection::BTI { #[cfg(all(target_arch = "aarch64", target_os = "linux"))] if std::arch::is_aarch64_feature_detected!("bti") { @@ -190,42 +196,54 @@ impl Memory { unsafe { if libc::mprotect(ptr as *mut libc::c_void, len, prot) < 0 { - panic!("unable to make memory readable+executable"); + return Err(ModuleError::Backend( + anyhow::Error::new(io::Error::last_os_error()) + .context("unable to make memory readable+executable"), + )); } } - return; + return Ok(()); } } unsafe { - region::protect(ptr, len, region::Protection::READ_EXECUTE) - .expect("unable to make memory readable+executable"); + region::protect(ptr, len, region::Protection::READ_EXECUTE).map_err(|e| { + ModuleError::Backend( + anyhow::Error::new(e).context("unable to make memory readable+executable"), + ) + })?; } + Ok(()) }; for &PtrLen { ptr, len, .. } in self.non_protected_allocations_iter() { - set_region_readable_and_executable(ptr, len); + set_region_readable_and_executable(ptr, len)?; } // Flush any in-flight instructions from the pipeline icache_coherence::pipeline_flush_mt().expect("Failed pipeline flush"); self.already_protected = self.allocations.len(); + Ok(()) } /// Set all memory allocated in this `Memory` up to now as readonly. - pub(crate) fn set_readonly(&mut self) { + pub(crate) fn set_readonly(&mut self) -> ModuleResult<()> { self.finish_current(); for &PtrLen { ptr, len, .. } in self.non_protected_allocations_iter() { unsafe { - region::protect(ptr, len, region::Protection::READ) - .expect("unable to make memory readonly"); + region::protect(ptr, len, region::Protection::READ).map_err(|e| { + ModuleError::Backend( + anyhow::Error::new(e).context("unable to make memory readonly"), + ) + })?; } } self.already_protected = self.allocations.len(); + Ok(()) } /// Iterates non protected memory allocations that are of not zero bytes in size. diff --git a/cranelift/jit/tests/basic.rs b/cranelift/jit/tests/basic.rs index 64957aa1f7..357d5ef533 100644 --- a/cranelift/jit/tests/basic.rs +++ b/cranelift/jit/tests/basic.rs @@ -209,5 +209,5 @@ fn libcall_function() { module.define_function(func_id, &mut ctx).unwrap(); - module.finalize_definitions(); + module.finalize_definitions().unwrap(); } diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index 6f79e1f0b0..2df3b4f5fe 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -229,6 +229,14 @@ pub enum ModuleError { /// Wraps a `cranelift-codegen` error Compilation(CodegenError), + /// Memory allocation failure from a backend + Allocation { + /// Tell where the allocation came from + message: &'static str, + /// Io error the allocation failed with + err: std::io::Error, + }, + /// Wraps a generic error from a backend Backend(anyhow::Error), } @@ -250,6 +258,7 @@ impl std::error::Error for ModuleError { | Self::DuplicateDefinition { .. } | Self::InvalidImportDefinition { .. } => None, Self::Compilation(source) => Some(source), + Self::Allocation { err: source, .. } => Some(source), Self::Backend(source) => Some(&**source), } } @@ -284,6 +293,9 @@ impl std::fmt::Display for ModuleError { Self::Compilation(err) => { write!(f, "Compilation error: {}", err) } + Self::Allocation { message, err } => { + write!(f, "Allocation error: {}: {}", message, err) + } Self::Backend(err) => write!(f, "Backend error: {}", err), } }