From 775b14c53797392f89db9f7f2530c38da113d9e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Sun, 11 Aug 2024 19:18:35 +0200 Subject: [PATCH 1/3] add detail to comment --- src/arena.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/arena.rs b/src/arena.rs index 6acb279b..3d1293a6 100644 --- a/src/arena.rs +++ b/src/arena.rs @@ -371,7 +371,8 @@ impl> AllocateInArena for P { } } -/* apparently this overlaps the planket impl above somehow +/* this isn't allowed due to https://github.com/rust-lang/rust/issues/20400 I think, + though P == ManuallyDrop

might also be a problem event though that shouldn't be possible impl>> AllocateInArena for P { fn arena_allocate(self, arena: &mut Arena) -> TypedArenaPtr { T::alloc(arena, ManuallyDrop::new(self)) From cc51b8150df625c95289bba51cd4f97a1c05fb9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Sun, 11 Aug 2024 19:19:00 +0200 Subject: [PATCH 2/3] check whether (re-)allocation succeeded fixes mthom/scryer-prolog#2449 --- src/ffi.rs | 90 ++++++++++++++++++++++++-------------------- src/machine/stack.rs | 4 +- src/raw_block.rs | 24 ++++++++---- 3 files changed, 70 insertions(+), 48 deletions(-) diff --git a/src/ffi.rs b/src/ffi.rs index 04e85ab3..a3e93380 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -21,7 +21,7 @@ and finally we add the pointer the size of what we've written. use crate::atom_table::Atom; -use std::alloc::{alloc, Layout}; +use std::alloc::{self, Layout}; use std::any::Any; use std::collections::HashMap; use std::convert::TryFrom; @@ -223,42 +223,47 @@ impl ForeignFunctionTable { arg: &mut Value, structs_table: &mut HashMap, ) -> Result<(Box, usize, usize), FFIError> { - unsafe { - match arg { - Value::Struct(ref name, ref mut struct_args) => { - if let Some(ref mut struct_type) = structs_table.clone().get_mut(name) { - let layout = Layout::from_size_align( - struct_type.ffi_type.size, - struct_type.ffi_type.alignment.into(), - ) - .unwrap(); - let align = struct_type.ffi_type.alignment as usize; - let size = struct_type.ffi_type.size; - let ptr = alloc(layout) as *mut c_void; - let mut field_ptr = ptr; + match arg { + Value::Struct(ref name, ref mut struct_args) => { + if let Some(ref mut struct_type) = structs_table.clone().get_mut(name) { + let layout = Layout::from_size_align( + struct_type.ffi_type.size, + struct_type.ffi_type.alignment.into(), + ) + .unwrap(); + let align = struct_type.ffi_type.alignment as usize; + let size = struct_type.ffi_type.size; + let ptr = unsafe { alloc::alloc(layout) as *mut c_void }; - #[allow(clippy::needless_range_loop)] - for i in 0..(struct_type.fields.len() - 1) { - macro_rules! try_write_int { - ($type:ty) => {{ - field_ptr = field_ptr - .add(field_ptr.align_offset(std::mem::align_of::<$type>())); - let n: $type = <$type>::try_from(struct_args[i].as_int()?) - .map_err(|_| FFIError::ValueDontFit)?; - std::ptr::write(field_ptr as *mut $type, n); - field_ptr = field_ptr.add(std::mem::size_of::<$type>()); - }}; - } + if ptr.is_null() { + panic!("allocation failed") + } - macro_rules! write { - ($type:ty, $value:expr) => {{ - let data: $type = $value; - std::ptr::write(field_ptr as *mut $type, data); - field_ptr = field_ptr.add(align); - }}; - } + let mut field_ptr = ptr; - let field = struct_type.fields[i]; + #[allow(clippy::needless_range_loop)] + for i in 0..(struct_type.fields.len() - 1) { + macro_rules! try_write_int { + ($type:ty) => {{ + field_ptr = field_ptr + .add(field_ptr.align_offset(std::mem::align_of::<$type>())); + let n: $type = <$type>::try_from(struct_args[i].as_int()?) + .map_err(|_| FFIError::ValueDontFit)?; + std::ptr::write(field_ptr as *mut $type, n); + field_ptr = field_ptr.add(std::mem::size_of::<$type>()); + }}; + } + + macro_rules! write { + ($type:ty, $value:expr) => {{ + let data: $type = $value; + std::ptr::write(field_ptr as *mut $type, data); + field_ptr = field_ptr.add(align); + }}; + } + + let field = struct_type.fields[i]; + unsafe { match (*field).type_ as u32 { libffi::raw::FFI_TYPE_UINT8 => try_write_int!(u8), libffi::raw::FFI_TYPE_SINT8 => try_write_int!(i8), @@ -294,14 +299,15 @@ impl ForeignFunctionTable { } } } - #[allow(clippy::from_raw_with_void_ptr)] - Ok((Box::from_raw(ptr), size, align)) - } else { - Err(FFIError::InvalidStructName) } + + #[allow(clippy::from_raw_with_void_ptr)] + Ok((unsafe { Box::from_raw(ptr) }, size, align)) + } else { + Err(FFIError::InvalidStructName) } - _ => Err(FFIError::ValueCast), } + _ => Err(FFIError::ValueCast), } } @@ -377,7 +383,11 @@ impl ForeignFunctionTable { struct_type.ffi_type.alignment.into(), ) .unwrap(); - let ptr = alloc(layout) as *mut c_void; + let ptr = alloc::alloc(layout) as *mut c_void; + + if ptr.is_null() { + panic!("allocation failed") + } libffi::raw::ffi_call( &mut function_impl.cif, diff --git a/src/machine/stack.rs b/src/machine/stack.rs index 7f35cdbc..210a7cac 100644 --- a/src/machine/stack.rs +++ b/src/machine/stack.rs @@ -174,7 +174,9 @@ impl Stack { let ptr = self.buf.alloc(frame_size); if ptr.is_null() { - self.buf.grow(); + if !self.buf.grow() { + panic!("growing the stack failed") + } } else { return ptr; } diff --git a/src/raw_block.rs b/src/raw_block.rs index 0c27bdde..8282a94d 100644 --- a/src/raw_block.rs +++ b/src/raw_block.rs @@ -41,22 +41,32 @@ impl RawBlock { unsafe fn init_at_size(&mut self, cap: usize) { let layout = alloc::Layout::from_size_align_unchecked(cap, T::align()); - - self.base = alloc::alloc(layout) as *const _; + let new_base = alloc::alloc(layout).cast_const(); + if new_base.is_null() { + panic!("failed to allocate for init_at_size"); + } + self.base = new_base; self.top = self.base.add(cap); - *self.ptr.get_mut() = self.base as *mut _; + *self.ptr.get_mut() = self.base.cast_mut(); } - pub unsafe fn grow(&mut self) { + pub unsafe fn grow(&mut self) -> bool { if self.base.is_null() { self.init_at_size(T::init_size()); + true } else { let size = self.size(); let layout = alloc::Layout::from_size_align_unchecked(size, T::align()); - self.base = alloc::realloc(self.base as *mut _, layout, size * 2) as *const _; - self.top = (self.base as usize + size * 2) as *const _; - *self.ptr.get_mut() = (self.base as usize + size) as *mut _; + let new_base = alloc::realloc(self.base.cast_mut(), layout, size * 2).cast_const(); + if new_base.is_null() { + false + } else { + self.base = new_base; + self.top = (self.base as usize + size * 2) as *const _; + *self.ptr.get_mut() = (self.base as usize + size) as *mut _; + true + } } } From 620bf385d171bf4f0c1150fad692b3e0b03b31a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Sun, 11 Aug 2024 21:53:02 +0200 Subject: [PATCH 3/3] include the type name for better pin-pointing of the error source - only done here as all other places are non-generic and the panic already includes file and line info, only here are we in a generic context and might need more differentiation --- src/raw_block.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/raw_block.rs b/src/raw_block.rs index 8282a94d..da757415 100644 --- a/src/raw_block.rs +++ b/src/raw_block.rs @@ -43,7 +43,10 @@ impl RawBlock { let layout = alloc::Layout::from_size_align_unchecked(cap, T::align()); let new_base = alloc::alloc(layout).cast_const(); if new_base.is_null() { - panic!("failed to allocate for init_at_size"); + panic!( + "failed to allocate in init_at_size for {}", + std::any::type_name::() + ); } self.base = new_base; self.top = self.base.add(cap);