Make ValRaw fields private (#4186)

* Make `ValRaw` fields private

Force accessing to go through constructors and accessors to localize the
knowledge about little-endian-ness. This is spawned since I made a
mistake in #4039 about endianness.

* Fix some tests

* Component model changes

benches/call.rs

@@ -160,7 +160,7 @@ fn bench_host_to_wasm<Params, Results>(
         let untyped = instance.get_func(&mut *store, name).unwrap();
         let params = typed_params.to_vals();
         let results = typed_results.to_vals();
-        let mut space = vec![ValRaw { i32: 0 }; params.len().max(results.len())];
+        let mut space = vec![ValRaw::i32(0); params.len().max(results.len())];
         b.iter(|| unsafe {
             for (i, param) in params.iter().enumerate() {
                 space[i] = param.to_raw(&mut *store);

crates/runtime/src/vmcontext.rs

@@ -794,7 +794,7 @@ pub union ValRaw {
     /// or unsigned. The Rust type `i32` is simply chosen for convenience.
     ///
     /// This value is always stored in a little-endian format.
-    pub i32: i32,
+    i32: i32,
 
     /// A WebAssembly `i64` value.
     ///
@@ -803,7 +803,7 @@ pub union ValRaw {
     /// or unsigned. The Rust type `i64` is simply chosen for convenience.
     ///
     /// This value is always stored in a little-endian format.
-    pub i64: i64,
+    i64: i64,
 
     /// A WebAssembly `f32` value.
     ///
@@ -813,7 +813,7 @@ pub union ValRaw {
     /// `u32` value is the return value of `f32::to_bits` in Rust.
     ///
     /// This value is always stored in a little-endian format.
-    pub f32: u32,
+    f32: u32,
 
     /// A WebAssembly `f64` value.
     ///
@@ -823,7 +823,7 @@ pub union ValRaw {
     /// `u64` value is the return value of `f64::to_bits` in Rust.
     ///
     /// This value is always stored in a little-endian format.
-    pub f64: u64,
+    f64: u64,
 
     /// A WebAssembly `v128` value.
     ///
@@ -833,7 +833,7 @@ pub union ValRaw {
     /// underlying bits is left up to the instructions which consume this value.
     ///
     /// This value is always stored in a little-endian format.
-    pub v128: u128,
+    v128: u128,
 
     /// A WebAssembly `funcref` value.
     ///
@@ -843,7 +843,7 @@ pub union ValRaw {
     /// carefully calling the correct functions throughout the runtime.
     ///
     /// This value is always stored in a little-endian format.
-    pub funcref: usize,
+    funcref: usize,
 
     /// A WebAssembly `externref` value.
     ///
@@ -853,7 +853,119 @@ pub union ValRaw {
     /// carefully calling the correct functions throughout the runtime.
     ///
     /// This value is always stored in a little-endian format.
-    pub externref: usize,
+    externref: usize,
+}
+
+impl ValRaw {
+    /// Creates a WebAssembly `i32` value
+    #[inline]
+    pub fn i32(i: i32) -> ValRaw {
+        ValRaw { i32: i.to_le() }
+    }
+
+    /// Creates a WebAssembly `i64` value
+    #[inline]
+    pub fn i64(i: i64) -> ValRaw {
+        ValRaw { i64: i.to_le() }
+    }
+
+    /// Creates a WebAssembly `i32` value
+    #[inline]
+    pub fn u32(i: u32) -> ValRaw {
+        ValRaw::i32(i as i32)
+    }
+
+    /// Creates a WebAssembly `i64` value
+    #[inline]
+    pub fn u64(i: u64) -> ValRaw {
+        ValRaw::i64(i as i64)
+    }
+
+    /// Creates a WebAssembly `f32` value
+    #[inline]
+    pub fn f32(i: u32) -> ValRaw {
+        ValRaw { f32: i.to_le() }
+    }
+
+    /// Creates a WebAssembly `f64` value
+    #[inline]
+    pub fn f64(i: u64) -> ValRaw {
+        ValRaw { f64: i.to_le() }
+    }
+
+    /// Creates a WebAssembly `v128` value
+    #[inline]
+    pub fn v128(i: u128) -> ValRaw {
+        ValRaw { v128: i.to_le() }
+    }
+
+    /// Creates a WebAssembly `funcref` value
+    #[inline]
+    pub fn funcref(i: usize) -> ValRaw {
+        ValRaw { funcref: i.to_le() }
+    }
+
+    /// Creates a WebAssembly `externref` value
+    #[inline]
+    pub fn externref(i: usize) -> ValRaw {
+        ValRaw {
+            externref: i.to_le(),
+        }
+    }
+
+    /// Gets the WebAssembly `i32` value
+    #[inline]
+    pub fn get_i32(&self) -> i32 {
+        unsafe { i32::from_le(self.i32) }
+    }
+
+    /// Gets the WebAssembly `i64` value
+    #[inline]
+    pub fn get_i64(&self) -> i64 {
+        unsafe { i64::from_le(self.i64) }
+    }
+
+    /// Gets the WebAssembly `i32` value
+    #[inline]
+    pub fn get_u32(&self) -> u32 {
+        self.get_i32() as u32
+    }
+
+    /// Gets the WebAssembly `i64` value
+    #[inline]
+    pub fn get_u64(&self) -> u64 {
+        self.get_i64() as u64
+    }
+
+    /// Gets the WebAssembly `f32` value
+    #[inline]
+    pub fn get_f32(&self) -> u32 {
+        unsafe { u32::from_le(self.f32) }
+    }
+
+    /// Gets the WebAssembly `f64` value
+    #[inline]
+    pub fn get_f64(&self) -> u64 {
+        unsafe { u64::from_le(self.f64) }
+    }
+
+    /// Gets the WebAssembly `v128` value
+    #[inline]
+    pub fn get_v128(&self) -> u128 {
+        unsafe { u128::from_le(self.v128) }
+    }
+
+    /// Gets the WebAssembly `funcref` value
+    #[inline]
+    pub fn get_funcref(&self) -> usize {
+        unsafe { usize::from_le(self.funcref) }
+    }
+
+    /// Gets the WebAssembly `externref` value
+    #[inline]
+    pub fn get_externref(&self) -> usize {
+        unsafe { usize::from_le(self.externref) }
+    }
 }
 
 /// Trampoline function pointer type.

crates/wasmtime/src/component/func/typed.rs

@@ -239,9 +239,7 @@ where
         // This comment about 64-bit integers is also referred to below with
         // "WRITEPTR64".
         let params_and_results = &mut MaybeUninit::new(ParamsAndResults {
-            params: ValRaw {
+            params: ValRaw::i64(ptr as i64),
-                i64: (ptr as i64).to_le(),
-            },
         });
 
         self.call_raw(store, params_and_results)
@@ -727,7 +725,7 @@ unsafe impl ComponentValue for () {
 // Macro to help generate `ComponentValue` implementations for primitive types
 // such as integers, char, bool, etc.
 macro_rules! integers {
-    ($($primitive:ident = $ty:ident in $field:ident $(as $unsigned:ident)?,)*) => ($(
+    ($($primitive:ident = $ty:ident in $field:ident/$get:ident,)*) => ($(
         unsafe impl ComponentValue for $primitive {
             type Lower = ValRaw;
             type Lift = $primitive;
@@ -745,8 +743,7 @@ macro_rules! integers {
                 _func: &Func,
                 dst: &mut MaybeUninit<Self::Lower>,
             ) -> Result<()> {
-                map_maybe_uninit!(dst.$field)
+                dst.write(ValRaw::$field(*self as $field));
-                    .write((*self $(as $unsigned)? as $field).to_le());
                 Ok(())
             }
 
@@ -767,17 +764,11 @@ macro_rules! integers {
 
             #[inline]
             fn lift(src: &Self::Lower) -> Result<Self::Lift> {
-                // Convert from little-endian and then view the signed storage
-                // as an optionally-unsigned type.
-                let field = unsafe {
-                    $field::from_le(src.$field) $(as $unsigned)?
-                };
-
                 // Perform a lossless cast from our field storage to the
                 // destination type. Note that `try_from` here is load bearing
                 // which rejects conversions like `500u32` to `u8` because
                 // that's out-of-bounds for `u8`.
-                Ok($primitive::try_from(field)?)
+                Ok($primitive::try_from(src.$get())?)
             }
         }
 
@@ -792,18 +783,18 @@ macro_rules! integers {
 }
 
 integers! {
-    i8 = S8 in i32,
+    i8 = S8 in i32/get_i32,
-    u8 = U8 in i32 as u32,
+    u8 = U8 in u32/get_u32,
-    i16 = S16 in i32,
+    i16 = S16 in i32/get_i32,
-    u16 = U16 in i32 as u32,
+    u16 = U16 in u32/get_u32,
-    i32 = S32 in i32,
+    i32 = S32 in i32/get_i32,
-    u32 = U32 in i32 as u32,
+    u32 = U32 in u32/get_u32,
-    i64 = S64 in i64,
+    i64 = S64 in i64/get_i64,
-    u64 = U64 in i64 as u64,
+    u64 = U64 in u64/get_u64,
 }
 
 macro_rules! floats {
-    ($($float:ident/$storage:ident = $ty:ident)*) => ($(const _: () = {
+    ($($float:ident/$get_float:ident = $ty:ident)*) => ($(const _: () = {
         /// All floats in-and-out of the canonical ABI always have their NaN
         /// payloads canonicalized. Conveniently the `NAN` constant in Rust has
         /// the same representation as canonical NAN, so we can use that for the
@@ -834,8 +825,7 @@ macro_rules! floats {
                 _func: &Func,
                 dst: &mut MaybeUninit<Self::Lower>,
             ) -> Result<()> {
-                map_maybe_uninit!(dst.$float)
+                dst.write(ValRaw::$float(canonicalize(*self).to_bits()));
-                    .write(canonicalize(*self).to_bits().to_le());
                 Ok(())
             }
 
@@ -855,8 +845,7 @@ macro_rules! floats {
 
             #[inline]
             fn lift(src: &Self::Lower) -> Result<Self::Lift> {
-                let field = $storage::from_le(unsafe { src.$float });
+                Ok(canonicalize($float::from_bits(src.$get_float())))
-                Ok(canonicalize($float::from_bits(field)))
             }
         }
 
@@ -874,8 +863,8 @@ macro_rules! floats {
 }
 
 floats! {
-    f32/u32 = Float32
+    f32/get_f32 = Float32
-    f64/u64 = Float64
+    f64/get_f64 = Float64
 }
 
 unsafe impl ComponentValue for bool {
@@ -895,7 +884,7 @@ unsafe impl ComponentValue for bool {
         _func: &Func,
         dst: &mut MaybeUninit<Self::Lower>,
     ) -> Result<()> {
-        map_maybe_uninit!(dst.i32).write((*self as i32).to_le());
+        dst.write(ValRaw::i32(*self as i32));
         Ok(())
     }
 
@@ -916,7 +905,7 @@ unsafe impl ComponentValue for bool {
 
     #[inline]
     fn lift(src: &Self::Lower) -> Result<Self::Lift> {
-        match i32::from_le(unsafe { src.i32 }) {
+        match src.get_i32() {
             0 => Ok(false),
             1 => Ok(true),
             _ => bail!("invalid boolean value"),
@@ -958,7 +947,7 @@ unsafe impl ComponentValue for char {
         _func: &Func,
         dst: &mut MaybeUninit<Self::Lower>,
     ) -> Result<()> {
-        map_maybe_uninit!(dst.i32).write((u32::from(*self) as i32).to_le());
+        dst.write(ValRaw::u32(u32::from(*self)));
         Ok(())
     }
 
@@ -979,8 +968,7 @@ unsafe impl ComponentValue for char {
 
     #[inline]
     fn lift(src: &Self::Lower) -> Result<Self::Lift> {
-        let bits = i32::from_le(unsafe { src.i32 }) as u32;
+        Ok(char::try_from(src.get_u32())?)
-        Ok(char::try_from(bits)?)
     }
 }
 
@@ -1018,8 +1006,8 @@ unsafe impl ComponentValue for str {
         let (ptr, len) = lower_string(&mut Memory::new(store.as_context_mut(), func), self)?;
         // See "WRITEPTR64" above for why this is always storing a 64-bit
         // integer.
-        map_maybe_uninit!(dst[0].i64).write((ptr as i64).to_le());
+        map_maybe_uninit!(dst[0]).write(ValRaw::i64(ptr as i64));
-        map_maybe_uninit!(dst[1].i64).write((len as i64).to_le());
+        map_maybe_uninit!(dst[1]).write(ValRaw::i64(len as i64));
         Ok(())
     }
 
@@ -1155,8 +1143,8 @@ where
         let (ptr, len) = lower_list(&mut Memory::new(store.as_context_mut(), func), self)?;
         // See "WRITEPTR64" above for why this is always storing a 64-bit
         // integer.
-        map_maybe_uninit!(dst[0].i64).write((ptr as i64).to_le());
+        map_maybe_uninit!(dst[0]).write(ValRaw::i64(ptr as i64));
-        map_maybe_uninit!(dst[1].i64).write((len as i64).to_le());
+        map_maybe_uninit!(dst[1]).write(ValRaw::i64(len as i64));
         Ok(())
     }
 
@@ -1311,7 +1299,7 @@ where
     ) -> Result<()> {
         match self {
             None => {
-                map_maybe_uninit!(dst.A1.i32).write(0_i32.to_le());
+                map_maybe_uninit!(dst.A1).write(ValRaw::i32(0));
                 // Note that this is unsafe as we're writing an arbitrary
                 // bit-pattern to an arbitrary type, but part of the unsafe
                 // contract of the `ComponentValue` trait is that we can assign
@@ -1323,7 +1311,7 @@ where
                 }
             }
             Some(val) => {
-                map_maybe_uninit!(dst.A1.i32).write(1_i32.to_le());
+                map_maybe_uninit!(dst.A1).write(ValRaw::i32(1));
                 val.lower(store, func, map_maybe_uninit!(dst.A2))?;
             }
         }
@@ -1354,7 +1342,7 @@ where
     }
 
     fn lift(src: &Self::Lower) -> Result<Self::Lift> {
-        Ok(match i32::from_le(unsafe { src.A1.i32 }) {
+        Ok(match src.A1.get_i32() {
             0 => None,
             1 => Some(T::lift(&src.A2)?),
             _ => bail!("invalid option discriminant"),
@@ -1442,11 +1430,11 @@ where
 
         match self {
             Ok(e) => {
-                map_maybe_uninit!(dst.tag.i32).write(0_i32.to_le());
+                map_maybe_uninit!(dst.tag).write(ValRaw::i32(0));
                 e.lower(store, func, map_maybe_uninit!(dst.payload.ok))?;
             }
             Err(e) => {
-                map_maybe_uninit!(dst.tag.i32).write(1_i32.to_le());
+                map_maybe_uninit!(dst.tag).write(ValRaw::i32(1));
                 e.lower(store, func, map_maybe_uninit!(dst.payload.err))?;
             }
         }
@@ -1492,7 +1480,7 @@ where
         // being used then both `T` and `E` have zero size.
         assert!(mem::size_of_val(&src.payload) == 0);
 
-        Ok(match i32::from_le(unsafe { src.tag.i32 }) {
+        Ok(match src.tag.get_i32() {
             0 => Ok(unsafe { T::lift(&src.payload.ok)? }),
             1 => Err(unsafe { E::lift(&src.payload.err)? }),
             _ => bail!("invalid expected discriminant"),
@@ -1778,7 +1766,7 @@ unsafe impl<T: ComponentValue> ComponentReturn for Value<T> {
 
     fn lift(store: &StoreOpaque, func: &Func, src: &Self::Lower) -> Result<Self> {
         // FIXME: needs to read an i64 for memory64
-        let ptr = u32::from_le(unsafe { src.i32 as u32 }) as usize;
+        let ptr = src.get_u32() as usize;
         Value::new(store, func, ptr)
     }
 }

crates/wasmtime/src/func.rs

@@ -966,7 +966,7 @@ impl Func {
         // Store the argument values into `values_vec`.
         let mut values_vec = store.0.take_wasm_val_raw_storage();
         debug_assert!(values_vec.is_empty());
-        values_vec.resize_with(values_vec_size, || ValRaw { i32: 0 });
+        values_vec.resize_with(values_vec_size, || ValRaw::i32(0));
         for (arg, slot) in params.iter().cloned().zip(&mut values_vec) {
             unsafe {
                 *slot = arg.to_raw(&mut *store);

crates/wasmtime/src/func/typed.rs

@@ -231,7 +231,7 @@ pub unsafe trait WasmTy: Send {
 }
 
 macro_rules! integers {
-    ($($primitive:ident => $ty:ident in $raw:ident)*) => ($(
+    ($($primitive:ident/$get_primitive:ident => $ty:ident in $raw:ident)*) => ($(
         unsafe impl WasmTy for $primitive {
             type Abi = $primitive;
             #[inline]
@@ -248,11 +248,11 @@ macro_rules! integers {
             }
             #[inline]
             unsafe fn abi_from_raw(raw: *mut ValRaw) -> $primitive {
-                $primitive::from_le((*raw).$raw as $primitive)
+                (*raw).$get_primitive()
             }
             #[inline]
             unsafe fn abi_into_raw(abi: $primitive, raw: *mut ValRaw) {
-                (*raw).$raw = abi.to_le() as $raw;
+                *raw = ValRaw::$primitive(abi);
             }
             #[inline]
             fn into_abi(self, _store: &mut StoreOpaque) -> Self::Abi {
@@ -267,14 +267,14 @@ macro_rules! integers {
 }
 
 integers! {
-    i32 => I32 in i32
+    i32/get_i32 => I32 in i32
-    i64 => I64 in i64
+    i64/get_i64 => I64 in i64
-    u32 => I32 in i32
+    u32/get_u32 => I32 in i32
-    u64 => I64 in i64
+    u64/get_u64 => I64 in i64
 }
 
 macro_rules! floats {
-    ($($float:ident/$int:ident => $ty:ident)*) => ($(
+    ($($float:ident/$int:ident/$get_float:ident => $ty:ident)*) => ($(
         unsafe impl WasmTy for $float {
             type Abi = $float;
             #[inline]
@@ -291,11 +291,11 @@ macro_rules! floats {
             }
             #[inline]
             unsafe fn abi_from_raw(raw: *mut ValRaw) -> $float {
-                $float::from_bits($int::from_le((*raw).$float))
+                $float::from_bits((*raw).$get_float())
             }
             #[inline]
             unsafe fn abi_into_raw(abi: $float, raw: *mut ValRaw) {
-                (*raw).$float = abi.to_bits().to_le();
+                *raw = ValRaw::$float(abi.to_bits());
             }
             #[inline]
             fn into_abi(self, _store: &mut StoreOpaque) -> Self::Abi {
@@ -310,8 +310,8 @@ macro_rules! floats {
 }
 
 floats! {
-    f32/u32 => F32
+    f32/u32/get_f32 => F32
-    f64/u64 => F64
+    f64/u64/get_f64 => F64
 }
 
 unsafe impl WasmTy for Option<ExternRef> {
@@ -334,12 +334,12 @@ unsafe impl WasmTy for Option<ExternRef> {
 
     #[inline]
     unsafe fn abi_from_raw(raw: *mut ValRaw) -> *mut u8 {
-        usize::from_le((*raw).externref) as *mut u8
+        (*raw).get_externref() as *mut u8
     }
 
     #[inline]
     unsafe fn abi_into_raw(abi: *mut u8, raw: *mut ValRaw) {
-        (*raw).externref = (abi as usize).to_le();
+        *raw = ValRaw::externref(abi as usize);
     }
 
     #[inline]
@@ -420,12 +420,12 @@ unsafe impl WasmTy for Option<Func> {
 
     #[inline]
     unsafe fn abi_from_raw(raw: *mut ValRaw) -> Self::Abi {
-        usize::from_le((*raw).funcref) as Self::Abi
+        (*raw).get_funcref() as Self::Abi
     }
 
     #[inline]
     unsafe fn abi_into_raw(abi: Self::Abi, raw: *mut ValRaw) {
-        (*raw).funcref = (abi as usize).to_le();
+        *raw = ValRaw::funcref(abi as usize);
     }
 
     #[inline]

crates/wasmtime/src/values.rs

@@ -103,28 +103,24 @@ impl Val {
     /// [`Func::to_raw`] are unsafe.
     pub unsafe fn to_raw(&self, store: impl AsContextMut) -> ValRaw {
         match self {
-            Val::I32(i) => ValRaw { i32: i.to_le() },
+            Val::I32(i) => ValRaw::i32(*i),
-            Val::I64(i) => ValRaw { i64: i.to_le() },
+            Val::I64(i) => ValRaw::i64(*i),
-            Val::F32(u) => ValRaw { f32: u.to_le() },
+            Val::F32(u) => ValRaw::f32(*u),
-            Val::F64(u) => ValRaw { f64: u.to_le() },
+            Val::F64(u) => ValRaw::f64(*u),
-            Val::V128(b) => ValRaw { v128: b.to_le() },
+            Val::V128(b) => ValRaw::v128(*b),
             Val::ExternRef(e) => {
                 let externref = match e {
                     Some(e) => e.to_raw(store),
                     None => 0,
                 };
-                ValRaw {
+                ValRaw::externref(externref)
-                    externref: externref.to_le(),
-                }
             }
             Val::FuncRef(f) => {
                 let funcref = match f {
                     Some(f) => f.to_raw(store),
                     None => 0,
                 };
-                ValRaw {
+                ValRaw::funcref(funcref)
-                    funcref: funcref.to_le(),
-                }
             }
         }
     }
@@ -138,15 +134,13 @@ impl Val {
     /// otherwise that `raw` should have the type `ty` specified.
     pub unsafe fn from_raw(store: impl AsContextMut, raw: ValRaw, ty: ValType) -> Val {
         match ty {
-            ValType::I32 => Val::I32(i32::from_le(raw.i32)),
+            ValType::I32 => Val::I32(raw.get_i32()),
-            ValType::I64 => Val::I64(i64::from_le(raw.i64)),
+            ValType::I64 => Val::I64(raw.get_i64()),
-            ValType::F32 => Val::F32(u32::from_le(raw.f32)),
+            ValType::F32 => Val::F32(raw.get_f32()),
-            ValType::F64 => Val::F64(u64::from_le(raw.f64)),
+            ValType::F64 => Val::F64(raw.get_f64()),
-            ValType::V128 => Val::V128(u128::from_le(raw.v128)),
+            ValType::V128 => Val::V128(raw.get_v128()),
-            ValType::ExternRef => {
+            ValType::ExternRef => Val::ExternRef(ExternRef::from_raw(raw.get_externref())),
-                Val::ExternRef(ExternRef::from_raw(usize::from_le(raw.externref)))
+            ValType::FuncRef => Val::FuncRef(Func::from_raw(store, raw.get_funcref())),
-            }
-            ValType::FuncRef => Val::FuncRef(Func::from_raw(store, usize::from_le(raw.funcref))),
         }
     }
 

tests/all/call_hook.rs

@@ -52,10 +52,10 @@ fn call_wrapped_func() -> Result<(), Error> {
             |caller: Caller<State>, space| {
                 verify(caller.data());
 
-                assert_eq!((*space.add(0)).i32, 1i32.to_le());
+                assert_eq!((*space.add(0)).get_i32(), 1i32);
-                assert_eq!((*space.add(1)).i64, 2i64.to_le());
+                assert_eq!((*space.add(1)).get_i64(), 2i64);
-                assert_eq!((*space.add(2)).f32, 3.0f32.to_bits().to_le());
+                assert_eq!((*space.add(2)).get_f32(), 3.0f32.to_bits());
-                assert_eq!((*space.add(3)).f64, 4.0f64.to_bits().to_le());
+                assert_eq!((*space.add(3)).get_f64(), 4.0f64.to_bits());
                 Ok(())
             },
         )