rework some unsafe parts

- removed some unsafe
- added some safety comments
- add explicit types to transmute calls
- reworked UntypedArenaPtr -> TypedArenaPtr conversion

might help with mthom/scryer-prolog#2438, I noticed fewer complains from miri after changing the default impl for `ArenaAllocated::alloc`

src/arena.rs

@@ -7,12 +7,14 @@ use crate::raw_block::*;
 use crate::rcu::Rcu;
 use crate::rcu::RcuRef;
 use crate::read::*;
+use crate::types::UntypedArenaPtr;
 
 use crate::parser::dashu::{Integer, Rational};
 use ordered_float::OrderedFloat;
 
 use std::cell::UnsafeCell;
 use std::fmt;
+use std::fmt::Debug;
 use std::hash::{Hash, Hasher};
 use std::mem;
 use std::net::TcpListener;
@@ -45,20 +47,6 @@ pub fn header_offset_from_payload<Payload: Sized>() -> usize {
     payload_offset - header_offset
 }
 
-pub fn ptr_to_allocated<Payload: ArenaAllocated>(slab: &mut AllocSlab) -> TypedArenaPtr<Payload> {
-    let typed_slab: &mut TypedAllocSlab<Payload> = unsafe { mem::transmute(slab) };
-    typed_slab.to_typed_arena_ptr()
-}
-
-#[macro_export]
-macro_rules! gen_ptr_to_allocated {
-    ($payload: ty) => {
-        fn ptr_to_allocated(slab: &mut AllocSlab) -> TypedArenaPtr<$payload> {
-            ptr_to_allocated::<$payload>(slab)
-        }
-    };
-}
-
 use std::sync::Arc;
 use std::sync::Mutex;
 use std::sync::Weak;
