diff --git a/crates/api/examples/gcd.rs b/crates/api/examples/gcd.rs index 033e90e98c..93313cb708 100644 --- a/crates/api/examples/gcd.rs +++ b/crates/api/examples/gcd.rs @@ -38,11 +38,10 @@ fn main() -> anyhow::Result<()> { // `Module` which is attached to a `Store` cache. let wasm = wat::parse_str(WAT)?; let store = Store::default(); - let module = HostRef::new(Module::new(&store, &wasm)?); + let module = Module::new(&store, &wasm)?; // Find index of the `gcd` export. let gcd_index = module - .borrow() .exports() .iter() .enumerate() diff --git a/crates/api/examples/hello.rs b/crates/api/examples/hello.rs index 6d2044e223..5caaa75af6 100644 --- a/crates/api/examples/hello.rs +++ b/crates/api/examples/hello.rs @@ -35,7 +35,7 @@ fn main() -> Result<()> { // Compiler the `*.wasm` binary into an in-memory instance of a `Module`. println!("Compiling module..."); - let module = HostRef::new(Module::new(&store, &binary).context("> Error compiling module!")?); + let module = Module::new(&store, &binary).context("> Error compiling module!")?; // Here we handle the imports of the module, which in this case is our // `HelloCallback` type and its associated implementation of `Callback. diff --git a/crates/api/examples/memory.rs b/crates/api/examples/memory.rs index 05b93711e8..bd7d0be21a 100644 --- a/crates/api/examples/memory.rs +++ b/crates/api/examples/memory.rs @@ -86,7 +86,7 @@ fn main() -> Result<(), Error> { // Compile. println!("Compiling module..."); - let module = HostRef::new(Module::new(&store, &binary).context("> Error compiling module!")?); + let module = Module::new(&store, &binary).context("> Error compiling module!")?; // Instantiate. println!("Instantiating module..."); diff --git a/crates/api/examples/multi.rs b/crates/api/examples/multi.rs index 13b06d1bd3..8898accf11 100644 --- a/crates/api/examples/multi.rs +++ b/crates/api/examples/multi.rs @@ -54,7 +54,7 @@ fn main() -> Result<()> { // Compile. println!("Compiling module..."); - let module = HostRef::new(Module::new(&store, &binary).context("Error compiling module!")?); + let module = Module::new(&store, &binary).context("Error compiling module!")?; // Create external print functions. println!("Creating callback..."); diff --git a/crates/api/src/callable.rs b/crates/api/src/callable.rs index cda332c3a8..c035252ae0 100644 --- a/crates/api/src/callable.rs +++ b/crates/api/src/callable.rs @@ -43,7 +43,7 @@ use wasmtime_runtime::Export; /// /// // Initialise environment and our module. /// let store = wasmtime::Store::default(); -/// let module = HostRef::new(wasmtime::Module::new(&store, &binary)?); +/// let module = wasmtime::Module::new(&store, &binary)?; /// /// // Define the type of the function we're going to call. /// let times_two_type = wasmtime::FuncType::new( diff --git a/crates/api/src/instance.rs b/crates/api/src/instance.rs index dde324a54a..0398ec3dd7 100644 --- a/crates/api/src/instance.rs +++ b/crates/api/src/instance.rs @@ -1,7 +1,6 @@ use crate::context::Context; use crate::externals::Extern; use crate::module::Module; -use crate::r#ref::HostRef; use crate::runtime::Store; use crate::trampoline::take_api_trap; use crate::trap::Trap; @@ -60,7 +59,7 @@ pub fn instantiate_in_context( pub struct Instance { instance_handle: InstanceHandle, - module: HostRef, + module: Module, // We need to keep CodeMemory alive. contexts: HashSet, @@ -69,29 +68,19 @@ pub struct Instance { } impl Instance { - pub fn new( - store: &Store, - module: &HostRef, - externs: &[Extern], - ) -> Result { + pub fn new(store: &Store, module: &Module, externs: &[Extern]) -> Result { let context = store.context().clone(); let exports = store.global_exports().clone(); let imports = module - .borrow() .imports() .iter() .zip(externs.iter()) .map(|(i, e)| (i.module().to_string(), i.name().to_string(), e.clone())) .collect::>(); - let (mut instance_handle, contexts) = instantiate_in_context( - module.borrow().binary().expect("binary"), - imports, - context, - exports, - )?; + let (mut instance_handle, contexts) = + instantiate_in_context(module.binary().expect("binary"), imports, context, exports)?; let exports = { - let module = module.borrow(); let mut exports = Vec::with_capacity(module.exports().len()); for export in module.exports() { let name = export.name().to_string(); @@ -116,14 +105,13 @@ impl Instance { &self.exports } - pub fn module(&self) -> &HostRef { + pub fn module(&self) -> &Module { &self.module } pub fn find_export_by_name(&self, name: &str) -> Option<&Extern> { let (i, _) = self .module - .borrow() .exports() .iter() .enumerate() @@ -154,10 +142,7 @@ impl Instance { )); } - let module = HostRef::new(Module::from_exports( - store, - exports_types.into_boxed_slice(), - )); + let module = Module::from_exports(store, exports_types.into_boxed_slice()); Instance { instance_handle, diff --git a/crates/api/src/module.rs b/crates/api/src/module.rs index 52a84a0824..7815dd169a 100644 --- a/crates/api/src/module.rs +++ b/crates/api/src/module.rs @@ -4,6 +4,7 @@ use crate::types::{ TableType, ValType, }; use anyhow::{Error, Result}; +use std::rc::Rc; use wasmparser::{ validate, ExternalKind, ImportSectionEntryType, ModuleReader, OperatorValidatorConfig, SectionCode, ValidatingParserConfig, @@ -170,8 +171,27 @@ pub(crate) enum ModuleCodeSource { Unknown, } +/// A compiled WebAssembly module, ready to be instantiated. +/// +/// A `Module` is a compiled in-memory representation of an input WebAssembly +/// binary. A `Module` is then used to create an [`Instance`](crate::Instance) +/// through an instantiation process. You cannot call functions or fetch +/// globals, for example, on a `Module` because it's purely a code +/// representation. Instead you'll need to create an +/// [`Instance`](crate::Instance) to interact with the wasm module. +/// +/// ## Modules and `Clone` +/// +/// Using `clone` on a `Module` is a cheap operation. It will not create an +/// entirely new module, but rather just a new reference to the existing module. +/// In other words it's a shallow copy, not a deep copy. #[derive(Clone)] pub struct Module { + // FIXME(#777) should be `Arc` and this type should be thread-safe + inner: Rc, +} + +struct ModuleInner { store: Store, source: ModuleCodeSource, imports: Box<[ImportType]>, @@ -179,29 +199,106 @@ pub struct Module { } impl Module { - /// Validate and decode the raw wasm data in `binary` and create a new - /// `Module` in the given `store`. + /// Creates a new WebAssembly `Module` from the given in-memory `binary` + /// data. + /// + /// The `binary` data provided must be a [binary-encoded][binary] + /// WebAssembly module. This means that the data for the wasm module must be + /// loaded in-memory if it's present elsewhere, for example on disk. + /// Additionally this requires that the entire binary is loaded into memory + /// all at once, this API does not support streaming compilation of a + /// module. + /// + /// The WebAssembly binary will be decoded and validated. It will also be + /// compiled according to the configuration of the provided `store` and + /// cached in this type. + /// + /// The provided `store` is a global cache for compiled resources as well as + /// configuration for what wasm features are enabled. It's recommended to + /// share a `store` among modules if possible. + /// + /// # Errors + /// + /// This function may fail and return an error. Errors may include + /// situations such as: + /// + /// * The binary provided could not be decoded because it's not a valid + /// WebAssembly binary + /// * The WebAssembly binary may not validate (e.g. contains type errors) + /// * Implementation-specific limits were exceeded with a valid binary (for + /// example too many locals) + /// * The wasm binary may use features that are not enabled in the + /// configuration of `store` + /// + /// The error returned should contain full information about why module + /// creation failed if one is returned. + /// + /// [binary]: https://webassembly.github.io/spec/core/binary/index.html pub fn new(store: &Store, binary: &[u8]) -> Result { Self::validate(store, binary)?; - Self::new_unchecked(store, binary) + // Note that the call to `unsafe` here should be ok because we + // previously validated the binary, meaning we're guaranteed to pass a + // valid binary for `store`. + unsafe { Self::new_unchecked(store, binary) } } - /// Similar to `new`, but does not perform any validation. Only use this - /// on modules which are known to have been validated already! - pub fn new_unchecked(store: &Store, binary: &[u8]) -> Result { + + /// Creates a new WebAssembly `Module` from the given in-memory `binary` + /// data, skipping validation and asserting that `binary` is a valid + /// WebAssembly module. + /// + /// This function is the same as [`Module::new`] except that it skips the + /// call to [`Module::validate`]. This means that the WebAssembly binary is + /// not validated for correctness and it is simply assumed as valid. + /// + /// For more information about creation of a module and the `store` argument + /// see the documentation of [`Module::new`]. + /// + /// # Unsafety + /// + /// This function is `unsafe` due to the unchecked assumption that the input + /// `binary` is valid. If the `binary` is not actually a valid wasm binary it + /// may cause invalid machine code to get generated, cause panics, etc. + /// + /// It is only safe to call this method if [`Module::validate`] succeeds on + /// the same arguments passed to this function. + /// + /// # Errors + /// + /// This function may fail for many of the same reasons as [`Module::new`]. + /// While this assumes that the binary is valid it still needs to actually + /// be somewhat valid for decoding purposes, and the basics of decoding can + /// still fail. + pub unsafe fn new_unchecked(store: &Store, binary: &[u8]) -> Result { let (imports, exports) = read_imports_and_exports(binary)?; Ok(Module { - store: store.clone(), - source: ModuleCodeSource::Binary(binary.into()), - imports, - exports, + inner: Rc::new(ModuleInner { + store: store.clone(), + source: ModuleCodeSource::Binary(binary.into()), + imports, + exports, + }), }) } - pub(crate) fn binary(&self) -> Option<&[u8]> { - match &self.source { - ModuleCodeSource::Binary(b) => Some(b), - _ => None, - } - } + + /// Validates `binary` input data as a WebAssembly binary given the + /// configuration in `store`. + /// + /// This function will perform a speedy validation of the `binary` input + /// WebAssembly module (which is in [binary form][binary]) and return either + /// `Ok` or `Err` depending on the results of validation. The `store` + /// argument indicates configuration for WebAssembly features, for example, + /// which are used to indicate what should be valid and what shouldn't be. + /// + /// Validation automatically happens as part of [`Module::new`], but is a + /// requirement for [`Module::new_unchecked`] to be safe. + /// + /// # Errors + /// + /// If validation fails for any reason (type check error, usage of a feature + /// that wasn't enabled, etc) then an error with a description of the + /// validation issue will be returned. + /// + /// [binary]: https://webassembly.github.io/spec/core/binary/index.html pub fn validate(store: &Store, binary: &[u8]) -> Result<()> { let features = store.engine().config.features.clone(); let config = ValidatingParserConfig { @@ -215,18 +312,39 @@ impl Module { }; validate(binary, Some(config)).map_err(Error::new) } - pub fn imports(&self) -> &[ImportType] { - &self.imports - } - pub fn exports(&self) -> &[ExportType] { - &self.exports - } + pub fn from_exports(store: &Store, exports: Box<[ExportType]>) -> Self { Module { - store: store.clone(), - source: ModuleCodeSource::Unknown, - imports: Box::new([]), - exports, + inner: Rc::new(ModuleInner { + store: store.clone(), + source: ModuleCodeSource::Unknown, + imports: Box::new([]), + exports, + }), } } + + pub(crate) fn binary(&self) -> Option<&[u8]> { + match &self.inner.source { + ModuleCodeSource::Binary(b) => Some(b), + _ => None, + } + } + + /// Returns the list of imports that this [`Module`] has and must be + /// satisfied. + pub fn imports(&self) -> &[ImportType] { + &self.inner.imports + } + + /// Returns the list of exports that this [`Module`] has and will be + /// available after instantiation. + pub fn exports(&self) -> &[ExportType] { + &self.inner.exports + } + + /// Returns the [`Store`] that this [`Module`] was compiled into. + pub fn store(&self) -> &Store { + &self.inner.store + } } diff --git a/crates/api/src/runtime.rs b/crates/api/src/runtime.rs index 90cd20ddc2..d2a4117950 100644 --- a/crates/api/src/runtime.rs +++ b/crates/api/src/runtime.rs @@ -331,6 +331,7 @@ impl Engine { /// ocnfiguration (see [`Config`] for more information). #[derive(Clone)] pub struct Store { + // FIXME(#777) should be `Arc` and this type should be thread-safe inner: Rc, } diff --git a/crates/api/src/wasm.rs b/crates/api/src/wasm.rs index eb32ba5cfb..1776eb06ec 100644 --- a/crates/api/src/wasm.rs +++ b/crates/api/src/wasm.rs @@ -662,7 +662,7 @@ pub unsafe extern "C" fn wasm_instance_new( let import = *imports.add(i); externs.push((*import).ext.clone()); } - let module = &(*module).module; + let module = &(*module).module.borrow(); match Instance::new(store, module, &externs) { Ok(instance) => { let instance = Box::new(wasm_instance_t { diff --git a/crates/api/tests/import_calling_export.rs b/crates/api/tests/import_calling_export.rs index 5538e19792..e8d94e6e25 100644 --- a/crates/api/tests/import_calling_export.rs +++ b/crates/api/tests/import_calling_export.rs @@ -34,7 +34,7 @@ fn test_import_calling_export() { let store = Store::default(); let wasm = wat::parse_str(WAT).unwrap(); - let module = HostRef::new(Module::new(&store, &wasm).expect("failed to create module")); + let module = Module::new(&store, &wasm).expect("failed to create module"); let callback = Rc::new(Callback { other: RefCell::new(None), diff --git a/crates/api/tests/import_calling_export.wat b/crates/api/tests/import_calling_export.wat deleted file mode 100644 index dc66e4f949..0000000000 --- a/crates/api/tests/import_calling_export.wat +++ /dev/null @@ -1,8 +0,0 @@ -(module - (type $t0 (func)) - (import "" "imp" (func $.imp (type $t0))) - (func $run call $.imp) - (func $other) - (export "run" (func $run)) - (export "other" (func $other)) -) \ No newline at end of file diff --git a/crates/api/tests/traps.rs b/crates/api/tests/traps.rs index 9f32d077de..a5b8cc9caf 100644 --- a/crates/api/tests/traps.rs +++ b/crates/api/tests/traps.rs @@ -15,17 +15,16 @@ fn test_trap_return() -> Result<(), String> { let store = Store::default(); let binary = parse_str( r#" - (module - (func $hello (import "" "hello")) - (func (export "run") (call $hello)) - ) - "#, + (module + (func $hello (import "" "hello")) + (func (export "run") (call $hello)) + ) + "#, ) .map_err(|e| format!("failed to parse WebAssembly text source: {}", e))?; - let module = HostRef::new( - Module::new(&store, &binary).map_err(|e| format!("failed to compile module: {}", e))?, - ); + let module = + Module::new(&store, &binary).map_err(|e| format!("failed to compile module: {}", e))?; let hello_type = FuncType::new(Box::new([]), Box::new([])); let hello_func = HostRef::new(Func::new(&store, hello_type, Rc::new(HelloCallback))); diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index d04ca8cb5c..5ebd621f40 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/crates/fuzzing/src/oracles.rs @@ -44,19 +44,15 @@ pub fn instantiate(wasm: &[u8], strategy: Strategy) { let engine = Engine::new(&config); let store = Store::new(&engine); - let module = - HostRef::new(Module::new(&store, wasm).expect("Failed to compile a valid Wasm module!")); + let module = Module::new(&store, wasm).expect("Failed to compile a valid Wasm module!"); - let imports = { - let module = module.borrow(); - match dummy_imports(&store, module.imports()) { - Ok(imps) => imps, - Err(_) => { - // There are some value types that we can't synthesize a - // dummy value for (e.g. anyrefs) and for modules that - // import things of these types we skip instantiation. - return; - } + let imports = match dummy_imports(&store, module.imports()) { + Ok(imps) => imps, + Err(_) => { + // There are some value types that we can't synthesize a + // dummy value for (e.g. anyrefs) and for modules that + // import things of these types we skip instantiation. + return; } }; @@ -92,7 +88,7 @@ pub fn make_api_calls(api: crate::generators::api::ApiCalls) { let mut config: Option = None; let mut engine: Option = None; let mut store: Option = None; - let mut modules: HashMap> = Default::default(); + let mut modules: HashMap = Default::default(); let mut instances: HashMap> = Default::default(); for call in api.calls { @@ -117,10 +113,10 @@ pub fn make_api_calls(api: crate::generators::api::ApiCalls) { } ApiCall::ModuleNew { id, wasm } => { - let module = HostRef::new(match Module::new(store.as_ref().unwrap(), &wasm.wasm) { + let module = match Module::new(store.as_ref().unwrap(), &wasm.wasm) { Ok(m) => m, Err(_) => continue, - }); + }; let old = modules.insert(id, module); assert!(old.is_none()); } @@ -135,16 +131,13 @@ pub fn make_api_calls(api: crate::generators::api::ApiCalls) { None => continue, }; - let imports = { - let module = module.borrow(); - match dummy_imports(store.as_ref().unwrap(), module.imports()) { - Ok(imps) => imps, - Err(_) => { - // There are some value types that we can't synthesize a - // dummy value for (e.g. anyrefs) and for modules that - // import things of these types we skip instantiation. - continue; - } + let imports = match dummy_imports(store.as_ref().unwrap(), module.imports()) { + Ok(imps) => imps, + Err(_) => { + // There are some value types that we can't synthesize a + // dummy value for (e.g. anyrefs) and for modules that + // import things of these types we skip instantiation. + continue; } }; diff --git a/crates/misc/py/src/instance.rs b/crates/misc/py/src/instance.rs index 5975371cd4..f5c0480885 100644 --- a/crates/misc/py/src/instance.rs +++ b/crates/misc/py/src/instance.rs @@ -21,7 +21,7 @@ impl Instance { let py = gil.python(); let exports = PyDict::new(py); let module = self.instance.borrow().module().clone(); - for (i, e) in module.borrow().exports().iter().enumerate() { + for (i, e) in module.exports().iter().enumerate() { match e.ty() { wasmtime::ExternType::Func(ft) => { let mut args_types = Vec::new(); diff --git a/crates/misc/py/src/lib.rs b/crates/misc/py/src/lib.rs index 20db96ba9d..7247b1e867 100644 --- a/crates/misc/py/src/lib.rs +++ b/crates/misc/py/src/lib.rs @@ -84,7 +84,7 @@ pub fn instantiate( let engine = wasmtime::Engine::new(&wasmtime::Config::new().wasm_multi_value(true)); let store = wasmtime::Store::new(&engine); - let module = wasmtime::HostRef::new(wasmtime::Module::new(&store, wasm_data).map_err(err2py)?); + let module = wasmtime::Module::new(&store, wasm_data).map_err(err2py)?; let data = Rc::new(ModuleData::new(wasm_data).map_err(err2py)?); @@ -99,7 +99,7 @@ pub fn instantiate( }; let mut imports: Vec = Vec::new(); - for i in module.borrow().imports() { + for i in module.imports() { let module_name = i.module(); if let Some(m) = import_obj.get_item(module_name) { let e = find_export_in(m, &store, i.name())?; diff --git a/crates/misc/py/src/module.rs b/crates/misc/py/src/module.rs index c916b11bfc..4c7e4a30c2 100644 --- a/crates/misc/py/src/module.rs +++ b/crates/misc/py/src/module.rs @@ -4,5 +4,5 @@ use pyo3::prelude::*; #[pyclass] pub struct Module { - pub module: wasmtime::HostRef, + pub module: wasmtime::Module, } diff --git a/crates/misc/rust/macro/src/lib.rs b/crates/misc/rust/macro/src/lib.rs index 378897d311..b70cccfa61 100644 --- a/crates/misc/rust/macro/src/lib.rs +++ b/crates/misc/rust/macro/src/lib.rs @@ -57,13 +57,13 @@ fn generate_load(item: &syn::ItemTrait) -> syn::Result { let data = #root::wasmtime_interface_types::ModuleData::new(&bytes)?; - let module = HostRef::new(Module::new(&store, &bytes)?); + let module = Module::new(&store, &bytes)?; let mut imports: Vec = Vec::new(); if let Some(module_name) = data.find_wasi_module_name() { let wasi_instance = #root::wasmtime_wasi::create_wasi_instance(&store, &[], &[], &[]) .map_err(|e| format_err!("wasm instantiation error: {:?}", e))?; - for i in module.borrow().imports().iter() { + for i in module.imports().iter() { if i.module() != module_name { bail!("unknown import module {}", i.module()); } diff --git a/crates/test-programs/tests/wasm_tests/runtime.rs b/crates/test-programs/tests/wasm_tests/runtime.rs index 5c4d10220b..0970d31f26 100644 --- a/crates/test-programs/tests/wasm_tests/runtime.rs +++ b/crates/test-programs/tests/wasm_tests/runtime.rs @@ -43,9 +43,8 @@ pub fn instantiate(data: &[u8], bin_name: &str, workspace: Option<&Path>) -> any .context("failed to instantiate wasi")?, ); - let module = HostRef::new(Module::new(&store, &data).context("failed to create wasm module")?); + let module = Module::new(&store, &data).context("failed to create wasm module")?; let imports = module - .borrow() .imports() .iter() .map(|i| { diff --git a/crates/wast/src/wast.rs b/crates/wast/src/wast.rs index 73d47c20ea..4f8f2ba0c6 100644 --- a/crates/wast/src/wast.rs +++ b/crates/wast/src/wast.rs @@ -65,9 +65,9 @@ impl WastContext { } fn instantiate(&self, module: &[u8]) -> Result>> { - let module = HostRef::new(Module::new(&self.store, module)?); + let module = Module::new(&self.store, module)?; let mut imports = Vec::new(); - for import in module.borrow().imports() { + for import in module.imports() { if import.module() == "spectest" { let spectest = self .spectest diff --git a/src/commands/run.rs b/src/commands/run.rs index 2ddf47fe00..3a71dc1900 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -234,15 +234,14 @@ impl RunCommand { store: &Store, module_registry: &HashMap>, path: &Path, - ) -> Result<(HostRef, HostRef, Vec)> { + ) -> Result<(HostRef, Module, Vec)> { // Read the wasm module binary either as `*.wat` or a raw binary let data = wat::parse_file(path)?; - let module = HostRef::new(Module::new(store, &data)?); + let module = Module::new(store, &data)?; // Resolve import using module_registry. let imports = module - .borrow() .imports() .iter() .map(|i| { @@ -285,7 +284,6 @@ impl RunCommand { let data = ModuleData::new(&data)?; self.invoke_export(instance, &data, name)?; } else if module - .borrow() .exports() .iter() .any(|export| export.name().is_empty())