[PATCH] Search for DWZ files in debug-file-directories as well

[PATCH] Search for DWZ files in debug-file-directories as well

[Sergio Durigan Junior]

When Debian (and Ubuntu) builds its binaries, it (still) doesn't use
dwz's "--relative" option.  This causes their debuginfo files to
carry a .gnu_debugaltlink section containing a full pathname to the
DWZ alt debug file, like this:

  $ readelf -wk /usr/bin/cat
  Contents of the .gnu_debugaltlink section:

    Separate debug info file: /usr/lib/debug/.dwz/x86_64-linux-gnu/coreutils.debug
    Build-ID (0x14 bytes):
   ee 76 5d 71 97 37 ce 46 99 44 32 bb e8 a9 1a ef 99 96 88 db

  Contents of the .gnu_debuglink section:

    Separate debug info file: 06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
    CRC value: 0x53267655

This usually works OK, because most of the debuginfo files installed
via apt will be present in /usr/lib/debug anyway.  However, imagine
the following scenario:

- You are using /usr/bin/cat, it crashes on you and generates a
  corefile.

- You don't want/need to "apt install" the debuginfo file for
  coreutils from the repositories.  Instead, you already have the
  debuginfo files in a separate directory (e.g., $HOME/dbgsym).

- You start GDB and "set debug-file-directory $HOME/dbgsym".
  You then get the following message:

  $ gdb -ex 'set debug-file-directory ./dbgsym/usr/lib/debug' -ex 'file /bin/cat' -ex 'core-file ./cat.core'
  GNU gdb (Ubuntu 10.1-0ubuntu1) 10.1
  ...
  Reading symbols from /bin/cat...
  Reading symbols from /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug...
  could not find '.gnu_debugaltlink' file for /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug

This error happens because GDB is trying to locate the build-id
link (inside /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id) for the
DWZ alt debug file, which doesn't exist.  Arguably, this is a problem
with how dh_dwz works in Debian, and it's something I'm also planning
to tackle.  But, back at the problem at hand.

Besides not being able to find the build-id link in the directory
mentioned above, GDB also tried to open the DWZ alt file using its
filename.  The problem here is that, since we don't have the distro's
debuginfo installed, it can't find anything under /usr/lib/debug that
satisfies it.

It occurred to me that a good way to workaround this problem is to
actually try to locate the DWZ alt debug file inside the
debug-file-directories (that were likely provided by the user).  So
this is what the proposed patch does.

The idea here is simple: get the filename extracted from the
.gnu_debugaltlink section, and manipulate it in order to replace the
initial part of the path (everything before "/.dwz/") by whatever
debug-file-directories the user might have provided.

I talked with Mark Wielaard and he agrees this is a sensible approach.
In fact, apparently this is something that eu-readelf also does.

I regtested this code, and no regressions were found.

2020-11-14  Sergio Durigan Junior  <sergiodj@sergiodj.net>

	* dwarf2/read.c (dwarf2_get_dwz_file): Convert 'filename' to a
	std::string.  Implement ability to search for DWZ files in the
	debug-file-directories provided by the user as well.
