Repository: davidlattimore/wild (rust)

Issue:
Rust test `run-make/return-non-c-like-enum` fails
Running `PATH=~/Projects/wild:$PATH ./x.py t tests/run-make/return-non-c-like-enum/` fails with:
```
Error: Failed copying from <epilogue> to output file

Caused by:
    Internal error: Unexpected dynamic symbol definition from <not loaded>. symbol `__letf2` (15116 local=4) in file #21251 (83/3) (libnonclike.a @ compiler_builtins-ff3289faa9344fd9.compiler_builtins.b66d79b79bdb8f0f-cgu.028.rcgu.o) (ADDRESS | CAN_BYPASS_GOT)

Stack backtrace:
   0: anyhow::error::<impl anyhow::Error>::msg
   1: libwild::elf_writer::write_dynamic_symbol_definitions
   2: libwild::elf_writer::<impl libwild::layout::FileLayout>::write
...
```
I have no clue why, but readelf, objdump and nm don't report this symbol, unless the underlying object is extracted from the archive:
```
❯ readelf -Ws ./libnonclike.a | rg left2 || echo missing
missing

❯ ar x libnonclike.a compiler_builtins-ff3289faa9344fd9.compiler_builtins.b66d79b79bdb8f0f-cgu.028.rcgu.o

❯ readelf -Ws ./compiler_builtins-ff3289faa9344fd9.compiler_builtins.b66d79b79bdb8f0f-cgu.028.rcgu.o

Symbol table '.symtab' contains 5 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
     1: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS compiler_builtins.b66d79b79bdb8f0f-cgu.028
     2: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT    5 .LCPI0_0
     3: 0000000000000000     0 SECTION LOCAL  DEFAULT    6 .text.__letf2
     4: 0000000000000000   177 FUNC    WEAK   HIDDEN     6 __letf2
```

Note: building Rust with wild requires not disabling `download-ci-llvm` before version script support is extended,
So, I'm also attaching the reproducer extracted with `WILD_SAVE_DIR`: 

[rust-fail.tar.gz](https://github.com/user-attachments/files/19031060/rust-fail.tar.gz)

Pull request description:
Don't attempt to export dynamic symbols for discarded archive entries
At the place where we were requesting export of the symbol, we didn't have easy access to whether the object defining the symbol was discarded, so the code is changed to send an ExportDynamic request to the defining object. It can then ignore the request if it wasn't loaded.

Fixes #498

Patch:
diff --git a/libwild/src/layout.rs b/libwild/src/layout.rs
index 3e9267d..031c219 100644
--- a/libwild/src/layout.rs
+++ b/libwild/src/layout.rs
@@ -1298,6 +1298,10 @@ enum WorkItem {
 
     /// A request to load a particular section.
     LoadSection(SectionLoadRequest),
+
+    /// Requests that the specified symbol be exported as a dynamic symbol. Will be ignored if the
+    /// object that defines the symbol is not loaded or is itself a shared object.
+    ExportDynamic(SymbolId),
 }
 
 #[derive(Copy, Clone, Debug)]
@@ -1316,6 +1320,7 @@ impl WorkItem {
                 symbol_db.file_id_for_symbol(s)
             }
             WorkItem::LoadSection(s) => s.file_id,
+            WorkItem::ExportDynamic(symbol_id) => symbol_db.file_id_for_symbol(symbol_id),
         }
     }
 }
@@ -2230,6 +2235,16 @@ impl<'data> FileLayoutState<'data> {
                     ),
                 _ => bail!("Request to load section from non-object: {self}"),
             },
+            WorkItem::ExportDynamic(symbol_id) => match self {
+                FileLayoutState::Object(object) => {
+                    object.export_dynamic::<S, A>(common, symbol_id, resources, queue)
+                }
+                _ => {
+                    // Non-loaded and dynamic objects don't do anything in response to a request to
+                    // export a dynamic symbol.
+                    Ok(())
+                }
+            },
         }
     }
 
@@ -3739,6 +3754,27 @@ impl<'data> ObjectLayoutState<'data> {
         }
         Ok(())
     }
+
+    fn export_dynamic<'scope, S: StorageModel, A: Arch>(
+        &mut self,
+        common: &mut CommonGroupState<'data>,
+        symbol_id: SymbolId,
+        resources: &GraphResources<'data, 'scope, S>,
+        queue: &mut LocalWorkQueue,
+    ) -> Result {
+        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)?;
+        }
+
+        Ok(())
+    }
 }
 
 pub(crate) struct SymbolCopyInfo<'data> {
