externref: Address review feedback

This commit is contained in:
Nick Fitzgerald
2020-06-12 17:22:54 -07:00
parent 618c278e41
commit 7e167cae10
20 changed files with 422 additions and 589 deletions

View File

@@ -195,16 +195,9 @@ macro_rules! getters {
let mut ret = None;
$(let $args = $args.into_abi();)*
{
let canary = 0;
let _auto_reset = instance
.store
.externref_activations_table()
.set_stack_canary(&canary);
catch_traps(export.vmctx, &instance.store, || {
ret = Some(fnptr(export.vmctx, ptr::null_mut(), $($args,)*));
})?;
}
invoke_wasm_and_catch_traps(export.vmctx, &instance.store, || {
ret = Some(fnptr(export.vmctx, ptr::null_mut(), $($args,)*));
})?;
Ok(ret.unwrap())
}
@@ -560,23 +553,14 @@ impl Func {
}
// Call the trampoline.
{
let canary = 0;
let _auto_reset = self
.instance
.store
.externref_activations_table()
.set_stack_canary(&canary);
catch_traps(self.export.vmctx, &self.instance.store, || unsafe {
(self.trampoline)(
self.export.vmctx,
ptr::null_mut(),
self.export.address,
values_vec.as_mut_ptr(),
)
})?;
}
invoke_wasm_and_catch_traps(self.export.vmctx, &self.instance.store, || unsafe {
(self.trampoline)(
self.export.vmctx,
ptr::null_mut(),
self.export.address,
values_vec.as_mut_ptr(),
)
})?;
// Load the return values out of `values_vec`.
let mut results = Vec::with_capacity(my_ty.results().len());
@@ -746,13 +730,18 @@ impl fmt::Debug for Func {
}
}
pub(crate) fn catch_traps(
pub(crate) fn invoke_wasm_and_catch_traps(
vmctx: *mut VMContext,
store: &Store,
closure: impl FnMut(),
) -> Result<(), Trap> {
let signalhandler = store.signal_handler();
unsafe {
let canary = 0;
let _auto_reset_canary = store
.externref_activations_table()
.set_stack_canary(&canary);
wasmtime_runtime::catch_traps(
vmctx,
store.engine().config().max_wasm_stack,

View File

@@ -3,13 +3,9 @@ use crate::{Engine, Export, Extern, Func, Global, Memory, Module, Store, Table,
use anyhow::{bail, Error, Result};
use std::any::Any;
use std::mem;
use std::rc::Rc;
use std::sync::Arc;
use wasmtime_environ::EntityIndex;
use wasmtime_jit::{CompiledModule, Resolver};
use wasmtime_runtime::{
InstantiationError, StackMapRegistry, VMContext, VMExternRefActivationsTable, VMFunctionBody,
};
use wasmtime_runtime::{InstantiationError, VMContext, VMFunctionBody};
struct SimpleResolver<'a> {
imports: &'a [Extern],
@@ -28,8 +24,6 @@ fn instantiate(
compiled_module: &CompiledModule,
imports: &[Extern],
host: Box<dyn Any>,
externref_activations_table: Rc<VMExternRefActivationsTable>,
stack_map_registry: Arc<StackMapRegistry>,
) -> Result<StoreInstanceHandle, Error> {
// For now we have a restriction that the `Store` that we're working
// with is the same for everything involved here.
@@ -56,8 +50,8 @@ fn instantiate(
config.memory_creator.as_ref().map(|a| a as _),
store.interrupts().clone(),
host,
externref_activations_table,
stack_map_registry,
&*store.externref_activations_table() as *const _ as *mut _,
&*store.stack_map_registry() as *const _ as *mut _,
)?;
// After we've created the `InstanceHandle` we still need to run
@@ -97,7 +91,7 @@ fn instantiate(
};
let vmctx_ptr = instance.handle.vmctx_ptr();
unsafe {
super::func::catch_traps(vmctx_ptr, store, || {
super::func::invoke_wasm_and_catch_traps(vmctx_ptr, store, || {
mem::transmute::<
*const VMFunctionBody,
unsafe extern "C" fn(*mut VMContext, *mut VMContext),
@@ -194,24 +188,11 @@ impl Instance {
let host_info = Box::new({
let frame_info_registration = module.register_frame_info();
store.register_jit_code(module.compiled_module().jit_code_ranges());
// We need to make sure that we keep this alive as long as the instance
// is alive, or else we could miss GC roots, reclaim objects too early,
// and get user-after-frees.
let stack_map_registration =
unsafe { module.register_stack_maps(&*store.stack_map_registry()) };
(frame_info_registration, stack_map_registration)
store.register_stack_maps(&module);
frame_info_registration
});
let handle = instantiate(
store,
module.compiled_module(),
imports,
host_info,
store.externref_activations_table().clone(),
store.stack_map_registry().clone(),
)?;
let handle = instantiate(store, module.compiled_module(), imports, host_info)?;
Ok(Instance {
handle,

View File

@@ -6,7 +6,6 @@ use std::path::Path;
use std::sync::{Arc, Mutex};
use wasmparser::validate;
use wasmtime_jit::CompiledModule;
use wasmtime_runtime::{StackMapRegistration, StackMapRegistry};
/// A compiled WebAssembly module, ready to be instantiated.
///
@@ -81,7 +80,6 @@ pub struct Module {
engine: Engine,
compiled: Arc<CompiledModule>,
frame_info_registration: Arc<Mutex<Option<Option<Arc<GlobalFrameInfoRegistration>>>>>,
stack_map_registration: Arc<Mutex<Option<Option<Arc<StackMapRegistration>>>>>,
}
impl Module {
@@ -309,7 +307,6 @@ impl Module {
engine: engine.clone(),
compiled: Arc::new(compiled),
frame_info_registration: Arc::new(Mutex::new(None)),
stack_map_registration: Arc::new(Mutex::new(None)),
})
}
@@ -537,41 +534,6 @@ impl Module {
*info = Some(ret.clone());
return ret;
}
/// Register this module's stack maps.
///
/// # Safety
///
/// The same as `wasmtime_runtime::StackMapRegistry::register_stack_maps`.
pub(crate) unsafe fn register_stack_maps(
&self,
registry: &Arc<StackMapRegistry>,
) -> Option<Arc<StackMapRegistration>> {
let mut registration = self.stack_map_registration.lock().unwrap();
if let Some(registration) = &*registration {
return registration.clone();
}
let module = &self.compiled;
let ret = registry
.register_stack_maps(
module
.finished_functions()
.values()
.zip(module.stack_maps().values())
.map(|(func, stack_maps)| {
let ptr = (**func).as_ptr();
let len = (**func).len();
let start = ptr as usize;
let end = ptr as usize + len;
let range = start..end;
(range, &stack_maps[..])
}),
)
.map(Arc::new);
*registration = Some(ret.clone());
ret
}
}
fn _assert_send_sync() {

6
crates/wasmtime/src/ref.rs Executable file → Normal file
View File

@@ -36,9 +36,9 @@ impl ExternRef {
&*self.inner
}
/// Get the reference count for this `ExternRef`.
pub fn get_reference_count(&self) -> usize {
self.inner.get_reference_count()
/// Get the strong reference count for this `ExternRef`.
pub fn strong_count(&self) -> usize {
self.inner.strong_count()
}
/// Does this `ExternRef` point to the same inner value as `other`?0

View File

@@ -1,6 +1,7 @@
use crate::externals::MemoryCreator;
use crate::r#ref::ExternRef;
use crate::trampoline::{MemoryCreatorProxy, StoreInstanceHandle};
use crate::Module;
use anyhow::{bail, Result};
use std::any::Any;
use std::cell::RefCell;
@@ -14,7 +15,7 @@ use std::rc::{Rc, Weak};
use std::sync::Arc;
use wasmparser::{OperatorValidatorConfig, ValidatingParserConfig};
use wasmtime_environ::settings::{self, Configurable};
use wasmtime_environ::{ir, wasm, CacheConfig, Tunables};
use wasmtime_environ::{ir, isa::TargetIsa, wasm, CacheConfig, Tunables};
use wasmtime_jit::{native, CompilationStrategy, Compiler};
use wasmtime_profiling::{JitDumpAgent, NullProfilerAgent, ProfilingAgent, VTuneAgent};
use wasmtime_runtime::{
@@ -195,6 +196,7 @@ impl Config {
self.validating_config
.operator_config
.enable_reference_types = enable;
self.flags
.set("enable_safepoints", if enable { "true" } else { "false" })
.unwrap();
@@ -597,8 +599,12 @@ impl Config {
self
}
pub(crate) fn target_isa(&self) -> Box<dyn TargetIsa> {
native::builder().finish(settings::Flags::new(self.flags.clone()))
}
fn build_compiler(&self) -> Compiler {
let isa = native::builder().finish(settings::Flags::new(self.flags.clone()));
let isa = self.target_isa();
Compiler::new(
isa,
self.strategy,
@@ -730,7 +736,6 @@ pub struct Engine {
struct EngineInner {
config: Config,
compiler: Compiler,
stack_map_registry: Arc<StackMapRegistry>,
}
impl Engine {
@@ -742,7 +747,6 @@ impl Engine {
inner: Arc::new(EngineInner {
config: config.clone(),
compiler: config.build_compiler(),
stack_map_registry: Arc::new(StackMapRegistry::default()),
}),
}
}
@@ -801,7 +805,7 @@ pub(crate) struct StoreInner {
jit_code_ranges: RefCell<Vec<(usize, usize)>>,
host_info: RefCell<HashMap<HostInfoKey, Rc<RefCell<dyn Any>>>>,
externref_activations_table: Rc<VMExternRefActivationsTable>,
stack_map_registry: Arc<StackMapRegistry>,
stack_map_registry: Rc<StackMapRegistry>,
}
struct HostInfoKey(VMExternRef);
@@ -843,7 +847,7 @@ impl Store {
jit_code_ranges: RefCell::new(Vec::new()),
host_info: RefCell::new(HashMap::new()),
externref_activations_table: Rc::new(VMExternRefActivationsTable::new()),
stack_map_registry: engine.inner.stack_map_registry.clone(),
stack_map_registry: Rc::new(StackMapRegistry::default()),
}),
}
}
@@ -916,6 +920,24 @@ impl Store {
}
}
pub(crate) fn register_stack_maps(&self, module: &Module) {
let module = &module.compiled_module();
self.stack_map_registry().register_stack_maps(
module
.finished_functions()
.values()
.zip(module.stack_maps().values())
.map(|(func, stack_maps)| unsafe {
let ptr = (**func).as_ptr();
let len = (**func).len();
let start = ptr as usize;
let end = ptr as usize + len;
let range = start..end;
(range, &stack_maps[..])
}),
);
}
pub(crate) unsafe fn add_instance(&self, handle: InstanceHandle) -> StoreInstanceHandle {
self.inner.instances.borrow_mut().push(handle.clone());
StoreInstanceHandle {
@@ -1091,14 +1113,15 @@ impl Store {
&self.inner.externref_activations_table
}
pub(crate) fn stack_map_registry(&self) -> &Arc<StackMapRegistry> {
&self.inner.engine.inner.stack_map_registry
pub(crate) fn stack_map_registry(&self) -> &Rc<StackMapRegistry> {
&self.inner.stack_map_registry
}
/// Perform garbage collection of `ExternRef`s.
pub fn gc(&self) {
// For this crate's API, we ensure that `set_stack_canary` invariants
// are upheld for all host-->Wasm calls.
// are upheld for all host-->Wasm calls, and we register every module
// used with this store in `self.inner.stack_map_registry`.
unsafe {
wasmtime_runtime::gc(
&*self.inner.stack_map_registry,

View File

@@ -46,8 +46,8 @@ pub(crate) fn create_handle(
signatures.into_boxed_slice(),
state,
store.interrupts().clone(),
store.externref_activations_table().clone(),
store.stack_map_registry().clone(),
&*store.externref_activations_table() as *const _ as *mut _,
&*store.stack_map_registry() as *const _ as *mut _,
)?;
Ok(store.add_instance(handle))
}

View File

@@ -2,7 +2,7 @@
use super::create_handle::create_handle;
use crate::trampoline::StoreInstanceHandle;
use crate::{FuncType, Store, Trap, ValType};
use crate::{FuncType, Store, Trap};
use anyhow::{bail, Result};
use std::any::Any;
use std::cmp;
@@ -11,9 +11,7 @@ use std::mem;
use std::panic::{self, AssertUnwindSafe};
use wasmtime_environ::entity::PrimaryMap;
use wasmtime_environ::isa::TargetIsa;
use wasmtime_environ::{
ir, settings, settings::Configurable, CompiledFunction, EntityIndex, Module,
};
use wasmtime_environ::{ir, settings, CompiledFunction, EntityIndex, Module};
use wasmtime_jit::trampoline::ir::{
ExternalName, Function, InstBuilder, MemFlags, StackSlotData, StackSlotKind,
};
@@ -210,18 +208,7 @@ pub fn create_handle_with_function(
func: Box<dyn Fn(*mut VMContext, *mut u128) -> Result<(), Trap>>,
store: &Store,
) -> Result<(StoreInstanceHandle, VMTrampoline)> {
let isa = {
let isa_builder = native::builder();
let mut flag_builder = settings::builder();
if ft.params().iter().any(|p| *p == ValType::ExternRef)
|| ft.results().iter().any(|r| *r == ValType::ExternRef)
{
flag_builder.set("enable_safepoints", "true").unwrap();
}
isa_builder.finish(settings::Flags::new(flag_builder))
};
let isa = store.engine().config().target_isa();
let pointer_type = isa.pointer_type();
let sig = match ft.get_wasmtime_signature(pointer_type) {

View File

@@ -106,7 +106,10 @@ impl ValType {
ValType::F32 => Some(ir::types::F32),
ValType::F64 => Some(ir::types::F64),
ValType::V128 => Some(ir::types::I8X16),
#[cfg(target_pointer_width = "64")]
ValType::ExternRef => Some(ir::types::R64),
#[cfg(target_pointer_width = "32")]
ValType::ExternRef => Some(ir::types::R32),
_ => None,
}
}
@@ -118,7 +121,10 @@ impl ValType {
ir::types::F32 => Some(ValType::F32),
ir::types::F64 => Some(ValType::F64),
ir::types::I8X16 => Some(ValType::V128),
#[cfg(target_pointer_width = "64")]
ir::types::R64 => Some(ValType::ExternRef),
#[cfg(target_pointer_width = "32")]
ir::types::R32 => Some(ValType::ExternRef),
_ => None,
}
}

View File

@@ -89,10 +89,9 @@ impl Val {
Val::ExternRef(None) => ptr::write(p, 0),
Val::ExternRef(Some(x)) => {
let externref_ptr = x.inner.as_raw();
if let Err(inner) = store.externref_activations_table().try_insert(x.inner) {
store.gc();
store.externref_activations_table().insert_slow_path(inner);
}
store
.externref_activations_table()
.insert_with_gc(x.inner, store.stack_map_registry());
ptr::write(p as *mut *mut u8, externref_ptr)
}
_ => unimplemented!("Val::write_value_to"),