From c241f18b8152e60a70717575aa58ce638b365a8f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 24 Mar 2020 17:37:32 -0500 Subject: [PATCH] Use `Linker` in `*.wast` testing (#1391) * Use `Linker` in `*.wast` testing By default `Linker` disallows shadowing previously defined items, but it looks like the `*.wast` test suites rely on this so this commit adds a boolean flag to `Linker` as well indicating whether duplicates are allowed. * Review comments * Add a test with a number of recursive instances * Deny warnings in doctests * No tabs --- crates/api/src/externals.rs | 9 +++-- crates/api/src/lib.rs | 2 ++ crates/api/src/linker.rs | 69 ++++++++++++++++++++++++++++++------- crates/api/tests/linker.rs | 31 +++++++++++++++++ crates/wast/src/lib.rs | 2 +- crates/wast/src/spectest.rs | 63 +++++++++++++-------------------- crates/wast/src/wast.rs | 43 +++++++---------------- 7 files changed, 133 insertions(+), 86 deletions(-) diff --git a/crates/api/src/externals.rs b/crates/api/src/externals.rs index beb7ac5a2d..dd4eed0424 100644 --- a/crates/api/src/externals.rs +++ b/crates/api/src/externals.rs @@ -557,23 +557,24 @@ impl Table { /// **unsafe** usages of `Memory`. Do not do these things! /// /// ```rust +/// # use anyhow::Result; /// use wasmtime::Memory; /// /// // NOTE: All code in this function is not safe to execute and may cause /// // segfaults/undefined behavior at runtime. Do not copy/paste these examples /// // into production code! -/// unsafe fn unsafe_examples(mem: &Memory) { +/// unsafe fn unsafe_examples(mem: &Memory) -> Result<()> { /// // First and foremost, any borrow can be invalidated at any time via the /// // `Memory::grow` function. This can relocate memory which causes any /// // previous pointer to be possibly invalid now. /// let pointer: &u8 = &mem.data_unchecked()[0x100]; -/// mem.grow(1); // invalidates `pointer`! +/// mem.grow(1)?; // invalidates `pointer`! /// // println!("{}", *pointer); // FATAL: use-after-free /// /// // Note that the use-after-free also applies to slices, whether they're /// // slices of bytes or strings. /// let slice: &[u8] = &mem.data_unchecked()[0x100..0x102]; -/// mem.grow(1); // invalidates `slice`! +/// mem.grow(1)?; // invalidates `slice`! /// // println!("{:?}", slice); // FATAL: use-after-free /// /// // Due to the reference-counted nature of `Memory` note that literal @@ -597,6 +598,8 @@ impl Table { /// let slice1: &mut [u8] = &mut mem.data_unchecked_mut()[0x100..][..3]; /// let slice2: &mut [u8] = &mut mem.data_unchecked_mut()[0x102..][..4]; /// // println!("{:?} {:?}", slice1, slice2); // FATAL: aliasing mutable pointers +/// +/// Ok(()) /// } /// # fn some_other_function() {} /// ``` diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index cf3a53759e..b4ff2207f6 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -7,6 +7,8 @@ //! itself for consumption from other languages. #![deny(missing_docs, intra_doc_link_resolution_failure)] +#![doc(test(attr(deny(warnings))))] +#![doc(test(attr(allow(dead_code, unused_variables, unused_mut))))] mod externals; mod frame_info; diff --git a/crates/api/src/linker.rs b/crates/api/src/linker.rs index b991e366b0..a5f28c646a 100644 --- a/crates/api/src/linker.rs +++ b/crates/api/src/linker.rs @@ -32,11 +32,15 @@ use std::rc::Rc; /// name `foo` and item name `bar`, so long as they have different function /// signatures. Currently duplicate memories and tables are not allowed, only /// one-per-name is allowed. +/// +/// Note that allowing duplicates by shadowing the previous definition can be +/// controlled with the [`Linker::allow_shadowing`] method as well. pub struct Linker { store: Store, string2idx: HashMap, usize>, strings: Vec>, map: HashMap, + allow_shadowing: bool, } #[derive(Hash, PartialEq, Eq)] @@ -77,9 +81,40 @@ impl Linker { map: HashMap::new(), string2idx: HashMap::new(), strings: Vec::new(), + allow_shadowing: false, } } + /// Configures whether this [`Linker`] will shadow previous duplicate + /// definitions of the same signature. + /// + /// By default a [`Linker`] will disallow duplicate definitions of the same + /// signature. This method, however, can be used to instead allow duplicates + /// and have the latest definition take precedence when linking modules. + /// + /// # Examples + /// + /// ``` + /// # use wasmtime::*; + /// # fn main() -> anyhow::Result<()> { + /// # let store = Store::default(); + /// let mut linker = Linker::new(&store); + /// linker.func("", "", || {})?; + /// + /// // by default, duplicates are disallowed + /// assert!(linker.func("", "", || {}).is_err()); + /// + /// // but shadowing can be configured to be allowed as well + /// linker.allow_shadowing(true); + /// linker.func("", "", || {})?; + /// # Ok(()) + /// # } + /// ``` + pub fn allow_shadowing(&mut self, allow: bool) -> &mut Linker { + self.allow_shadowing = allow; + self + } + /// Defines a new item in this [`Linker`]. /// /// This method will add a new definition, by name, to this instance of @@ -89,8 +124,8 @@ impl Linker { /// # Errors /// /// Returns an error if the `module` and `name` already identify an item - /// of the same type as the `item` provided. For more information see the - /// documentation on [`Linker`]. + /// of the same type as the `item` provided and if shadowing is disallowed. + /// For more information see the documentation on [`Linker`]. /// /// Also returns an error if `item` comes from a different store than this /// [`Linker`] was created with. @@ -104,7 +139,7 @@ impl Linker { /// let mut linker = Linker::new(&store); /// let ty = GlobalType::new(ValType::I32, Mutability::Const); /// let global = Global::new(&store, ty, Val::I32(0x1234))?; - /// linker.define("host", "offset", global); + /// linker.define("host", "offset", global)?; /// /// let wat = r#" /// (module @@ -142,9 +177,9 @@ impl Linker { /// /// # Errors /// - /// Returns an error if the `module` and `name` already identify a function - /// of the same signature as `func`. For more information see the - /// documentation on [`Linker`]. + /// Returns an error if the `module` and `name` already identify an item + /// of the same type as the `item` provided and if shadowing is disallowed. + /// For more information see the documentation on [`Linker`]. /// /// # Examples /// @@ -190,9 +225,9 @@ impl Linker { /// # Errors /// /// Returns an error if the any item is redefined twice in this linker (for - /// example the same `module_name` was already defined), or if `instance` - /// comes from a different [`Store`] than this [`Linker`] originally was - /// created with. + /// example the same `module_name` was already defined) and shadowing is + /// disallowed, or if `instance` comes from a different [`Store`] than this + /// [`Linker`] originally was created with. /// /// # Examples /// @@ -238,12 +273,15 @@ impl Linker { fn insert(&mut self, module: &str, name: &str, ty: &ExternType, item: Extern) -> Result<()> { let key = self.import_key(module, name, ty); match self.map.entry(key) { - Entry::Occupied(o) => bail!( - "import of `{}::{}` with as {:?} defined twice", + Entry::Occupied(o) if !self.allow_shadowing => bail!( + "import of `{}::{}` with kind {:?} defined twice", module, name, o.key().kind, ), + Entry::Occupied(mut o) => { + o.insert(item); + } Entry::Vacant(v) => { v.insert(item); } @@ -337,14 +375,14 @@ impl Linker { } if options.len() == 0 { bail!( - "import of `{}::{}` has not been defined", + "unknown import: `{}::{}` has not been defined", import.module(), import.name() ) } bail!( - "failed to find import of `{}::{}` with matching signature\n\ + "incompatible import type for `{}::{}` specified\n\ desired signature was: {:?}\n\ signatures available:\n\n{}", import.module(), @@ -365,4 +403,9 @@ impl Linker { }; self.map.get(&key) } + + /// Returns the [`Store`] that this linker is connected to. + pub fn store(&self) -> &Store { + &self.store + } } diff --git a/crates/api/tests/linker.rs b/crates/api/tests/linker.rs index 4edba8eee9..2fcb589805 100644 --- a/crates/api/tests/linker.rs +++ b/crates/api/tests/linker.rs @@ -64,3 +64,34 @@ fn link_twice_bad() -> Result<()> { assert!(linker.define("", "", table.clone()).is_err()); Ok(()) } + +#[test] +fn interposition() -> Result<()> { + let store = Store::default(); + let mut linker = Linker::new(&store); + linker.allow_shadowing(true); + let mut module = Module::new( + &store, + r#"(module (func (export "export") (result i32) (i32.const 7)))"#, + )?; + for _ in 0..4 { + let instance = linker.instantiate(&module)?; + linker.define( + "red", + "green", + instance.get_export("export").unwrap().clone(), + )?; + module = Module::new( + &store, + r#"(module + (import "red" "green" (func (result i32))) + (func (export "export") (result i32) (i32.mul (call 0) (i32.const 2))) + )"#, + )?; + } + let instance = linker.instantiate(&module)?; + let func = instance.get_export("export").unwrap().func().unwrap(); + let func = func.get0::()?; + assert_eq!(func()?, 112); + Ok(()) +} diff --git a/crates/wast/src/lib.rs b/crates/wast/src/lib.rs index 4f8f54f733..1b136d9776 100644 --- a/crates/wast/src/lib.rs +++ b/crates/wast/src/lib.rs @@ -25,7 +25,7 @@ mod spectest; mod wast; -pub use crate::spectest::instantiate_spectest; +pub use crate::spectest::link_spectest; pub use crate::wast::WastContext; /// Version number of this crate. diff --git a/crates/wast/src/spectest.rs b/crates/wast/src/spectest.rs index 3004e1e04d..60f6a6d09f 100644 --- a/crates/wast/src/spectest.rs +++ b/crates/wast/src/spectest.rs @@ -1,61 +1,46 @@ -use std::collections::HashMap; +use anyhow::Result; use wasmtime::*; /// Return an instance implementing the "spectest" interface used in the /// spec testsuite. -pub fn instantiate_spectest(store: &Store) -> HashMap<&'static str, Extern> { - let mut ret = HashMap::new(); - - let func = Func::wrap(store, || {}); - ret.insert("print", Extern::Func(func)); - - let func = Func::wrap(store, |val: i32| println!("{}: i32", val)); - ret.insert("print_i32", Extern::Func(func)); - - let func = Func::wrap(store, |val: i64| println!("{}: i64", val)); - ret.insert("print_i64", Extern::Func(func)); - - let func = Func::wrap(store, |val: f32| println!("{}: f32", val)); - ret.insert("print_f32", Extern::Func(func)); - - let func = Func::wrap(store, |val: f64| println!("{}: f64", val)); - ret.insert("print_f64", Extern::Func(func)); - - let func = Func::wrap(store, |i: i32, f: f32| { +pub fn link_spectest(linker: &mut Linker) -> Result<()> { + linker.func("spectest", "print", || {})?; + linker.func("spectest", "print_i32", |val: i32| println!("{}: i32", val))?; + linker.func("spectest", "print_i64", |val: i64| println!("{}: i64", val))?; + linker.func("spectest", "print_f32", |val: f32| println!("{}: f32", val))?; + linker.func("spectest", "print_f64", |val: f64| println!("{}: f64", val))?; + linker.func("spectest", "print_i32_f32", |i: i32, f: f32| { println!("{}: i32", i); println!("{}: f32", f); - }); - ret.insert("print_i32_f32", Extern::Func(func)); - - let func = Func::wrap(store, |f1: f64, f2: f64| { + })?; + linker.func("spectest", "print_f64_f64", |f1: f64, f2: f64| { println!("{}: f64", f1); println!("{}: f64", f2); - }); - ret.insert("print_f64_f64", Extern::Func(func)); + })?; let ty = GlobalType::new(ValType::I32, Mutability::Const); - let g = Global::new(store, ty, Val::I32(666)).unwrap(); - ret.insert("global_i32", Extern::Global(g)); + let g = Global::new(linker.store(), ty, Val::I32(666))?; + linker.define("spectest", "global_i32", g)?; let ty = GlobalType::new(ValType::I64, Mutability::Const); - let g = Global::new(store, ty, Val::I64(666)).unwrap(); - ret.insert("global_i64", Extern::Global(g)); + let g = Global::new(linker.store(), ty, Val::I64(666))?; + linker.define("spectest", "global_i64", g)?; let ty = GlobalType::new(ValType::F32, Mutability::Const); - let g = Global::new(store, ty, Val::F32(0x4426_8000)).unwrap(); - ret.insert("global_f32", Extern::Global(g)); + let g = Global::new(linker.store(), ty, Val::F32(0x4426_8000))?; + linker.define("spectest", "global_f32", g)?; let ty = GlobalType::new(ValType::F64, Mutability::Const); - let g = Global::new(store, ty, Val::F64(0x4084_d000_0000_0000)).unwrap(); - ret.insert("global_f64", Extern::Global(g)); + let g = Global::new(linker.store(), ty, Val::F64(0x4084_d000_0000_0000))?; + linker.define("spectest", "global_f64", g)?; let ty = TableType::new(ValType::FuncRef, Limits::new(10, Some(20))); - let table = Table::new(store, ty, Val::AnyRef(AnyRef::Null)).unwrap(); - ret.insert("table", Extern::Table(table)); + let table = Table::new(linker.store(), ty, Val::AnyRef(AnyRef::Null))?; + linker.define("spectest", "table", table)?; let ty = MemoryType::new(Limits::new(1, Some(2))); - let memory = Memory::new(store, ty); - ret.insert("memory", Extern::Memory(memory)); + let memory = Memory::new(linker.store(), ty); + linker.define("spectest", "memory", memory)?; - return ret; + Ok(()) } diff --git a/crates/wast/src/wast.rs b/crates/wast/src/wast.rs index 453c255de1..456b9a2dfa 100644 --- a/crates/wast/src/wast.rs +++ b/crates/wast/src/wast.rs @@ -1,4 +1,4 @@ -use crate::spectest::instantiate_spectest; +use crate::spectest::link_spectest; use anyhow::{anyhow, bail, Context as _, Result}; use std::collections::HashMap; use std::path::Path; @@ -30,8 +30,8 @@ pub struct WastContext { current: Option, instances: HashMap, + linker: Linker, store: Store, - spectest: Option>, } enum Outcome> { @@ -51,11 +51,16 @@ impl Outcome { impl WastContext { /// Construct a new instance of `WastContext`. pub fn new(store: Store) -> Self { + // Spec tests will redefine the same module/name sometimes, so we need + // to allow shadowing in the linker which picks the most recent + // definition as what to link when linking. + let mut linker = Linker::new(&store); + linker.allow_shadowing(true); Self { current: None, - store, - spectest: None, instances: HashMap::new(), + linker, + store, } } @@ -75,31 +80,7 @@ impl WastContext { fn instantiate(&self, module: &[u8]) -> Result> { let module = Module::new(&self.store, module)?; - let mut imports = Vec::new(); - for import in module.imports() { - if import.module() == "spectest" { - let spectest = self - .spectest - .as_ref() - .ok_or_else(|| anyhow!("spectest module isn't instantiated"))?; - let export = spectest - .get(import.name()) - .ok_or_else(|| anyhow!("unknown import `spectest::{}`", import.name()))?; - imports.push(export.clone()); - continue; - } - - let instance = self - .instances - .get(import.module()) - .ok_or_else(|| anyhow!("no module named `{}`", import.module()))?; - let export = instance - .get_export(import.name()) - .ok_or_else(|| anyhow!("unknown import `{}::{}`", import.name(), import.module()))? - .clone(); - imports.push(export); - } - let instance = match Instance::new(&module, &imports) { + let instance = match self.linker.instantiate(&module) { Ok(i) => i, Err(e) => return e.downcast::().map(Outcome::Trap), }; @@ -108,7 +89,7 @@ impl WastContext { /// Register "spectest" which is used by the spec testsuite. pub fn register_spectest(&mut self) -> Result<()> { - self.spectest = Some(instantiate_spectest(&self.store)); + link_spectest(&mut self.linker)?; Ok(()) } @@ -145,6 +126,7 @@ impl WastContext { }; if let Some(name) = instance_name { self.instances.insert(name.to_string(), instance.clone()); + self.linker.instance(name, &instance)?; } self.current = Some(instance); Ok(()) @@ -153,6 +135,7 @@ impl WastContext { /// Register an instance to make it available for performing actions. fn register(&mut self, name: Option<&str>, as_name: &str) -> Result<()> { let instance = self.get_instance(name)?.clone(); + self.linker.instance(as_name, &instance)?; self.instances.insert(as_name.to_string(), instance); Ok(()) }