Merge pull request #2474 from Skgland/fix-looping-memory-allocation

Fix looping memory allocation
mthom · Aug 18, 2024 · 264e3e9 · 264e3e9
2 parents 3f25ee3 + 620bf38
commit 264e3e9
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 50 deletions.
diff --git a/src/arena.rs b/src/arena.rs
@@ -371,7 +371,8 @@ impl<P, T: ArenaAllocated<Payload = P>> AllocateInArena<T> 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<P> might also be a problem event though that shouldn't be possible
 impl<P, T: ArenaAllocated<Payload = ManuallyDrop<P>>> AllocateInArena<T> for P {
     fn arena_allocate(self, arena: &mut Arena) -> TypedArenaPtr<T> {
         T::alloc(arena, ManuallyDrop::new(self))

diff --git 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<String, StructImpl>,
     ) -> Result<(Box<dyn Any>, 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;
-
-                        #[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>());
-                                }};
-                            }
+        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 };
 
-                            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);
-                                }};
-                            }
+                    if ptr.is_null() {
+                        panic!("allocation failed")
+                    }
 
-                            let field = struct_type.fields[i];
+                    let mut field_ptr = ptr;
+
+                    #[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
@@ -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
@@ -41,22 +41,35 @@ impl<T: RawBlockTraits> RawBlock<T> {
 
     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 in init_at_size for {}",
+                std::any::type_name::<Self>()
+            );
+        }
+        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
+            }
         }
     }