[v2,PR,gdb/27570] missing support for debuginfod in core_target::build_file_mappings

Message ID 20210603000121.418619-1-amerey@redhat.com
State New
Headers show
Series
  • [v2,PR,gdb/27570] missing support for debuginfod in core_target::build_file_mappings
Related show

Commit Message

Tom de Vries via Gdb-patches June 3, 2021, 12:01 a.m.
Hi Tom,

Thanks for the review, I've included some comments below as well as v2
the patch based on your feedback.

On Wed, May 26, 2021 at 4:43 PM Tom Tromey <tom@tromey.com> wrote:
> Aaron> +      gdb::unique_xmalloc_ptr<char> execpath;

> Aaron> +      scoped_fd fd (debuginfod_exec_query (build_id->data, build_id->size,

> Aaron> +                                           abfd->filename, &execpath));

> Aaron> +

> Aaron> +      if (fd.get () >= 0)

> Aaron> +        {

> Aaron> +          execbfd = gdb_bfd_open (execpath.get (), gnutarget);

>

> Does 'fd' already refer to this same file?  If so it can just be reused,

> I think, by passing 'fd.release ()' to gdb_bfd_open.

>

> Or maybe there's some reason not to do this?  If so just let me know.


I didn't do this because when an fd is passed to gdb_bfd_open the resulting
bfd has a null build_id. Not sure if this is intended behavior.

> Aaron> +static std::unordered_map<std::string, std::string> soname_to_buildid;

>

> It seems like there ought to be some spot where this map is cleared.

> For example, it's not uncommon to rebuild a program and then re-"run" it

> in an existing gdb session.  In this situation the mapping could change.


Added a function debuginfod_clear_soname_buildids to do this. It gets
called in core_target::close.

> Aaron> +

> Aaron> +static int

> Aaron> +scan_soname_tag (struct bfd *abfd, char **soname)

> Aaron> +{

>

> This should have an intro comment explaining its purpose, the return

> value, and parameters.


Done.

> Aaron> +  /* Read in .dynamic and .dynstr from the BFD.  */

> Aaron> +  int sect_size = bfd_section_size (sect);

> Aaron> +  gdb_byte *bufstart = (gdb_byte *) alloca (sect_size);

>

> I think we should avoid unbounded use of alloca.

> A gdb::byte_vector would be fine.


Done.

> Aaron> +  /* Read the soname from the string table.  */

> Aaron> +  buf = (gdb_byte *) (xmalloc (sect_size));

> Aaron> +  if (!bfd_get_section_contents (abfd, dynstr, buf, 0, sect_size)) {

> Aaron> +    return 0;

> Aaron> +  }

> Aaron> +

> Aaron> +  *soname = (char *) (buf + soname_idx);

>

> I think this leaks 'buf'.


Fixed by using a gdb::char_vector instead of xmalloc.

> Aaron> +int

> Aaron> +debuginfod_set_soname_buildid (struct bfd *bfd, const bfd_build_id *build_id)

> Aaron> +{

> Aaron> +  gdb_bfd_ref_ptr abfd (gdb_bfd_open (bfd->filename, gnutarget));

> Aaron> +  bfd_check_format (abfd.get (), bfd_object);

>

> Does this work ok if abfd == nullptr?


Added an early return when abfd == nullptr.

> Aaron> +

> Aaron> +  if (build_id == nullptr)

> Aaron> +    return 0;

>

> Seems like this check could be hoisted above opening the BFD.


Done.

> Aaron> +extern scoped_fd

> Aaron> +debuginfod_exec_query (const unsigned char *build_id,

> Aaron> +                       int build_id_len,

> Aaron> +                       const char *filename,

> Aaron> +                       gdb::unique_xmalloc_ptr<char> *destname);

>

> gdb style is to put the type on the same line as the name in

> declarations (but not in definitions).


Done.

> Aaron> +      /* If a build-id was previously associated with this soname

> Aaron> +         then use it to query debuginfod for the library.  */ 

> Aaron> +      if (debuginfod_get_soname_buildid (so->so_name, &buildid_hexstr))

> Aaron> +        {

> Aaron> +          scoped_fd fd = debuginfod_exec_query

> Aaron> +                          ((const unsigned char*) buildid_hexstr.get (), 

> Aaron> +                           0, so->so_name, &filename);

>

> I didn't really track through this too well, but seeing this comment

> made me wonder if the build-id should just be directly attached to the 

> solib.


The resulting solib bfd (execbfd) already ends up containing the build-id.


One other point I should mention is this debuginfod support for downloading
solibs referenced in core files depends on the presence of proc file mappings
in the core file. If no mappings can be found then there will be no soname
build-id information available in solib_map_sections, which is needed to query
debuginfod. (Debuginfo and source queries for local solibs would still work
properly though.)

It may be possible to support solib downloading for core files without proc
mappings but it could get a bit tricky. We could still search each ELF section
of the core file for a build-id but we wouldn't be able to tell which filename
it belongs to without the proc mappings. To get the missing solibs we could
query debuginfod for binaries with build-ids that are present in the core file
but were not seen among the solibs that GDB found locally.

The problem is that debuginfod would have to indiscriminately download all of
the binaries associated with these build-ids when there is a chance some still
might be found locally. This could happen when a missing library links to
another library which is present on the local machine. In this case debuginfod
would download this library before GDB has tried to look for it. 

AFAIK it's only valgrind's vgcore files which don't currently include proc
file mappings. I might approach the valgrind folks about adding proc file
mappings to vgcore files. 

Anyways I just wanted to make sure this proc file mapping dependency was 
made clear.

Thanks,
Aaron

---

From 0f874e434f37f7e24798a5da17b4dbabb2e46b60 Mon Sep 17 00:00:00 2001
From: Aaron Merey <amerey@redhat.com>

Date: Wed, 2 Jun 2021 17:41:26 -0400
Subject: [PATCH] [PR gdb/27570] missing support for debuginfod in
 core_target::build_file_mappings

Add debuginfod support at various stages of core file processing to enable
the downloading of missing executables and shared libraries from debuginfod
servers.

In order to perform these queries debuginfod requires the build-id of the
missing executable or shared library. These are read from the core file in
linux_read_core_file_mappings and passed to core_target::build_file_mappings
where the query occurs if the file that backs the core file mapping cannot be
found.

GDB will attempt to open shared libraries found in core file mappings during
solib_map_sections even if they were already opened during
core_target::build_file_mappings. This presented a problem since libraries
downloaded from debuginfod won't be found at the path which solib_map_sections
attempts to open. They will instead be stored in the local debuginfod cache.

We already have a BFD handle for these downloaded libraries opened in
core_target::build_file_mappings but there is no easy way to identify and
reuse the correct BFD during solib_map_sections. This is because the filename
used to open the library in solib_map_sections may differ from the filename
used in core_target::build_file_mappings, even though they refer to the same
shared object. This difference can be seen by comparing library filenames from
'info proc mapping' with those from 'info sharedlibrary' (for example,
"libc-2.32.so" vs "libc.so.6").

Therefore GDB needs a way to associate library filenames used in
solib_map_sections with the corresponding build-id from the core file mappings
in case it needs to reopen the library from the local debuginfod cache. This
patch adds facilities to debuginfod-support.c to make this possible.

We populate a map of sonames to build-ids during core_target::build_file_mappings
by using the new function debuginfod_set_soname_buildid. The filenames used in
solib_map_sections originate from each library's DT_SONAME tag in its .dynamic
segment so we added another function scan_soname_tag (which is modelled after
solib-svr4.c:scan_dyntag) that will lookup the value of DT_SONAME in the dynamic
string table segment of the library. This name gets mapped to the corresponding
build-id. Then using the new function debuginfod_get_soname_buildid we can
retrieve the desired build-id in solib_map_sections and check the debuginfod
cache in case the library can't otherwise be found.

Tested on Fedora 33 x86_64.

gdb/ChangeLog:

	PR gdb/27570
	* arch-utils.c (default_read_core_file_mappings): Add build_id
	parameter.
	* arch-utils.h (default_read_core_file_mappings): Add build_id
	parameter.
	* corelow.c (core_target::build_file_mappings): Add debuginfod
	executable query and map library soname to build-id.
	(core_target::close): Call debuginfod_clear_soname_buildids.
	(locate_exec_from_corefile_build_id): Add debuginfod executable query
	* debuginfod-support.c (debuginfod_exec_query): New function.
	(debuginfod_set_soname_buildid): New function.
	(debuginfod_get_soname_buildid): New function.
	(debuginfod_clear_soname_buildids): New function.
	(scan_soname_tag): New function.
	* debuginfod-support.h (debuginfod_exec_query): New function.
	(debuginfod_set_soname_buildid): New function.
	(debuginfod_get_soname_buildid): New function.
	(debuginfod_clear_soname_buildids): New function.
	* gcore.in: unset $DEBUGINFOD_URLS.
	* gdbarch.c: Regenerate.
	* gdbarch.h: Regenerate.
	* gdbarch.sh (read_core_file_mappings): Add build_id parameter.
	* linux-tdep.c (linux_read_core_file_mappings): Map shared library
	filenames in core file mappings to build-id.
	(linux_core_info_proc_mappings): Add build-id parameter.
	* solib.c (solib_map_sections): Add debuginfod executable query.

gdb/testsuite/ChangeLog:

	PR gdb/27570
	* gdb.debuginfod/fetch_src_and_symbols.exp: Add new testcases.
---
 gdb/arch-utils.c                              |   4 +-
 gdb/arch-utils.h                              |   4 +-
 gdb/corelow.c                                 |  40 +++-
 gdb/debuginfod-support.c                      | 210 ++++++++++++++++++
 gdb/debuginfod-support.h                      |  41 ++++
 gdb/gcore.in                                  |   3 +
 gdb/gdbarch.c                                 |   2 +-
 gdb/gdbarch.h                                 |   4 +-
 gdb/gdbarch.sh                                |   2 +-
 gdb/linux-tdep.c                              |  34 ++-
 gdb/solib.c                                   |  19 ++
 .../gdb.debuginfod/fetch_src_and_symbols.exp  |  25 ++-
 12 files changed, 376 insertions(+), 12 deletions(-)

-- 
2.31.1

Comments

Tom de Vries via Gdb-patches June 21, 2021, 10:18 p.m. | #1
Ping

Thanks,
Aaron

On Wed, Jun 2, 2021 at 8:01 PM Aaron Merey via Gdb-patches
<gdb-patches@sourceware.org> wrote:
>

> Hi Tom,

>

> Thanks for the review, I've included some comments below as well as v2

> the patch based on your feedback.

>

> On Wed, May 26, 2021 at 4:43 PM Tom Tromey <tom@tromey.com> wrote:

> > Aaron> +      gdb::unique_xmalloc_ptr<char> execpath;

> > Aaron> +      scoped_fd fd (debuginfod_exec_query (build_id->data, build_id->size,

> > Aaron> +                                           abfd->filename, &execpath));

> > Aaron> +

> > Aaron> +      if (fd.get () >= 0)

> > Aaron> +        {

> > Aaron> +          execbfd = gdb_bfd_open (execpath.get (), gnutarget);

> >

> > Does 'fd' already refer to this same file?  If so it can just be reused,

> > I think, by passing 'fd.release ()' to gdb_bfd_open.

> >

> > Or maybe there's some reason not to do this?  If so just let me know.

>

> I didn't do this because when an fd is passed to gdb_bfd_open the resulting

> bfd has a null build_id. Not sure if this is intended behavior.

>

> > Aaron> +static std::unordered_map<std::string, std::string> soname_to_buildid;

> >

> > It seems like there ought to be some spot where this map is cleared.

> > For example, it's not uncommon to rebuild a program and then re-"run" it

> > in an existing gdb session.  In this situation the mapping could change.

>

> Added a function debuginfod_clear_soname_buildids to do this. It gets

> called in core_target::close.

>

> > Aaron> +

> > Aaron> +static int

> > Aaron> +scan_soname_tag (struct bfd *abfd, char **soname)

> > Aaron> +{

> >

> > This should have an intro comment explaining its purpose, the return

> > value, and parameters.

>

> Done.

>

> > Aaron> +  /* Read in .dynamic and .dynstr from the BFD.  */

> > Aaron> +  int sect_size = bfd_section_size (sect);

> > Aaron> +  gdb_byte *bufstart = (gdb_byte *) alloca (sect_size);

> >

> > I think we should avoid unbounded use of alloca.

> > A gdb::byte_vector would be fine.

>

> Done.

>

> > Aaron> +  /* Read the soname from the string table.  */

> > Aaron> +  buf = (gdb_byte *) (xmalloc (sect_size));

> > Aaron> +  if (!bfd_get_section_contents (abfd, dynstr, buf, 0, sect_size)) {

> > Aaron> +    return 0;

> > Aaron> +  }

> > Aaron> +

> > Aaron> +  *soname = (char *) (buf + soname_idx);

> >

> > I think this leaks 'buf'.

>

> Fixed by using a gdb::char_vector instead of xmalloc.

>

> > Aaron> +int

> > Aaron> +debuginfod_set_soname_buildid (struct bfd *bfd, const bfd_build_id *build_id)

> > Aaron> +{

> > Aaron> +  gdb_bfd_ref_ptr abfd (gdb_bfd_open (bfd->filename, gnutarget));

> > Aaron> +  bfd_check_format (abfd.get (), bfd_object);

> >

> > Does this work ok if abfd == nullptr?

>

> Added an early return when abfd == nullptr.

>

> > Aaron> +

> > Aaron> +  if (build_id == nullptr)

> > Aaron> +    return 0;

> >

> > Seems like this check could be hoisted above opening the BFD.

>

> Done.

>

> > Aaron> +extern scoped_fd

> > Aaron> +debuginfod_exec_query (const unsigned char *build_id,

> > Aaron> +                       int build_id_len,

> > Aaron> +                       const char *filename,

> > Aaron> +                       gdb::unique_xmalloc_ptr<char> *destname);

> >

> > gdb style is to put the type on the same line as the name in

> > declarations (but not in definitions).

>

> Done.

>

> > Aaron> +      /* If a build-id was previously associated with this soname

> > Aaron> +         then use it to query debuginfod for the library.  */

> > Aaron> +      if (debuginfod_get_soname_buildid (so->so_name, &buildid_hexstr))

> > Aaron> +        {

> > Aaron> +          scoped_fd fd = debuginfod_exec_query

> > Aaron> +                          ((const unsigned char*) buildid_hexstr.get (),

> > Aaron> +                           0, so->so_name, &filename);

> >

> > I didn't really track through this too well, but seeing this comment

> > made me wonder if the build-id should just be directly attached to the

> > solib.

>

> The resulting solib bfd (execbfd) already ends up containing the build-id.

>

>

> One other point I should mention is this debuginfod support for downloading

> solibs referenced in core files depends on the presence of proc file mappings

> in the core file. If no mappings can be found then there will be no soname

> build-id information available in solib_map_sections, which is needed to query

> debuginfod. (Debuginfo and source queries for local solibs would still work

> properly though.)

>

> It may be possible to support solib downloading for core files without proc

> mappings but it could get a bit tricky. We could still search each ELF section

> of the core file for a build-id but we wouldn't be able to tell which filename

> it belongs to without the proc mappings. To get the missing solibs we could

> query debuginfod for binaries with build-ids that are present in the core file

> but were not seen among the solibs that GDB found locally.

>

> The problem is that debuginfod would have to indiscriminately download all of

> the binaries associated with these build-ids when there is a chance some still

> might be found locally. This could happen when a missing library links to

> another library which is present on the local machine. In this case debuginfod

> would download this library before GDB has tried to look for it.

>

> AFAIK it's only valgrind's vgcore files which don't currently include proc

> file mappings. I might approach the valgrind folks about adding proc file

> mappings to vgcore files.

>

> Anyways I just wanted to make sure this proc file mapping dependency was

> made clear.

>

> Thanks,

> Aaron

>

> ---

>

> From 0f874e434f37f7e24798a5da17b4dbabb2e46b60 Mon Sep 17 00:00:00 2001

> From: Aaron Merey <amerey@redhat.com>

> Date: Wed, 2 Jun 2021 17:41:26 -0400

> Subject: [PATCH] [PR gdb/27570] missing support for debuginfod in

>  core_target::build_file_mappings

>

> Add debuginfod support at various stages of core file processing to enable

> the downloading of missing executables and shared libraries from debuginfod

> servers.

>

> In order to perform these queries debuginfod requires the build-id of the

> missing executable or shared library. These are read from the core file in

> linux_read_core_file_mappings and passed to core_target::build_file_mappings

> where the query occurs if the file that backs the core file mapping cannot be

> found.

>

> GDB will attempt to open shared libraries found in core file mappings during

> solib_map_sections even if they were already opened during

> core_target::build_file_mappings. This presented a problem since libraries

> downloaded from debuginfod won't be found at the path which solib_map_sections

> attempts to open. They will instead be stored in the local debuginfod cache.

>

> We already have a BFD handle for these downloaded libraries opened in

> core_target::build_file_mappings but there is no easy way to identify and

> reuse the correct BFD during solib_map_sections. This is because the filename

> used to open the library in solib_map_sections may differ from the filename

> used in core_target::build_file_mappings, even though they refer to the same

> shared object. This difference can be seen by comparing library filenames from

> 'info proc mapping' with those from 'info sharedlibrary' (for example,

> "libc-2.32.so" vs "libc.so.6").

>

> Therefore GDB needs a way to associate library filenames used in

> solib_map_sections with the corresponding build-id from the core file mappings

> in case it needs to reopen the library from the local debuginfod cache. This

> patch adds facilities to debuginfod-support.c to make this possible.

>

> We populate a map of sonames to build-ids during core_target::build_file_mappings

> by using the new function debuginfod_set_soname_buildid. The filenames used in

> solib_map_sections originate from each library's DT_SONAME tag in its .dynamic

> segment so we added another function scan_soname_tag (which is modelled after

> solib-svr4.c:scan_dyntag) that will lookup the value of DT_SONAME in the dynamic

> string table segment of the library. This name gets mapped to the corresponding

> build-id. Then using the new function debuginfod_get_soname_buildid we can

> retrieve the desired build-id in solib_map_sections and check the debuginfod

> cache in case the library can't otherwise be found.

>

> Tested on Fedora 33 x86_64.

>

> gdb/ChangeLog:

>

>         PR gdb/27570

>         * arch-utils.c (default_read_core_file_mappings): Add build_id

>         parameter.

>         * arch-utils.h (default_read_core_file_mappings): Add build_id

>         parameter.

>         * corelow.c (core_target::build_file_mappings): Add debuginfod

>         executable query and map library soname to build-id.

>         (core_target::close): Call debuginfod_clear_soname_buildids.

>         (locate_exec_from_corefile_build_id): Add debuginfod executable query

>         * debuginfod-support.c (debuginfod_exec_query): New function.

>         (debuginfod_set_soname_buildid): New function.

>         (debuginfod_get_soname_buildid): New function.

>         (debuginfod_clear_soname_buildids): New function.

>         (scan_soname_tag): New function.

>         * debuginfod-support.h (debuginfod_exec_query): New function.

>         (debuginfod_set_soname_buildid): New function.

>         (debuginfod_get_soname_buildid): New function.

>         (debuginfod_clear_soname_buildids): New function.

>         * gcore.in: unset $DEBUGINFOD_URLS.

>         * gdbarch.c: Regenerate.

>         * gdbarch.h: Regenerate.

>         * gdbarch.sh (read_core_file_mappings): Add build_id parameter.

>         * linux-tdep.c (linux_read_core_file_mappings): Map shared library

>         filenames in core file mappings to build-id.

>         (linux_core_info_proc_mappings): Add build-id parameter.

>         * solib.c (solib_map_sections): Add debuginfod executable query.

>

> gdb/testsuite/ChangeLog:

>

>         PR gdb/27570

>         * gdb.debuginfod/fetch_src_and_symbols.exp: Add new testcases.

> ---

>  gdb/arch-utils.c                              |   4 +-

>  gdb/arch-utils.h                              |   4 +-

>  gdb/corelow.c                                 |  40 +++-

>  gdb/debuginfod-support.c                      | 210 ++++++++++++++++++

>  gdb/debuginfod-support.h                      |  41 ++++

>  gdb/gcore.in                                  |   3 +

>  gdb/gdbarch.c                                 |   2 +-

>  gdb/gdbarch.h                                 |   4 +-

>  gdb/gdbarch.sh                                |   2 +-

>  gdb/linux-tdep.c                              |  34 ++-

>  gdb/solib.c                                   |  19 ++

>  .../gdb.debuginfod/fetch_src_and_symbols.exp  |  25 ++-

>  12 files changed, 376 insertions(+), 12 deletions(-)

>

> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c

> index c75a79757ea..44ffaa2c2c3 100644

> --- a/gdb/arch-utils.c

> +++ b/gdb/arch-utils.c

> @@ -1104,7 +1104,9 @@ default_read_core_file_mappings (struct gdbarch *gdbarch,

>                                                           ULONGEST start,

>                                                           ULONGEST end,

>                                                           ULONGEST file_ofs,

> -                                                         const char *filename)>

> +                                                         const char *filename,

> +                                                         const bfd_build_id

> +                                                           *build_id)>

>                                    loop_cb)

>  {

>  }

> diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h

> index a5b40ad8ff1..6c288f0fe92 100644

> --- a/gdb/arch-utils.h

> +++ b/gdb/arch-utils.h

> @@ -309,6 +309,8 @@ extern void default_read_core_file_mappings (struct gdbarch *gdbarch,

>                                                                       ULONGEST start,

>                                                                       ULONGEST end,

>                                                                       ULONGEST file_ofs,

> -                                                                     const char *filename)>

> +                                                                     const char *filename,

> +                                                                     const bfd_build_id

> +                                                                       *build_id)>

>                                                loop_cb);

>  #endif /* ARCH_UTILS_H */

> diff --git a/gdb/corelow.c b/gdb/corelow.c

> index a1943ab2ea6..9d060591b31 100644

> --- a/gdb/corelow.c

> +++ b/gdb/corelow.c

> @@ -46,6 +46,8 @@

>  #include "gdbsupport/filestuff.h"

>  #include "build-id.h"

>  #include "gdbsupport/pathstuff.h"

> +#include "gdbsupport/scoped_fd.h"

> +#include "debuginfod-support.h"

>  #include <unordered_map>

>  #include <unordered_set>

>  #include "gdbcmd.h"

> @@ -200,7 +202,7 @@ core_target::build_file_mappings ()

>      /* read_core_file_mappings will invoke this lambda for each mapping

>         that it finds.  */

>      [&] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs,

> -        const char *filename)

> +        const char *filename, const bfd_build_id *build_id)

>        {

>         /* Architecture-specific read_core_mapping methods are expected to

>            weed out non-file-backed mappings.  */

> @@ -215,6 +217,11 @@ core_target::build_file_mappings ()

>                canonical) pathname will be provided.  */

>             gdb::unique_xmalloc_ptr<char> expanded_fname

>               = exec_file_find (filename, NULL);

> +

> +           if (expanded_fname == nullptr && build_id != nullptr)

> +             debuginfod_exec_query (build_id->data, build_id->size,

> +                                    filename, &expanded_fname);

> +

>             if (expanded_fname == nullptr)

>               {

>                 m_core_unavailable_mappings.emplace_back (start, end - start);

> @@ -268,6 +275,10 @@ core_target::build_file_mappings ()

>

>         /* Set target_section fields.  */

>         m_core_file_mappings.emplace_back (start, end, sec);

> +

> +       /* If this bfd is of a shared library, record its soname and build id

> +          for debuginfod.  */

> +       debuginfod_set_soname_buildid (bfd, build_id);

>        });

>

>    normalize_mem_ranges (&m_core_unavailable_mappings);

> @@ -290,6 +301,7 @@ core_target::close ()

>        /* Clear out solib state while the bfd is still open.  See

>          comments in clear_solib in solib.c.  */

>        clear_solib ();

> +      debuginfod_clear_soname_buildids ();

>

>        current_program_space->cbfd.reset (nullptr);

>      }

