Repository: davidlattimore/wild (rust)

Issue:
Tried to write dynamic symbol definition without a resolution
Error:
```
  Running command: "make libs" in dir: /home/mateusz/projects/rust-rdkafka/target/debug/build/rdkafka-sys-e3110b4b51e6f8c5/out
  Error: Failed copying from <epilogue> to output file

  Caused by:
      Tried to write dynamic symbol definition without a resolution: symbol `_ZTv0_n56_N7RdKafka10HandleImpl8outq_lenEv` (1257 local=239) in file #1536 (6/0) (ProducerImpl.o) defined as 566 in file #1024 (4/0) (HandleImpl.o) (ADDRESS)
  collect2: error: ld returned 255 exit status
```

Reproduction:
```
jj git clone https://github.com/fede1024/rust-rdkafka

cd rust-rdkafka

PATH=~/projects/wild:$PATH cargo b
```
It doesn't reproduce by changing `~/.cargo/config.toml` like in the README because `[target.<triple>]` doesn't apply when linking build scripts.

Pull request:
When building shared objects, don't export non-canonical definitions
Fixes #381

Patch:
diff --git a/libwild/src/layout.rs b/libwild/src/layout.rs
index 1585ca2..f1ac494 100644
--- a/libwild/src/layout.rs
+++ b/libwild/src/layout.rs
@@ -3643,15 +3643,24 @@ impl<'data> ObjectLayoutState<'data> {
         for (sym_index, sym) in self.object.symbols.enumerate() {
             if can_export_symbol(sym) {
                 let symbol_id = self.symbol_id_range().input_to_id(sym_index);
+
+                if !resources.symbol_db.is_canonical(symbol_id) {
+                    continue;
+                }
+
                 let value_flags = resources.symbol_db.local_symbol_value_flags(symbol_id);
+
                 if value_flags.contains(ValueFlags::DOWNGRADE_TO_LOCAL) {
                     continue;
                 }
+
                 let old_flags = resources.symbol_resolution_flags[symbol_id.as_usize()]
                     .fetch_or(ResolutionFlags::EXPORT_DYNAMIC);
+
                 if old_flags.is_empty() {
                     self.load_symbol::<A>(common, symbol_id, resources, queue)?;
                 }
+
                 if !old_flags.contains(ResolutionFlags::EXPORT_DYNAMIC) {
                     export_dynamic(common, symbol_id, resources)?;
                 }


Test patch:
diff --git a/wild/tests/integration_tests.rs b/wild/tests/integration_tests.rs
index 24f1400..2cf7104 100644
--- a/wild/tests/integration_tests.rs
+++ b/wild/tests/integration_tests.rs
@@ -87,7 +87,7 @@ impl Linker {
 
     fn link_shared(
         &self,
-        obj_path: &Path,
+        obj_paths: &[PathBuf],
         so_path: &Path,
         config: &Config,
         cross_arch: Option<Architecture>,
@@ -102,14 +102,17 @@ impl Linker {
 
         let mut command = LinkCommand::new(
             self,
-            &[LinkerInput::new(obj_path.to_owned())],
+            &obj_paths
+                .iter()
+                .map(|p| LinkerInput::new(p.clone()))
+                .collect_vec(),
             so_path,
             &linker_args,
             config,
             cross_arch,
         )?;
 
-        if self.is_wild() || !is_newer(so_path, obj_path) || !command.can_skip {
+        if self.is_wild() || !is_newer(so_path, obj_paths.iter()) || !command.can_skip {
             command.run(config)?;
             write_cmd_file(so_path, &command.to_string())?;
         }
@@ -291,7 +294,7 @@ impl Config {
 
 #[derive(Clone, PartialEq, Eq)]
 struct Dep {
-    filename: String,
+    filenames: Vec<String>,
     input_type: InputType,
 }
 
@@ -501,15 +504,15 @@ fn parse_configs(src_filename: &Path) -> Result<Vec<Config>> {
                         .map(|(a, b)| (a.to_owned(), b.to_owned()))?,
                 ),
                 "Object" => config.deps.push(Dep {
-                    filename: arg.to_owned(),
+                    filenames: arg.split(',').map(|s| s.to_owned()).collect(),
                     input_type: InputType::Object,
                 }),
                 "Archive" => config.deps.push(Dep {
-                    filename: arg.to_owned(),
+                    filenames: arg.split(',').map(|s| s.to_owned()).collect(),
                     input_type: InputType::Archive,
                 }),
                 "Shared" => config.deps.push(Dep {
-                    filename: arg.to_owned(),
+                    filenames: arg.split(',').map(|s| s.to_owned()).collect(),
                     input_type: InputType::SharedObject,
                 }),
                 "Compiler" => config.compiler = arg.trim().to_owned(),
@@ -562,7 +565,7 @@ impl ProgramInputs {
     ) -> Result<Program<'a>> {
         let primary = build_linker_input(
             &Dep {
-                filename: self.source_file.to_owned(),
+                filenames: vec![self.source_file.to_owned()],
                 input_type: InputType::Object,
             },
             config,
@@ -683,24 +686,44 @@ fn build_linker_input(
     linker: &Linker,
     cross_arch: Option<Architecture>,
 ) -> Result<LinkerInput> {
-    let src_path = src_path(&dep.filename);
-    if dep.filename.ends_with(".a") {
-        return Ok(LinkerInput::new(src_path));
+    if let [single_filename] = dep.filenames.as_slice() {
+        if single_filename.ends_with(".a") {
+            return Ok(LinkerInput::new(src_path(single_filename)));
+        }
     }
-    let obj_path = build_obj(dep, config, dep.input_type, cross_arch)?;
+
+    let obj_paths = dep
+        .filenames
+        .iter()
+        .map(|filename| build_obj(filename, config, dep.input_type, cross_arch))
+        .collect::<Result<Vec<PathBuf>>>()?;
+
+    // When building archives or shared objects, we use the name of the first object to determine
+    // the name.
+    let first_obj_path = obj_paths
+        .first()
+        .context("At least one object is required")?;
 
     match dep.input_type {
         InputType::Archive => {
-            let archive_path = obj_path.with_extension("a");
-            if !is_newer(&archive_path, &obj_path) {
-                make_archive(&archive_path, &obj_path)?;
+            let archive_path = first_obj_path.with_extension("a");
+            if !is_newer(&archive_path, obj_paths.iter()) {
+                make_archive(&archive_path, &obj_paths)?;
             }
             Ok(LinkerInput::new(archive_path))
         }
-        InputType::Object => Ok(LinkerInput::new(obj_path)),
+        InputType::Object => {
+            if obj_paths.len() > 1 {
+                bail!(
+                    "Multiple source files on a single line is only supported with Shared/Archive"
+                );
+            }
+
+            Ok(LinkerInput::new(first_obj_path.clone()))
+        }
         InputType::SharedObject => {
-            let so_path = obj_path.with_extension(format!("{linker}.so"));
-            let out = linker.link_shared(&obj_path, &so_path, config, cross_arch)?;
+            let so_path = first_obj_path.with_extension(format!("{linker}.so"));
+            let out = linker.link_shared(&obj_paths, &so_path, config, cross_arch)?;
             let assertions = Assertions::default();
             assertions
                 .check_path(&out.path, linker)
@@ -758,12 +781,12 @@ fn get_c_compiler(
 
 /// Builds some C source and returns the path to the object file.
 fn build_obj(
-    dep: &Dep,
+    filename: &str,
     config: &Config,
     input_type: InputType,
     cross_arch: Option<Architecture>,
 ) -> Result<PathBuf> {
-    let src_path = src_path(&dep.filename);
+    let src_path = src_path(filename);
     let extension = src_path
         .extension()
         .context("Missing extension")?
@@ -875,7 +898,7 @@ fn build_obj(
 
     let arch_str = cross_name(cross_arch);
 
-    let output_path = build_dir().join(Path::new(&dep.filename).with_extension(format!(
+    let output_path = build_dir().join(Path::new(filename).with_extension(format!(
         "{}-{arch_str}-{command_hash:x}{suffix}",
         config.name
     )));
@@ -894,7 +917,7 @@ fn build_obj(
     let mutex = mutex_for_path(&output_path);
     let _guard = mutex.lock().unwrap();
 
-    if is_newer(&output_path, &src_path) {
+    if is_newer(&output_path, std::iter::once(&src_path)) {
         return Ok(output_path);
     }
 
@@ -1002,19 +1025,22 @@ fn src_path(filename: &str) -> PathBuf {
     base_dir().join("tests").join("sources").join(filename)
 }
 
-/// Returns whether both `output_path` and `src_path` exist and `output_path` has a modification
-/// timestamp >= that of `src_path`.
-fn is_newer(output_path: &Path, src_path: &Path) -> bool {
+/// Returns whether both `output_path` all `src_paths` exist and `output_path` has a modification
+/// timestamp >= that of all elements of `src_paths`.
+fn is_newer<P: AsRef<Path>>(output_path: &Path, mut src_paths: impl Iterator<Item = P>) -> bool {
     let Ok(out) = std::fs::metadata(output_path) else {
         return false;
     };
-    let Ok(src) = std::fs::metadata(src_path) else {
-        return false;
-    };
-    let (Ok(mod_out), Ok(mod_src)) = (out.modified(), src.modified()) else {
+
+    let Ok(mod_out) = out.modified() else {
         return false;
     };
-    mod_out >= mod_src
+
+    src_paths.all(|src_path| {
+        std::fs::metadata(src_path.as_ref())
+            .and_then(|src| src.modified())
+            .is_ok_and(|mod_src| mod_out >= mod_src)
+    })
 }
 
 impl Linker {
@@ -1058,10 +1084,10 @@ impl Linker {
     }
 }
 
-fn make_archive(archive_path: &Path, path: &Path) -> Result {
+fn make_archive(archive_path: &Path, paths: &[PathBuf]) -> Result {
     let _ = std::fs::remove_file(archive_path);
     let mut cmd = Command::new("ar");
-    cmd.arg("cr").arg(archive_path).arg(path);
+    cmd.arg("cr").arg(archive_path).args(paths);
     let status = cmd.status()?;
     if !status.success() {
         bail!("Failed to create archive");
@@ -1191,9 +1217,7 @@ impl LinkCommand {
         // our output file, but not if we're linking with our linker, since we're always changing
         // that. We also require that the command we're going to run hasn't changed.
         let can_skip = !matches!(linker, Linker::Wild)
-            && inputs
-                .iter()
-                .all(|input| is_newer(output_path, &input.path))
+            && is_newer(output_path, inputs.iter().map(|i| i.path.as_path()))
             && cmd_file_is_current(output_path, &link_command.to_string());
         link_command.can_skip = can_skip;
 
diff --git a/wild/tests/sources/libc-integration-0.c b/wild/tests/sources/libc-integration-0.c
index 7c718d9..587bcdf 100644
--- a/wild/tests/sources/libc-integration-0.c
+++ b/wild/tests/sources/libc-integration-0.c
@@ -72,3 +72,8 @@ int sometimes_weak_fn(void) {
 int black_box(int v) {
     return v;
 }
+
+// This function is also defined in libc-integration-0b.c. The definition here should be used.
+int __attribute__ ((weak)) weak_fn3(void) {
+    return 15;
+}
diff --git a/wild/tests/sources/libc-integration-0b.c b/wild/tests/sources/libc-integration-0b.c
new file mode 100644
index 0000000..282f440
--- /dev/null
+++ b/wild/tests/sources/libc-integration-0b.c
@@ -0,0 +1,4 @@
+// This function is also defined in libc-integration-0.c. The other definition should be used.
+int __attribute__ ((weak)) weak_fn3(void) {
+    return 16;
+}
diff --git a/wild/tests/sources/libc-integration.c b/wild/tests/sources/libc-integration.c
index 7233701..88c45db 100644
--- a/wild/tests/sources/libc-integration.c
+++ b/wild/tests/sources/libc-integration.c
@@ -15,7 +15,7 @@
 //#Cross: false
 
 //#AbstractConfig:shared:default
-//#Shared:libc-integration-0.c
+//#Shared:libc-integration-0.c,libc-integration-0b.c
 //#Shared:libc-integration-1.c
 // Each binary links against shared objects created by that linker. So different names are expected.
 //#DiffIgnore:.dynamic.DT_NEEDED
@@ -92,6 +92,7 @@ void set_tvar2(int v);
 
 int __attribute__ ((weak)) weak_fn1(void);
 int __attribute__ ((weak)) weak_fn2(void);
+int __attribute__ ((weak)) weak_fn3(void);
 
 int __attribute__ ((weak)) sometimes_weak_fn(void) {
     return 7;
@@ -273,5 +274,9 @@ int main() {
     }
 #endif
 
+    if (weak_fn3() != 15) {
+        return 123;
+    }
+
     return 42;
 }