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

@@ -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() {}
 /// ```

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;

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<Rc<str>, usize>,
     strings: Vec<Rc<str>>,
     map: HashMap<ImportKey, Extern>,
+    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
+    /// of the same type as the `item` provided and if shadowing is disallowed.
-    /// documentation on [`Linker`].
+    /// 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
+    /// Returns an error if the `module` and `name` already identify an item
-    /// of the same signature as `func`. For more information see the
+    /// of the same type as the `item` provided and if shadowing is disallowed.
-    /// documentation on [`Linker`].
+    /// 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`
+    /// example the same `module_name` was already defined) and shadowing is
-    /// comes from a different [`Store`] than this [`Linker`] originally was
+    /// disallowed, or if `instance` comes from a different [`Store`] than this
-    /// created with.
+    /// [`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!(
+            Entry::Occupied(o) if !self.allow_shadowing => bail!(
-                "import of `{}::{}` with as {:?} defined twice",
+                "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
+    }
 }

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::<i32>()?;
+    assert_eq!(func()?, 112);
+    Ok(())
+}

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.

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> {
+pub fn link_spectest(linker: &mut Linker) -> Result<()> {
-    let mut ret = HashMap::new();
+    linker.func("spectest", "print", || {})?;
-
+    linker.func("spectest", "print_i32", |val: i32| println!("{}: i32", val))?;
-    let func = Func::wrap(store, || {});
+    linker.func("spectest", "print_i64", |val: i64| println!("{}: i64", val))?;
-    ret.insert("print", Extern::Func(func));
+    linker.func("spectest", "print_f32", |val: f32| println!("{}: f32", val))?;
-
+    linker.func("spectest", "print_f64", |val: f64| println!("{}: f64", val))?;
-    let func = Func::wrap(store, |val: i32| println!("{}: i32", val));
+    linker.func("spectest", "print_i32_f32", |i: i32, f: f32| {
-    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| {
         println!("{}: i32", i);
         println!("{}: f32", f);
-    });
+    })?;
-    ret.insert("print_i32_f32", Extern::Func(func));
+    linker.func("spectest", "print_f64_f64", |f1: f64, f2: f64| {
-
-    let func = Func::wrap(store, |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();
+    let g = Global::new(linker.store(), ty, Val::I32(666))?;
-    ret.insert("global_i32", Extern::Global(g));
+    linker.define("spectest", "global_i32", g)?;
 
     let ty = GlobalType::new(ValType::I64, Mutability::Const);
-    let g = Global::new(store, ty, Val::I64(666)).unwrap();
+    let g = Global::new(linker.store(), ty, Val::I64(666))?;
-    ret.insert("global_i64", Extern::Global(g));
+    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();
+    let g = Global::new(linker.store(), ty, Val::F32(0x4426_8000))?;
-    ret.insert("global_f32", Extern::Global(g));
+    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();
+    let g = Global::new(linker.store(), ty, Val::F64(0x4084_d000_0000_0000))?;
-    ret.insert("global_f64", Extern::Global(g));
+    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();
+    let table = Table::new(linker.store(), ty, Val::AnyRef(AnyRef::Null))?;
-    ret.insert("table", Extern::Table(table));
+    linker.define("spectest", "table", table)?;
 
     let ty = MemoryType::new(Limits::new(1, Some(2)));
-    let memory = Memory::new(store, ty);
+    let memory = Memory::new(linker.store(), ty);
-    ret.insert("memory", Extern::Memory(memory));
+    linker.define("spectest", "memory", memory)?;
 
-    return ret;
+    Ok(())
 }

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<Instance>,
 
     instances: HashMap<String, Instance>,
+    linker: Linker,
     store: Store,
-    spectest: Option<HashMap<&'static str, Extern>>,
 }
 
 enum Outcome<T = Vec<Val>> {
@@ -51,11 +51,16 @@ impl<T> Outcome<T> {
 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<Outcome<Instance>> {
         let module = Module::new(&self.store, module)?;
-        let mut imports = Vec::new();
+        let instance = match self.linker.instantiate(&module) {
-        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) {
             Ok(i) => i,
             Err(e) => return e.downcast::<Trap>().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(())
     }