> @@ -391,6 +403,32 @@ locate_exec_from_corefile_build_id (bfd *abfd, int from_tty)

>        symbol_file_add_main (bfd_get_filename (execbfd.get ()),

>                             symfile_add_flag (from_tty ? SYMFILE_VERBOSE : 0));

>      }

> +  else

> +    {

> +      gdb::unique_xmalloc_ptr<char> execpath;

> +      scoped_fd fd (debuginfod_exec_query (build_id->data, build_id->size,

> +                                          abfd->filename, &execpath));

> +

> +      if (fd.get () >= 0)

> +       {

> +         execbfd = gdb_bfd_open (execpath.get (), gnutarget);

> +

> +         if (execbfd == nullptr)

> +           warning (_("File \"%s\" from debuginfod cannot be opened as bfd"),

> +                    execpath.get ());

> +         else if (!build_id_verify (execbfd.get (), build_id->size,

> +                                    build_id->data))

> +           execbfd.reset (nullptr);

> +         else

> +           {

> +             /* Successful download */

> +             exec_file_attach (execpath.get (), from_tty);

> +             symbol_file_add_main (execpath.get (),

> +                                   symfile_add_flag (from_tty ?

> +                                                     SYMFILE_VERBOSE : 0));

> +           }

> +       }

> +    }

>  }

>

>  /* See gdbcore.h.  */

> diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c

> index 2d626e335a0..19f12feb0fd 100644

> --- a/gdb/debuginfod-support.c

> +++ b/gdb/debuginfod-support.c

