From de637d106ebc5d971b97d504d98f0e2376b5a8b9 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 14 Apr 2022 08:25:15 -0700 Subject: [PATCH 1/2] Store the `ValRaw` type in little-endian format This commit changes the internal representation of the `ValRaw` type to an unconditionally little-endian format instead of its current native-endian format. The documentation and various accessors here have been updated as well as the associated trampolines that read `ValRaw` to always work with little-endian values, converting to the host endianness as necessary. The motivation for this change originally comes from the implementation of the component model that I'm working on. One aspect of the component model's canonical ABI is how variants are passed to functions as immediate arguments. For example for a component model function: ``` foo: function(x: expected) ``` This translates to a core wasm function: ```wasm (module (func (export "foo") (param i32 i64) ;; ... ) ) ``` The first `i32` parameter to the core wasm function is the discriminant of whether the result is an "ok" or an "err". The second `i64`, however, is the "join" operation on the `i32` and `f64` payloads. Essentially these two types are unioned into one type to get passed into the function. Currently in the implementation of the component model my plan is to construct a `*mut [ValRaw]` to pass through to WebAssembly, always invoking component exports through host trampolines. This means that the implementation for `Result` needs to do the correct "join" operation here when encoding a particular case into the corresponding `ValRaw`. I personally found this particularly tricky to do structurally. The solution that I settled on with fitzgen was that if `ValRaw` was always stored in a little endian format then we could employ a trick where when encoding a variant we first set all the `ValRaw` slots to zero, then the associated case we have is encoding. Afterwards the `ValRaw` values are already encoded into the correct format as if they'd been "join"ed. For example if we were to encode `Ok(1i32)` then this would produce `ValRaw { i32: 1 }`, which memory-wise is equivalent to `ValRaw { i64: 1 }` if the other bytes in the `ValRaw` are guaranteed to be zero. Similarly storing `ValRaw { f64 }` is equivalent to the storage required for `ValRaw { i64 }` here in the join operation. Note, though, that this equivalence relies on everything being little-endian. Otherwise the in-memory representations of `ValRaw { i32: 1 }` and `ValRaw { i64: 1 }` are different. That motivation is what leads to this change. It's expected that this is a low-to-zero cost change in the sense that little-endian platforms will see no change and big-endian platforms are already required to efficiently byte-swap loads/stores as WebAssembly requires that. Additionally the `ValRaw` type is an esoteric niche use case primarily used for accelerating the C API right now, so it's expected that not many users will have to update for this change. --- crates/c-api/include/wasmtime/val.h | 14 +++++++ crates/cranelift/src/compiler.rs | 8 ++-- crates/runtime/src/vmcontext.rs | 65 +++++++++++++++++++++++++++++ crates/wasmtime/src/values.rs | 34 ++++++++------- 4 files changed, 103 insertions(+), 18 deletions(-) diff --git a/crates/c-api/include/wasmtime/val.h b/crates/c-api/include/wasmtime/val.h index c17f605cd82b..ae0a1961cea0 100644 --- a/crates/c-api/include/wasmtime/val.h +++ b/crates/c-api/include/wasmtime/val.h @@ -119,24 +119,38 @@ typedef uint8_t wasmtime_v128[16]; */ typedef union wasmtime_valunion { /// Field used if #wasmtime_val_t::kind is #WASMTIME_I32 + /// + /// Note that this field is always stored in a little-endian format. int32_t i32; /// Field used if #wasmtime_val_t::kind is #WASMTIME_I64 + /// + /// Note that this field is always stored in a little-endian format. int64_t i64; /// Field used if #wasmtime_val_t::kind is #WASMTIME_F32 + /// + /// Note that this field is always stored in a little-endian format. float32_t f32; /// Field used if #wasmtime_val_t::kind is #WASMTIME_F64 + /// + /// Note that this field is always stored in a little-endian format. float64_t f64; /// Field used if #wasmtime_val_t::kind is #WASMTIME_FUNCREF /// /// If this value represents a `ref.null func` value then the `store_id` field /// is set to zero. + /// + /// Note that this field is always stored in a little-endian format. wasmtime_func_t funcref; /// Field used if #wasmtime_val_t::kind is #WASMTIME_EXTERNREF /// /// If this value represents a `ref.null extern` value then this pointer will /// be `NULL`. + /// + /// Note that this field is always stored in a little-endian format. wasmtime_externref_t *externref; /// Field used if #wasmtime_val_t::kind is #WASMTIME_V128 + /// + /// Note that this field is always stored in a little-endian format. wasmtime_v128 v128; } wasmtime_valunion_t; diff --git a/crates/cranelift/src/compiler.rs b/crates/cranelift/src/compiler.rs index 967605de2089..b0739f0d36b1 100644 --- a/crates/cranelift/src/compiler.rs +++ b/crates/cranelift/src/compiler.rs @@ -427,7 +427,8 @@ impl Compiler { }; // Load the argument values out of `values_vec`. - let mflags = ir::MemFlags::trusted(); + let mut mflags = ir::MemFlags::trusted(); + mflags.set_endianness(ir::Endianness::Little); let callee_args = wasm_signature .params .iter() @@ -458,7 +459,6 @@ impl Compiler { let results = builder.func.dfg.inst_results(call).to_vec(); // Store the return values into `values_vec`. - let mflags = ir::MemFlags::trusted(); for (i, r) in results.iter().enumerate() { builder .ins() @@ -507,7 +507,8 @@ impl Compiler { builder.seal_block(block0); let values_vec_ptr_val = builder.ins().stack_addr(pointer_type, ss, 0); - let mflags = MemFlags::trusted(); + let mut mflags = MemFlags::trusted(); + mflags.set_endianness(ir::Endianness::Little); for i in 0..ty.params().len() { let val = builder.func.dfg.block_params(block0)[i + 2]; builder @@ -528,7 +529,6 @@ impl Compiler { .ins() .call_indirect(new_sig, callee_value, &callee_args); - let mflags = MemFlags::trusted(); let mut results = Vec::new(); for (i, r) in ty.returns().iter().enumerate() { let load = builder.ins().load( diff --git a/crates/runtime/src/vmcontext.rs b/crates/runtime/src/vmcontext.rs index d4bd2290ff75..c670e3d9972d 100644 --- a/crates/runtime/src/vmcontext.rs +++ b/crates/runtime/src/vmcontext.rs @@ -778,16 +778,81 @@ impl VMContext { /// This is provided for use with the `Func::new_unchecked` and /// `Func::call_unchecked` APIs. In general it's unlikely you should be using /// this from Rust, rather using APIs like `Func::wrap` and `TypedFunc::call`. +/// +/// This is notably an "unsafe" way to work with `Val` and it's recommended to +/// instead use `Val` where possible. An important note about this union is that +/// fields are all stored in little-endian format, regardless of the endianness +/// of the host system. #[allow(missing_docs)] #[repr(C)] #[derive(Copy, Clone)] pub union ValRaw { + /// A WebAssembly `i32` value. + /// + /// Note that the payload here is a Rust `i32` but the WebAssembly `i32` + /// type does not assign an interpretation of the upper bit as either signed + /// or unsigned. The Rust type `i32` is simply chosen for convenience. + /// + /// This value is always stored in a little-endian format. pub i32: i32, + + /// A WebAssembly `i64` value. + /// + /// Note that the payload here is a Rust `i64` but the WebAssembly `i64` + /// type does not assign an interpretation of the upper bit as either signed + /// or unsigned. The Rust type `i64` is simply chosen for convenience. + /// + /// This value is always stored in a little-endian format. pub i64: i64, + + /// A WebAssembly `f32` value. + /// + /// Note that the payload here is a Rust `u32`. This is to allow passing any + /// representation of NaN into WebAssembly without risk of changing NaN + /// payload bits as its gets passed around the system. Otherwise though this + /// `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, + + /// A WebAssembly `f64` value. + /// + /// Note that the payload here is a Rust `u64`. This is to allow passing any + /// representation of NaN into WebAssembly without risk of changing NaN + /// payload bits as its gets passed around the system. Otherwise though this + /// `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, + + /// A WebAssembly `v128` value. + /// + /// The payload here is a Rust `u128` which has the same number of bits but + /// note that `v128` in WebAssembly is often considered a vector type such + /// as `i32x4` or `f64x2`. This means that the actual interpretation of the + /// 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, + + /// A WebAssembly `funcref` value. + /// + /// The payload here is a pointer which is runtime-defined. This is one of + /// the main points of unsafety about the `ValRaw` type as the validity of + /// the pointer here is not easily verified and must be preserved by + /// carefully calling the correct functions throughout the runtime. + /// + /// This value is always stored in a little-endian format. pub funcref: usize, + + /// A WebAssembly `externref` value. + /// + /// The payload here is a pointer which is runtime-defined. This is one of + /// the main points of unsafety about the `ValRaw` type as the validity of + /// the pointer here is not easily verified and must be preserved by + /// carefully calling the correct functions throughout the runtime. + /// + /// This value is always stored in a little-endian format. pub externref: usize, } diff --git a/crates/wasmtime/src/values.rs b/crates/wasmtime/src/values.rs index c9b10a8190e3..45d1b10bc12e 100644 --- a/crates/wasmtime/src/values.rs +++ b/crates/wasmtime/src/values.rs @@ -103,24 +103,28 @@ 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 }, - Val::I64(i) => ValRaw { i64: *i }, - Val::F32(u) => ValRaw { f32: *u }, - Val::F64(u) => ValRaw { f64: *u }, - Val::V128(b) => ValRaw { v128: *b }, + Val::I32(i) => ValRaw { i32: i.to_le() }, + Val::I64(i) => ValRaw { i64: i.to_le() }, + Val::F32(u) => ValRaw { f32: u.to_le() }, + Val::F64(u) => ValRaw { f64: u.to_le() }, + Val::V128(b) => ValRaw { v128: b.to_le() }, Val::ExternRef(e) => { let externref = match e { Some(e) => e.to_raw(store), None => 0, }; - ValRaw { externref } + ValRaw { + externref: externref.to_le(), + } } Val::FuncRef(f) => { let funcref = match f { Some(f) => f.to_raw(store), None => 0, }; - ValRaw { funcref } + ValRaw { + funcref: funcref.to_le(), + } } } } @@ -134,13 +138,15 @@ 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(raw.i32), - ValType::I64 => Val::I64(raw.i64), - ValType::F32 => Val::F32(raw.f32), - ValType::F64 => Val::F64(raw.f64), - ValType::V128 => Val::V128(raw.v128), - ValType::ExternRef => Val::ExternRef(ExternRef::from_raw(raw.externref)), - ValType::FuncRef => Val::FuncRef(Func::from_raw(store, raw.funcref)), + ValType::I32 => Val::I32(i32::from_le(raw.i32)), + ValType::I64 => Val::I64(i64::from_le(raw.i64)), + ValType::F32 => Val::F32(u32::from_le(raw.f32)), + ValType::F64 => Val::F64(u64::from_le(raw.f64)), + ValType::V128 => Val::V128(u128::from_le(raw.v128)), + ValType::ExternRef => { + Val::ExternRef(ExternRef::from_raw(usize::from_le(raw.externref))) + } + ValType::FuncRef => Val::FuncRef(Func::from_raw(store, usize::from_le(raw.funcref))), } } From f26673474e5c98d1e2fddc21673ba167c5e7851c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 14 Apr 2022 09:38:32 -0700 Subject: [PATCH 2/2] Track down some more endianness conversions --- crates/wasmtime/src/func.rs | 6 +- crates/wasmtime/src/func/typed.rs | 91 +++++++++++++++++++++++++++---- tests/all/call_hook.rs | 8 +-- 3 files changed, 88 insertions(+), 17 deletions(-) diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index 4142b2322e8d..832aa99b13a9 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -1395,7 +1395,7 @@ where } unsafe fn wrap_trampoline(ptr: *mut ValRaw, f: impl FnOnce(Self::Retptr) -> Self::Abi) { - *ptr.cast::() = f(()); + T::abi_into_raw(f(()), ptr); } fn into_fallible(self) -> Result { @@ -1483,7 +1483,7 @@ macro_rules! impl_wasm_host_results { unsafe fn wrap_trampoline(mut _ptr: *mut ValRaw, f: impl FnOnce(Self::Retptr) -> Self::Abi) { let ($($t,)*) = <($($t::Abi,)*) as HostAbi>::call(f); $( - *_ptr.cast() = $t; + $t::abi_into_raw($t, _ptr); _ptr = _ptr.add(1); )* } @@ -1936,7 +1936,7 @@ macro_rules! impl_into_func { let mut _n = 0; $( - let $args = *args.add(_n).cast::<$args::Abi>(); + let $args = $args::abi_from_raw(args.add(_n)); _n += 1; )* R::wrap_trampoline(args, |retptr| { diff --git a/crates/wasmtime/src/func/typed.rs b/crates/wasmtime/src/func/typed.rs index 4ddced224d1e..f503c2e57dc0 100644 --- a/crates/wasmtime/src/func/typed.rs +++ b/crates/wasmtime/src/func/typed.rs @@ -1,6 +1,6 @@ use super::{invoke_wasm_and_catch_traps, HostAbi}; use crate::store::{AutoAssertNoGc, StoreOpaque}; -use crate::{AsContextMut, ExternRef, Func, StoreContextMut, Trap, ValType}; +use crate::{AsContextMut, ExternRef, Func, StoreContextMut, Trap, ValRaw, ValType}; use anyhow::{bail, Result}; use std::marker; use std::mem::{self, MaybeUninit}; @@ -203,13 +203,17 @@ pub unsafe trait WasmTy: Send { #[doc(hidden)] fn is_externref(&self) -> bool; #[doc(hidden)] + unsafe fn abi_from_raw(raw: *mut ValRaw) -> Self::Abi; + #[doc(hidden)] + unsafe fn abi_into_raw(abi: Self::Abi, raw: *mut ValRaw); + #[doc(hidden)] fn into_abi(self, store: &mut StoreOpaque) -> Self::Abi; #[doc(hidden)] unsafe fn from_abi(abi: Self::Abi, store: &mut StoreOpaque) -> Self; } -macro_rules! primitives { - ($($primitive:ident => $ty:ident)*) => ($( +macro_rules! integers { + ($($primitive:ident => $ty:ident in $raw:ident)*) => ($( unsafe impl WasmTy for $primitive { type Abi = $primitive; #[inline] @@ -225,6 +229,57 @@ macro_rules! primitives { false } #[inline] + unsafe fn abi_from_raw(raw: *mut ValRaw) -> $primitive { + $primitive::from_le((*raw).$raw as $primitive) + } + #[inline] + unsafe fn abi_into_raw(abi: $primitive, raw: *mut ValRaw) { + (*raw).$raw = abi.to_le() as $raw; + } + #[inline] + fn into_abi(self, _store: &mut StoreOpaque) -> Self::Abi { + self + } + #[inline] + unsafe fn from_abi(abi: Self::Abi, _store: &mut StoreOpaque) -> Self { + abi + } + } + )*) +} + +integers! { + i32 => I32 in i32 + i64 => I64 in i64 + u32 => I32 in i32 + u64 => I64 in i64 +} + +macro_rules! floats { + ($($float:ident/$int:ident => $ty:ident)*) => ($( + unsafe impl WasmTy for $float { + type Abi = $float; + #[inline] + fn valtype() -> ValType { + ValType::$ty + } + #[inline] + fn compatible_with_store(&self, _: &StoreOpaque) -> bool { + true + } + #[inline] + fn is_externref(&self) -> bool { + false + } + #[inline] + unsafe fn abi_from_raw(raw: *mut ValRaw) -> $float { + $float::from_bits($int::from_le((*raw).$float)) + } + #[inline] + unsafe fn abi_into_raw(abi: $float, raw: *mut ValRaw) { + (*raw).$float = abi.to_bits().to_le(); + } + #[inline] fn into_abi(self, _store: &mut StoreOpaque) -> Self::Abi { self } @@ -236,13 +291,9 @@ macro_rules! primitives { )*) } -primitives! { - i32 => I32 - u32 => I32 - i64 => I64 - u64 => I64 - f32 => F32 - f64 => F64 +floats! { + f32/u32 => F32 + f64/u64 => F64 } unsafe impl WasmTy for Option { @@ -263,6 +314,16 @@ unsafe impl WasmTy for Option { true } + #[inline] + unsafe fn abi_from_raw(raw: *mut ValRaw) -> *mut u8 { + usize::from_le((*raw).externref) as *mut u8 + } + + #[inline] + unsafe fn abi_into_raw(abi: *mut u8, raw: *mut ValRaw) { + (*raw).externref = (abi as usize).to_le(); + } + #[inline] fn into_abi(self, store: &mut StoreOpaque) -> Self::Abi { if let Some(x) = self { @@ -339,6 +400,16 @@ unsafe impl WasmTy for Option { false } + #[inline] + unsafe fn abi_from_raw(raw: *mut ValRaw) -> Self::Abi { + usize::from_le((*raw).funcref) as Self::Abi + } + + #[inline] + unsafe fn abi_into_raw(abi: Self::Abi, raw: *mut ValRaw) { + (*raw).funcref = (abi as usize).to_le(); + } + #[inline] fn into_abi(self, store: &mut StoreOpaque) -> Self::Abi { if let Some(f) = self { diff --git a/tests/all/call_hook.rs b/tests/all/call_hook.rs index 2ea6ffcb43ed..927e8eaae930 100644 --- a/tests/all/call_hook.rs +++ b/tests/all/call_hook.rs @@ -52,10 +52,10 @@ fn call_wrapped_func() -> Result<(), Error> { |caller: Caller, space| { verify(caller.data()); - assert_eq!((*space.add(0)).i32, 1); - assert_eq!((*space.add(1)).i64, 2); - assert_eq!((*space.add(2)).f32, 3.0f32.to_bits()); - assert_eq!((*space.add(3)).f64, 4.0f64.to_bits()); + assert_eq!((*space.add(0)).i32, 1i32.to_le()); + assert_eq!((*space.add(1)).i64, 2i64.to_le()); + assert_eq!((*space.add(2)).f32, 3.0f32.to_bits().to_le()); + assert_eq!((*space.add(3)).f64, 4.0f64.to_bits().to_le()); Ok(()) }, )