From 01a92aef95fceed5e1913c26bf774c80b8ae9ed0 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 20 May 2020 13:41:34 -0700 Subject: [PATCH 01/14] cranelift_wasm: Use the `TableIndex` type instead of raw `u32` About half of the `FuncEnvironment::translate_table_*` methods were using the `TableIndex` newtype, while the other half were using raw `u32`s. This commit makes everything use `TableIndex`. --- cranelift/wasm/src/code_translator.rs | 12 ++++++++---- cranelift/wasm/src/environ/dummy.rs | 8 ++++---- cranelift/wasm/src/environ/spec.rs | 8 ++++---- crates/environ/src/func_environ.rs | 8 ++++---- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/cranelift/wasm/src/code_translator.rs b/cranelift/wasm/src/code_translator.rs index a8db0433bc..4e740b99e4 100644 --- a/cranelift/wasm/src/code_translator.rs +++ b/cranelift/wasm/src/code_translator.rs @@ -1171,23 +1171,26 @@ pub fn translate_operator( )?); } Operator::TableGrow { table } => { + let table_index = TableIndex::from_u32(*table); let delta = state.pop1(); let init_value = state.pop1(); state.push1(environ.translate_table_grow( builder.cursor(), - *table, + table_index, delta, init_value, )?); } Operator::TableGet { table } => { + let table_index = TableIndex::from_u32(*table); let index = state.pop1(); - state.push1(environ.translate_table_get(builder.cursor(), *table, index)?); + state.push1(environ.translate_table_get(builder.cursor(), table_index, index)?); } Operator::TableSet { table } => { + let table_index = TableIndex::from_u32(*table); let value = state.pop1(); let index = state.pop1(); - environ.translate_table_set(builder.cursor(), *table, value, index)?; + environ.translate_table_set(builder.cursor(), table_index, value, index)?; } Operator::TableCopy { dst_table: dst_table_index, @@ -1210,10 +1213,11 @@ pub fn translate_operator( )?; } Operator::TableFill { table } => { + let table_index = TableIndex::from_u32(*table); let len = state.pop1(); let val = state.pop1(); let dest = state.pop1(); - environ.translate_table_fill(builder.cursor(), *table, dest, val, len)?; + environ.translate_table_fill(builder.cursor(), table_index, dest, val, len)?; } Operator::TableInit { segment, diff --git a/cranelift/wasm/src/environ/dummy.rs b/cranelift/wasm/src/environ/dummy.rs index cf8b997c84..d5c9d63e40 100644 --- a/cranelift/wasm/src/environ/dummy.rs +++ b/cranelift/wasm/src/environ/dummy.rs @@ -433,7 +433,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ fn translate_table_grow( &mut self, mut pos: FuncCursor, - _table_index: u32, + _table_index: TableIndex, _delta: ir::Value, _init_value: ir::Value, ) -> WasmResult { @@ -443,7 +443,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ fn translate_table_get( &mut self, mut pos: FuncCursor, - _table_index: u32, + _table_index: TableIndex, _index: ir::Value, ) -> WasmResult { Ok(pos.ins().null(self.reference_type())) @@ -452,7 +452,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ fn translate_table_set( &mut self, _pos: FuncCursor, - _table_index: u32, + _table_index: TableIndex, _value: ir::Value, _index: ir::Value, ) -> WasmResult<()> { @@ -476,7 +476,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ fn translate_table_fill( &mut self, _pos: FuncCursor, - _table_index: u32, + _table_index: TableIndex, _dst: ir::Value, _val: ir::Value, _len: ir::Value, diff --git a/cranelift/wasm/src/environ/spec.rs b/cranelift/wasm/src/environ/spec.rs index 6788599797..aeec47cede 100644 --- a/cranelift/wasm/src/environ/spec.rs +++ b/cranelift/wasm/src/environ/spec.rs @@ -347,7 +347,7 @@ pub trait FuncEnvironment: TargetEnvironment { fn translate_table_grow( &mut self, pos: FuncCursor, - table_index: u32, + table_index: TableIndex, delta: ir::Value, init_value: ir::Value, ) -> WasmResult; @@ -356,7 +356,7 @@ pub trait FuncEnvironment: TargetEnvironment { fn translate_table_get( &mut self, pos: FuncCursor, - table_index: u32, + table_index: TableIndex, index: ir::Value, ) -> WasmResult; @@ -364,7 +364,7 @@ pub trait FuncEnvironment: TargetEnvironment { fn translate_table_set( &mut self, pos: FuncCursor, - table_index: u32, + table_index: TableIndex, value: ir::Value, index: ir::Value, ) -> WasmResult<()>; @@ -387,7 +387,7 @@ pub trait FuncEnvironment: TargetEnvironment { fn translate_table_fill( &mut self, pos: FuncCursor, - table_index: u32, + table_index: TableIndex, dst: ir::Value, val: ir::Value, len: ir::Value, diff --git a/crates/environ/src/func_environ.rs b/crates/environ/src/func_environ.rs index 6433a203e9..6ca501eb9a 100644 --- a/crates/environ/src/func_environ.rs +++ b/crates/environ/src/func_environ.rs @@ -727,7 +727,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m fn translate_table_grow( &mut self, _: cranelift_codegen::cursor::FuncCursor<'_>, - _: u32, + _: TableIndex, _: ir::Value, _: ir::Value, ) -> WasmResult { @@ -739,7 +739,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m fn translate_table_get( &mut self, _: cranelift_codegen::cursor::FuncCursor<'_>, - _: u32, + _: TableIndex, _: ir::Value, ) -> WasmResult { Err(WasmError::Unsupported( @@ -750,7 +750,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m fn translate_table_set( &mut self, _: cranelift_codegen::cursor::FuncCursor<'_>, - _: u32, + _: TableIndex, _: ir::Value, _: ir::Value, ) -> WasmResult<()> { @@ -762,7 +762,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m fn translate_table_fill( &mut self, _: cranelift_codegen::cursor::FuncCursor<'_>, - _: u32, + _: TableIndex, _: ir::Value, _: ir::Value, _: ir::Value, From 1898d529660a623917d74093d1ef4448ef792f12 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 21 May 2020 11:30:02 -0700 Subject: [PATCH 02/14] runtime: Introduce `VMExternRef` and `VMExternData` `VMExternRef` is a reference-counted box for any kind of data that is external and opaque to running Wasm. Sometimes it might hold a Wasmtime thing, other times it might hold something from a Wasmtime embedder and is opaque even to us. It is morally equivalent to `Rc` in Rust, but additionally always fits in a pointer-sized word. `VMExternRef` is non-nullable, but `Option` is a null pointer. The one part of `VMExternRef` that can't ever be opaque to us is the reference count. Even when we don't know what's inside an `VMExternRef`, we need to be able to manipulate its reference count as we add and remove references to it. And we need to do this from compiled Wasm code, so it must be `repr(C)`! `VMExternRef` itself is just a pointer to an `VMExternData`, which holds the opaque, boxed value, its reference count, and its vtable pointer. The `VMExternData` struct is *preceded* by the dynamically-sized value boxed up and referenced by one or more `VMExternRef`s: ```ignore ,-------------------------------------------------------. | | V | +----------------------------+-----------+-----------+ | | dynamically-sized value... | ref_count | value_ptr |---' +----------------------------+-----------+-----------+ | VMExternData | +-----------------------+ ^ +-------------+ | | VMExternRef |-------------------+ +-------------+ | | +-------------+ | | VMExternRef |-------------------+ +-------------+ | | ... === | +-------------+ | | VMExternRef |-------------------' +-------------+ ``` The `value_ptr` member always points backwards to the start of the dynamically-sized value (which is also the start of the heap allocation for this value-and-`VMExternData` pair). Because it is a `dyn` pointer, it is fat, and also points to the value's `Any` vtable. The boxed value and the `VMExternRef` footer are held a single heap allocation. The layout described above is used to make satisfying the value's alignment easy: we just need to ensure that the heap allocation used to hold everything satisfies its alignment. It also ensures that we don't need a ton of excess padding between the `VMExternData` and the value for values with large alignment. --- crates/environ/src/vmoffsets.rs | 8 + crates/runtime/src/externref.rs | 430 ++++++++++++++++++++++++++++++++ crates/runtime/src/lib.rs | 2 + 3 files changed, 440 insertions(+) create mode 100644 crates/runtime/src/externref.rs diff --git a/crates/environ/src/vmoffsets.rs b/crates/environ/src/vmoffsets.rs index 3988198df0..940792557a 100644 --- a/crates/environ/src/vmoffsets.rs +++ b/crates/environ/src/vmoffsets.rs @@ -583,6 +583,14 @@ impl VMOffsets { } } +/// Offsets for `VMExternData`. +impl VMOffsets { + /// Return the offset for `VMExternData::ref_count`. + pub fn vm_extern_data_ref_count() -> u32 { + 0 + } +} + /// Target specific type for shared signature index. #[derive(Debug, Copy, Clone)] pub struct TargetSharedSignatureIndex(u32); diff --git a/crates/runtime/src/externref.rs b/crates/runtime/src/externref.rs new file mode 100644 index 0000000000..f7f085fe67 --- /dev/null +++ b/crates/runtime/src/externref.rs @@ -0,0 +1,430 @@ +//! # `VMExternRef` +//! +//! `VMExternRef` is a reference-counted box for any kind of data that is +//! external and opaque to running Wasm. Sometimes it might hold a Wasmtime +//! thing, other times it might hold something from a Wasmtime embedder and is +//! opaque even to us. It is morally equivalent to `Rc` in Rust, but +//! additionally always fits in a pointer-sized word. `VMExternRef` is +//! non-nullable, but `Option` is a null pointer. +//! +//! The one part of `VMExternRef` that can't ever be opaque to us is the +//! reference count. Even when we don't know what's inside an `VMExternRef`, we +//! need to be able to manipulate its reference count as we add and remove +//! references to it. And we need to do this from compiled Wasm code, so it must +//! be `repr(C)`! +//! +//! ## Memory Layout +//! +//! `VMExternRef` itself is just a pointer to an `VMExternData`, which holds the +//! opaque, boxed value, its reference count, and its vtable pointer. +//! +//! The `VMExternData` struct is *preceded* by the dynamically-sized value boxed +//! up and referenced by one or more `VMExternRef`s: +//! +//! ```ignore +//! ,-------------------------------------------------------. +//! | | +//! V | +//! +----------------------------+-----------+-----------+ | +//! | dynamically-sized value... | ref_count | value_ptr |---' +//! +----------------------------+-----------+-----------+ +//! | VMExternData | +//! +-----------------------+ +//! ^ +//! +-------------+ | +//! | VMExternRef |-------------------+ +//! +-------------+ | +//! | +//! +-------------+ | +//! | VMExternRef |-------------------+ +//! +-------------+ | +//! | +//! ... === +//! | +//! +-------------+ | +//! | VMExternRef |-------------------' +//! +-------------+ +//! ``` +//! +//! The `value_ptr` member always points backwards to the start of the +//! dynamically-sized value (which is also the start of the heap allocation for +//! this value-and-`VMExternData` pair). Because it is a `dyn` pointer, it is +//! fat, and also points to the value's `Any` vtable. +//! +//! The boxed value and the `VMExternRef` footer are held a single heap +//! allocation. The layout described above is used to make satisfying the +//! value's alignment easy: we just need to ensure that the heap allocation used +//! to hold everything satisfies its alignment. It also ensures that we don't +//! need a ton of excess padding between the `VMExternData` and the value for +//! values with large alignment. + +use std::alloc::Layout; +use std::any::Any; +use std::cell::UnsafeCell; +use std::cmp::Ordering; +use std::hash::{Hash, Hasher}; +use std::mem; +use std::ops::Deref; +use std::ptr::{self, NonNull}; + +/// An external reference to some opaque data. +/// +/// `VMExternRef`s dereference to their underlying opaque data as `dyn Any`. +/// +/// Unlike the `externref` in the Wasm spec, `VMExternRef`s are non-nullable, +/// and always point to a valid value. You may use `Option` to +/// represent nullable references, and `Option` is guaranteed to +/// have the same size and alignment as a raw pointer, with `None` represented +/// with the null pointer. +/// +/// `VMExternRef`s are reference counted, so cloning is a cheap, shallow +/// operation. It also means they are inherently shared, so you may not get a +/// mutable, exclusive reference to their inner contents, only a shared, +/// immutable reference. You may use interior mutability with `RefCell` or +/// `Mutex` to work around this restriction, if necessary. +/// +/// `VMExternRef`s have pointer-equality semantics, not structural-equality +/// semantics. Given two `VMExternRef`s `a` and `b`, `a == b` only if `a` and +/// `b` point to the same allocation. `a` and `b` are considered not equal, even +/// if `a` and `b` are two different identical copies of the same data, if they +/// are in two different allocations. The hashing and ordering implementations +/// also only operate on the pointer. +/// +/// # Example +/// +/// ``` +/// # fn foo() -> Result<(), Box> { +/// use std::cell::RefCell; +/// use wasmtime_runtime::VMExternRef; +/// +/// // Open a file. Wasm doesn't know about files, but we can let Wasm instances +/// // work with files via opaque `externref` handles. +/// let file = std::fs::File::create("some/file/path")?; +/// +/// // Wrap the file up as an `VMExternRef` that can be passed to Wasm. +/// let extern_ref_to_file = VMExternRef::new(RefCell::new(file)); +/// +/// // `VMExternRef`s dereference to `dyn Any`, so you can use `Any` methods to +/// // perform runtime type checks and downcasts. +/// +/// assert!(extern_ref_to_file.is::>()); +/// assert!(!extern_ref_to_file.is::()); +/// +/// if let Some(file) = extern_ref_to_file.downcast_ref::>() { +/// use std::io::Write; +/// let mut file = file.borrow_mut(); +/// writeln!(&mut file, "Hello, `VMExternRef`!")?; +/// } +/// # Ok(()) +/// # } +/// ``` +#[derive(Debug)] +#[repr(transparent)] +pub struct VMExternRef(NonNull); + +#[repr(C)] +struct VMExternData { + // Implicit, dynamically-sized member that always preceded an + // `VMExternData`. + // + // value: [u8], + // + /// The reference count for this `VMExternData` and value. When it reaches + /// zero, we can safely destroy the value and free this heap + /// allocation. This is an `UnsafeCell`, rather than plain `Cell`, because + /// it can be modified by compiled Wasm code. + /// + /// Note: this field's offset must be kept in sync with + /// `wasmtime_environ::VMOffsets::vm_extern_data_ref_count()` which is + /// currently always zero. + ref_count: UnsafeCell, + + /// Always points to the implicit, dynamically-sized `value` member that + /// precedes this `VMExternData`. + value_ptr: NonNull, +} + +impl Clone for VMExternRef { + #[inline] + fn clone(&self) -> VMExternRef { + self.extern_data().increment_ref_count(); + VMExternRef(self.0) + } +} + +impl Drop for VMExternRef { + #[inline] + fn drop(&mut self) { + let data = self.extern_data(); + data.decrement_ref_count(); + if data.get_ref_count() == 0 { + // Drop our live reference to `data` before we drop it itself. + drop(data); + unsafe { + VMExternData::drop_and_dealloc(self.0); + } + } + } +} + +impl VMExternData { + unsafe fn drop_and_dealloc(mut data: NonNull) { + // Note: we introduce a block scope so that we drop the live + // reference to the data before we free the heap allocation it + // resides within after this block. + let (alloc_ptr, layout) = { + let data = data.as_mut(); + debug_assert_eq!(data.get_ref_count(), 0); + + // Same thing, but for the dropping the reference to `value` before + // we drop it itself. + let layout = { + let value = data.value_ptr.as_ref(); + + let value_size = mem::size_of_val(value); + let value_align = mem::align_of_val(value); + + let extern_data_size = mem::size_of::(); + let extern_data_align = mem::align_of::(); + + let value_and_padding_size = round_up_to_align(value_size, extern_data_align) + .unwrap_or_else(|| unreachable!()); + + let alloc_align = std::cmp::max(value_align, extern_data_align); + let alloc_size = value_and_padding_size + extern_data_size; + + debug_assert!(Layout::from_size_align(alloc_size, alloc_align).is_ok()); + Layout::from_size_align_unchecked(alloc_size, alloc_align) + }; + + ptr::drop_in_place(data.value_ptr.as_ptr()); + let alloc_ptr = data.value_ptr.cast::(); + + (alloc_ptr, layout) + }; + + ptr::drop_in_place(data.as_ptr()); + std::alloc::dealloc(alloc_ptr.as_ptr(), layout); + } + + #[inline] + fn get_ref_count(&self) -> usize { + unsafe { *self.ref_count.get() } + } + + #[inline] + fn increment_ref_count(&self) { + unsafe { + let count = self.ref_count.get(); + *count += 1; + } + } + + #[inline] + fn decrement_ref_count(&self) { + unsafe { + let count = self.ref_count.get(); + *count -= 1; + } + } +} + +#[inline] +fn round_up_to_align(n: usize, align: usize) -> Option { + debug_assert!(align.is_power_of_two()); + let align_minus_one = align - 1; + Some(n.checked_add(align_minus_one)? & !align_minus_one) +} + +impl VMExternRef { + /// Wrap the given value inside an `VMExternRef`. + pub fn new(value: T) -> VMExternRef + where + T: 'static + Any, + { + VMExternRef::new_with(|| value) + } + + /// Construct a new `VMExternRef` in place by invoking `make_value`. + pub fn new_with(make_value: impl FnOnce() -> T) -> VMExternRef + where + T: 'static + Any, + { + let value_size = mem::size_of::(); + let value_align = mem::align_of::(); + + let extern_data_align = mem::align_of::(); + let extern_data_size = mem::size_of::(); + + let value_and_padding_size = round_up_to_align(value_size, extern_data_align) + .unwrap_or_else(|| { + Self::alloc_failure(); + }); + + let alloc_align = std::cmp::max(value_align, extern_data_align); + let alloc_size = value_and_padding_size + .checked_add(extern_data_size) + .unwrap_or_else(|| Self::alloc_failure()); + + unsafe { + debug_assert!(Layout::from_size_align(alloc_size, alloc_align).is_ok()); + let layout = Layout::from_size_align_unchecked(alloc_size, alloc_align); + + let alloc_ptr = std::alloc::alloc(layout); + let alloc_ptr = NonNull::new(alloc_ptr).unwrap_or_else(|| { + Self::alloc_failure(); + }); + + let value_ptr = alloc_ptr.cast::(); + ptr::write(value_ptr.as_ptr(), make_value()); + + let value_ref: &T = value_ptr.as_ref(); + let value_ref: &dyn Any = value_ref as _; + let value_ptr: *const dyn Any = value_ref as _; + let value_ptr: *mut dyn Any = value_ptr as _; + let value_ptr = NonNull::new_unchecked(value_ptr); + + let extern_data_ptr = + alloc_ptr.cast::().as_ptr().add(value_and_padding_size) as *mut VMExternData; + ptr::write( + extern_data_ptr, + VMExternData { + ref_count: UnsafeCell::new(1), + value_ptr, + }, + ); + + VMExternRef(NonNull::new_unchecked(extern_data_ptr)) + } + } + + /// Turn this `VMExternRef` into a raw, untyped pointer. + /// + /// This forgets `self` and does *not* decrement the reference count on the + /// pointed-to data. + /// + /// This `VMExternRef` may be recovered with `VMExternRef::from_raw`. + pub fn into_raw(self) -> *mut u8 { + let ptr = self.0.cast::().as_ptr(); + mem::forget(self); + ptr + } + + /// Create a `VMExternRef` from a pointer returned from a previous call to + /// `VMExternRef::into_raw`. + /// + /// # Safety + /// + /// Wildly unsafe to use with anything other than the result of a previous + /// `into_raw` call! + /// + /// This method does *not* increment the reference count on the pointed-to + /// data, so `from_raw` must be called at most *once* on the result of a + /// previous `into_raw` call. (Ideally, every `into_raw` is later followed + /// by a `from_raw`, but it is technically memory safe to never call + /// `from_raw` after `into_raw`: it will leak the pointed-to value, which is + /// memory safe). + pub unsafe fn from_raw(ptr: *mut u8) -> Self { + debug_assert!(!ptr.is_null()); + VMExternRef(NonNull::new_unchecked(ptr).cast()) + } + + #[inline(never)] + #[cold] + fn alloc_failure() -> ! { + panic!("VMExternRef allocation failure") + } + + #[inline] + fn extern_data(&self) -> &VMExternData { + unsafe { self.0.as_ref() } + } +} + +impl PartialEq for VMExternRef { + #[inline] + fn eq(&self, rhs: &Self) -> bool { + ptr::eq(self.0.as_ptr() as *const _, rhs.0.as_ptr() as *const _) + } +} + +impl Eq for VMExternRef {} + +impl Hash for VMExternRef { + #[inline] + fn hash(&self, hasher: &mut H) + where + H: Hasher, + { + ptr::hash(self.0.as_ptr() as *const _, hasher); + } +} + +impl PartialOrd for VMExternRef { + #[inline] + fn partial_cmp(&self, rhs: &Self) -> Option { + let a = self.0.as_ptr() as usize; + let b = rhs.0.as_ptr() as usize; + a.partial_cmp(&b) + } +} + +impl Ord for VMExternRef { + #[inline] + fn cmp(&self, rhs: &Self) -> Ordering { + let a = self.0.as_ptr() as usize; + let b = rhs.0.as_ptr() as usize; + a.cmp(&b) + } +} + +impl Deref for VMExternRef { + type Target = dyn Any; + + fn deref(&self) -> &dyn Any { + unsafe { self.extern_data().value_ptr.as_ref() } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::convert::TryInto; + + #[test] + fn extern_ref_is_pointer_sized_and_aligned() { + assert_eq!(mem::size_of::(), mem::size_of::<*mut ()>()); + assert_eq!(mem::align_of::(), mem::align_of::<*mut ()>()); + assert_eq!( + mem::size_of::>(), + mem::size_of::<*mut ()>() + ); + assert_eq!( + mem::align_of::>(), + mem::align_of::<*mut ()>() + ); + } + + #[test] + fn ref_count_is_at_correct_offset() { + let s = "hi"; + let s: &dyn Any = &s as _; + let s: *const dyn Any = s as _; + let s: *mut dyn Any = s as _; + + let extern_data = VMExternData { + ref_count: UnsafeCell::new(0), + value_ptr: NonNull::new(s).unwrap(), + }; + + let extern_data_ptr = &extern_data as *const _; + let ref_count_ptr = &extern_data.ref_count as *const _; + + let actual_offset = (ref_count_ptr as usize) - (extern_data_ptr as usize); + + assert_eq!( + wasmtime_environ::VMOffsets::vm_extern_data_ref_count(), + actual_offset.try_into().unwrap(), + ); + } +} diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs index 97983cde2d..6f2896054a 100644 --- a/crates/runtime/src/lib.rs +++ b/crates/runtime/src/lib.rs @@ -22,6 +22,7 @@ )] mod export; +mod externref; mod imports; mod instance; mod jit_int; @@ -36,6 +37,7 @@ pub mod debug_builtins; pub mod libcalls; pub use crate::export::*; +pub use crate::externref::VMExternRef; pub use crate::imports::Imports; pub use crate::instance::{InstanceHandle, InstantiationError, LinkError}; pub use crate::jit_int::GdbJitImageRegistration; From 38a92d89de625c9926feb47cc42cfc15281923c8 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 21 May 2020 13:24:24 -0700 Subject: [PATCH 03/14] Initialize `env_logger` for our `*.wast` tests --- Cargo.lock | 1 + Cargo.toml | 1 + build.rs | 3 ++- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index be772cc0e1..fb823661f8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2211,6 +2211,7 @@ name = "wasmtime-cli" version = "0.16.0" dependencies = [ "anyhow", + "env_logger 0.7.1", "faerie", "file-per-thread-logger", "filecheck", diff --git a/Cargo.toml b/Cargo.toml index fa5f4cf312..60809ddcec 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,7 @@ rayon = "1.2.1" humantime = "1.3.0" [dev-dependencies] +env_logger = "0.7.1" filecheck = "0.5.0" more-asserts = "0.2.1" tempfile = "3.1.0" diff --git a/build.rs b/build.rs index 3841b85303..3176abd157 100644 --- a/build.rs +++ b/build.rs @@ -155,9 +155,10 @@ fn write_testsuite_tests( writeln!(out, "#[ignore]")?; } writeln!(out, "fn r#{}() {{", &testname)?; + writeln!(out, " let _ = env_logger::try_init();")?; writeln!( out, - "crate::wast::run_wast(r#\"{}\"#, crate::wast::Strategy::{}).unwrap();", + " crate::wast::run_wast(r#\"{}\"#, crate::wast::Strategy::{}).unwrap();", path.display(), strategy )?; From acf8ad0df7b6373946a85de97bf0e770dede6591 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Fri, 22 May 2020 15:00:52 -0700 Subject: [PATCH 04/14] cranelift_wasm: expose the original Wasm function signature In the `ModuleEnvironment::declare_signature` callback, also pass the original Wasm function signature, so that consumers may associate this information with each compiled function. This is often necessary because while each Wasm signature gets compiled down into a single native signature, multiple Wasm signatures might compile down into the same native signature, and in these cases the original Wasm signature is required for dynamic type checking of calls. --- cranelift/wasm/src/environ/dummy.rs | 5 ++- cranelift/wasm/src/environ/mod.rs | 2 +- cranelift/wasm/src/environ/spec.rs | 13 ++++++- cranelift/wasm/src/lib.rs | 2 +- cranelift/wasm/src/sections_translator.rs | 46 +++++++++-------------- crates/environ/src/module_environ.rs | 4 +- 6 files changed, 36 insertions(+), 36 deletions(-) diff --git a/cranelift/wasm/src/environ/dummy.rs b/cranelift/wasm/src/environ/dummy.rs index d5c9d63e40..671414bcb6 100644 --- a/cranelift/wasm/src/environ/dummy.rs +++ b/cranelift/wasm/src/environ/dummy.rs @@ -6,7 +6,8 @@ //! [Wasmtime]: https://github.com/bytecodealliance/wasmtime use crate::environ::{ - FuncEnvironment, GlobalVariable, ModuleEnvironment, ReturnMode, TargetEnvironment, WasmResult, + FuncEnvironment, GlobalVariable, ModuleEnvironment, ReturnMode, TargetEnvironment, + WasmFuncType, WasmResult, }; use crate::func_translator::FuncTranslator; use crate::state::ModuleTranslationState; @@ -534,7 +535,7 @@ impl TargetEnvironment for DummyEnvironment { } impl<'data> ModuleEnvironment<'data> for DummyEnvironment { - fn declare_signature(&mut self, sig: ir::Signature) -> WasmResult<()> { + fn declare_signature(&mut self, _wasm: &WasmFuncType, sig: ir::Signature) -> WasmResult<()> { self.info.signatures.push(sig); Ok(()) } diff --git a/cranelift/wasm/src/environ/mod.rs b/cranelift/wasm/src/environ/mod.rs index 1cdb0b292a..bb4b7cc34e 100644 --- a/cranelift/wasm/src/environ/mod.rs +++ b/cranelift/wasm/src/environ/mod.rs @@ -7,5 +7,5 @@ mod spec; pub use crate::environ::dummy::DummyEnvironment; pub use crate::environ::spec::{ FuncEnvironment, GlobalVariable, ModuleEnvironment, ReturnMode, TargetEnvironment, WasmError, - WasmResult, + WasmFuncType, WasmResult, WasmType, }; diff --git a/cranelift/wasm/src/environ/spec.rs b/cranelift/wasm/src/environ/spec.rs index aeec47cede..0473ad3c5a 100644 --- a/cranelift/wasm/src/environ/spec.rs +++ b/cranelift/wasm/src/environ/spec.rs @@ -22,6 +22,13 @@ use thiserror::Error; use wasmparser::BinaryReaderError; use wasmparser::Operator; +// Re-export `wasmparser`'s function and value types so that consumers can +// associate this the original Wasm signature with each compiled function. This +// is often necessary because while each Wasm signature gets compiled down into +// a single native signature, multiple Wasm signatures might compile down into +// the same native signature. +pub use wasmparser::{FuncType as WasmFuncType, Type as WasmType}; + /// The value of a WebAssembly global variable. #[derive(Clone, Copy)] pub enum GlobalVariable { @@ -472,7 +479,11 @@ pub trait ModuleEnvironment<'data>: TargetEnvironment { } /// Declares a function signature to the environment. - fn declare_signature(&mut self, sig: ir::Signature) -> WasmResult<()>; + fn declare_signature( + &mut self, + wasm_func_type: &WasmFuncType, + sig: ir::Signature, + ) -> WasmResult<()>; /// Provides the number of imports up front. By default this does nothing, but /// implementations can use this to preallocate memory if desired. diff --git a/cranelift/wasm/src/lib.rs b/cranelift/wasm/src/lib.rs index 366dd7a0dc..b3836fd251 100644 --- a/cranelift/wasm/src/lib.rs +++ b/cranelift/wasm/src/lib.rs @@ -59,7 +59,7 @@ mod translation_utils; pub use crate::environ::{ DummyEnvironment, FuncEnvironment, GlobalVariable, ModuleEnvironment, ReturnMode, - TargetEnvironment, WasmError, WasmResult, + TargetEnvironment, WasmError, WasmFuncType, WasmResult, WasmType, }; pub use crate::func_translator::FuncTranslator; pub use crate::module_translator::translate_module; diff --git a/cranelift/wasm/src/sections_translator.rs b/cranelift/wasm/src/sections_translator.rs index bb51945d69..967612178a 100644 --- a/cranelift/wasm/src/sections_translator.rs +++ b/cranelift/wasm/src/sections_translator.rs @@ -23,7 +23,7 @@ use std::boxed::Box; use std::vec::Vec; use wasmparser::{ self, CodeSectionReader, Data, DataKind, DataSectionReader, Element, ElementItem, ElementItems, - ElementKind, ElementSectionReader, Export, ExportSectionReader, ExternalKind, FuncType, + ElementKind, ElementSectionReader, Export, ExportSectionReader, ExternalKind, FunctionSectionReader, GlobalSectionReader, GlobalType, ImportSectionEntryType, ImportSectionReader, MemorySectionReader, MemoryType, NameSectionReader, Naming, NamingReader, Operator, TableSectionReader, Type, TypeSectionReader, @@ -40,34 +40,22 @@ pub fn parse_type_section( environ.reserve_signatures(count)?; for entry in types { - match entry? { - FuncType { - form: wasmparser::Type::Func, - params, - returns, - } => { - let mut sig = - Signature::new(ModuleEnvironment::target_config(environ).default_call_conv); - sig.params.extend(params.iter().map(|ty| { - let cret_arg: ir::Type = type_to_type(*ty, environ) - .expect("only numeric types are supported in function signatures"); - AbiParam::new(cret_arg) - })); - sig.returns.extend(returns.iter().map(|ty| { - let cret_arg: ir::Type = type_to_type(*ty, environ) - .expect("only numeric types are supported in function signatures"); - AbiParam::new(cret_arg) - })); - environ.declare_signature(sig)?; - module_translation_state.wasm_types.push((params, returns)); - } - ty => { - return Err(wasm_unsupported!( - "unsupported type in type section: {:?}", - ty - )) - } - } + let wasm_func_ty = entry?; + let mut sig = Signature::new(ModuleEnvironment::target_config(environ).default_call_conv); + sig.params.extend(wasm_func_ty.params.iter().map(|ty| { + let cret_arg: ir::Type = type_to_type(*ty, environ) + .expect("only numeric types are supported in function signatures"); + AbiParam::new(cret_arg) + })); + sig.returns.extend(wasm_func_ty.returns.iter().map(|ty| { + let cret_arg: ir::Type = type_to_type(*ty, environ) + .expect("only numeric types are supported in function signatures"); + AbiParam::new(cret_arg) + })); + environ.declare_signature(&wasm_func_ty, sig)?; + module_translation_state + .wasm_types + .push((wasm_func_ty.params, wasm_func_ty.returns)); } Ok(()) } diff --git a/crates/environ/src/module_environ.rs b/crates/environ/src/module_environ.rs index 118570b124..9061775d66 100644 --- a/crates/environ/src/module_environ.rs +++ b/crates/environ/src/module_environ.rs @@ -7,7 +7,7 @@ use cranelift_entity::PrimaryMap; use cranelift_wasm::{ self, translate_module, DataIndex, DefinedFuncIndex, ElemIndex, FuncIndex, Global, GlobalIndex, Memory, MemoryIndex, ModuleTranslationState, SignatureIndex, Table, TableIndex, - TargetEnvironment, WasmError, WasmResult, + TargetEnvironment, WasmError, WasmFuncType, WasmResult, }; use std::convert::TryFrom; use std::sync::Arc; @@ -106,7 +106,7 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data Ok(()) } - fn declare_signature(&mut self, sig: ir::Signature) -> WasmResult<()> { + fn declare_signature(&mut self, _wasm: &WasmFuncType, sig: ir::Signature) -> WasmResult<()> { let sig = translate_signature(sig, self.pointer_type()); // TODO: Deduplicate signatures. self.result.module.local.signatures.push(sig); From 137e182750261fdaba787eae15fd8ad75161f225 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Fri, 22 May 2020 15:29:51 -0700 Subject: [PATCH 05/14] Update `wasmparser` to 0.57.0 --- Cargo.lock | 22 ++++++++++++++-------- cranelift/wasm/Cargo.toml | 2 +- cranelift/wasm/src/code_translator.rs | 4 ++++ crates/debug/Cargo.toml | 2 +- crates/environ/Cargo.toml | 2 +- crates/fuzzing/Cargo.toml | 2 +- crates/jit/Cargo.toml | 2 +- crates/lightbeam/Cargo.toml | 2 +- crates/wasmtime/Cargo.toml | 2 +- crates/wasmtime/src/runtime.rs | 1 + 10 files changed, 26 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fb823661f8..64665c28bc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -579,7 +579,7 @@ dependencies = [ "serde", "target-lexicon", "thiserror", - "wasmparser", + "wasmparser 0.57.0", "wat", ] @@ -1114,7 +1114,7 @@ dependencies = [ "staticvec", "thiserror", "typemap", - "wasmparser", + "wasmparser 0.57.0", "wat", ] @@ -2151,6 +2151,12 @@ version = "0.55.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "af931e2e1960c53f4a28b063fec4cacd036f35acbec8ff3a4739125b17382a87" +[[package]] +name = "wasmparser" +version = "0.57.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32fddd575d477c6e9702484139cf9f23dcd554b06d185ed0f56c857dd3a47aa6" + [[package]] name = "wasmprinter" version = "0.2.5" @@ -2158,7 +2164,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c93ba310101ec5ee980db66b47b3d276577c8310df1570e19994347137650454" dependencies = [ "anyhow", - "wasmparser", + "wasmparser 0.55.0", ] [[package]] @@ -2175,7 +2181,7 @@ dependencies = [ "rustc-demangle", "target-lexicon", "tempfile", - "wasmparser", + "wasmparser 0.57.0", "wasmtime-environ", "wasmtime-jit", "wasmtime-profiling", @@ -2247,7 +2253,7 @@ dependencies = [ "more-asserts", "target-lexicon", "thiserror", - "wasmparser", + "wasmparser 0.57.0", "wasmtime-environ", ] @@ -2278,7 +2284,7 @@ dependencies = [ "tempfile", "thiserror", "toml", - "wasmparser", + "wasmparser 0.57.0", "winapi", "zstd", ] @@ -2307,7 +2313,7 @@ dependencies = [ "env_logger 0.7.1", "log", "rayon", - "wasmparser", + "wasmparser 0.57.0", "wasmprinter", "wasmtime", "wasmtime-wast", @@ -2331,7 +2337,7 @@ dependencies = [ "region", "target-lexicon", "thiserror", - "wasmparser", + "wasmparser 0.57.0", "wasmtime-debug", "wasmtime-environ", "wasmtime-profiling", diff --git a/cranelift/wasm/Cargo.toml b/cranelift/wasm/Cargo.toml index c92a13a269..ae5056effd 100644 --- a/cranelift/wasm/Cargo.toml +++ b/cranelift/wasm/Cargo.toml @@ -12,7 +12,7 @@ keywords = ["webassembly", "wasm"] edition = "2018" [dependencies] -wasmparser = { version = "0.55.0", default-features = false } +wasmparser = { version = "0.57.0", default-features = false } cranelift-codegen = { path = "../codegen", version = "0.63.0", default-features = false } cranelift-entity = { path = "../entity", version = "0.63.0" } cranelift-frontend = { path = "../frontend", version = "0.63.0", default-features = false } diff --git a/cranelift/wasm/src/code_translator.rs b/cranelift/wasm/src/code_translator.rs index 4e740b99e4..db9bef5983 100644 --- a/cranelift/wasm/src/code_translator.rs +++ b/cranelift/wasm/src/code_translator.rs @@ -1565,6 +1565,10 @@ pub fn translate_operator( | Operator::I32x4WidenHighI16x8U { .. } => { return Err(wasm_unsupported!("proposed SIMD operator {:?}", op)); } + + Operator::ReturnCall { .. } | Operator::ReturnCallIndirect { .. } => { + return Err(wasm_unsupported!("proposed tail-call operator {:?}", op)); + } }; Ok(()) } diff --git a/crates/debug/Cargo.toml b/crates/debug/Cargo.toml index 1e83b01b3c..6ef2dfc054 100644 --- a/crates/debug/Cargo.toml +++ b/crates/debug/Cargo.toml @@ -13,7 +13,7 @@ edition = "2018" [dependencies] gimli = "0.20.0" -wasmparser = "0.55.0" +wasmparser = "0.57.0" faerie = "0.15.0" wasmtime-environ = { path = "../environ", version = "0.16.0" } target-lexicon = { version = "0.10.0", default-features = false } diff --git a/crates/environ/Cargo.toml b/crates/environ/Cargo.toml index 2d2db6a31e..79a2601deb 100644 --- a/crates/environ/Cargo.toml +++ b/crates/environ/Cargo.toml @@ -16,7 +16,7 @@ anyhow = "1.0" cranelift-codegen = { path = "../../cranelift/codegen", version = "0.63.0", features = ["enable-serde"] } cranelift-entity = { path = "../../cranelift/entity", version = "0.63.0", features = ["enable-serde"] } cranelift-wasm = { path = "../../cranelift/wasm", version = "0.63.0", features = ["enable-serde"] } -wasmparser = "0.55.0" +wasmparser = "0.57.0" lightbeam = { path = "../lightbeam", optional = true, version = "0.16.0" } indexmap = "1.0.2" rayon = "1.2.1" diff --git a/crates/fuzzing/Cargo.toml b/crates/fuzzing/Cargo.toml index 23bb8704d1..7af689724b 100644 --- a/crates/fuzzing/Cargo.toml +++ b/crates/fuzzing/Cargo.toml @@ -13,7 +13,7 @@ binaryen = { version = "0.10.0", optional = true } env_logger = "0.7.1" log = "0.4.8" rayon = "1.2.1" -wasmparser = "0.55.0" +wasmparser = "0.57.0" wasmprinter = "0.2.5" wasmtime = { path = "../wasmtime" } wasmtime-wast = { path = "../wast" } diff --git a/crates/jit/Cargo.toml b/crates/jit/Cargo.toml index 31ca4bc3b0..094330fbce 100644 --- a/crates/jit/Cargo.toml +++ b/crates/jit/Cargo.toml @@ -24,7 +24,7 @@ wasmtime-profiling = { path = "../profiling", version = "0.16.0" } region = "2.0.0" thiserror = "1.0.4" target-lexicon = { version = "0.10.0", default-features = false } -wasmparser = "0.55.0" +wasmparser = "0.57.0" more-asserts = "0.2.1" anyhow = "1.0" cfg-if = "0.1.9" diff --git a/crates/lightbeam/Cargo.toml b/crates/lightbeam/Cargo.toml index 7e6acbabc0..50e73d7606 100644 --- a/crates/lightbeam/Cargo.toml +++ b/crates/lightbeam/Cargo.toml @@ -24,7 +24,7 @@ smallvec = "1.0.0" staticvec = "0.9" thiserror = "1.0.9" typemap = "0.3" -wasmparser = "0.55.0" +wasmparser = "0.57.0" [dev-dependencies] lazy_static = "1.2" diff --git a/crates/wasmtime/Cargo.toml b/crates/wasmtime/Cargo.toml index 4334f72a64..03d958721b 100644 --- a/crates/wasmtime/Cargo.toml +++ b/crates/wasmtime/Cargo.toml @@ -14,7 +14,7 @@ wasmtime-runtime = { path = "../runtime", version = "0.16.0" } wasmtime-environ = { path = "../environ", version = "0.16.0" } wasmtime-jit = { path = "../jit", version = "0.16.0" } wasmtime-profiling = { path = "../profiling", version = "0.16.0" } -wasmparser = "0.55.0" +wasmparser = "0.57.0" target-lexicon = { version = "0.10.0", default-features = false } anyhow = "1.0.19" region = "2.0.0" diff --git a/crates/wasmtime/src/runtime.rs b/crates/wasmtime/src/runtime.rs index 03a7c349d0..23168d8800 100644 --- a/crates/wasmtime/src/runtime.rs +++ b/crates/wasmtime/src/runtime.rs @@ -83,6 +83,7 @@ impl Config { enable_bulk_memory: false, enable_simd: false, enable_multi_value: true, + enable_tail_call: false, }, }, flags, From a8ee0554a9eb9c07c933e5364cee32bf63694d5c Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Fri, 22 May 2020 17:12:45 -0700 Subject: [PATCH 06/14] wasmtime: Initial, partial support for `externref` This is enough to get an `externref -> externref` identity function passing. However, `externref`s that are dropped by compiled Wasm code are (safely) leaked. Follow up work will leverage cranelift's stack maps to resolve this issue. --- build.rs | 1 + crates/c-api/macros/src/lib.rs | 2 +- crates/c-api/src/engine.rs | 8 +- crates/c-api/src/error.rs | 6 +- crates/c-api/src/extern.rs | 10 +- crates/c-api/src/func.rs | 22 +- crates/c-api/src/global.rs | 6 +- crates/c-api/src/instance.rs | 29 +- crates/c-api/src/linker.rs | 6 +- crates/c-api/src/memory.rs | 4 +- crates/c-api/src/module.rs | 8 +- crates/c-api/src/ref.rs | 16 +- crates/c-api/src/store.rs | 8 +- crates/c-api/src/table.rs | 36 ++- crates/c-api/src/trap.rs | 12 +- crates/c-api/src/wasi.rs | 7 +- crates/environ/src/cranelift.rs | 2 +- crates/environ/src/data_structures.rs | 2 +- crates/environ/src/func_environ.rs | 13 +- crates/environ/src/module.rs | 17 +- crates/environ/src/module_environ.rs | 15 +- crates/jit/src/compiler.rs | 10 +- crates/jit/src/imports.rs | 4 +- crates/jit/src/instantiate.rs | 2 +- crates/runtime/src/externref.rs | 15 + crates/runtime/src/sig_registry.rs | 74 +++-- crates/wasmtime/src/func.rs | 38 ++- crates/wasmtime/src/ref.rs | 278 ++++++++---------- crates/wasmtime/src/runtime.rs | 38 ++- .../wasmtime/src/trampoline/create_handle.rs | 7 +- crates/wasmtime/src/trampoline/func.rs | 20 +- crates/wasmtime/src/types.rs | 65 +++- crates/wasmtime/src/values.rs | 57 +++- crates/wast/src/spectest.rs | 2 +- crates/wast/src/wast.rs | 23 +- tests/all/externals.rs | 38 +-- tests/all/linker.rs | 4 +- tests/all/table.rs | 4 +- tests/all/traps.rs | 2 +- tests/all/use_after_drop.rs | 2 +- .../externref-id-function.wast | 8 + 41 files changed, 545 insertions(+), 376 deletions(-) mode change 100644 => 100755 crates/wasmtime/src/ref.rs create mode 100644 tests/misc_testsuite/reference-types/externref-id-function.wast diff --git a/build.rs b/build.rs index 3176abd157..dff3527127 100644 --- a/build.rs +++ b/build.rs @@ -205,6 +205,7 @@ fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool { // Still working on implementing these. See #929. ("reference_types", "table_copy_on_imported_tables") => return false, + ("reference_types", "externref_id_function") => return false, ("reference_types", _) => return true, _ => {} diff --git a/crates/c-api/macros/src/lib.rs b/crates/c-api/macros/src/lib.rs index 304b95c4e9..1ca2655660 100644 --- a/crates/c-api/macros/src/lib.rs +++ b/crates/c-api/macros/src/lib.rs @@ -96,7 +96,7 @@ pub fn declare_ref(input: proc_macro::TokenStream) -> proc_macro::TokenStream { #[no_mangle] pub extern fn #as_ref(a: &#ty) -> Box { - let r = a.externref(); + let r = Some(a.externref()); Box::new(crate::wasm_ref_t { r }) } diff --git a/crates/c-api/src/engine.rs b/crates/c-api/src/engine.rs index 843c261ea6..aad34312e6 100644 --- a/crates/c-api/src/engine.rs +++ b/crates/c-api/src/engine.rs @@ -1,10 +1,10 @@ use crate::wasm_config_t; -use wasmtime::{Engine, HostRef}; +use wasmtime::Engine; #[repr(C)] #[derive(Clone)] pub struct wasm_engine_t { - pub(crate) engine: HostRef, + pub(crate) engine: Engine, } wasmtime_c_api_macros::declare_own!(wasm_engine_t); @@ -22,7 +22,7 @@ pub extern "C" fn wasm_engine_new() -> Box { drop(env_logger::try_init()); Box::new(wasm_engine_t { - engine: HostRef::new(Engine::default()), + engine: Engine::default(), }) } @@ -30,6 +30,6 @@ pub extern "C" fn wasm_engine_new() -> Box { pub extern "C" fn wasm_engine_new_with_config(c: Box) -> Box { let config = c.config; Box::new(wasm_engine_t { - engine: HostRef::new(Engine::new(&config)), + engine: Engine::new(&config), }) } diff --git a/crates/c-api/src/error.rs b/crates/c-api/src/error.rs index cbb84873df..3ea634b560 100644 --- a/crates/c-api/src/error.rs +++ b/crates/c-api/src/error.rs @@ -1,6 +1,6 @@ use crate::{wasm_name_t, wasm_trap_t}; use anyhow::{anyhow, Error, Result}; -use wasmtime::Trap; +use wasmtime::{Store, Trap}; #[repr(C)] pub struct wasmtime_error_t { @@ -10,8 +10,8 @@ pub struct wasmtime_error_t { wasmtime_c_api_macros::declare_own!(wasmtime_error_t); impl wasmtime_error_t { - pub(crate) fn to_trap(self) -> Box { - Box::new(wasm_trap_t::new(Trap::from(self.error))) + pub(crate) fn to_trap(self, store: &Store) -> Box { + Box::new(wasm_trap_t::new(store, Trap::from(self.error))) } } diff --git a/crates/c-api/src/extern.rs b/crates/c-api/src/extern.rs index 025f233493..4a71807cd9 100644 --- a/crates/c-api/src/extern.rs +++ b/crates/c-api/src/extern.rs @@ -18,12 +18,12 @@ pub(crate) enum ExternHost { } impl wasm_extern_t { - fn externref(&self) -> wasmtime::ExternRef { + pub(crate) fn externref(&self) -> wasmtime::ExternRef { match &self.which { - ExternHost::Func(f) => f.externref(), - ExternHost::Global(f) => f.externref(), - ExternHost::Memory(f) => f.externref(), - ExternHost::Table(f) => f.externref(), + ExternHost::Func(f) => f.clone().into(), + ExternHost::Global(f) => f.clone().into(), + ExternHost::Memory(f) => f.clone().into(), + ExternHost::Table(f) => f.clone().into(), } } } diff --git a/crates/c-api/src/func.rs b/crates/c-api/src/func.rs index 0d3e3d10c8..855770dff3 100644 --- a/crates/c-api/src/func.rs +++ b/crates/c-api/src/func.rs @@ -71,7 +71,7 @@ impl wasm_func_t { } fn externref(&self) -> wasmtime::ExternRef { - self.func().externref() + self.func().clone().into() } } @@ -90,7 +90,7 @@ fn create_function( ty: &wasm_functype_t, func: impl Fn(Caller<'_>, *const wasm_val_t, *mut wasm_val_t) -> Option> + 'static, ) -> Box { - let store = &store.store.borrow(); + let store = &store.store; let ty = ty.ty().ty.clone(); let func = Func::new(store, ty, move |caller, params, results| { let params = params @@ -107,7 +107,7 @@ fn create_function( } Ok(()) }); - Box::new(HostRef::new(func).into()) + Box::new(HostRef::new(store, func).into()) } #[no_mangle] @@ -182,7 +182,7 @@ pub unsafe extern "C" fn wasm_func_call( &mut trap, ); match error { - Some(err) => Box::into_raw(err.to_trap()), + Some(err) => Box::into_raw(err.to_trap(&wasm_func.ext.externref().store().unwrap())), None => trap, } } @@ -210,6 +210,7 @@ fn _wasmtime_func_call( results: &mut [wasm_val_t], trap_ptr: &mut *mut wasm_trap_t, ) -> Option> { + let store = &func.ext.externref().store().unwrap(); let func = func.func().borrow(); if results.len() != func.result_arity() { return Some(Box::new(anyhow!("wrong number of results provided").into())); @@ -230,7 +231,7 @@ fn _wasmtime_func_call( } Ok(Err(trap)) => match trap.downcast::() { Ok(trap) => { - *trap_ptr = Box::into_raw(Box::new(wasm_trap_t::new(trap))); + *trap_ptr = Box::into_raw(Box::new(wasm_trap_t::new(store, trap))); None } Err(err) => Some(Box::new(err.into())), @@ -243,7 +244,7 @@ fn _wasmtime_func_call( } else { Trap::new("rust panic happened") }; - let trap = Box::new(wasm_trap_t::new(trap)); + let trap = Box::new(wasm_trap_t::new(store, trap)); *trap_ptr = Box::into_raw(trap); None } @@ -277,11 +278,12 @@ pub unsafe extern "C" fn wasmtime_caller_export_get( ) -> Option> { let name = str::from_utf8(name.as_slice()).ok()?; let export = caller.caller.get_export(name)?; + let store = caller.caller.store(); let which = match export { - Extern::Func(f) => ExternHost::Func(HostRef::new(f)), - Extern::Global(g) => ExternHost::Global(HostRef::new(g)), - Extern::Memory(m) => ExternHost::Memory(HostRef::new(m)), - Extern::Table(t) => ExternHost::Table(HostRef::new(t)), + Extern::Func(f) => ExternHost::Func(HostRef::new(&store, f)), + Extern::Global(g) => ExternHost::Global(HostRef::new(&store, g)), + Extern::Memory(m) => ExternHost::Memory(HostRef::new(&store, m)), + Extern::Table(t) => ExternHost::Table(HostRef::new(&store, t)), }; Some(Box::new(wasm_extern_t { which })) } diff --git a/crates/c-api/src/global.rs b/crates/c-api/src/global.rs index b7ef6ebd7b..29e9477fa6 100644 --- a/crates/c-api/src/global.rs +++ b/crates/c-api/src/global.rs @@ -27,7 +27,7 @@ impl wasm_global_t { } fn externref(&self) -> wasmtime::ExternRef { - self.global().externref() + self.global().clone().into() } } @@ -54,11 +54,11 @@ pub extern "C" fn wasmtime_global_new( val: &wasm_val_t, ret: &mut *mut wasm_global_t, ) -> Option> { - let global = Global::new(&store.store.borrow(), gt.ty().ty.clone(), val.val()); + let global = Global::new(&store.store, gt.ty().ty.clone(), val.val()); handle_result(global, |global| { *ret = Box::into_raw(Box::new(wasm_global_t { ext: wasm_extern_t { - which: ExternHost::Global(HostRef::new(global)), + which: ExternHost::Global(HostRef::new(&store.store, global)), }, })); }) diff --git a/crates/c-api/src/instance.rs b/crates/c-api/src/instance.rs index 6d528530d6..a833f462eb 100644 --- a/crates/c-api/src/instance.rs +++ b/crates/c-api/src/instance.rs @@ -16,14 +16,15 @@ wasmtime_c_api_macros::declare_ref!(wasm_instance_t); impl wasm_instance_t { pub(crate) fn new(instance: Instance) -> wasm_instance_t { + let store = instance.store().clone(); wasm_instance_t { - instance: HostRef::new(instance), + instance: HostRef::new(&store, instance), exports_cache: RefCell::new(None), } } fn externref(&self) -> wasmtime::ExternRef { - self.instance.externref() + self.instance.clone().into() } } @@ -34,12 +35,12 @@ pub unsafe extern "C" fn wasm_instance_new( imports: *const Box, result: Option<&mut *mut wasm_trap_t>, ) -> Option> { - let store = &store.store.borrow(); + let store = &store.store; let module = &wasm_module.module.borrow(); if !Store::same(&store, module.store()) { if let Some(result) = result { let trap = Trap::new("wasm_store_t must match store in wasm_module_t"); - let trap = Box::new(wasm_trap_t::new(trap)); + let trap = Box::new(wasm_trap_t::new(store, trap)); *result = Box::into_raw(trap); } return None; @@ -58,7 +59,7 @@ pub unsafe extern "C" fn wasm_instance_new( assert!(trap.is_null()); assert!(instance.is_null()); if let Some(result) = result { - *result = Box::into_raw(err.to_trap()); + *result = Box::into_raw(err.to_trap(store)); } None } @@ -111,10 +112,16 @@ fn _wasmtime_instance_new( }) .collect::>(); let module = &module.module.borrow(); - handle_instantiate(Instance::new(module, &imports), instance_ptr, trap_ptr) + handle_instantiate( + module.store(), + Instance::new(module, &imports), + instance_ptr, + trap_ptr, + ) } pub fn handle_instantiate( + store: &Store, instance: Result, instance_ptr: &mut *mut wasm_instance_t, trap_ptr: &mut *mut wasm_trap_t, @@ -130,7 +137,7 @@ pub fn handle_instantiate( } Err(e) => match e.downcast::() { Ok(trap) => { - write(trap_ptr, wasm_trap_t::new(trap)); + write(trap_ptr, wasm_trap_t::new(store, trap)); None } Err(e) => Some(Box::new(e.into())), @@ -146,10 +153,10 @@ pub extern "C" fn wasm_instance_exports(instance: &wasm_instance_t, out: &mut wa instance .exports() .map(|e| match e.into_extern() { - Extern::Func(f) => ExternHost::Func(HostRef::new(f)), - Extern::Global(f) => ExternHost::Global(HostRef::new(f)), - Extern::Memory(f) => ExternHost::Memory(HostRef::new(f)), - Extern::Table(f) => ExternHost::Table(HostRef::new(f)), + Extern::Func(f) => ExternHost::Func(HostRef::new(instance.store(), f)), + Extern::Global(f) => ExternHost::Global(HostRef::new(instance.store(), f)), + Extern::Memory(f) => ExternHost::Memory(HostRef::new(instance.store(), f)), + Extern::Table(f) => ExternHost::Table(HostRef::new(instance.store(), f)), }) .collect() }); diff --git a/crates/c-api/src/linker.rs b/crates/c-api/src/linker.rs index 3578544ef2..24c565c383 100644 --- a/crates/c-api/src/linker.rs +++ b/crates/c-api/src/linker.rs @@ -12,7 +12,7 @@ pub struct wasmtime_linker_t { #[no_mangle] pub extern "C" fn wasmtime_linker_new(store: &wasm_store_t) -> Box { Box::new(wasmtime_linker_t { - linker: Linker::new(&store.store.borrow()), + linker: Linker::new(&store.store), }) } @@ -87,7 +87,7 @@ pub unsafe extern "C" fn wasmtime_linker_instantiate( trap_ptr: &mut *mut wasm_trap_t, ) -> Option> { let result = linker.linker.instantiate(&module.module.borrow()); - super::instance::handle_instantiate(result, instance_ptr, trap_ptr) + super::instance::handle_instantiate(linker.linker.store(), result, instance_ptr, trap_ptr) } #[no_mangle] @@ -116,6 +116,6 @@ pub unsafe extern "C" fn wasmtime_linker_get_default( Err(_) => return bad_utf8(), }; handle_result(linker.get_default(name), |f| { - *func = Box::into_raw(Box::new(HostRef::new(f).into())) + *func = Box::into_raw(Box::new(HostRef::new(linker.store(), f).into())) }) } diff --git a/crates/c-api/src/memory.rs b/crates/c-api/src/memory.rs index 48159b94ed..aa3e2e4db3 100644 --- a/crates/c-api/src/memory.rs +++ b/crates/c-api/src/memory.rs @@ -27,7 +27,7 @@ impl wasm_memory_t { } fn externref(&self) -> wasmtime::ExternRef { - self.memory().externref() + self.memory().clone().into() } } @@ -36,7 +36,7 @@ pub extern "C" fn wasm_memory_new( store: &wasm_store_t, mt: &wasm_memorytype_t, ) -> Box { - let memory = HostRef::new(Memory::new(&store.store.borrow(), mt.ty().ty.clone())); + let memory = HostRef::new(&store.store, Memory::new(&store.store, mt.ty().ty.clone())); Box::new(wasm_memory_t { ext: wasm_extern_t { which: ExternHost::Memory(memory), diff --git a/crates/c-api/src/module.rs b/crates/c-api/src/module.rs index 23c5eae364..2349e20bef 100644 --- a/crates/c-api/src/module.rs +++ b/crates/c-api/src/module.rs @@ -16,7 +16,7 @@ wasmtime_c_api_macros::declare_ref!(wasm_module_t); impl wasm_module_t { fn externref(&self) -> wasmtime::ExternRef { - self.module.externref() + self.module.clone().into() } } @@ -42,7 +42,7 @@ pub extern "C" fn wasmtime_module_new( ret: &mut *mut wasm_module_t, ) -> Option> { let binary = binary.as_slice(); - let store = &store.store.borrow(); + let store = &store.store; handle_result(Module::from_binary(store, binary), |module| { let imports = module .imports() @@ -53,7 +53,7 @@ pub extern "C" fn wasmtime_module_new( .map(|e| wasm_exporttype_t::new(e.name().to_owned(), e.ty())) .collect::>(); let module = Box::new(wasm_module_t { - module: HostRef::new(module), + module: HostRef::new(store, module), imports, exports, }); @@ -72,7 +72,7 @@ pub extern "C" fn wasmtime_module_validate( binary: &wasm_byte_vec_t, ) -> Option> { let binary = binary.as_slice(); - let store = &store.store.borrow(); + let store = &store.store; handle_result(Module::validate(store, binary), |()| {}) } diff --git a/crates/c-api/src/ref.rs b/crates/c-api/src/ref.rs index 00f93865ce..5e14c51c38 100644 --- a/crates/c-api/src/ref.rs +++ b/crates/c-api/src/ref.rs @@ -5,7 +5,7 @@ use wasmtime::ExternRef; #[repr(C)] #[derive(Clone)] pub struct wasm_ref_t { - pub(crate) r: ExternRef, + pub(crate) r: Option, } wasmtime_c_api_macros::declare_own!(wasm_ref_t); @@ -17,7 +17,11 @@ pub extern "C" fn wasm_ref_copy(r: &wasm_ref_t) -> Box { #[no_mangle] pub extern "C" fn wasm_ref_same(a: &wasm_ref_t, b: &wasm_ref_t) -> bool { - a.r.ptr_eq(&b.r) + match (a.r.as_ref(), b.r.as_ref()) { + (Some(a), Some(b)) => a.ptr_eq(b), + (None, None) => true, + _ => false, + } } pub(crate) fn get_host_info(r: &ExternRef) -> *mut c_void { @@ -25,6 +29,7 @@ pub(crate) fn get_host_info(r: &ExternRef) -> *mut c_void { Some(info) => info, None => return std::ptr::null_mut(), }; + let host_info = host_info.borrow(); match host_info.downcast_ref::() { Some(state) => state.info, None => std::ptr::null_mut(), @@ -33,7 +38,8 @@ pub(crate) fn get_host_info(r: &ExternRef) -> *mut c_void { #[no_mangle] pub extern "C" fn wasm_ref_get_host_info(a: &wasm_ref_t) -> *mut c_void { - get_host_info(&a.r) + a.r.as_ref() + .map_or(std::ptr::null_mut(), |r| get_host_info(r)) } pub(crate) fn set_host_info( @@ -51,7 +57,7 @@ pub(crate) fn set_host_info( #[no_mangle] pub extern "C" fn wasm_ref_set_host_info(a: &wasm_ref_t, info: *mut c_void) { - set_host_info(&a.r, info, None) + a.r.as_ref().map(|r| set_host_info(r, info, None)); } #[no_mangle] @@ -60,5 +66,5 @@ pub extern "C" fn wasm_ref_set_host_info_with_finalizer( info: *mut c_void, finalizer: Option, ) { - set_host_info(&a.r, info, finalizer) + a.r.as_ref().map(|r| set_host_info(r, info, finalizer)); } diff --git a/crates/c-api/src/store.rs b/crates/c-api/src/store.rs index a2a5b1c25b..625b3831ea 100644 --- a/crates/c-api/src/store.rs +++ b/crates/c-api/src/store.rs @@ -1,10 +1,10 @@ use crate::wasm_engine_t; -use wasmtime::{HostRef, InterruptHandle, Store}; +use wasmtime::{InterruptHandle, Store}; #[repr(C)] #[derive(Clone)] pub struct wasm_store_t { - pub(crate) store: HostRef, + pub(crate) store: Store, } wasmtime_c_api_macros::declare_own!(wasm_store_t); @@ -13,7 +13,7 @@ wasmtime_c_api_macros::declare_own!(wasm_store_t); pub extern "C" fn wasm_store_new(engine: &wasm_engine_t) -> Box { let engine = &engine.engine; Box::new(wasm_store_t { - store: HostRef::new(Store::new(&engine.borrow())), + store: Store::new(&engine), }) } @@ -29,7 +29,7 @@ pub extern "C" fn wasmtime_interrupt_handle_new( store: &wasm_store_t, ) -> Option> { Some(Box::new(wasmtime_interrupt_handle_t { - handle: store.store.borrow().interrupt_handle().ok()?, + handle: store.store.interrupt_handle().ok()?, })) } diff --git a/crates/c-api/src/table.rs b/crates/c-api/src/table.rs index 16626acf7b..e49daddef8 100644 --- a/crates/c-api/src/table.rs +++ b/crates/c-api/src/table.rs @@ -1,7 +1,7 @@ use crate::{handle_result, wasm_func_t, wasm_ref_t, wasmtime_error_t}; use crate::{wasm_extern_t, wasm_store_t, wasm_tabletype_t, ExternHost}; use std::ptr; -use wasmtime::{ExternRef, HostRef, Table, Val}; +use wasmtime::{HostRef, Table, Val}; #[derive(Clone)] #[repr(transparent)] @@ -29,7 +29,7 @@ impl wasm_table_t { } fn externref(&self) -> wasmtime::ExternRef { - self.table().externref() + self.table().clone().into() } } @@ -41,12 +41,12 @@ pub extern "C" fn wasm_table_new( ) -> Option> { let init: Val = match init { Some(init) => init.r.into(), - None => Val::ExternRef(ExternRef::Null), + None => Val::ExternRef(None), }; - let table = Table::new(&store.store.borrow(), tt.ty().ty.clone(), init).ok()?; + let table = Table::new(&store.store, tt.ty().ty.clone(), init).ok()?; Some(Box::new(wasm_table_t { ext: wasm_extern_t { - which: ExternHost::Table(HostRef::new(table)), + which: ExternHost::Table(HostRef::new(&store.store, table)), }, })) } @@ -60,14 +60,14 @@ pub extern "C" fn wasmtime_funcref_table_new( ) -> Option> { let init: Val = match init { Some(val) => Val::FuncRef(val.func().borrow().clone()), - None => Val::ExternRef(ExternRef::Null), + None => Val::ExternRef(None), }; handle_result( - Table::new(&store.store.borrow(), tt.ty().ty.clone(), init), + Table::new(&store.store, tt.ty().ty.clone(), init), |table| { *out = Box::into_raw(Box::new(wasm_table_t { ext: wasm_extern_t { - which: ExternHost::Table(HostRef::new(table)), + which: ExternHost::Table(HostRef::new(&store.store, table)), }, })); }, @@ -84,7 +84,7 @@ pub extern "C" fn wasm_table_type(t: &wasm_table_t) -> Box { pub extern "C" fn wasm_table_get(t: &wasm_table_t, index: wasm_table_size_t) -> *mut wasm_ref_t { match t.table().borrow().get(index) { Some(val) => into_funcref(val), - None => into_funcref(Val::ExternRef(ExternRef::Null)), + None => into_funcref(Val::ExternRef(None)), } } @@ -98,8 +98,14 @@ pub extern "C" fn wasmtime_funcref_table_get( Some(val) => { *ptr = match val { // TODO: what do do about creating new `HostRef` handles here? - Val::FuncRef(f) => Box::into_raw(Box::new(HostRef::new(f).into())), - Val::ExternRef(ExternRef::Null) => ptr::null_mut(), + Val::FuncRef(f) => { + let store = match t.table().as_ref().store() { + None => return false, + Some(store) => store, + }; + Box::into_raw(Box::new(HostRef::new(&store, f).into())) + } + Val::ExternRef(None) => ptr::null_mut(), _ => return false, }; } @@ -127,13 +133,13 @@ pub extern "C" fn wasmtime_funcref_table_set( ) -> Option> { let val = match val { Some(val) => Val::FuncRef(val.func().borrow().clone()), - None => Val::ExternRef(ExternRef::Null), + None => Val::ExternRef(None), }; handle_result(t.table().borrow().set(index, val), |()| {}) } fn into_funcref(val: Val) -> *mut wasm_ref_t { - if let Val::ExternRef(ExternRef::Null) = val { + if let Val::ExternRef(None) = val { return ptr::null_mut(); } let externref = match val.externref() { @@ -148,7 +154,7 @@ unsafe fn from_funcref(r: *mut wasm_ref_t) -> Val { if !r.is_null() { Box::from_raw(r).r.into() } else { - Val::ExternRef(ExternRef::Null) + Val::ExternRef(None) } } @@ -176,7 +182,7 @@ pub extern "C" fn wasmtime_funcref_table_grow( ) -> Option> { let val = match init { Some(val) => Val::FuncRef(val.func().borrow().clone()), - None => Val::ExternRef(ExternRef::Null), + None => Val::ExternRef(None), }; handle_result(t.table().borrow().grow(delta, val), |prev| { if let Some(ptr) = prev_size { diff --git a/crates/c-api/src/trap.rs b/crates/c-api/src/trap.rs index 5f0cc2df98..c7a906e3eb 100644 --- a/crates/c-api/src/trap.rs +++ b/crates/c-api/src/trap.rs @@ -1,6 +1,6 @@ use crate::{wasm_frame_vec_t, wasm_instance_t, wasm_name_t, wasm_store_t}; use once_cell::unsync::OnceCell; -use wasmtime::{HostRef, Trap}; +use wasmtime::{HostRef, Store, Trap}; #[repr(C)] #[derive(Clone)] @@ -11,14 +11,14 @@ pub struct wasm_trap_t { wasmtime_c_api_macros::declare_ref!(wasm_trap_t); impl wasm_trap_t { - pub(crate) fn new(trap: Trap) -> wasm_trap_t { + pub(crate) fn new(store: &Store, trap: Trap) -> wasm_trap_t { wasm_trap_t { - trap: HostRef::new(trap), + trap: HostRef::new(store, trap), } } fn externref(&self) -> wasmtime::ExternRef { - self.trap.externref() + self.trap.clone().into() } } @@ -37,7 +37,7 @@ pub type wasm_message_t = wasm_name_t; #[no_mangle] pub extern "C" fn wasm_trap_new( - _store: &wasm_store_t, + store: &wasm_store_t, message: &wasm_message_t, ) -> Box { let message = message.as_slice(); @@ -46,7 +46,7 @@ pub extern "C" fn wasm_trap_new( } let message = String::from_utf8_lossy(&message[..message.len() - 1]); Box::new(wasm_trap_t { - trap: HostRef::new(Trap::new(message)), + trap: HostRef::new(&store.store, Trap::new(message)), }) } diff --git a/crates/c-api/src/wasi.rs b/crates/c-api/src/wasi.rs index a3b3ef8e84..f1db5d6c6b 100644 --- a/crates/c-api/src/wasi.rs +++ b/crates/c-api/src/wasi.rs @@ -282,7 +282,7 @@ pub unsafe extern "C" fn wasi_instance_new( config: Box, trap: &mut *mut wasm_trap_t, ) -> Option> { - let store = &store.store.borrow(); + let store = &store.store; let result = match CStr::from_ptr(name).to_str().unwrap_or("") { "wasi_snapshot_preview1" => create_preview1_instance(store, *config), @@ -297,7 +297,7 @@ pub unsafe extern "C" fn wasi_instance_new( })), Err(e) => { *trap = Box::into_raw(Box::new(wasm_trap_t { - trap: HostRef::new(Trap::new(e)), + trap: HostRef::new(store, Trap::new(e)), })); None @@ -335,13 +335,14 @@ pub extern "C" fn wasi_instance_bind_import<'a>( if &export.ty() != import.ty.func()? { return None; } + let store = export.store(); let entry = instance .export_cache .entry(name.to_string()) .or_insert_with(|| { Box::new(wasm_extern_t { - which: ExternHost::Func(HostRef::new(export.clone())), + which: ExternHost::Func(HostRef::new(store, export.clone())), }) }); Some(entry) diff --git a/crates/environ/src/cranelift.rs b/crates/environ/src/cranelift.rs index e8e05c2438..d7d0146a6e 100644 --- a/crates/environ/src/cranelift.rs +++ b/crates/environ/src/cranelift.rs @@ -303,7 +303,7 @@ fn compile(env: CompileEnv<'_>) -> Result { } fn signature(&self, index: u32) -> &Self::Signature { - &self.module.signatures[SignatureIndex::from_u32(index)] + &self.module.signatures[SignatureIndex::from_u32(index)].1 } fn defined_table_index(&self, table_index: u32) -> Option { @@ -658,6 +658,13 @@ impl<'module_environment> TargetEnvironment for FuncEnvironment<'module_environm fn target_config(&self) -> TargetFrontendConfig { self.target_config } + + fn reference_type(&self) -> ir::Type { + // For now, the only reference types we support are `externref`, which + // don't require tracing GC and stack maps. So we just use the target's + // pointer type. This will have to change once we move to tracing GC. + self.pointer_type() + } } impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'module_environment> { @@ -915,7 +922,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m func: &mut ir::Function, index: SignatureIndex, ) -> WasmResult { - Ok(func.import_signature(self.module.signatures[index].clone())) + Ok(func.import_signature(self.module.signatures[index].1.clone())) } fn make_direct_func( @@ -923,7 +930,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m func: &mut ir::Function, index: FuncIndex, ) -> WasmResult { - let sig = self.module.func_signature(index); + let sig = self.module.native_func_signature(index); let signature = func.import_signature(sig.clone()); let name = get_func_name(index); Ok(func.import_function(ir::ExtFuncData { diff --git a/crates/environ/src/module.rs b/crates/environ/src/module.rs index 9a079e157b..280210cfa4 100644 --- a/crates/environ/src/module.rs +++ b/crates/environ/src/module.rs @@ -7,7 +7,7 @@ use cranelift_entity::{EntityRef, PrimaryMap}; use cranelift_wasm::{ DataIndex, DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex, ElemIndex, FuncIndex, Global, GlobalIndex, Memory, MemoryIndex, SignatureIndex, Table, - TableIndex, + TableIndex, WasmFuncType, }; use indexmap::IndexMap; use more_asserts::assert_ge; @@ -181,7 +181,7 @@ pub struct Module { #[derive(Debug, Hash)] pub struct ModuleLocal { /// Unprocessed signatures exactly as provided by `declare_signature()`. - pub signatures: PrimaryMap, + pub signatures: PrimaryMap, /// Number of imported functions in the module. pub num_imported_funcs: usize, @@ -332,8 +332,15 @@ impl ModuleLocal { index.index() < self.num_imported_globals } - /// Convenience method for looking up the signature of a function. - pub fn func_signature(&self, func_index: FuncIndex) -> &ir::Signature { - &self.signatures[self.functions[func_index]] + /// Convenience method for looking up the native signature of a compiled + /// Wasm function. + pub fn native_func_signature(&self, func_index: FuncIndex) -> &ir::Signature { + &self.signatures[self.functions[func_index]].1 + } + + /// Convenience method for looking up the original Wasm signature of a + /// function. + pub fn wasm_func_type(&self, func_index: FuncIndex) -> &WasmFuncType { + &self.signatures[self.functions[func_index]].0 } } diff --git a/crates/environ/src/module_environ.rs b/crates/environ/src/module_environ.rs index 9061775d66..7553215119 100644 --- a/crates/environ/src/module_environ.rs +++ b/crates/environ/src/module_environ.rs @@ -92,6 +92,13 @@ impl<'data> TargetEnvironment for ModuleEnvironment<'data> { fn target_config(&self) -> TargetFrontendConfig { self.result.target_config } + + fn reference_type(&self) -> ir::Type { + // For now, the only reference types we support are `externref`, which + // don't require tracing GC and stack maps. So we just use the target's + // pointer type. This will have to change once we move to tracing GC. + self.pointer_type() + } } /// This trait is useful for `translate_module` because it tells how to translate @@ -106,10 +113,14 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data Ok(()) } - fn declare_signature(&mut self, _wasm: &WasmFuncType, sig: ir::Signature) -> WasmResult<()> { + fn declare_signature(&mut self, wasm: &WasmFuncType, sig: ir::Signature) -> WasmResult<()> { let sig = translate_signature(sig, self.pointer_type()); // TODO: Deduplicate signatures. - self.result.module.local.signatures.push(sig); + self.result + .module + .local + .signatures + .push((wasm.clone(), sig)); Ok(()) } diff --git a/crates/jit/src/compiler.rs b/crates/jit/src/compiler.rs index 0bcef5f0fc..7ba94986a5 100644 --- a/crates/jit/src/compiler.rs +++ b/crates/jit/src/compiler.rs @@ -149,8 +149,10 @@ impl Compiler { let mut cx = FunctionBuilderContext::new(); let mut trampolines = HashMap::new(); let mut trampoline_relocations = HashMap::new(); - for sig in translation.module.local.signatures.values() { - let index = self.signatures.register(sig); + for (wasm_func_ty, native_sig) in translation.module.local.signatures.values() { + let index = self + .signatures + .register(wasm_func_ty.clone(), native_sig.clone()); if trampolines.contains_key(&index) { continue; } @@ -158,7 +160,7 @@ impl Compiler { &*self.isa, &mut self.code_memory, &mut cx, - sig, + native_sig, std::mem::size_of::(), )?; trampolines.insert(index, trampoline); @@ -167,7 +169,7 @@ impl Compiler { // show up be sure to log it in case anyone's listening and there's // an accidental bug. if relocations.len() > 0 { - log::info!("relocations found in trampoline for {:?}", sig); + log::info!("relocations found in trampoline for {:?}", native_sig); trampoline_relocations.insert(index, relocations); } } diff --git a/crates/jit/src/imports.rs b/crates/jit/src/imports.rs index d2ef3536fc..cf0f497e15 100644 --- a/crates/jit/src/imports.rs +++ b/crates/jit/src/imports.rs @@ -31,8 +31,8 @@ pub fn resolve_imports( match (import, &export) { (EntityIndex::Function(func_index), Some(Export::Function(f))) => { - let import_signature = module.local.func_signature(*func_index); - let signature = signatures.lookup(f.signature).unwrap(); + let import_signature = module.local.native_func_signature(*func_index); + let signature = signatures.lookup_native(f.signature).unwrap(); if signature != *import_signature { // TODO: If the difference is in the calling convention, // we could emit a wrapper function to fix it up. diff --git a/crates/jit/src/instantiate.rs b/crates/jit/src/instantiate.rs index e3f1a5eb96..d92a55fe3c 100644 --- a/crates/jit/src/instantiate.rs +++ b/crates/jit/src/instantiate.rs @@ -93,7 +93,7 @@ impl<'data> RawCompiledModule<'data> { .local .signatures .values() - .map(|sig| signature_registry.register(sig)) + .map(|(wasm, native)| signature_registry.register(wasm.clone(), native.clone())) .collect::>() }; diff --git a/crates/runtime/src/externref.rs b/crates/runtime/src/externref.rs index f7f085fe67..9f2ea1e481 100644 --- a/crates/runtime/src/externref.rs +++ b/crates/runtime/src/externref.rs @@ -57,6 +57,21 @@ //! to hold everything satisfies its alignment. It also ensures that we don't //! need a ton of excess padding between the `VMExternData` and the value for //! values with large alignment. +//! +//! ## Reference Counting Protocol and Wasm Functions +//! +//! Currently, `VMExternRef`s passed into compiled Wasm functions have move +//! semantics: the host code gives up ownership and does not decrement the +//! reference count. Similarly, `VMExternRef`s returned from compiled Wasm +//! functions also have move semantics: host code takes ownership and the +//! reference count is not incremented. +//! +//! This works well when a reference is passed into Wasm and then passed back +//! out again. However, if a reference is passed into Wasm, but not passed back +//! out, then the reference is leaked. This is only a temporary state, and +//! follow up work will leverage stack maps to fix this issue. Follow +//! https://github.com/bytecodealliance/wasmtime/issues/929 to keep an eye on +//! this. use std::alloc::Layout; use std::any::Any; diff --git a/crates/runtime/src/sig_registry.rs b/crates/runtime/src/sig_registry.rs index 7ad66149ac..7ea07ddf48 100644 --- a/crates/runtime/src/sig_registry.rs +++ b/crates/runtime/src/sig_registry.rs @@ -2,11 +2,11 @@ //! signature checking. use crate::vmcontext::VMSharedSignatureIndex; -use more_asserts::{assert_lt, debug_assert_lt}; -use std::collections::{hash_map, HashMap}; +use more_asserts::assert_lt; +use std::collections::HashMap; use std::convert::TryFrom; use std::sync::RwLock; -use wasmtime_environ::ir; +use wasmtime_environ::{ir, wasm::WasmFuncType}; /// WebAssembly requires that the caller and callee signatures in an indirect /// call must match. To implement this efficiently, keep a registry of all @@ -24,8 +24,13 @@ pub struct SignatureRegistry { #[derive(Debug, Default)] struct Inner { - signature2index: HashMap, - index2signature: HashMap, + wasm2index: HashMap, + + // Maps the index to the original Wasm signature. + index2wasm: HashMap, + + // Maps the index to the native signature. + index2native: HashMap, } impl SignatureRegistry { @@ -37,37 +42,42 @@ impl SignatureRegistry { } /// Register a signature and return its unique index. - pub fn register(&self, sig: &ir::Signature) -> VMSharedSignatureIndex { - let mut inner = self.inner.write().unwrap(); - let len = inner.signature2index.len(); - match inner.signature2index.entry(sig.clone()) { - hash_map::Entry::Occupied(entry) => *entry.get(), - hash_map::Entry::Vacant(entry) => { - // Keep `signature_hash` len under 2**32 -- VMSharedSignatureIndex::new(std::u32::MAX) - // is reserved for VMSharedSignatureIndex::default(). - debug_assert_lt!( - len, - std::u32::MAX as usize, - "Invariant check: signature_hash.len() < std::u32::MAX" - ); - let sig_id = VMSharedSignatureIndex::new(u32::try_from(len).unwrap()); - entry.insert(sig_id); - inner.index2signature.insert(sig_id, sig.clone()); - sig_id - } - } + pub fn register(&self, wasm: WasmFuncType, native: ir::Signature) -> VMSharedSignatureIndex { + let Inner { + wasm2index, + index2wasm, + index2native, + } = &mut *self.inner.write().unwrap(); + let len = wasm2index.len(); + + *wasm2index.entry(wasm.clone()).or_insert_with(|| { + // Keep `signature_hash` len under 2**32 -- VMSharedSignatureIndex::new(std::u32::MAX) + // is reserved for VMSharedSignatureIndex::default(). + assert_lt!( + len, + std::u32::MAX as usize, + "Invariant check: signature_hash.len() < std::u32::MAX" + ); + let index = VMSharedSignatureIndex::new(u32::try_from(len).unwrap()); + index2wasm.insert(index, wasm); + index2native.insert(index, native); + index + }) } - /// Looks up a shared signature index within this registry. + /// Looks up a shared native signature within this registry. /// /// Note that for this operation to be semantically correct the `idx` must /// have previously come from a call to `register` of this same object. - pub fn lookup(&self, idx: VMSharedSignatureIndex) -> Option { - self.inner - .read() - .unwrap() - .index2signature - .get(&idx) - .cloned() + pub fn lookup_native(&self, idx: VMSharedSignatureIndex) -> Option { + self.inner.read().unwrap().index2native.get(&idx).cloned() + } + + /// Looks up a shared Wasm signature within this registry. + /// + /// Note that for this operation to be semantically correct the `idx` must + /// have previously come from a call to `register` of this same object. + pub fn lookup_wasm(&self, idx: VMSharedSignatureIndex) -> Option { + self.inner.read().unwrap().index2wasm.get(&idx).cloned() } } diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index 70bf2843bc..febd02b76e 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -244,9 +244,10 @@ impl Func { // number of arguments and the right types of arguments. As a result // we should be able to safely run through them all and read them. let mut args = Vec::with_capacity(ty_clone.params().len()); + let store = Store::upgrade(&store_weak).unwrap(); for (i, ty) in ty_clone.params().iter().enumerate() { unsafe { - args.push(Val::read_value_from(values_vec.add(i), ty)); + args.push(Val::read_value_from(&store, values_vec.add(i), ty)); } } let mut returns = vec![Val::null(); ty_clone.results().len()]; @@ -483,25 +484,25 @@ impl Func { .store .compiler() .signatures() - .lookup(self.export.signature) + .lookup_wasm(self.export.signature) .expect("failed to lookup signature"); // This is only called with `Export::Function`, and since it's coming // from wasmtime_runtime itself we should support all the types coming // out of it, so assert such here. - FuncType::from_wasmtime_signature(&sig).expect("core wasm signature should be supported") + FuncType::from_wasm_func_type(&sig).expect("core wasm signature should be supported") } /// Returns the number of parameters that this function takes. pub fn param_arity(&self) -> usize { - let sig = self - .instance + self.instance .store .compiler() .signatures() - .lookup(self.export.signature) - .expect("failed to lookup signature"); - sig.params.len() - 2 // skip the two vmctx leading parameters + .lookup_wasm(self.export.signature) + .expect("failed to lookup signature") + .params + .len() } /// Returns the number of results this function produces. @@ -511,7 +512,7 @@ impl Func { .store .compiler() .signatures() - .lookup(self.export.signature) + .lookup_wasm(self.export.signature) .expect("failed to lookup signature"); sig.returns.len() } @@ -546,7 +547,11 @@ impl Func { let param_tys = my_ty.params().iter(); for ((arg, slot), ty) in params.iter().zip(&mut values_vec).zip(param_tys) { if arg.ty() != *ty { - bail!("argument type mismatch"); + bail!( + "argument type mismatch: found {} but expected {}", + arg.ty(), + ty + ); } if !arg.comes_from_same_store(&self.instance.store) { bail!("cross-`Store` values are not currently supported"); @@ -571,7 +576,7 @@ impl Func { for (index, ty) in my_ty.results().iter().enumerate() { unsafe { let ptr = values_vec.as_ptr().add(index); - results.push(Val::read_value_from(ptr, ty)); + results.push(Val::read_value_from(&self.instance.store, ptr, ty)); } } @@ -722,7 +727,10 @@ impl Func { (get15, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15) } - pub(crate) fn store(&self) -> &Store { + // This should be `pub(crate)` but the C API happens to need it in one + // place. + #[doc(hidden)] + pub fn store(&self) -> &Store { &self.instance.store } } @@ -1038,6 +1046,12 @@ impl Caller<'_> { Some(Extern::Memory(mem)) } } + + /// Get a handle to this caller's store. + pub fn store(&self) -> Store { + // See comment above the `store` member for why this unwrap is OK. + Store::upgrade(&self.store).unwrap() + } } macro_rules! impl_into_func { diff --git a/crates/wasmtime/src/ref.rs b/crates/wasmtime/src/ref.rs old mode 100644 new mode 100755 index f78aaf0e9c..5c6cc540cb --- a/crates/wasmtime/src/ref.rs +++ b/crates/wasmtime/src/ref.rs @@ -2,224 +2,182 @@ use std::any::Any; use std::cell::{self, RefCell}; +use std::convert::TryFrom; use std::fmt; +use std::marker::PhantomData; use std::rc::{Rc, Weak}; - -trait InternalRefBase: Any { - fn as_any(&self) -> &dyn Any; - fn host_info(&self) -> Option>>; - fn set_host_info(&self, info: Option>); - fn ptr_eq(&self, other: &dyn InternalRefBase) -> bool; -} - -#[derive(Clone)] -pub struct InternalRef(Rc); - -impl InternalRef { - pub fn is_ref(&self) -> bool { - let r = self.0.as_any(); - Any::is::>(r) - } - pub fn get_ref(&self) -> HostRef { - let r = self.0.as_any(); - r.downcast_ref::>() - .expect("reference is not T type") - .clone() - } -} - -struct AnyAndHostInfo { - any: Box, - host_info: Option>, -} - -#[derive(Clone)] -pub struct OtherRef(Rc>); +use wasmtime_runtime::VMExternRef; /// Represents an opaque reference to any data within WebAssembly. #[derive(Clone)] -pub enum ExternRef { - /// A reference to no data. - Null, - /// A reference to data stored internally in `wasmtime`. - Ref(InternalRef), - /// A reference to data located outside of `wasmtime`. - Other(OtherRef), +pub struct ExternRef { + pub(crate) inner: VMExternRef, + pub(crate) store: Weak, } impl ExternRef { - /// Creates a new instance of `ExternRef` from `Box`. - pub fn new(data: Box) -> Self { - let info = AnyAndHostInfo { - any: data, - host_info: None, - }; - ExternRef::Other(OtherRef(Rc::new(RefCell::new(info)))) + /// Creates a new instance of `ExternRef` wrapping the given value. + pub fn new(store: &crate::Store, value: T) -> ExternRef + where + T: 'static + Any, + { + let inner = VMExternRef::new(value); + let store = store.weak(); + ExternRef { inner, store } } /// Creates a `Null` reference. - pub fn null() -> Self { - ExternRef::Null + pub fn null() -> Option { + None } - /// Returns the data stored in the reference if available. - /// # Panics - /// Panics if the variant isn't `ExternRef::Other`. - pub fn data(&self) -> cell::Ref> { - match self { - ExternRef::Other(OtherRef(r)) => cell::Ref::map(r.borrow(), |r| &r.any), - _ => panic!("expected ExternRef::Other"), - } + /// Get this reference's store. + /// + /// Returns `None` if this reference outlived its store. + pub fn store(&self) -> Option { + crate::runtime::Store::upgrade(&self.store) } - /// Returns true if the two `ExternRef`'s point to the same value (not just - /// values that compare as equal). + /// Get the underlying data for this `ExternRef`. + pub fn data(&self) -> &dyn Any { + &*self.inner + } + + /// Does this `ExternRef` point to the same inner value as `other`?0 + /// + /// This is *only* pointer equality, and does *not* run any inner value's + /// `Eq` implementation. pub fn ptr_eq(&self, other: &ExternRef) -> bool { - match (self, other) { - (ExternRef::Null, ExternRef::Null) => true, - (ExternRef::Ref(InternalRef(ref a)), ExternRef::Ref(InternalRef(ref b))) => { - a.ptr_eq(b.as_ref()) - } - (ExternRef::Other(OtherRef(ref a)), ExternRef::Other(OtherRef(ref b))) => { - Rc::ptr_eq(a, b) - } - _ => false, - } + self.inner == other.inner } - /// Returns a mutable reference to the host information if available. - /// # Panics - /// Panics if `ExternRef` is already borrowed or `ExternRef` is `Null`. - pub fn host_info(&self) -> Option>> { - match self { - ExternRef::Null => panic!("null"), - ExternRef::Ref(r) => r.0.host_info(), - ExternRef::Other(r) => { - let info = cell::RefMut::map(r.0.borrow_mut(), |b| &mut b.host_info); - if info.is_none() { - return None; - } - Some(cell::RefMut::map(info, |info| info.as_mut().unwrap())) - } - } + /// Returns the host information for this `externref`, if previously created + /// with `set_host_info`. + pub fn host_info(&self) -> Option>> { + let store = crate::Store::upgrade(&self.store)?; + store.host_info(self) } - /// Sets the host information for an `ExternRef`. - /// # Panics - /// Panics if `ExternRef` is already borrowed or `ExternRef` is `Null`. - pub fn set_host_info(&self, info: Option>) { - match self { - ExternRef::Null => panic!("null"), - ExternRef::Ref(r) => r.0.set_host_info(info), - ExternRef::Other(r) => { - r.0.borrow_mut().host_info = info; - } - } + /// Set the host information for this `externref`, returning the old host + /// information if it was previously set. + pub fn set_host_info(&self, info: T) -> Option>> + where + T: 'static + Any, + { + let store = crate::Store::upgrade(&self.store)?; + store.set_host_info(self, Some(Rc::new(RefCell::new(info)))) + } + + /// Remove the host information for this `externref`, returning the old host + /// information if it was previously set. + pub fn remove_host_info(&self) -> Option>> { + let store = crate::Store::upgrade(&self.store)?; + store.set_host_info(self, None) } } impl fmt::Debug for ExternRef { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - ExternRef::Null => write!(f, "null"), - ExternRef::Ref(_) => write!(f, "externref"), - ExternRef::Other(_) => write!(f, "other ref"), - } + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let ExternRef { inner, store: _ } = self; + f.debug_struct("ExternRef") + .field("inner", &inner) + .field("store", &"..") + .finish() } } -struct ContentBox { - content: T, - host_info: Option>, - externref_data: Weak, -} - /// Represents a piece of data located in the host environment. -pub struct HostRef(Rc>>); +#[derive(Debug)] +pub struct HostRef +where + T: 'static + Any, +{ + externref: ExternRef, + _phantom: PhantomData, +} -impl HostRef { +impl HostRef +where + T: 'static + Any, +{ /// Creates a new `HostRef` from `T`. - pub fn new(item: T) -> HostRef { - let externref_data: Weak> = Weak::new(); - let content = ContentBox { - content: item, - host_info: None, - externref_data, - }; - HostRef(Rc::new(RefCell::new(content))) + pub fn new(store: &crate::Store, item: T) -> HostRef { + HostRef { + externref: ExternRef::new(store, RefCell::new(item)), + _phantom: PhantomData, + } } /// Immutably borrows the wrapped data. + /// /// # Panics + /// /// Panics if the value is currently mutably borrowed. pub fn borrow(&self) -> cell::Ref { - cell::Ref::map(self.0.borrow(), |b| &b.content) + self.inner().borrow() } /// Mutably borrows the wrapped data. + /// /// # Panics + /// /// Panics if the `HostRef` is already borrowed. pub fn borrow_mut(&self) -> cell::RefMut { - cell::RefMut::map(self.0.borrow_mut(), |b| &mut b.content) + self.inner().borrow_mut() } /// Returns true if the two `HostRef`'s point to the same value (not just /// values that compare as equal). pub fn ptr_eq(&self, other: &HostRef) -> bool { - Rc::ptr_eq(&self.0, &other.0) + self.externref.ptr_eq(&other.externref) } - /// Returns an opaque reference to the wrapped data in the form of - /// an `ExternRef`. - /// # Panics - /// Panics if `HostRef` is already mutably borrowed. - pub fn externref(&self) -> ExternRef { - let r = self.0.borrow_mut().externref_data.upgrade(); - if let Some(r) = r { - return ExternRef::Ref(InternalRef(r)); - } - let externref_data: Rc = Rc::new(self.clone()); - self.0.borrow_mut().externref_data = Rc::downgrade(&externref_data); - ExternRef::Ref(InternalRef(externref_data)) + fn inner(&self) -> &RefCell { + self.externref + .inner + .downcast_ref::>() + .expect("`HostRef`s always wrap an `ExternRef` of `RefCell`") } } -impl InternalRefBase for HostRef { - fn ptr_eq(&self, other: &dyn InternalRefBase) -> bool { - if let Some(other) = other.as_any().downcast_ref() { - self.ptr_eq(other) +impl AsRef for HostRef { + fn as_ref(&self) -> &ExternRef { + &self.externref + } +} + +impl From> for ExternRef +where + T: 'static + Any, +{ + fn from(host: HostRef) -> ExternRef { + host.externref + } +} + +impl TryFrom for HostRef +where + T: 'static + Any, +{ + type Error = ExternRef; + + fn try_from(externref: ExternRef) -> Result { + if externref.inner.is::>() { + Ok(HostRef { + externref, + _phantom: PhantomData, + }) } else { - false + Err(externref) } } - - fn as_any(&self) -> &dyn Any { - self - } - - fn host_info(&self) -> Option>> { - let info = cell::RefMut::map(self.0.borrow_mut(), |b| &mut b.host_info); - if info.is_none() { - return None; - } - Some(cell::RefMut::map(info, |info| info.as_mut().unwrap())) - } - - fn set_host_info(&self, info: Option>) { - self.0.borrow_mut().host_info = info; - } } impl Clone for HostRef { fn clone(&self) -> HostRef { - HostRef(self.0.clone()) - } -} - -impl fmt::Debug for HostRef { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "Ref(")?; - self.0.borrow().content.fmt(f)?; - write!(f, ")") + HostRef { + externref: self.externref.clone(), + _phantom: PhantomData, + } } } diff --git a/crates/wasmtime/src/runtime.rs b/crates/wasmtime/src/runtime.rs index 23168d8800..4c1c514bc8 100644 --- a/crates/wasmtime/src/runtime.rs +++ b/crates/wasmtime/src/runtime.rs @@ -1,8 +1,11 @@ use crate::externals::MemoryCreator; +use crate::r#ref::ExternRef; use crate::trampoline::{MemoryCreatorProxy, StoreInstanceHandle}; use anyhow::{bail, Result}; +use std::any::Any; use std::cell::RefCell; use std::cmp; +use std::collections::HashMap; use std::convert::TryFrom; use std::fmt; use std::path::Path; @@ -14,7 +17,7 @@ use wasmtime_environ::{CacheConfig, Tunables}; use wasmtime_jit::{native, CompilationStrategy, Compiler}; use wasmtime_profiling::{JitDumpAgent, NullProfilerAgent, ProfilingAgent, VTuneAgent}; use wasmtime_runtime::{ - debug_builtins, InstanceHandle, RuntimeMemoryCreator, SignalHandler, VMInterrupts, + debug_builtins, InstanceHandle, RuntimeMemoryCreator, SignalHandler, VMExternRef, VMInterrupts, }; // Runtime Environment @@ -733,6 +736,7 @@ pub(crate) struct StoreInner { compiler: RefCell, instances: RefCell>, signal_handler: RefCell>>>, + host_info: RefCell>>>, } impl Store { @@ -758,6 +762,7 @@ impl Store { compiler: RefCell::new(compiler), instances: RefCell::new(Vec::new()), signal_handler: RefCell::new(None), + host_info: RefCell::new(HashMap::new()), }), } } @@ -809,6 +814,37 @@ impl Store { Rc::downgrade(&self.inner) } + pub(crate) fn upgrade(weak: &Weak) -> Option { + let inner = weak.upgrade()?; + Some(Self { inner }) + } + + pub(crate) fn host_info(&self, externref: &ExternRef) -> Option>> { + debug_assert!( + std::rc::Weak::ptr_eq(&self.weak(), &externref.store), + "externref must be from this store" + ); + let infos = self.inner.host_info.borrow(); + infos.get(&externref.inner).cloned() + } + + pub(crate) fn set_host_info( + &self, + externref: &ExternRef, + info: Option>>, + ) -> Option>> { + debug_assert!( + std::rc::Weak::ptr_eq(&self.weak(), &externref.store), + "externref must be from this store" + ); + let mut infos = self.inner.host_info.borrow_mut(); + if let Some(info) = info { + infos.insert(externref.inner.clone(), info) + } else { + infos.remove(&externref.inner) + } + } + pub(crate) fn signal_handler(&self) -> std::cell::Ref<'_, Option>>> { self.inner.signal_handler.borrow() } diff --git a/crates/wasmtime/src/trampoline/create_handle.rs b/crates/wasmtime/src/trampoline/create_handle.rs index 428c6c35ac..6d78d49f07 100644 --- a/crates/wasmtime/src/trampoline/create_handle.rs +++ b/crates/wasmtime/src/trampoline/create_handle.rs @@ -32,7 +32,12 @@ pub(crate) fn create_handle( .local .signatures .values() - .map(|sig| store.compiler().signatures().register(sig)) + .map(|(wasm, native)| { + store + .compiler() + .signatures() + .register(wasm.clone(), native.clone()) + }) .collect::>(); unsafe { diff --git a/crates/wasmtime/src/trampoline/func.rs b/crates/wasmtime/src/trampoline/func.rs index 41d6ac61b3..1c3a4a31d9 100644 --- a/crates/wasmtime/src/trampoline/func.rs +++ b/crates/wasmtime/src/trampoline/func.rs @@ -225,7 +225,10 @@ pub fn create_handle_with_function( // First up we manufacture a trampoline which has the ABI specified by `ft` // and calls into `stub_fn`... - let sig_id = module.local.signatures.push(sig.clone()); + let sig_id = module + .local + .signatures + .push((ft.to_wasm_func_type(), sig.clone())); let func_id = module.local.functions.push(sig_id); module .exports @@ -244,7 +247,10 @@ pub fn create_handle_with_function( mem::size_of::(), )?; assert!(relocations.is_empty()); - let sig_id = store.compiler().signatures().register(&sig); + let sig_id = store + .compiler() + .signatures() + .register(ft.to_wasm_func_type(), sig); trampolines.insert(sig_id, trampoline); // Next up we wrap everything up into an `InstanceHandle` by publishing our @@ -285,13 +291,19 @@ pub unsafe fn create_handle_with_raw_function( let mut finished_functions = PrimaryMap::new(); let mut trampolines = HashMap::new(); - let sig_id = module.local.signatures.push(sig.clone()); + let sig_id = module + .local + .signatures + .push((ft.to_wasm_func_type(), sig.clone())); let func_id = module.local.functions.push(sig_id); module .exports .insert("trampoline".to_string(), EntityIndex::Function(func_id)); finished_functions.push(func); - let sig_id = store.compiler().signatures().register(&sig); + let sig_id = store + .compiler() + .signatures() + .register(ft.to_wasm_func_type(), sig); trampolines.insert(sig_id, trampoline); create_handle(module, store, finished_functions, trampolines, state) diff --git a/crates/wasmtime/src/types.rs b/crates/wasmtime/src/types.rs index e03c6e41c0..75c67ae188 100644 --- a/crates/wasmtime/src/types.rs +++ b/crates/wasmtime/src/types.rs @@ -67,6 +67,20 @@ pub enum ValType { FuncRef, } +impl fmt::Display for ValType { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + ValType::I32 => write!(f, "i32"), + ValType::I64 => write!(f, "i64"), + ValType::F32 => write!(f, "f32"), + ValType::F64 => write!(f, "f64"), + ValType::V128 => write!(f, "v128"), + ValType::ExternRef => write!(f, "externref"), + ValType::FuncRef => write!(f, "funcref"), + } + } +} + impl ValType { /// Returns true if `ValType` matches any of the numeric types. (e.g. `I32`, /// `I64`, `F32`, `F64`). @@ -106,6 +120,31 @@ impl ValType { _ => None, } } + + pub(crate) fn to_wasm_type(&self) -> wasm::WasmType { + match self { + Self::I32 => wasm::WasmType::I32, + Self::I64 => wasm::WasmType::I64, + Self::F32 => wasm::WasmType::F32, + Self::F64 => wasm::WasmType::F64, + Self::V128 => wasm::WasmType::V128, + Self::FuncRef => wasm::WasmType::FuncRef, + Self::ExternRef => wasm::WasmType::ExternRef, + } + } + + pub(crate) fn from_wasm_type(ty: &wasm::WasmType) -> Option { + match ty { + wasm::WasmType::I32 => Some(Self::I32), + wasm::WasmType::I64 => Some(Self::I64), + wasm::WasmType::F32 => Some(Self::F32), + wasm::WasmType::F64 => Some(Self::F64), + wasm::WasmType::V128 => Some(Self::V128), + wasm::WasmType::FuncRef => Some(Self::FuncRef), + wasm::WasmType::ExternRef => Some(Self::ExternRef), + wasm::WasmType::Func | wasm::WasmType::EmptyBlockType => None, + } + } } // External Types @@ -184,12 +223,6 @@ impl From for ExternType { } } -// Function Types -fn from_wasmtime_abiparam(param: &ir::AbiParam) -> Option { - assert_eq!(param.purpose, ir::ArgumentPurpose::Normal); - ValType::from_wasmtime_type(param.value_type) -} - /// A descriptor for a function in a WebAssembly module. /// /// WebAssembly functions can have 0 or more parameters and results. @@ -218,6 +251,13 @@ impl FuncType { &self.results } + pub(crate) fn to_wasm_func_type(&self) -> wasm::WasmFuncType { + wasm::WasmFuncType { + params: self.params.iter().map(|p| p.to_wasm_type()).collect(), + returns: self.results.iter().map(|r| r.to_wasm_type()).collect(), + } + } + /// Returns `Some` if this function signature was compatible with cranelift, /// or `None` if one of the types/results wasn't supported or compatible /// with cranelift. @@ -251,17 +291,16 @@ impl FuncType { /// Returns `None` if any types in the signature can't be converted to the /// types in this crate, but that should very rarely happen and largely only /// indicate a bug in our cranelift integration. - pub(crate) fn from_wasmtime_signature(signature: &ir::Signature) -> Option { + pub(crate) fn from_wasm_func_type(signature: &wasm::WasmFuncType) -> Option { let params = signature .params .iter() - .skip(2) // skip the caller/callee vmctx - .map(|p| from_wasmtime_abiparam(p)) + .map(|p| ValType::from_wasm_type(p)) .collect::>>()?; let results = signature .returns .iter() - .map(|p| from_wasmtime_abiparam(p)) + .map(|r| ValType::from_wasm_type(r)) .collect::>>()?; Some(FuncType { params: params.into_boxed_slice(), @@ -390,7 +429,7 @@ impl MemoryType { #[derive(Clone, Hash, Eq, PartialEq)] pub(crate) enum EntityType<'module> { - Function(&'module ir::Signature), + Function(&'module wasm::WasmFuncType), Table(&'module wasm::Table), Memory(&'module wasm::Memory), Global(&'module wasm::Global), @@ -404,7 +443,7 @@ impl<'module> EntityType<'module> { ) -> EntityType<'module> { match entity_index { EntityIndex::Function(func_index) => { - let sig = module.local.func_signature(*func_index); + let sig = module.local.wasm_func_type(*func_index); EntityType::Function(&sig) } EntityIndex::Table(table_index) => { @@ -422,7 +461,7 @@ impl<'module> EntityType<'module> { /// Convert this `EntityType` to an `ExternType`. pub(crate) fn extern_type(&self) -> ExternType { match self { - EntityType::Function(sig) => FuncType::from_wasmtime_signature(sig) + EntityType::Function(sig) => FuncType::from_wasm_func_type(sig) .expect("core wasm function type should be supported") .into(), EntityType::Table(table) => TableType::from_wasmtime_table(table).into(), diff --git a/crates/wasmtime/src/values.rs b/crates/wasmtime/src/values.rs index c0b8dd8102..07588cabb8 100644 --- a/crates/wasmtime/src/values.rs +++ b/crates/wasmtime/src/values.rs @@ -2,6 +2,7 @@ use crate::r#ref::ExternRef; use crate::{Func, Store, ValType}; use anyhow::{bail, Result}; use std::ptr; +use wasmtime_runtime::VMExternRef; /// Possible runtime values that a WebAssembly module can either consume or /// produce. @@ -26,9 +27,7 @@ pub enum Val { F64(u64), /// An `externref` value which can hold opaque data to the wasm instance itself. - /// - /// Note that this is a nullable value as well. - ExternRef(ExternRef), + ExternRef(Option), /// A first-class reference to a WebAssembly function. FuncRef(Func), @@ -87,18 +86,31 @@ impl Val { Val::F32(u) => ptr::write(p as *mut u32, *u), Val::F64(u) => ptr::write(p as *mut u64, *u), Val::V128(b) => ptr::write(p as *mut u128, *b), + Val::ExternRef(None) => ptr::write(p, 0), + Val::ExternRef(Some(x)) => ptr::write(p as *mut *mut u8, x.inner.clone().into_raw()), _ => unimplemented!("Val::write_value_to"), } } - pub(crate) unsafe fn read_value_from(p: *const u128, ty: &ValType) -> Val { + pub(crate) unsafe fn read_value_from(store: &Store, p: *const u128, ty: &ValType) -> Val { match ty { ValType::I32 => Val::I32(ptr::read(p as *const i32)), ValType::I64 => Val::I64(ptr::read(p as *const i64)), ValType::F32 => Val::F32(ptr::read(p as *const u32)), ValType::F64 => Val::F64(ptr::read(p as *const u64)), ValType::V128 => Val::V128(ptr::read(p as *const u128)), - _ => unimplemented!("Val::read_value_from"), + ValType::ExternRef => { + let raw = ptr::read(p as *const *mut u8); + if raw.is_null() { + Val::ExternRef(None) + } else { + Val::ExternRef(Some(ExternRef { + inner: VMExternRef::from_raw(raw), + store: store.weak(), + })) + } + } + _ => unimplemented!("Val::read_value_from: {:?}", ty), } } @@ -112,24 +124,31 @@ impl Val { (V128(u128) v128 unwrap_v128 *e) } - /// Attempt to access the underlying value of this `Val`, returning - /// `None` if it is not the correct type. + /// Attempt to access the underlying `externref` value of this `Val`. /// - /// This will return `Some` for both the `ExternRef` and `FuncRef` types. - pub fn externref(&self) -> Option { + /// If this is not an `externref`, then `None` is returned. + /// + /// If this is a null `externref`, then `Some(None)` is returned. + /// + /// If this is a non-null `externref`, then `Some(Some(..))` is returned. + pub fn externref(&self) -> Option> { match self { Val::ExternRef(e) => Some(e.clone()), _ => None, } } - /// Returns the underlying value of this `Val`, panicking if it's the + /// Returns the underlying `externref` value of this `Val`, panicking if it's the /// wrong type. /// + /// If this is a null `externref`, then `None` is returned. + /// + /// If this is a non-null `externref`, then `Some(..)` is returned. + /// /// # Panics /// - /// Panics if `self` is not of the right type. - pub fn unwrap_externref(&self) -> ExternRef { + /// Panics if `self` is not a (nullable) `externref`. + pub fn unwrap_externref(&self) -> Option { self.externref().expect("expected externref") } @@ -140,8 +159,8 @@ impl Val { // TODO: need to implement this once we actually finalize what // `externref` will look like and it's actually implemented to pass it // to compiled wasm as well. - Val::ExternRef(ExternRef::Ref(_)) | Val::ExternRef(ExternRef::Other(_)) => false, - Val::ExternRef(ExternRef::Null) => true, + Val::ExternRef(Some(e)) => e.store.ptr_eq(&store.weak()), + Val::ExternRef(None) => true, // Integers have no association with any particular store, so // they're always considered as "yes I came from that store", @@ -176,6 +195,12 @@ impl From for Val { impl From for Val { fn from(val: ExternRef) -> Val { + Val::ExternRef(Some(val)) + } +} + +impl From> for Val { + fn from(val: Option) -> Val { Val::ExternRef(val) } } @@ -194,7 +219,7 @@ pub(crate) fn into_checked_anyfunc( bail!("cross-`Store` values are not supported"); } Ok(match val { - Val::ExternRef(ExternRef::Null) => wasmtime_runtime::VMCallerCheckedAnyfunc { + Val::ExternRef(None) => wasmtime_runtime::VMCallerCheckedAnyfunc { func_ptr: ptr::null(), type_index: wasmtime_runtime::VMSharedSignatureIndex::default(), vmctx: ptr::null_mut(), @@ -216,7 +241,7 @@ pub(crate) fn from_checked_anyfunc( store: &Store, ) -> Val { if item.type_index == wasmtime_runtime::VMSharedSignatureIndex::default() { - return Val::ExternRef(ExternRef::Null); + return Val::ExternRef(None); } let instance_handle = unsafe { wasmtime_runtime::InstanceHandle::from_vmctx(item.vmctx) }; let export = wasmtime_runtime::ExportFunction { diff --git a/crates/wast/src/spectest.rs b/crates/wast/src/spectest.rs index ed10b77862..f49125dbe8 100644 --- a/crates/wast/src/spectest.rs +++ b/crates/wast/src/spectest.rs @@ -35,7 +35,7 @@ pub fn link_spectest(linker: &mut Linker) -> Result<()> { linker.define("spectest", "global_f64", g)?; let ty = TableType::new(ValType::FuncRef, Limits::new(10, Some(20))); - let table = Table::new(linker.store(), ty, Val::ExternRef(ExternRef::Null))?; + let table = Table::new(linker.store(), ty, Val::ExternRef(None))?; linker.define("spectest", "table", table)?; let ty = MemoryType::new(Limits::new(1, Some(2))); diff --git a/crates/wast/src/wast.rs b/crates/wast/src/wast.rs index 5574525dae..303df76100 100644 --- a/crates/wast/src/wast.rs +++ b/crates/wast/src/wast.rs @@ -3,11 +3,14 @@ use anyhow::{anyhow, bail, Context as _, Result}; use std::path::Path; use std::str; use wasmtime::*; -use wast::parser::{self, ParseBuffer}; use wast::Wat; +use wast::{ + parser::{self, ParseBuffer}, + RefType, +}; /// Translate from a `script::Value` to a `RuntimeValue`. -fn runtime_value(v: &wast::Expression<'_>) -> Result { +fn runtime_value(store: &Store, v: &wast::Expression<'_>) -> Result { use wast::Instruction::*; if v.instrs.len() != 1 { @@ -19,6 +22,8 @@ fn runtime_value(v: &wast::Expression<'_>) -> Result { F32Const(x) => Val::F32(x.bits), F64Const(x) => Val::F64(x.bits), V128Const(x) => Val::V128(u128::from_le_bytes(x.to_le_bytes())), + RefNull(RefType::Extern) => Val::ExternRef(None), + RefExtern(x) => Val::ExternRef(Some(ExternRef::new(store, *x))), other => bail!("couldn't convert {:?} to a runtime value", other), }) } @@ -114,7 +119,7 @@ impl WastContext { let values = exec .args .iter() - .map(runtime_value) + .map(|v| runtime_value(&self.store, v)) .collect::>>()?; self.invoke(exec.module.map(|i| i.name()), exec.name, &values) } @@ -403,6 +408,18 @@ fn val_matches(actual: &Val, expected: &wast::AssertExpression) -> Result (Val::F32(a), wast::AssertExpression::F32(b)) => f32_matches(*a, b), (Val::F64(a), wast::AssertExpression::F64(b)) => f64_matches(*a, b), (Val::V128(a), wast::AssertExpression::V128(b)) => v128_matches(*a, b), + (Val::ExternRef(x), wast::AssertExpression::RefNull(wast::RefType::Extern)) => x.is_none(), + (Val::ExternRef(x), wast::AssertExpression::RefExtern(y)) => { + if let Some(x) = x { + let x = x + .data() + .downcast_ref::() + .expect("only u32 externrefs created in wast test suites"); + x == y + } else { + false + } + } _ => bail!( "don't know how to compare {:?} and {:?} yet", actual, diff --git a/tests/all/externals.rs b/tests/all/externals.rs index 0919f13685..281365d18e 100644 --- a/tests/all/externals.rs +++ b/tests/all/externals.rs @@ -28,47 +28,27 @@ fn bad_tables() { // get out of bounds let ty = TableType::new(ValType::FuncRef, Limits::new(0, Some(1))); - let t = Table::new( - &Store::default(), - ty.clone(), - Val::ExternRef(ExternRef::Null), - ) - .unwrap(); + let t = Table::new(&Store::default(), ty.clone(), Val::ExternRef(None)).unwrap(); assert!(t.get(0).is_none()); assert!(t.get(u32::max_value()).is_none()); // set out of bounds or wrong type let ty = TableType::new(ValType::FuncRef, Limits::new(1, Some(1))); - let t = Table::new( - &Store::default(), - ty.clone(), - Val::ExternRef(ExternRef::Null), - ) - .unwrap(); + let t = Table::new(&Store::default(), ty.clone(), Val::ExternRef(None)).unwrap(); assert!(t.set(0, Val::I32(0)).is_err()); - assert!(t.set(0, Val::ExternRef(ExternRef::Null)).is_ok()); - assert!(t.set(1, Val::ExternRef(ExternRef::Null)).is_err()); + assert!(t.set(0, Val::ExternRef(None)).is_ok()); + assert!(t.set(1, Val::ExternRef(None)).is_err()); // grow beyond max let ty = TableType::new(ValType::FuncRef, Limits::new(1, Some(1))); - let t = Table::new( - &Store::default(), - ty.clone(), - Val::ExternRef(ExternRef::Null), - ) - .unwrap(); - assert!(t.grow(0, Val::ExternRef(ExternRef::Null)).is_ok()); - assert!(t.grow(1, Val::ExternRef(ExternRef::Null)).is_err()); + let t = Table::new(&Store::default(), ty.clone(), Val::ExternRef(None)).unwrap(); + assert!(t.grow(0, Val::ExternRef(None)).is_ok()); + assert!(t.grow(1, Val::ExternRef(None)).is_err()); assert_eq!(t.size(), 1); // grow wrong type let ty = TableType::new(ValType::FuncRef, Limits::new(1, Some(2))); - let t = Table::new( - &Store::default(), - ty.clone(), - Val::ExternRef(ExternRef::Null), - ) - .unwrap(); + let t = Table::new(&Store::default(), ty.clone(), Val::ExternRef(None)).unwrap(); assert!(t.grow(1, Val::I32(0)).is_err()); assert_eq!(t.size(), 1); } @@ -88,7 +68,7 @@ fn cross_store() -> anyhow::Result<()> { let ty = MemoryType::new(Limits::new(1, None)); let memory = Memory::new(&store2, ty); let ty = TableType::new(ValType::FuncRef, Limits::new(1, None)); - let table = Table::new(&store2, ty, Val::ExternRef(ExternRef::Null))?; + let table = Table::new(&store2, ty, Val::ExternRef(None))?; let need_func = Module::new(&store1, r#"(module (import "" "" (func)))"#)?; assert!(Instance::new(&need_func, &[func.into()]).is_err()); diff --git a/tests/all/linker.rs b/tests/all/linker.rs index b2c7a64da5..958dcc8ecc 100644 --- a/tests/all/linker.rs +++ b/tests/all/linker.rs @@ -56,11 +56,11 @@ fn link_twice_bad() -> Result<()> { // tables let ty = TableType::new(ValType::FuncRef, Limits::new(1, None)); - let table = Table::new(&store, ty, Val::ExternRef(ExternRef::Null))?; + let table = Table::new(&store, ty, Val::ExternRef(None))?; linker.define("", "", table.clone())?; assert!(linker.define("", "", table.clone()).is_err()); let ty = TableType::new(ValType::FuncRef, Limits::new(2, None)); - let table = Table::new(&store, ty, Val::ExternRef(ExternRef::Null))?; + let table = Table::new(&store, ty, Val::ExternRef(None))?; assert!(linker.define("", "", table.clone()).is_err()); Ok(()) } diff --git a/tests/all/table.rs b/tests/all/table.rs index f7599e87cc..9bf7cb4cc4 100644 --- a/tests/all/table.rs +++ b/tests/all/table.rs @@ -4,9 +4,9 @@ use wasmtime::*; fn get_none() { let store = Store::default(); let ty = TableType::new(ValType::FuncRef, Limits::new(1, None)); - let table = Table::new(&store, ty, Val::ExternRef(ExternRef::Null)).unwrap(); + let table = Table::new(&store, ty, Val::ExternRef(None)).unwrap(); match table.get(0) { - Some(Val::ExternRef(ExternRef::Null)) => {} + Some(Val::ExternRef(None)) => {} _ => panic!(), } assert!(table.get(1).is_none()); diff --git a/tests/all/traps.rs b/tests/all/traps.rs index 575e6741b8..f6cc691a6b 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -341,7 +341,7 @@ fn mismatched_arguments() -> Result<()> { ); assert_eq!( func.call(&[Val::F32(0)]).unwrap_err().to_string(), - "argument type mismatch", + "argument type mismatch: found f32 but expected i32", ); assert_eq!( func.call(&[Val::I32(0), Val::I32(1)]) diff --git a/tests/all/use_after_drop.rs b/tests/all/use_after_drop.rs index 67edfc158a..625bac897f 100644 --- a/tests/all/use_after_drop.rs +++ b/tests/all/use_after_drop.rs @@ -11,7 +11,7 @@ fn use_func_after_drop() -> Result<()> { assert_eq!(closed_over_data, "abcd"); }); let ty = TableType::new(ValType::FuncRef, Limits::new(1, None)); - table = Table::new(&store, ty, Val::ExternRef(ExternRef::Null))?; + table = Table::new(&store, ty, Val::ExternRef(None))?; table.set(0, func.into())?; } let func = table.get(0).unwrap().funcref().unwrap().clone(); diff --git a/tests/misc_testsuite/reference-types/externref-id-function.wast b/tests/misc_testsuite/reference-types/externref-id-function.wast new file mode 100644 index 0000000000..0262d33f9c --- /dev/null +++ b/tests/misc_testsuite/reference-types/externref-id-function.wast @@ -0,0 +1,8 @@ +(module + (func (export "identity") (param externref) (result externref) + local.get 0)) + +(assert_return (invoke "identity" (ref.null extern)) + (ref.null extern)) +(assert_return (invoke "identity" (ref.extern 1)) + (ref.extern 1)) From 25548d7fbe6a2999ef73807dbd9dc023f0bff690 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 1 Jun 2020 13:51:08 -0700 Subject: [PATCH 07/14] externref: Share more `Layout`-computing code --- crates/runtime/src/externref.rs | 64 +++++++++++++++------------------ 1 file changed, 29 insertions(+), 35 deletions(-) diff --git a/crates/runtime/src/externref.rs b/crates/runtime/src/externref.rs index 9f2ea1e481..10cc2f12ad 100644 --- a/crates/runtime/src/externref.rs +++ b/crates/runtime/src/externref.rs @@ -183,6 +183,30 @@ impl Drop for VMExternRef { } impl VMExternData { + /// Get the `Layout` for a value with the given size and alignment, and the + /// offset within that layout where the `VMExternData` footer resides. + /// + /// This doesn't take a `value: &T` because `VMExternRef::new_with` hasn't + /// constructed a `T` value yet, and it isn't generic over `T` because + /// `VMExternData::drop_and_dealloc` doesn't know what `T` to use, and has + /// to use `std::mem::{size,align}_of_val` instead. + unsafe fn layout_for(value_size: usize, value_align: usize) -> (Layout, usize) { + let extern_data_size = mem::size_of::(); + let extern_data_align = mem::align_of::(); + + let value_and_padding_size = round_up_to_align(value_size, extern_data_align).unwrap(); + + let alloc_align = std::cmp::max(value_align, extern_data_align); + let alloc_size = value_and_padding_size + extern_data_size; + + debug_assert!(Layout::from_size_align(alloc_size, alloc_align).is_ok()); + ( + Layout::from_size_align_unchecked(alloc_size, alloc_align), + value_and_padding_size, + ) + } + + /// Drop the inner value and then free this `VMExternData` heap allocation. unsafe fn drop_and_dealloc(mut data: NonNull) { // Note: we introduce a block scope so that we drop the live // reference to the data before we free the heap allocation it @@ -193,23 +217,9 @@ impl VMExternData { // Same thing, but for the dropping the reference to `value` before // we drop it itself. - let layout = { + let (layout, _) = { let value = data.value_ptr.as_ref(); - - let value_size = mem::size_of_val(value); - let value_align = mem::align_of_val(value); - - let extern_data_size = mem::size_of::(); - let extern_data_align = mem::align_of::(); - - let value_and_padding_size = round_up_to_align(value_size, extern_data_align) - .unwrap_or_else(|| unreachable!()); - - let alloc_align = std::cmp::max(value_align, extern_data_align); - let alloc_size = value_and_padding_size + extern_data_size; - - debug_assert!(Layout::from_size_align(alloc_size, alloc_align).is_ok()); - Layout::from_size_align_unchecked(alloc_size, alloc_align) + Self::layout_for(mem::size_of_val(value), mem::align_of_val(value)) }; ptr::drop_in_place(data.value_ptr.as_ptr()); @@ -265,25 +275,9 @@ impl VMExternRef { where T: 'static + Any, { - let value_size = mem::size_of::(); - let value_align = mem::align_of::(); - - let extern_data_align = mem::align_of::(); - let extern_data_size = mem::size_of::(); - - let value_and_padding_size = round_up_to_align(value_size, extern_data_align) - .unwrap_or_else(|| { - Self::alloc_failure(); - }); - - let alloc_align = std::cmp::max(value_align, extern_data_align); - let alloc_size = value_and_padding_size - .checked_add(extern_data_size) - .unwrap_or_else(|| Self::alloc_failure()); - unsafe { - debug_assert!(Layout::from_size_align(alloc_size, alloc_align).is_ok()); - let layout = Layout::from_size_align_unchecked(alloc_size, alloc_align); + let (layout, footer_offset) = + VMExternData::layout_for(mem::size_of::(), mem::align_of::()); let alloc_ptr = std::alloc::alloc(layout); let alloc_ptr = NonNull::new(alloc_ptr).unwrap_or_else(|| { @@ -300,7 +294,7 @@ impl VMExternRef { let value_ptr = NonNull::new_unchecked(value_ptr); let extern_data_ptr = - alloc_ptr.cast::().as_ptr().add(value_and_padding_size) as *mut VMExternData; + alloc_ptr.cast::().as_ptr().add(footer_offset) as *mut VMExternData; ptr::write( extern_data_ptr, VMExternData { From 98da9ee8a96aeb5bea3af86f8096236411e7937c Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 1 Jun 2020 13:54:26 -0700 Subject: [PATCH 08/14] Use `std::alloc::handle_alloc_failure` instead of home-rolled version --- crates/runtime/src/externref.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/crates/runtime/src/externref.rs b/crates/runtime/src/externref.rs index 10cc2f12ad..1a43e33932 100644 --- a/crates/runtime/src/externref.rs +++ b/crates/runtime/src/externref.rs @@ -281,7 +281,7 @@ impl VMExternRef { let alloc_ptr = std::alloc::alloc(layout); let alloc_ptr = NonNull::new(alloc_ptr).unwrap_or_else(|| { - Self::alloc_failure(); + std::alloc::handle_alloc_error(layout); }); let value_ptr = alloc_ptr.cast::(); @@ -338,12 +338,6 @@ impl VMExternRef { VMExternRef(NonNull::new_unchecked(ptr).cast()) } - #[inline(never)] - #[cold] - fn alloc_failure() -> ! { - panic!("VMExternRef allocation failure") - } - #[inline] fn extern_data(&self) -> &VMExternData { unsafe { self.0.as_ref() } From b78eafcfd33bf487b12b2257282d6353bc9f84ec Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 1 Jun 2020 14:17:04 -0700 Subject: [PATCH 09/14] externref: Do not impl common traits, have free functions instead If you aren't expecting `VMExternRef`'s pointer-equality semantics, then these trait implementations can be foot guns. Instead of implementing the trait, make free functions in the `VMExternRef` namespace. This way, callers have to be a little more explicit. --- crates/runtime/src/externref.rs | 52 ++++++++++++++++++--------------- crates/wasmtime/src/ref.rs | 2 +- crates/wasmtime/src/runtime.rs | 28 +++++++++++++++--- 3 files changed, 53 insertions(+), 29 deletions(-) diff --git a/crates/runtime/src/externref.rs b/crates/runtime/src/externref.rs index 1a43e33932..8e8aeecfea 100644 --- a/crates/runtime/src/externref.rs +++ b/crates/runtime/src/externref.rs @@ -77,7 +77,7 @@ use std::alloc::Layout; use std::any::Any; use std::cell::UnsafeCell; use std::cmp::Ordering; -use std::hash::{Hash, Hasher}; +use std::hash::Hasher; use std::mem; use std::ops::Deref; use std::ptr::{self, NonNull}; @@ -344,39 +344,43 @@ impl VMExternRef { } } -impl PartialEq for VMExternRef { +/// Methods that would normally be trait implementations, but aren't to avoid +/// potential footguns around `VMExternRef`'s pointer-equality semantics. +/// +/// Note that none of these methods are on `&self`, they all require a +/// fully-qualified `VMExternRef::foo(my_ref)` invocation. +impl VMExternRef { + /// Check whether two `VMExternRef`s point to the same inner allocation. + /// + /// Note that this uses pointer-equality semantics, not structural-equality + /// semantics, and so only pointers are compared, and doesn't use any `Eq` + /// or `PartialEq` implementation of the pointed-to values. #[inline] - fn eq(&self, rhs: &Self) -> bool { - ptr::eq(self.0.as_ptr() as *const _, rhs.0.as_ptr() as *const _) + pub fn eq(a: &Self, b: &Self) -> bool { + ptr::eq(a.0.as_ptr() as *const _, b.0.as_ptr() as *const _) } -} -impl Eq for VMExternRef {} - -impl Hash for VMExternRef { + /// Hash a given `VMExternRef`. + /// + /// Note that this just hashes the pointer to the inner value, it does *not* + /// use the inner value's `Hash` implementation (if any). #[inline] - fn hash(&self, hasher: &mut H) + pub fn hash(externref: &Self, hasher: &mut H) where H: Hasher, { - ptr::hash(self.0.as_ptr() as *const _, hasher); + ptr::hash(externref.0.as_ptr() as *const _, hasher); } -} -impl PartialOrd for VMExternRef { + /// Compare two `VMExternRef`s. + /// + /// Note that this uses pointer-equality semantics, not structural-equality + /// semantics, and so only pointers are compared, and doesn't use any `Cmp` + /// or `PartialCmp` implementation of the pointed-to values. #[inline] - fn partial_cmp(&self, rhs: &Self) -> Option { - let a = self.0.as_ptr() as usize; - let b = rhs.0.as_ptr() as usize; - a.partial_cmp(&b) - } -} - -impl Ord for VMExternRef { - #[inline] - fn cmp(&self, rhs: &Self) -> Ordering { - let a = self.0.as_ptr() as usize; - let b = rhs.0.as_ptr() as usize; + pub fn cmp(a: &Self, b: &Self) -> Ordering { + let a = a.0.as_ptr() as usize; + let b = b.0.as_ptr() as usize; a.cmp(&b) } } diff --git a/crates/wasmtime/src/ref.rs b/crates/wasmtime/src/ref.rs index 5c6cc540cb..d779b8cc76 100755 --- a/crates/wasmtime/src/ref.rs +++ b/crates/wasmtime/src/ref.rs @@ -48,7 +48,7 @@ impl ExternRef { /// This is *only* pointer equality, and does *not* run any inner value's /// `Eq` implementation. pub fn ptr_eq(&self, other: &ExternRef) -> bool { - self.inner == other.inner + VMExternRef::eq(&self.inner, &other.inner) } /// Returns the host information for this `externref`, if previously created diff --git a/crates/wasmtime/src/runtime.rs b/crates/wasmtime/src/runtime.rs index 4c1c514bc8..c2e62dcf5c 100644 --- a/crates/wasmtime/src/runtime.rs +++ b/crates/wasmtime/src/runtime.rs @@ -8,6 +8,7 @@ use std::cmp; use std::collections::HashMap; use std::convert::TryFrom; use std::fmt; +use std::hash::{Hash, Hasher}; use std::path::Path; use std::rc::{Rc, Weak}; use std::sync::Arc; @@ -736,7 +737,26 @@ pub(crate) struct StoreInner { compiler: RefCell, instances: RefCell>, signal_handler: RefCell>>>, - host_info: RefCell>>>, + host_info: RefCell>>>, +} + +struct HostInfoKey(VMExternRef); + +impl PartialEq for HostInfoKey { + fn eq(&self, rhs: &Self) -> bool { + VMExternRef::eq(&self.0, &rhs.0) + } +} + +impl Eq for HostInfoKey {} + +impl Hash for HostInfoKey { + fn hash(&self, hasher: &mut H) + where + H: Hasher, + { + VMExternRef::hash(&self.0, hasher); + } } impl Store { @@ -825,7 +845,7 @@ impl Store { "externref must be from this store" ); let infos = self.inner.host_info.borrow(); - infos.get(&externref.inner).cloned() + infos.get(&HostInfoKey(externref.inner.clone())).cloned() } pub(crate) fn set_host_info( @@ -839,9 +859,9 @@ impl Store { ); let mut infos = self.inner.host_info.borrow_mut(); if let Some(info) = info { - infos.insert(externref.inner.clone(), info) + infos.insert(HostInfoKey(externref.inner.clone()), info) } else { - infos.remove(&externref.inner) + infos.remove(&HostInfoKey(externref.inner.clone())) } } From 652f47495aeb7c27496292eb9eff3944b3bff31a Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 1 Jun 2020 14:28:08 -0700 Subject: [PATCH 10/14] wasmtime: Make `Func::store` a public method --- crates/wasmtime/src/func.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index febd02b76e..95fd6460dc 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -727,9 +727,7 @@ impl Func { (get15, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15) } - // This should be `pub(crate)` but the C API happens to need it in one - // place. - #[doc(hidden)] + /// Get a reference to this function's store. pub fn store(&self) -> &Store { &self.instance.store } From d5bdce99c76f8e2deaf18c2d89e5d034943317d9 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 1 Jun 2020 14:29:07 -0700 Subject: [PATCH 11/14] Remove executable bit from Rust source files --- cranelift/peepmatic/crates/runtime/src/lib.rs | 0 cranelift/peepmatic/src/lib.rs | 0 cranelift/src/clif-util.rs | 0 crates/environ/src/data_structures.rs | 0 crates/wasmtime/src/ref.rs | 0 fuzz/fuzz_targets/api_calls.rs | 0 fuzz/fuzz_targets/compile.rs | 0 fuzz/fuzz_targets/differential.rs | 0 fuzz/fuzz_targets/instantiate.rs | 0 fuzz/fuzz_targets/instantiate_translated.rs | 0 10 files changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 cranelift/peepmatic/crates/runtime/src/lib.rs mode change 100755 => 100644 cranelift/peepmatic/src/lib.rs mode change 100755 => 100644 cranelift/src/clif-util.rs mode change 100755 => 100644 crates/environ/src/data_structures.rs mode change 100755 => 100644 crates/wasmtime/src/ref.rs mode change 100755 => 100644 fuzz/fuzz_targets/api_calls.rs mode change 100755 => 100644 fuzz/fuzz_targets/compile.rs mode change 100755 => 100644 fuzz/fuzz_targets/differential.rs mode change 100755 => 100644 fuzz/fuzz_targets/instantiate.rs mode change 100755 => 100644 fuzz/fuzz_targets/instantiate_translated.rs diff --git a/cranelift/peepmatic/crates/runtime/src/lib.rs b/cranelift/peepmatic/crates/runtime/src/lib.rs old mode 100755 new mode 100644 diff --git a/cranelift/peepmatic/src/lib.rs b/cranelift/peepmatic/src/lib.rs old mode 100755 new mode 100644 diff --git a/cranelift/src/clif-util.rs b/cranelift/src/clif-util.rs old mode 100755 new mode 100644 diff --git a/crates/environ/src/data_structures.rs b/crates/environ/src/data_structures.rs old mode 100755 new mode 100644 diff --git a/crates/wasmtime/src/ref.rs b/crates/wasmtime/src/ref.rs old mode 100755 new mode 100644 diff --git a/fuzz/fuzz_targets/api_calls.rs b/fuzz/fuzz_targets/api_calls.rs old mode 100755 new mode 100644 diff --git a/fuzz/fuzz_targets/compile.rs b/fuzz/fuzz_targets/compile.rs old mode 100755 new mode 100644 diff --git a/fuzz/fuzz_targets/differential.rs b/fuzz/fuzz_targets/differential.rs old mode 100755 new mode 100644 diff --git a/fuzz/fuzz_targets/instantiate.rs b/fuzz/fuzz_targets/instantiate.rs old mode 100755 new mode 100644 diff --git a/fuzz/fuzz_targets/instantiate_translated.rs b/fuzz/fuzz_targets/instantiate_translated.rs old mode 100755 new mode 100644 From 2bb6a82794ba03627e534eae081eebd1815382b5 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 1 Jun 2020 14:30:51 -0700 Subject: [PATCH 12/14] Remove the `ExternRef::null` constructor Shorter and clearer to just use `None`. --- crates/wasmtime/src/ref.rs | 5 ----- crates/wasmtime/src/values.rs | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/crates/wasmtime/src/ref.rs b/crates/wasmtime/src/ref.rs index d779b8cc76..358cc0451c 100644 --- a/crates/wasmtime/src/ref.rs +++ b/crates/wasmtime/src/ref.rs @@ -26,11 +26,6 @@ impl ExternRef { ExternRef { inner, store } } - /// Creates a `Null` reference. - pub fn null() -> Option { - None - } - /// Get this reference's store. /// /// Returns `None` if this reference outlived its store. diff --git a/crates/wasmtime/src/values.rs b/crates/wasmtime/src/values.rs index 07588cabb8..9c598cd513 100644 --- a/crates/wasmtime/src/values.rs +++ b/crates/wasmtime/src/values.rs @@ -63,7 +63,7 @@ macro_rules! accessors { impl Val { /// Returns a null `externref` value. pub fn null() -> Val { - Val::ExternRef(ExternRef::null()) + Val::ExternRef(None) } /// Returns the corresponding [`ValType`] for this `Val`. From 8adae5d1adabe7aa0a6ece7fc96d054837ed0b4e Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 1 Jun 2020 14:40:45 -0700 Subject: [PATCH 13/14] Create a simple `Debug` implementation for `Store` --- crates/wasmtime/src/ref.rs | 3 ++- crates/wasmtime/src/runtime.rs | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/wasmtime/src/ref.rs b/crates/wasmtime/src/ref.rs index 358cc0451c..b8bd3693e5 100644 --- a/crates/wasmtime/src/ref.rs +++ b/crates/wasmtime/src/ref.rs @@ -74,9 +74,10 @@ impl ExternRef { impl fmt::Debug for ExternRef { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let ExternRef { inner, store: _ } = self; + let store = self.store(); f.debug_struct("ExternRef") .field("inner", &inner) - .field("store", &"..") + .field("store", &store) .finish() } } diff --git a/crates/wasmtime/src/runtime.rs b/crates/wasmtime/src/runtime.rs index c2e62dcf5c..bf3a01d1f0 100644 --- a/crates/wasmtime/src/runtime.rs +++ b/crates/wasmtime/src/runtime.rs @@ -983,6 +983,13 @@ impl Default for Store { } } +impl fmt::Debug for Store { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let inner = &*self.inner as *const StoreInner; + f.debug_struct("Store").field("inner", &inner).finish() + } +} + impl Drop for StoreInner { fn drop(&mut self) { for instance in self.instances.get_mut().iter() { From 58b08b9d3a0ba1d50953d6c6cc14ebd99affc821 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 1 Jun 2020 14:48:45 -0700 Subject: [PATCH 14/14] Move `HostRef` into the C API crate It isn't used by anything except for the C API and all of our embedder-exposed APIs are already internally `Rc`-based, so it doesn't make sense to use with them. --- crates/c-api/src/extern.rs | 3 +- crates/c-api/src/func.rs | 3 +- crates/c-api/src/global.rs | 3 +- crates/c-api/src/host_ref.rs | 101 +++++++++++++++++++++++++++++++++++ crates/c-api/src/instance.rs | 3 +- crates/c-api/src/lib.rs | 1 + crates/c-api/src/linker.rs | 3 +- crates/c-api/src/memory.rs | 3 +- crates/c-api/src/module.rs | 3 +- crates/c-api/src/table.rs | 3 +- crates/c-api/src/trap.rs | 3 +- crates/c-api/src/wasi.rs | 3 +- crates/wasmtime/src/lib.rs | 2 +- crates/wasmtime/src/ref.rs | 100 +--------------------------------- 14 files changed, 124 insertions(+), 110 deletions(-) create mode 100644 crates/c-api/src/host_ref.rs diff --git a/crates/c-api/src/extern.rs b/crates/c-api/src/extern.rs index 4a71807cd9..9f50f20f90 100644 --- a/crates/c-api/src/extern.rs +++ b/crates/c-api/src/extern.rs @@ -1,6 +1,7 @@ +use crate::host_ref::HostRef; use crate::wasm_externkind_t; use crate::{wasm_externtype_t, wasm_func_t, wasm_global_t, wasm_memory_t, wasm_table_t}; -use wasmtime::{ExternType, Func, Global, HostRef, Memory, Table}; +use wasmtime::{ExternType, Func, Global, Memory, Table}; #[derive(Clone)] pub struct wasm_extern_t { diff --git a/crates/c-api/src/func.rs b/crates/c-api/src/func.rs index 855770dff3..5f62ec30b2 100644 --- a/crates/c-api/src/func.rs +++ b/crates/c-api/src/func.rs @@ -1,3 +1,4 @@ +use crate::host_ref::HostRef; 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, ExternHost}; use anyhow::anyhow; @@ -5,7 +6,7 @@ use std::ffi::c_void; use std::panic::{self, AssertUnwindSafe}; use std::ptr; use std::str; -use wasmtime::{Caller, Extern, Func, HostRef, Trap}; +use wasmtime::{Caller, Extern, Func, Trap}; #[derive(Clone)] #[repr(transparent)] diff --git a/crates/c-api/src/global.rs b/crates/c-api/src/global.rs index 29e9477fa6..3d4f768ba1 100644 --- a/crates/c-api/src/global.rs +++ b/crates/c-api/src/global.rs @@ -1,7 +1,8 @@ +use crate::host_ref::HostRef; use crate::{handle_result, wasmtime_error_t}; use crate::{wasm_extern_t, wasm_globaltype_t, wasm_store_t, wasm_val_t, ExternHost}; use std::ptr; -use wasmtime::{Global, HostRef}; +use wasmtime::Global; #[derive(Clone)] #[repr(transparent)] diff --git a/crates/c-api/src/host_ref.rs b/crates/c-api/src/host_ref.rs new file mode 100644 index 0000000000..41ac6a16f1 --- /dev/null +++ b/crates/c-api/src/host_ref.rs @@ -0,0 +1,101 @@ +use std::any::Any; +use std::cell::{self, RefCell}; +use std::convert::TryFrom; +use std::marker::PhantomData; +use wasmtime::{ExternRef, Store}; + +/// Represents a piece of data located in the host environment. +#[derive(Debug)] +pub struct HostRef +where + T: 'static + Any, +{ + externref: ExternRef, + _phantom: PhantomData, +} + +impl HostRef +where + T: 'static + Any, +{ + /// Creates a new `HostRef` from `T`. + pub fn new(store: &Store, item: T) -> HostRef { + HostRef { + externref: ExternRef::new(store, RefCell::new(item)), + _phantom: PhantomData, + } + } + + /// Immutably borrows the wrapped data. + /// + /// # Panics + /// + /// Panics if the value is currently mutably borrowed. + pub fn borrow(&self) -> cell::Ref { + self.inner().borrow() + } + + /// Mutably borrows the wrapped data. + /// + /// # Panics + /// + /// Panics if the `HostRef` is already borrowed. + pub fn borrow_mut(&self) -> cell::RefMut { + self.inner().borrow_mut() + } + + /// Returns true if the two `HostRef`'s point to the same value (not just + /// values that compare as equal). + pub fn ptr_eq(&self, other: &HostRef) -> bool { + self.externref.ptr_eq(&other.externref) + } + + fn inner(&self) -> &RefCell { + self.externref + .data() + .downcast_ref::>() + .expect("`HostRef`s always wrap an `ExternRef` of `RefCell`") + } +} + +impl AsRef for HostRef { + fn as_ref(&self) -> &ExternRef { + &self.externref + } +} + +impl From> for ExternRef +where + T: 'static + Any, +{ + fn from(host: HostRef) -> ExternRef { + host.externref + } +} + +impl TryFrom for HostRef +where + T: 'static + Any, +{ + type Error = ExternRef; + + fn try_from(externref: ExternRef) -> Result { + if externref.data().is::>() { + Ok(HostRef { + externref, + _phantom: PhantomData, + }) + } else { + Err(externref) + } + } +} + +impl Clone for HostRef { + fn clone(&self) -> HostRef { + HostRef { + externref: self.externref.clone(), + _phantom: PhantomData, + } + } +} diff --git a/crates/c-api/src/instance.rs b/crates/c-api/src/instance.rs index a833f462eb..d974a5f2dd 100644 --- a/crates/c-api/src/instance.rs +++ b/crates/c-api/src/instance.rs @@ -1,9 +1,10 @@ +use crate::host_ref::HostRef; use crate::{wasm_extern_t, wasm_extern_vec_t, wasm_module_t, wasm_trap_t}; use crate::{wasm_store_t, wasmtime_error_t, ExternHost}; use anyhow::Result; use std::cell::RefCell; use std::ptr; -use wasmtime::{Extern, HostRef, Instance, Store, Trap}; +use wasmtime::{Extern, Instance, Store, Trap}; #[repr(C)] #[derive(Clone)] diff --git a/crates/c-api/src/lib.rs b/crates/c-api/src/lib.rs index 9a62dad2be..ae5eb75575 100644 --- a/crates/c-api/src/lib.rs +++ b/crates/c-api/src/lib.rs @@ -11,6 +11,7 @@ mod error; mod r#extern; mod func; mod global; +mod host_ref; mod instance; mod linker; mod memory; diff --git a/crates/c-api/src/linker.rs b/crates/c-api/src/linker.rs index 24c565c383..0152d2d49b 100644 --- a/crates/c-api/src/linker.rs +++ b/crates/c-api/src/linker.rs @@ -1,8 +1,9 @@ +use crate::host_ref::HostRef; use crate::{bad_utf8, handle_result, wasmtime_error_t}; use crate::{wasm_extern_t, wasm_store_t, ExternHost}; use crate::{wasm_func_t, wasm_instance_t, wasm_module_t, wasm_name_t, wasm_trap_t}; use std::str; -use wasmtime::{Extern, HostRef, Linker}; +use wasmtime::{Extern, Linker}; #[repr(C)] pub struct wasmtime_linker_t { diff --git a/crates/c-api/src/memory.rs b/crates/c-api/src/memory.rs index aa3e2e4db3..ed25b24bea 100644 --- a/crates/c-api/src/memory.rs +++ b/crates/c-api/src/memory.rs @@ -1,5 +1,6 @@ +use crate::host_ref::HostRef; use crate::{wasm_extern_t, wasm_memorytype_t, wasm_store_t, ExternHost}; -use wasmtime::{HostRef, Memory}; +use wasmtime::Memory; #[derive(Clone)] #[repr(transparent)] diff --git a/crates/c-api/src/module.rs b/crates/c-api/src/module.rs index 2349e20bef..605c3e7233 100644 --- a/crates/c-api/src/module.rs +++ b/crates/c-api/src/module.rs @@ -1,8 +1,9 @@ +use crate::host_ref::HostRef; use crate::{handle_result, wasmtime_error_t}; use crate::{wasm_byte_vec_t, wasm_exporttype_vec_t, wasm_importtype_vec_t}; use crate::{wasm_exporttype_t, wasm_importtype_t, wasm_store_t}; use std::ptr; -use wasmtime::{HostRef, Module}; +use wasmtime::Module; #[repr(C)] #[derive(Clone)] diff --git a/crates/c-api/src/table.rs b/crates/c-api/src/table.rs index e49daddef8..773164a59c 100644 --- a/crates/c-api/src/table.rs +++ b/crates/c-api/src/table.rs @@ -1,7 +1,8 @@ +use crate::host_ref::HostRef; use crate::{handle_result, wasm_func_t, wasm_ref_t, wasmtime_error_t}; use crate::{wasm_extern_t, wasm_store_t, wasm_tabletype_t, ExternHost}; use std::ptr; -use wasmtime::{HostRef, Table, Val}; +use wasmtime::{Table, Val}; #[derive(Clone)] #[repr(transparent)] diff --git a/crates/c-api/src/trap.rs b/crates/c-api/src/trap.rs index c7a906e3eb..633577573d 100644 --- a/crates/c-api/src/trap.rs +++ b/crates/c-api/src/trap.rs @@ -1,6 +1,7 @@ +use crate::host_ref::HostRef; use crate::{wasm_frame_vec_t, wasm_instance_t, wasm_name_t, wasm_store_t}; use once_cell::unsync::OnceCell; -use wasmtime::{HostRef, Store, Trap}; +use wasmtime::{Store, Trap}; #[repr(C)] #[derive(Clone)] diff --git a/crates/c-api/src/wasi.rs b/crates/c-api/src/wasi.rs index f1db5d6c6b..4a7ed6c30a 100644 --- a/crates/c-api/src/wasi.rs +++ b/crates/c-api/src/wasi.rs @@ -1,4 +1,5 @@ //! The WASI embedding API definitions for Wasmtime. +use crate::host_ref::HostRef; use crate::{wasm_extern_t, wasm_importtype_t, wasm_store_t, wasm_trap_t, ExternHost}; use anyhow::Result; use std::collections::HashMap; @@ -12,7 +13,7 @@ use wasi_common::{ old::snapshot_0::WasiCtxBuilder as WasiSnapshot0CtxBuilder, preopen_dir, WasiCtxBuilder as WasiPreview1CtxBuilder, }; -use wasmtime::{HostRef, Linker, Store, Trap}; +use wasmtime::{Linker, Store, Trap}; use wasmtime_wasi::{old::snapshot_0::Wasi as WasiSnapshot0, Wasi as WasiPreview1}; unsafe fn cstr_to_path<'a>(path: *const c_char) -> Option<&'a Path> { diff --git a/crates/wasmtime/src/lib.rs b/crates/wasmtime/src/lib.rs index e08fbb15f9..b31b806b49 100644 --- a/crates/wasmtime/src/lib.rs +++ b/crates/wasmtime/src/lib.rs @@ -29,7 +29,7 @@ pub use crate::func::*; pub use crate::instance::Instance; pub use crate::linker::*; pub use crate::module::Module; -pub use crate::r#ref::{ExternRef, HostRef}; +pub use crate::r#ref::ExternRef; pub use crate::runtime::*; pub use crate::trap::Trap; pub use crate::types::*; diff --git a/crates/wasmtime/src/ref.rs b/crates/wasmtime/src/ref.rs index b8bd3693e5..3f87dbd8a2 100644 --- a/crates/wasmtime/src/ref.rs +++ b/crates/wasmtime/src/ref.rs @@ -1,10 +1,8 @@ #![allow(missing_docs)] use std::any::Any; -use std::cell::{self, RefCell}; -use std::convert::TryFrom; +use std::cell::RefCell; use std::fmt; -use std::marker::PhantomData; use std::rc::{Rc, Weak}; use wasmtime_runtime::VMExternRef; @@ -81,99 +79,3 @@ impl fmt::Debug for ExternRef { .finish() } } - -/// Represents a piece of data located in the host environment. -#[derive(Debug)] -pub struct HostRef -where - T: 'static + Any, -{ - externref: ExternRef, - _phantom: PhantomData, -} - -impl HostRef -where - T: 'static + Any, -{ - /// Creates a new `HostRef` from `T`. - pub fn new(store: &crate::Store, item: T) -> HostRef { - HostRef { - externref: ExternRef::new(store, RefCell::new(item)), - _phantom: PhantomData, - } - } - - /// Immutably borrows the wrapped data. - /// - /// # Panics - /// - /// Panics if the value is currently mutably borrowed. - pub fn borrow(&self) -> cell::Ref { - self.inner().borrow() - } - - /// Mutably borrows the wrapped data. - /// - /// # Panics - /// - /// Panics if the `HostRef` is already borrowed. - pub fn borrow_mut(&self) -> cell::RefMut { - self.inner().borrow_mut() - } - - /// Returns true if the two `HostRef`'s point to the same value (not just - /// values that compare as equal). - pub fn ptr_eq(&self, other: &HostRef) -> bool { - self.externref.ptr_eq(&other.externref) - } - - fn inner(&self) -> &RefCell { - self.externref - .inner - .downcast_ref::>() - .expect("`HostRef`s always wrap an `ExternRef` of `RefCell`") - } -} - -impl AsRef for HostRef { - fn as_ref(&self) -> &ExternRef { - &self.externref - } -} - -impl From> for ExternRef -where - T: 'static + Any, -{ - fn from(host: HostRef) -> ExternRef { - host.externref - } -} - -impl TryFrom for HostRef -where - T: 'static + Any, -{ - type Error = ExternRef; - - fn try_from(externref: ExternRef) -> Result { - if externref.inner.is::>() { - Ok(HostRef { - externref, - _phantom: PhantomData, - }) - } else { - Err(externref) - } - } -} - -impl Clone for HostRef { - fn clone(&self) -> HostRef { - HostRef { - externref: self.externref.clone(), - _phantom: PhantomData, - } - } -}