Skip to content

Commit

Permalink
wasmtime-c-api: Add support for externref values
Browse files Browse the repository at this point in the history
This required that `wasm_val_t` have a `Drop` implementation, an explicit
`Clone` implementation, and no longer be `Copy`, which rippled out through the
crate a bit.

Additionally, `wasm_func_call` and friends were creating references to
uninitialized data for its out pointers and assigning to them. As soon as
`wasm_val_t` gained a `Drop` impl and tried to drop the old value of the
assignment (which is uninitialized data), then things blew up. The fix is to
properly represent the out pointers with `MaybeUninit`, and use `ptr::write` to
initialize them without dropping the old data.

Part of bytecodealliance#929
  • Loading branch information
fitzgen committed Jul 8, 2020
1 parent 2a4f72a commit 62a27e8
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 39 deletions.
17 changes: 12 additions & 5 deletions crates/c-api/src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::{wasm_extern_t, wasm_functype_t, wasm_store_t, wasm_val_t};
use crate::{wasm_name_t, wasm_trap_t, wasmtime_error_t};
use anyhow::anyhow;
use std::ffi::c_void;
use std::mem::MaybeUninit;
use std::panic::{self, AssertUnwindSafe};
use std::ptr;
use std::str;
Expand Down Expand Up @@ -89,6 +90,7 @@ fn create_function(
let func = Func::new(store, ty, move |caller, params, results| {
let params = params
.iter()
.cloned()
.map(|p| wasm_val_t::from_val(p))
.collect::<Vec<_>>();
let mut out_results = vec![wasm_val_t::default(); results.len()];
Expand Down Expand Up @@ -163,7 +165,7 @@ pub extern "C" fn wasmtime_func_new_with_env(
pub unsafe extern "C" fn wasm_func_call(
wasm_func: &wasm_func_t,
args: *const wasm_val_t,
results: *mut wasm_val_t,
results: *mut MaybeUninit<wasm_val_t>,
) -> *mut wasm_trap_t {
let func = wasm_func.func();
let mut trap = ptr::null_mut();
Expand All @@ -186,7 +188,7 @@ pub unsafe extern "C" fn wasmtime_func_call(
func: &wasm_func_t,
args: *const wasm_val_t,
num_args: usize,
results: *mut wasm_val_t,
results: *mut MaybeUninit<wasm_val_t>,
num_results: usize,
trap_ptr: &mut *mut wasm_trap_t,
) -> Option<Box<wasmtime_error_t>> {
Expand All @@ -201,7 +203,7 @@ pub unsafe extern "C" fn wasmtime_func_call(
fn _wasmtime_func_call(
func: &wasm_func_t,
args: &[wasm_val_t],
results: &mut [wasm_val_t],
results: &mut [MaybeUninit<wasm_val_t>],
trap_ptr: &mut *mut wasm_trap_t,
) -> Option<Box<wasmtime_error_t>> {
let func = func.func();
Expand All @@ -217,8 +219,13 @@ fn _wasmtime_func_call(
let result = panic::catch_unwind(AssertUnwindSafe(|| func.call(&params)));
match result {
Ok(Ok(out)) => {
for (slot, val) in results.iter_mut().zip(out.iter()) {
*slot = wasm_val_t::from_val(val);
for (slot, val) in results.iter_mut().zip(out.into_vec().into_iter()) {
unsafe {
// NB: The results array is likely uninitialized memory, so
// use `ptr::write` rather than assignment (which tries to
// run destructors).
ptr::write(slot.as_mut_ptr(), wasm_val_t::from_val(val));
}
}
None
}
Expand Down
10 changes: 8 additions & 2 deletions crates/c-api/src/global.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{handle_result, wasmtime_error_t};
use crate::{wasm_extern_t, wasm_globaltype_t, wasm_store_t, wasm_val_t};
use std::mem::MaybeUninit;
use std::ptr;
use wasmtime::{Extern, Global};

Expand Down Expand Up @@ -72,8 +73,13 @@ pub extern "C" fn wasm_global_type(g: &wasm_global_t) -> Box<wasm_globaltype_t>
}

#[no_mangle]
pub extern "C" fn wasm_global_get(g: &wasm_global_t, out: &mut wasm_val_t) {
out.set(g.global().get());
pub extern "C" fn wasm_global_get(g: &wasm_global_t, out: &mut MaybeUninit<wasm_val_t>) {
unsafe {
ptr::write(
out.as_mut_ptr(),
wasm_val_t::from_val(g.global().get()),
);
}
}

#[no_mangle]
Expand Down
87 changes: 55 additions & 32 deletions crates/c-api/src/val.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use crate::{from_valtype, into_valtype, wasm_ref_t, wasm_valkind_t, WASM_I32};
use std::mem::MaybeUninit;
use std::ptr;
use wasmtime::{Val, ValType};

#[repr(C)]
#[derive(Copy, Clone)]
pub struct wasm_val_t {
pub kind: wasm_valkind_t,
pub of: wasm_val_union,
Expand All @@ -20,6 +21,34 @@ pub union wasm_val_union {
pub ref_: *mut wasm_ref_t,
}

impl Drop for wasm_val_t {
fn drop(&mut self) {
match into_valtype(self.kind) {
ValType::ExternRef => unsafe {
drop(Box::from_raw(self.of.ref_));
},
_ => {}
}
}
}

impl Clone for wasm_val_t {
fn clone(&self) -> Self {
match into_valtype(self.kind) {
ValType::ExternRef => wasm_val_t {
kind: self.kind,
of: wasm_val_union {
ref_: unsafe { Box::into_raw(Box::new((*self.of.ref_).clone())) },
},
},
_ => wasm_val_t {
kind: self.kind,
of: self.of,
},
}
}
}

impl Default for wasm_val_t {
fn default() -> Self {
wasm_val_t {
Expand All @@ -30,48 +59,36 @@ impl Default for wasm_val_t {
}

impl wasm_val_t {
pub fn from_val(val: &Val) -> wasm_val_t {
pub fn from_val(val: Val) -> wasm_val_t {
match val {
Val::I32(i) => wasm_val_t {
kind: from_valtype(&ValType::I32),
of: wasm_val_union { i32: *i },
of: wasm_val_union { i32: i },
},
Val::I64(i) => wasm_val_t {
kind: from_valtype(&ValType::I64),
of: wasm_val_union { i64: *i },
of: wasm_val_union { i64: i },
},
Val::F32(f) => wasm_val_t {
kind: from_valtype(&ValType::F32),
of: wasm_val_union { u32: *f },
of: wasm_val_union { u32: f },
},
Val::F64(f) => wasm_val_t {
kind: from_valtype(&ValType::F64),
of: wasm_val_union { u64: *f },
of: wasm_val_union { u64: f },
},
Val::ExternRef(r) => wasm_val_t {
kind: from_valtype(&ValType::ExternRef),
of: wasm_val_union {
ref_: Box::into_raw(Box::new(wasm_ref_t { r })),
},
},
_ => unimplemented!("wasm_val_t::from_val {:?}", val),
}
}

pub fn set(&mut self, val: Val) {
match val {
Val::I32(i) => {
self.kind = from_valtype(&ValType::I32);
self.of = wasm_val_union { i32: i };
}
Val::I64(i) => {
self.kind = from_valtype(&ValType::I64);
self.of = wasm_val_union { i64: i };
}
Val::F32(f) => {
self.kind = from_valtype(&ValType::F32);
self.of = wasm_val_union { u32: f };
}
Val::F64(f) => {
self.kind = from_valtype(&ValType::F64);
self.of = wasm_val_union { u64: f };
}
_ => unimplemented!("wasm_val_t::from_val {:?}", val),
}
*self = Self::from_val(val);
}

pub fn val(&self) -> Val {
Expand All @@ -80,20 +97,26 @@ impl wasm_val_t {
ValType::I64 => Val::from(unsafe { self.of.i64 }),
ValType::F32 => Val::from(unsafe { self.of.f32 }),
ValType::F64 => Val::from(unsafe { self.of.f64 }),
ValType::ExternRef => Val::ExternRef(unsafe { (*self.of.ref_).r.clone() }),
_ => unimplemented!("wasm_val_t::val {:?}", self.kind),
}
}
}

#[no_mangle]
pub unsafe extern "C" fn wasm_val_copy(out: *mut wasm_val_t, source: &wasm_val_t) {
*out = match into_valtype(source.kind) {
ValType::I32 | ValType::I64 | ValType::F32 | ValType::F64 => *source,
_ => unimplemented!("wasm_val_copy arg"),
};
pub unsafe extern "C" fn wasm_val_copy(out: &mut MaybeUninit<wasm_val_t>, source: &wasm_val_t) {
ptr::write(
out.as_mut_ptr(),
match into_valtype(source.kind) {
ValType::I32 | ValType::I64 | ValType::F32 | ValType::F64 | ValType::ExternRef => {
source.clone()
}
_ => unimplemented!("wasm_val_copy arg"),
},
);
}

#[no_mangle]
pub extern "C" fn wasm_val_delete(_val: &mut wasm_val_t) {
// currently we only support integers/floats which need no deletion
pub unsafe extern "C" fn wasm_val_delete(val: &mut wasm_val_t) {
ptr::drop_in_place(val);
}

0 comments on commit 62a27e8

Please sign in to comment.