This is an archive of the discontinued LLVM Phabricator instance.

SymbolVendorELF: Perform build-id lookup even without a debug link
ClosedPublic

Authored by labath on Aug 1 2019, 1:42 AM.

Details

Summary

The debug link and build-id lookups are two independent ways one can
search for a separate symbol file. However, our implementation in
SymbolVendorELF was tying the two together and refusing to look up the
symbol file based on a build id if the file did not contain a debug
link.

This patch makes it possible to search for the symbol file with
just one of the two methods available. To demonstrate, I split the
build-id-case test into two, so that we test the search using both
methods.

Event Timeline

labath created this revision.Aug 1 2019, 1:42 AM
clayborg accepted this revision.Aug 1 2019, 2:31 PM
This revision is now accepted and ready to land.Aug 1 2019, 2:31 PM
jankratochvil requested changes to this revision.EditedAug 4 2019, 1:56 PM

I find it as a performance regression that even with an unstripped full debug info file (clang -g) LLDB has now started to look for the separate debug info file based on its build-id:

stat("/home/jkratoch/t/.build-id/f9/bf63a422425da856baacfadaa4a4afe43c991c.debug", 0x7ffe7f496470) = -1 ENOENT (No such file or directory)
stat("/home/jkratoch/t/.build-id/f9/bf63a422425da856baacfadaa4a4afe43c991c.debug", 0x7ffe7f4968d0) = -1 ENOENT (No such file or directory)
stat("/home/jkratoch/t/.build-id/f9/bf63a422425da856baacfadaa4a4afe43c991c.debug", 0x7ffe7f496690) = -1 ENOENT (No such file or directory)
stat("/usr/lib/debug/.build-id/f9/bf63a422425da856baacfadaa4a4afe43c991c.debug", 0x7ffe7f496470) = -1 ENOENT (No such file or directory)
stat("/usr/lib/debug/.build-id/f9/bf63a422425da856baacfadaa4a4afe43c991c.debug", 0x7ffe7f4968d0) = -1 ENOENT (No such file or directory)
stat("/usr/lib/debug/.build-id/f9/bf63a422425da856baacfadaa4a4afe43c991c.debug", 0x7ffe7f496690) = -1 ENOENT (No such file or directory)

Any needless file lookups are reported as a performance problem on NFS-mounted disks.
Existence of .gnu_debuglink is considered as a flag marking files which did have debug info which is now split out.
Maybe you could explain what is the real goal.

This revision now requires changes to proceed.Aug 4 2019, 1:56 PM

The "real" goal is being able to search for external debug files using the build-id without them having the gnu_debuglink thingy. :)

That would seem to at odds with your " .gnu_debuglink is considered as a flag" assertion, but I am not sure how you came to believe that. The gdb documentation for separate debug files https://sourceware.org/gdb/onlinedocs/gdb/Separate-Debug-Files.html stops short of saying that the build-id can be used without the debug link, but it does speak of them as two "methods" for finding external debug info, which at least to me, is a pretty clear indication that they can be used independently. And indeed, if you try to create such a situation, gdb will happily load the debug info from the separate file based on the build id alone:

/tmp/B $ readelf -S a.out | grep gnu # no .gnu_debuglink
  [ 2] .note.gnu.build-i NOTE             00000000000002c4  000002c4
  [ 4] .gnu.hash         GNU_HASH         0000000000000308  00000308
  [ 7] .gnu.version      VERSYM           0000000000000498  00000498
  [ 8] .gnu.version_r    VERNEED          00000000000004a8  000004a8
/tmp/B $ gdb
GNU gdb (Gentoo 8.3 vanilla) 8.3
...
(gdb) set debug-file-directory /tmp/B/D
(gdb) file a.out
Reading symbols from a.out...
Reading symbols from /tmp/B/D/.build-id/7e/d1cb4d4300b8ff67f2f2ef5b273666e339a8ba.debug... # <== symbols found here

That said, I agree the extra filesystem accesses are bad, and they are a completely unintended side-effect of this patch. If that is the only issue, then I think this can be easily fixed by first checking whether the main object file contains any debug info, and skipping the rest of the lookup in that case.

That would seem to at odds with your " .gnu_debuglink is considered as a flag" assertion, but I am not sure how you came to believe that.

I see I remembered it wrongly, already the [[ https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=77069918ac29da24f1ae0e388cef2ea5045397fd | initial build-id implementation for GDB ignored .gnu_debuglink ]].

That said, I agree the extra filesystem accesses are bad, and they are a completely unintended side-effect of this patch. If that is the only issue, then I think this can be easily fixed by first checking whether the main object file contains any debug info, and skipping the rest of the lookup in that case.

Yes, this is what GDB does:

  /* If the file has its own symbol tables it has no separate debug
     info.  `.dynsym'/`.symtab' go to MSYMBOLS, `.debug_info' goes to
     SYMTABS/PSYMTABS.  `.gnu_debuglink' may no longer be present with
     `.note.gnu.build-id'.
...
  else if (!objfile_has_partial_symbols (objfile)   
      std::string debugfile = find_separate_debug_file_by_buildid (objfile);
labath updated this revision to Diff 213298.Aug 5 2019, 2:47 AM
  • don't go off searching for separate files if the main one already contains debug info.
jankratochvil accepted this revision.Aug 5 2019, 3:04 AM
This revision is now accepted and ready to land.Aug 5 2019, 3:04 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 6 2019, 1:18 AM