@@ -294,10 +282,11 @@ impl<T: fmt::Display> fmt::Display for TypedArenaPtr<T> {
 }
 
 impl<T: ?Sized + ArenaAllocated> TypedArenaPtr<T> {
-    // data must be allocated in the arena already.
+    /// # Safety
-    #[allow(clippy::not_unsafe_ptr_arg_deref)]
+    /// - the pointers referee type is correct, safe code depends on the correctness of the type argument
+    /// - the pointer is allocated in the arena
     #[inline]
-    pub const fn new(data: *mut T) -> Self {
+    pub const unsafe fn new(data: *mut T) -> Self {
         unsafe { TypedArenaPtr(ptr::NonNull::new_unchecked(data)) }
     }
 
@@ -349,30 +338,38 @@ impl<T: ?Sized + ArenaAllocated> TypedArenaPtr<T> {
 }
 
 pub trait ArenaAllocated: Sized {
-    type PtrToAllocated;
-
     fn tag() -> ArenaHeaderTag;
-    fn ptr_to_allocated(slab: &mut AllocSlab) -> Self::PtrToAllocated;
 
     fn header_offset_from_payload() -> usize {
         header_offset_from_payload::<Self>()
     }
 
+    /// #  Safety
+    /// - the caller must guarantee that the pointee type of UntypedArenaPtr is Self
+    unsafe fn typed_ptr(ptr: UntypedArenaPtr) -> TypedArenaPtr<Self> {
+        // safety:
+        // - allocated in an arena as from an UntypedArenaPtr
+        // - caller guarantees the type is correct
+        unsafe { TypedArenaPtr::new(ptr.payload_offset().cast_mut().cast::<Self>()) }
+    }
+
     #[allow(clippy::missing_safety_doc)]
-    fn alloc(arena: &mut Arena, value: Self) -> Self::PtrToAllocated {
+    fn alloc(arena: &mut Arena, value: Self) -> TypedArenaPtr<Self> {
         let size = mem::size_of::<TypedAllocSlab<Self>>();
-        let slab = Box::new(TypedAllocSlab {
+        let mut slab = Box::new(TypedAllocSlab {
             slab: AllocSlab {
                 next: arena.base.take(),
                 #[cfg(target_pointer_width = "32")]
                 _padding: 0,
-                header: ArenaHeader::build_with(size as u64, Self::tag()),
+                header: HeaderOrIdxPtr {
+                    header: ArenaHeader::build_with(size as u64, Self::tag()),
+                },
             },
             payload: value,
         });
 
-        let mut untyped_slab = unsafe { Box::from_raw(Box::into_raw(slab) as *mut AllocSlab) };
+        let allocated_ptr = slab.to_typed_arena_ptr();
-        let allocated_ptr = Self::ptr_to_allocated(untyped_slab.as_mut());
+        let untyped_slab = unsafe { Box::from_raw(Box::into_raw(slab) as *mut AllocSlab) };
 
         arena.base = Some(untyped_slab);
 
@@ -512,10 +509,6 @@ impl fmt::Display for F64Offset {
 }
 
 impl ArenaAllocated for Integer {
-    type PtrToAllocated = TypedArenaPtr<Integer>;
-
-    gen_ptr_to_allocated!(Integer);
-
     #[inline]
     fn tag() -> ArenaHeaderTag {
         ArenaHeaderTag::Integer
@@ -523,10 +516,6 @@ impl ArenaAllocated for Integer {
 }
 
 impl ArenaAllocated for Rational {
-    type PtrToAllocated = TypedArenaPtr<Rational>;
-
-    gen_ptr_to_allocated!(Rational);
-
     #[inline]
     fn tag() -> ArenaHeaderTag {
         ArenaHeaderTag::Rational
@@ -534,10 +523,6 @@ impl ArenaAllocated for Rational {
 }
 
 impl ArenaAllocated for LiveLoadState {
-    type PtrToAllocated = TypedArenaPtr<LiveLoadState>;
-
-    gen_ptr_to_allocated!(LiveLoadState);
-
     #[inline]
     fn tag() -> ArenaHeaderTag {
         ArenaHeaderTag::LiveLoadState
@@ -545,10 +530,6 @@ impl ArenaAllocated for LiveLoadState {
 }
 
 impl ArenaAllocated for TcpListener {
-    type PtrToAllocated = TypedArenaPtr<TcpListener>;
-
-    gen_ptr_to_allocated!(TcpListener);
-
     #[inline]
     fn tag() -> ArenaHeaderTag {
         ArenaHeaderTag::TcpListener
@@ -557,10 +538,6 @@ impl ArenaAllocated for TcpListener {
 
 #[cfg(feature = "http")]
 impl ArenaAllocated for HttpListener {
-    type PtrToAllocated = TypedArenaPtr<HttpListener>;
-
-    gen_ptr_to_allocated!(HttpListener);
-
     #[inline]
     fn tag() -> ArenaHeaderTag {
         ArenaHeaderTag::HttpListener
@@ -569,10 +546,6 @@ impl ArenaAllocated for HttpListener {
 
 #[cfg(feature = "http")]
 impl ArenaAllocated for HttpResponse {
-    type PtrToAllocated = TypedArenaPtr<HttpResponse>;
-
-    gen_ptr_to_allocated!(HttpResponse);
-
     #[inline]
     fn tag() -> ArenaHeaderTag {
         ArenaHeaderTag::HttpResponse
@@ -580,46 +553,71 @@ impl ArenaAllocated for HttpResponse {
 }
 
 impl ArenaAllocated for IndexPtr {
-    type PtrToAllocated = TypedArenaPtr<IndexPtr>;
-
     #[inline]
     fn tag() -> ArenaHeaderTag {
         ArenaHeaderTag::IndexPtrUndefined
     }
 
-    #[inline]
-    fn ptr_to_allocated(slab: &mut AllocSlab) -> Self::PtrToAllocated {
-        TypedArenaPtr::new(ptr::addr_of_mut!(slab.header) as *mut _)
-    }
-
     #[inline]
     fn header_offset_from_payload() -> usize {
         0
     }
 
+    /// #  Safety
+    /// - the caller must guarantee that the pointee type of UntypedArenaPtr is T
+    unsafe fn typed_ptr(ptr: UntypedArenaPtr) -> TypedArenaPtr<Self> {
+        unsafe { TypedArenaPtr::new(std::mem::transmute::<_, *mut IndexPtr>(ptr.get_ptr())) }
+    }
+
     #[inline]
-    fn alloc(arena: &mut Arena, value: Self) -> Self::PtrToAllocated {
+    fn alloc(arena: &mut Arena, value: Self) -> TypedArenaPtr<Self> {
         let mut slab = Box::new(AllocSlab {
             next: arena.base.take(),
             #[cfg(target_pointer_width = "32")]
             _padding: 0,
-            header: unsafe { mem::transmute(value) },
+            header: HeaderOrIdxPtr { idx_ptr: value },
         });
 
-        let allocated_ptr =
+        let allocated_ptr = unsafe { TypedArenaPtr::new(ptr::addr_of_mut!(slab.header.idx_ptr)) };
-            TypedArenaPtr::new(unsafe { mem::transmute(ptr::addr_of_mut!(slab.header)) });
         arena.base = Some(slab);
         allocated_ptr
     }
 }
 
+#[repr(C)]
+union HeaderOrIdxPtr {
+    header: ArenaHeader,
+    idx_ptr: IndexPtr,
+}
+
+const _: () = {
+    if std::mem::size_of::<ArenaHeader>() != std::mem::size_of::<IndexPtr>() {
+        panic!("Size of ArenaHeader != IndexPtr")
+    }
+};
+
+impl Debug for HeaderOrIdxPtr {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        unsafe { &self.header }.fmt(f)
+    }
+}
+
+impl Clone for HeaderOrIdxPtr {
+    fn clone(&self) -> Self {
+        // safety:
+        // - we created the pointer from a valid reference
+        // - both ArenaHeader and IndexPtr are plain old datatypes, i.e. no managed resources that need to be cloned
+        unsafe { std::ptr::read(self) }
+    }
+}
+
 #[repr(C)]
 #[derive(Clone, Debug)]
 pub struct AllocSlab {
     next: Option<Box<AllocSlab>>,
     #[cfg(target_pointer_width = "32")]
     _padding: u32,
-    header: ArenaHeader,
+    header: HeaderOrIdxPtr,
 }
 
 #[repr(C)]
@@ -632,7 +630,9 @@ pub struct TypedAllocSlab<Payload> {
 impl<Payload: ArenaAllocated> TypedAllocSlab<Payload> {
     #[inline]
     pub fn to_typed_arena_ptr(&mut self) -> TypedArenaPtr<Payload> {
-        TypedArenaPtr::new(&mut self.payload as *mut _)
+        // safety:
+        // - this is the arena allocation of corresponding type
+        unsafe { TypedArenaPtr::new(&mut self.payload) }
     }
 }
 
@@ -664,7 +664,7 @@ unsafe fn drop_slab_in_place(value: &mut AllocSlab) {
         };
     }
 
-    match value.header.tag() {
+    match value.header.header.tag() {
         ArenaHeaderTag::Integer => {
             drop_typed_slab_in_place!(Integer, value);
         }

src/machine/machine_indices.rs

@@ -160,7 +160,7 @@ impl From<CodeIndex> for UntypedArenaPtr {
 impl From<UntypedArenaPtr> for CodeIndex {
     #[inline(always)]
     fn from(ptr: UntypedArenaPtr) -> CodeIndex {
-        CodeIndex(TypedArenaPtr::new(ptr.get_ptr() as *mut IndexPtr))
+        CodeIndex(unsafe { ptr.as_typed_ptr() })
     }
 }
 

src/machine/streams.rs

@@ -452,10 +452,6 @@ impl<T> DerefMut for StreamLayout<T> {
 macro_rules! arena_allocated_impl_for_stream {
     ($stream_type:ty, $stream_tag:ident) => {
         impl ArenaAllocated for StreamLayout<$stream_type> {
-            type PtrToAllocated = TypedArenaPtr<StreamLayout<$stream_type>>;
-
-            gen_ptr_to_allocated!(StreamLayout<$stream_type>);
-
             #[inline]
             fn tag() -> ArenaHeaderTag {
                 ArenaHeaderTag::$stream_tag
@@ -539,29 +535,27 @@ impl Stream {
         ))
     }
 
-    pub fn from_tag(tag: ArenaHeaderTag, ptr: *const u8) -> Self {
+    pub fn from_tag(tag: ArenaHeaderTag, ptr: UntypedArenaPtr) -> Self {
         match tag {
-            ArenaHeaderTag::ByteStream => Stream::Byte(TypedArenaPtr::new(ptr as *mut _)),
+            ArenaHeaderTag::ByteStream => Stream::Byte(unsafe { ptr.as_typed_ptr() }),
-            ArenaHeaderTag::InputFileStream => Stream::InputFile(TypedArenaPtr::new(ptr as *mut _)),
+            ArenaHeaderTag::InputFileStream => Stream::InputFile(unsafe { ptr.as_typed_ptr() }),
-            ArenaHeaderTag::OutputFileStream => {
+            ArenaHeaderTag::OutputFileStream => Stream::OutputFile(unsafe { ptr.as_typed_ptr() }),
-                Stream::OutputFile(TypedArenaPtr::new(ptr as *mut _))
+            ArenaHeaderTag::NamedTcpStream => Stream::NamedTcp(unsafe { ptr.as_typed_ptr() }),
-            }
-            ArenaHeaderTag::NamedTcpStream => Stream::NamedTcp(TypedArenaPtr::new(ptr as *mut _)),
             #[cfg(feature = "tls")]
-            ArenaHeaderTag::NamedTlsStream => Stream::NamedTls(TypedArenaPtr::new(ptr as *mut _)),
+            ArenaHeaderTag::NamedTlsStream => Stream::NamedTls(unsafe { ptr.as_typed_ptr() }),
             #[cfg(feature = "http")]
-            ArenaHeaderTag::HttpReadStream => Stream::HttpRead(TypedArenaPtr::new(ptr as *mut _)),
+            ArenaHeaderTag::HttpReadStream => Stream::HttpRead(unsafe { ptr.as_typed_ptr() }),
             #[cfg(feature = "http")]
-            ArenaHeaderTag::HttpWriteStream => Stream::HttpWrite(TypedArenaPtr::new(ptr as *mut _)),
+            ArenaHeaderTag::HttpWriteStream => Stream::HttpWrite(unsafe { ptr.as_typed_ptr() }),
-            ArenaHeaderTag::ReadlineStream => Stream::Readline(TypedArenaPtr::new(ptr as *mut _)),
+            ArenaHeaderTag::ReadlineStream => Stream::Readline(unsafe { ptr.as_typed_ptr() }),
             ArenaHeaderTag::StaticStringStream => {
-                Stream::StaticString(TypedArenaPtr::new(ptr as *mut _))
+                Stream::StaticString(unsafe { ptr.as_typed_ptr() })
             }
             ArenaHeaderTag::StandardOutputStream => {
-                Stream::StandardOutput(TypedArenaPtr::new(ptr as *mut _))
+                Stream::StandardOutput(unsafe { ptr.as_typed_ptr() })
             }
             ArenaHeaderTag::StandardErrorStream => {
-                Stream::StandardError(TypedArenaPtr::new(ptr as *mut _))
+                Stream::StandardError(unsafe { ptr.as_typed_ptr() })
             }
             ArenaHeaderTag::Dropped | ArenaHeaderTag::NullStream => {
                 Stream::Null(StreamOptions::default())

src/macros.rs

@@ -173,13 +173,15 @@ macro_rules! typed_arena_ptr_as_cell {
 macro_rules! raw_ptr_as_cell {
     ($ptr:expr) => {
         // Cell is 64-bit, but raw ptr is 32-bit in 32-bit systems
-        HeapCellValue::from_raw_ptr_bytes(unsafe { std::mem::transmute($ptr) })
+        // TODO use <*{const,mut} _>::addr instead of as when the strict_provenance feature is stable rust-lang/rust#95228
+        // we might need <*{const,mut} _>::expose_provenance for strict provenance, dependening on how we recreate a pointer later
+        HeapCellValue::from_raw_ptr_bytes(($ptr as usize).to_ne_bytes())
     };
 }
 
 macro_rules! untyped_arena_ptr_as_cell {
     ($ptr:expr) => {
-        HeapCellValue::from_bytes(unsafe { std::mem::transmute($ptr) })
+        HeapCellValue::from_bytes(UntypedArenaPtr::into_bytes($ptr))
     };
 }
 
@@ -224,86 +226,69 @@ macro_rules! stream_as_cell {
 macro_rules! cell_as_stream {
     ($cell:expr) => {{
         let ptr = cell_as_untyped_arena_ptr!($cell);
-        Stream::from_tag(ptr.get_tag(), ptr.payload_offset())
+        Stream::from_tag(ptr.get_tag(), ptr)
     }};
 }
 
 macro_rules! cell_as_load_state_payload {
-    ($cell:expr) => {
+    ($cell:expr) => {{
-        unsafe {
+        let ptr = cell_as_untyped_arena_ptr!($cell);
-            let ptr = cell_as_untyped_arena_ptr!($cell);
+        unsafe { ptr.as_typed_ptr::<LiveLoadState>() }
-            let ptr = std::mem::transmute::<_, *mut LiveLoadState>(ptr.payload_offset());
+    }};
-
-            TypedArenaPtr::new(ptr)
-        }
-    };
 }
 
 macro_rules! match_untyped_arena_ptr_pat_body {
     ($ptr:ident, Integer, $n:ident, $code:expr) => {{
-        let payload_ptr = unsafe { std::mem::transmute::<_, *mut Integer>($ptr.payload_offset()) };
+        let $n = unsafe { $ptr.as_typed_ptr::<Integer>() };
-        let $n = TypedArenaPtr::new(payload_ptr);
         #[allow(unused_braces)]
         $code
     }};
     ($ptr:ident, Rational, $n:ident, $code:expr) => {{
-        let payload_ptr = unsafe { std::mem::transmute::<_, *mut Rational>($ptr.payload_offset()) };
+        let $n = unsafe { $ptr.as_typed_ptr::<Rational>() };
-        let $n = TypedArenaPtr::new(payload_ptr);
         #[allow(unused_braces)]
         $code
     }};
     ($ptr:ident, OssifiedOpDir, $n:ident, $code:expr) => {{
-        let payload_ptr =
+        let $n = unsafe { $ptr.as_typed_ptr::<OssifiedOpDir>() };
-            unsafe { std::mem::transmute::<_, *mut OssifiedOpDir>($ptr.payload_offset()) };
-        let $n = TypedArenaPtr::new(payload_ptr);
         #[allow(unused_braces)]
         $code
     }};
     ($ptr:ident, LiveLoadState, $n:ident, $code:expr) => {{
-        let payload_ptr =
+        let $n = unsafe { $ptr.as_typed_ptr::<LiveLoadState>() };
-            unsafe { std::mem::transmute::<_, *mut LiveLoadState>($ptr.payload_offset()) };
-        let $n = TypedArenaPtr::new(payload_ptr);
         #[allow(unused_braces)]
         $code
     }};
     ($ptr:ident, Stream, $s:ident, $code:expr) => {{
-        let $s = Stream::from_tag($ptr.get_tag(), $ptr.payload_offset());
+        let $s = Stream::from_tag($ptr.get_tag(), $ptr);
         #[allow(unused_braces)]
         $code
     }};
     ($ptr:ident, TcpListener, $listener:ident, $code:expr) => {{
-        let payload_ptr =
-            unsafe { std::mem::transmute::<_, *mut TcpListener>($ptr.payload_offset()) };
         #[allow(unused_mut)]
-        let mut $listener = TypedArenaPtr::new(payload_ptr);
+        let mut $listener = unsafe { $ptr.as_typed_ptr::<TcpListener>() };
         #[allow(unused_braces)]
         $code
     }};
     ($ptr:ident, HttpListener, $listener:ident, $code:expr) => {{
-        let payload_ptr =
-            unsafe { std::mem::transmute::<_, *mut HttpListener>($ptr.payload_offset()) };
         #[allow(unused_mut)]
-        let mut $listener = TypedArenaPtr::new(payload_ptr);
+        let mut $listener = unsafe { $ptr.as_typed_ptr::<HttpListener>() };
         #[allow(unused_braces)]
         $code
     }};
     ($ptr:ident, HttpResponse, $listener:ident, $code:expr) => {{
-        let payload_ptr =
-            unsafe { std::mem::transmute::<_, *mut HttpResponse>($ptr.payload_offset()) };
         #[allow(unused_mut)]
-        let mut $listener = TypedArenaPtr::new(payload_ptr);
+        let mut $listener = unsafe { $ptr.as_typed_ptr::<HttpResponse>() };
         #[allow(unused_braces)]
         $code
     }};
     ($ptr:ident, IndexPtr, $ip:ident, $code:expr) => {{
         #[allow(unused_mut)]
-        let mut $ip =
+        let mut $ip = unsafe { $ptr.as_typed_ptr::<IndexPtr>() };
-            TypedArenaPtr::new(unsafe { std::mem::transmute::<_, *mut IndexPtr>($ptr.get_ptr()) });
         #[allow(unused_braces)]
         $code
     }};
     ($ptr:ident, $($tags:tt)|+, $s:ident, $code:expr) => {{
-        let $s = Stream::from_tag($ptr.get_tag(), $ptr.payload_offset());
+        let $s = Stream::from_tag($ptr.get_tag(), $ptr);
         #[allow(unused_braces)]
         $code
     }};

src/types.rs

@@ -743,6 +743,13 @@ impl UntypedArenaPtr {
         unsafe { self.get_ptr().add(mem::size_of::<ArenaHeader>()) }
     }
 
+    /// Safety
+    /// - this UntypedArenaPtr actuall pointee type is T
+    #[inline]
+    pub unsafe fn as_typed_ptr<T: ArenaAllocated>(self) -> TypedArenaPtr<T> {
+        T::typed_ptr(self)
+    }
+
     #[inline]
     pub fn get_mark_bit(self) -> bool {
         self.m()