Handle same-named imports with different signatures

This commit fixes the `wasmtime::Instance` instantiation API when
imports have the same name but might be imported under different types.
This is handled in the API by listing imports as a list instead of as a
name map, but they were interpreted as a name map under the hood causing
collisions.

This commit now keeps track of the index used to define each import, and
the index is passed through in the `Resolver`. Existing implementaitons
of `Resolver` all ignore this, but the API now uses it exclusivley to
match up `Extern` definitions to imports.

crates/api/src/externals.rs

@@ -56,7 +56,7 @@ impl Extern {
         }
     }
 
-    pub(crate) fn get_wasmtime_export(&mut self) -> wasmtime_runtime::Export {
+    pub(crate) fn get_wasmtime_export(&self) -> wasmtime_runtime::Export {
         match self {
             Extern::Func(f) => f.borrow().wasmtime_export().clone(),
             Extern::Global(g) => g.borrow().wasmtime_export().clone(),

crates/api/src/instance.rs

@@ -12,23 +12,21 @@ use std::rc::Rc;
 use wasmtime_jit::{instantiate, Resolver, SetupError};
 use wasmtime_runtime::{Export, InstanceHandle, InstantiationError};
 
-struct SimpleResolver {
+struct SimpleResolver<'a> {
-    imports: Vec<(String, String, Extern)>,
+    imports: &'a [Extern],
 }
 
-impl Resolver for SimpleResolver {
+impl Resolver for SimpleResolver<'_> {
-    fn resolve(&mut self, name: &str, field: &str) -> Option<Export> {
+    fn resolve(&mut self, idx: u32, _name: &str, _field: &str) -> Option<Export> {
-        // TODO speedup lookup
         self.imports
-            .iter_mut()
+            .get(idx as usize)
-            .find(|(n, f, _)| name == n && field == f)
+            .map(|i| i.get_wasmtime_export())
-            .map(|(_, _, e)| e.get_wasmtime_export())
     }
 }
 
 pub fn instantiate_in_context(
     data: &[u8],
-    imports: Vec<(String, String, Extern)>,
+    imports: &[Extern],
     module_name: Option<String>,
     context: Context,
     exports: Rc<RefCell<HashMap<String, Option<wasmtime_runtime::Export>>>>,
@@ -73,15 +71,9 @@ impl Instance {
     pub fn new(store: &Store, module: &Module, externs: &[Extern]) -> Result<Instance, Error> {
         let context = store.context().clone();
         let exports = store.global_exports().clone();
-        let imports = module
-            .imports()
-            .iter()
-            .zip(externs.iter())
-            .map(|(i, e)| (i.module().to_string(), i.name().to_string(), e.clone()))
-            .collect::<Vec<_>>();
         let (mut instance_handle, contexts) = instantiate_in_context(
             module.binary().expect("binary"),
-            imports,
+            externs,
             module.name().cloned(),
             context,
             exports,

crates/api/tests/import-indexes.rs

@@ -0,0 +1,68 @@
+use std::rc::Rc;
+use wasmtime::*;
+
+#[test]
+fn same_import_names_still_distinct() -> anyhow::Result<()> {
+    const WAT: &str = r#"
+(module
+  (import "" "" (func $a (result i32)))
+  (import "" "" (func $b (result f32)))
+  (func (export "foo") (result i32)
+    call $a
+    call $b
+    i32.trunc_f32_u
+    i32.add)
+)
+    "#;
+
+    struct Ret1;
+
+    impl Callable for Ret1 {
+        fn call(&self, params: &[Val], results: &mut [Val]) -> Result<(), Trap> {
+            assert!(params.is_empty());
+            assert_eq!(results.len(), 1);
+            results[0] = 1i32.into();
+            Ok(())
+        }
+    }
+
+    struct Ret2;
+
+    impl Callable for Ret2 {
+        fn call(&self, params: &[Val], results: &mut [Val]) -> Result<(), Trap> {
+            assert!(params.is_empty());
+            assert_eq!(results.len(), 1);
+            results[0] = 2.0f32.into();
+            Ok(())
+        }
+    }
+
+    let store = Store::default();
+    let wasm = wat::parse_str(WAT)?;
+    let module = Module::new(&store, &wasm)?;
+
+    let imports = [
+        HostRef::new(Func::new(
+            &store,
+            FuncType::new(Box::new([]), Box::new([ValType::I32])),
+            Rc::new(Ret1),
+        ))
+        .into(),
+        HostRef::new(Func::new(
+            &store,
+            FuncType::new(Box::new([]), Box::new([ValType::F32])),
+            Rc::new(Ret2),
+        ))
+        .into(),
+    ];
+    let instance = Instance::new(&store, &module, &imports)?;
+
+    let func = instance.find_export_by_name("foo").unwrap().func().unwrap();
+    let results = func.borrow().call(&[])?;
+    assert_eq!(results.len(), 1);
+    match results[0] {
+        Val::I32(n) => assert_eq!(n, 3),
+        _ => panic!("unexpected type of return"),
+    }
+    Ok(())
+}

crates/environ/src/module.rs

@@ -136,17 +136,18 @@ pub struct Module {
     /// Unprocessed signatures exactly as provided by `declare_signature()`.
     pub signatures: PrimaryMap<SignatureIndex, ir::Signature>,
 
-    /// Names of imported functions.
+    /// Names of imported functions, as well as the index of the import that
-    pub imported_funcs: PrimaryMap<FuncIndex, (String, String)>,
+    /// performed this import.
+    pub imported_funcs: PrimaryMap<FuncIndex, (String, String, u32)>,
 
     /// Names of imported tables.
-    pub imported_tables: PrimaryMap<TableIndex, (String, String)>,
+    pub imported_tables: PrimaryMap<TableIndex, (String, String, u32)>,
 
     /// Names of imported memories.
-    pub imported_memories: PrimaryMap<MemoryIndex, (String, String)>,
+    pub imported_memories: PrimaryMap<MemoryIndex, (String, String, u32)>,
 
     /// Names of imported globals.
-    pub imported_globals: PrimaryMap<GlobalIndex, (String, String)>,
+    pub imported_globals: PrimaryMap<GlobalIndex, (String, String, u32)>,
 
     /// Types of functions, imported and local.
     pub functions: PrimaryMap<FuncIndex, SignatureIndex>,

crates/environ/src/module_environ.rs

@@ -55,6 +55,7 @@ impl<'data> ModuleTranslation<'data> {
 pub struct ModuleEnvironment<'data> {
     /// The result to be filled in.
     result: ModuleTranslation<'data>,
+    imports: u32,
 }
 
 impl<'data> ModuleEnvironment<'data> {
@@ -69,6 +70,7 @@ impl<'data> ModuleEnvironment<'data> {
                 tunables,
                 module_translation: None,
             },
+            imports: 0,
         }
     }
 
@@ -123,10 +125,12 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
         );
         self.result.module.functions.push(sig_index);
 
-        self.result
+        self.result.module.imported_funcs.push((
-            .module
+            String::from(module),
-            .imported_funcs
+            String::from(field),
-            .push((String::from(module), String::from(field)));
+            self.imports,
+        ));
+        self.imports += 1;
         Ok(())
     }
 
@@ -139,10 +143,12 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
         let plan = TablePlan::for_table(table, &self.result.tunables);
         self.result.module.table_plans.push(plan);
 
-        self.result
+        self.result.module.imported_tables.push((
-            .module
+            String::from(module),
-            .imported_tables
+            String::from(field),
-            .push((String::from(module), String::from(field)));
+            self.imports,
+        ));
+        self.imports += 1;
         Ok(())
     }
 
@@ -160,10 +166,12 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
         let plan = MemoryPlan::for_memory(memory, &self.result.tunables);
         self.result.module.memory_plans.push(plan);
 
-        self.result
+        self.result.module.imported_memories.push((
-            .module
+            String::from(module),
-            .imported_memories
+            String::from(field),
-            .push((String::from(module), String::from(field)));
+            self.imports,
+        ));
+        self.imports += 1;
         Ok(())
     }
 
@@ -180,10 +188,12 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
         );
         self.result.module.globals.push(global);
 
-        self.result
+        self.result.module.imported_globals.push((
-            .module
+            String::from(module),
-            .imported_globals
+            String::from(field),
-            .push((String::from(module), String::from(field)));
+            self.imports,
+        ));
+        self.imports += 1;
         Ok(())
     }
 

crates/jit/src/link.rs

@@ -30,8 +30,8 @@ pub fn link_module(
     let mut dependencies = HashSet::new();
 
     let mut function_imports = PrimaryMap::with_capacity(module.imported_funcs.len());
-    for (index, (ref module_name, ref field)) in module.imported_funcs.iter() {
+    for (index, (module_name, field, import_idx)) in module.imported_funcs.iter() {
-        match resolver.resolve(module_name, field) {
+        match resolver.resolve(*import_idx, module_name, field) {
             Some(export_value) => match export_value {
                 Export::Function {
                     address,
@@ -71,8 +71,8 @@ pub fn link_module(
     }
 
     let mut table_imports = PrimaryMap::with_capacity(module.imported_tables.len());
-    for (index, (ref module_name, ref field)) in module.imported_tables.iter() {
+    for (index, (module_name, field, import_idx)) in module.imported_tables.iter() {
-        match resolver.resolve(module_name, field) {
+        match resolver.resolve(*import_idx, module_name, field) {
             Some(export_value) => match export_value {
                 Export::Table {
                     definition,
@@ -110,8 +110,8 @@ pub fn link_module(
     }
 
     let mut memory_imports = PrimaryMap::with_capacity(module.imported_memories.len());
-    for (index, (ref module_name, ref field)) in module.imported_memories.iter() {
+    for (index, (module_name, field, import_idx)) in module.imported_memories.iter() {
-        match resolver.resolve(module_name, field) {
+        match resolver.resolve(*import_idx, module_name, field) {
             Some(export_value) => match export_value {
                 Export::Memory {
                     definition,
@@ -163,8 +163,8 @@ pub fn link_module(
     }
 
     let mut global_imports = PrimaryMap::with_capacity(module.imported_globals.len());
-    for (index, (ref module_name, ref field)) in module.imported_globals.iter() {
+    for (index, (module_name, field, import_idx)) in module.imported_globals.iter() {
-        match resolver.resolve(module_name, field) {
+        match resolver.resolve(*import_idx, module_name, field) {
             Some(export_value) => match export_value {
                 Export::Table { .. } | Export::Memory { .. } | Export::Function { .. } => {
                     return Err(LinkError(format!(

crates/jit/src/namespace.rs

@@ -36,7 +36,7 @@ impl Namespace {
 }
 
 impl Resolver for Namespace {
-    fn resolve(&mut self, name: &str, field: &str) -> Option<Export> {
+    fn resolve(&mut self, _idx: u32, name: &str, field: &str) -> Option<Export> {
         if let Some(instance) = self.names.get_mut(name) {
             instance.lookup(field)
         } else {

crates/jit/src/resolver.rs

@@ -5,15 +5,22 @@ use wasmtime_runtime::Export;
 
 /// Import resolver connects imports with available exported values.
 pub trait Resolver {
-    /// Resolve the given module/field combo.
+    /// Resolves an import a WebAssembly module to an export it's hooked up to.
-    fn resolve(&mut self, module: &str, field: &str) -> Option<Export>;
+    ///
+    /// The `index` provided is the index of the import in the wasm module
+    /// that's being resolved. For example 1 means that it's the second import
+    /// listed in the wasm module.
+    ///
+    /// The `module` and `field` arguments provided are the module/field names
+    /// listed on the import itself.
+    fn resolve(&mut self, index: u32, module: &str, field: &str) -> Option<Export>;
 }
 
 /// `Resolver` implementation that always resolves to `None`.
 pub struct NullResolver {}
 
 impl Resolver for NullResolver {
-    fn resolve(&mut self, _module: &str, _field: &str) -> Option<Export> {
+    fn resolve(&mut self, _idx: u32, _module: &str, _field: &str) -> Option<Export> {
         None
     }
 }