wit-component: use library order to determine duplicate symbol priori…

…ty (bytecodealliance#1258)

This addresses a couple of todo items in the shared-everything linking code in
`wit-component`.  Now, instead of erroring out on any duplicate symbols, we pick
the first library in the order they were specified.

Signed-off-by: Joel Dice <joel.dice@fermyon.com>

crates/wit-component/src/linking.rs

@@ -836,7 +836,8 @@ fn resolve_symbols<'a>(
                     // symbol, in which case we may redundantly insert the same value.
                     resolved.insert(*key, *exporter);
                 }
-                _ => {
+                [exporter, ..] => {
+                    resolved.insert(*key, *exporter);
                     duplicates.push((metadata.name, *key, value.as_slice()));
                 }
             }
@@ -879,14 +880,15 @@ fn resolve_symbols<'a>(
     for metadata in metadata {
         for name in &metadata.table_address_imports {
             if let Some((key, value)) = function_exporters.get(name) {
+                // Note that we do not use `insert_unique` here since multiple libraries may import the same
+                // symbol, in which case we may redundantly insert the same value.
                 match value.as_slice() {
                     [] => unreachable!(),
                     [exporter] => {
-                        // Note that we do not use `insert_unique` here since multiple libraries may import the
-                        // same symbol, in which case we may redundantly insert the same value.
                         resolved.insert(key, *exporter);
                     }
-                    _ => {
+                    [exporter, ..] => {
+                        resolved.insert(key, *exporter);
                         duplicates.push((metadata.name, *key, value.as_slice()));
                     }
                 }
@@ -1158,6 +1160,8 @@ fn find_reachable<'a>(
 #[derive(Default)]
 pub struct Linker {
     /// The `(name, module, dl_openable)` triple representing the libraries to be composed
+    ///
+    /// The order of this list determines priority in cases where more than one library exports the same symbol.
     libraries: Vec<(String, Vec<u8>, bool)>,
 
     /// The set of adapters to use when generating the component
@@ -1286,14 +1290,12 @@ impl Linker {
                 }),
             })
             .map(|exporters| {
-                // TODO: When there are multiple exporters, we should choose the one that came first in the order
-                // they were specified.
                 let first = *exporters.first().unwrap();
                 *exporters = vec![first];
                 first.0
             });
 
-        let (exporters, missing, duplicates) = resolve_symbols(&metadata, &exporters);
+        let (exporters, missing, _) = resolve_symbols(&metadata, &exporters);
 
         if !missing.is_empty() {
             if missing
@@ -1324,28 +1326,6 @@ impl Linker {
             }
         }
 
-        if !duplicates.is_empty() {
-            // TODO: When there are multiple exporters for a given symbol, we should choose the one that came first
-            // in the order they were specified.
-            bail!(
-                "duplicate symbol(s):\n{}",
-                duplicates
-                    .iter()
-                    .map(|(importer, key, exporters)| {
-                        format!(
-                            "\t{importer} needs {key}, provided by {}",
-                            exporters
-                                .iter()
-                                .map(|(exporter, _)| *exporter)
-                                .collect::<Vec<_>>()
-                                .join(", ")
-                        )
-                    })
-                    .collect::<Vec<_>>()
-                    .join("\n")
-            );
-        }
-
         let dependencies = find_dependencies(&metadata, &exporters)?;
 
         {

crates/wit-component/tests/components.rs

@@ -75,22 +75,28 @@ fn component_encoding_via_flags() -> Result<()> {
                 .encode()
         } else {
             let mut libs = glob::glob(path.join("lib-*.wat").to_str().unwrap())?
-                .map(|path| ("lib-", path, false))
+                .map(|path| Ok(("lib-", path?, false)))
                 .chain(
                     glob::glob(path.join("dlopen-lib-*.wat").to_str().unwrap())?
-                        .map(|path| ("dlopen-lib-", path, true)),
-                );
+                        .map(|path| Ok(("dlopen-lib-", path?, true))),
+                )
+                .collect::<Result<Vec<_>>>()?;
+
+            // Sort list to ensure deterministic order, which determines priority in cases of duplicate symbols:
+            libs.sort_by(|(_, a, _), (_, b, _)| a.cmp(b));
 
             let mut linker = Linker::default().validate(true);
 
             if path.join("stub-missing-functions").is_file() {
                 linker = linker.stub_missing_functions(true);
             }
 
-            let linker = libs.try_fold(linker, |linker, (prefix, path, dl_openable)| {
-                let (name, wasm) = read_name_and_module(prefix, &path?, &resolve, pkg)?;
-                Ok::<_, Error>(linker.library(&name, &wasm, dl_openable)?)
-            })?;
+            let linker =
+                libs.into_iter()
+                    .try_fold(linker, |linker, (prefix, path, dl_openable)| {
+                        let (name, wasm) = read_name_and_module(prefix, &path, &resolve, pkg)?;
+                        Ok::<_, Error>(linker.library(&name, &wasm, dl_openable)?)
+                    })?;
 
             adapters
                 .try_fold(linker, |linker, path| {

crates/wit-component/tests/components/link-duplicate-symbols/component.wat

@@ -0,0 +1,170 @@
+(component
+  (type (;0;)
+    (instance
+      (type (;0;) (func (param "v" s32) (result s32)))
+      (export (;0;) "foo" (func (type 0)))
+    )
+  )
+  (import (interface "test:test/test") (instance (;0;) (type 0)))
+  (core module (;0;)
+    (type (;0;) (func (param i32) (result i32)))
+    (func (;0;) (type 0) (param i32) (result i32)
+      local.get 0
+      i32.const 0
+      call_indirect (type 0)
+    )
+    (table (;0;) 1 funcref)
+    (memory (;0;) 17)
+    (global (;0;) (mut i32) i32.const 1048576)
+    (global (;1;) i32 i32.const 1048592)
+    (global (;2;) i32 i32.const 0)
+    (global (;3;) i32 i32.const 1048592)
+    (global (;4;) i32 i32.const 0)
+    (global (;5;) (mut i32) i32.const 1048592)
+    (global (;6;) (mut i32) i32.const 1114112)
+    (export "__stack_pointer" (global 0))
+    (export "bar:memory_base" (global 1))
+    (export "bar:table_base" (global 2))
+    (export "foo:memory_base" (global 3))
+    (export "foo:table_base" (global 4))
+    (export "__heap_base" (global 5))
+    (export "__heap_end" (global 6))
+    (export "foo" (func 0))
+    (export "__indirect_function_table" (table 0))
+    (export "memory" (memory 0))
+    (@producers
+      (processed-by "wit-component" "$CARGO_PKG_VERSION")
+    )
+  )
+  (core module (;1;)
+    (@dylink.0
+      (mem-info (memory 0 4))
+      (needed "foo")
+    )
+    (type (;0;) (func (param i32) (result i32)))
+    (import "env" "foo" (func $import_foo (;0;) (type 0)))
+    (func $bar (;1;) (type 0) (param i32) (result i32)
+      unreachable
+    )
+    (export "bar" (func $bar))
+    (export "foo" (func $bar))
+  )
+  (core module (;2;)
+    (@dylink.0
+      (mem-info (memory 0 4))
+      (needed "bar")
+    )
+    (type (;0;) (func (param i32) (result i32)))
+    (import "test:test/test" "foo" (func $import_foo (;0;) (type 0)))
+    (import "env" "foo" (func $import_foo2 (;1;) (type 0)))
+    (import "env" "bar" (func $import_bar (;2;) (type 0)))
+    (func $foo (;3;) (type 0) (param i32) (result i32)
+      unreachable
+    )
+    (export "test:test/test#foo" (func $foo))
+    (export "foo" (func $foo))
+  )
+  (core module (;3;)
+    (type (;0;) (func))
+    (type (;1;) (func (param i32)))
+    (type (;2;) (func (param i32) (result i32)))
+    (import "env" "memory" (memory (;0;) 0))
+    (import "env" "__indirect_function_table" (table (;0;) 0 funcref))
+    (import "bar" "foo" (func (;0;) (type 2)))
+    (func (;1;) (type 0))
+    (start 1)
+    (elem (;0;) (i32.const 0) func)
+    (elem (;1;) (i32.const 0) func 0)
+    (data (;0;) (i32.const 1048576) "\00\00\00\00\00\00\10\00")
+    (@producers
+      (processed-by "wit-component" "$CARGO_PKG_VERSION")
+    )
+  )
+  (core instance (;0;) (instantiate 0))
+  (alias core export 0 "memory" (core memory (;0;)))
+  (alias core export 0 "__heap_base" (core global (;0;)))
+  (alias core export 0 "__heap_end" (core global (;1;)))
+  (core instance (;1;)
+    (export "__heap_base" (global 0))
+    (export "__heap_end" (global 1))
+  )
+  (core instance (;2;))
+  (alias core export 0 "memory" (core memory (;1;)))
+  (alias core export 0 "__indirect_function_table" (core table (;0;)))
+  (alias core export 0 "__stack_pointer" (core global (;2;)))
+  (alias core export 0 "bar:memory_base" (core global (;3;)))
+  (alias core export 0 "bar:table_base" (core global (;4;)))
+  (alias core export 0 "foo" (core func (;0;)))
+  (core instance (;3;)
+    (export "memory" (memory 1))
+    (export "__indirect_function_table" (table 0))
+    (export "__stack_pointer" (global 2))
+    (export "__memory_base" (global 3))
+    (export "__table_base" (global 4))
+    (export "foo" (func 0))
+  )
+  (core instance (;4;) (instantiate 1
+      (with "GOT.mem" (instance 1))
+      (with "GOT.func" (instance 2))
+      (with "env" (instance 3))
+    )
+  )
+  (alias core export 0 "__heap_base" (core global (;5;)))
+  (alias core export 0 "__heap_end" (core global (;6;)))
+  (core instance (;5;)
+    (export "__heap_base" (global 5))
+    (export "__heap_end" (global 6))
+  )
+  (core instance (;6;))
+  (alias core export 0 "memory" (core memory (;2;)))
+  (alias core export 0 "__indirect_function_table" (core table (;1;)))
+  (alias core export 0 "__stack_pointer" (core global (;7;)))
+  (alias core export 0 "foo:memory_base" (core global (;8;)))
+  (alias core export 0 "foo:table_base" (core global (;9;)))
+  (alias core export 4 "bar" (core func (;1;)))
+  (alias core export 4 "foo" (core func (;2;)))
+  (core instance (;7;)
+    (export "memory" (memory 2))
+    (export "__indirect_function_table" (table 1))
+    (export "__stack_pointer" (global 7))
+    (export "__memory_base" (global 8))
+    (export "__table_base" (global 9))
+    (export "bar" (func 1))
+    (export "foo" (func 2))
+  )
+  (alias export 0 "foo" (func (;0;)))
+  (core func (;3;) (canon lower (func 0)))
+  (core instance (;8;)
+    (export "foo" (func 3))
+  )
+  (core instance (;9;) (instantiate 2
+      (with "GOT.mem" (instance 5))
+      (with "GOT.func" (instance 6))
+      (with "env" (instance 7))
+      (with "test:test/test" (instance 8))
+    )
+  )
+  (core instance (;10;) (instantiate 3
+      (with "env" (instance 0))
+      (with "bar" (instance 4))
+      (with "foo" (instance 9))
+    )
+  )
+  (type (;1;) (func (param "v" s32) (result s32)))
+  (alias core export 9 "test:test/test#foo" (core func (;4;)))
+  (func (;1;) (type 1) (canon lift (core func 4)))
+  (component (;0;)
+    (type (;0;) (func (param "v" s32) (result s32)))
+    (import "import-func-foo" (func (;0;) (type 0)))
+    (type (;1;) (func (param "v" s32) (result s32)))
+    (export (;1;) "foo" (func 0) (func (type 1)))
+  )
+  (instance (;1;) (instantiate 0
+      (with "import-func-foo" (func 1))
+    )
+  )
+  (export (;2;) (interface "test:test/test") (instance 1))
+  (@producers
+    (processed-by "wit-component" "$CARGO_PKG_VERSION")
+  )
+)

crates/wit-component/tests/components/link-duplicate-symbols/component.wit.print

@@ -0,0 +1,7 @@
+package root:component
+
+world root {
+  import test:test/test
+
+  export test:test/test
+}

src/bin/wasm-tools/component.rs

@@ -271,6 +271,9 @@ fn parse_library(s: &str) -> Result<(String, Vec<u8>)> {
 /// The resulting component's type will be the union of the types found in any `component-type*` custom sections in
 /// the input modules.
 ///
+/// Note that the order in which input libraries are listed determines priority in cases where more than one
+/// library exports the same symbol.
+///
 /// See
 /// https://github.com/WebAssembly/component-model/blob/main/design/mvp/examples/SharedEverythingDynamicLinking.md
 /// for further details.