> @@ -41,8 +41,41 @@ debuginfod_debuginfo_query (const unsigned char *build_id,

>  {

>    return scoped_fd (-ENOSYS);

>  }

> +

> +scoped_fd

> +debuginfod_exec_query (const unsigned char *build_id,

> +                      int build_id_len,

> +                      const char *filename,

> +                      gdb::unique_xmalloc_ptr<char> *destname)

> +{

> +  return scoped_fd (-ENOSYS);

> +}

> +

> +int

> +debuginfod_set_soname_buildid (struct bfd *bfd, const bfd_build_id *build_id)

> +{

> +  return 0;

> +}

> +

> +int

> +debuginfod_get_soname_buildid (char *soname, gdb::unique_xmalloc_ptr<char> *build_id_hex)

> +{

> +  return 0;

> +}

> +

> +void

> +debuginfod_clear_soname_buildids ()

> +{

> +  return;

> +}

>  #else

> +#include "elf-bfd.h"

> +#include "gdbcore.h"

> +#include <libgen.h>

>  #include <elfutils/debuginfod.h>

> +#include <unordered_map>

> +

> +static std::unordered_map<std::string, std::string> soname_to_buildid;

>

>  struct user_data

>  {

> @@ -194,4 +227,181 @@ debuginfod_debuginfo_query (const unsigned char *build_id,

>

>    return fd;

>  }

> +

> +/* See debuginfod-support.h  */

> +

> +scoped_fd

> +debuginfod_exec_query (const unsigned char *build_id,

> +                      int build_id_len,

> +                      const char *filename,

> +                      gdb::unique_xmalloc_ptr<char> *destname)

> +{

> +  const char *urls_env_var = getenv (DEBUGINFOD_URLS_ENV_VAR);

> +  if (urls_env_var == NULL || urls_env_var[0] == '\0')

> +    return scoped_fd (-ENOSYS);

> +

> +  debuginfod_client *c = get_debuginfod_client ();

> +

> +  if (c == nullptr)

> +    return scoped_fd (-ENOMEM);

> +

> +  char *dname = nullptr;

> +  user_data data ("executable for", filename);

> +

> +  debuginfod_set_user_data (c, &data);

> +  scoped_fd fd (debuginfod_find_executable (c, build_id, build_id_len, &dname));

> +  debuginfod_set_user_data (c, nullptr);

> +

> +  if (fd.get () < 0 && fd.get () != -ENOENT)

> +    printf_filtered (_("Download failed: %s.  Continuing without executable for %ps.\n"),

> +                    safe_strerror (-fd.get ()),

> +                    styled_string (file_name_style.style (),  filename));

> +

> +  if (fd.get () >= 0)

> +    destname->reset (dname);

> +

> +  return fd;

> +}

> +

> +/* Attempt to locate the DT_SONAME tag within ABFD's .dynamic section.

> +   If found copy the string referred to by the tag to SONAME and return 1.

> +   Otherwise return 0.  */

> +

> +static int

> +scan_soname_tag (struct bfd *abfd, std::string &soname)

> +{

> +  if (abfd == nullptr)

> +    return 0;

> +

> +  if (bfd_get_flavour (abfd) != bfd_target_elf_flavour)

> +    return 0;

> +

> +  int arch_size = bfd_get_arch_size (abfd);

> +  if (arch_size == -1)

> +    return 0;

> +

> +  /* Find the start address of the .dynamic section.  */

> +  struct bfd_section *sect = bfd_get_section_by_name (abfd, ".dynamic");

> +  if (sect == nullptr)

> +    return 0;

> +

> +  /* Read in .dynamic from the BFD.  */

> +  int sect_size = bfd_section_size (sect);

> +  gdb::byte_vector bufstart (sect_size);

> +

> +  if (!bfd_get_section_contents (abfd, sect, bufstart.data (), 0, sect_size))

> +    return 0;

> +

> +  gdb_byte *buf = bufstart.data ();

> +  struct bfd_section *dynstr = nullptr;

> +  CORE_ADDR soname_idx = 0;

> +  int step = (arch_size == 32) ? sizeof (Elf32_External_Dyn)

> +                              : sizeof (Elf64_External_Dyn);

> +

> +  /* Iterate over BUF and search for DT_SONAME and DT_STRTAB.  */

> +  for (gdb_byte *bufend = buf + sect_size;

> +       (buf < bufend) && (soname_idx == 0 || dynstr == nullptr);

> +       buf += step)

> +  {

> +    long current_dyntag;

> +    CORE_ADDR dyn_ptr;

> +

> +    if (arch_size == 32)

> +      {

> +       Elf32_External_Dyn *x_dynp_32 = (Elf32_External_Dyn *) buf;

> +       current_dyntag = bfd_h_get_32 (abfd, (bfd_byte *) x_dynp_32->d_tag);

> +       dyn_ptr = bfd_h_get_32 (abfd, (bfd_byte *) x_dynp_32->d_un.d_ptr);

> +      }

> +    else

> +      {

> +       Elf64_External_Dyn *x_dynp_64 = (Elf64_External_Dyn *) buf;

> +       current_dyntag = bfd_h_get_64 (abfd, (bfd_byte *) x_dynp_64->d_tag);

> +       dyn_ptr = bfd_h_get_64 (abfd, (bfd_byte *) x_dynp_64->d_un.d_ptr);

> +      }

> +

> +    if (current_dyntag == DT_NULL)

> +      buf = bufend;

> +    else if (current_dyntag == DT_SONAME)

> +      soname_idx = dyn_ptr;

> +    else if (current_dyntag == DT_STRTAB)

> +      for (struct bfd_section *s = abfd->sections; s != nullptr; s = s->next)

> +       if (s->vma == dyn_ptr)

> +         dynstr = s;

> +  }

> +

> +  if (soname_idx == 0 || dynstr == nullptr)

> +    return 0;

> +

> +  sect_size = bfd_section_size (dynstr);

> +  if (sect_size <= soname_idx)

> +    return 0;

> +

> +  /* Read the soname from the string table.  */

> +  gdb::char_vector dynstr_buf (sect_size);

> +  if (!bfd_get_section_contents (abfd, dynstr, dynstr_buf.data (), 0, sect_size))

> +    return 0;

> +

> +  soname = dynstr_buf.data () + soname_idx;

> +  return 1;

> +}

> +

> +/* See debuginfod-support.h  */

> +

> +int

> +debuginfod_set_soname_buildid (struct bfd *bfd, const bfd_build_id *build_id)

> +{

> +  if (bfd == nullptr || build_id == nullptr)

> +    return 0;

> +

> +  gdb_bfd_ref_ptr abfd (gdb_bfd_open (bfd->filename, gnutarget));

> +

> +  if (abfd == nullptr)

> +    return 0;

> +

> +  /* Check that bfd is an ET_DYN ELF file.  */

> +  bfd_check_format (abfd.get (), bfd_object);

> +  if (bfd_get_flavour (abfd.get ()) != bfd_target_elf_flavour

> +      || !(bfd_get_file_flags (abfd.get ()) & DYNAMIC))

> +    return 0;

> +

> +  std::string soname;

> +  std::string build_id_hex;

> +

> +  /* Determine soname of shared library.  If found map soname to build-id.  */

> +  if (scan_soname_tag (abfd.get (), soname))

> +    {

> +      for (int i = 0; i < build_id->size; i++)

> +       {

> +         build_id_hex += "0123456789abcdef"[build_id->data[i] >> 4];

> +         build_id_hex += "0123456789abcdef"[build_id->data[i] & 0xf];

> +       }

> +

> +      soname_to_buildid[soname] = build_id_hex;

> +      return 1;

> +    }

> +

> +  return 0;

> +}

> +

> +/* See debuginfod-support.h  */

> +

> +int

> +debuginfod_get_soname_buildid (char *soname, gdb::unique_xmalloc_ptr<char> *build_id_hex)

> +{

> +  auto it = soname_to_buildid.find (basename (soname));

> +  if (it == soname_to_buildid.end ())

> +    return 0;

> +

> +  build_id_hex->reset (xstrdup (it->second.c_str ()));

> +  return 1;

> +}

> +

> +/* See debuginfod-support.h  */

> +

> +void debuginfod_clear_soname_buildids ()

> +{

> +  soname_to_buildid.clear ();

> +  return;

> +}

> +

>  #endif

> diff --git a/gdb/debuginfod-support.h b/gdb/debuginfod-support.h

> index 5e5aab56e74..185c5be22fd 100644

> --- a/gdb/debuginfod-support.h

> +++ b/gdb/debuginfod-support.h

> @@ -61,4 +61,45 @@ debuginfod_debuginfo_query (const unsigned char *build_id,

>                             const char *filename,

>                             gdb::unique_xmalloc_ptr<char> *destname);

>

> +/* Query debuginfod servers for an executable file with BUILD_ID.

> +   BUILD_ID can be given as a binary blob or a null-terminated string.

> +   If given as a binary blob, BUILD_ID_LEN should be the number of bytes.

> +   If given as a null-terminated string, BUILD_ID_LEN should be 0.

> +

> +   FILENAME should be the name or path associated with the executable.

> +   It is used for printing messages to the user.

> +

> +   If the file is successfully retrieved, its path on the local machine

> +   is stored in DESTNAME.  If GDB is not built with debuginfod, this

> +   function returns -ENOSYS.  */

> +

> +extern scoped_fd debuginfod_exec_query (const unsigned char *build_id,

> +                                       int build_id_len,

> +                                       const char *filename,

> +                                       gdb::unique_xmalloc_ptr<char> *destname);

> +

> +/* If BFD is of an ELF shared library then associate its soname with

> +   BUILD_ID so that it can be retrieved with debuginfod_get_soname_buildid().

> +

> +   Return 1 if the soname was successully associated with BUILD-ID, otherwise

> +   return 0.  */

> +

> +extern int debuginfod_set_soname_buildid (struct bfd *bfd,

> +                                         const bfd_build_id *build_id);

> +

> +/* If SONAME had a build-id associated with it by a previous call to

> +   debuginfod_set_soname_buildid() then store the build-id in BUILD_ID_HEX

> +   as a NULL-terminated string.

> +

> +   Return 1 if SONAME's build-id was found and stored in BUILD_ID_HEX,

> +   otherwise return 0.  */

> +

> +extern int debuginfod_get_soname_buildid (char *soname,

> +                                         gdb::unique_xmalloc_ptr<char> *build_id_hex);

> +

> +/* Clear all soname-to-build-id mappings that have been created by

> +   debuginfod_set_soname_buildid.  */

> +

> +extern void debuginfod_clear_soname_buildids ();

> +

>  #endif /* DEBUGINFOD_SUPPORT_H */

> diff --git a/gdb/gcore.in b/gdb/gcore.in

> index 24354a79e27..25b24c3cd3d 100644

> --- a/gdb/gcore.in

> +++ b/gdb/gcore.in

> @@ -89,6 +89,9 @@ if [ ! -f "$binary_path/@GDB_TRANSFORM_NAME@" ]; then

>    exit 1

>  fi

>

> +# Prevent unnecessary debuginfod queries during core file generation.

> +unset DEBUGINFOD_URLS

> +

>  # Initialise return code.

>  rc=0

>

> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c

> index 208cf4b5aaa..8a2d88c5f96 100644

> --- a/gdb/gdbarch.c

> +++ b/gdb/gdbarch.c

> @@ -5411,7 +5411,7 @@ set_gdbarch_get_pc_address_flags (struct gdbarch *gdbarch,

>  }

>

>  void

> -gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename)> loop_cb)

> +gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const bfd_build_id *build_id)> loop_cb)

>  {

>    gdb_assert (gdbarch != NULL);

>    gdb_assert (gdbarch->read_core_file_mappings != NULL);

> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h

> index 7157e5596fd..ca7949bafb1 100644

> --- a/gdb/gdbarch.h

> +++ b/gdb/gdbarch.h

> @@ -1709,8 +1709,8 @@ extern void set_gdbarch_get_pc_address_flags (struct gdbarch *gdbarch, gdbarch_g

>

>  /* Read core file mappings */

>

> -typedef void (gdbarch_read_core_file_mappings_ftype) (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename)> loop_cb);

> -extern void gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename)> loop_cb);

> +typedef void (gdbarch_read_core_file_mappings_ftype) (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const bfd_build_id *build_id)> loop_cb);

> +extern void gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const bfd_build_id *build_id)> loop_cb);

>  extern void set_gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, gdbarch_read_core_file_mappings_ftype *read_core_file_mappings);

>

>  extern struct gdbarch_tdep *gdbarch_tdep (struct gdbarch *gdbarch);

> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh

> index 43e51341f97..ea651e8c0dc 100755

> --- a/gdb/gdbarch.sh

> +++ b/gdb/gdbarch.sh

> @@ -1209,7 +1209,7 @@ m;ULONGEST;type_align;struct type *type;type;;default_type_align;;0

>  f;std::string;get_pc_address_flags;frame_info *frame, CORE_ADDR pc;frame, pc;;default_get_pc_address_flags;;0

>

>  # Read core file mappings

> -m;void;read_core_file_mappings;struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename)> loop_cb;cbfd, pre_loop_cb, loop_cb;;default_read_core_file_mappings;;0

> +m;void;read_core_file_mappings;struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const bfd_build_id *build_id)> loop_cb;cbfd, pre_loop_cb, loop_cb;;default_read_core_file_mappings;;0

>

>  EOF

>  }

> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c

> index 927e69bf1e1..757b5bc02e9 100644

> --- a/gdb/linux-tdep.c

> +++ b/gdb/linux-tdep.c

> @@ -43,6 +43,7 @@

>  #include "gcore-elf.h"

>

>  #include <ctype.h>

> +#include <unordered_map>

>

>  /* This enum represents the values that the user can choose when

>     informing the Linux kernel about which memory mappings will be

> @@ -1104,7 +1105,8 @@ linux_read_core_file_mappings (struct gdbarch *gdbarch,

>                                                         ULONGEST start,

>                                                         ULONGEST end,

>                                                         ULONGEST file_ofs,

> -                                                       const char *filename)>

> +                                                       const char *filename,

> +                                                       const bfd_build_id *build_id)>

>                                  loop_cb)

>  {

>    /* Ensure that ULONGEST is big enough for reading 64-bit core files.  */

> @@ -1174,6 +1176,23 @@ linux_read_core_file_mappings (struct gdbarch *gdbarch,

>    if (f != descend)

>      warning (_("malformed note - filename area is too big"));

>

> +  const bfd_build_id *orig_build_id = cbfd->build_id;

> +  std::unordered_map<ULONGEST, const bfd_build_id *> vma_map;

> +  std::unordered_map<char *, const bfd_build_id *> filename_map;

> +

> +  /* Search for solib build-ids in the core file.  Each time one is found,

> +     map the start vma of the corresponding elf header to the build-id.  */

> +  for (bfd_section *sec = cbfd->sections; sec != nullptr; sec = sec->next)

> +    {

> +      cbfd->build_id = nullptr;

> +

> +      if (sec->flags & SEC_LOAD

> +         && get_elf_backend_data (cbfd)->elf_backend_core_find_build_id

> +              (cbfd, (bfd_vma) sec->filepos))

> +       vma_map[sec->vma] = cbfd->build_id;

> +    }

> +

> +  cbfd->build_id = orig_build_id;

>    pre_loop_cb (count);

>

>    for (int i = 0; i < count; i++)

> @@ -1187,8 +1206,17 @@ linux_read_core_file_mappings (struct gdbarch *gdbarch,

>        descdata += addr_size;

>        char * filename = filenames;

>        filenames += strlen ((char *) filenames) + 1;

> +      const bfd_build_id *build_id = nullptr;

> +

> +      /* Map filename to the build-id associated with this start vma,

> +        if such a build-id was found.  Otherwise use the build-id

> +        already associated with this filename if it exists. */

> +      if ((build_id = vma_map[start]) != nullptr)

> +       filename_map[filename] = build_id;

> +      else

> +       build_id = filename_map[filename];

>

> -      loop_cb (i, start, end, file_ofs, filename);

> +      loop_cb (i, start, end, file_ofs, filename, build_id);

>      }

>  }

>

> @@ -1217,7 +1245,7 @@ linux_core_info_proc_mappings (struct gdbarch *gdbarch, const char *args)

>           }

>        },

>      [=] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs,

> -        const char *filename)

> +        const char *filename, const bfd_build_id *build_id)

