Bug 20876

Summary: objdump is not aware about the build ID method to find detached debug info
Product: binutils Reporter: Matthias Klose <doko>
Component: binutilsAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: mark, nickc
Priority: P2    
Version: 2.28   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed:
Attachments: Proposed patch
Proposed patch
Proposed patch

Description Matthias Klose 2016-11-27 15:15:32 UTC
[forwarded from https://bugs.debian.org/838875]

find_separate_debug_file is not aware of finding the detached debuginfo using the Build-Id method.  It looks like when this was introduced, it was only done for gdb, not for bfd in opncls.c(find_separate_debug_file).

$ readelf -n /bin/ls | grep 'Build ID'
    Build ID: 6774d0a9b81709df5c4dec5df14749c7b645ff7a

$ strace objdump --disassemble -S /bin/ls 2>&1 | grep debug
open("/bin/74d0a9b81709df5c4dec5df14749c7b645ff7a.debug", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/bin/.debug/74d0a9b81709df5c4dec5df14749c7b645ff7a.debug", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/debug/bin/74d0a9b81709df5c4dec5df14749c7b645ff7a.debug", O_RDONLY) = -1 ENOENT (No such file or directory)

$ dpkg -S 4d0a9b81709df5c4dec5df14749c7b645ff7a
coreutils-dbgsym: /usr/lib/debug/.build-id/67/74d0a9b81709df5c4dec5df14749c7b645ff7a.debug

gdb documents this lookup as:

   * For the "build ID" method, GDB looks in the '.build-id'
     subdirectory of each one of the global debug directories for a file
     named 'NN/NNNNNNNN.debug', where NN are the first 2 hex characters
     of the build ID bit string, and NNNNNNNN are the rest of the bit
     string.  (Real build ID strings are 32 or more hex characters, not
     10.)
Comment 1 Nick Clifton 2017-01-10 17:21:15 UTC
Created attachment 9746 [details]
Proposed patch

Hi Matthias,

  Sorry for taking so long to respond to this PR.

  Please could you give the attached patch a try out and let me know what
  you think ?

  One thing that this patch does not do is confirm that the build-id in the
  separate debug info file actually matches the build-id in the original file.
  I do not know if this is an important omission however since the pathname
  to the separate debug info is based upon the build-id.

Cheers
  Nick
Comment 2 Matthias Klose 2017-01-11 00:00:56 UTC
this doesn't seem to work yet. I now get:

$ readelf -n /bin/ls | grep 'Build ID'
    Build ID: 1b1d2d0a5c825e50e07e80595427894ef0af116b

$ strace objdump --disassemble -S /bin/ls 2>&1 | grep debug
open("/bin/.build-id/1b/1d2d0a5c825e50e07e80595427894ef0af116b.debug", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/bin/.debug/.build-id/1b/1d2d0a5c825e50e07e80595427894ef0af116b.debug", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/debug/bin/.build-id/1b/1d2d0a5c825e50e07e80595427894ef0af116b.debug", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/bin/1d2d0a5c825e50e07e80595427894ef0af116b.debug", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/bin/.debug/1d2d0a5c825e50e07e80595427894ef0af116b.debug", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/debug/bin/1d2d0a5c825e50e07e80595427894ef0af116b.debug", O_RDONLY) = -1 ENOENT (No such file or directory)

the line
open("/usr/lib/debug/bin/.build-id/1b/1d2d0a5c825e50e07e80595427894ef0af116b.debug", O_RDONLY) = -1 ENOENT (No such file or directory)

looks fine, however the dirname needs to be stripped
Comment 3 Mark Wielaard 2017-01-11 13:44:18 UTC
Just a note that elfutils searches only the (global?) absolute debuginfo paths directly.

So given the default of "":.debug:/usr/lib/debug", it skips the "" relative cwd
and the ".debug" postfix, but only tries /usr/lib/debug/.build-id/xx/yyyy.debug

So the search strategy of elfutils for a separate .debug file given a build-id xxyyyy and a list of debuginfo search paths is to take the absolute paths (starting with '/') and trying /path/.build-id/xx/yyyy.debug. Where the default list of (absolute) debuginfo search paths is just "/usr/lib/debug". It then takes the canonicalize_file_name assuming it might be a symlink and the user might find the resolved name more interesting. As a sanity check elfutils also only allows build-ids that are at a minimum 4 and at a maximum 64 hex characters long. It does normally also verify that the found file does contain the build-id it should and reject the file if it doesn't.
Comment 4 Nick Clifton 2017-01-11 17:08:55 UTC
Created attachment 9748 [details]
Proposed patch

Hi Matthias,

  OK, here is a revised patch.  Please let me know what you think.

  I have not added build-id verification as I am lazy, but that can be done
  if you are happy with the rest of the patch.

  More controversially however, I have not restricted the path generation to
  absolute paths.  The reason is basically testing.  If the build-id based
  debug info files *have* to be installed into admin controlled parts of the
  file system then it is not going to be possible to test the feature as an
  ordinary user.  (See the addition to objdump.exp in the updated patch for
  an example of how I envisage testing the feature currently).

  Is it such a bad thing to be able to load locally stored build-id based
  debug info files ?  It makes testing possible, and I imagine that it
  would be quite useful in development too.  If the user wants to debug a
  built, but not installed, application (which uses build-id based separate
  debug files), then all that they need to do is to make sure that the files
  are in the correct .build-id/NN directories and away they go.

Cheers
  Nick
Comment 5 Matthias Klose 2017-01-11 19:38:08 UTC
no, still doesn't strip the dirname, and one location is search twice.

open("/usr/lib/debug/bin/.build-id/1b/1d2d0a5c825e50e07e80595427894ef0af116b.debug", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/debug/usr/bin/.build-id/1b/1d2d0a5c825e50e07e80595427894ef0af116b.debug", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/debug/bin/.build-id/1b/1d2d0a5c825e50e07e80595427894ef0af116b.debug", O_RDONLY) = -1 ENOENT (No such file or directory)
Comment 6 Matthias Klose 2017-01-11 20:44:30 UTC
applying this on top of your patch works for me. The global debug directory doesn't have the dirname of the original file, so don't append `canon_dir'.  Not sure if that needs to be stripped from the tests with EXTRA_DEBUG_ROOT1 and EXTRA_DEBUG_ROOT2 prefixes as well?

--- bfd/opncls.c~	2017-01-11 20:29:07.061189932 +0100
+++ bfd/opncls.c	2017-01-11 21:39:51.211288793 +0100
@@ -1481,11 +1481,8 @@
   /* Then try in the global debugfile directory.  */
   strcpy (debugfile, debug_file_directory);
   dirlen = strlen (debug_file_directory) - 1;
-  if (dirlen > 0
-      && debug_file_directory[dirlen] != '/'
-      && canon_dir[0] != '/')
+  if (dirlen > 0 && debug_file_directory[dirlen] != '/')
     strcat (debugfile, "/");
-  strcat (debugfile, canon_dir);
   strcat (debugfile, base);
 
   if (check_func (debugfile, crc32))
Comment 7 Nick Clifton 2017-01-12 15:56:57 UTC
Created attachment 9749 [details]
Proposed patch

Hi Matthias,

  OK - I have applied your patch on top of mine, made the same change to the EXTRA_DEBUG_ROOT checks, and also added verification of the build-id in the located file.  Are you happy with this version ?

Cheers
  Nick
Comment 8 Matthias Klose 2017-01-12 15:58:58 UTC
yes, I am, thanks for working on this.  Could you apply it to the 2.28 branch as well?
Comment 9 cvs-commit@gcc.gnu.org 2017-01-12 16:58:49 UTC
The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=2425a30e406a0523020b7e70abb864a06a45bb97

commit 2425a30e406a0523020b7e70abb864a06a45bb97
Author: Nick Clifton <nickc@redhat.com>
Date:   Thu Jan 12 16:56:54 2017 +0000

    Add support for locating separate debug info files via the build-id method.
    
    	PR binutils/20876
    bfd	* opncls.c (find_separate_debug_file): Add include_dirs
    	parameter.  Only include the directory part of the bfd's filename
    	in search paths if include_dirs is true.  Add a couple of extra
    	locations for looking for debug files.
    	( bfd_follow_gnu_debuglink): Update invocation of
    	find_separate_debug_file.
    	(bfd_follow_gnu_debugaltlink): Likewise.
    	(get_build_id): New function: Finds the build-id of the given bfd.
    	(get_build_id_name): New function: Computes the name of the
    	separate debug info file for a bfd, based upon its build-id.
    	(check_build_id_file): New function: Checks to see if a separate
    	debug info file exists at the given location, and that its
    	build-id matches that of the original bfd.
    	(bfd_follow_build_id_debuglink): New function: Finds a separate
    	debug info file for a given bfd by using the build-id method.
    	* dwarf2.c (_bfd_dwarf2_slurp_debug_info): Try using the build-id
    	method of locating a separate debug info file before using the
    	debuglink method.
    	* bfd-in2.h: Regenerate.
    
    binutils * NEWS: Mention the new feature.
    	* testsuite/binutils-all/objdump.exp (test_build_id_debuglink):
    	New proc to test the location of separate debug info files using
    	the build-id method.
Comment 10 Nick Clifton 2017-01-12 17:03:55 UTC
I have applied the patch to the mainline.

I have also asked Tristan if it will be OK to apply the patch to the 2.28 branch.
Comment 11 cvs-commit@gcc.gnu.org 2017-01-13 09:34:06 UTC
The binutils-2_28-branch branch has been updated by Nick Clifton <nickc@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=695f5e30a3668bb65141b13351f8e3d9da2b841b

commit 695f5e30a3668bb65141b13351f8e3d9da2b841b
Author: Nick Clifton <nickc@redhat.com>
Date:   Fri Jan 13 09:32:22 2017 +0000

    Add support for finding separate debug info files via the build-id method.
    
    	PR binutils/20876
    bfd	* opncls.c (find_separate_debug_file): Add include_dirs
    	parameter.  Only include the directory part of the bfd's filename
    	in search paths if include_dirs is true.  Add a couple of extra
    	locations for looking for debug files.
    	( bfd_follow_gnu_debuglink): Update invocation of
    	find_separate_debug_file.
    	(bfd_follow_gnu_debugaltlink): Likewise.
    	(get_build_id): New function: Finds the build-id of the given bfd.
    	(get_build_id_name): New function: Computes the name of the
    	separate debug info file for a bfd, based upon its build-id.
    	(check_build_id_file): New function: Checks to see if a separate
    	debug info file exists at the given location, and that its
    	build-id matches that of the original bfd.
    	(bfd_follow_build_id_debuglink): New function: Finds a separate
    	debug info file for a given bfd by using the build-id method.
    	* dwarf2.c (_bfd_dwarf2_slurp_debug_info): Try using the build-id
    	method of locating a separate debug info file before using the
    	debuglink method.
    	* bfd-in2.h: Regenerate.
    
    binutils* NEWS: Mention the new feature.
    	* testsuite/binutils-all/objdump.exp (test_build_id_debuglink):
    	New proc to test the location of separate debug info files using
    	the build-id method.
Comment 12 Nick Clifton 2017-01-13 09:35:34 UTC
Patch applied to the 2.28 branch as well.