Fix duplicated imported linear memories and tables (#593)

* add regression tests asserting the existence of the issue

* add iterators over internal memories and tables to fix the issue

* apply clippy suggestions

* fix intra doc links

crates/wasmi/src/module/instantiate/mod.rs

@@ -1,6 +1,9 @@
 mod error;
 mod pre;
 
+#[cfg(test)]
+mod tests;
+
 pub use self::{error::InstantiationError, pre::InstancePre};
 use super::{export, InitExpr, Module, ModuleImportType};
 use crate::{
@@ -190,7 +193,7 @@ impl Module {
     ///
     /// [`Store`]: struct.Store.html
     fn extract_tables(&self, context: &mut impl AsContextMut, builder: &mut InstanceEntityBuilder) {
-        for table_type in self.tables.iter().copied() {
+        for table_type in self.internal_tables().copied() {
             builder.push_table(Table::new(context.as_context_mut(), table_type));
         }
     }
@@ -205,7 +208,7 @@ impl Module {
         context: &mut impl AsContextMut,
         builder: &mut InstanceEntityBuilder,
     ) {
-        for memory_type in self.memories.iter().copied() {
+        for memory_type in self.internal_memories().copied() {
             let memory =
                 Memory::new(context.as_context_mut(), memory_type).unwrap_or_else(|error| {
                     panic!(

crates/wasmi/src/module/instantiate/tests.rs

@@ -0,0 +1,104 @@
+//! Regression tests for GitHub issue:
+//! https://github.com/paritytech/wasmi/issues/587
+//!
+//! The problem was that Wasm memories (and tables) were defined twice for a
+//! `wasmi` instance for every imported Wasm memory (or table). Since `wasmi`
+//! does not support the `multi-memory` Wasm proposal this resulted Wasm
+//! instances with more than 1 memory (or table) if the Wasm module imported
+//! those entities.
+
+use crate::{Engine, Error, Instance, Linker, Memory, MemoryType, Module, Store, Table, TableType};
+
+fn try_instantiate_from_wat(wat: &str) -> Result<(Store<()>, Instance), Error> {
+    let wasm = wat::parse_str(wat).unwrap();
+    let engine = Engine::default();
+    let module = Module::new(&engine, &mut &wasm[..])?;
+    let mut store = Store::new(&engine, ());
+    let mut linker = <Linker<()>>::new();
+    // Define one memory that can be used by the tests as import.
+    let memory_type = MemoryType::new(4, None)?;
+    let memory = Memory::new(&mut store, memory_type)?;
+    linker.define("env", "memory", memory)?;
+    // Define one table that can be used by the tests as import.
+    let table_type = TableType::new(4, None);
+    let table = Table::new(&mut store, table_type);
+    linker.define("env", "table", table)?;
+    let instance = linker
+        .instantiate(&mut store, &module)
+        .unwrap()
+        .start(&mut store)?;
+    Ok((store, instance))
+}
+
+fn instantiate_from_wat(wat: &str) -> (Store<()>, Instance) {
+    try_instantiate_from_wat(wat).unwrap()
+}
+
+fn assert_no_duplicates(store: &Store<()>, instance: Instance) {
+    assert!(instance.get_memory(store, 1).is_none());
+    assert!(instance.get_table(store, 1).is_none());
+}
+
+#[test]
+fn test_import_memory_and_table() {
+    let wat = r#"
+        (module
+            (import "env" "memory" (memory 4))
+            (import "env" "table" (table 4 funcref))
+        )"#;
+    let (store, instance) = instantiate_from_wat(wat);
+    assert!(instance.get_memory(&store, 0).is_some());
+    assert!(instance.get_table(&store, 0).is_some());
+    assert_no_duplicates(&store, instance);
+}
+
+#[test]
+fn test_import_memory() {
+    let wat = r#"
+        (module
+            (import "env" "memory" (memory 4))
+        )"#;
+    let (store, instance) = instantiate_from_wat(wat);
+    assert!(instance.get_memory(&store, 0).is_some());
+    assert!(instance.get_table(&store, 0).is_none());
+    assert_no_duplicates(&store, instance);
+}
+
+#[test]
+fn test_import_table() {
+    let wat = r#"
+        (module
+            (import "env" "table" (table 4 funcref))
+        )"#;
+    let (store, instance) = instantiate_from_wat(wat);
+    assert!(instance.get_memory(&store, 0).is_none());
+    assert!(instance.get_table(&store, 0).is_some());
+    assert_no_duplicates(&store, instance);
+}
+
+#[test]
+fn test_no_memory_no_table() {
+    let wat = "(module)";
+    let (store, instance) = instantiate_from_wat(wat);
+    assert!(instance.get_memory(&store, 0).is_none());
+    assert!(instance.get_table(&store, 0).is_none());
+    assert_no_duplicates(&store, instance);
+}
+
+#[test]
+fn test_internal_memory() {
+    let wat = "(module (memory 1 10) )";
+    let (store, instance) = instantiate_from_wat(wat);
+    assert!(instance.get_memory(&store, 0).is_some());
+    assert!(instance.get_table(&store, 0).is_none());
+    assert_no_duplicates(&store, instance);
+}
+
+#[test]
+fn test_internal_table() {
+    let wat = "(module (table 4 funcref) )";
+    let (store, instance) = instantiate_from_wat(wat);
+    assert!(instance.get_memory(&store, 0).is_none());
+    assert!(instance.get_table(&store, 0).is_some());
+    assert_no_duplicates(&store, instance);
+}

crates/wasmi/src/module/mod.rs

@@ -101,13 +101,23 @@ pub struct ModuleImports {
     len_funcs: usize,
     /// The amount of imported [`Global`].
     len_globals: usize,
+    /// The amount of imported [`Memory`].
+    ///
+    /// [`Memory`]: [`crate::Memory`]
+    len_memories: usize,
+    /// The amount of imported [`Table`].
+    ///
+    /// [`Table`]: [`crate::Table`]
+    len_tables: usize,
 }
 
 impl ModuleImports {
     /// Creates a new [`ModuleImports`] from the [`ModuleBuilder`] definitions.
     fn from_builder(imports: builder::ModuleImports) -> Self {
         let len_funcs = imports.funcs.len();
         let len_globals = imports.globals.len();
+        let len_memories = imports.memories.len();
+        let len_tables = imports.tables.len();
         let funcs = imports.funcs.into_iter().map(Imported::Func);
         let tables = imports.tables.into_iter().map(Imported::Table);
         let memories = imports.memories.into_iter().map(Imported::Memory);
@@ -122,6 +132,8 @@ impl ModuleImports {
             items,
             len_funcs,
             len_globals,
+            len_memories,
+            len_tables,
         }
     }
 }
@@ -208,6 +220,26 @@ impl Module {
         }
     }
 
+    /// Returns an iterator over the [`MemoryType`] of internal linear memories.
+    fn internal_memories(&self) -> SliceIter<MemoryType> {
+        let len_imported = self.imports.len_memories;
+        // We skip the first `len_imported` elements in `memories`
+        // since they refer to imported and not internally defined
+        // linear memories.
+        let memories = &self.memories[len_imported..];
+        memories.iter()
+    }
+
+    /// Returns an iterator over the [`TableType`] of internal tables.
+    fn internal_tables(&self) -> SliceIter<TableType> {
+        let len_imported = self.imports.len_tables;
+        // We skip the first `len_imported` elements in `memories`
+        // since they refer to imported and not internally defined
+        // linear memories.
+        let tables = &self.tables[len_imported..];
+        tables.iter()
+    }
+
     /// Returns an iterator over the internally defined [`Global`].
     fn internal_globals(&self) -> InternalGlobalsIter {
         let len_imported = self.imports.len_globals;