>        {

>         if (gdbarch_addr_bit (gdbarch) == 32)

>           printf_filtered ("\t%10s %10s %10s %10s %s\n",

> diff --git a/gdb/solib.c b/gdb/solib.c

> index 2df52118143..5f96d463aa4 100644

> --- a/gdb/solib.c

> +++ b/gdb/solib.c

> @@ -46,6 +46,8 @@

>  #include "filesystem.h"

>  #include "gdb_bfd.h"

>  #include "gdbsupport/filestuff.h"

> +#include "gdbsupport/scoped_fd.h"

> +#include "debuginfod-support.h"

>  #include "source.h"

>  #include "cli/cli-style.h"

>

> @@ -536,6 +538,23 @@ solib_map_sections (struct so_list *so)

>    gdb::unique_xmalloc_ptr<char> filename (tilde_expand (so->so_name));

>    gdb_bfd_ref_ptr abfd (ops->bfd_open (filename.get ()));

>

> +  if (abfd == NULL)

> +    {

> +      gdb::unique_xmalloc_ptr<char> build_id_hex;

> +

> +      /* If a build-id was previously associated with this soname

> +        then use it to query debuginfod for the library.  */

> +      if (debuginfod_get_soname_buildid (so->so_name, &build_id_hex))

> +       {

> +         scoped_fd fd = debuginfod_exec_query

> +                        ((const unsigned char*) build_id_hex.get (),

> +                         0, so->so_name, &filename);

> +

> +         if (fd.get () >= 0)

> +           abfd = ops->bfd_open (filename.get ());

> +       }

> +    }

> +

>    if (abfd == NULL)

>      return 0;

>

> diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp

> index 81d4791eb6d..79ec4e6f853 100644

> --- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp

> +++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp

> @@ -58,6 +58,11 @@ if { [gdb_compile "$sourcetmp" "$binfile" executable {debug}] != "" } {

>      return -1

>  }

>

> +if { [gdb_compile "$sourcetmp" "${binfile}2" executable {debug}] != "" } {

> +    fail "compile"

> +    return -1

> +}

> +

>  # Write some assembly that just has a .gnu_debugaltlink section.

>  # Copied from testsuite/gdb.dwarf2/dwzbuildid.exp.

>  proc write_just_debugaltlink {filename dwzname buildid} {

> @@ -109,8 +114,10 @@ proc write_dwarf_file {filename buildid {value 99}} {

>      }

>  }

>

> +set corefile "corefile"

> +

>  proc no_url { } {

> -    global binfile outputdir debugdir

> +    global binfile corefile outputdir debugdir

>

>      setenv DEBUGINFOD_URLS ""

>

> @@ -162,10 +169,20 @@ proc no_url { } {

>      gdb_test "file ${binfile}_alt.o" \

>         ".*could not find '.gnu_debugaltlink'.*" \

>         "file [file tail ${binfile}_alt.o]"

> +

> +    # Generate a core file and test that gdb cannot find the executable

> +    clean_restart ${binfile}2

> +    gdb_test "start" "Temporary breakpoint.*"

> +    gdb_test "generate-core-file $corefile" \

> +       "Saved corefile $corefile"

> +    file rename -force ${binfile}2 $debugdir

> +

> +    clean_restart

> +    gdb_test "core $corefile" ".*in ?? ().*"

>  }

>

>  proc local_url { } {

> -    global binfile outputdir db debugdir

> +    global binfile corefile outputdir db debugdir

>

>      # Find an unused port

>      set port 7999

> @@ -228,6 +245,10 @@ proc local_url { } {

>      gdb_test "file ${binfile}_alt.o" \

>         ".*Reading symbols from ${binfile}_alt.o\.\.\.*" \

>         "file [file tail ${binfile}_alt.o]"

> +

> +    # gdb should now find the executable file

> +    clean_restart

> +    gdb_test "core $corefile" ".*return 0.*"

>  }

>

>  set envlist \

> --

> 2.31.1

>
Tom de Vries via Gdb-patches July 13, 2021, 11 p.m. | #2
Ping

Thanks,
Aaron

On Wed, Jun 2, 2021 at 8:01 PM Aaron Merey via Gdb-patches
<gdb-patches@sourceware.org> wrote:
>

> Hi Tom,

>

> Thanks for the review, I've included some comments below as well as v2

> the patch based on your feedback.

>

> On Wed, May 26, 2021 at 4:43 PM Tom Tromey <tom@tromey.com> wrote:

> > Aaron> +      gdb::unique_xmalloc_ptr<char> execpath;

> > Aaron> +      scoped_fd fd (debuginfod_exec_query (build_id->data, build_id->size,

> > Aaron> +                                           abfd->filename, &execpath));

> > Aaron> +

> > Aaron> +      if (fd.get () >= 0)

> > Aaron> +        {

> > Aaron> +          execbfd = gdb_bfd_open (execpath.get (), gnutarget);

> >

> > Does 'fd' already refer to this same file?  If so it can just be reused,

> > I think, by passing 'fd.release ()' to gdb_bfd_open.

> >

> > Or maybe there's some reason not to do this?  If so just let me know.

>

> I didn't do this because when an fd is passed to gdb_bfd_open the resulting

> bfd has a null build_id. Not sure if this is intended behavior.

>

> > Aaron> +static std::unordered_map<std::string, std::string> soname_to_buildid;

> >

> > It seems like there ought to be some spot where this map is cleared.

> > For example, it's not uncommon to rebuild a program and then re-"run" it

> > in an existing gdb session.  In this situation the mapping could change.

>

> Added a function debuginfod_clear_soname_buildids to do this. It gets

> called in core_target::close.

>

> > Aaron> +

> > Aaron> +static int

> > Aaron> +scan_soname_tag (struct bfd *abfd, char **soname)

> > Aaron> +{

> >

> > This should have an intro comment explaining its purpose, the return

> > value, and parameters.

>

> Done.

>

> > Aaron> +  /* Read in .dynamic and .dynstr from the BFD.  */

> > Aaron> +  int sect_size = bfd_section_size (sect);

> > Aaron> +  gdb_byte *bufstart = (gdb_byte *) alloca (sect_size);

> >

> > I think we should avoid unbounded use of alloca.

> > A gdb::byte_vector would be fine.

>

> Done.

>

> > Aaron> +  /* Read the soname from the string table.  */

> > Aaron> +  buf = (gdb_byte *) (xmalloc (sect_size));

> > Aaron> +  if (!bfd_get_section_contents (abfd, dynstr, buf, 0, sect_size)) {

> > Aaron> +    return 0;

> > Aaron> +  }

> > Aaron> +

> > Aaron> +  *soname = (char *) (buf + soname_idx);

> >

> > I think this leaks 'buf'.

>

> Fixed by using a gdb::char_vector instead of xmalloc.

>

> > Aaron> +int

> > Aaron> +debuginfod_set_soname_buildid (struct bfd *bfd, const bfd_build_id *build_id)

> > Aaron> +{

> > Aaron> +  gdb_bfd_ref_ptr abfd (gdb_bfd_open (bfd->filename, gnutarget));

> > Aaron> +  bfd_check_format (abfd.get (), bfd_object);

> >

> > Does this work ok if abfd == nullptr?

>

> Added an early return when abfd == nullptr.

>

> > Aaron> +

> > Aaron> +  if (build_id == nullptr)

> > Aaron> +    return 0;

> >

> > Seems like this check could be hoisted above opening the BFD.

>

> Done.

>

> > Aaron> +extern scoped_fd

> > Aaron> +debuginfod_exec_query (const unsigned char *build_id,

> > Aaron> +                       int build_id_len,

> > Aaron> +                       const char *filename,

> > Aaron> +                       gdb::unique_xmalloc_ptr<char> *destname);

> >

> > gdb style is to put the type on the same line as the name in

> > declarations (but not in definitions).

>

> Done.

>

> > Aaron> +      /* If a build-id was previously associated with this soname

> > Aaron> +         then use it to query debuginfod for the library.  */

> > Aaron> +      if (debuginfod_get_soname_buildid (so->so_name, &buildid_hexstr))

> > Aaron> +        {

> > Aaron> +          scoped_fd fd = debuginfod_exec_query

> > Aaron> +                          ((const unsigned char*) buildid_hexstr.get (),

> > Aaron> +                           0, so->so_name, &filename);

> >

> > I didn't really track through this too well, but seeing this comment

> > made me wonder if the build-id should just be directly attached to the

> > solib.

>

> The resulting solib bfd (execbfd) already ends up containing the build-id.

>

>

> One other point I should mention is this debuginfod support for downloading

> solibs referenced in core files depends on the presence of proc file mappings

> in the core file. If no mappings can be found then there will be no soname

> build-id information available in solib_map_sections, which is needed to query

> debuginfod. (Debuginfo and source queries for local solibs would still work

> properly though.)

>

> It may be possible to support solib downloading for core files without proc

> mappings but it could get a bit tricky. We could still search each ELF section

> of the core file for a build-id but we wouldn't be able to tell which filename

> it belongs to without the proc mappings. To get the missing solibs we could

> query debuginfod for binaries with build-ids that are present in the core file

> but were not seen among the solibs that GDB found locally.

>

> The problem is that debuginfod would have to indiscriminately download all of

> the binaries associated with these build-ids when there is a chance some still

> might be found locally. This could happen when a missing library links to

> another library which is present on the local machine. In this case debuginfod

> would download this library before GDB has tried to look for it.

>

> AFAIK it's only valgrind's vgcore files which don't currently include proc

> file mappings. I might approach the valgrind folks about adding proc file

> mappings to vgcore files.

>

> Anyways I just wanted to make sure this proc file mapping dependency was

> made clear.

>

> Thanks,

> Aaron

>

> ---

>

> From 0f874e434f37f7e24798a5da17b4dbabb2e46b60 Mon Sep 17 00:00:00 2001

> From: Aaron Merey <amerey@redhat.com>

> Date: Wed, 2 Jun 2021 17:41:26 -0400

> Subject: [PATCH] [PR gdb/27570] missing support for debuginfod in

>  core_target::build_file_mappings

>

> Add debuginfod support at various stages of core file processing to enable

> the downloading of missing executables and shared libraries from debuginfod

> servers.

>

> In order to perform these queries debuginfod requires the build-id of the

> missing executable or shared library. These are read from the core file in

> linux_read_core_file_mappings and passed to core_target::build_file_mappings

> where the query occurs if the file that backs the core file mapping cannot be

> found.

>

> GDB will attempt to open shared libraries found in core file mappings during

> solib_map_sections even if they were already opened during

> core_target::build_file_mappings. This presented a problem since libraries

> downloaded from debuginfod won't be found at the path which solib_map_sections

> attempts to open. They will instead be stored in the local debuginfod cache.

>

> We already have a BFD handle for these downloaded libraries opened in

> core_target::build_file_mappings but there is no easy way to identify and

> reuse the correct BFD during solib_map_sections. This is because the filename

> used to open the library in solib_map_sections may differ from the filename

> used in core_target::build_file_mappings, even though they refer to the same

> shared object. This difference can be seen by comparing library filenames from

> 'info proc mapping' with those from 'info sharedlibrary' (for example,

> "libc-2.32.so" vs "libc.so.6").

>

> Therefore GDB needs a way to associate library filenames used in

> solib_map_sections with the corresponding build-id from the core file mappings

> in case it needs to reopen the library from the local debuginfod cache. This

> patch adds facilities to debuginfod-support.c to make this possible.

>

> We populate a map of sonames to build-ids during core_target::build_file_mappings

> by using the new function debuginfod_set_soname_buildid. The filenames used in

> solib_map_sections originate from each library's DT_SONAME tag in its .dynamic

> segment so we added another function scan_soname_tag (which is modelled after

> solib-svr4.c:scan_dyntag) that will lookup the value of DT_SONAME in the dynamic

> string table segment of the library. This name gets mapped to the corresponding

> build-id. Then using the new function debuginfod_get_soname_buildid we can

> retrieve the desired build-id in solib_map_sections and check the debuginfod

> cache in case the library can't otherwise be found.

>

> Tested on Fedora 33 x86_64.

>

> gdb/ChangeLog:

>

>         PR gdb/27570

>         * arch-utils.c (default_read_core_file_mappings): Add build_id

>         parameter.

>         * arch-utils.h (default_read_core_file_mappings): Add build_id

>         parameter.

>         * corelow.c (core_target::build_file_mappings): Add debuginfod

>         executable query and map library soname to build-id.

>         (core_target::close): Call debuginfod_clear_soname_buildids.

>         (locate_exec_from_corefile_build_id): Add debuginfod executable query

>         * debuginfod-support.c (debuginfod_exec_query): New function.

>         (debuginfod_set_soname_buildid): New function.

>         (debuginfod_get_soname_buildid): New function.

>         (debuginfod_clear_soname_buildids): New function.

>         (scan_soname_tag): New function.

>         * debuginfod-support.h (debuginfod_exec_query): New function.

>         (debuginfod_set_soname_buildid): New function.

>         (debuginfod_get_soname_buildid): New function.

>         (debuginfod_clear_soname_buildids): New function.

>         * gcore.in: unset $DEBUGINFOD_URLS.

>         * gdbarch.c: Regenerate.

>         * gdbarch.h: Regenerate.

>         * gdbarch.sh (read_core_file_mappings): Add build_id parameter.

>         * linux-tdep.c (linux_read_core_file_mappings): Map shared library

>         filenames in core file mappings to build-id.

>         (linux_core_info_proc_mappings): Add build-id parameter.

>         * solib.c (solib_map_sections): Add debuginfod executable query.

>

> gdb/testsuite/ChangeLog:

>

>         PR gdb/27570

>         * gdb.debuginfod/fetch_src_and_symbols.exp: Add new testcases.

> ---

>  gdb/arch-utils.c                              |   4 +-

>  gdb/arch-utils.h                              |   4 +-

>  gdb/corelow.c                                 |  40 +++-

>  gdb/debuginfod-support.c                      | 210 ++++++++++++++++++

>  gdb/debuginfod-support.h                      |  41 ++++

>  gdb/gcore.in                                  |   3 +

>  gdb/gdbarch.c                                 |   2 +-

>  gdb/gdbarch.h                                 |   4 +-

>  gdb/gdbarch.sh                                |   2 +-

>  gdb/linux-tdep.c                              |  34 ++-

>  gdb/solib.c                                   |  19 ++

>  .../gdb.debuginfod/fetch_src_and_symbols.exp  |  25 ++-

>  12 files changed, 376 insertions(+), 12 deletions(-)

>

> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c

> index c75a79757ea..44ffaa2c2c3 100644

> --- a/gdb/arch-utils.c

> +++ b/gdb/arch-utils.c

> @@ -1104,7 +1104,9 @@ default_read_core_file_mappings (struct gdbarch *gdbarch,

>                                                           ULONGEST start,

>                                                           ULONGEST end,

>                                                           ULONGEST file_ofs,

> -                                                         const char *filename)>

> +                                                         const char *filename,

> +                                                         const bfd_build_id

> +                                                           *build_id)>

>                                    loop_cb)

>  {

>  }

> diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h

> index a5b40ad8ff1..6c288f0fe92 100644

> --- a/gdb/arch-utils.h

> +++ b/gdb/arch-utils.h

> @@ -309,6 +309,8 @@ extern void default_read_core_file_mappings (struct gdbarch *gdbarch,

>                                                                       ULONGEST start,

>                                                                       ULONGEST end,

>                                                                       ULONGEST file_ofs,

> -                                                                     const char *filename)>

> +                                                                     const char *filename,

> +                                                                     const bfd_build_id

> +                                                                       *build_id)>

>                                                loop_cb);

>  #endif /* ARCH_UTILS_H */

> diff --git a/gdb/corelow.c b/gdb/corelow.c

> index a1943ab2ea6..9d060591b31 100644

> --- a/gdb/corelow.c

> +++ b/gdb/corelow.c

> @@ -46,6 +46,8 @@

>  #include "gdbsupport/filestuff.h"

>  #include "build-id.h"

>  #include "gdbsupport/pathstuff.h"

> +#include "gdbsupport/scoped_fd.h"

> +#include "debuginfod-support.h"

>  #include <unordered_map>

>  #include <unordered_set>

>  #include "gdbcmd.h"

> @@ -200,7 +202,7 @@ core_target::build_file_mappings ()

>      /* read_core_file_mappings will invoke this lambda for each mapping

>         that it finds.  */

>      [&] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs,

> -        const char *filename)

> +        const char *filename, const bfd_build_id *build_id)

>        {

>         /* Architecture-specific read_core_mapping methods are expected to

>            weed out non-file-backed mappings.  */

> @@ -215,6 +217,11 @@ core_target::build_file_mappings ()

>                canonical) pathname will be provided.  */

>             gdb::unique_xmalloc_ptr<char> expanded_fname

>               = exec_file_find (filename, NULL);

> +

> +           if (expanded_fname == nullptr && build_id != nullptr)

> +             debuginfod_exec_query (build_id->data, build_id->size,

> +                                    filename, &expanded_fname);

> +

>             if (expanded_fname == nullptr)

>               {

>                 m_core_unavailable_mappings.emplace_back (start, end - start);

> @@ -268,6 +275,10 @@ core_target::build_file_mappings ()

>

>         /* Set target_section fields.  */

>         m_core_file_mappings.emplace_back (start, end, sec);

> +

> +       /* If this bfd is of a shared library, record its soname and build id

> +          for debuginfod.  */

> +       debuginfod_set_soname_buildid (bfd, build_id);

>        });

>

>    normalize_mem_ranges (&m_core_unavailable_mappings);

> @@ -290,6 +301,7 @@ core_target::close ()

>        /* Clear out solib state while the bfd is still open.  See

>          comments in clear_solib in solib.c.  */

>        clear_solib ();

> +      debuginfod_clear_soname_buildids ();

>

>        current_program_space->cbfd.reset (nullptr);

>      }

> @@ -391,6 +403,32 @@ locate_exec_from_corefile_build_id (bfd *abfd, int from_tty)

>        symbol_file_add_main (bfd_get_filename (execbfd.get ()),

>                             symfile_add_flag (from_tty ? SYMFILE_VERBOSE : 0));

>      }

> +  else

> +    {

> +      gdb::unique_xmalloc_ptr<char> execpath;

> +      scoped_fd fd (debuginfod_exec_query (build_id->data, build_id->size,

> +                                          abfd->filename, &execpath));

> +

> +      if (fd.get () >= 0)

> +       {

> +         execbfd = gdb_bfd_open (execpath.get (), gnutarget);

> +

> +         if (execbfd == nullptr)

> +           warning (_("File \"%s\" from debuginfod cannot be opened as bfd"),

> +                    execpath.get ());

> +         else if (!build_id_verify (execbfd.get (), build_id->size,

> +                                    build_id->data))

> +           execbfd.reset (nullptr);

> +         else

> +           {

> +             /* Successful download */

> +             exec_file_attach (execpath.get (), from_tty);

> +             symbol_file_add_main (execpath.get (),

> +                                   symfile_add_flag (from_tty ?

> +                                                     SYMFILE_VERBOSE : 0));

> +           }

> +       }

> +    }

>  }

>

>  /* See gdbcore.h.  */

> diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c

> index 2d626e335a0..19f12feb0fd 100644

> --- a/gdb/debuginfod-support.c

> +++ b/gdb/debuginfod-support.c

