Fix miri reported UB in FuncRef and ExternRef conversions (#1201)

* add #[repr(C)] to Transposer as mandated by the Rust reference

* add getters for Transposer::{value,reftype}

* make the transmute explicit

* fix miri reported UB in reftype conversions

* remove reftype.rs utility

No longer needed.

* fix unrelated clippy warning

crates/cli/tests/run.rs

@@ -39,8 +39,6 @@ fn test_verbose() {
     assert!(contains_slice(stdout, b"proc_exit.wat\")::()"));
 }
 
-/// UTILS
-
 /// gets the path to a wasm binary given it's name
 fn get_bin_path(name: &str) -> PathBuf {
     let mut path = PathBuf::new();

crates/wasmi/src/externref.rs

@@ -1,12 +1,11 @@
 use crate::{
     collections::arena::ArenaIndex,
     core::UntypedVal,
-    reftype::Transposer,
     store::Stored,
     AsContextMut,
     StoreContext,
 };
-use core::{any::Any, num::NonZeroU32};
+use core::{any::Any, mem, num::NonZeroU32};
 use std::boxed::Box;
 
 /// A raw index to a function entity.
@@ -117,24 +116,30 @@ fn externref_null_to_zero() {
 
 impl From<UntypedVal> for ExternRef {
     fn from(untyped: UntypedVal) -> Self {
+        if u64::from(untyped) == 0 {
+            return ExternRef::null();
+        }
         // Safety: This operation is safe since there are no invalid
         //         bit patterns for [`ExternRef`] instances. Therefore
         //         this operation cannot produce invalid [`ExternRef`]
         //         instances even though the input [`UntypedVal`]
         //         was modified arbitrarily.
-        unsafe { <Transposer<Self>>::from(untyped).reftype }.canonicalize()
+        unsafe { mem::transmute::<u64, Self>(untyped.into()) }
     }
 }
 
 impl From<ExternRef> for UntypedVal {
     fn from(externref: ExternRef) -> Self {
-        let externref = externref.canonicalize();
+        if externref.is_null() {
+            return UntypedVal::from(0_u64);
+        }
         // Safety: This operation is safe since there are no invalid
         //         bit patterns for [`UntypedVal`] instances. Therefore
         //         this operation cannot produce invalid [`UntypedVal`]
         //         instances even if it was possible to arbitrarily modify
         //         the input [`ExternRef`] instance.
-        Self::from(unsafe { <Transposer<ExternRef>>::new(externref).value })
+        let bits = unsafe { mem::transmute::<ExternRef, u64>(externref) };
+        UntypedVal::from(bits)
     }
 }
 
@@ -149,25 +154,6 @@ impl ExternRef {
             .map(|object| ExternObject::new(ctx, object))
             .map(Self::from_object)
             .unwrap_or_else(Self::null)
-            .canonicalize()
-    }
-
-    /// Canonicalize `self` so that all `null` values have the same representation.
-    ///
-    /// # Note
-    ///
-    /// The underlying issue is that `ExternRef` has many possible values for the
-    /// `null` value. However, to simplify operating on encoded `ExternRef` instances
-    /// (encoded as `UntypedValue`) we want it to encode to exactly one `null`
-    /// value. The most trivial of all possible `null` values is `0_u64`, therefore
-    /// we canonicalize all `null` values to be represented by `0_u64`.
-    fn canonicalize(self) -> Self {
-        if self.is_null() {
-            // Safety: This is safe since `0u64` can be bit
-            //         interpreted as a valid `ExternRef` value.
-            return unsafe { <Transposer<Self>>::null().reftype };
-        }
-        self
     }
 
     /// Creates a new [`ExternRef`] to the given [`ExternObject`].
@@ -177,16 +163,16 @@ impl ExternRef {
         }
     }
 
-    /// Creates a new [`ExternRef`] which is `null`.
-    pub fn null() -> Self {
-        Self { inner: None }.canonicalize()
-    }
-
     /// Returns `true` if [`ExternRef`] is `null`.
     pub fn is_null(&self) -> bool {
         self.inner.is_none()
     }
 
+    /// Creates a new [`ExternRef`] which is `null`.
+    pub fn null() -> Self {
+        Self { inner: None }
+    }
+
     /// Returns a shared reference to the underlying data for this [`ExternRef`].
     ///
     /// # Panics

crates/wasmi/src/func/funcref.rs

@@ -1,5 +1,6 @@
 use super::Func;
-use crate::{core::UntypedVal, reftype::Transposer};
+use crate::core::UntypedVal;
+use core::mem;
 
 /// A nullable [`Func`] reference.
 #[derive(Debug, Default, Copy, Clone)]
@@ -35,24 +36,30 @@ fn funcref_null_to_zero() {
 
 impl From<UntypedVal> for FuncRef {
     fn from(untyped: UntypedVal) -> Self {
+        if u64::from(untyped) == 0 {
+            return FuncRef::null();
+        }
         // Safety: This union access is safe since there are no invalid
         //         bit patterns for [`FuncRef`] instances. Therefore
         //         this operation cannot produce invalid [`FuncRef`]
         //         instances even though the input [`UntypedVal`]
         //         was modified arbitrarily.
-        unsafe { <Transposer<Self>>::from(untyped).reftype }.canonicalize()
+        unsafe { mem::transmute::<u64, Self>(untyped.into()) }
     }
 }
 
 impl From<FuncRef> for UntypedVal {
     fn from(funcref: FuncRef) -> Self {
-        let funcref = funcref.canonicalize();
+        if funcref.is_null() {
+            return UntypedVal::from(0_u64);
+        }
         // Safety: This operation is safe since there are no invalid
         //         bit patterns for [`UntypedVal`] instances. Therefore
         //         this operation cannot produce invalid [`UntypedVal`]
         //         instances even if it was possible to arbitrarily modify
         //         the input [`FuncRef`] instance.
-        Self::from(unsafe { <Transposer<FuncRef>>::new(funcref).value })
+        let bits = unsafe { mem::transmute::<FuncRef, u64>(funcref) };
+        UntypedVal::from(bits)
     }
 }
 
@@ -62,22 +69,9 @@ impl FuncRef {
         self.inner.is_none()
     }
 
-    /// Canonicalize `self` so that all `null` values have the same representation.
-    ///
-    /// # Note
-    ///
-    /// The underlying issue is that `FuncRef` has many possible values for the
-    /// `null` value. However, to simplify operating on encoded `FuncRef` instances
-    /// (encoded as `UntypedValue`) we want it to encode to exactly one `null`
-    /// value. The most trivial of all possible `null` values is `0_u64`, therefore
-    /// we canonicalize all `null` values to be represented by `0_u64`.
-    fn canonicalize(self) -> Self {
-        if self.is_null() {
-            // Safety: This is safe since `0u64` can be bit
-            //         interpreted as a valid `FuncRef` value.
-            return unsafe { <Transposer<Self>>::null().reftype };
-        }
-        self
+    /// Creates a `null` [`FuncRef`].
+    pub fn null() -> Self {
+        Self::new(None)
     }
 
     /// Creates a new [`FuncRef`].
@@ -95,7 +89,6 @@ impl FuncRef {
         Self {
             inner: nullable_func.into(),
         }
-        .canonicalize()
     }
 
     /// Returns the inner [`Func`] if [`FuncRef`] is not `null`.
@@ -104,9 +97,4 @@ impl FuncRef {
     pub fn func(&self) -> Option<&Func> {
         self.inner.as_ref()
     }
-
-    /// Creates a `null` [`FuncRef`].
-    pub fn null() -> Self {
-        Self::new(None).canonicalize()
-    }
 }

crates/wasmi/src/lib.rs

@@ -97,7 +97,6 @@ mod limits;
 mod linker;
 mod memory;
 mod module;
-mod reftype;
 mod store;
 mod table;
 mod value;