@@ -4255,44 +4291,42 @@ impl<'data> DynamicLayoutState<'data> {
     ) -> Result {
         let dt_info = DynamicTagValues::read(self.object)?;
         self.symbol_versions_needed = vec![false; dt_info.verdefnum as usize];
+
         if let Some(soname) = dt_info.soname {
             self.lib_name = soname;
         }
+
         common.allocate(
             part_id::DYNAMIC,
             size_of::<crate::elf::DynamicEntry>() as u64,
         );
+
         common.allocate(part_id::DYNSTR, self.lib_name.len() as u64 + 1);
-        self.request_all_undefined_symbols(common, resources, queue)
+
+        self.request_all_undefined_symbols(resources, queue);
+
+        Ok(())
     }
 
     fn request_all_undefined_symbols<S: StorageModel>(
         &self,
-        common: &mut CommonGroupState<'data>,
         resources: &GraphResources<'data, '_, S>,
         queue: &mut LocalWorkQueue,
-    ) -> Result {
+    ) {
         for symbol_id in self.symbol_id_range() {
             if resources.symbol_db.is_canonical(symbol_id) {
                 continue;
             }
+
             let definition_symbol_id = resources.symbol_db.definition(symbol_id);
             let file_id = resources.symbol_db.file_id_for_symbol(definition_symbol_id);
-            // If a shared object references a symbol from say another shared object, there's
-            // nothing we need to do.
-            if !resources.symbol_db.file(file_id).is_regular_object() {
-                continue;
-            }
-            let old_flags = resources.symbol_resolution_flags[definition_symbol_id.as_usize()]
-                .fetch_or(ResolutionFlags::EXPORT_DYNAMIC);
-            if old_flags.is_empty() {
-                queue.send_symbol_request(definition_symbol_id, resources);
-            }
-            if !old_flags.contains(ResolutionFlags::EXPORT_DYNAMIC) {
-                export_dynamic(common, definition_symbol_id, resources)?;
-            }
+
+            queue.send_work(
+                resources,
+                file_id,
+                WorkItem::ExportDynamic(definition_symbol_id),
+            );
         }
-        Ok(())
     }
 
     fn finalise_sizes(&mut self, common: &mut CommonGroupState<'data>) -> Result {
diff --git a/libwild/src/parsing.rs b/libwild/src/parsing.rs
index 1d2115f..8e0c934 100644
--- a/libwild/src/parsing.rs
+++ b/libwild/src/parsing.rs
@@ -204,14 +204,6 @@ impl<'data> ParsedInput<'data> {
         }
     }
 
-    pub(crate) fn is_regular_object(&self) -> bool {
-        match self {
-            ParsedInput::Object(o) => !o.is_dynamic(),
-            ParsedInput::Prelude(_) => false,
-            ParsedInput::Epilogue(_) => false,
-        }
-    }
-
     pub(crate) fn file_id(&self) -> FileId {
         let file_id = match self {
             ParsedInput::Prelude(_) => PRELUDE_FILE_ID,


Test patch:
diff --git a/wild/tests/integration_tests.rs b/wild/tests/integration_tests.rs
index b7f216d..791de0b 100644
--- a/wild/tests/integration_tests.rs
+++ b/wild/tests/integration_tests.rs
@@ -1825,7 +1825,8 @@ fn integration_test(
         "ifunc2.c",
         "tls-local-exec.c",
         "undefined_symbols.c",
-        "whole_archive.c"
+        "whole_archive.c",
+        "shared.c"
     )]
     program_name: &'static str,
     #[allow(unused_variables)] setup_symlink: (),
diff --git a/wild/tests/sources/shared-a1.c b/wild/tests/sources/shared-a1.c
new file mode 100644
index 0000000..12371c6
--- /dev/null
+++ b/wild/tests/sources/shared-a1.c
@@ -0,0 +1,3 @@
+int bar1(void) {
+    return 40;
+}
diff --git a/wild/tests/sources/shared-a2.c b/wild/tests/sources/shared-a2.c
new file mode 100644
index 0000000..12fd4ad
--- /dev/null
+++ b/wild/tests/sources/shared-a2.c
@@ -0,0 +1,3 @@
+int baz(void) {
+    return 10;
+}
diff --git a/wild/tests/sources/shared-s1.c b/wild/tests/sources/shared-s1.c
new file mode 100644
index 0000000..5cf55d7
--- /dev/null
+++ b/wild/tests/sources/shared-s1.c
@@ -0,0 +1,9 @@
+int baz(void);
+
+int call_baz(void) {
+    return baz();
+}
+
+int bar2(void) {
+    return 2;
+}
diff --git a/wild/tests/sources/shared.c b/wild/tests/sources/shared.c
new file mode 100644
index 0000000..70f4f2c
--- /dev/null
+++ b/wild/tests/sources/shared.c
@@ -0,0 +1,21 @@
+// We don't currently run this, we just make sure that we can produce a shared object and that it
+// passes the diff test.
+//
+// One notable scenario that this test tests is having a non-weak undefined symbol (baz) in a shared
+// object and having that symbol be defined by an archive entry that we don't load.
+
+//#RunEnabled:false
+//#LinkArgs:-shared -z now
+//#Static:false
+//#Archive:shared-a1.c,shared-a2.c
+//#Shared:shared-s1.c
+//#DiffIgnore:.dynamic.DT_RELA
+//#DiffIgnore:.dynamic.DT_RELAENT
+//#DiffIgnore:.dynamic.DT_NEEDED
+
+int bar1(void);
+int bar2(void);
+
+int foo(void) {
+    return bar1() + bar2();
+}