> @@ -41,8 +41,41 @@ debuginfod_debuginfo_query (const unsigned char *build_id,

>  {

>    return scoped_fd (-ENOSYS);

>  }

> +

> +scoped_fd

> +debuginfod_exec_query (const unsigned char *build_id,

> +                      int build_id_len,

> +                      const char *filename,

> +                      gdb::unique_xmalloc_ptr<char> *destname)

> +{

> +  return scoped_fd (-ENOSYS);

> +}

> +

> +int

> +debuginfod_set_soname_buildid (struct bfd *bfd, const bfd_build_id *build_id)

> +{

> +  return 0;

> +}

> +

> +int

> +debuginfod_get_soname_buildid (char *soname, gdb::unique_xmalloc_ptr<char> *build_id_hex)

> +{

> +  return 0;

> +}

> +

> +void

> +debuginfod_clear_soname_buildids ()

> +{

> +  return;

> +}

>  #else

> +#include "elf-bfd.h"

> +#include "gdbcore.h"

> +#include <libgen.h>

>  #include <elfutils/debuginfod.h>

> +#include <unordered_map>

> +

> +static std::unordered_map<std::string, std::string> soname_to_buildid;

>

>  struct user_data

>  {

> @@ -194,4 +227,181 @@ debuginfod_debuginfo_query (const unsigned char *build_id,

>

>    return fd;

>  }

> +

> +/* See debuginfod-support.h  */

> +

> +scoped_fd

> +debuginfod_exec_query (const unsigned char *build_id,

> +                      int build_id_len,

> +                      const char *filename,

> +                      gdb::unique_xmalloc_ptr<char> *destname)

> +{

> +  const char *urls_env_var = getenv (DEBUGINFOD_URLS_ENV_VAR);

> +  if (urls_env_var == NULL || urls_env_var[0] == '\0')

> +    return scoped_fd (-ENOSYS);

> +

> +  debuginfod_client *c = get_debuginfod_client ();

> +

> +  if (c == nullptr)

> +    return scoped_fd (-ENOMEM);

> +

> +  char *dname = nullptr;

> +  user_data data ("executable for", filename);

> +

> +  debuginfod_set_user_data (c, &data);

> +  scoped_fd fd (debuginfod_find_executable (c, build_id, build_id_len, &dname));

> +  debuginfod_set_user_data (c, nullptr);

> +

> +  if (fd.get () < 0 && fd.get () != -ENOENT)

> +    printf_filtered (_("Download failed: %s.  Continuing without executable for %ps.\n"),

> +                    safe_strerror (-fd.get ()),

> +                    styled_string (file_name_style.style (),  filename));

> +

> +  if (fd.get () >= 0)

> +    destname->reset (dname);

> +

> +  return fd;

> +}

> +

> +/* Attempt to locate the DT_SONAME tag within ABFD's .dynamic section.

> +   If found copy the string referred to by the tag to SONAME and return 1.

> +   Otherwise return 0.  */

> +

> +static int

> +scan_soname_tag (struct bfd *abfd, std::string &soname)

> +{

> +  if (abfd == nullptr)

> +    return 0;

> +

> +  if (bfd_get_flavour (abfd) != bfd_target_elf_flavour)

> +    return 0;

> +

> +  int arch_size = bfd_get_arch_size (abfd);

> +  if (arch_size == -1)

> +    return 0;

> +

> +  /* Find the start address of the .dynamic section.  */

> +  struct bfd_section *sect = bfd_get_section_by_name (abfd, ".dynamic");

> +  if (sect == nullptr)

> +    return 0;

> +

> +  /* Read in .dynamic from the BFD.  */

> +  int sect_size = bfd_section_size (sect);

> +  gdb::byte_vector bufstart (sect_size);

> +

> +  if (!bfd_get_section_contents (abfd, sect, bufstart.data (), 0, sect_size))

> +    return 0;

> +

> +  gdb_byte *buf = bufstart.data ();

> +  struct bfd_section *dynstr = nullptr;

> +  CORE_ADDR soname_idx = 0;

> +  int step = (arch_size == 32) ? sizeof (Elf32_External_Dyn)

> +                              : sizeof (Elf64_External_Dyn);

> +

> +  /* Iterate over BUF and search for DT_SONAME and DT_STRTAB.  */

> +  for (gdb_byte *bufend = buf + sect_size;

> +       (buf < bufend) && (soname_idx == 0 || dynstr == nullptr);

> +       buf += step)

> +  {

> +    long current_dyntag;

> +    CORE_ADDR dyn_ptr;

> +

> +    if (arch_size == 32)

> +      {

> +       Elf32_External_Dyn *x_dynp_32 = (Elf32_External_Dyn *) buf;

> +       current_dyntag = bfd_h_get_32 (abfd, (bfd_byte *) x_dynp_32->d_tag);

> +       dyn_ptr = bfd_h_get_32 (abfd, (bfd_byte *) x_dynp_32->d_un.d_ptr);

> +      }

> +    else

> +      {

> +       Elf64_External_Dyn *x_dynp_64 = (Elf64_External_Dyn *) buf;

> +       current_dyntag = bfd_h_get_64 (abfd, (bfd_byte *) x_dynp_64->d_tag);

> +       dyn_ptr = bfd_h_get_64 (abfd, (bfd_byte *) x_dynp_64->d_un.d_ptr);

> +      }

> +

> +    if (current_dyntag == DT_NULL)

> +      buf = bufend;

> +    else if (current_dyntag == DT_SONAME)

> +      soname_idx = dyn_ptr;

> +    else if (current_dyntag == DT_STRTAB)

> +      for (struct bfd_section *s = abfd->sections; s != nullptr; s = s->next)

> +       if (s->vma == dyn_ptr)

> +         dynstr = s;

> +  }

> +

> +  if (soname_idx == 0 || dynstr == nullptr)

> +    return 0;

> +

> +  sect_size = bfd_section_size (dynstr);

> +  if (sect_size <= soname_idx)

> +    return 0;

> +

> +  /* Read the soname from the string table.  */

> +  gdb::char_vector dynstr_buf (sect_size);

> +  if (!bfd_get_section_contents (abfd, dynstr, dynstr_buf.data (), 0, sect_size))

> +    return 0;

> +

> +  soname = dynstr_buf.data () + soname_idx;

> +  return 1;

> +}

> +

> +/* See debuginfod-support.h  */

> +

> +int

> +debuginfod_set_soname_buildid (struct bfd *bfd, const bfd_build_id *build_id)

> +{

> +  if (bfd == nullptr || build_id == nullptr)

> +    return 0;

> +

> +  gdb_bfd_ref_ptr abfd (gdb_bfd_open (bfd->filename, gnutarget));

> +

> +  if (abfd == nullptr)

> +    return 0;

> +

> +  /* Check that bfd is an ET_DYN ELF file.  */

> +  bfd_check_format (abfd.get (), bfd_object);

> +  if (bfd_get_flavour (abfd.get ()) != bfd_target_elf_flavour

> +      || !(bfd_get_file_flags (abfd.get ()) & DYNAMIC))

> +    return 0;

> +

> +  std::string soname;

> +  std::string build_id_hex;

> +

> +  /* Determine soname of shared library.  If found map soname to build-id.  */

> +  if (scan_soname_tag (abfd.get (), soname))

> +    {

> +      for (int i = 0; i < build_id->size; i++)

> +       {

> +         build_id_hex += "0123456789abcdef"[build_id->data[i] >> 4];

> +         build_id_hex += "0123456789abcdef"[build_id->data[i] & 0xf];

> +       }

> +

> +      soname_to_buildid[soname] = build_id_hex;

> +      return 1;

> +    }

> +

> +  return 0;

> +}

> +

> +/* See debuginfod-support.h  */

> +

> +int

> +debuginfod_get_soname_buildid (char *soname, gdb::unique_xmalloc_ptr<char> *build_id_hex)

> +{

> +  auto it = soname_to_buildid.find (basename (soname));

> +  if (it == soname_to_buildid.end ())

> +    return 0;

> +

> +  build_id_hex->reset (xstrdup (it->second.c_str ()));

> +  return 1;

> +}

> +

> +/* See debuginfod-support.h  */

> +

> +void debuginfod_clear_soname_buildids ()

> +{

> +  soname_to_buildid.clear ();

> +  return;

> +}

> +

>  #endif

> diff --git a/gdb/debuginfod-support.h b/gdb/debuginfod-support.h

> index 5e5aab56e74..185c5be22fd 100644

> --- a/gdb/debuginfod-support.h

> +++ b/gdb/debuginfod-support.h

> @@ -61,4 +61,45 @@ debuginfod_debuginfo_query (const unsigned char *build_id,

>                             const char *filename,

>                             gdb::unique_xmalloc_ptr<char> *destname);

>

> +/* Query debuginfod servers for an executable file with BUILD_ID.

> +   BUILD_ID can be given as a binary blob or a null-terminated string.

> +   If given as a binary blob, BUILD_ID_LEN should be the number of bytes.

> +   If given as a null-terminated string, BUILD_ID_LEN should be 0.

> +

> +   FILENAME should be the name or path associated with the executable.

> +   It is used for printing messages to the user.

> +

> +   If the file is successfully retrieved, its path on the local machine

> +   is stored in DESTNAME.  If GDB is not built with debuginfod, this

> +   function returns -ENOSYS.  */

> +

> +extern scoped_fd debuginfod_exec_query (const unsigned char *build_id,

> +                                       int build_id_len,

> +                                       const char *filename,

> +                                       gdb::unique_xmalloc_ptr<char> *destname);

> +

> +/* If BFD is of an ELF shared library then associate its soname with

> +   BUILD_ID so that it can be retrieved with debuginfod_get_soname_buildid().

> +

> +   Return 1 if the soname was successully associated with BUILD-ID, otherwise

> +   return 0.  */

> +

> +extern int debuginfod_set_soname_buildid (struct bfd *bfd,

> +                                         const bfd_build_id *build_id);

> +

> +/* If SONAME had a build-id associated with it by a previous call to

> +   debuginfod_set_soname_buildid() then store the build-id in BUILD_ID_HEX

> +   as a NULL-terminated string.

> +

> +   Return 1 if SONAME's build-id was found and stored in BUILD_ID_HEX,

> +   otherwise return 0.  */

> +

> +extern int debuginfod_get_soname_buildid (char *soname,

> +                                         gdb::unique_xmalloc_ptr<char> *build_id_hex);

> +

> +/* Clear all soname-to-build-id mappings that have been created by

> +   debuginfod_set_soname_buildid.  */

> +

> +extern void debuginfod_clear_soname_buildids ();

> +

>  #endif /* DEBUGINFOD_SUPPORT_H */

> diff --git a/gdb/gcore.in b/gdb/gcore.in

> index 24354a79e27..25b24c3cd3d 100644

> --- a/gdb/gcore.in

> +++ b/gdb/gcore.in

> @@ -89,6 +89,9 @@ if [ ! -f "$binary_path/@GDB_TRANSFORM_NAME@" ]; then

>    exit 1

>  fi

>

> +# Prevent unnecessary debuginfod queries during core file generation.

> +unset DEBUGINFOD_URLS

> +

>  # Initialise return code.

>  rc=0

>

> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c

> index 208cf4b5aaa..8a2d88c5f96 100644

> --- a/gdb/gdbarch.c

> +++ b/gdb/gdbarch.c

> @@ -5411,7 +5411,7 @@ set_gdbarch_get_pc_address_flags (struct gdbarch *gdbarch,

>  }

>

>  void

> -gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename)> loop_cb)

> +gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const bfd_build_id *build_id)> loop_cb)

>  {

>    gdb_assert (gdbarch != NULL);

>    gdb_assert (gdbarch->read_core_file_mappings != NULL);

> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h

> index 7157e5596fd..ca7949bafb1 100644

> --- a/gdb/gdbarch.h

> +++ b/gdb/gdbarch.h

> @@ -1709,8 +1709,8 @@ extern void set_gdbarch_get_pc_address_flags (struct gdbarch *gdbarch, gdbarch_g

>

>  /* Read core file mappings */

>

> -typedef void (gdbarch_read_core_file_mappings_ftype) (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename)> loop_cb);

> -extern void gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename)> loop_cb);

> +typedef void (gdbarch_read_core_file_mappings_ftype) (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const bfd_build_id *build_id)> loop_cb);

> +extern void gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const bfd_build_id *build_id)> loop_cb);

>  extern void set_gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, gdbarch_read_core_file_mappings_ftype *read_core_file_mappings);

>

>  extern struct gdbarch_tdep *gdbarch_tdep (struct gdbarch *gdbarch);

> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh

> index 43e51341f97..ea651e8c0dc 100755

> --- a/gdb/gdbarch.sh

> +++ b/gdb/gdbarch.sh

> @@ -1209,7 +1209,7 @@ m;ULONGEST;type_align;struct type *type;type;;default_type_align;;0

>  f;std::string;get_pc_address_flags;frame_info *frame, CORE_ADDR pc;frame, pc;;default_get_pc_address_flags;;0

>

>  # Read core file mappings

> -m;void;read_core_file_mappings;struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename)> loop_cb;cbfd, pre_loop_cb, loop_cb;;default_read_core_file_mappings;;0

> +m;void;read_core_file_mappings;struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const bfd_build_id *build_id)> loop_cb;cbfd, pre_loop_cb, loop_cb;;default_read_core_file_mappings;;0

>

>  EOF

>  }

> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c

> index 927e69bf1e1..757b5bc02e9 100644

> --- a/gdb/linux-tdep.c

> +++ b/gdb/linux-tdep.c

> @@ -43,6 +43,7 @@

>  #include "gcore-elf.h"

>

>  #include <ctype.h>

> +#include <unordered_map>

>

>  /* This enum represents the values that the user can choose when

>     informing the Linux kernel about which memory mappings will be

> @@ -1104,7 +1105,8 @@ linux_read_core_file_mappings (struct gdbarch *gdbarch,

>                                                         ULONGEST start,

>                                                         ULONGEST end,

>                                                         ULONGEST file_ofs,

> -                                                       const char *filename)>

> +                                                       const char *filename,

> +                                                       const bfd_build_id *build_id)>

>                                  loop_cb)

>  {

>    /* Ensure that ULONGEST is big enough for reading 64-bit core files.  */

> @@ -1174,6 +1176,23 @@ linux_read_core_file_mappings (struct gdbarch *gdbarch,

>    if (f != descend)

>      warning (_("malformed note - filename area is too big"));

>

> +  const bfd_build_id *orig_build_id = cbfd->build_id;

> +  std::unordered_map<ULONGEST, const bfd_build_id *> vma_map;

> +  std::unordered_map<char *, const bfd_build_id *> filename_map;

> +

> +  /* Search for solib build-ids in the core file.  Each time one is found,

> +     map the start vma of the corresponding elf header to the build-id.  */

> +  for (bfd_section *sec = cbfd->sections; sec != nullptr; sec = sec->next)

> +    {

> +      cbfd->build_id = nullptr;

> +

> +      if (sec->flags & SEC_LOAD

> +         && get_elf_backend_data (cbfd)->elf_backend_core_find_build_id

> +              (cbfd, (bfd_vma) sec->filepos))

> +       vma_map[sec->vma] = cbfd->build_id;

> +    }

> +

> +  cbfd->build_id = orig_build_id;

>    pre_loop_cb (count);

>

>    for (int i = 0; i < count; i++)

> @@ -1187,8 +1206,17 @@ linux_read_core_file_mappings (struct gdbarch *gdbarch,

>        descdata += addr_size;

>        char * filename = filenames;

>        filenames += strlen ((char *) filenames) + 1;

> +      const bfd_build_id *build_id = nullptr;

> +

> +      /* Map filename to the build-id associated with this start vma,

> +        if such a build-id was found.  Otherwise use the build-id

> +        already associated with this filename if it exists. */

> +      if ((build_id = vma_map[start]) != nullptr)

> +       filename_map[filename] = build_id;

> +      else

> +       build_id = filename_map[filename];

>

> -      loop_cb (i, start, end, file_ofs, filename);

> +      loop_cb (i, start, end, file_ofs, filename, build_id);

>      }

>  }

>

> @@ -1217,7 +1245,7 @@ linux_core_info_proc_mappings (struct gdbarch *gdbarch, const char *args)