---
 gdb/dwarf2/read.c | 65 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 59 insertions(+), 6 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 1b43fc8cdc..300599aae3 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2190,7 +2190,7 @@ locate_dwz_sections (bfd *abfd, asection *sectp, dwz_file *dwz_file)
 struct dwz_file *
 dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
 {
-  const char *filename;
+  std::string filename;
   bfd_size_type buildid_len_arg;
   size_t buildid_len;
   bfd_byte *buildid;
@@ -2216,19 +2216,17 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
 
   filename = data.get ();
 
-  std::string abs_storage;
-  if (!IS_ABSOLUTE_PATH (filename))
+  if (!IS_ABSOLUTE_PATH (filename.c_str ()))
     {
       gdb::unique_xmalloc_ptr<char> abs
 	= gdb_realpath (bfd_get_filename (per_bfd->obfd));
 
-      abs_storage = ldirname (abs.get ()) + SLASH_STRING + filename;
-      filename = abs_storage.c_str ();
+      filename = ldirname (abs.get ()) + SLASH_STRING + filename;
     }
 
   /* First try the file name given in the section.  If that doesn't
      work, try to use the build-id instead.  */
-  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename, gnutarget));
+  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename.c_str (), gnutarget));
   if (dwz_bfd != NULL)
     {
       if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
@@ -2238,6 +2236,61 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
   if (dwz_bfd == NULL)
     dwz_bfd = build_id_to_debug_bfd (buildid_len, buildid);
 
+  if (dwz_bfd == nullptr)
+    {
+      /* If the user has provided us with different
+	 debug-file-directories, we can try them in order.  */
+      size_t dwz_pos = filename.find ("/.dwz/");
+
+      if (dwz_pos != std::string::npos)
+	{
+	  std::string tmpfilename = filename.erase (0, dwz_pos);
+	  std::vector<gdb::unique_xmalloc_ptr<char>> debugdir_vec
+	    = dirnames_to_char_ptr_vec (debug_file_directory);
+
+	  for (const gdb::unique_xmalloc_ptr<char> &debugdir : debugdir_vec)
+	    {
+	      /* The idea is to iterate over the
+		 debug-file-directories provided by the user and
+		 replace the hard-coded path in the "filename" by each
+		 debug-file-directory.
+
+		 For example, suppose that filename is:
+
+		   /usr/lib/debug/.dwz/foo.debug
+
+		 And suppose that we have "$HOME/bar" as the
+		 debug-file-directory.  We would then adjust filename
+		 to look like:
+
+		   $HOME/bar/.dwz/foo.debug
+
+		 which would hopefully allow us to find the alt debug
+		 file.  */
+	      std::string ddir = debugdir.get ();
+
+	      if (filename.size () > ddir.size ()
+		  && filename.compare (0, ddir.size (), ddir) == 0)
+		continue;
+
+	      std::string new_filename = tmpfilename.insert (0, ddir);
+
+	      dwz_bfd = gdb_bfd_open (new_filename.c_str (), gnutarget);
+
+	      if (dwz_bfd != nullptr)
+		{
+		  if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
+		    {
+		      dwz_bfd.reset (nullptr);
+		      continue;
+		    }
+		  /* Found it.  */
+		  break;
+		}
+	    }
+	}
+    }
+
   if (dwz_bfd == nullptr)
     {
       gdb::unique_xmalloc_ptr<char> alt_filename;
-- 
2.28.0

Re: [PATCH] Search for DWZ files in debug-file-directories as well

[Mark Wielaard]

Hi Sergio,

On Sat, 2020-11-14 at 18:48 -0500, Sergio Durigan Junior wrote:
> Separate debug info file: /usr/lib/debug/.dwz/x86_64-linux-gnu/coreutils.debug
> [...]
> It occurred to me that a good way to workaround this problem is to
> actually try to locate the DWZ alt debug file inside the
> debug-file-directories (that were likely provided by the user).  So
> this is what the proposed patch does.
> 
> The idea here is simple: get the filename extracted from the
> .gnu_debugaltlink section, and manipulate it in order to replace the
> initial part of the path (everything before "/.dwz/") by whatever
> debug-file-directories the user might have provided.
> 
> I talked with Mark Wielaard and he agrees this is a sensible
> approach.
> In fact, apparently this is something that eu-readelf also does.

I double checked what elfutils libdw (the dwfl interface) does for alt
files. In general it tries to find them similar to how .debug files are
found described in libdwfl.h under *** Standard callbacks ***:
https://sourceware.org/git/?p=elfutils.git;a=blob;f=libdwfl/libdwfl.h;hb=HEAD#l242
Then for alt files it also looks for a .dwz subdir to find the basename
of the alt file. Which is close to what you are proposing. But your
version would also find alt files in (arch) subdirs of the .dwz dir.
Which seems to be an extension of Debian/Ubuntu. In Fedora/CentOS the
arch is part of the basename (e.g. the elfutils supplemental file is
called elfutils-0.179-1.fc32.i386 or elfutils-0.182-1.fc33.x86_64). It
might make sense to match what you are doing if the alt file path
contains /.dwz/ and has any directories after it (presumably
representing the arch).

Cheers,

Mark

Re: [PATCH] Search for DWZ files in debug-file-directories as well

[Simon Marchi]

On 2020-11-14 6:48 p.m., Sergio Durigan Junior via Gdb-patches wrote:
> When Debian (and Ubuntu) builds its binaries, it (still) doesn't use
> dwz's "--relative" option.  This causes their debuginfo files to
> carry a .gnu_debugaltlink section containing a full pathname to the
> DWZ alt debug file, like this:
>
>   $ readelf -wk /usr/bin/cat
>   Contents of the .gnu_debugaltlink section:
>
>     Separate debug info file: /usr/lib/debug/.dwz/x86_64-linux-gnu/coreutils.debug
>     Build-ID (0x14 bytes):
>    ee 76 5d 71 97 37 ce 46 99 44 32 bb e8 a9 1a ef 99 96 88 db
>
>   Contents of the .gnu_debuglink section:
>
>     Separate debug info file: 06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
>     CRC value: 0x53267655
>
> This usually works OK, because most of the debuginfo files installed
> via apt will be present in /usr/lib/debug anyway.  However, imagine
> the following scenario:
>
> - You are using /usr/bin/cat, it crashes on you and generates a
>   corefile.
>
> - You don't want/need to "apt install" the debuginfo file for
>   coreutils from the repositories.  Instead, you already have the
>   debuginfo files in a separate directory (e.g., $HOME/dbgsym).
>
> - You start GDB and "set debug-file-directory $HOME/dbgsym".
>   You then get the following message:
>
>   $ gdb -ex 'set debug-file-directory ./dbgsym/usr/lib/debug' -ex 'file /bin/cat' -ex 'core-file ./cat.core'
>   GNU gdb (Ubuntu 10.1-0ubuntu1) 10.1
>   ...
>   Reading symbols from /bin/cat...
>   Reading symbols from /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug...
>   could not find '.gnu_debugaltlink' file for /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
>
> This error happens because GDB is trying to locate the build-id
> link (inside /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id) for the
> DWZ alt debug file, which doesn't exist.  Arguably, this is a problem
> with how dh_dwz works in Debian, and it's something I'm also planning
> to tackle.  But, back at the problem at hand.
>
> Besides not being able to find the build-id link in the directory
> mentioned above, GDB also tried to open the DWZ alt file using its
> filename.  The problem here is that, since we don't have the distro's
> debuginfo installed, it can't find anything under /usr/lib/debug that
> satisfies it.
>
> It occurred to me that a good way to workaround this problem is to
> actually try to locate the DWZ alt debug file inside the
> debug-file-directories (that were likely provided by the user).  So
> this is what the proposed patch does.
>
> The idea here is simple: get the filename extracted from the
> .gnu_debugaltlink section, and manipulate it in order to replace the
> initial part of the path (everything before "/.dwz/") by whatever
> debug-file-directories the user might have provided.
>
> I talked with Mark Wielaard and he agrees this is a sensible approach.
> In fact, apparently this is something that eu-readelf also does.
>
> I regtested this code, and no regressions were found.

Hi Sergio,

I don't really have an opinion on this at the moment because I don't
know much about how dwz files are used in practice.  Is having a ".dwz"
somewhat standard?

> @@ -2238,6 +2236,61 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>    if (dwz_bfd == NULL)
>      dwz_bfd = build_id_to_debug_bfd (buildid_len, buildid);
>
> +  if (dwz_bfd == nullptr)
> +    {
> +      /* If the user has provided us with different
> +	 debug-file-directories, we can try them in order.  */
> +      size_t dwz_pos = filename.find ("/.dwz/");
> +
> +      if (dwz_pos != std::string::npos)
> +	{
> +	  std::string tmpfilename = filename.erase (0, dwz_pos);

"filename.erase", I think that modifies filename in place.  If so, is
this what you intended?

> +	  std::vector<gdb::unique_xmalloc_ptr<char>> debugdir_vec
> +	    = dirnames_to_char_ptr_vec (debug_file_directory);
> +
> +	  for (const gdb::unique_xmalloc_ptr<char> &debugdir : debugdir_vec)
> +	    {
> +	      /* The idea is to iterate over the
> +		 debug-file-directories provided by the user and
> +		 replace the hard-coded path in the "filename" by each
> +		 debug-file-directory.
> +
> +		 For example, suppose that filename is:
> +
> +		   /usr/lib/debug/.dwz/foo.debug
> +
> +		 And suppose that we have "$HOME/bar" as the
> +		 debug-file-directory.  We would then adjust filename
> +		 to look like:
> +
> +		   $HOME/bar/.dwz/foo.debug
> +
> +		 which would hopefully allow us to find the alt debug
> +		 file.  */
> +	      std::string ddir = debugdir.get ();
> +
> +	      if (filename.size () > ddir.size ()
> +		  && filename.compare (0, ddir.size (), ddir) == 0)
> +		continue;

What's the intent of this condition?  Can you add a comment to make it
explicit?

> +
> +	      std::string new_filename = tmpfilename.insert (0, ddir);

Same question as above for "tmpfilename.insert".

Simon

Re: [PATCH] Search for DWZ files in debug-file-directories as well

[Mark Wielaard]

Hi Simon,

On Sun, 2020-11-15 at 20:25 -0500, Simon Marchi wrote:
> I don't really have an opinion on this at the moment because I don't
> know much about how dwz files are used in practice.  Is having a
> ".dwz" somewhat standard?

Yes, most GNU/Linux distros and some other packaging initiatives, like
flatpaks, create dwz alt files, with GDB support since 7.5 and the
first distros using it since 2012. dwz https://sourceware.org/dwz/
implements Appendix E DWARF Compression and Duplicate Elimination (since Dwarf3) and the alt file extension was standardized as Supplemental Dwarf files in Dwarf5 (dwz currently still produces the GNU extension format, which is close, but not identical to the standardized variant, but work is being done to fully comply with Dwarf5).

Cheers,

Mark

Re: [PATCH] Search for DWZ files in debug-file-directories as well

[Sergio Durigan Junior]

On Sunday, November 15 2020, Simon Marchi wrote:

> On 2020-11-14 6:48 p.m., Sergio Durigan Junior via Gdb-patches wrote:
>> When Debian (and Ubuntu) builds its binaries, it (still) doesn't use
>> dwz's "--relative" option.  This causes their debuginfo files to
>> carry a .gnu_debugaltlink section containing a full pathname to the
>> DWZ alt debug file, like this:
>>
>>   $ readelf -wk /usr/bin/cat
>>   Contents of the .gnu_debugaltlink section:
>>
>>     Separate debug info file: /usr/lib/debug/.dwz/x86_64-linux-gnu/coreutils.debug
>>     Build-ID (0x14 bytes):
>>    ee 76 5d 71 97 37 ce 46 99 44 32 bb e8 a9 1a ef 99 96 88 db
>>
>>   Contents of the .gnu_debuglink section:
>>
>>     Separate debug info file: 06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
>>     CRC value: 0x53267655
>>
>> This usually works OK, because most of the debuginfo files installed
>> via apt will be present in /usr/lib/debug anyway.  However, imagine
>> the following scenario:
>>
>> - You are using /usr/bin/cat, it crashes on you and generates a
>>   corefile.
>>
>> - You don't want/need to "apt install" the debuginfo file for
>>   coreutils from the repositories.  Instead, you already have the
>>   debuginfo files in a separate directory (e.g., $HOME/dbgsym).
>>
>> - You start GDB and "set debug-file-directory $HOME/dbgsym".
>>   You then get the following message:
>>
>>   $ gdb -ex 'set debug-file-directory ./dbgsym/usr/lib/debug' -ex 'file /bin/cat' -ex 'core-file ./cat.core'
>>   GNU gdb (Ubuntu 10.1-0ubuntu1) 10.1
>>   ...
>>   Reading symbols from /bin/cat...
>>   Reading symbols from /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug...
>>   could not find '.gnu_debugaltlink' file for /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
>>
>> This error happens because GDB is trying to locate the build-id
>> link (inside /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id) for the
>> DWZ alt debug file, which doesn't exist.  Arguably, this is a problem
>> with how dh_dwz works in Debian, and it's something I'm also planning
>> to tackle.  But, back at the problem at hand.
>>
>> Besides not being able to find the build-id link in the directory
>> mentioned above, GDB also tried to open the DWZ alt file using its
>> filename.  The problem here is that, since we don't have the distro's
>> debuginfo installed, it can't find anything under /usr/lib/debug that
>> satisfies it.
>>
>> It occurred to me that a good way to workaround this problem is to
>> actually try to locate the DWZ alt debug file inside the
>> debug-file-directories (that were likely provided by the user).  So
>> this is what the proposed patch does.
>>
>> The idea here is simple: get the filename extracted from the
>> .gnu_debugaltlink section, and manipulate it in order to replace the
>> initial part of the path (everything before "/.dwz/") by whatever
>> debug-file-directories the user might have provided.
>>
>> I talked with Mark Wielaard and he agrees this is a sensible approach.
>> In fact, apparently this is something that eu-readelf also does.
>>
>> I regtested this code, and no regressions were found.
>
> Hi Sergio,

Bonjour, Simon,

Thanks for the review.

> I don't really have an opinion on this at the moment because I don't
> know much about how dwz files are used in practice.  Is having a ".dwz"
> somewhat standard?

Yeah, both Fedora and Debian/Ubuntu generate and use DWZ files
extensively.  Fedora has the 'find-debuginfo.sh' script which is called
during rpmbuild and takes care of the debuginfo/DWZ generation:

  https://github.com/rpm-software-management/rpm/blob/HEAD/scripts/find-debuginfo.sh#L510

Debian/Ubuntu have the dh_dwz step which is invoked via debhelper during
the package build:

  https://salsa.debian.org/debian/debhelper/-/blob/master/dh_dwz

>> @@ -2238,6 +2236,61 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>>    if (dwz_bfd == NULL)
>>      dwz_bfd = build_id_to_debug_bfd (buildid_len, buildid);
>>
>> +  if (dwz_bfd == nullptr)
>> +    {
>> +      /* If the user has provided us with different
>> +	 debug-file-directories, we can try them in order.  */
>> +      size_t dwz_pos = filename.find ("/.dwz/");
>> +
>> +      if (dwz_pos != std::string::npos)
>> +	{
>> +	  std::string tmpfilename = filename.erase (0, dwz_pos);
>
> "filename.erase", I think that modifies filename in place.  If so, is
> this what you intended?

Oh, you're right, both the .erase and the .insert methods modify their
arguments in-place.  This is not what I intended, but it's an easy fix.
Thanks for pointing it out.

>> +	  std::vector<gdb::unique_xmalloc_ptr<char>> debugdir_vec
>> +	    = dirnames_to_char_ptr_vec (debug_file_directory);
>> +
>> +	  for (const gdb::unique_xmalloc_ptr<char> &debugdir : debugdir_vec)
>> +	    {
>> +	      /* The idea is to iterate over the
>> +		 debug-file-directories provided by the user and
>> +		 replace the hard-coded path in the "filename" by each
>> +		 debug-file-directory.
>> +
>> +		 For example, suppose that filename is:
>> +
>> +		   /usr/lib/debug/.dwz/foo.debug
>> +
>> +		 And suppose that we have "$HOME/bar" as the
>> +		 debug-file-directory.  We would then adjust filename
>> +		 to look like:
>> +
>> +		   $HOME/bar/.dwz/foo.debug
>> +
>> +		 which would hopefully allow us to find the alt debug
>> +		 file.  */
>> +	      std::string ddir = debugdir.get ();
>> +
>> +	      if (filename.size () > ddir.size ()
>> +		  && filename.compare (0, ddir.size (), ddir) == 0)
>> +		continue;
>
> What's the intent of this condition?  Can you add a comment to make it
> explicit?

It's just a check to verify whether "filename" already refers to a file
under the "ddir" debug directory.  If it does, then there's nothing to
do here (because the function already tried to open the file before).
I'll add a comment on top of this check.

>> +
>> +	      std::string new_filename = tmpfilename.insert (0, ddir);
>
> Same question as above for "tmpfilename.insert".

Same answer as above :-).

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
https://sergiodj.net/

[PATCH v2] Search for DWZ files in debug-file-directories as well

[Sergio Durigan Junior]

Changes from v1:

- Addressed Simon's comments (new comment explaining how we try to
  match for the current debug-file-directory; properly use
  .erase/.insert methods -- keeping in mind that they modify the
  string in-place).


When Debian (and Ubuntu) builds its binaries, it (still) doesn't use
dwz's "--relative" option.  This causes their debuginfo files to
carry a .gnu_debugaltlink section containing a full pathname to the
DWZ alt debug file, like this:

  $ readelf -wk /usr/bin/cat
  Contents of the .gnu_debugaltlink section:

    Separate debug info file: /usr/lib/debug/.dwz/x86_64-linux-gnu/coreutils.debug
    Build-ID (0x14 bytes):
   ee 76 5d 71 97 37 ce 46 99 44 32 bb e8 a9 1a ef 99 96 88 db

  Contents of the .gnu_debuglink section:

    Separate debug info file: 06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
    CRC value: 0x53267655

This usually works OK, because most of the debuginfo files installed
via apt will be present in /usr/lib/debug anyway.  However, imagine
the following scenario:

- You are using /usr/bin/cat, it crashes on you and generates a
  corefile.

- You don't want/need to "apt install" the debuginfo file for
  coreutils from the repositories.  Instead, you already have the
  debuginfo files in a separate directory (e.g., $HOME/dbgsym).

- You start GDB and "set debug-file-directory $HOME/dbgsym".
  You then get the following message:

  $ gdb -ex 'set debug-file-directory ./dbgsym/usr/lib/debug' -ex 'file /bin/cat' -ex 'core-file ./cat.core'
  GNU gdb (Ubuntu 10.1-0ubuntu1) 10.1
  ...
  Reading symbols from /bin/cat...
  Reading symbols from /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug...
  could not find '.gnu_debugaltlink' file for /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug

This error happens because GDB is trying to locate the build-id
link (inside /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id) for the
DWZ alt debug file, which doesn't exist.  Arguably, this is a problem
with how dh_dwz works in Debian, and it's something I'm also planning
to tackle.  But, back at the problem at hand.

Besides not being able to find the build-id link in the directory
mentioned above, GDB also tried to open the DWZ alt file using its
filename.  The problem here is that, since we don't have the distro's
debuginfo installed, it can't find anything under /usr/lib/debug that
satisfies it.

It occurred to me that a good way to workaround this problem is to
actually try to locate the DWZ alt debug file inside the
debug-file-directories (that were likely provided by the user).  So
this is what the proposed patch does.

The idea here is simple: get the filename extracted from the
.gnu_debugaltlink section, and manipulate it in order to replace the
initial part of the path (everything before "/.dwz/") by whatever
debug-file-directories the user might have provided.

I talked with Mark Wielaard and he agrees this is a sensible approach.
In fact, apparently this is something that eu-readelf also does.

I regtested this code, and no regressions were found.

2020-11-14  Sergio Durigan Junior  <sergiodj@sergiodj.net>

	* dwarf2/read.c (dwarf2_get_dwz_file): Convert 'filename' to a
	std::string.  Implement ability to search for DWZ files in the
	debug-file-directories provided by the user as well.
---
 gdb/dwarf2/read.c | 73 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 67 insertions(+), 6 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 3c59826291..c462b9bb2c 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2190,7 +2190,7 @@ locate_dwz_sections (bfd *abfd, asection *sectp, dwz_file *dwz_file)
 struct dwz_file *
 dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
 {
-  const char *filename;
+  std::string filename;
   bfd_size_type buildid_len_arg;
   size_t buildid_len;
   bfd_byte *buildid;
@@ -2216,19 +2216,17 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
 
   filename = data.get ();
 
-  std::string abs_storage;
-  if (!IS_ABSOLUTE_PATH (filename))
+  if (!IS_ABSOLUTE_PATH (filename.c_str ()))
     {
       gdb::unique_xmalloc_ptr<char> abs
 	= gdb_realpath (bfd_get_filename (per_bfd->obfd));
 
-      abs_storage = ldirname (abs.get ()) + SLASH_STRING + filename;
-      filename = abs_storage.c_str ();
+      filename = ldirname (abs.get ()) + SLASH_STRING + filename;
     }
 
   /* First try the file name given in the section.  If that doesn't
      work, try to use the build-id instead.  */
-  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename, gnutarget));
+  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename.c_str (), gnutarget));
   if (dwz_bfd != NULL)
     {
       if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
@@ -2238,6 +2236,69 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
   if (dwz_bfd == NULL)
     dwz_bfd = build_id_to_debug_bfd (buildid_len, buildid);
 
+  if (dwz_bfd == nullptr)
+    {
+      /* If the user has provided us with different
+	 debug-file-directories, we can try them in order.  */
+      size_t dwz_pos = filename.find ("/.dwz/");
+
+      if (dwz_pos != std::string::npos)
+	{
+	  std::vector<gdb::unique_xmalloc_ptr<char>> debugdir_vec
+	    = dirnames_to_char_ptr_vec (debug_file_directory);
+
+	  for (const gdb::unique_xmalloc_ptr<char> &debugdir : debugdir_vec)
+	    {
+	      /* The idea is to iterate over the
+		 debug-file-directories provided by the user and
+		 replace the hard-coded path in the "filename" by each
+		 debug-file-directory.
+
+		 For example, suppose that filename is:
+
+		   /usr/lib/debug/.dwz/foo.dwz
+
+		 And suppose that we have "$HOME/bar" as the
+		 debug-file-directory.  We would then adjust filename
+		 to look like:
+
+		   $HOME/bar/.dwz/foo.dwz
+
+		 which would hopefully allow us to find the alt debug
+		 file.  */
+	      std::string ddir = debugdir.get ();
+
+	      /* Check whether the beginning of FILENAME is DDIR.  If
+		 it is, then we are dealing with a file which we
+		 already attempted to open before, so we just skip it
+		 and continue processing the reamining
+		 debug-file-directories.  */
+	      if (filename.size () > ddir.size ()
+		  && filename.compare (0, ddir.size (), ddir) == 0)
+		continue;
+
+	      /* Replace FILENAME's default debug-file-directory with
+		 DDIR.  */
+	      std::string new_filename = filename;
+	      new_filename.erase (0, dwz_pos);
+	      new_filename.insert (0, ddir);
+
+	      dwz_bfd = gdb_bfd_open (new_filename.c_str (), gnutarget);
+
+	      if (dwz_bfd != nullptr)
+		{
+		  if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
+		    {
+		      dwz_bfd.reset (nullptr);
+		      continue;
+		    }
+		  /* Found it.  */
+		  break;
+		}
+	    }
+	}
+    }
+
   if (dwz_bfd == nullptr)
     {
       gdb::unique_xmalloc_ptr<char> alt_filename;
-- 
2.28.0

Re: [PATCH v2] Search for DWZ files in debug-file-directories as well

[Sergio Durigan Junior]

On Wednesday, November 18 2020, Sergio Durigan Junior via Gdb-patches wrote:

> Changes from v1:
>
> - Addressed Simon's comments (new comment explaining how we try to
>   match for the current debug-file-directory; properly use
>   .erase/.insert methods -- keeping in mind that they modify the
>   string in-place).

Ping.

>
> When Debian (and Ubuntu) builds its binaries, it (still) doesn't use
> dwz's "--relative" option.  This causes their debuginfo files to
> carry a .gnu_debugaltlink section containing a full pathname to the
> DWZ alt debug file, like this:
>
>   $ readelf -wk /usr/bin/cat
>   Contents of the .gnu_debugaltlink section:
>
>     Separate debug info file: /usr/lib/debug/.dwz/x86_64-linux-gnu/coreutils.debug
>     Build-ID (0x14 bytes):
>    ee 76 5d 71 97 37 ce 46 99 44 32 bb e8 a9 1a ef 99 96 88 db
>
>   Contents of the .gnu_debuglink section:
>
>     Separate debug info file: 06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
>     CRC value: 0x53267655
>
> This usually works OK, because most of the debuginfo files installed
> via apt will be present in /usr/lib/debug anyway.  However, imagine
> the following scenario:
>
> - You are using /usr/bin/cat, it crashes on you and generates a
>   corefile.
>
> - You don't want/need to "apt install" the debuginfo file for
>   coreutils from the repositories.  Instead, you already have the
>   debuginfo files in a separate directory (e.g., $HOME/dbgsym).
>
> - You start GDB and "set debug-file-directory $HOME/dbgsym".
>   You then get the following message:
>
>   $ gdb -ex 'set debug-file-directory ./dbgsym/usr/lib/debug' -ex 'file /bin/cat' -ex 'core-file ./cat.core'
>   GNU gdb (Ubuntu 10.1-0ubuntu1) 10.1
>   ...
>   Reading symbols from /bin/cat...
>   Reading symbols from /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug...
>   could not find '.gnu_debugaltlink' file for /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
>
> This error happens because GDB is trying to locate the build-id
> link (inside /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id) for the
> DWZ alt debug file, which doesn't exist.  Arguably, this is a problem
> with how dh_dwz works in Debian, and it's something I'm also planning
> to tackle.  But, back at the problem at hand.
>
> Besides not being able to find the build-id link in the directory
> mentioned above, GDB also tried to open the DWZ alt file using its
> filename.  The problem here is that, since we don't have the distro's
> debuginfo installed, it can't find anything under /usr/lib/debug that
> satisfies it.
>
> It occurred to me that a good way to workaround this problem is to
> actually try to locate the DWZ alt debug file inside the
> debug-file-directories (that were likely provided by the user).  So
> this is what the proposed patch does.
>
> The idea here is simple: get the filename extracted from the
> .gnu_debugaltlink section, and manipulate it in order to replace the
> initial part of the path (everything before "/.dwz/") by whatever
> debug-file-directories the user might have provided.
>
> I talked with Mark Wielaard and he agrees this is a sensible approach.
> In fact, apparently this is something that eu-readelf also does.
>
> I regtested this code, and no regressions were found.
>
> 2020-11-14  Sergio Durigan Junior  <sergiodj@sergiodj.net>
>
> 	* dwarf2/read.c (dwarf2_get_dwz_file): Convert 'filename' to a
> 	std::string.  Implement ability to search for DWZ files in the
> 	debug-file-directories provided by the user as well.
> ---
>  gdb/dwarf2/read.c | 73 +++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 67 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 3c59826291..c462b9bb2c 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -2190,7 +2190,7 @@ locate_dwz_sections (bfd *abfd, asection *sectp, dwz_file *dwz_file)
>  struct dwz_file *
>  dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>  {
> -  const char *filename;
> +  std::string filename;
>    bfd_size_type buildid_len_arg;
>    size_t buildid_len;
>    bfd_byte *buildid;
> @@ -2216,19 +2216,17 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>  
>    filename = data.get ();
>  
> -  std::string abs_storage;
> -  if (!IS_ABSOLUTE_PATH (filename))
> +  if (!IS_ABSOLUTE_PATH (filename.c_str ()))
>      {
>        gdb::unique_xmalloc_ptr<char> abs
>  	= gdb_realpath (bfd_get_filename (per_bfd->obfd));
>  
> -      abs_storage = ldirname (abs.get ()) + SLASH_STRING + filename;
> -      filename = abs_storage.c_str ();
> +      filename = ldirname (abs.get ()) + SLASH_STRING + filename;
>      }
>  
>    /* First try the file name given in the section.  If that doesn't
>       work, try to use the build-id instead.  */
> -  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename, gnutarget));
> +  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename.c_str (), gnutarget));
>    if (dwz_bfd != NULL)
>      {
>        if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
> @@ -2238,6 +2236,69 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>    if (dwz_bfd == NULL)
>      dwz_bfd = build_id_to_debug_bfd (buildid_len, buildid);
>  
> +  if (dwz_bfd == nullptr)
> +    {
> +      /* If the user has provided us with different
> +	 debug-file-directories, we can try them in order.  */
> +      size_t dwz_pos = filename.find ("/.dwz/");
> +
> +      if (dwz_pos != std::string::npos)
> +	{
> +	  std::vector<gdb::unique_xmalloc_ptr<char>> debugdir_vec
> +	    = dirnames_to_char_ptr_vec (debug_file_directory);
> +
> +	  for (const gdb::unique_xmalloc_ptr<char> &debugdir : debugdir_vec)
> +	    {
> +	      /* The idea is to iterate over the
> +		 debug-file-directories provided by the user and
> +		 replace the hard-coded path in the "filename" by each
> +		 debug-file-directory.
> +
> +		 For example, suppose that filename is:
> +
> +		   /usr/lib/debug/.dwz/foo.dwz
> +
> +		 And suppose that we have "$HOME/bar" as the
> +		 debug-file-directory.  We would then adjust filename
> +		 to look like:
> +
> +		   $HOME/bar/.dwz/foo.dwz
> +
> +		 which would hopefully allow us to find the alt debug
> +		 file.  */
> +	      std::string ddir = debugdir.get ();
> +
> +	      /* Check whether the beginning of FILENAME is DDIR.  If
> +		 it is, then we are dealing with a file which we
> +		 already attempted to open before, so we just skip it
> +		 and continue processing the reamining
> +		 debug-file-directories.  */
> +	      if (filename.size () > ddir.size ()
> +		  && filename.compare (0, ddir.size (), ddir) == 0)
> +		continue;
> +
> +	      /* Replace FILENAME's default debug-file-directory with
> +		 DDIR.  */
> +	      std::string new_filename = filename;
> +	      new_filename.erase (0, dwz_pos);
> +	      new_filename.insert (0, ddir);
> +
> +	      dwz_bfd = gdb_bfd_open (new_filename.c_str (), gnutarget);
> +
> +	      if (dwz_bfd != nullptr)
> +		{
> +		  if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
> +		    {
> +		      dwz_bfd.reset (nullptr);
> +		      continue;
> +		    }
> +		  /* Found it.  */
> +		  break;
> +		}
> +	    }
> +	}
> +    }
> +
>    if (dwz_bfd == nullptr)
>      {
>        gdb::unique_xmalloc_ptr<char> alt_filename;
> -- 
> 2.28.0

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
https://sergiodj.net/

Re: [PATCH v2] Search for DWZ files in debug-file-directories as well

[Luis Machado]

Hi,

I like the idea of GDB trying harder to find a particular file. 
Sometimes it is a bit frustrating to deal with GDB and hardcoded paths.

With that said, the code is a bit confusing as-is, maybe because there 
is a lot of fiddling with the paths etc.

I think putting the new code into a function with the appropriate name 
would help here. Your choice.

In any case, it looks OK for me as-is. Just a nit below...

On 11/18/20 11:27 PM, Sergio Durigan Junior via Gdb-patches wrote:
> Changes from v1:
> 
> - Addressed Simon's comments (new comment explaining how we try to
>    match for the current debug-file-directory; properly use
>    .erase/.insert methods -- keeping in mind that they modify the
>    string in-place).
> 
> 
> When Debian (and Ubuntu) builds its binaries, it (still) doesn't use
> dwz's "--relative" option.  This causes their debuginfo files to
> carry a .gnu_debugaltlink section containing a full pathname to the
> DWZ alt debug file, like this:
> 
>    $ readelf -wk /usr/bin/cat
>    Contents of the .gnu_debugaltlink section:
> 
>      Separate debug info file: /usr/lib/debug/.dwz/x86_64-linux-gnu/coreutils.debug
>      Build-ID (0x14 bytes):
>     ee 76 5d 71 97 37 ce 46 99 44 32 bb e8 a9 1a ef 99 96 88 db
> 
>    Contents of the .gnu_debuglink section:
> 
>      Separate debug info file: 06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
>      CRC value: 0x53267655
> 
> This usually works OK, because most of the debuginfo files installed
> via apt will be present in /usr/lib/debug anyway.  However, imagine
> the following scenario:
> 
> - You are using /usr/bin/cat, it crashes on you and generates a
>    corefile.
> 
> - You don't want/need to "apt install" the debuginfo file for
>    coreutils from the repositories.  Instead, you already have the
>    debuginfo files in a separate directory (e.g., $HOME/dbgsym).
> 
> - You start GDB and "set debug-file-directory $HOME/dbgsym".
>    You then get the following message:
> 
>    $ gdb -ex 'set debug-file-directory ./dbgsym/usr/lib/debug' -ex 'file /bin/cat' -ex 'core-file ./cat.core'
>    GNU gdb (Ubuntu 10.1-0ubuntu1) 10.1
>    ...
>    Reading symbols from /bin/cat...
>    Reading symbols from /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug...
>    could not find '.gnu_debugaltlink' file for /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
> 
> This error happens because GDB is trying to locate the build-id
> link (inside /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id) for the
> DWZ alt debug file, which doesn't exist.  Arguably, this is a problem
> with how dh_dwz works in Debian, and it's something I'm also planning
> to tackle.  But, back at the problem at hand.
> 
> Besides not being able to find the build-id link in the directory
> mentioned above, GDB also tried to open the DWZ alt file using its
> filename.  The problem here is that, since we don't have the distro's
> debuginfo installed, it can't find anything under /usr/lib/debug that
> satisfies it.
> 
> It occurred to me that a good way to workaround this problem is to
> actually try to locate the DWZ alt debug file inside the
> debug-file-directories (that were likely provided by the user).  So
> this is what the proposed patch does.
> 
> The idea here is simple: get the filename extracted from the
> .gnu_debugaltlink section, and manipulate it in order to replace the
> initial part of the path (everything before "/.dwz/") by whatever
> debug-file-directories the user might have provided.
> 
> I talked with Mark Wielaard and he agrees this is a sensible approach.
> In fact, apparently this is something that eu-readelf also does.
> 
> I regtested this code, and no regressions were found.
> 
> 2020-11-14  Sergio Durigan Junior  <sergiodj@sergiodj.net>
> 
> 	* dwarf2/read.c (dwarf2_get_dwz_file): Convert 'filename' to a
> 	std::string.  Implement ability to search for DWZ files in the
> 	debug-file-directories provided by the user as well.
> ---
>   gdb/dwarf2/read.c | 73 +++++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 67 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 3c59826291..c462b9bb2c 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -2190,7 +2190,7 @@ locate_dwz_sections (bfd *abfd, asection *sectp, dwz_file *dwz_file)
>   struct dwz_file *
>   dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>   {
> -  const char *filename;
> +  std::string filename;
>     bfd_size_type buildid_len_arg;
>     size_t buildid_len;
>     bfd_byte *buildid;
> @@ -2216,19 +2216,17 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>   
>     filename = data.get ();
>   
> -  std::string abs_storage;
> -  if (!IS_ABSOLUTE_PATH (filename))
> +  if (!IS_ABSOLUTE_PATH (filename.c_str ()))
>       {
>         gdb::unique_xmalloc_ptr<char> abs
>   	= gdb_realpath (bfd_get_filename (per_bfd->obfd));
>   
> -      abs_storage = ldirname (abs.get ()) + SLASH_STRING + filename;
> -      filename = abs_storage.c_str ();
> +      filename = ldirname (abs.get ()) + SLASH_STRING + filename;
>       }
>   
>     /* First try the file name given in the section.  If that doesn't
>        work, try to use the build-id instead.  */
> -  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename, gnutarget));
> +  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename.c_str (), gnutarget));
>     if (dwz_bfd != NULL)
>       {
>         if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
> @@ -2238,6 +2236,69 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>     if (dwz_bfd == NULL)
>       dwz_bfd = build_id_to_debug_bfd (buildid_len, buildid);
>   
> +  if (dwz_bfd == nullptr)
> +    {
> +      /* If the user has provided us with different
> +	 debug-file-directories, we can try them in order.  */
> +      size_t dwz_pos = filename.find ("/.dwz/");
> +
> +      if (dwz_pos != std::string::npos)
> +	{
> +	  std::vector<gdb::unique_xmalloc_ptr<char>> debugdir_vec
> +	    = dirnames_to_char_ptr_vec (debug_file_directory);
> +
> +	  for (const gdb::unique_xmalloc_ptr<char> &debugdir : debugdir_vec)
> +	    {
> +	      /* The idea is to iterate over the
> +		 debug-file-directories provided by the user and
> +		 replace the hard-coded path in the "filename" by each
> +		 debug-file-directory.
> +
> +		 For example, suppose that filename is:
> +
> +		   /usr/lib/debug/.dwz/foo.dwz
> +
> +		 And suppose that we have "$HOME/bar" as the
> +		 debug-file-directory.  We would then adjust filename
> +		 to look like:
> +
> +		   $HOME/bar/.dwz/foo.dwz
> +
> +		 which would hopefully allow us to find the alt debug
> +		 file.  */
> +	      std::string ddir = debugdir.get ();
> +
> +	      /* Check whether the beginning of FILENAME is DDIR.  If
> +		 it is, then we are dealing with a file which we
> +		 already attempted to open before, so we just skip it
> +		 and continue processing the reamining

remaining?

> +		 debug-file-directories.  */

debug-file-directories doesn't exist in the code. I suppose this should 
be "debug file directories" or "debug-file-directory's"?

> +	      if (filename.size () > ddir.size ()
> +		  && filename.compare (0, ddir.size (), ddir) == 0)
> +		continue;
> +
> +	      /* Replace FILENAME's default debug-file-directory with
> +		 DDIR.  */
> +	      std::string new_filename = filename;
> +	      new_filename.erase (0, dwz_pos);
> +	      new_filename.insert (0, ddir);
> +
> +	      dwz_bfd = gdb_bfd_open (new_filename.c_str (), gnutarget);
> +
> +	      if (dwz_bfd != nullptr)
> +		{
> +		  if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
> +		    {
> +		      dwz_bfd.reset (nullptr);
> +		      continue;
> +		    }
> +		  /* Found it.  */
> +		  break;
> +		}
> +	    }
> +	}
> +    }
> +
>     if (dwz_bfd == nullptr)
>       {
>         gdb::unique_xmalloc_ptr<char> alt_filename;
> 

Re: [PATCH v2] Search for DWZ files in debug-file-directories as well

[Simon Marchi]

On 2020-11-18 9:27 p.m., Sergio Durigan Junior via Gdb-patches wrote:
> Changes from v1:
>
> - Addressed Simon's comments (new comment explaining how we try to
>   match for the current debug-file-directory; properly use
>   .erase/.insert methods -- keeping in mind that they modify the
>   string in-place).
>
>
> When Debian (and Ubuntu) builds its binaries, it (still) doesn't use
> dwz's "--relative" option.  This causes their debuginfo files to
> carry a .gnu_debugaltlink section containing a full pathname to the
> DWZ alt debug file, like this:
>
>   $ readelf -wk /usr/bin/cat
>   Contents of the .gnu_debugaltlink section:
>
>     Separate debug info file: /usr/lib/debug/.dwz/x86_64-linux-gnu/coreutils.debug
>     Build-ID (0x14 bytes):
>    ee 76 5d 71 97 37 ce 46 99 44 32 bb e8 a9 1a ef 99 96 88 db
>
>   Contents of the .gnu_debuglink section:
>
>     Separate debug info file: 06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
>     CRC value: 0x53267655
>
> This usually works OK, because most of the debuginfo files installed
> via apt will be present in /usr/lib/debug anyway.  However, imagine
> the following scenario:
>
> - You are using /usr/bin/cat, it crashes on you and generates a
>   corefile.
>
> - You don't want/need to "apt install" the debuginfo file for
>   coreutils from the repositories.  Instead, you already have the
>   debuginfo files in a separate directory (e.g., $HOME/dbgsym).
>
> - You start GDB and "set debug-file-directory $HOME/dbgsym".

Should the above read $HOME/dbgsym/usr/lib/debug to be consistent with
the example below?

> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 3c59826291..c462b9bb2c 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -2190,7 +2190,7 @@ locate_dwz_sections (bfd *abfd, asection *sectp, dwz_file *dwz_file)
>  struct dwz_file *
>  dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>  {
> -  const char *filename;
> +  std::string filename;
>    bfd_size_type buildid_len_arg;
>    size_t buildid_len;
>    bfd_byte *buildid;
> @@ -2216,19 +2216,17 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>
>    filename = data.get ();

Declare the filename variable here.

>
> -  std::string abs_storage;
> -  if (!IS_ABSOLUTE_PATH (filename))
> +  if (!IS_ABSOLUTE_PATH (filename.c_str ()))
>      {
>        gdb::unique_xmalloc_ptr<char> abs
>  	= gdb_realpath (bfd_get_filename (per_bfd->obfd));
>
> -      abs_storage = ldirname (abs.get ()) + SLASH_STRING + filename;
> -      filename = abs_storage.c_str ();
> +      filename = ldirname (abs.get ()) + SLASH_STRING + filename;
>      }
>
>    /* First try the file name given in the section.  If that doesn't
>       work, try to use the build-id instead.  */
> -  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename, gnutarget));
> +  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename.c_str (), gnutarget));
>    if (dwz_bfd != NULL)
>      {
>        if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
> @@ -2238,6 +2236,69 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>    if (dwz_bfd == NULL)
>      dwz_bfd = build_id_to_debug_bfd (buildid_len, buildid);
>
> +  if (dwz_bfd == nullptr)
> +    {
> +      /* If the user has provided us with different
> +	 debug-file-directories, we can try them in order.  */
> +      size_t dwz_pos = filename.find ("/.dwz/");
> +
> +      if (dwz_pos != std::string::npos)
> +	{
> +	  std::vector<gdb::unique_xmalloc_ptr<char>> debugdir_vec
> +	    = dirnames_to_char_ptr_vec (debug_file_directory);
> +
> +	  for (const gdb::unique_xmalloc_ptr<char> &debugdir : debugdir_vec)
> +	    {
> +	      /* The idea is to iterate over the
> +		 debug-file-directories provided by the user and
> +		 replace the hard-coded path in the "filename" by each
> +		 debug-file-directory.
> +
> +		 For example, suppose that filename is:
> +
> +		   /usr/lib/debug/.dwz/foo.dwz
> +
> +		 And suppose that we have "$HOME/bar" as the
> +		 debug-file-directory.  We would then adjust filename
> +		 to look like:
> +
> +		   $HOME/bar/.dwz/foo.dwz
> +
> +		 which would hopefully allow us to find the alt debug
> +		 file.  */
> +	      std::string ddir = debugdir.get ();
> +
> +	      /* Check whether the beginning of FILENAME is DDIR.  If
> +		 it is, then we are dealing with a file which we
> +		 already attempted to open before, so we just skip it
> +		 and continue processing the reamining
> +		 debug-file-directories.  */
> +	      if (filename.size () > ddir.size ()
> +		  && filename.compare (0, ddir.size (), ddir) == 0)
> +		continue;

Corner case, but what if file name is /usr/lib/abcde/.dwz/foo.dwz and
ddir is /usr/lib/abc?  We will wrongfully skip that debug dir I guess?

Filesystem paths are hard :(.  Is there some "is parent of" function we
can use?  Worst case, we skip that check and do an unnecessary attempt.

> +
> +	      /* Replace FILENAME's default debug-file-directory with
> +		 DDIR.  */
> +	      std::string new_filename = filename;
> +	      new_filename.erase (0, dwz_pos);
> +	      new_filename.insert (0, ddir);

I think it would be more readable and efficient (less bytes copied
around) to do just build the new_filename string with something like:

  std::string new_filename = debugdir + &filename[dwz_pos];

This is untested, but I think you'll get the point.  &filename[dwz_pos]
could also be put into a variable with a meaningful name, for clarity
(though I can't think of a good name right now).

And we probably don't need the allocated `ddir` std::string, you
construct it but never really use it (you could just refer to debugdir).

> +
> +	      dwz_bfd = gdb_bfd_open (new_filename.c_str (), gnutarget);
> +
> +	      if (dwz_bfd != nullptr)
> +		{
> +		  if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
> +		    {
> +		      dwz_bfd.reset (nullptr);

Just ".reset ()".

> +		      continue;
> +		    }
> +		  /* Found it.  */
> +		  break;
> +		}

Try to minimize the indentation levels where possible, by using early
returns or continues.  For example, here:

  if (dwz_bfd == nullptr)
    continue;

  if (!build_id_verify (...))
    {
      dwz_bfd.reset ();
      continue;
    }

  /* Found it.  */
  break;

Simon

Re: [PATCH v2] Search for DWZ files in debug-file-directories as well

[Sergio Durigan Junior]

On Wednesday, November 25 2020, Luis Machado wrote:

> Hi,
>
> I like the idea of GDB trying harder to find a particular
> file. Sometimes it is a bit frustrating to deal with GDB and hardcoded
> paths.
>
> With that said, the code is a bit confusing as-is, maybe because there
> is a lot of fiddling with the paths etc.
>
> I think putting the new code into a function with the appropriate name
> would help here. Your choice.

Thanks for the review.

I will see about putting this code into a separate function.

> In any case, it looks OK for me as-is. Just a nit below...
>
> On 11/18/20 11:27 PM, Sergio Durigan Junior via Gdb-patches wrote:
>> Changes from v1:
>> - Addressed Simon's comments (new comment explaining how we try to
>>    match for the current debug-file-directory; properly use
>>    .erase/.insert methods -- keeping in mind that they modify the
>>    string in-place).
>> 
>> When Debian (and Ubuntu) builds its binaries, it (still) doesn't use
>> dwz's "--relative" option.  This causes their debuginfo files to
>> carry a .gnu_debugaltlink section containing a full pathname to the
>> DWZ alt debug file, like this:
>>    $ readelf -wk /usr/bin/cat
>>    Contents of the .gnu_debugaltlink section:
>>      Separate debug info file:
>> /usr/lib/debug/.dwz/x86_64-linux-gnu/coreutils.debug
>>      Build-ID (0x14 bytes):
>>     ee 76 5d 71 97 37 ce 46 99 44 32 bb e8 a9 1a ef 99 96 88 db
>>    Contents of the .gnu_debuglink section:
>>      Separate debug info file:
>> 06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
>>      CRC value: 0x53267655
>> This usually works OK, because most of the debuginfo files installed
>> via apt will be present in /usr/lib/debug anyway.  However, imagine
>> the following scenario:
>> - You are using /usr/bin/cat, it crashes on you and generates a
>>    corefile.
>> - You don't want/need to "apt install" the debuginfo file for
>>    coreutils from the repositories.  Instead, you already have the
>>    debuginfo files in a separate directory (e.g., $HOME/dbgsym).
>> - You start GDB and "set debug-file-directory $HOME/dbgsym".
>>    You then get the following message:
>>    $ gdb -ex 'set debug-file-directory ./dbgsym/usr/lib/debug' -ex
>> 'file /bin/cat' -ex 'core-file ./cat.core'
>>    GNU gdb (Ubuntu 10.1-0ubuntu1) 10.1
>>    ...
>>    Reading symbols from /bin/cat...
>>    Reading symbols from /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug...
>>    could not find '.gnu_debugaltlink' file for /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
>> This error happens because GDB is trying to locate the build-id
>> link (inside /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id) for the
>> DWZ alt debug file, which doesn't exist.  Arguably, this is a problem
>> with how dh_dwz works in Debian, and it's something I'm also planning
>> to tackle.  But, back at the problem at hand.
>> Besides not being able to find the build-id link in the directory
>> mentioned above, GDB also tried to open the DWZ alt file using its
>> filename.  The problem here is that, since we don't have the distro's
>> debuginfo installed, it can't find anything under /usr/lib/debug that
>> satisfies it.
>> It occurred to me that a good way to workaround this problem is to
>> actually try to locate the DWZ alt debug file inside the
>> debug-file-directories (that were likely provided by the user).  So
>> this is what the proposed patch does.
>> The idea here is simple: get the filename extracted from the
>> .gnu_debugaltlink section, and manipulate it in order to replace the
>> initial part of the path (everything before "/.dwz/") by whatever
>> debug-file-directories the user might have provided.
>> I talked with Mark Wielaard and he agrees this is a sensible
>> approach.
>> In fact, apparently this is something that eu-readelf also does.
>> I regtested this code, and no regressions were found.
>> 2020-11-14  Sergio Durigan Junior  <sergiodj@sergiodj.net>
>> 	* dwarf2/read.c (dwarf2_get_dwz_file): Convert 'filename' to a
>> 	std::string.  Implement ability to search for DWZ files in the
>> 	debug-file-directories provided by the user as well.
>> ---
>>   gdb/dwarf2/read.c | 73 +++++++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 67 insertions(+), 6 deletions(-)
>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>> index 3c59826291..c462b9bb2c 100644
>> --- a/gdb/dwarf2/read.c
>> +++ b/gdb/dwarf2/read.c
>> @@ -2190,7 +2190,7 @@ locate_dwz_sections (bfd *abfd, asection *sectp, dwz_file *dwz_file)
>>   struct dwz_file *
>>   dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>>   {
>> -  const char *filename;
>> +  std::string filename;
>>     bfd_size_type buildid_len_arg;
>>     size_t buildid_len;
>>     bfd_byte *buildid;
>> @@ -2216,19 +2216,17 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>>       filename = data.get ();
>>   -  std::string abs_storage;
>> -  if (!IS_ABSOLUTE_PATH (filename))
>> +  if (!IS_ABSOLUTE_PATH (filename.c_str ()))
>>       {
>>         gdb::unique_xmalloc_ptr<char> abs
>>   	= gdb_realpath (bfd_get_filename (per_bfd->obfd));
>>   -      abs_storage = ldirname (abs.get ()) + SLASH_STRING +
>> filename;
>> -      filename = abs_storage.c_str ();
>> +      filename = ldirname (abs.get ()) + SLASH_STRING + filename;
>>       }
>>       /* First try the file name given in the section.  If that
>> doesn't
>>        work, try to use the build-id instead.  */
>> -  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename, gnutarget));
>> +  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename.c_str (), gnutarget));
>>     if (dwz_bfd != NULL)
>>       {
>>         if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
>> @@ -2238,6 +2236,69 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>>     if (dwz_bfd == NULL)
>>       dwz_bfd = build_id_to_debug_bfd (buildid_len, buildid);
>>   +  if (dwz_bfd == nullptr)
>> +    {
>> +      /* If the user has provided us with different
>> +	 debug-file-directories, we can try them in order.  */
>> +      size_t dwz_pos = filename.find ("/.dwz/");
>> +
>> +      if (dwz_pos != std::string::npos)
>> +	{
>> +	  std::vector<gdb::unique_xmalloc_ptr<char>> debugdir_vec
>> +	    = dirnames_to_char_ptr_vec (debug_file_directory);
>> +
>> +	  for (const gdb::unique_xmalloc_ptr<char> &debugdir : debugdir_vec)
>> +	    {
>> +	      /* The idea is to iterate over the
>> +		 debug-file-directories provided by the user and
>> +		 replace the hard-coded path in the "filename" by each
>> +		 debug-file-directory.
>> +
>> +		 For example, suppose that filename is:
>> +
>> +		   /usr/lib/debug/.dwz/foo.dwz
>> +
>> +		 And suppose that we have "$HOME/bar" as the
>> +		 debug-file-directory.  We would then adjust filename
>> +		 to look like:
>> +
>> +		   $HOME/bar/.dwz/foo.dwz
>> +
>> +		 which would hopefully allow us to find the alt debug
>> +		 file.  */
>> +	      std::string ddir = debugdir.get ();
>> +
>> +	      /* Check whether the beginning of FILENAME is DDIR.  If
>> +		 it is, then we are dealing with a file which we
>> +		 already attempted to open before, so we just skip it
>> +		 and continue processing the reamining
>
> remaining?

Fixed.

>> +		 debug-file-directories.  */
>
> debug-file-directories doesn't exist in the code. I suppose this
> should be "debug file directories" or "debug-file-directory's"?

OK, I see your rationale.  I don't mind having debug-file-directories,
but I replaced it with "debug file directories".

Thanks for the review.

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
https://sergiodj.net/

Re: [PATCH v2] Search for DWZ files in debug-file-directories as well

[Sergio Durigan Junior]

On Thursday, November 26 2020, Simon Marchi wrote:

> On 2020-11-18 9:27 p.m., Sergio Durigan Junior via Gdb-patches wrote:
>> Changes from v1:
>>
>> - Addressed Simon's comments (new comment explaining how we try to
>>   match for the current debug-file-directory; properly use
>>   .erase/.insert methods -- keeping in mind that they modify the
>>   string in-place).
>>
>>
>> When Debian (and Ubuntu) builds its binaries, it (still) doesn't use
>> dwz's "--relative" option.  This causes their debuginfo files to
>> carry a .gnu_debugaltlink section containing a full pathname to the
>> DWZ alt debug file, like this:
>>
>>   $ readelf -wk /usr/bin/cat
>>   Contents of the .gnu_debugaltlink section:
>>
>>     Separate debug info file: /usr/lib/debug/.dwz/x86_64-linux-gnu/coreutils.debug
>>     Build-ID (0x14 bytes):
>>    ee 76 5d 71 97 37 ce 46 99 44 32 bb e8 a9 1a ef 99 96 88 db
>>
>>   Contents of the .gnu_debuglink section:
>>
>>     Separate debug info file: 06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
>>     CRC value: 0x53267655
>>
>> This usually works OK, because most of the debuginfo files installed
>> via apt will be present in /usr/lib/debug anyway.  However, imagine
>> the following scenario:
>>
>> - You are using /usr/bin/cat, it crashes on you and generates a
>>   corefile.
>>
>> - You don't want/need to "apt install" the debuginfo file for
>>   coreutils from the repositories.  Instead, you already have the
>>   debuginfo files in a separate directory (e.g., $HOME/dbgsym).
>>
>> - You start GDB and "set debug-file-directory $HOME/dbgsym".
>
> Should the above read $HOME/dbgsym/usr/lib/debug to be consistent with
> the example below?

Sure.

>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>> index 3c59826291..c462b9bb2c 100644
>> --- a/gdb/dwarf2/read.c
>> +++ b/gdb/dwarf2/read.c
>> @@ -2190,7 +2190,7 @@ locate_dwz_sections (bfd *abfd, asection *sectp, dwz_file *dwz_file)
>>  struct dwz_file *
>>  dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>>  {
>> -  const char *filename;
>> +  std::string filename;
>>    bfd_size_type buildid_len_arg;
>>    size_t buildid_len;
>>    bfd_byte *buildid;
>> @@ -2216,19 +2216,17 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>>
>>    filename = data.get ();
>
> Declare the filename variable here.

Done.

>>
>> -  std::string abs_storage;
>> -  if (!IS_ABSOLUTE_PATH (filename))
>> +  if (!IS_ABSOLUTE_PATH (filename.c_str ()))
>>      {
>>        gdb::unique_xmalloc_ptr<char> abs
>>  	= gdb_realpath (bfd_get_filename (per_bfd->obfd));
>>
>> -      abs_storage = ldirname (abs.get ()) + SLASH_STRING + filename;
>> -      filename = abs_storage.c_str ();
>> +      filename = ldirname (abs.get ()) + SLASH_STRING + filename;
>>      }
>>
>>    /* First try the file name given in the section.  If that doesn't
>>       work, try to use the build-id instead.  */
>> -  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename, gnutarget));
>> +  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename.c_str (), gnutarget));
>>    if (dwz_bfd != NULL)
>>      {
>>        if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
>> @@ -2238,6 +2236,69 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
>>    if (dwz_bfd == NULL)
>>      dwz_bfd = build_id_to_debug_bfd (buildid_len, buildid);
>>
>> +  if (dwz_bfd == nullptr)
>> +    {
>> +      /* If the user has provided us with different
>> +	 debug-file-directories, we can try them in order.  */
>> +      size_t dwz_pos = filename.find ("/.dwz/");
>> +
>> +      if (dwz_pos != std::string::npos)
>> +	{
>> +	  std::vector<gdb::unique_xmalloc_ptr<char>> debugdir_vec
>> +	    = dirnames_to_char_ptr_vec (debug_file_directory);
>> +
>> +	  for (const gdb::unique_xmalloc_ptr<char> &debugdir : debugdir_vec)
>> +	    {
>> +	      /* The idea is to iterate over the
>> +		 debug-file-directories provided by the user and
>> +		 replace the hard-coded path in the "filename" by each
>> +		 debug-file-directory.
>> +
>> +		 For example, suppose that filename is:
>> +
>> +		   /usr/lib/debug/.dwz/foo.dwz
>> +
>> +		 And suppose that we have "$HOME/bar" as the
>> +		 debug-file-directory.  We would then adjust filename
>> +		 to look like:
>> +
>> +		   $HOME/bar/.dwz/foo.dwz
>> +
>> +		 which would hopefully allow us to find the alt debug
>> +		 file.  */
>> +	      std::string ddir = debugdir.get ();
>> +
>> +	      /* Check whether the beginning of FILENAME is DDIR.  If
>> +		 it is, then we are dealing with a file which we
>> +		 already attempted to open before, so we just skip it
>> +		 and continue processing the reamining
>> +		 debug-file-directories.  */
>> +	      if (filename.size () > ddir.size ()
>> +		  && filename.compare (0, ddir.size (), ddir) == 0)
>> +		continue;
>
> Corner case, but what if file name is /usr/lib/abcde/.dwz/foo.dwz and
> ddir is /usr/lib/abc?  We will wrongfully skip that debug dir I guess?
>
> Filesystem paths are hard :(.  Is there some "is parent of" function we
> can use?  Worst case, we skip that check and do an unnecessary attempt.

So, this case is only problematic if the debug-file-directory in
question doesn't end with DIR_SEPARATOR.

What I can do is check whether ddir ends with DIR_SEPARATOR, and if not,
I can add one.  This would make sure that we always check for the full
directory name, instead of the incomplete path.

>> +
>> +	      /* Replace FILENAME's default debug-file-directory with
>> +		 DDIR.  */
>> +	      std::string new_filename = filename;
>> +	      new_filename.erase (0, dwz_pos);
>> +	      new_filename.insert (0, ddir);
>
> I think it would be more readable and efficient (less bytes copied
> around) to do just build the new_filename string with something like:
>
>   std::string new_filename = debugdir + &filename[dwz_pos];
>
> This is untested, but I think you'll get the point.  &filename[dwz_pos]
> could also be put into a variable with a meaningful name, for clarity
> (though I can't think of a good name right now).

It is more efficient, but using .erase + .insert can be more readable.
But OK, I don't have a strong opinion here.

> And we probably don't need the allocated `ddir` std::string, you
> construct it but never really use it (you could just refer to debugdir).

I'll need it for the new version of the code, which will implement the
idea I gave above (always guaranteeing that ddir ends with
DIR_SEPARATOR).

>> +
>> +	      dwz_bfd = gdb_bfd_open (new_filename.c_str (), gnutarget);
>> +
>> +	      if (dwz_bfd != nullptr)
>> +		{
>> +		  if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
>> +		    {
>> +		      dwz_bfd.reset (nullptr);
>
> Just ".reset ()".

OK.

>> +		      continue;
>> +		    }
>> +		  /* Found it.  */
>> +		  break;
>> +		}
>
> Try to minimize the indentation levels where possible, by using early
> returns or continues.  For example, here:
>
>   if (dwz_bfd == nullptr)
>     continue;
>
>   if (!build_id_verify (...))
>     {
>       dwz_bfd.reset ();
>       continue;
>     }
>
>   /* Found it.  */
>   break;

Done.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
https://sergiodj.net/

Re: [PATCH v2] Search for DWZ files in debug-file-directories as well

[Sergio Durigan Junior]

On Saturday, November 28 2020, Sergio Durigan Junior via Gdb-patches wrote:

> On Thursday, November 26 2020, Simon Marchi wrote:
>
>> On 2020-11-18 9:27 p.m., Sergio Durigan Junior via Gdb-patches wrote:
>>> +
>>> +	      dwz_bfd = gdb_bfd_open (new_filename.c_str (), gnutarget);
>>> +
>>> +	      if (dwz_bfd != nullptr)
>>> +		{
>>> +		  if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
>>> +		    {
>>> +		      dwz_bfd.reset (nullptr);
>>
>> Just ".reset ()".
>
> OK.

FWIW, ".reset ()" doesn't work.  It has to be ".reset (nullptr)".

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
https://sergiodj.net/

Re: [PATCH v2] Search for DWZ files in debug-file-directories as well

[Simon Marchi]

On 2020-11-28 3:58 p.m., Sergio Durigan Junior wrote:
>> I think it would be more readable and efficient (less bytes copied
>> around) to do just build the new_filename string with something like:
>>
>>   std::string new_filename = debugdir + &filename[dwz_pos];
>>
>> This is untested, but I think you'll get the point.  &filename[dwz_pos]
>> could also be put into a variable with a meaningful name, for clarity
>> (though I can't think of a good name right now).
>
> It is more efficient, but using .erase + .insert can be more readable.
> But OK, I don't have a strong opinion here.

Well, I suggested the other way because I think that is more readable
:).  I see it more as "the new path is made of this part plus that
part".  I'll let you choose what you prefer, it's not code that needs to
be extremely optimized anyway.

>> And we probably don't need the allocated `ddir` std::string, you
>> construct it but never really use it (you could just refer to debugdir).
>
> I'll need it for the new version of the code, which will implement the
> idea I gave above (always guaranteeing that ddir ends with
> DIR_SEPARATOR).

Ok, if you do that I think you can move ddir out of the loop, it doesn't
need to be re-computed for each loop iteration.

Simon

Re: [PATCH v2] Search for DWZ files in debug-file-directories as well

[Simon Marchi]

On 2020-11-28 4:35 p.m., Sergio Durigan Junior via Gdb-patches wrote:
> FWIW, ".reset ()" doesn't work.  It has to be ".reset (nullptr)".

Ah, I thought this was based on unique_ptr.  We should make it work!

Simon

Re: [PATCH v2] Search for DWZ files in debug-file-directories as well

[Sergio Durigan Junior]

On Saturday, November 28 2020, Simon Marchi wrote:

> On 2020-11-28 3:58 p.m., Sergio Durigan Junior wrote:
>>> And we probably don't need the allocated `ddir` std::string, you
>>> construct it but never really use it (you could just refer to debugdir).
>>
>> I'll need it for the new version of the code, which will implement the
>> idea I gave above (always guaranteeing that ddir ends with
>> DIR_SEPARATOR).
>
> Ok, if you do that I think you can move ddir out of the loop, it doesn't
> need to be re-computed for each loop iteration.

ddir is just a temporary variable that will hold the current
debug-file-directory + SLASH_STRING.  I don't see how I could
pre-compute it outside the loop, where I won't have access to the
debug-file-directory.  But maybe I'm missing something?

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
https://sergiodj.net/

[PATCH v3] Search for DWZ files in debug-file-directories as well

[Sergio Durigan Junior]

Changes from v2:

- Put the new code into a new function.

- The code now makes sure that the debug-file-directory being
  processed ends with a directory separator, which makes things more
  robust.

- Minimized indentation level; fixed typos and improved code
  readability by adding more comments.


When Debian (and Ubuntu) builds its binaries, it (still) doesn't use
dwz's "--relative" option.  This causes their debuginfo files to
carry a .gnu_debugaltlink section containing a full pathname to the
DWZ alt debug file, like this:

  $ readelf -wk /usr/bin/cat
  Contents of the .gnu_debugaltlink section:

    Separate debug info file: /usr/lib/debug/.dwz/x86_64-linux-gnu/coreutils.debug
    Build-ID (0x14 bytes):
   ee 76 5d 71 97 37 ce 46 99 44 32 bb e8 a9 1a ef 99 96 88 db

  Contents of the .gnu_debuglink section:

    Separate debug info file: 06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
    CRC value: 0x53267655

This usually works OK, because most of the debuginfo files installed
via apt will be present in /usr/lib/debug anyway.  However, imagine
the following scenario:

- You are using /usr/bin/cat, it crashes on you and generates a
  corefile.

- You don't want/need to "apt install" the debuginfo file for
  coreutils from the repositories.  Instead, you already have the
  debuginfo files in a separate directory (e.g., $HOME/dbgsym).

- You start GDB and "set debug-file-directory $HOME/dbgsym/usr/lib/debug".
  You then get the following message:

  $ gdb -ex 'set debug-file-directory ./dbgsym/usr/lib/debug' -ex 'file /bin/cat' -ex 'core-file ./cat.core'
  GNU gdb (Ubuntu 10.1-0ubuntu1) 10.1
  ...
  Reading symbols from /bin/cat...
  Reading symbols from /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug...
  could not find '.gnu_debugaltlink' file for /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug

This error happens because GDB is trying to locate the build-id
link (inside /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id) for the
DWZ alt debug file, which doesn't exist.  Arguably, this is a problem
with how dh_dwz works in Debian, and it's something I'm also planning
to tackle.  But, back at the problem at hand.

Besides not being able to find the build-id link in the directory
mentioned above, GDB also tried to open the DWZ alt file using its
filename.  The problem here is that, since we don't have the distro's
debuginfo installed, it can't find anything under /usr/lib/debug that
satisfies it.

It occurred to me that a good way to workaround this problem is to
actually try to locate the DWZ alt debug file inside the
debug-file-directories (that were likely provided by the user).  So
this is what the proposed patch does.

The idea here is simple: get the filename extracted from the
.gnu_debugaltlink section, and manipulate it in order to replace the
initial part of the path (everything before "/.dwz/") by whatever
debug-file-directories the user might have provided.

I talked with Mark Wielaard and he agrees this is a sensible approach.
In fact, apparently this is something that eu-readelf also does.

I regtested this code, and no regressions were found.

2020-11-28  Sergio Durigan Junior  <sergiodj@sergiodj.net>

	* dwarf2/read.c (dwz_search_other_debugdirs): New function.
	(dwarf2_get_dwz_file): Convert 'filename' to a
	std::string.  Use dwz_search_other_debugdirs to search for DWZ
	files in the debug-file-directories provided by the user as well.
---
 gdb/dwarf2/read.c | 107 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 100 insertions(+), 7 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 601a571943..9468b9144e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2185,12 +2185,100 @@ locate_dwz_sections (bfd *abfd, asection *sectp, dwz_file *dwz_file)
     }
 }
 
+/* Attempt to find a .dwz file (whose full path is represented by
+   FILENAME) in all of the specified debug file directories provided.
+
+   Return the equivalent gdb_bfd_ref_ptr of the .dwz file found, or
+   nullptr if it could not find anything.  */
+
+static gdb_bfd_ref_ptr
+dwz_search_other_debugdirs (std::string &filename, bfd_byte *buildid,
+			    size_t buildid_len)
+{
+  /* Let's assume that the path represented by FILENAME has the
+     "/.dwz/" subpath in it.  This is what (most) GNU/Linux
+     distributions do, anyway.  */
+  size_t dwz_pos = filename.find ("/.dwz/");
+
+  if (dwz_pos == std::string::npos)
+    return nullptr;
+
+  /* This is an obvious assertion, but it's here more to educate
+     future readers of this code that FILENAME at DWZ_POS *must*
+     contain a directory separator.  */
+  gdb_assert (IS_DIR_SEPARATOR (filename[dwz_pos]));
+
+  gdb_bfd_ref_ptr dwz_bfd;
+  std::vector<gdb::unique_xmalloc_ptr<char>> debugdir_vec
+    = dirnames_to_char_ptr_vec (debug_file_directory);
+
+  for (const gdb::unique_xmalloc_ptr<char> &debugdir : debugdir_vec)
+    {
+      /* The idea is to iterate over the
+	 debug file directories provided by the user and
+	 replace the hard-coded path in the "filename" by each
+	 debug-file-directory.
+
+	 For example, suppose that filename is:
+
+	   /usr/lib/debug/.dwz/foo.dwz
+
+	 And suppose that we have "$HOME/bar" as the
+	 debug-file-directory.  We would then adjust filename
+	 to look like:
+
+	   $HOME/bar/.dwz/foo.dwz
+
+	 which would hopefully allow us to find the alt debug
+	 file.  */
+      std::string ddir = debugdir.get ();
+
+      if (ddir.empty ())
+	continue;
+
+      /* Make sure the current debug-file-directory ends with a
+	 directory separator.  This is needed because, if FILENAME
+	 contains something like "/usr/lib/abcde/.dwz/foo.dwz" and
+	 DDIR is "/usr/lib/abc", then could wrongfully skip it
+	 below.  */
+      if (!IS_DIR_SEPARATOR (ddir.back ()))
+	ddir += SLASH_STRING;
+
+      /* Check whether the beginning of FILENAME is DDIR.  If it is,
+	 then we are dealing with a file which we already attempted to
+	 open before, so we just skip it and continue processing the
+	 remaining debug file directories.  */
+      if (filename.size () > ddir.size ()
+	  && filename.compare (0, ddir.size (), ddir) == 0)
+	continue;
+
+      /* Replace FILENAME's default debug-file-directory with
+	 DDIR.  */
+      std::string new_filename = ddir + &filename[dwz_pos + 1];
+
+      dwz_bfd = gdb_bfd_open (new_filename.c_str (), gnutarget);
+
+      if (dwz_bfd == nullptr)
+	continue;
+
+      if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
+	{
+	  dwz_bfd.reset (nullptr);
+	  continue;
+	}
+
+      /* Found it.  */
+      break;
+    }
+
+  return dwz_bfd;
+}
+
 /* See dwarf2read.h.  */
 
 struct dwz_file *
 dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
 {
-  const char *filename;
   bfd_size_type buildid_len_arg;
   size_t buildid_len;
   bfd_byte *buildid;
@@ -2214,21 +2302,19 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
 
   buildid_len = (size_t) buildid_len_arg;
 
-  filename = data.get ();
+  std::string filename = data.get ();
 
-  std::string abs_storage;
-  if (!IS_ABSOLUTE_PATH (filename))
+  if (!IS_ABSOLUTE_PATH (filename.c_str ()))
     {
       gdb::unique_xmalloc_ptr<char> abs
 	= gdb_realpath (bfd_get_filename (per_bfd->obfd));
 
-      abs_storage = ldirname (abs.get ()) + SLASH_STRING + filename;
-      filename = abs_storage.c_str ();
+      filename = ldirname (abs.get ()) + SLASH_STRING + filename;
     }
 
   /* First try the file name given in the section.  If that doesn't
      work, try to use the build-id instead.  */
-  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename, gnutarget));
+  gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename.c_str (), gnutarget));
   if (dwz_bfd != NULL)
     {
       if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
@@ -2238,6 +2324,13 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
   if (dwz_bfd == NULL)
     dwz_bfd = build_id_to_debug_bfd (buildid_len, buildid);
 
+  if (dwz_bfd == nullptr)
+    {
+      /* If the user has provided us with different
+	 debug file directories, we can try them in order.  */
+      dwz_bfd = dwz_search_other_debugdirs (filename, buildid, buildid_len);
+    }
+
   if (dwz_bfd == nullptr)
     {
       gdb::unique_xmalloc_ptr<char> alt_filename;
-- 
2.29.2

Re: [PATCH v2] Search for DWZ files in debug-file-directories as well

[Simon Marchi]

On 2020-11-28 7:57 p.m., Sergio Durigan Junior wrote:
> ddir is just a temporary variable that will hold the current
> debug-file-directory + SLASH_STRING.  I don't see how I could
> pre-compute it outside the loop, where I won't have access to the
> debug-file-directory.  But maybe I'm missing something?

Ah sorry, I was confused then, forget about that.

Simon

Re: [PATCH v3] Search for DWZ files in debug-file-directories as well

[Simon Marchi]

On 2020-11-28 8:08 p.m., Sergio Durigan Junior via Gdb-patches wrote:
> Changes from v2:
>
> - Put the new code into a new function.
>
> - The code now makes sure that the debug-file-directory being
>   processed ends with a directory separator, which makes things more
>   robust.
>
> - Minimized indentation level; fixed typos and improved code
>   readability by adding more comments.
>
>
> When Debian (and Ubuntu) builds its binaries, it (still) doesn't use
> dwz's "--relative" option.  This causes their debuginfo files to
> carry a .gnu_debugaltlink section containing a full pathname to the
> DWZ alt debug file, like this:
>
>   $ readelf -wk /usr/bin/cat
>   Contents of the .gnu_debugaltlink section:
>
>     Separate debug info file: /usr/lib/debug/.dwz/x86_64-linux-gnu/coreutils.debug
>     Build-ID (0x14 bytes):
>    ee 76 5d 71 97 37 ce 46 99 44 32 bb e8 a9 1a ef 99 96 88 db
>
>   Contents of the .gnu_debuglink section:
>
>     Separate debug info file: 06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
>     CRC value: 0x53267655
>
> This usually works OK, because most of the debuginfo files installed
> via apt will be present in /usr/lib/debug anyway.  However, imagine
> the following scenario:
>
> - You are using /usr/bin/cat, it crashes on you and generates a
>   corefile.
>
> - You don't want/need to "apt install" the debuginfo file for
>   coreutils from the repositories.  Instead, you already have the
>   debuginfo files in a separate directory (e.g., $HOME/dbgsym).
>
> - You start GDB and "set debug-file-directory $HOME/dbgsym/usr/lib/debug".
>   You then get the following message:
>
>   $ gdb -ex 'set debug-file-directory ./dbgsym/usr/lib/debug' -ex 'file /bin/cat' -ex 'core-file ./cat.core'
>   GNU gdb (Ubuntu 10.1-0ubuntu1) 10.1
>   ...
>   Reading symbols from /bin/cat...
>   Reading symbols from /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug...
>   could not find '.gnu_debugaltlink' file for /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
>
> This error happens because GDB is trying to locate the build-id
> link (inside /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id) for the
> DWZ alt debug file, which doesn't exist.  Arguably, this is a problem
> with how dh_dwz works in Debian, and it's something I'm also planning
> to tackle.  But, back at the problem at hand.
>
> Besides not being able to find the build-id link in the directory
> mentioned above, GDB also tried to open the DWZ alt file using its
> filename.  The problem here is that, since we don't have the distro's
> debuginfo installed, it can't find anything under /usr/lib/debug that
> satisfies it.
>
> It occurred to me that a good way to workaround this problem is to
> actually try to locate the DWZ alt debug file inside the
> debug-file-directories (that were likely provided by the user).  So
> this is what the proposed patch does.
>
> The idea here is simple: get the filename extracted from the
> .gnu_debugaltlink section, and manipulate it in order to replace the
> initial part of the path (everything before "/.dwz/") by whatever
> debug-file-directories the user might have provided.
>
> I talked with Mark Wielaard and he agrees this is a sensible approach.
> In fact, apparently this is something that eu-readelf also does.
>
> I regtested this code, and no regressions were found.
>
> 2020-11-28  Sergio Durigan Junior  <sergiodj@sergiodj.net>
>
> 	* dwarf2/read.c (dwz_search_other_debugdirs): New function.
> 	(dwarf2_get_dwz_file): Convert 'filename' to a
> 	std::string.  Use dwz_search_other_debugdirs to search for DWZ
> 	files in the debug-file-directories provided by the user as well.

Thanks this is OK.

I forgot to mention that before, but a test would be nice to make sure
this feature doesn't break when doing changes to the DWARF reader.

Simon

Re: [PATCH v3] Search for DWZ files in debug-file-directories as well

[Sergio Durigan Junior]

On Tuesday, December 01 2020, Simon Marchi wrote:

> On 2020-11-28 8:08 p.m., Sergio Durigan Junior via Gdb-patches wrote:
>> Changes from v2:
>>
>> - Put the new code into a new function.
>>
>> - The code now makes sure that the debug-file-directory being
>>   processed ends with a directory separator, which makes things more
>>   robust.
>>
>> - Minimized indentation level; fixed typos and improved code
>>   readability by adding more comments.
>>
>>
>> When Debian (and Ubuntu) builds its binaries, it (still) doesn't use
>> dwz's "--relative" option.  This causes their debuginfo files to
>> carry a .gnu_debugaltlink section containing a full pathname to the
>> DWZ alt debug file, like this:
>>
>>   $ readelf -wk /usr/bin/cat
>>   Contents of the .gnu_debugaltlink section:
>>
>>     Separate debug info file: /usr/lib/debug/.dwz/x86_64-linux-gnu/coreutils.debug
>>     Build-ID (0x14 bytes):
>>    ee 76 5d 71 97 37 ce 46 99 44 32 bb e8 a9 1a ef 99 96 88 db
>>
>>   Contents of the .gnu_debuglink section:
>>
>>     Separate debug info file: 06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
>>     CRC value: 0x53267655
>>
>> This usually works OK, because most of the debuginfo files installed
>> via apt will be present in /usr/lib/debug anyway.  However, imagine
>> the following scenario:
>>
>> - You are using /usr/bin/cat, it crashes on you and generates a
>>   corefile.
>>
>> - You don't want/need to "apt install" the debuginfo file for
>>   coreutils from the repositories.  Instead, you already have the
>>   debuginfo files in a separate directory (e.g., $HOME/dbgsym).
>>
>> - You start GDB and "set debug-file-directory $HOME/dbgsym/usr/lib/debug".
>>   You then get the following message:
>>
>>   $ gdb -ex 'set debug-file-directory ./dbgsym/usr/lib/debug' -ex 'file /bin/cat' -ex 'core-file ./cat.core'
>>   GNU gdb (Ubuntu 10.1-0ubuntu1) 10.1
>>   ...
>>   Reading symbols from /bin/cat...
>>   Reading symbols from /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug...
>>   could not find '.gnu_debugaltlink' file for /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id/bc/06d3bee37b8c7e67b31cb2689cb351102ae73b.debug
>>
>> This error happens because GDB is trying to locate the build-id
>> link (inside /home/sergio/gdb/dbgsym/usr/lib/debug/.build-id) for the
>> DWZ alt debug file, which doesn't exist.  Arguably, this is a problem
>> with how dh_dwz works in Debian, and it's something I'm also planning
>> to tackle.  But, back at the problem at hand.
>>
>> Besides not being able to find the build-id link in the directory
>> mentioned above, GDB also tried to open the DWZ alt file using its
>> filename.  The problem here is that, since we don't have the distro's
>> debuginfo installed, it can't find anything under /usr/lib/debug that
>> satisfies it.
>>
>> It occurred to me that a good way to workaround this problem is to
>> actually try to locate the DWZ alt debug file inside the
>> debug-file-directories (that were likely provided by the user).  So
>> this is what the proposed patch does.
>>
>> The idea here is simple: get the filename extracted from the
>> .gnu_debugaltlink section, and manipulate it in order to replace the
>> initial part of the path (everything before "/.dwz/") by whatever
>> debug-file-directories the user might have provided.
>>
>> I talked with Mark Wielaard and he agrees this is a sensible approach.
>> In fact, apparently this is something that eu-readelf also does.
>>
>> I regtested this code, and no regressions were found.
>>
>> 2020-11-28  Sergio Durigan Junior  <sergiodj@sergiodj.net>
>>
>> 	* dwarf2/read.c (dwz_search_other_debugdirs): New function.
>> 	(dwarf2_get_dwz_file): Convert 'filename' to a
>> 	std::string.  Use dwz_search_other_debugdirs to search for DWZ
>> 	files in the debug-file-directories provided by the user as well.
>
> Thanks this is OK.

Thanks, I've just pushed the patch.

> I forgot to mention that before, but a test would be nice to make sure
> this feature doesn't break when doing changes to the DWARF reader.

Totally agreed.  I'll see if I can come up with a testcase for this, and
will submit it later.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
https://sergiodj.net/