review comments

crates/runtime/src/instance/allocator.rs

@@ -219,12 +219,6 @@ impl SharedSignatures {
     }
 }
 
-impl std::default::Default for SharedSignatures {
-    fn default() -> Self {
-        SharedSignatures::None
-    }
-}
-
 impl From<VMSharedSignatureIndex> for SharedSignatures {
     fn from(val: VMSharedSignatureIndex) -> SharedSignatures {
         SharedSignatures::Always(val)

crates/runtime/src/libcalls.rs

@@ -58,7 +58,7 @@
 
 use crate::externref::VMExternRef;
 use crate::instance::Instance;
-use crate::table::{Table, TableElementType, REF_MASK};
+use crate::table::{Table, TableElementType};
 use crate::traphandlers::{raise_lib_trap, resume_panic, Trap};
 use crate::vmcontext::{VMCallerCheckedAnyfunc, VMContext};
 use backtrace::Backtrace;
@@ -395,7 +395,7 @@ pub unsafe extern "C" fn ref_func(vmctx: *mut VMContext, func_index: u32) -> *mu
     let instance = (*vmctx).instance_mut();
     let anyfunc = instance
         .get_caller_checked_anyfunc(FuncIndex::from_u32(func_index))
-        .unwrap();
+        .expect("ref_func: caller_checked_anyfunc should always be available for given func index");
     anyfunc as *mut _
 }
 
@@ -421,7 +421,7 @@ pub unsafe extern "C" fn table_get_lazy_init_funcref(
         .get(index)
         .expect("table access already bounds-checked");
 
-    (elem.into_raw() & REF_MASK) as *mut _
+    elem.into_ref_asserting_initialized() as *mut _
 }
 
 /// Drop a `VMExternRef`.

crates/runtime/src/table.rs

@@ -39,13 +39,6 @@ pub enum TableElementType {
 unsafe impl Send for TableElement where VMExternRef: Send {}
 unsafe impl Sync for TableElement where VMExternRef: Sync {}
 
-/// The mask we apply to all refs loaded from tables.
-///
-/// This allows us to use the LSB as an "initialized flag" (see below)
-/// to distinguish from an uninitialized element in a
-/// lazily-initialized funcref table.
-pub(crate) const REF_MASK: usize = !1;
-
 /// An "initialized bit" in a table.
 ///
 /// We lazily initialize tables of funcrefs, and this mechanism
@@ -74,15 +67,22 @@ pub(crate) const REF_MASK: usize = !1;
 /// accessing tables direclty in fastpaths in generated code as well
 const REF_INIT_BIT: usize = 1;
 
+/// The mask we apply to all refs loaded from tables.
+///
+/// This allows us to use the LSB as an "initialized flag" (see below)
+/// to distinguish from an uninitialized element in a
+/// lazily-initialized funcref table.
+const REF_MASK: usize = !REF_INIT_BIT;
+
 impl TableElement {
-    /// Consumes the given raw pointer into a table element.
+    /// Consumes the given raw table element value into a table element.
     ///
     /// # Safety
     ///
     /// This is unsafe as it will *not* clone any externref, leaving the reference count unchanged.
     ///
     /// This should only be used if the raw pointer is no longer in use.
-    unsafe fn from_raw(ty: TableElementType, ptr: usize) -> Self {
+    unsafe fn from_table_value(ty: TableElementType, ptr: usize) -> Self {
         match (ty, ptr) {
             (TableElementType::Func, 0) => Self::UninitFunc,
             (TableElementType::Func, ptr) => Self::FuncRef((ptr & REF_MASK) as _),
@@ -93,12 +93,12 @@ impl TableElement {
         }
     }
 
-    /// Clones a table element from the underlying raw pointer.
+    /// Clones a table element from the underlying table element.
     ///
     /// # Safety
     ///
     /// This is unsafe as it will clone any externref, incrementing the reference count.
-    unsafe fn clone_from_raw(ty: TableElementType, ptr: usize) -> Self {
+    unsafe fn clone_from_table_value(ty: TableElementType, ptr: usize) -> Self {
         match (ty, ptr) {
             (TableElementType::Func, 0) => Self::UninitFunc,
             (TableElementType::Func, ptr) => Self::FuncRef((ptr & REF_MASK) as _),
@@ -109,21 +109,42 @@ impl TableElement {
         }
     }
 
-    /// Consumes a table element into a raw pointer.
+    /// Consumes a table element into a raw table element value. This
+    /// includes any tag bits or other storage details that we
+    /// maintain in the table slot.
     ///
     /// # Safety
     ///
     /// This is unsafe as it will consume any underlying externref into a raw pointer without modifying
     /// the reference count.
     ///
     /// Use `from_raw` to properly drop any table elements stored as raw pointers.
-    pub(crate) unsafe fn into_raw(self) -> usize {
+    pub(crate) unsafe fn into_table_value(self) -> usize {
         match self {
             Self::UninitFunc => 0,
             Self::FuncRef(e) => (e as usize) | REF_INIT_BIT,
             Self::ExternRef(e) => e.map_or(0, |e| e.into_raw() as usize),
         }
     }
+
+    /// Consumes a table element into a pointer/reference, as it
+    /// exists outside the table itself. This strips off any tag bits
+    /// or other information that only lives inside the table.
+    ///
+    /// Can only be done to an initialized table element; lazy init
+    /// must occur first. (In other words, lazy values do not survive
+    /// beyond the table, as every table read path initializes them.)
+    ///
+    /// # Safety
+    ///
+    /// The same warnings as for `into_table_values()` apply.
+    pub(crate) unsafe fn into_ref_asserting_initialized(self) -> usize {
+        match self {
+            Self::FuncRef(e) => (e as usize),
+            Self::ExternRef(e) => e.map_or(0, |e| e.into_raw() as usize),
+            Self::UninitFunc => panic!("Uninitialized table element value outside of table slot"),
+        }
+    }
 }
 
 impl From<*mut VMCallerCheckedAnyfunc> for TableElement {
@@ -449,7 +470,7 @@ impl Table {
     pub fn get(&self, index: u32) -> Option<TableElement> {
         self.elements()
             .get(index as usize)
-            .map(|p| unsafe { TableElement::clone_from_raw(self.element_type(), *p) })
+            .map(|p| unsafe { TableElement::clone_from_table_value(self.element_type(), *p) })
     }
 
     /// Set reference to the specified element.
@@ -551,10 +572,10 @@ impl Table {
     fn set_raw(ty: TableElementType, elem: &mut usize, val: TableElement) {
         unsafe {
             let old = *elem;
-            *elem = val.into_raw();
+            *elem = val.into_table_value();
 
             // Drop the old element
-            let _ = TableElement::from_raw(ty, old);
+            let _ = TableElement::from_table_value(ty, old);
         }
     }
 
@@ -580,7 +601,7 @@ impl Table {
                 let dst = dst_table.elements_mut();
                 let src = src_table.elements();
                 for (s, d) in src_range.zip(dst_range) {
-                    let elem = unsafe { TableElement::clone_from_raw(ty, src[s]) };
+                    let elem = unsafe { TableElement::clone_from_table_value(ty, src[s]) };
                     Self::set_raw(ty, &mut dst[d], elem);
                 }
             }
@@ -600,12 +621,12 @@ impl Table {
                 // ranges
                 if dst_range.start <= src_range.start {
                     for (s, d) in src_range.zip(dst_range) {
-                        let elem = unsafe { TableElement::clone_from_raw(ty, dst[s]) };
+                        let elem = unsafe { TableElement::clone_from_table_value(ty, dst[s]) };
                         Self::set_raw(ty, &mut dst[d], elem);
                     }
                 } else {
                     for (s, d) in src_range.rev().zip(dst_range.rev()) {
-                        let elem = unsafe { TableElement::clone_from_raw(ty, dst[s]) };
+                        let elem = unsafe { TableElement::clone_from_table_value(ty, dst[s]) };
                         Self::set_raw(ty, &mut dst[d], elem);
                     }
                 }
@@ -625,7 +646,7 @@ impl Drop for Table {
 
         // Properly drop any table elements stored in the table
         for element in self.elements() {
-            drop(unsafe { TableElement::from_raw(ty, *element) });
+            drop(unsafe { TableElement::from_table_value(ty, *element) });
         }
     }
 }
@@ -716,7 +737,7 @@ impl TableAndOwningInstance {
                     if let Some(anyfunc) = instance.get_caller_checked_anyfunc(func) {
                         // Reborrow ptr.
                         let elt_ptr = table.elements_mut().get_mut(index as usize).unwrap();
-                        *elt_ptr = unsafe { TableElement::FuncRef(anyfunc).into_raw() };
+                        *elt_ptr = unsafe { TableElement::FuncRef(anyfunc).into_table_value() };
                     }
                 }
             }