Basic comparison of DWARF CUs in linker-diff

[davidlattimore]

I'm not sure if performance is a concern here, but if it is, diffing without first converting to strings is likely to be faster. But probably it doesn't matter

[marxin]

Sure, that's a subtle change. However, once I wrote the first version of the verifies, I noticed we included debug info for all objects for all archives. That's something we shouldn't do and include just debug info if an object is loaded from an archive. David, could you let me know where to hook it in? A hint would be appreciated.

Then my general idea about the compilation unit was that other linkers might include more CUs as Wild does the garbage collection by default. And for these units, we ought to check size and the corresponding tags.

[davidlattimore]

Sure, that's a subtle change. However, once I wrote the first version of the verifies, I noticed we included debug info for all objects for all archives. That's something we shouldn't do and include just debug info if an object is loaded from an archive. David, could you let me know where to hook it in? A hint would be appreciated.

That's surprising. I wouldn't expect we'd be loading any sections from archive entries that we decide not to load. To help debug, here's how I'd expect it should be working...

process_object (in resolution.rs) should be called for each object that we decide to use. i.e. it'll be called for all non-archived objects and for any archived objects if they define symbols that undefined in objects we're already loading.

process_object then creates a ResolvedObject for that object. These then find their way into a ResolvedFile::Object. For archive entries that we're not loading, they'll end up as ResolvedFile::NotLoaded. Objects in the NotLoaded state should then be ignored for the rest of the processing, including in resolve_sections where debug sections get initially resolved.

Then my general idea about the compilation unit was that other linkers might include more CUs as Wild does the garbage collection by default. And for these units, we ought to check size and the corresponding tags.

I'm not 100% sure what you mean about checking size and corresponding tags.

[marxin]

process_object then creates a ResolvedObject for that object. These then find their way into a ResolvedFile::Object. For archive entries that we're not loading, they'll end up as ResolvedFile::NotLoaded. Objects in the NotLoaded state should then be ignored for the rest of the processing, including in resolve_sections where debug sections get initially resolved.

All right, so does it mean I should not have reached the following assert where ResolvedObject::new is called for the problematic archive member? Or does it happen later when a ResolvedFile::NotLoaded is created?

diff --git a/wild_lib/src/resolution.rs b/wild_lib/src/resolution.rs
index b3bfdd2..5e1223b 100644
--- a/wild_lib/src/resolution.rs
+++ b/wild_lib/src/resolution.rs
@@ -679,6 +679,11 @@ impl<'data> ResolvedObject<'data> {
         definitions_out: &mut [SymbolId],
         undefined_symbols_out: &SegQueue<UndefinedSymbol<'data>>,
     ) -> Result<Self> {
+        let objname = obj.input.to_string();
+        if objname.contains("@ symlink.lo") {
+            dbg!(&objname);
+            todo!("should not be created");
+        }
         let mut non_dynamic = None;

         if obj.is_dynamic() {

$ cargo t integration_test::program_name_27___rust_integration_rs__
...
running 1 test
[wild_lib/src/resolution.rs:684:13] &objname = "/home/marxin/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-musl/lib/self-contained/libc.a @ symlink.lo"
thread '<unnamed>' panicked at wild_lib/src/resolution.rs:685:13:
not yet implemented: should not be created

[marxin]

@davidlattimore Or if you have spare cycles, feel free to debug why this particular archive member is loaded. That would help me a lot :P

[davidlattimore]

Sure, I'll take a look...

[davidlattimore]

It looks like it's being loaded because libstd has a reference to the symbol symlink, which sounds right to me. Why were you thinking it shouldn't be loaded?

[davidlattimore]

Thinking about this some more, I'm guessing your answer will be that the other linkers aren't including debug info for symlink.lo, but we are. If that's the case, then one possibility is that loading of archive entries and loading of debug info from a file have two different criteria.

The criteria for loading an archive entry is that it defines a symbol that another loaded object references. This happens even if the code in question later gets GCed. i.e. it's a separate coarser-grained phase than the GC phase. When an archive entry gets loaded, that means that any sections that are marked as non-GC will be kept. This in particular applies to section like .init_array.

However, it's possible that other linkers are using a different rule to decide whether to load debug info from a file. e.g. perhaps if all sections of a file get GCed, or maybe just if all executable sections get GCed, then loading of the debug info is skipped.

[marxin]

All right, so it's a potential improvement we might make in the future. When it comes to a real-world use-cases, like Clang, we're doing pretty well when it comes to the size of debug info. I'm suggesting merging a first step where we compare the basics of the compilation units.