[RFC] Support debuginfo and source file fetching via debuginfo server

  Debuginfo server is a lightweight web service that indexes debuginfo
and source files by build-id and serves them over HTTP. Debuginfo server
is able to index unpackaged, locally-built software in addition to RPM
files. Debuginfo server is packaged with a shared library, libdbgserver,
that provides a small set of client functions for fetching files from
debuginfo server. This patch adds debuginfo server support to GDB. In
case a source file or separate debuginfo file cannot be found locally,
GDB can use libdbgserver to query debuginfo server for the file in
question, if enabled to do so.

We plan on packaging debuginfo server with elfutils. For more information
please see the 'dbgserver' branch of the elfutils git repo
(https://sourceware.org/git/?p=elfutils.git;a=shortlog;h=refs/heads/dbgserver).

This patch was tested on x86-64 Fedora 29. The testsuite was run both
with and without this patch applied and the summaries indicated no
regressions. Debuginfo server tests were also added to the testsuite
under gdb/testsuite/gdb.dbgserver/.

gdb/ChangeLog:

        * configure.ac: Add --with-dbgserver option.
        * config.in: Add libdbgserver.
        * elfread.c (elf_symfile_read): Query debuginfo server if
        separate debuginfo file cannot be found.
        * Makefile.in (CLIBS): Add libdbgserver.
        * source.c (open_source_file): Query debuginfo server if source
        file cannot be found.
        * top.c (print_gdb_configuration): Print "--with-dbgserver" or
        "--without-dbgserver" depending on whether GDB was
        configured with debuginfo server.

gdb/testsuite/ChangeLog:

        * gdb.dbgserver/: New directory for debuginfo server tests.
        * gdb.dbgserver/fetch_src_and_symbols.c: New file.
        * gdb.dbgserver/fetch_src_and_symbols.exp: Test debuginfo
        server's ability to fetch source and separate debuginfo files.
---
 gdb/Makefile.in                               |  5 +-
 gdb/config.in                                 |  3 +
 gdb/configure.ac                              | 26 ++++++
 gdb/elfread.c                                 | 27 ++++++
 gdb/source.c                                  | 56 +++++++++++
 .../gdb.dbgserver/fetch_src_and_symbols.c     |  1 +
 .../gdb.dbgserver/fetch_src_and_symbols.exp   | 93 +++++++++++++++++++
 gdb/top.c                                     |  9 ++
 8 files changed, 219 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.dbgserver/fetch_src_and_symbols.c
 create mode 100644 gdb/testsuite/gdb.dbgserver/fetch_src_and_symbols.exp
  

>>>>> "Aaron" == Aaron Merey <amerey@redhat.com> writes:

Aaron> Debuginfo server is a lightweight web service that indexes debuginfo
Aaron> and source files by build-id and serves them over HTTP.

Thank you for the patch.

I think the idea is fine for gdb, so all that's left is some nits in the
patch.

Aaron> +#if HAVE_LIBDBGSERVER
Aaron> +      else
Aaron> +        {
Aaron> +          const struct bfd_build_id *build_id;
Aaron> +          char *debugfile_path;
Aaron> +
Aaron> +          build_id = build_id_bfd_get (objfile->obfd);
Aaron> +          int fd = dbgserver_find_debuginfo (build_id->data,
Aaron> +                                             build_id->size,
Aaron> +                                             &debugfile_path);

I was wondering what the fd represents.  If it's open on the file, can
we simply reuse it rather than trying to reopen the file?

Instead of "int", using scoped_fd would be better.

Aaron> +              symbol_file_add_separate(debug_bfd.get (), debugfile_path,

GNU style is a space before parens - there are a few instances.

Aaron> +              free(debugfile_path);

xfree instead of free.

Aaron> +#if HAVE_LIBDBGSERVER
Aaron> +  if (fd.get() < 0)
Aaron> +    {
Aaron> +      if (SYMTAB_COMPUNIT(s) != NULL)
Aaron> +        {
Aaron> +          const struct bfd_build_id *build_id;
Aaron> +          const objfile *ofp = COMPUNIT_OBJFILE (SYMTAB_COMPUNIT (s));
Aaron> +
Aaron> +          /* prefix the comp_dir to relative file names */
Aaron> +          const char* dirname = SYMTAB_DIRNAME (s);
Aaron> +          int suffname_len = strlen(dirname) + strlen(s->filename) + 2;
Aaron> +          char *suffname = (char *) alloca(suffname_len);

I think it's better to just use std::string for this kind of thing.

Probably this area will need some refactoring since other patches have
touched this.

Aaron> +              char *name_in_cache;
Aaron> +              int dbgsrv_rc = dbgserver_find_source (build_id->data,
Aaron> +                                                     build_id->size,
Aaron> +                                                     suffname,
Aaron> +                                                     &name_in_cache);
Aaron> +              if (dbgsrv_rc >= 0)
Aaron> +                {
Aaron> +                  fullname.reset (xstrdup(name_in_cache));
Aaron> +                  free (name_in_cache);

It seems like you could just use

    fullname.reset (name_in_cache);

here.

Aaron> +                }
Aaron> +              else if (dbgsrv_rc == -ENOSYS)
Aaron> +                {
Aaron> +                  /* -ENOSYS indicates that libdbgserver could not find
Aaron> +                     any dbgserver URLs to query due to $DBGSERVER_URLS
Aaron> +                     not being defined. Replace -ENOSYS with -ENOENT so
Aaron> +                     that users who have not configured dbgserver see the
Aaron> +                     usual error message when a source file cannot be found.  */
Aaron> +                  dbgsrv_rc = -ENOENT;

This assignment doesn't seem useful here.

Tom
  

On Tue, Aug 20, 2019 at 4:28 PM Aaron Merey <amerey@redhat.com> wrote:
> Debuginfo server is a lightweight web service that indexes debuginfo
> and source files by build-id and serves them over HTTP. Debuginfo server
> is able to index unpackaged, locally-built software in addition to RPM
> files. Debuginfo server is packaged with a shared library, libdbgserver,
> that provides a small set of client functions for fetching files from
> debuginfo server. This patch adds debuginfo server support to GDB. In
> case a source file or separate debuginfo file cannot be found locally,
> GDB can use libdbgserver to query debuginfo server for the file in
> question, if enabled to do so.
>
[...]
> @@ -1296,6 +1299,30 @@ elf_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
>           symbol_file_add_separate (debug_bfd.get (), debugfile.c_str (),
>                                     symfile_flags, objfile);
>         }
> +#if HAVE_LIBDBGSERVER
> +      else
> +        {
> +          const struct bfd_build_id *build_id;
> +          char *debugfile_path;
> +
> +          build_id = build_id_bfd_get (objfile->obfd);
> +          int fd = dbgserver_find_debuginfo (build_id->data,
> +                                             build_id->size,
> +                                             &debugfile_path);
> +
> +          if (fd >= 0)
> +            {
> +              /* debuginfo successfully retrieved from server, reopen
> +                 the file as a bfd instead.  */
> +              gdb_bfd_ref_ptr debug_bfd (symfile_bfd_open (debugfile_path));
> +
> +              symbol_file_add_separate(debug_bfd.get (), debugfile_path,
> +                                       symfile_flags, objfile);
> +              close(fd);
> +              free(debugfile_path);
> +            }
> +        }
> +#endif /* LIBDBGSERVER */
>      }
>  }
>

You wrote that the debuginfo server will download symbols over HTTP.
Does that mean that this call to dbgserver_find_debuginfo will block
as it downloads the file? (will ctrl+c work as it does that?)

If so, any way to do this download on a background thread?

Christian
  

>>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:

>> Debuginfo server is a lightweight web service that indexes debuginfo
>> and source files by build-id and serves them over HTTP. 

Christian> You wrote that the debuginfo server will download symbols over HTTP.
Christian> Does that mean that this call to dbgserver_find_debuginfo will block
Christian> as it downloads the file? (will ctrl+c work as it does that?)

The control-c question is a good one -- I'd also like to know the
answer.  Most things in gdb are interruptible this way.

Christian> If so, any way to do this download on a background thread?

I think gdb can't support this without some additional re-architecting.
It's a good goal, but I think I would not require it in order to get the
patch in.

Tom
  

On Wed, Oct 9, 2019 at 10:55 AM Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:
>
> >> Debuginfo server is a lightweight web service that indexes debuginfo
> >> and source files by build-id and serves them over HTTP.
>
> Christian> You wrote that the debuginfo server will download symbols over HTTP.
> Christian> Does that mean that this call to dbgserver_find_debuginfo will block
> Christian> as it downloads the file? (will ctrl+c work as it does that?)
>
> The control-c question is a good one -- I'd also like to know the
> answer.  Most things in gdb are interruptible this way.

We are now using the libcurl multi interface to fetch files over HTTP
and it is mostly non-blocking (although blocking can happen during
domain name resolution). However ctrl+c does not interrupt our symbol
downloading within GDB. This may be due to GDB's SIGINT handling,
which just sets a flag for the event loop when it is not safe to
throw an exception. Based on a comment in
gdb/event-top.c:handle_sigint(), symfile reading is one such region.

Also, we have an environment var $DEBUGINFOD_TIMEOUT that lets users
control how much time is spent attempting each download without having
to rely on ctrl+c.

Aaron
  

Tom> The control-c question is a good one -- I'd also like to know the
Tom> answer.  Most things in gdb are interruptible this way.

Aaron> We are now using the libcurl multi interface to fetch files over HTTP
Aaron> and it is mostly non-blocking (although blocking can happen during
Aaron> domain name resolution). However ctrl+c does not interrupt our symbol
Aaron> downloading within GDB. This may be due to GDB's SIGINT handling,
Aaron> which just sets a flag for the event loop when it is not safe to
Aaron> throw an exception. Based on a comment in
Aaron> gdb/event-top.c:handle_sigint(), symfile reading is one such region.

Aaron> Also, we have an environment var $DEBUGINFOD_TIMEOUT that lets users
Aaron> control how much time is spent attempting each download without having
Aaron> to rely on ctrl+c.

I think an environment variable may be too late.  I realize gdb doesn't
generally allow interrupting the reading of symbol tables.  But, this
case is a little different, in that the main thing happening is the
download of the debug info.  Being able to interrupt this would be good,
because servers can wedge, download speeds can be low, etc.  Also, maybe
printing something if the download takes too long would be good to do.

Tom
  

On Fri, Nov 1, 2019 at 3:31 PM Tom Tromey <tom@tromey.com> wrote:
> Aaron> domain name resolution). However ctrl+c does not interrupt our symbol
> Aaron> downloading within GDB. This may be due to GDB's SIGINT handling,
> Aaron> which just sets a flag for the event loop when it is not safe to
> Aaron> throw an exception. Based on a comment in
> Aaron> gdb/event-top.c:handle_sigint(), symfile reading is one such region.
>
> Aaron> Also, we have an environment var $DEBUGINFOD_TIMEOUT that lets users
> Aaron> control how much time is spent attempting each download without having
> Aaron> to rely on ctrl+c.
>
> I think an environment variable may be too late.  I realize gdb doesn't
> generally allow interrupting the reading of symbol tables.  But, this
> case is a little different, in that the main thing happening is the
> download of the debug info.  Being able to interrupt this would be good,
> because servers can wedge, download speeds can be low, etc.  Also, maybe
> printing something if the download takes too long would be good to do.

Ok makes sense. One approach would be for the debuginfod_find_*
functions to temporarily install their own SIGINT handler that cancels
any downloading. Though during this time ctrl+c won't have the same
gdb-wide effect that it currently has. When this happens maybe we
could print a message to indicate that the interrupt applied
specifically to the downloading. How does this sound to you?

Aaron
  

amerey wrote:

> Ok makes sense. One approach would be for the debuginfod_find_*
> functions to temporarily install their own SIGINT handler that cancels
> any downloading. Though during this time ctrl+c won't have the same
> gdb-wide effect that it currently has. When this happens maybe we
> could print a message to indicate that the interrupt applied
> specifically to the downloading. How does this sound to you?

Actually we can probably do even better: trap the SIGINT ourselves in
libdebuginfod, but upon return to gdb with an -EINTR, the caller can
call set_quit_flag().  Then the rest of gdb can react the normal way.

- FChE
  

fche@redhat.com (Frank Ch. Eigler) writes:

> Actually we can probably do even better: trap the SIGINT ourselves in
> libdebuginfod, but upon return to gdb with an -EINTR, the caller can
> call set_quit_flag().  Then the rest of gdb can react the normal way.

Actually2, prototyped a solution whereby the library temporarily hijacks
SIGINT to interrupt its ongoing transfers, then raises a SIGINT back to
gdb or other caller upon return.  If this works, then no changes at all
are required to the gdb/libdebuginfod interface code.

- FChe
  

On 2019-11-04 10:03 a.m., Frank Ch. Eigler wrote:
> fche@redhat.com (Frank Ch. Eigler) writes:
> 
>> Actually we can probably do even better: trap the SIGINT ourselves in
>> libdebuginfod, but upon return to gdb with an -EINTR, the caller can
>> call set_quit_flag().  Then the rest of gdb can react the normal way.
> 
> Actually2, prototyped a solution whereby the library temporarily hijacks
> SIGINT to interrupt its ongoing transfers, then raises a SIGINT back to
> gdb or other caller upon return.  If this works, then no changes at all
> are required to the gdb/libdebuginfod interface code.
> 
> - FChe
> 

I haven't followed this thread closely, so it might be an obvious "no", but
I was wondering if the library could offer to install a callback that is call
relatively often, allowing GDB to interrupt the operation if it returns a
certain value.  GDB would check its quit_flag in there.  I would find that
cleaner and easier to understand than having the main program and libraries
competing for the same signal handler.

At the same time, that callback would be used to report progress, and GDB
could display a nice progress bar!

Simon
  

Hi -

> I haven't followed this thread closely, so it might be an obvious "no", but
> I was wondering if the library could offer to install a callback that is call
> relatively often, allowing GDB to interrupt the operation [...]

That could also work, at the cost of extending the API.  Will play with it.

- FChE