Re: [PATCH] Avoid race when reading dwz file

[Lancelot SIX <lancelot.six@amd.com>]

On Fri, Mar 08, 2024 at 07:09:46AM -0700, Tom Tromey wrote:
> >>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
> Tom> PR gdb/31260 points out a race introduced by the background reading
> Tom> changes.  If a given objfile is re-opened when it is already being
> Tom> read, dwarf2_initialize_objfile will call dwarf2_read_dwz_file again,
> Tom> causing the 'dwz_file' to be reset.
> 
> Tom> This patch fixes the problem by arranging to open the dwz just once:
> Tom> when the dwarf2_per_bfd object is created.
> 
> Tom> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31260
> 
> I'm checking this in.
> 
> Tom

Hi Tom,

It looks like this patch introduces a slight behavior change, and I am
not sure if this is intentional (or desired).

Before this patch we had:

    bool
    dwarf2_initialize_objfile (...)
    {
      if (!dwarf2_has_info (...))
        return false;
      ...
         dwarf2_read_dwz_file (...);
    }

This meas that dwarf2_read_dwz_file is only called if dwarf2_has_info
returns true.

Since this patch, the call to dwarf2_read_dwz_file has been moved inside
dwarf2_has_info.  As a consequence, dwarf2_read_dwz_file can be called
in cases where dwarf2_has_info returns false.

I can observe this on ubuntu-22.06 when loading
/lib/x86_64-linux-gnu/libtinfo.so.6 (provided by libtinfo6).  That file
has no .debug_info section, but has a .gnu_debugaltlink section.  The
old logic would skip dwarf2_read_dwz_file because of the absence of
.debug_info, but the new behavior tries (and might fail) to load the
debug information referenced in .gnu_debugaltlink:

    $ ./gdb/gdb -data-directory ./gdb/data-directory -q /lib/x86_64-linux-gnu/libtinfo.so.6
    Reading symbols from /lib/x86_64-linux-gnu/libtinfo.so.6...
    
    warning: could not find '.gnu_debugaltlink' file for /usr/lib/x86_64-linux-gnu/libtinfo.so.6.3
    (No debugging symbols found in /lib/x86_64-linux-gnu/libtinfo.so.6)
    (gdb)

I am not too sure if it makes sense to try to load the debug info
referenced in .gnu_debugaltlink if the "main" debug info section is
missing.  It can be better than nothing as it can provide some types
information, but it is definitely not sufficient for a "full" debug
session.

Should the call to dwarf2_read_dwz_file be guarded only if "main" debug
info sections are present?  I can prepare a patch doing this.

[Arguably, .gnu_debugaltlink should also be removed when stripping the
debug information, but it is not.]

Best,
Lancelot.