Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store the ValRaw type in little-endian format #4035

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions crates/c-api/include/wasmtime/val.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe store char[4], char[8], ... instead? Otherwise people will likely forget to do the endianness transform. Especially as all common architectures are little endian too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to leave this in to avoid the need for funky casts and such in C. This is not intended to be super heavily used either and other language bindings will have to deal with this anyway.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need funky casts either way to support big endian systems.

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;

Expand Down
8 changes: 4 additions & 4 deletions crates/cranelift/src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand Down
65 changes: 65 additions & 0 deletions crates/runtime/src/vmcontext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*ValRaw

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment here is actually intended to point to Val in that ValRaw is the unsafe way of working with what is otherwise a wasm Val value.

/// 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,
}

Expand Down
6 changes: 3 additions & 3 deletions crates/wasmtime/src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1395,7 +1395,7 @@ where
}

unsafe fn wrap_trampoline(ptr: *mut ValRaw, f: impl FnOnce(Self::Retptr) -> Self::Abi) {
*ptr.cast::<Self::Abi>() = f(());
T::abi_into_raw(f(()), ptr);
}

fn into_fallible(self) -> Result<T, Trap> {
Expand Down Expand Up @@ -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);
)*
}
Expand Down Expand Up @@ -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| {
Expand Down
91 changes: 81 additions & 10 deletions crates/wasmtime/src/func/typed.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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]
Expand All @@ -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
}
Expand All @@ -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<ExternRef> {
Expand All @@ -263,6 +314,16 @@ unsafe impl WasmTy for Option<ExternRef> {
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 {
Expand Down Expand Up @@ -339,6 +400,16 @@ unsafe impl WasmTy for Option<Func> {
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 {
Expand Down
34 changes: 20 additions & 14 deletions crates/wasmtime/src/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
}
}
}
Expand All @@ -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))),
}
}

Expand Down
8 changes: 4 additions & 4 deletions tests/all/call_hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ fn call_wrapped_func() -> Result<(), Error> {
|caller: Caller<State>, 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(())
},
)
Expand Down