Re: [PATCH 7/7] gdb/debuginfod: Add .debug_line downloading

[Aaron Merey <amerey@redhat.com>]

On Tue, Mar 7, 2023 at 3:36 PM Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Aaron" == Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Aaron> This patch adds functions read_formatted_entries_separate and
> Aaron> dwarf_decode_line_header_separate.  They are similar to
> Aaron> read_formatted_entries and dwarf_decode_line_header except that they are
> Aaron> able to work with .debug_line sections originating from separately
> Aaron> downloaded files.
>
> I think there has to be some other refactoring to avoid duplicating code
> in this patch.  Copying ~300 lines like that seems bad, especially
> considering they'll need parallel updates whenever we find bugs, when
> DWARF changes, etc.

Will do.

>
> Aaron> +  gdb::unique_xmalloc_ptr<char> line_path;
> Aaron> +  scoped_fd line_fd = debuginfod_section_query (build_id->data,
> Aaron> +                                                build_id->size,
> Aaron> +                                                bfd_get_filename
> Aaron> +                                                  (objfile->obfd.get ()),
> Aaron> +                                                ".debug_line",
> Aaron> +                                                &line_path);
>
> For gdb's purposes, it's a shame debuginfod works explicitly on sections
> and not as more of a locally-caching filesystem-like API.  With the
> latter we would perhaps have very little or nothing to do to make this
> work, provided we were careful to keep the gdb-index code lazy about
> reading sections.  Also, inside gdb, all the sharing across inferiors
> and such would automatically work.  The cost would be whatever BFD
> requests when identifying a file, not sure how much data that is.

That's an interesting idea.  Though I wonder if the extra latency from
possibly many more small, frequent transfers outweighs the benefits
over downloading just .gdb_index and maybe .debug_line and the
full debuginfo.

>
> Anyway, back to current reality -- the DWARF reader already does try to
> lazily map section data.  So I think one big question is, why can't this
> be the mechanism for all the sections with debuginfod?  That is, stick
> the debuginfod calls into dwarf2_section_info::read.  I don't know what
> the debuginfod client does under the hood, but if it doesn't cache this
> data somewhere, perhaps gdb could.

Debuginfod does cache sections.  But before the full debuginfo is downloaded
gdb only knows about sections present in the solib binary.  This won't
include any .debug_* sections (otherwise gdb wouldn't be using debuginfod
here).  I think to make this work debuginfod would have to provide the
section header table too?

Since .debug_info is so connected to the other .debug_* sections do we
have much to gain by downloading sections separately once we know we'll
need the .debug_info?  At that point we might as well just download the
whole file.

Thanks for reviewing these patches.

Aaron