>           }

>        },

>      [=] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs,

> -        const char *filename)

> +        const char *filename, const bfd_build_id *build_id)

>        {

>         if (gdbarch_addr_bit (gdbarch) == 32)

>           printf_filtered ("\t%10s %10s %10s %10s %s\n",

> diff --git a/gdb/solib.c b/gdb/solib.c

> index 2df52118143..5f96d463aa4 100644

> --- a/gdb/solib.c

> +++ b/gdb/solib.c

> @@ -46,6 +46,8 @@

>  #include "filesystem.h"

>  #include "gdb_bfd.h"

>  #include "gdbsupport/filestuff.h"

> +#include "gdbsupport/scoped_fd.h"

> +#include "debuginfod-support.h"

>  #include "source.h"

>  #include "cli/cli-style.h"

>

> @@ -536,6 +538,23 @@ solib_map_sections (struct so_list *so)

>    gdb::unique_xmalloc_ptr<char> filename (tilde_expand (so->so_name));

>    gdb_bfd_ref_ptr abfd (ops->bfd_open (filename.get ()));

>

> +  if (abfd == NULL)

> +    {

> +      gdb::unique_xmalloc_ptr<char> build_id_hex;

> +

> +      /* If a build-id was previously associated with this soname

> +        then use it to query debuginfod for the library.  */

> +      if (debuginfod_get_soname_buildid (so->so_name, &build_id_hex))

> +       {

> +         scoped_fd fd = debuginfod_exec_query

> +                        ((const unsigned char*) build_id_hex.get (),

> +                         0, so->so_name, &filename);

> +

> +         if (fd.get () >= 0)

> +           abfd = ops->bfd_open (filename.get ());

> +       }

> +    }

> +

>    if (abfd == NULL)

>      return 0;

>

> diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp

> index 81d4791eb6d..79ec4e6f853 100644

> --- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp

> +++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp

> @@ -58,6 +58,11 @@ if { [gdb_compile "$sourcetmp" "$binfile" executable {debug}] != "" } {

>      return -1

>  }

>

> +if { [gdb_compile "$sourcetmp" "${binfile}2" executable {debug}] != "" } {

> +    fail "compile"

> +    return -1

> +}

> +

>  # Write some assembly that just has a .gnu_debugaltlink section.

>  # Copied from testsuite/gdb.dwarf2/dwzbuildid.exp.

>  proc write_just_debugaltlink {filename dwzname buildid} {

> @@ -109,8 +114,10 @@ proc write_dwarf_file {filename buildid {value 99}} {

>      }

>  }

>

> +set corefile "corefile"

> +

>  proc no_url { } {

> -    global binfile outputdir debugdir

> +    global binfile corefile outputdir debugdir

>

>      setenv DEBUGINFOD_URLS ""

>

> @@ -162,10 +169,20 @@ proc no_url { } {

>      gdb_test "file ${binfile}_alt.o" \

>         ".*could not find '.gnu_debugaltlink'.*" \

>         "file [file tail ${binfile}_alt.o]"

> +

> +    # Generate a core file and test that gdb cannot find the executable

> +    clean_restart ${binfile}2

> +    gdb_test "start" "Temporary breakpoint.*"

> +    gdb_test "generate-core-file $corefile" \

> +       "Saved corefile $corefile"

> +    file rename -force ${binfile}2 $debugdir

> +

> +    clean_restart

> +    gdb_test "core $corefile" ".*in ?? ().*"

>  }

>

>  proc local_url { } {

> -    global binfile outputdir db debugdir

> +    global binfile corefile outputdir db debugdir

>

>      # Find an unused port

>      set port 7999

> @@ -228,6 +245,10 @@ proc local_url { } {

>      gdb_test "file ${binfile}_alt.o" \

>         ".*Reading symbols from ${binfile}_alt.o\.\.\.*" \

>         "file [file tail ${binfile}_alt.o]"

> +

> +    # gdb should now find the executable file

> +    clean_restart

> +    gdb_test "core $corefile" ".*return 0.*"

>  }

>

>  set envlist \

> --

> 2.31.1

>
Tom de Vries via Gdb-patches July 15, 2021, 2:21 a.m. | #3
> We populate a map of sonames to build-ids during core_target::build_file_mappings

> by using the new function debuginfod_set_soname_buildid. The filenames used in

> solib_map_sections originate from each library's DT_SONAME tag in its .dynamic

> segment so we added another function scan_soname_tag (which is modelled after

> solib-svr4.c:scan_dyntag) that will lookup the value of DT_SONAME in the dynamic


Instead of duplicating scan_dyntag, can we please extract a common
base?  It is some non-trivial code, so it would be really good to
avoid having two copies.

Everything until setting the dynptr looks like it could be
commonized.  The common function could work like scan_dyntag, where you
pass a desired dyntag.  But since you are actually looking for two
values, that would require two calls, so two scans.  So instead I would
suggest making a "foreach_dyntag" function that takes a callback (a
gdb::function_view), where you invoke the callback for each dyntag.
The rest could then easily be implemented on top of that, and dyntag
parsing code would be at a single place.

> string table segment of the library. This name gets mapped to the corresponding

> build-id. Then using the new function debuginfod_get_soname_buildid we can

> retrieve the desired build-id in solib_map_sections and check the debuginfod

> cache in case the library can't otherwise be found.

> 

> Tested on Fedora 33 x86_64.

> 

> gdb/ChangeLog:

> 

> 	PR gdb/27570

> 	* arch-utils.c (default_read_core_file_mappings): Add build_id

> 	parameter.

> 	* arch-utils.h (default_read_core_file_mappings): Add build_id

> 	parameter.

> 	* corelow.c (core_target::build_file_mappings): Add debuginfod

> 	executable query and map library soname to build-id.

> 	(core_target::close): Call debuginfod_clear_soname_buildids.

> 	(locate_exec_from_corefile_build_id): Add debuginfod executable query

> 	* debuginfod-support.c (debuginfod_exec_query): New function.

> 	(debuginfod_set_soname_buildid): New function.

> 	(debuginfod_get_soname_buildid): New function.

> 	(debuginfod_clear_soname_buildids): New function.

> 	(scan_soname_tag): New function.

> 	* debuginfod-support.h (debuginfod_exec_query): New function.

> 	(debuginfod_set_soname_buildid): New function.

> 	(debuginfod_get_soname_buildid): New function.

> 	(debuginfod_clear_soname_buildids): New function.

> 	* gcore.in: unset $DEBUGINFOD_URLS.

> 	* gdbarch.c: Regenerate.

> 	* gdbarch.h: Regenerate.

> 	* gdbarch.sh (read_core_file_mappings): Add build_id parameter.

> 	* linux-tdep.c (linux_read_core_file_mappings): Map shared library

> 	filenames in core file mappings to build-id.

> 	(linux_core_info_proc_mappings): Add build-id parameter.

> 	* solib.c (solib_map_sections): Add debuginfod executable query.

> 

> gdb/testsuite/ChangeLog:

> 

> 	PR gdb/27570

> 	* gdb.debuginfod/fetch_src_and_symbols.exp: Add new testcases.


Note that you can now get rid of the ChangeLog entries (instead of
updating them).

> ---

>  gdb/arch-utils.c                              |   4 +-

>  gdb/arch-utils.h                              |   4 +-

>  gdb/corelow.c                                 |  40 +++-

>  gdb/debuginfod-support.c                      | 210 ++++++++++++++++++

>  gdb/debuginfod-support.h                      |  41 ++++

>  gdb/gcore.in                                  |   3 +

>  gdb/gdbarch.c                                 |   2 +-

>  gdb/gdbarch.h                                 |   4 +-

>  gdb/gdbarch.sh                                |   2 +-

>  gdb/linux-tdep.c                              |  34 ++-

>  gdb/solib.c                                   |  19 ++

>  .../gdb.debuginfod/fetch_src_and_symbols.exp  |  25 ++-

>  12 files changed, 376 insertions(+), 12 deletions(-)

> 

> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c

> index c75a79757ea..44ffaa2c2c3 100644

> --- a/gdb/arch-utils.c

> +++ b/gdb/arch-utils.c

> @@ -1104,7 +1104,9 @@ default_read_core_file_mappings (struct gdbarch *gdbarch,

>  							  ULONGEST start,

>  							  ULONGEST end,

>  							  ULONGEST file_ofs,

> -							  const char *filename)>

> +							  const char *filename,

> +							  const bfd_build_id

> +							    *build_id)>

>  				   loop_cb)


When it gets unreasonably crammed on the right border like that, I
suggest doing like this:

extern void default_read_core_file_mappings
  (struct gdbarch *gdbarch,
   struct bfd *cbfd,
   ...

That gives plenty of space for the parameters.

> @@ -194,4 +227,181 @@ debuginfod_debuginfo_query (const unsigned char *build_id,

>  

>    return fd;

>  }

> +

> +/* See debuginfod-support.h  */

> +

> +scoped_fd

> +debuginfod_exec_query (const unsigned char *build_id,

> +		       int build_id_len,

> +		       const char *filename,

> +		       gdb::unique_xmalloc_ptr<char> *destname)

> +{

> +  const char *urls_env_var = getenv (DEBUGINFOD_URLS_ENV_VAR);

> +  if (urls_env_var == NULL || urls_env_var[0] == '\0')

> +    return scoped_fd (-ENOSYS);

> +

> +  debuginfod_client *c = get_debuginfod_client ();

> +

> +  if (c == nullptr)

> +    return scoped_fd (-ENOMEM);

> +

> +  char *dname = nullptr;

> +  user_data data ("executable for", filename);

> +

> +  debuginfod_set_user_data (c, &data);

> +  scoped_fd fd (debuginfod_find_executable (c, build_id, build_id_len, &dname));

> +  debuginfod_set_user_data (c, nullptr);

> +

> +  if (fd.get () < 0 && fd.get () != -ENOENT)

> +    printf_filtered (_("Download failed: %s.  Continuing without executable for %ps.\n"),


This line is > 80 columns.  There are a few others in the patch.

> +		     safe_strerror (-fd.get ()),

> +		     styled_string (file_name_style.style (),  filename));

> +

> +  if (fd.get () >= 0)

> +    destname->reset (dname);

> +

> +  return fd;

> +}

> +

> +/* Attempt to locate the DT_SONAME tag within ABFD's .dynamic section.

> +   If found copy the string referred to by the tag to SONAME and return 1.

> +   Otherwise return 0.  */

> +

> +static int

> +scan_soname_tag (struct bfd *abfd, std::string &soname)


Instead of returning 1 / 0 for work / did not work and returning the
soname by parameter, I suggest returning a gdb::optional<std::string>.
This way, both informations (whether it worked and if it did, the value)
are encoded in the return value, it's more straightforward.

> +{

> +  if (abfd == nullptr)

> +    return 0;


If abfd being nullptr is a programmer error, then use:

  gdb_assert (abfd != nullptr);


> +

> +  if (bfd_get_flavour (abfd) != bfd_target_elf_flavour)

> +    return 0;


This was already checked at the level above, so if you want to keep it,
then make it an assert:

  gdb_assert (bfd_get_flavour (abfd) == bfd_target_elf_flavour);

> +

> +  int arch_size = bfd_get_arch_size (abfd);

> +  if (arch_size == -1)

> +    return 0;

> +

> +  /* Find the start address of the .dynamic section.  */

> +  struct bfd_section *sect = bfd_get_section_by_name (abfd, ".dynamic");

> +  if (sect == nullptr)

> +    return 0;

> +

> +  /* Read in .dynamic from the BFD.  */

> +  int sect_size = bfd_section_size (sect);

> +  gdb::byte_vector bufstart (sect_size);

> +

> +  if (!bfd_get_section_contents (abfd, sect, bufstart.data (), 0, sect_size))

> +    return 0;


You could use gdb_bfd_get_full_section_contents.

Side-note: I did write gdb_bfd_get_full_section_contents, and now I
wonder why I made it return a bool instead of
gdb::optional<gdb::byte_vector>... it would be much nicer to return an
optional :).

> +

> +  gdb_byte *buf = bufstart.data ();

> +  struct bfd_section *dynstr = nullptr;

> +  CORE_ADDR soname_idx = 0;

> +  int step = (arch_size == 32) ? sizeof (Elf32_External_Dyn)

> +			       : sizeof (Elf64_External_Dyn);

> +

> +  /* Iterate over BUF and search for DT_SONAME and DT_STRTAB.  */

> +  for (gdb_byte *bufend = buf + sect_size;

> +       (buf < bufend) && (soname_idx == 0 || dynstr == nullptr);

> +       buf += step)

> +  {

> +    long current_dyntag;

> +    CORE_ADDR dyn_ptr;

> +

> +    if (arch_size == 32)

> +      {

> +	Elf32_External_Dyn *x_dynp_32 = (Elf32_External_Dyn *) buf;

> +	current_dyntag = bfd_h_get_32 (abfd, (bfd_byte *) x_dynp_32->d_tag);

> +	dyn_ptr = bfd_h_get_32 (abfd, (bfd_byte *) x_dynp_32->d_un.d_ptr);

> +      }

> +    else

> +      {

> +	Elf64_External_Dyn *x_dynp_64 = (Elf64_External_Dyn *) buf;

> +	current_dyntag = bfd_h_get_64 (abfd, (bfd_byte *) x_dynp_64->d_tag);

> +	dyn_ptr = bfd_h_get_64 (abfd, (bfd_byte *) x_dynp_64->d_un.d_ptr);

> +      }

> +

> +    if (current_dyntag == DT_NULL)

> +      buf = bufend;

> +    else if (current_dyntag == DT_SONAME)

> +      soname_idx = dyn_ptr;

> +    else if (current_dyntag == DT_STRTAB)

> +      for (struct bfd_section *s = abfd->sections; s != nullptr; s = s->next)

> +	if (s->vma == dyn_ptr)

> +	  dynstr = s;

> +  }

> +

> +  if (soname_idx == 0 || dynstr == nullptr)

> +    return 0;

> +

> +  sect_size = bfd_section_size (dynstr);

> +  if (sect_size <= soname_idx)

> +    return 0;

> +

> +  /* Read the soname from the string table.  */

> +  gdb::char_vector dynstr_buf (sect_size);

> +  if (!bfd_get_section_contents (abfd, dynstr, dynstr_buf.data (), 0, sect_size))


Use gdb::byte_vector.  But like above, you can use
gdb_bfd_get_full_section_contents.

> +    return 0;

> +

> +  soname = dynstr_buf.data () + soname_idx;

> +  return 1;

> +}

> +

> +/* See debuginfod-support.h  */

> +

> +int

> +debuginfod_set_soname_buildid (struct bfd *bfd, const bfd_build_id *build_id)


There's one caller of this and it doesn't care about the return value,
so make this function return void.

> +{

> +  if (bfd == nullptr || build_id == nullptr)

> +    return 0;


Is there a legitimate use case to passing a nullptr bfd or build_id?  I
would prefer if it were:

  gdb_assert (bfd != nullptr);
  gdb_assert (build_id != nullptr);

> +

> +  gdb_bfd_ref_ptr abfd (gdb_bfd_open (bfd->filename, gnutarget));


A nit, but we prefer the style:

  gdb_bfd_ref_ptr abfd = gdb_bfd_open (bfd->filename, gnutarget);

(same when using unique_ptrs)

> +

> +  if (abfd == nullptr)

> +    return 0;

> +

> +  /* Check that bfd is an ET_DYN ELF file.  */

> +  bfd_check_format (abfd.get (), bfd_object);


What's the point of calling bfd_check_format without checking the
result?  It looks like a function without side-effects.

> +  if (bfd_get_flavour (abfd.get ()) != bfd_target_elf_flavour

> +      || !(bfd_get_file_flags (abfd.get ()) & DYNAMIC))

> +    return 0;

> +

> +  std::string soname;

> +  std::string build_id_hex;


Move the declaration of build_id_hex to the scope where it's used, so we
don't construct/destroy it if we don't need it.

> +

> +  /* Determine soname of shared library.  If found map soname to build-id.  */

> +  if (scan_soname_tag (abfd.get (), soname))

> +    {

> +      for (int i = 0; i < build_id->size; i++)

> +	{

> +	  build_id_hex += "0123456789abcdef"[build_id->data[i] >> 4];

> +	  build_id_hex += "0123456789abcdef"[build_id->data[i] & 0xf];

> +	}


Would the existing build_id_to_string function work here?

> +

> +      soname_to_buildid[soname] = build_id_hex;


I'm not completely sure I understand the flow of all of this yet, but
since the map is global, can there be collisions in soname?  For
example, if we debug two core files in the same GDB, and both have a
foo.so library, but different versions with different build-ids.  What
will happen here?

It sounds like something needs to be per-inferior or per-program-space
here.

In fact, that soname to buildid mapping doesn't seem related to
debuginfod at all, it's just a generic soname to buildid mapping.  That
could exist and be useful even if debuginfod didn't exist, right?  I'm
not sure what is the best place.  But that information is produced by
the core target and consumed by the solib subsystem... so it should
probably be close to one of them.

> +      return 1;

> +    }

> +

> +  return 0;

> +}

> +

> +/* See debuginfod-support.h  */

> +

> +int

> +debuginfod_get_soname_buildid (char *soname, gdb::unique_xmalloc_ptr<char> *build_id_hex)

> +{

> +  auto it = soname_to_buildid.find (basename (soname));

> +  if (it == soname_to_buildid.end ())

> +    return 0;

> +

> +  build_id_hex->reset (xstrdup (it->second.c_str ()));


If we're going to return a copy, then let's make it an std::string.  But
do we really need to return a copy?  If not, I would suggest making it:

    const char *
    debuginfod_get_soname_buildid (const char *soname)
    {
      auto it = soname_to_buildid.find (basename (soname));
      if (it == soname_to_buildid.end ())
	return nullptr;

      return it->second.c_str ();
    }

> +  return 1;

> +}

> +

> +/* See debuginfod-support.h  */

> +

> +void debuginfod_clear_soname_buildids ()


"void" on its own line.

> +{

> +  soname_to_buildid.clear ();

> +  return;

> +}

> +

>  #endif

> diff --git a/gdb/debuginfod-support.h b/gdb/debuginfod-support.h

> index 5e5aab56e74..185c5be22fd 100644

> --- a/gdb/debuginfod-support.h

> +++ b/gdb/debuginfod-support.h

> @@ -61,4 +61,45 @@ debuginfod_debuginfo_query (const unsigned char *build_id,

>  			    const char *filename,

>  			    gdb::unique_xmalloc_ptr<char> *destname);

>  

> +/* Query debuginfod servers for an executable file with BUILD_ID.

> +   BUILD_ID can be given as a binary blob or a null-terminated string.

> +   If given as a binary blob, BUILD_ID_LEN should be the number of bytes.

> +   If given as a null-terminated string, BUILD_ID_LEN should be 0.

> +

> +   FILENAME should be the name or path associated with the executable.

> +   It is used for printing messages to the user.

> +

> +   If the file is successfully retrieved, its path on the local machine

> +   is stored in DESTNAME.  If GDB is not built with debuginfod, this

> +   function returns -ENOSYS.  */

> +

> +extern scoped_fd debuginfod_exec_query (const unsigned char *build_id,

> +					int build_id_len,

> +					const char *filename,

> +					gdb::unique_xmalloc_ptr<char> *destname);

> +

> +/* If BFD is of an ELF shared library then associate its soname with

> +   BUILD_ID so that it can be retrieved with debuginfod_get_soname_buildid().

> +

> +   Return 1 if the soname was successully associated with BUILD-ID, otherwise

> +   return 0.  */

> +

> +extern int debuginfod_set_soname_buildid (struct bfd *bfd,

> +					  const bfd_build_id *build_id);


I think the return type should be bool.

> +

> +/* If SONAME had a build-id associated with it by a previous call to

> +   debuginfod_set_soname_buildid() then store the build-id in BUILD_ID_HEX


Remove the ().

> +   as a NULL-terminated string.

> +

> +   Return 1 if SONAME's build-id was found and stored in BUILD_ID_HEX,

> +   otherwise return 0.  */

> +

> +extern int debuginfod_get_soname_buildid (char *soname,

> +					  gdb::unique_xmalloc_ptr<char> *build_id_hex);


soname should probably be `const char *`?  And the return type bool as
well.

> @@ -1187,8 +1206,17 @@ linux_read_core_file_mappings (struct gdbarch *gdbarch,

>        descdata += addr_size;

>        char * filename = filenames;

>        filenames += strlen ((char *) filenames) + 1;

> +      const bfd_build_id *build_id = nullptr;

> +

> +      /* Map filename to the build-id associated with this start vma,

> +	 if such a build-id was found.  Otherwise use the build-id

> +	 already associated with this filename if it exists. */

> +      if ((build_id = vma_map[start]) != nullptr)


Please split the assignment on its own line.

> +	filename_map[filename] = build_id;

> +      else

> +	build_id = filename_map[filename];

>  

> -      loop_cb (i, start, end, file_ofs, filename);

> +      loop_cb (i, start, end, file_ofs, filename, build_id);

>      }

>  }

>  

> @@ -1217,7 +1245,7 @@ linux_core_info_proc_mappings (struct gdbarch *gdbarch, const char *args)

>  	  }

