improvement(runtime): clarify usage of unsafe TypeInfo

Garbage collection uses TypeInfo pointers that cannot directly be
linked to the lifetime of their assemblies. As a result any usage of
these pointers is unsafe. To clarify this, the RawTypeInfo type has been
renamed to UnsafeTypeInfo and everywhere it is used, unsafe must be
added, similar to Rust's UnsafeCell type.

crates/mun_runtime/src/assembly.rs

@@ -8,9 +8,9 @@ use libloading::Symbol;
 mod temp_library;
 
 use self::temp_library::TempLibrary;
-use crate::garbage_collector::{GarbageCollector, RawTypeInfo};
+use crate::garbage_collector::{GarbageCollector, UnsafeTypeInfo};
 use memory::mapping::{Mapping, MemoryMapper};
-use std::{collections::HashSet, sync::Arc};
+use std::{collections::HashSet, ptr::NonNull, sync::Arc};
 
 /// An assembly is a hot reloadable compilation unit, consisting of one or more Mun modules.
 pub struct Assembly {
@@ -145,20 +145,30 @@ impl Assembly {
         let mut new_assembly =
             Assembly::load(library_path, self.allocator.clone(), runtime_dispatch_table)?;
 
-        let old_types: Vec<RawTypeInfo> = self
+        let old_types: Vec<UnsafeTypeInfo> = self
             .info
             .symbols
             .types()
             .iter()
-            .map(|ty| (*ty as *const abi::TypeInfo).into())
+            .map(|ty| {
+                // Safety: `ty` is a shared reference, so is guaranteed to not be `ptr::null()`.
+                UnsafeTypeInfo::new(unsafe {
+                    NonNull::new_unchecked(*ty as *const abi::TypeInfo as *mut _)
+                })
+            })
             .collect();
 
-        let new_types: Vec<RawTypeInfo> = new_assembly
+        let new_types: Vec<UnsafeTypeInfo> = new_assembly
             .info
             .symbols
             .types()
             .iter()
-            .map(|ty| (*ty as *const abi::TypeInfo).into())
+            .map(|ty| {
+                // Safety: `ty` is a shared reference, so is guaranteed to not be `ptr::null()`.
+                UnsafeTypeInfo::new(unsafe {
+                    NonNull::new_unchecked(*ty as *const abi::TypeInfo as *mut _)
+                })
+            })
             .collect();
 
         let mapping = Mapping::new(&old_types, &new_types);

crates/mun_runtime/src/garbage_collector.rs

@@ -1,100 +1,99 @@
 use memory::gc::{self, HasIndirectionPtr};
-use std::{alloc::Layout, hash::Hash};
-
+use std::{alloc::Layout, hash::Hash, ptr::NonNull};
+
+/// `UnsafeTypeInfo` is a type that wraps a `NonNull<TypeInfo>` and indicates unsafe interior
+/// operations on the wrapped `TypeInfo`. The unsafety originates from uncertainty about the
+/// lifetime of the wrapped `TypeInfo`.
+///
+/// Rust lifetime rules do not allow separate lifetimes for struct fields, but we can make `unsafe`
+/// guarantees about their lifetimes. Thus the `UnsafeTypeInfo` type is the only legal way to obtain
+/// shared references to the wrapped `TypeInfo`.
 #[derive(Clone, Copy, Debug)]
 #[repr(transparent)]
-pub struct RawTypeInfo(*const abi::TypeInfo);
+pub struct UnsafeTypeInfo(NonNull<abi::TypeInfo>);
 
-impl RawTypeInfo {
-    /// Returns the inner `TypeInfo` pointer.
-    ///
-    /// # Safety
-    ///
-    /// This method is unsafe because there are no guarantees about the lifetime of the inner
+impl UnsafeTypeInfo {
+    /// Constructs a new instance of `UnsafeTypeInfo`, which will wrap the specified `type_info`
     /// pointer.
-    pub unsafe fn inner(self) -> *const abi::TypeInfo {
+    ///
+    /// All access to the inner value through methods is `unsafe`.
+    pub fn new(type_info: NonNull<abi::TypeInfo>) -> Self {
+        Self(type_info)
+    }
+
+    /// Unwraps the value.
+    pub fn into_inner(self) -> NonNull<abi::TypeInfo> {
         self.0
     }
 }
 
-impl PartialEq for RawTypeInfo {
+impl PartialEq for UnsafeTypeInfo {
     fn eq(&self, other: &Self) -> bool {
-        let this = unsafe { &*self.0 };
-        let other = unsafe { &*other.0 };
-        *this == *other
+        unsafe { *self.0.as_ref() == *other.0.as_ref() }
     }
 }
 
-impl Eq for RawTypeInfo {}
+impl Eq for UnsafeTypeInfo {}
 
-impl Hash for RawTypeInfo {
+impl Hash for UnsafeTypeInfo {
     fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
-        let this = unsafe { &*self.0 };
-        this.hash(state);
+        unsafe { self.0.as_ref().hash(state) };
     }
 }
 
-impl memory::TypeDesc for RawTypeInfo {
+impl memory::TypeDesc for UnsafeTypeInfo {
     fn name(&self) -> &str {
-        let this = unsafe { &*self.0 };
-        this.name()
+        unsafe { self.0.as_ref().name() }
     }
+
     fn guid(&self) -> &abi::Guid {
-        let this = unsafe { &*self.0 };
-        &this.guid
+        unsafe { &self.0.as_ref().guid }
     }
+
     fn group(&self) -> abi::TypeGroup {
-        let this = unsafe { &*self.0 };
-        this.group
+        unsafe { self.0.as_ref().group }
     }
 }
 
-impl memory::TypeFields<RawTypeInfo> for RawTypeInfo {
+impl memory::TypeFields<UnsafeTypeInfo> for UnsafeTypeInfo {
     fn fields(&self) -> Vec<(&str, Self)> {
-        let this = unsafe { &*self.0 };
-        if let Some(s) = this.as_struct() {
+        if let Some(s) = unsafe { self.0.as_ref().as_struct() } {
             s.field_names()
-                .zip(
-                    s.field_types()
-                        .iter()
-                        .map(|ty| (*ty as *const abi::TypeInfo).into()),
-                )
+                .zip(s.field_types().iter().map(|ty| {
+                    // Safety: `ty` is a shared reference, so is guaranteed to not be `ptr::null()`.
+                    UnsafeTypeInfo::new(unsafe {
+                        NonNull::new_unchecked(*ty as *const abi::TypeInfo as *mut _)
+                    })
+                }))
                 .collect()
         } else {
             Vec::new()
         }
     }
 
     fn offsets(&self) -> &[u16] {
-        let this = unsafe { &*self.0 };
-        if let Some(s) = this.as_struct() {
+        if let Some(s) = unsafe { self.0.as_ref().as_struct() } {
             s.field_offsets()
         } else {
             &[]
         }
     }
 }
 
-impl Into<RawTypeInfo> for *const abi::TypeInfo {
-    fn into(self) -> RawTypeInfo {
-        RawTypeInfo(self)
-    }
-}
-
-unsafe impl Send for RawTypeInfo {}
-unsafe impl Sync for RawTypeInfo {}
+unsafe impl Send for UnsafeTypeInfo {}
+unsafe impl Sync for UnsafeTypeInfo {}
 
 pub struct Trace {
     obj: GcPtr,
-    ty: RawTypeInfo,
+    ty: UnsafeTypeInfo,
     index: usize,
 }
 
 impl Iterator for Trace {
     type Item = GcPtr;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let struct_ty = unsafe { self.ty.0.as_ref() }.unwrap().as_struct()?;
+        let struct_ty = unsafe { self.ty.0.as_ref() }.as_struct()?;
         let field_count = struct_ty.field_types().len();
         while self.index < field_count {
             let index = self.index;
@@ -114,15 +113,15 @@ impl Iterator for Trace {
     }
 }
 
-impl memory::TypeLayout for RawTypeInfo {
+impl memory::TypeLayout for UnsafeTypeInfo {
     fn layout(&self) -> Layout {
-        let ty = unsafe { &*self.0 };
+        let ty = unsafe { self.0.as_ref() };
         Layout::from_size_align(ty.size_in_bytes(), ty.alignment())
             .unwrap_or_else(|_| panic!("invalid layout from Mun Type: {:?}", ty))
     }
 }
 
-impl gc::TypeTrace for RawTypeInfo {
+impl gc::TypeTrace for UnsafeTypeInfo {
     type Trace = Trace;
 
     fn trace(&self, obj: GcPtr) -> Self::Trace {
@@ -135,7 +134,7 @@ impl gc::TypeTrace for RawTypeInfo {
 }
 
 /// Defines the garbage collector used by the `Runtime`.
-pub type GarbageCollector = gc::MarkSweep<RawTypeInfo, gc::NoopObserver<gc::Event>>;
+pub type GarbageCollector = gc::MarkSweep<UnsafeTypeInfo, gc::NoopObserver<gc::Event>>;
 
 pub use gc::GcPtr;
-pub type GcRootPtr = gc::GcRootPtr<RawTypeInfo, GarbageCollector>;
+pub type GcRootPtr = gc::GcRootPtr<UnsafeTypeInfo, GarbageCollector>;

crates/mun_runtime/src/lib.rs

@@ -14,14 +14,15 @@ mod reflection;
 mod r#struct;
 
 use failure::Error;
-use garbage_collector::GarbageCollector;
+use garbage_collector::{GarbageCollector, UnsafeTypeInfo};
 use memory::gc::{self, GcRuntime};
 use notify::{DebouncedEvent, RecommendedWatcher, RecursiveMode, Watcher};
 use rustc_hash::FxHashMap;
 use std::{
     collections::HashMap,
     ffi, io, mem,
     path::{Path, PathBuf},
+    ptr::NonNull,
     string::ToString,
     sync::{
         mpsc::{channel, Receiver},
@@ -182,8 +183,13 @@ extern "C" fn new(
     type_info: *const abi::TypeInfo,
     alloc_handle: *mut ffi::c_void,
 ) -> *const *mut ffi::c_void {
+    // Safety: `new` is only called from within Mun assemblies' core logic, so we are guaranteed
+    // that the `Runtime` and its `GarbageCollector` still exist if this function is called, and
+    // will continue to do so for the duration of this function.
     let allocator = unsafe { get_allocator(alloc_handle) };
-    let handle = allocator.alloc(type_info.into());
+    // Safety: the Mun Compiler guarantees that `new` is never called with `ptr::null()`.
+    let type_info = UnsafeTypeInfo::new(unsafe { NonNull::new_unchecked(type_info as *mut _) });
+    let handle = allocator.alloc(type_info);
 
     // Prevent destruction of the allocator
     mem::forget(allocator);

crates/mun_runtime/src/reflection.rs

@@ -2,8 +2,8 @@ use crate::{marshal::Marshal, Runtime, StructRef};
 use abi::HasStaticTypeInfo;
 
 /// Returns whether the specified argument type matches the `type_info`.
-pub fn equals_argument_type<'r, 'e, 'f, T: ArgumentReflection>(
-    runtime: &'r Runtime,
+pub fn equals_argument_type<'e, 'f, T: ArgumentReflection>(
+    runtime: &'f Runtime,
     type_info: &'e abi::TypeInfo,
     arg: &'f T,
 ) -> Result<(), (&'e str, &'f str)> {
@@ -58,7 +58,7 @@ pub trait ArgumentReflection: Sized {
     fn type_guid(&self, runtime: &Runtime) -> abi::Guid;
 
     /// Retrieves the name of the value's type.
-    fn type_name(&self, runtime: &Runtime) -> &str;
+    fn type_name<'r>(&'r self, runtime: &'r Runtime) -> &'r str;
 
     /// Marshals the value.
     fn marshal(self) -> Self::Marshalled;

crates/mun_runtime/src/struct.rs

@@ -1,4 +1,4 @@
-use crate::garbage_collector::{GcPtr, GcRootPtr};
+use crate::garbage_collector::{GcPtr, GcRootPtr, UnsafeTypeInfo};
 use crate::{
     marshal::Marshal,
     reflection::{
@@ -37,9 +37,17 @@ impl StructRef {
     fn new(runtime: Rc<RefCell<Runtime>>, raw: RawStruct) -> Self {
         let handle = {
             let runtime_ref = runtime.borrow();
-            assert!(unsafe { &*runtime_ref.gc().ptr_type(raw.0).inner() }
-                .group
-                .is_struct());
+            // Safety: The type returned from `ptr_type` is guaranteed to live at least as long as
+            // `Runtime` does not change. As we hold a shared reference to `Runtime`, this is safe.
+            assert!(unsafe {
+                runtime_ref
+                    .gc()
+                    .ptr_type(raw.0)
+                    .into_inner()
+                    .as_ref()
+                    .group
+                    .is_struct()
+            });
 
             GcRootPtr::new(&runtime_ref.gc, raw.0)
         };
@@ -54,9 +62,16 @@ impl StructRef {
 
     /// Returns the type information of the struct.
     pub fn type_info<'r>(struct_ref: &Self, runtime_ref: &'r Runtime) -> &'r abi::TypeInfo {
-        // Safety: The lifetime of `TypeInfo` is tied to the lifetime of `Runtime` and thus the
-        // underlying pointer cannot change during the lifetime of the shared reference.
-        unsafe { &*runtime_ref.gc.ptr_type(struct_ref.handle.handle()).inner() }
+        // Safety: The type returned from `ptr_type` is guaranteed to live at least as long as
+        // `Runtime` does not change. As the lifetime of `TypeInfo` is tied to the lifetime of
+        // `Runtime`, this is safe.
+        unsafe {
+            &*runtime_ref
+                .gc
+                .ptr_type(struct_ref.handle.handle())
+                .into_inner()
+                .as_ptr()
+        }
     }
 
     ///
@@ -176,13 +191,29 @@ impl ArgumentReflection for StructRef {
     type Marshalled = RawStruct;
 
     fn type_guid(&self, runtime: &Runtime) -> abi::Guid {
-        let type_info = unsafe { &*runtime.gc().ptr_type(self.handle.handle()).inner() };
-        type_info.guid
+        // Safety: The type returned from `ptr_type` is guaranteed to live at least as long as
+        // `Runtime` does not change. As we hold a shared reference to `Runtime`, this is safe.
+        unsafe {
+            runtime
+                .gc()
+                .ptr_type(self.handle.handle())
+                .into_inner()
+                .as_ref()
+                .guid
+        }
     }
 
     fn type_name(&self, runtime: &Runtime) -> &str {
-        let type_info = unsafe { &*runtime.gc().ptr_type(self.handle.handle()).inner() };
-        type_info.name()
+        // Safety: The type returned from `ptr_type` is guaranteed to live at least as long as
+        // `Runtime` does not change. As we hold a shared reference to `Runtime`, this is safe.
+        unsafe {
+            (&*runtime
+                .gc()
+                .ptr_type(self.handle.handle())
+                .into_inner()
+                .as_ptr())
+                .name()
+        }
     }
 
     fn marshal(self) -> Self::Marshalled {
@@ -212,17 +243,19 @@ impl Marshal<StructRef> for RawStruct {
         let type_info = type_info.unwrap();
         let struct_info = type_info.as_struct().unwrap();
 
-        // HACK: This is very hacky since we know nothing about the lifetime of abi::TypeInfo.
-        let type_info_ptr = (type_info as *const abi::TypeInfo).into();
-
         // Copy the contents of the struct based on what kind of pointer we are dealing with
         let gc_handle = if struct_info.memory_kind == abi::StructMemoryKind::Value {
             // For a value struct, `ptr` points to a struct value.
 
             // Create a new object using the runtime's intrinsic
             let mut gc_handle = {
                 let runtime_ref = runtime.borrow();
-                runtime_ref.gc().alloc(type_info_ptr)
+                runtime_ref.gc().alloc(
+                    // Safety: `ty` is a shared reference, so is guaranteed to not be `ptr::null()`.
+                    UnsafeTypeInfo::new(unsafe {
+                        NonNull::new_unchecked(type_info as *const abi::TypeInfo as *mut _)
+                    }),
+                )
             };
 
             // Construct