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

[Simon Marchi <simon.marchi@ericsson.com>]

On 2019-01-28 1:53 p.m., John Baldwin wrote:
> 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?

Thanks, this is now pushed.

Simon