>        },

>      [=] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs,

> -	 const char *filename)

> +	 const char *filename, const bfd_build_id *build_id)


Not related to your patch but... would it be a good idea to show the
build-ids in "info proc mappings"?  It might sound useful to have that
information in order to determine / find the right binaries that your
process was using.

>        {

>  	if (gdbarch_addr_bit (gdbarch) == 32)

>  	  printf_filtered ("\t%10s %10s %10s %10s %s\n",

> diff --git a/gdb/solib.c b/gdb/solib.c

> index 2df52118143..5f96d463aa4 100644

> --- a/gdb/solib.c

> +++ b/gdb/solib.c

> @@ -46,6 +46,8 @@

>  #include "filesystem.h"

>  #include "gdb_bfd.h"

>  #include "gdbsupport/filestuff.h"

> +#include "gdbsupport/scoped_fd.h"

> +#include "debuginfod-support.h"

>  #include "source.h"

>  #include "cli/cli-style.h"

>  

> @@ -536,6 +538,23 @@ solib_map_sections (struct so_list *so)

>    gdb::unique_xmalloc_ptr<char> filename (tilde_expand (so->so_name));

>    gdb_bfd_ref_ptr abfd (ops->bfd_open (filename.get ()));

>  

> +  if (abfd == NULL)

> +    {

> +      gdb::unique_xmalloc_ptr<char> build_id_hex;

> +

> +      /* If a build-id was previously associated with this soname

> +	 then use it to query debuginfod for the library.  */

> +      if (debuginfod_get_soname_buildid (so->so_name, &build_id_hex))

> +	{

> +	  scoped_fd fd = debuginfod_exec_query

> +			 ((const unsigned char*) build_id_hex.get (),

> +			  0, so->so_name, &filename);

> +

> +	  if (fd.get () >= 0)

> +	    abfd = ops->bfd_open (filename.get ());

> +	}

> +    }


I have a question about the order of the operations here.  Let's say I
generate a core on my ARM machine and bring it back to debug it on my
x86-64 machine.  And let's say I have a /usr/lib/foo.so on both
machines.  Will the first ops->bfd_open manage to open the one of my
development machine (the wrong one), and abfd will be != NULL?

I am not sure what happens in that case, but if we could somehow
determine that we didn't open the right library (for example, seeing
that the lib's build-id doesn't match the expected build-id), and
instead try downloading the right one using debuginfod... would that
make sense?

Simon

Patch

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index c75a79757ea..44ffaa2c2c3 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -1104,7 +1104,9 @@  default_read_core_file_mappings (struct gdbarch *gdbarch,
 							  ULONGEST start,
 							  ULONGEST end,
 							  ULONGEST file_ofs,
-							  const char *filename)>
+							  const char *filename,
+							  const bfd_build_id
+							    *build_id)>
 				   loop_cb)
 {
 }
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index a5b40ad8ff1..6c288f0fe92 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -309,6 +309,8 @@  extern void default_read_core_file_mappings (struct gdbarch *gdbarch,
 								      ULONGEST start,
 								      ULONGEST end,
 								      ULONGEST file_ofs,
-								      const char *filename)>
+								      const char *filename,
+								      const bfd_build_id
+									*build_id)>
 					       loop_cb);
 #endif /* ARCH_UTILS_H */
diff --git a/gdb/corelow.c b/gdb/corelow.c
index a1943ab2ea6..9d060591b31 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -46,6 +46,8 @@ 
 #include "gdbsupport/filestuff.h"
 #include "build-id.h"
 #include "gdbsupport/pathstuff.h"
+#include "gdbsupport/scoped_fd.h"
+#include "debuginfod-support.h"
 #include <unordered_map>
 #include <unordered_set>
 #include "gdbcmd.h"
@@ -200,7 +202,7 @@  core_target::build_file_mappings ()
     /* read_core_file_mappings will invoke this lambda for each mapping
        that it finds.  */
     [&] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs,
-	 const char *filename)
+	 const char *filename, const bfd_build_id *build_id)
       {
 	/* Architecture-specific read_core_mapping methods are expected to
 	   weed out non-file-backed mappings.  */
@@ -215,6 +217,11 @@  core_target::build_file_mappings ()
 	       canonical) pathname will be provided.  */
 	    gdb::unique_xmalloc_ptr<char> expanded_fname
 	      = exec_file_find (filename, NULL);
+
+	    if (expanded_fname == nullptr && build_id != nullptr)
+	      debuginfod_exec_query (build_id->data, build_id->size,
+				     filename, &expanded_fname);
+
 	    if (expanded_fname == nullptr)
 	      {
 		m_core_unavailable_mappings.emplace_back (start, end - start);
@@ -268,6 +275,10 @@  core_target::build_file_mappings ()
 
 	/* Set target_section fields.  */
 	m_core_file_mappings.emplace_back (start, end, sec);
+
+	/* If this bfd is of a shared library, record its soname and build id
+	   for debuginfod.  */
+	debuginfod_set_soname_buildid (bfd, build_id);
       });
 
   normalize_mem_ranges (&m_core_unavailable_mappings);
@@ -290,6 +301,7 @@  core_target::close ()
       /* Clear out solib state while the bfd is still open.  See
 	 comments in clear_solib in solib.c.  */
       clear_solib ();
+      debuginfod_clear_soname_buildids ();
 
       current_program_space->cbfd.reset (nullptr);
     }
@@ -391,6 +403,32 @@  locate_exec_from_corefile_build_id (bfd *abfd, int from_tty)
       symbol_file_add_main (bfd_get_filename (execbfd.get ()),
 			    symfile_add_flag (from_tty ? SYMFILE_VERBOSE : 0));
     }
+  else
+    {
+      gdb::unique_xmalloc_ptr<char> execpath;
+      scoped_fd fd (debuginfod_exec_query (build_id->data, build_id->size,
+					   abfd->filename, &execpath));
+
+      if (fd.get () >= 0)
+	{
+	  execbfd = gdb_bfd_open (execpath.get (), gnutarget);
+
+	  if (execbfd == nullptr)
+	    warning (_("File \"%s\" from debuginfod cannot be opened as bfd"),
+		     execpath.get ());
+	  else if (!build_id_verify (execbfd.get (), build_id->size,
+				     build_id->data))
+	    execbfd.reset (nullptr);
+	  else
+	    {
+	      /* Successful download */
+	      exec_file_attach (execpath.get (), from_tty);
+	      symbol_file_add_main (execpath.get (),
+				    symfile_add_flag (from_tty ?
+						      SYMFILE_VERBOSE : 0));
+	    }
+	}
+    }
 }
 
 /* See gdbcore.h.  */
diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
index 2d626e335a0..19f12feb0fd 100644
--- a/gdb/debuginfod-support.c
+++ b/gdb/debuginfod-support.c
@@ -41,8 +41,41 @@  debuginfod_debuginfo_query (const unsigned char *build_id,
 {
   return scoped_fd (-ENOSYS);
 }
+
+scoped_fd
+debuginfod_exec_query (const unsigned char *build_id,
+		       int build_id_len,
+		       const char *filename,
+		       gdb::unique_xmalloc_ptr<char> *destname)
+{
+  return scoped_fd (-ENOSYS);
+}
+
+int
+debuginfod_set_soname_buildid (struct bfd *bfd, const bfd_build_id *build_id)
+{
+  return 0;
+}
+
+int
+debuginfod_get_soname_buildid (char *soname, gdb::unique_xmalloc_ptr<char> *build_id_hex)
+{
+  return 0;
+}
+
+void
+debuginfod_clear_soname_buildids ()
+{
+  return;
+}
 #else
+#include "elf-bfd.h"
+#include "gdbcore.h"
+#include <libgen.h>
 #include <elfutils/debuginfod.h>
+#include <unordered_map>
+
+static std::unordered_map<std::string, std::string> soname_to_buildid;
 
 struct user_data
 {
@@ -194,4 +227,181 @@  debuginfod_debuginfo_query (const unsigned char *build_id,
 
   return fd;
 }
+
+/* See debuginfod-support.h  */
+
+scoped_fd
+debuginfod_exec_query (const unsigned char *build_id,
+		       int build_id_len,
+		       const char *filename,
+		       gdb::unique_xmalloc_ptr<char> *destname)
+{
+  const char *urls_env_var = getenv (DEBUGINFOD_URLS_ENV_VAR);
+  if (urls_env_var == NULL || urls_env_var[0] == '\0')
+    return scoped_fd (-ENOSYS);
+
+  debuginfod_client *c = get_debuginfod_client ();
+
+  if (c == nullptr)
+    return scoped_fd (-ENOMEM);
+
+  char *dname = nullptr;
+  user_data data ("executable for", filename);
+
+  debuginfod_set_user_data (c, &data);
+  scoped_fd fd (debuginfod_find_executable (c, build_id, build_id_len, &dname));
+  debuginfod_set_user_data (c, nullptr);
+
+  if (fd.get () < 0 && fd.get () != -ENOENT)
+    printf_filtered (_("Download failed: %s.  Continuing without executable for %ps.\n"),
+		     safe_strerror (-fd.get ()),
+		     styled_string (file_name_style.style (),  filename));
+
+  if (fd.get () >= 0)
+    destname->reset (dname);
+
+  return fd;
+}
+
+/* Attempt to locate the DT_SONAME tag within ABFD's .dynamic section.
+   If found copy the string referred to by the tag to SONAME and return 1.
+   Otherwise return 0.  */
+
+static int
+scan_soname_tag (struct bfd *abfd, std::string &soname)
+{
+  if (abfd == nullptr)
+    return 0;
+
+  if (bfd_get_flavour (abfd) != bfd_target_elf_flavour)
+    return 0;
+
+  int arch_size = bfd_get_arch_size (abfd);
+  if (arch_size == -1)
+    return 0;
+
+  /* Find the start address of the .dynamic section.  */
+  struct bfd_section *sect = bfd_get_section_by_name (abfd, ".dynamic");
+  if (sect == nullptr)
+    return 0;
+
+  /* Read in .dynamic from the BFD.  */
+  int sect_size = bfd_section_size (sect);
+  gdb::byte_vector bufstart (sect_size);
+
+  if (!bfd_get_section_contents (abfd, sect, bufstart.data (), 0, sect_size))
+    return 0;
+
+  gdb_byte *buf = bufstart.data ();
+  struct bfd_section *dynstr = nullptr;
+  CORE_ADDR soname_idx = 0;
+  int step = (arch_size == 32) ? sizeof (Elf32_External_Dyn)
+			       : sizeof (Elf64_External_Dyn);
+
+  /* Iterate over BUF and search for DT_SONAME and DT_STRTAB.  */
+  for (gdb_byte *bufend = buf + sect_size;
+       (buf < bufend) && (soname_idx == 0 || dynstr == nullptr);
+       buf += step)
+  {
+    long current_dyntag;
+    CORE_ADDR dyn_ptr;
+
+    if (arch_size == 32)
+      {
+	Elf32_External_Dyn *x_dynp_32 = (Elf32_External_Dyn *) buf;
+	current_dyntag = bfd_h_get_32 (abfd, (bfd_byte *) x_dynp_32->d_tag);
+	dyn_ptr = bfd_h_get_32 (abfd, (bfd_byte *) x_dynp_32->d_un.d_ptr);
+      }
+    else
+      {
+	Elf64_External_Dyn *x_dynp_64 = (Elf64_External_Dyn *) buf;
+	current_dyntag = bfd_h_get_64 (abfd, (bfd_byte *) x_dynp_64->d_tag);
+	dyn_ptr = bfd_h_get_64 (abfd, (bfd_byte *) x_dynp_64->d_un.d_ptr);
+      }
+
+    if (current_dyntag == DT_NULL)
+      buf = bufend;
+    else if (current_dyntag == DT_SONAME)
+      soname_idx = dyn_ptr;
+    else if (current_dyntag == DT_STRTAB)
+      for (struct bfd_section *s = abfd->sections; s != nullptr; s = s->next)
+	if (s->vma == dyn_ptr)
+	  dynstr = s;
+  }
+
+  if (soname_idx == 0 || dynstr == nullptr)
+    return 0;
+
+  sect_size = bfd_section_size (dynstr);
+  if (sect_size <= soname_idx)
+    return 0;
+
+  /* Read the soname from the string table.  */
+  gdb::char_vector dynstr_buf (sect_size);
+  if (!bfd_get_section_contents (abfd, dynstr, dynstr_buf.data (), 0, sect_size))
+    return 0;
+
+  soname = dynstr_buf.data () + soname_idx;
+  return 1;
+}
+
+/* See debuginfod-support.h  */
+
+int
+debuginfod_set_soname_buildid (struct bfd *bfd, const bfd_build_id *build_id)
+{
+  if (bfd == nullptr || build_id == nullptr)
+    return 0;
+
+  gdb_bfd_ref_ptr abfd (gdb_bfd_open (bfd->filename, gnutarget));
+
+  if (abfd == nullptr)
+    return 0;
+
+  /* Check that bfd is an ET_DYN ELF file.  */
+  bfd_check_format (abfd.get (), bfd_object);
+  if (bfd_get_flavour (abfd.get ()) != bfd_target_elf_flavour
+      || !(bfd_get_file_flags (abfd.get ()) & DYNAMIC))
+    return 0;
+
+  std::string soname;
+  std::string build_id_hex;
+
+  /* Determine soname of shared library.  If found map soname to build-id.  */
+  if (scan_soname_tag (abfd.get (), soname))
+    {
+      for (int i = 0; i < build_id->size; i++)
+	{
+	  build_id_hex += "0123456789abcdef"[build_id->data[i] >> 4];
+	  build_id_hex += "0123456789abcdef"[build_id->data[i] & 0xf];
+	}
+
+      soname_to_buildid[soname] = build_id_hex;
+      return 1;
+    }
+
+  return 0;
+}
+
+/* See debuginfod-support.h  */
+
+int
+debuginfod_get_soname_buildid (char *soname, gdb::unique_xmalloc_ptr<char> *build_id_hex)
+{
+  auto it = soname_to_buildid.find (basename (soname));
+  if (it == soname_to_buildid.end ())
+    return 0;
+
+  build_id_hex->reset (xstrdup (it->second.c_str ()));
+  return 1;
+}
+
+/* See debuginfod-support.h  */
+
+void debuginfod_clear_soname_buildids ()
+{
+  soname_to_buildid.clear ();
+  return;
+}
+
 #endif
diff --git a/gdb/debuginfod-support.h b/gdb/debuginfod-support.h
index 5e5aab56e74..185c5be22fd 100644
--- a/gdb/debuginfod-support.h
+++ b/gdb/debuginfod-support.h
@@ -61,4 +61,45 @@  debuginfod_debuginfo_query (const unsigned char *build_id,
 			    const char *filename,
 			    gdb::unique_xmalloc_ptr<char> *destname);
 
+/* Query debuginfod servers for an executable file with BUILD_ID.
+   BUILD_ID can be given as a binary blob or a null-terminated string.
+   If given as a binary blob, BUILD_ID_LEN should be the number of bytes.
+   If given as a null-terminated string, BUILD_ID_LEN should be 0.
+
+   FILENAME should be the name or path associated with the executable.
+   It is used for printing messages to the user.
+
+   If the file is successfully retrieved, its path on the local machine
+   is stored in DESTNAME.  If GDB is not built with debuginfod, this
+   function returns -ENOSYS.  */
+
+extern scoped_fd debuginfod_exec_query (const unsigned char *build_id,
+					int build_id_len,
+					const char *filename,
+					gdb::unique_xmalloc_ptr<char> *destname);
+
+/* If BFD is of an ELF shared library then associate its soname with
+   BUILD_ID so that it can be retrieved with debuginfod_get_soname_buildid().
+
+   Return 1 if the soname was successully associated with BUILD-ID, otherwise
+   return 0.  */
+
+extern int debuginfod_set_soname_buildid (struct bfd *bfd,
+					  const bfd_build_id *build_id);
+
+/* If SONAME had a build-id associated with it by a previous call to
+   debuginfod_set_soname_buildid() then store the build-id in BUILD_ID_HEX
+   as a NULL-terminated string.
+
+   Return 1 if SONAME's build-id was found and stored in BUILD_ID_HEX,
+   otherwise return 0.  */
+
+extern int debuginfod_get_soname_buildid (char *soname,
+					  gdb::unique_xmalloc_ptr<char> *build_id_hex);
+
+/* Clear all soname-to-build-id mappings that have been created by
+   debuginfod_set_soname_buildid.  */
+
+extern void debuginfod_clear_soname_buildids ();
+
 #endif /* DEBUGINFOD_SUPPORT_H */
diff --git a/gdb/gcore.in b/gdb/gcore.in
index 24354a79e27..25b24c3cd3d 100644
--- a/gdb/gcore.in
+++ b/gdb/gcore.in
@@ -89,6 +89,9 @@  if [ ! -f "$binary_path/@GDB_TRANSFORM_NAME@" ]; then
   exit 1
 fi
 
+# Prevent unnecessary debuginfod queries during core file generation.
+unset DEBUGINFOD_URLS
+
 # Initialise return code.
 rc=0
 
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 208cf4b5aaa..8a2d88c5f96 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -5411,7 +5411,7 @@  set_gdbarch_get_pc_address_flags (struct gdbarch *gdbarch,
 }
 
 void
-gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename)> loop_cb)
+gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const bfd_build_id *build_id)> loop_cb)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->read_core_file_mappings != NULL);
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 7157e5596fd..ca7949bafb1 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1709,8 +1709,8 @@  extern void set_gdbarch_get_pc_address_flags (struct gdbarch *gdbarch, gdbarch_g
 
 /* Read core file mappings */
 
-typedef void (gdbarch_read_core_file_mappings_ftype) (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename)> loop_cb);
-extern void gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename)> loop_cb);
+typedef void (gdbarch_read_core_file_mappings_ftype) (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const bfd_build_id *build_id)> loop_cb);
+extern void gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const bfd_build_id *build_id)> loop_cb);
 extern void set_gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, gdbarch_read_core_file_mappings_ftype *read_core_file_mappings);
 
 extern struct gdbarch_tdep *gdbarch_tdep (struct gdbarch *gdbarch);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 43e51341f97..ea651e8c0dc 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1209,7 +1209,7 @@  m;ULONGEST;type_align;struct type *type;type;;default_type_align;;0
 f;std::string;get_pc_address_flags;frame_info *frame, CORE_ADDR pc;frame, pc;;default_get_pc_address_flags;;0
 
 # Read core file mappings
-m;void;read_core_file_mappings;struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename)> loop_cb;cbfd, pre_loop_cb, loop_cb;;default_read_core_file_mappings;;0
+m;void;read_core_file_mappings;struct bfd *cbfd, gdb::function_view<void (ULONGEST count)> pre_loop_cb, gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const bfd_build_id *build_id)> loop_cb;cbfd, pre_loop_cb, loop_cb;;default_read_core_file_mappings;;0
 
 EOF
 }
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 927e69bf1e1..757b5bc02e9 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -43,6 +43,7 @@ 
 #include "gcore-elf.h"
 
 #include <ctype.h>
+#include <unordered_map>
 
 /* This enum represents the values that the user can choose when
    informing the Linux kernel about which memory mappings will be
@@ -1104,7 +1105,8 @@  linux_read_core_file_mappings (struct gdbarch *gdbarch,
 							ULONGEST start,
 							ULONGEST end,
 							ULONGEST file_ofs,
-							const char *filename)>
+							const char *filename,
+							const bfd_build_id *build_id)>
 				 loop_cb)
 {
   /* Ensure that ULONGEST is big enough for reading 64-bit core files.  */
@@ -1174,6 +1176,23 @@  linux_read_core_file_mappings (struct gdbarch *gdbarch,
   if (f != descend)
     warning (_("malformed note - filename area is too big"));
 
+  const bfd_build_id *orig_build_id = cbfd->build_id;
+  std::unordered_map<ULONGEST, const bfd_build_id *> vma_map;
+  std::unordered_map<char *, const bfd_build_id *> filename_map;
+
+  /* Search for solib build-ids in the core file.  Each time one is found,
+     map the start vma of the corresponding elf header to the build-id.  */
+  for (bfd_section *sec = cbfd->sections; sec != nullptr; sec = sec->next)
+    {
+      cbfd->build_id = nullptr;
+
+      if (sec->flags & SEC_LOAD
+	  && get_elf_backend_data (cbfd)->elf_backend_core_find_build_id
+	       (cbfd, (bfd_vma) sec->filepos))
+	vma_map[sec->vma] = cbfd->build_id;
+    }
+
+  cbfd->build_id = orig_build_id;
   pre_loop_cb (count);
 
   for (int i = 0; i < count; i++)
@@ -1187,8 +1206,17 @@  linux_read_core_file_mappings (struct gdbarch *gdbarch,
       descdata += addr_size;
       char * filename = filenames;
       filenames += strlen ((char *) filenames) + 1;
+      const bfd_build_id *build_id = nullptr;
+
+      /* Map filename to the build-id associated with this start vma,
+	 if such a build-id was found.  Otherwise use the build-id
+	 already associated with this filename if it exists. */
+      if ((build_id = vma_map[start]) != nullptr)
+	filename_map[filename] = build_id;
+      else
+	build_id = filename_map[filename];
 
-      loop_cb (i, start, end, file_ofs, filename);
+      loop_cb (i, start, end, file_ofs, filename, build_id);
     }
 }
 
@@ -1217,7 +1245,7 @@  linux_core_info_proc_mappings (struct gdbarch *gdbarch, const char *args)
 	  }
       },
     [=] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs,
-	 const char *filename)
+	 const char *filename, const bfd_build_id *build_id)
       {
 	if (gdbarch_addr_bit (gdbarch) == 32)
 	  printf_filtered ("\t%10s %10s %10s %10s %s\n",
diff --git a/gdb/solib.c b/gdb/solib.c
index 2df52118143..5f96d463aa4 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -46,6 +46,8 @@ 
 #include "filesystem.h"
 #include "gdb_bfd.h"
 #include "gdbsupport/filestuff.h"
+#include "gdbsupport/scoped_fd.h"
+#include "debuginfod-support.h"
 #include "source.h"
 #include "cli/cli-style.h"
 
@@ -536,6 +538,23 @@  solib_map_sections (struct so_list *so)
   gdb::unique_xmalloc_ptr<char> filename (tilde_expand (so->so_name));
   gdb_bfd_ref_ptr abfd (ops->bfd_open (filename.get ()));
 
+  if (abfd == NULL)
+    {
+      gdb::unique_xmalloc_ptr<char> build_id_hex;
+
+      /* If a build-id was previously associated with this soname
+	 then use it to query debuginfod for the library.  */
+      if (debuginfod_get_soname_buildid (so->so_name, &build_id_hex))
+	{
+	  scoped_fd fd = debuginfod_exec_query
+			 ((const unsigned char*) build_id_hex.get (),
+			  0, so->so_name, &filename);
+
+	  if (fd.get () >= 0)
+	    abfd = ops->bfd_open (filename.get ());
+	}
+    }
+
   if (abfd == NULL)
     return 0;
 
diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
index 81d4791eb6d..79ec4e6f853 100644
--- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
+++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
@@ -58,6 +58,11 @@  if { [gdb_compile "$sourcetmp" "$binfile" executable {debug}] != "" } {
     return -1
 }
 
+if { [gdb_compile "$sourcetmp" "${binfile}2" executable {debug}] != "" } {
+    fail "compile"
+    return -1
+}
+
 # Write some assembly that just has a .gnu_debugaltlink section.
 # Copied from testsuite/gdb.dwarf2/dwzbuildid.exp.
 proc write_just_debugaltlink {filename dwzname buildid} {
@@ -109,8 +114,10 @@  proc write_dwarf_file {filename buildid {value 99}} {
     }
 }
 
+set corefile "corefile"
+
 proc no_url { } {
-    global binfile outputdir debugdir
+    global binfile corefile outputdir debugdir
 
     setenv DEBUGINFOD_URLS ""
 
@@ -162,10 +169,20 @@  proc no_url { } {
     gdb_test "file ${binfile}_alt.o" \
 	".*could not find '.gnu_debugaltlink'.*" \
 	"file [file tail ${binfile}_alt.o]"
+
+    # Generate a core file and test that gdb cannot find the executable
+    clean_restart ${binfile}2
+    gdb_test "start" "Temporary breakpoint.*"
+    gdb_test "generate-core-file $corefile" \
+	"Saved corefile $corefile"
+    file rename -force ${binfile}2 $debugdir
+
+    clean_restart
+    gdb_test "core $corefile" ".*in ?? ().*"
 }
 
 proc local_url { } {
-    global binfile outputdir db debugdir
+    global binfile corefile outputdir db debugdir
 
     # Find an unused port
     set port 7999
@@ -228,6 +245,10 @@  proc local_url { } {
     gdb_test "file ${binfile}_alt.o" \
 	".*Reading symbols from ${binfile}_alt.o\.\.\.*" \
 	"file [file tail ${binfile}_alt.o]"
+
+    # gdb should now find the executable file
+    clean_restart
+    gdb_test "core $corefile" ".*return 0.*"
 }
 
 set envlist \