Re: [PATCH 1/2] Look for separate debug files in debug directories under a sysroot.

[John Baldwin <jhb@FreeBSD.org>]

On 1/26/19 9:12 AM, Simon Marchi wrote:
> On 2019-01-26 12:16 a.m., Simon Marchi wrote:
>> As discussed on IRC, we should probably add the same behavior for 
>> build-id-based separate debug files.
> 
> I gave a shot to this, here's what I have:
> 
> From 377ef615ad6e2c57966c8853c61cbce95ea6c2b3 Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Date: Sat, 26 Jan 2019 11:34:45 -0500
> Subject: [PATCH] Look for build-id-based separate debug files under the
>  sysroot
> 
> When looking for a separate debug file that matches a given build-id,
> GDB only looks in the host's debug dir (typically /usr/lib/debug).  This
> patch makes it look in the sysroot as well.  This is to match the
> behavior of GDB when using debuglink-based separate debug files.
> 
> In the following example, my sysroot is "/tmp/sysroot" and I am trying
> to load symbols for
> /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so.  This is
> the current behavior:
> 
>     (gdb) file /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so
>     Reading symbols from /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so...
> 
>     Looking for separate debug info (build-id) for /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so
>       Trying /usr/lib/debug/.build-id/f3/d6594d2600e985812cd4ba2ad083ac2aceae22.debug... no, unable to compute real path
> 
>     <snip>
>     (No debugging symbols found in /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so)
> 
> With this patch:
> 
>     (gdb) file /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so
>     Reading symbols from /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so...
> 
>     Looking for separate debug info (build-id) for /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so
>       Trying /usr/lib/debug/.build-id/f3/d6594d2600e985812cd4ba2ad083ac2aceae22.debug... no, unable to compute real path
>       Trying /tmp/sysroot/usr/lib/debug/.build-id/f3/d6594d2600e985812cd4ba2ad083ac2aceae22.debug... yes!
>     Reading symbols from /tmp/sysroot/usr/lib/debug/.build-id/f3/d6594d2600e985812cd4ba2ad083ac2aceae22.debug...
> 
> In the original code, there is a suspicious "abfd.release ()" in
> build_id_to_debug_bfd, that I don't understand.  If a file with the
> right name exists but its build-id note doesn't match, we release (leak)
> our reference, meaning the file will stay open?  I removed it in the new
> code, so that the reference is dropped if we end up not using that file.
> I tested briefly by corrupting a separate debug file to trigger this
> code, nothing exploded.

I think this looks good to me.  I think the .release () should have been
abfd.reset (nullptr) instead as you noted.  I think this isn't the first
time we've seen release () used when reset (nullptr) should have been used
instead unfortunately.  The name of that method is a bit ambiguous
unfortunately.  Do you want me to pull this into my series or just push it
after I push the final version of mine?

-- 
John Baldwin