Fix module-linking handling of instance subtypes (#2466)

* Fix module-linking handling of instance subtypes

When we alias the nth export of an instance, due to subtyping the nth
export may not actually be what we want. Instead we need to look at our
local type definition's nth export's name, and lookup that name off the
export.

* Update crates/wasmtime/src/instance.rs

Co-authored-by: Peter Huene <[email protected]>

Co-authored-by: Peter Huene <[email protected]>

crates/wasmtime/src/instance.rs

@@ -82,18 +82,29 @@ fn instantiate(
                 imports.modules.push(module.submodule(*idx));
             }
 
-            // Here we lookup our instance handle, ask it for the nth export,
+            // Here we lookup our instance handle, find the right export,
             // and then push that item into our own index space. We eschew
             // type-checking since only valid modules reach this point.
             //
+            // Note that export lookup here needs to happen by name. The
+            // `export` index is an index into our local type definition of the
+            // type of the instance to figure out what name it was assigned.
+            // This is where the subtyping happens!
+            //
             // Note that the unsafety here is because we're asserting that the
             // handle comes from our same store, but this should be true because
             // we acquired the handle from an instance in the store.
             Initializer::AliasInstanceExport { instance, export } => {
+                let instance_ty = env_module.instances[*instance];
+                let export_name = module.types().instance_signatures[instance_ty]
+                    .exports
+                    .get_index(*export)
+                    .expect("validation bug - should be valid")
+                    .0;
                 let handle = &imports.instances[*instance];
-                let export_index = &handle.module().exports[*export];
+                let entity_index = &handle.module().exports[export_name];
                 let item = Extern::from_wasmtime_export(
-                    handle.lookup_by_declaration(export_index),
+                    handle.lookup_by_declaration(entity_index),
                     unsafe { store.existing_instance_handle(handle.clone()) },
                 );
                 imports.push_extern(&item);

tests/misc_testsuite/module-linking/instantiate.wast

@@ -233,3 +233,52 @@
     (import "b" "i" (instance (export "" (func))))
   )
   "instance types incompatible")
+
+;; ensure we ignore other exported items
+(module $b
+  (module $m
+    (func (export "f") (result i32)
+      i32.const 300)
+    (global (export "g") i32 (i32.const 0xfeed))
+  )
+
+  (instance (export "i") (instantiate 0))
+)
+(module
+  (import "b" "i" (instance $i
+    (export "g" (global $g i32))
+  ))
+
+  (func (export "get") (result i32)
+    global.get $i.$g)
+)
+(assert_return (invoke "get") (i32.const 0xfeed))
+
+;; ensure the right export is used even when subtyping comes into play
+(module $b
+  (module $m
+    (func (export "f") (result i32)
+      i32.const 300)
+    (func (export "g") (param i32) (result i32)
+      i32.const 100
+      local.get 0
+      i32.add)
+  )
+
+  (instance (export "i") (instantiate 0))
+)
+(module
+  (import "b" "i" (instance $i
+    ;; notice that this order is swapped
+    (export "g" (func $g (param i32) (result i32)))
+    (export "f" (func $f (result i32)))
+  ))
+
+  (func (export "f") (result i32)
+    call $i.$f)
+  (func (export "g") (param i32) (result i32)
+    local.get 0
+    call $i.$g)
+)
+(assert_return (invoke "f") (i32.const 300))
+(assert_return (invoke "g" (i32.const 3000)) (i32.const 3100))