[PATCH v3 0/3] Add debuginfod core file support

[PATCH v3 0/3] Add debuginfod core file support

[Aaron Merey]

Hello,

This patch series has been updated based on feedback from
https://sourceware.org/pipermail/gdb-patches/2021-July/180956.html

I want to address some points from Simon's review.

> 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.

The first patch in this series moves the multiple implementations of
scan_dyntag to solib.c so that duplication is avoided. I decided not 
to add a foreach_dyntag function because the reason for having more
than one call to scan_dyntag was just to get the value of the DT_STRTAB
tag.  I don't think it's really necessary to get this value because,
as far as I know, the value of DT_STRTAB always refers to the .dynstr
section, which is easily found with bfd_get_section_by_name. Unless
there are cases where DT_STRTAB doesn't refer to .dynstr, a
foreach_dyntag shouldn't be needed.

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

For some reason this function appears to update the flags and build-id
fields of the bfd with the correct values.

> 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.

The second patch in this series implements a per-program_space soname to
build-id mapping that is independent of debuginfod.

> 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.

I think it would be useful in some circumstances. The changes to
linux_read_core_file_mappings in the second patch of this series should
make it easy to include build-ids in 'info proc mappings'.

> > @@ -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?

If the solib installed at the time of debugging has the same soname
mentioned in the core file but a different build-id, then gdb will
silently use this installed solib even though it's the wrong build.
At least this is what happens when the arch stays the same. I have
not tested this senario when using multiple architectures. The third
patch in this series prints a warning message when a build-id mismatch
happens. If debuginfod is enabled then gdb will also query debuginfod
for the correct build of the solib.

One thing I should point out is that if we have the wrong build of the
solib installed locally and the debuginfod query fails because of some
error, the user will see something like the following:

warning: Build-id of /lib64/libc.so.6 does not match core file.
Download failed: No route to host. Continuing without executable for /lib64/libc.so.6.

However gdb is continuing with libc.so.6, although it is the wrong
build. We could modify solib_map_sections so that if there is
a build-id mismatch gdb refuses to use the solib. Or we could
add a parameter to debuginfod_exec_query that replaces "Continuing
without executable..." with some other string indicating that
gdb will use the local solib despite the mismatch.

Thanks,
Aaron

Aaron Merey (3):
  gdb/solib: Refactor scan_dyntag
  gdb: Add soname to build-id mapping for corefiles
  PR gdb/27570: missing support for debuginfod in
    core_target::build_file_mappings

 gdb/arch-utils.c                              |  21 ++-
 gdb/arch-utils.h                              |  21 ++-
 gdb/build-id.h                                |   2 +
 gdb/corelow.c                                 |  46 ++++-
 gdb/debuginfod-support.c                      |  46 +++++
 gdb/debuginfod-support.h                      |  16 ++
 gdb/gcore.in                                  |   3 +
 gdb/gdbarch.c                                 |   2 +-
 gdb/gdbarch.h                                 |   4 +-
 gdb/gdbarch.sh                                |   2 +-
 gdb/linux-tdep.c                              |  52 ++++--
 gdb/progspace.c                               |  36 ++++
 gdb/progspace.h                               |  17 ++
 gdb/solib-dsbt.c                              | 104 +----------
 gdb/solib-svr4.c                              | 118 +-----------
 gdb/solib.c                                   | 174 ++++++++++++++++++
 gdb/solib.h                                   |  11 ++
 .../gdb.debuginfod/fetch_src_and_symbols.exp  |  25 ++-
 18 files changed, 451 insertions(+), 249 deletions(-)

-- 
2.31.1

[PATCH 1/3] gdb/solib: Refactor scan_dyntag

[Aaron Merey]

scan_dyntag is unnecessarily duplicated in solib-svr4.c and solib-dsbt.c.

Move this function to solib.c and rename it to gdb_bfd_scan_elf_dyntag.
Also add it to solib.h so it is included in both solib-svr4 and solib-dsbt.
---
 gdb/solib-dsbt.c | 104 ++---------------------------------------
 gdb/solib-svr4.c | 118 ++++-------------------------------------------
 gdb/solib.c      | 104 +++++++++++++++++++++++++++++++++++++++++
 gdb/solib.h      |   6 +++
 4 files changed, 122 insertions(+), 210 deletions(-)

diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index 803467dd489..d7f4b252eee 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -396,106 +396,6 @@ fetch_loadmap (CORE_ADDR ldmaddr)
 static void dsbt_relocate_main_executable (void);
 static int enable_break (void);
 
-/* Scan for DYNTAG in .dynamic section of ABFD. If DYNTAG is found 1 is
-   returned and the corresponding PTR is set.  */
-
-static int
-scan_dyntag (int dyntag, bfd *abfd, CORE_ADDR *ptr)
-{
-  int arch_size, step, sect_size;
-  long dyn_tag;
-  CORE_ADDR dyn_ptr, dyn_addr;
-  gdb_byte *bufend, *bufstart, *buf;
-  Elf32_External_Dyn *x_dynp_32;
-  Elf64_External_Dyn *x_dynp_64;
-  struct bfd_section *sect;
-
-  if (abfd == NULL)
-    return 0;
-
-  if (bfd_get_flavour (abfd) != bfd_target_elf_flavour)
-    return 0;
-
-  arch_size = bfd_get_arch_size (abfd);
-  if (arch_size == -1)
-    return 0;
-
-  /* Find the start address of the .dynamic section.  */
-  sect = bfd_get_section_by_name (abfd, ".dynamic");
-  if (sect == NULL)
-    return 0;
-
-  bool found = false;
-  for (const target_section &target_section
-	 : current_program_space->target_sections ())
-    if (sect == target_section.the_bfd_section)
-      {
-	dyn_addr = target_section.addr;
-	found = true;
-	break;
-      }
-  if (!found)
-    {
-      /* ABFD may come from OBJFILE acting only as a symbol file without being
-	 loaded into the target (see add_symbol_file_command).  This case is
-	 such fallback to the file VMA address without the possibility of
-	 having the section relocated to its actual in-memory address.  */
-
-      dyn_addr = bfd_section_vma (sect);
-    }
-
-  /* Read in .dynamic from the BFD.  We will get the actual value
-     from memory later.  */
-  sect_size = bfd_section_size (sect);
-  buf = bufstart = (gdb_byte *) alloca (sect_size);
-  if (!bfd_get_section_contents (abfd, sect,
-				 buf, 0, sect_size))
-    return 0;
-
-  /* Iterate over BUF and scan for DYNTAG.  If found, set PTR and return.  */
-  step = (arch_size == 32) ? sizeof (Elf32_External_Dyn)
-			   : sizeof (Elf64_External_Dyn);
-  for (bufend = buf + sect_size;
-       buf < bufend;
-       buf += step)
-  {
-    if (arch_size == 32)
-      {
-	x_dynp_32 = (Elf32_External_Dyn *) buf;
-	dyn_tag = 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
-      {
-	x_dynp_64 = (Elf64_External_Dyn *) buf;
-	dyn_tag = 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 (dyn_tag == DT_NULL)
-       return 0;
-     if (dyn_tag == dyntag)
-       {
-	 /* If requested, try to read the runtime value of this .dynamic
-	    entry.  */
-	 if (ptr)
-	   {
-	     struct type *ptr_type;
-	     gdb_byte ptr_buf[8];
-	     CORE_ADDR ptr_addr;
-
-	     ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
-	     ptr_addr = dyn_addr + (buf - bufstart) + arch_size / 8;
-	     if (target_read_memory (ptr_addr, ptr_buf, arch_size / 8) == 0)
-	       dyn_ptr = extract_typed_address (ptr_buf, ptr_type);
-	     *ptr = dyn_ptr;
-	   }
-	 return 1;
-       }
-  }
-
-  return 0;
-}
-
 /* See solist.h. */
 
 static int
@@ -565,7 +465,9 @@ lm_base (void)
 			    "lm_base: get addr %x by _GLOBAL_OFFSET_TABLE_.\n",
 			    (unsigned int) addr);
     }
-  else if (scan_dyntag (DT_PLTGOT, current_program_space->exec_bfd (), &addr))
+  else if (gdb_bfd_scan_elf_dyntag (DT_PLTGOT,
+				    current_program_space->exec_bfd (),
+				    &addr, NULL))
     {
       struct int_elf32_dsbt_loadmap *ldm;
 
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index a8a7d1171dc..3de1bb9c7f7 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -582,109 +582,6 @@ find_program_interpreter (void)
 }
 
 
-/* Scan for DESIRED_DYNTAG in .dynamic section of ABFD.  If DESIRED_DYNTAG is
-   found, 1 is returned and the corresponding PTR is set.  */
-
-static int
-scan_dyntag (const int desired_dyntag, bfd *abfd, CORE_ADDR *ptr,
-	     CORE_ADDR *ptr_addr)
-{
-  int arch_size, step, sect_size;
-  long current_dyntag;
-  CORE_ADDR dyn_ptr, dyn_addr;
-  gdb_byte *bufend, *bufstart, *buf;
-  Elf32_External_Dyn *x_dynp_32;
-  Elf64_External_Dyn *x_dynp_64;
-  struct bfd_section *sect;
-
-  if (abfd == NULL)
-    return 0;
-
-  if (bfd_get_flavour (abfd) != bfd_target_elf_flavour)
-    return 0;
-
-  arch_size = bfd_get_arch_size (abfd);
-  if (arch_size == -1)
-    return 0;
-
-  /* Find the start address of the .dynamic section.  */
-  sect = bfd_get_section_by_name (abfd, ".dynamic");
-  if (sect == NULL)
-    return 0;
-
-  bool found = false;
-  for (const target_section &target_section
-	 : current_program_space->target_sections ())
-    if (sect == target_section.the_bfd_section)
-      {
-	dyn_addr = target_section.addr;
-	found = true;
-	break;
-      }
-  if (!found)
-    {
-      /* ABFD may come from OBJFILE acting only as a symbol file without being
-	 loaded into the target (see add_symbol_file_command).  This case is
-	 such fallback to the file VMA address without the possibility of
-	 having the section relocated to its actual in-memory address.  */
-
-      dyn_addr = bfd_section_vma (sect);
-    }
-
-  /* Read in .dynamic from the BFD.  We will get the actual value
-     from memory later.  */
-  sect_size = bfd_section_size (sect);
-  buf = bufstart = (gdb_byte *) alloca (sect_size);
-  if (!bfd_get_section_contents (abfd, sect,
-				 buf, 0, sect_size))
-    return 0;
-
-  /* Iterate over BUF and scan for DYNTAG.  If found, set PTR and return.  */
-  step = (arch_size == 32) ? sizeof (Elf32_External_Dyn)
-			   : sizeof (Elf64_External_Dyn);
-  for (bufend = buf + sect_size;
-       buf < bufend;
-       buf += step)
-  {
-    if (arch_size == 32)
-      {
-	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
-      {
-	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)
-       return 0;
-     if (current_dyntag == desired_dyntag)
-       {
-	 /* If requested, try to read the runtime value of this .dynamic
-	    entry.  */
-	 if (ptr)
-	   {
-	     struct type *ptr_type;
-	     gdb_byte ptr_buf[8];
-	     CORE_ADDR ptr_addr_1;
-
-	     ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
-	     ptr_addr_1 = dyn_addr + (buf - bufstart) + arch_size / 8;
-	     if (target_read_memory (ptr_addr_1, ptr_buf, arch_size / 8) == 0)
-	       dyn_ptr = extract_typed_address (ptr_buf, ptr_type);
-	     *ptr = dyn_ptr;
-	     if (ptr_addr)
-	       *ptr_addr = dyn_addr + (buf - bufstart);
-	   }
-	 return 1;
-       }
-  }
-
-  return 0;
-}
-
 /* Scan for DESIRED_DYNTAG in .dynamic section of the target's main executable,
    found by consulting the OS auxillary vector.  If DESIRED_DYNTAG is found, 1
    is returned and the corresponding PTR is set.  */
@@ -768,8 +665,9 @@ elf_locate_base (void)
   /* Look for DT_MIPS_RLD_MAP first.  MIPS executables use this
      instead of DT_DEBUG, although they sometimes contain an unused
      DT_DEBUG.  */
-  if (scan_dyntag (DT_MIPS_RLD_MAP, current_program_space->exec_bfd (),
-		   &dyn_ptr, NULL)
+  if (gdb_bfd_scan_elf_dyntag (DT_MIPS_RLD_MAP,
+			       current_program_space->exec_bfd (),
+			       &dyn_ptr, NULL)
       || scan_dyntag_auxv (DT_MIPS_RLD_MAP, &dyn_ptr, NULL))
     {
       struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
@@ -787,8 +685,9 @@ elf_locate_base (void)
   /* Then check DT_MIPS_RLD_MAP_REL.  MIPS executables now use this form
      because of needing to support PIE.  DT_MIPS_RLD_MAP will also exist
      in non-PIE.  */
-  if (scan_dyntag (DT_MIPS_RLD_MAP_REL, current_program_space->exec_bfd (),
-		   &dyn_ptr, &dyn_ptr_addr)
+  if (gdb_bfd_scan_elf_dyntag (DT_MIPS_RLD_MAP_REL,
+			       current_program_space->exec_bfd (),
+			       &dyn_ptr, &dyn_ptr_addr)
       || scan_dyntag_auxv (DT_MIPS_RLD_MAP_REL, &dyn_ptr, &dyn_ptr_addr))
     {
       struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
@@ -804,7 +703,8 @@ elf_locate_base (void)
     }
 
   /* Find DT_DEBUG.  */
-  if (scan_dyntag (DT_DEBUG, current_program_space->exec_bfd (), &dyn_ptr, NULL)
+  if (gdb_bfd_scan_elf_dyntag (DT_DEBUG, current_program_space->exec_bfd (),
+			       &dyn_ptr, NULL)
       || scan_dyntag_auxv (DT_DEBUG, &dyn_ptr, NULL))
     return dyn_ptr;
 
@@ -3258,7 +3158,7 @@ svr4_iterate_over_objfiles_in_search_order
 	abfd = current_objfile->obfd;
 
       if (abfd != nullptr
-	  && scan_dyntag (DT_SYMBOLIC, abfd, nullptr, nullptr) == 1)
+	  && gdb_bfd_scan_elf_dyntag (DT_SYMBOLIC, abfd, nullptr, nullptr) == 1)
 	{
 	  checked_current_objfile = true;
 	  if (cb (current_objfile, cb_data) != 0)
diff --git a/gdb/solib.c b/gdb/solib.c
index 317f7eb485e..e30affbb7e7 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -35,6 +35,8 @@
 #include "language.h"
 #include "gdbcmd.h"
 #include "completer.h"
+#include "elf/external.h"
+#include "elf/common.h"
 #include "filenames.h"		/* for DOSish file names */
 #include "exec.h"
 #include "solist.h"
@@ -1481,6 +1483,108 @@ gdb_bfd_lookup_symbol_from_symtab (bfd *abfd,
   return symaddr;
 }
 
+/* See solib.h.  */
+
+int
+gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd, CORE_ADDR *ptr,
+			 CORE_ADDR *ptr_addr)
+{
+  int arch_size, step, sect_size;
+  long current_dyntag;
+  CORE_ADDR dyn_ptr, dyn_addr;
+  gdb_byte *bufend, *bufstart, *buf;
+  Elf32_External_Dyn *x_dynp_32;
+  Elf64_External_Dyn *x_dynp_64;
+  struct bfd_section *sect;
+
+  if (abfd == NULL)
+    return 0;
+
+  if (bfd_get_flavour (abfd) != bfd_target_elf_flavour)
+    return 0;
+
+  arch_size = bfd_get_arch_size (abfd);
+  if (arch_size == -1)
+    return 0;
+
+  /* Find the start address of the .dynamic section.  */
+  sect = bfd_get_section_by_name (abfd, ".dynamic");
+  if (sect == NULL)
+    return 0;
+
+  bool found = false;
+  for (const target_section &target_section
+	 : current_program_space->target_sections ())
+    if (sect == target_section.the_bfd_section)
+      {
+	dyn_addr = target_section.addr;
+	found = true;
+	break;
+      }
+  if (!found)
+    {
+      /* ABFD may come from OBJFILE acting only as a symbol file without being
+	 loaded into the target (see add_symbol_file_command).  This case is
+	 such fallback to the file VMA address without the possibility of
+	 having the section relocated to its actual in-memory address.  */
+
+      dyn_addr = bfd_section_vma (sect);
+    }
+
+  /* Read in .dynamic from the BFD.  We will get the actual value
+     from memory later.  */
+  sect_size = bfd_section_size (sect);
+  buf = bufstart = (gdb_byte *) alloca (sect_size);
+  if (!bfd_get_section_contents (abfd, sect,
+				 buf, 0, sect_size))
+    return 0;
+
+  /* Iterate over BUF and scan for DYNTAG.  If found, set PTR and return.  */
+  step = (arch_size == 32) ? sizeof (Elf32_External_Dyn)
+			   : sizeof (Elf64_External_Dyn);
+  for (bufend = buf + sect_size;
+       buf < bufend;
+       buf += step)
+  {
+    if (arch_size == 32)
+      {
+	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
+      {
+	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)
+      return 0;
+    if (current_dyntag == desired_dyntag)
+      {
+	/* If requested, try to read the runtime value of this .dynamic
+	   entry.  */
+	if (ptr)
+	  {
+	    struct type *ptr_type;
+	    gdb_byte ptr_buf[8];
+	    CORE_ADDR ptr_addr_1;
+
+	    ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
+	    ptr_addr_1 = dyn_addr + (buf - bufstart) + arch_size / 8;
+	    if (target_read_memory (ptr_addr_1, ptr_buf, arch_size / 8) == 0)
+	      dyn_ptr = extract_typed_address (ptr_buf, ptr_type);
+	    *ptr = dyn_ptr;
+	    if (ptr_addr)
+	      *ptr_addr = dyn_addr + (buf - bufstart);
+	  }
+	return 1;
+      }
+  }
+
+  return 0;
+}
+
 /* Lookup the value for a specific symbol from symbol table.  Look up symbol
    from ABFD.  MATCH_SYM is a callback function to determine whether to pick
    up a symbol.  DATA is the input of this callback function.  Return NULL
diff --git a/gdb/solib.h b/gdb/solib.h
index a94e9d3cd9e..c50f74e06bf 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -112,6 +112,12 @@ extern CORE_ADDR gdb_bfd_lookup_symbol_from_symtab (bfd *abfd,
 						       const void *),
 						    const void *data);
 
+/* Scan for DESIRED_DYNTAG in .dynamic section of ABFD.  If DESIRED_DYNTAG is
+   found, 1 is returned and the corresponding PTR and PTR_ADDR are set.  */
+
+extern int gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd,
+				    CORE_ADDR *ptr, CORE_ADDR *ptr_addr);
+
 /* Enable or disable optional solib event breakpoints as appropriate.  */
 
 extern void update_solib_breakpoints (void);
-- 
2.31.1

Re: [PATCH 1/3] gdb/solib: Refactor scan_dyntag

[Simon Marchi]

On 2021-08-12 12:24 a.m., Aaron Merey wrote:
> scan_dyntag is unnecessarily duplicated in solib-svr4.c and solib-dsbt.c.
> 
> Move this function to solib.c and rename it to gdb_bfd_scan_elf_dyntag.
> Also add it to solib.h so it is included in both solib-svr4 and solib-dsbt.

Thanks, this is OK, and I think it can be merged on its own.

Simon

Re: [PATCH 1/3] gdb/solib: Refactor scan_dyntag

[Aaron Merey]

On Tue, Aug 17, 2021 at 9:29 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
> Thanks, this is OK, and I think it can be merged on its own.

Thanks, pushed as commit 8ddf46454aa

Aaron

[PATCH 2/3] gdb: Add soname to build-id mapping for corefiles

[Aaron Merey]

Since commit aa2d5a422 gdb has been able to read executable and shared
library build-ids within core files.

Expand this functionality so that each program_space maintains a map of
sonames to build-ids for each shared library referenced in the program_space's
core file.

This feature may be used to verify that gdb has found the correct shared
libraries for core files and to facilitate downloading shared libaries via
debuginfod.
---
 gdb/arch-utils.c | 21 +++++++++----------
 gdb/arch-utils.h | 21 +++++++++----------
 gdb/build-id.h   |  2 ++
 gdb/corelow.c    | 13 +++++++++++-
 gdb/gdbarch.c    |  2 +-
 gdb/gdbarch.h    |  4 ++--
 gdb/gdbarch.sh   |  2 +-
 gdb/linux-tdep.c | 52 +++++++++++++++++++++++++++++++++++++-----------
 gdb/progspace.c  | 36 +++++++++++++++++++++++++++++++++
 gdb/progspace.h  | 17 ++++++++++++++++
 gdb/solib.c      | 35 ++++++++++++++++++++++++++++++++
 gdb/solib.h      |  5 +++++
 12 files changed, 173 insertions(+), 37 deletions(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 4290d637ce1..4c7497e6b4c 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -1072,16 +1072,17 @@ default_get_pc_address_flags (frame_info *frame, CORE_ADDR pc)
 
 /* See arch-utils.h.  */
 void
-default_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)
+default_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)
 {
 }
 
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index 03e9082f6d7..9139438c5fd 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -295,14 +295,15 @@ extern std::string default_get_pc_address_flags (frame_info *frame,
 						 CORE_ADDR pc);
 
 /* Default implementation of gdbarch read_core_file_mappings method.  */
-extern void default_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);
+extern void default_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);
 #endif /* ARCH_UTILS_H */
diff --git a/gdb/build-id.h b/gdb/build-id.h
index 42f8d57ede1..3c9402ee71b 100644
--- a/gdb/build-id.h
+++ b/gdb/build-id.h
@@ -20,8 +20,10 @@
 #ifndef BUILD_ID_H
 #define BUILD_ID_H
 
+#include "defs.h"
 #include "gdb_bfd.h"
 #include "gdbsupport/rsp-low.h"
+#include <string>
 
 /* Locate NT_GNU_BUILD_ID from ABFD and return its content.  */
 
diff --git a/gdb/corelow.c b/gdb/corelow.c
index eb785a08633..97eadceed84 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -214,7 +214,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.  */
@@ -282,6 +282,16 @@ core_target::build_file_mappings ()
 
 	/* Set target_section fields.  */
 	m_core_file_mappings.emplace_back (start, end, sec);
+
+	/* If this is a bfd of a shared library, record its soname
+	   and build id.  */
+	if (build_id != nullptr)
+	  {
+	    gdb::optional<std::string> soname = gdb_bfd_read_elf_soname (bfd);
+	    if (soname)
+	      current_program_space->set_cbfd_soname_build_id (soname->data (),
+							       build_id);
+	  }
       });
 
   normalize_mem_ranges (&m_core_unavailable_mappings);
@@ -305,6 +315,7 @@ core_target::close ()
 	 comments in clear_solib in solib.c.  */
       clear_solib ();
 
+      current_program_space->clear_cbfd_soname_build_ids ();
       current_program_space->cbfd.reset (nullptr);
     }
 
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 830a86df89f..b6472bb36d5 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 7db3e36d76a..dbd1fa0afc7 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1710,8 +1710,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 9bc9de91c30..56679b8fee6 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1210,7 +1210,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 637d3d36a0b..eb35a2b5297 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
@@ -1096,16 +1097,17 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
    for each mapping.  */
 
 static void
-linux_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)
+linux_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)
 {
   /* Ensure that ULONGEST is big enough for reading 64-bit core files.  */
   gdb_static_assert (sizeof (ULONGEST) >= 8);
@@ -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 = vma_map[start];
+
+      /* 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 != 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/progspace.c b/gdb/progspace.c
index 7080bf8ee27..d39bd45fcf4 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -17,6 +17,7 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#include "build-id.h"
 #include "defs.h"
 #include "gdbcmd.h"
 #include "objfiles.h"
@@ -358,6 +359,41 @@ print_program_space (struct ui_out *uiout, int requested)
     }
 }
 
+/* See progspace.h.  */
+
+void
+program_space::set_cbfd_soname_build_id (std::string soname,
+					 const bfd_build_id *build_id)
+{
+  std::string build_id_hex = build_id_to_string (build_id);
+  cbfd_soname_to_build_id[soname] = build_id_hex;
+
+  return;
+}
+
+/* See progspace.h.  */
+
+const char *
+program_space::get_cbfd_soname_build_id (const char *soname)
+{
+  gdb_assert (soname);
+
+  auto it = cbfd_soname_to_build_id.find (basename (soname));
+  if (it == cbfd_soname_to_build_id.end ())
+    return nullptr;
+
+  return it->second.c_str ();
+}
+
+/* See progspace.h.  */
+
+void
+program_space::clear_cbfd_soname_build_ids ()
+{
+  cbfd_soname_to_build_id.clear ();
+  return;
+}
+
 /* Boolean test for an already-known program space id.  */
 
 static int
diff --git a/gdb/progspace.h b/gdb/progspace.h
index fb348ca7539..b42b3ffc4f1 100644
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -30,6 +30,7 @@
 #include "gdbsupport/safe-iterator.h"
 #include <list>
 #include <vector>
+#include <unordered_map>
 
 struct target_ops;
 struct bfd;
@@ -324,6 +325,19 @@ struct program_space
   /* Binary file diddling handle for the core file.  */
   gdb_bfd_ref_ptr cbfd;
 
+  /* Associate a core file SONAME with BUILD_ID so that it can be retrieved
+     with get_cbfd_soname_build_id.  */
+  void set_cbfd_soname_build_id (std::string soname,
+				 const bfd_build_id *build_id);
+
+  /* If a core file SONAME had a build-id associated with it by a previous
+     call to set_cbfd_soname_build_id then return the build-id as a
+     NULL-terminated hex string.  */
+  const char *get_cbfd_soname_build_id (const char *soname);
+
+  /* Clear all core file soname to build-id mappings.  */
+  void clear_cbfd_soname_build_ids ();
+
   /* The address space attached to this program space.  More than one
      program space may be bound to the same address space.  In the
      traditional unix-like debugging scenario, this will usually
@@ -378,6 +392,9 @@ struct program_space
   /* The set of target sections matching the sections mapped into
      this program space.  Managed by both exec_ops and solib.c.  */
   target_section_table m_target_sections;
+
+  /* Mapping of a core file's library sonames to their respective build-ids.  */
+  std::unordered_map<std::string, std::string> cbfd_soname_to_build_id;
 };
 
 /* An address space.  It is used for comparing if
diff --git a/gdb/solib.c b/gdb/solib.c
index e30affbb7e7..8b92cf7db53 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -23,6 +23,7 @@
 #include <fcntl.h>
 #include "symtab.h"
 #include "bfd.h"
+#include "build-id.h"
 #include "symfile.h"
 #include "objfiles.h"
 #include "gdbcore.h"
@@ -1585,6 +1586,40 @@ gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd, CORE_ADDR *ptr,
   return 0;
 }
 
+/* See solib.h.  */
+
+gdb::optional<std::string>
+gdb_bfd_read_elf_soname (struct bfd *bfd)
+{
+  gdb_assert (bfd != nullptr);
+
+  gdb_bfd_ref_ptr abfd = gdb_bfd_open (bfd->filename, gnutarget);
+
+  if (abfd == nullptr)
+    return gdb::optional<std::string> ();
+
+  /* Check that bfd is an ET_DYN ELF file.  */
+  bfd_check_format (abfd.get (), bfd_object);
+  if (!(bfd_get_file_flags (abfd.get ()) & DYNAMIC))
+    return gdb::optional<std::string> ();
+
+  /* Determine soname of shared library.  If found map soname to build-id.  */
+  CORE_ADDR idx;
+  if (!gdb_bfd_scan_elf_dyntag (DT_SONAME, abfd.get (), &idx, nullptr))
+    return gdb::optional<std::string> ();
+
+  struct bfd_section *dynstr = bfd_get_section_by_name (abfd.get (), ".dynstr");
+  if (dynstr == nullptr)
+    return gdb::optional<std::string> ();
+
+  /* Read the soname from the string table.  */
+  gdb::byte_vector dynstr_buf;
+  if (!gdb_bfd_get_full_section_contents (abfd.get (), dynstr, &dynstr_buf))
+    return gdb::optional<std::string> ();
+
+  return gdb::optional<std::string> ((char *)dynstr_buf.data () + idx);
+}
+
 /* Lookup the value for a specific symbol from symbol table.  Look up symbol
    from ABFD.  MATCH_SYM is a callback function to determine whether to pick
    up a symbol.  DATA is the input of this callback function.  Return NULL
diff --git a/gdb/solib.h b/gdb/solib.h
index c50f74e06bf..51cc047463f 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -118,6 +118,11 @@ extern CORE_ADDR gdb_bfd_lookup_symbol_from_symtab (bfd *abfd,
 extern int gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd,
 				    CORE_ADDR *ptr, CORE_ADDR *ptr_addr);
 
+/* If BFD is an ELF shared object then attempt to return the string
+   referred to by its DT_SONAME tag.   */
+
+extern gdb::optional<std::string> gdb_bfd_read_elf_soname (struct bfd *bfd);
+
 /* Enable or disable optional solib event breakpoints as appropriate.  */
 
 extern void update_solib_breakpoints (void);
-- 
2.31.1

Re: [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles

[Lancelot SIX]

Hi,

I have a few comments I placed bellow.

On Thu, Aug 12, 2021 at 12:24:05AM -0400, Aaron Merey via Gdb-patches wrote:
> Since commit aa2d5a422 gdb has been able to read executable and shared
> library build-ids within core files.
> 
> Expand this functionality so that each program_space maintains a map of
> sonames to build-ids for each shared library referenced in the program_space's
> core file.
> 
> This feature may be used to verify that gdb has found the correct shared
> libraries for core files and to facilitate downloading shared libaries via
> debuginfod.
> ---
>  gdb/arch-utils.c | 21 +++++++++----------
>  gdb/arch-utils.h | 21 +++++++++----------
>  gdb/build-id.h   |  2 ++
>  gdb/corelow.c    | 13 +++++++++++-
>  gdb/gdbarch.c    |  2 +-
>  gdb/gdbarch.h    |  4 ++--
>  gdb/gdbarch.sh   |  2 +-
>  gdb/linux-tdep.c | 52 +++++++++++++++++++++++++++++++++++++-----------
>  gdb/progspace.c  | 36 +++++++++++++++++++++++++++++++++
>  gdb/progspace.h  | 17 ++++++++++++++++
>  gdb/solib.c      | 35 ++++++++++++++++++++++++++++++++
>  gdb/solib.h      |  5 +++++
>  12 files changed, 173 insertions(+), 37 deletions(-)
> 
> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
> index 4290d637ce1..4c7497e6b4c 100644
> --- a/gdb/arch-utils.c
> +++ b/gdb/arch-utils.c
> @@ -1072,16 +1072,17 @@ default_get_pc_address_flags (frame_info *frame, CORE_ADDR pc)
>  
>  /* See arch-utils.h.  */
>  void
> -default_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)
> +default_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)

It looks like 'loop_cb' could go on the previous line.

If the type of the function callbacks are too big, I guess it could be
possible to give them a name before declaring the function.  Something
like

    using loop_cb_ftype = gdb::function_view<void (...)>;

>  {
>  }
>  
> diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
> index 03e9082f6d7..9139438c5fd 100644
> --- a/gdb/arch-utils.h
> +++ b/gdb/arch-utils.h
> @@ -295,14 +295,15 @@ extern std::string default_get_pc_address_flags (frame_info *frame,
>  						 CORE_ADDR pc);
>  
>  /* Default implementation of gdbarch read_core_file_mappings method.  */
> -extern void default_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);
> +extern void default_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);

loop_cb could also go up one line here.

>  #endif /* ARCH_UTILS_H */
> diff --git a/gdb/build-id.h b/gdb/build-id.h
> index 42f8d57ede1..3c9402ee71b 100644
> --- a/gdb/build-id.h
> +++ b/gdb/build-id.h
> @@ -20,8 +20,10 @@
>  #ifndef BUILD_ID_H
>  #define BUILD_ID_H
>  
> +#include "defs.h"
>  #include "gdb_bfd.h"
>  #include "gdbsupport/rsp-low.h"
> +#include <string>
>  
>  /* Locate NT_GNU_BUILD_ID from ABFD and return its content.  */
>  
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index eb785a08633..97eadceed84 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -214,7 +214,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.  */
> @@ -282,6 +282,16 @@ core_target::build_file_mappings ()
>  
>  	/* Set target_section fields.  */
>  	m_core_file_mappings.emplace_back (start, end, sec);
> +
> +	/* If this is a bfd of a shared library, record its soname
> +	   and build id.  */
> +	if (build_id != nullptr)
> +	  {
> +	    gdb::optional<std::string> soname = gdb_bfd_read_elf_soname (bfd);
> +	    if (soname)
> +	      current_program_space->set_cbfd_soname_build_id (soname->data (),
> +							       build_id);

Here, since set_cbfd_soname_build_id's first argument is a std::string,
you could just use '*soname' instead of 'soname->data ()'.

> +	  }
>        });
>  
>    normalize_mem_ranges (&m_core_unavailable_mappings);
> @@ -305,6 +315,7 @@ core_target::close ()
>  	 comments in clear_solib in solib.c.  */
>        clear_solib ();
>  
> +      current_program_space->clear_cbfd_soname_build_ids ();
>        current_program_space->cbfd.reset (nullptr);
>      }
>  
> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> index 830a86df89f..b6472bb36d5 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 7db3e36d76a..dbd1fa0afc7 100644
> --- a/gdb/gdbarch.h
> +++ b/gdb/gdbarch.h
> @@ -1710,8 +1710,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 9bc9de91c30..56679b8fee6 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -1210,7 +1210,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 637d3d36a0b..eb35a2b5297 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
> @@ -1096,16 +1097,17 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
>     for each mapping.  */
>  
>  static void
> -linux_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)
> +linux_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)

'loop_cb' could be on the line above.

>  {
>    /* Ensure that ULONGEST is big enough for reading 64-bit core files.  */
>    gdb_static_assert (sizeof (ULONGEST) >= 8);
> @@ -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 = vma_map[start];
> +
> +      /* 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 != 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/progspace.c b/gdb/progspace.c
> index 7080bf8ee27..d39bd45fcf4 100644
> --- a/gdb/progspace.c
> +++ b/gdb/progspace.c
> @@ -17,6 +17,7 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
> +#include "build-id.h"
>  #include "defs.h"
>  #include "gdbcmd.h"
>  #include "objfiles.h"
> @@ -358,6 +359,41 @@ print_program_space (struct ui_out *uiout, int requested)
>      }
>  }
>  
> +/* See progspace.h.  */
> +
> +void
> +program_space::set_cbfd_soname_build_id (std::string soname,

This parameter could be 'std::string const &' or...

> +					 const bfd_build_id *build_id)
> +{
> +  std::string build_id_hex = build_id_to_string (build_id);
> +  cbfd_soname_to_build_id[soname] = build_id_hex;

... use 'std::move (soname)' here.

I guess the more 'usual' approach would be to have the argument as a
const reference (but to be honest, the implication of calling one more
ctor and copying the soname is negligible, to say the least).

> +
> +  return;

I am not sure if the GNU coding standard says something about this, but
'return;' as the last statement of a void function is redundant.

> +}
> +
> +/* See progspace.h.  */
> +
> +const char *
> +program_space::get_cbfd_soname_build_id (const char *soname)

With set_cbfd_soname_build_id using a std::string, I would find it more
consistent to use std::string here also.  Any reason not to use it I
missed?

You could use 'basename (soname.c_str ())' bellow.

The return type could also be 'const std::string *' (the map stores
std::string internally), but keeping a const char * is pretty similar.

> +{
> +  gdb_assert (soname);
> +
> +  auto it = cbfd_soname_to_build_id.find (basename (soname));
> +  if (it == cbfd_soname_to_build_id.end ())
> +    return nullptr;
> +
> +  return it->second.c_str ();
> +}
> +
> +/* See progspace.h.  */
> +
> +void
> +program_space::clear_cbfd_soname_build_ids ()
> +{
> +  cbfd_soname_to_build_id.clear ();
> +  return;

Same here, I guess 'return;' could be removed.

> +}
> +
>  /* Boolean test for an already-known program space id.  */
>  
>  static int
> diff --git a/gdb/progspace.h b/gdb/progspace.h
> index fb348ca7539..b42b3ffc4f1 100644
> --- a/gdb/progspace.h
> +++ b/gdb/progspace.h
> @@ -30,6 +30,7 @@
>  #include "gdbsupport/safe-iterator.h"
>  #include <list>
>  #include <vector>
> +#include <unordered_map>
>  
>  struct target_ops;
>  struct bfd;
> @@ -324,6 +325,19 @@ struct program_space
>    /* Binary file diddling handle for the core file.  */
>    gdb_bfd_ref_ptr cbfd;
>  
> +  /* Associate a core file SONAME with BUILD_ID so that it can be retrieved
> +     with get_cbfd_soname_build_id.  */
> +  void set_cbfd_soname_build_id (std::string soname,
> +				 const bfd_build_id *build_id);
> +
> +  /* If a core file SONAME had a build-id associated with it by a previous
> +     call to set_cbfd_soname_build_id then return the build-id as a
> +     NULL-terminated hex string.  */
> +  const char *get_cbfd_soname_build_id (const char *soname);
> +
> +  /* Clear all core file soname to build-id mappings.  */
> +  void clear_cbfd_soname_build_ids ();
> +
>    /* The address space attached to this program space.  More than one
>       program space may be bound to the same address space.  In the
>       traditional unix-like debugging scenario, this will usually
> @@ -378,6 +392,9 @@ struct program_space
>    /* The set of target sections matching the sections mapped into
>       this program space.  Managed by both exec_ops and solib.c.  */
>    target_section_table m_target_sections;
> +
> +  /* Mapping of a core file's library sonames to their respective build-ids.  */
> +  std::unordered_map<std::string, std::string> cbfd_soname_to_build_id;
>  };
>  
>  /* An address space.  It is used for comparing if
> diff --git a/gdb/solib.c b/gdb/solib.c
> index e30affbb7e7..8b92cf7db53 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -23,6 +23,7 @@
>  #include <fcntl.h>
>  #include "symtab.h"
>  #include "bfd.h"
> +#include "build-id.h"
>  #include "symfile.h"
>  #include "objfiles.h"
>  #include "gdbcore.h"
> @@ -1585,6 +1586,40 @@ gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd, CORE_ADDR *ptr,
>    return 0;
>  }
>  
> +/* See solib.h.  */
> +
> +gdb::optional<std::string>
> +gdb_bfd_read_elf_soname (struct bfd *bfd)
> +{
> +  gdb_assert (bfd != nullptr);
> +
> +  gdb_bfd_ref_ptr abfd = gdb_bfd_open (bfd->filename, gnutarget);
> +
> +  if (abfd == nullptr)
> +    return gdb::optional<std::string> ();
> +
> +  /* Check that bfd is an ET_DYN ELF file.  */
> +  bfd_check_format (abfd.get (), bfd_object);
> +  if (!(bfd_get_file_flags (abfd.get ()) & DYNAMIC))
> +    return gdb::optional<std::string> ();
> +
> +  /* Determine soname of shared library.  If found map soname to build-id.  */
> +  CORE_ADDR idx;
> +  if (!gdb_bfd_scan_elf_dyntag (DT_SONAME, abfd.get (), &idx, nullptr))
> +    return gdb::optional<std::string> ();
> +
> +  struct bfd_section *dynstr = bfd_get_section_by_name (abfd.get (), ".dynstr");
> +  if (dynstr == nullptr)
> +    return gdb::optional<std::string> ();
> +
> +  /* Read the soname from the string table.  */
> +  gdb::byte_vector dynstr_buf;
> +  if (!gdb_bfd_get_full_section_contents (abfd.get (), dynstr, &dynstr_buf))
> +    return gdb::optional<std::string> ();
> +
> +  return gdb::optional<std::string> ((char *)dynstr_buf.data () + idx);

This will not change much, but you could cast to 'const char *' (this
is the type the std::string constructor expects).

> +}
> +
>  /* Lookup the value for a specific symbol from symbol table.  Look up symbol
>     from ABFD.  MATCH_SYM is a callback function to determine whether to pick
>     up a symbol.  DATA is the input of this callback function.  Return NULL
> diff --git a/gdb/solib.h b/gdb/solib.h
> index c50f74e06bf..51cc047463f 100644
> --- a/gdb/solib.h
> +++ b/gdb/solib.h
> @@ -118,6 +118,11 @@ extern CORE_ADDR gdb_bfd_lookup_symbol_from_symtab (bfd *abfd,
>  extern int gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd,
>  				    CORE_ADDR *ptr, CORE_ADDR *ptr_addr);
>  
> +/* If BFD is an ELF shared object then attempt to return the string
> +   referred to by its DT_SONAME tag.   */
> +
> +extern gdb::optional<std::string> gdb_bfd_read_elf_soname (struct bfd *bfd);
> +
>  /* Enable or disable optional solib event breakpoints as appropriate.  */
>  
>  extern void update_solib_breakpoints (void);
> -- 
> 2.31.1
> 

I hope the comments are helpful.

Best,
Lancelot.

Re: [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles

[Simon Marchi]

I did not do a full review, since Lancelot's comments were already a
good start.  I just added some complements to his comments.

>>  /* See arch-utils.h.  */
>>  void
>> -default_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)
>> +default_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)
> 
> It looks like 'loop_cb' could go on the previous line.
> 
> If the type of the function callbacks are too big, I guess it could be
> possible to give them a name before declaring the function.  Something
> like
> 
>     using loop_cb_ftype = gdb::function_view<void (...)>;

Agreed, and giving a name to the function type helps readability too.

>> @@ -282,6 +282,16 @@ core_target::build_file_mappings ()
>>  
>>  	/* Set target_section fields.  */
>>  	m_core_file_mappings.emplace_back (start, end, sec);
>> +
>> +	/* If this is a bfd of a shared library, record its soname
>> +	   and build id.  */
>> +	if (build_id != nullptr)
>> +	  {
>> +	    gdb::optional<std::string> soname = gdb_bfd_read_elf_soname (bfd);
>> +	    if (soname)
>> +	      current_program_space->set_cbfd_soname_build_id (soname->data (),
>> +							       build_id);
> 
> Here, since set_cbfd_soname_build_id's first argument is a std::string,
> you could just use '*soname' instead of 'soname->data ()'.

Since the caller (here) doesn't need to keep the string, I would suggest
`std::move (*soname)` (see below).

>> @@ -358,6 +359,41 @@ print_program_space (struct ui_out *uiout, int requested)
>>      }
>>  }
>>  
>> +/* See progspace.h.  */
>> +
>> +void
>> +program_space::set_cbfd_soname_build_id (std::string soname,
> 
> This parameter could be 'std::string const &' or...
> 
>> +					 const bfd_build_id *build_id)
>> +{
>> +  std::string build_id_hex = build_id_to_string (build_id);
>> +  cbfd_soname_to_build_id[soname] = build_id_hex;
> 
> ... use 'std::move (soname)' here.
> 
> I guess the more 'usual' approach would be to have the argument as a
> const reference (but to be honest, the implication of calling one more
> ctor and copying the soname is negligible, to say the least).

Since set_cbfd_soname_build_id needs to keep its own copy of the object,
I think it's better to take it by value like this, and have the caller
std::move its copy if it doesn't need to keep it around.  And have
set_cbfd_soname_build_id std::move it into the map:

  cbfd_soname_to_build_id[std::move (soname)] = build_id_hex;

This way, soname is never copied if it doesn't need to.  Taking by
const-reference means that set_cbfd_soname_build_id will have no choice
but to copy the string into the map.

And in fact, I would suggest doing:

  cbfd_soname_to_build_id[std::move (soname)]
    = build_id_to_string (build_id);

... to avoid copying the build_id_hex string.  Alternatively, you could
do:

  std::string build_id_hex = build_id_to_string (build_id);
  cbfd_soname_to_build_id[std::move (soname)] = std::move (build_id_hex);

... but I think the first version is fine.


>> +
>> +  return;
> 
> I am not sure if the GNU coding standard says something about this, but
> 'return;' as the last statement of a void function is redundant.

Indeed.

>> +}
>> +
>> +/* See progspace.h.  */
>> +
>> +const char *
>> +program_space::get_cbfd_soname_build_id (const char *soname)
> 
> With set_cbfd_soname_build_id using a std::string, I would find it more
> consistent to use std::string here also.  Any reason not to use it I
> missed?
> 
> You could use 'basename (soname.c_str ())' bellow.
> 
> The return type could also be 'const std::string *' (the map stores
> std::string internally), but keeping a const char * is pretty similar.

We wouldn't want to return an std::string, that would make an
unnecessary copy.  If returning a `const char *` doesn't make life
difficult for the caller, I think it's perfectly fine.  Previously, I
would have suggested returning string_view, because that abstracts how
the string is stored.  But that isn't right, since the string_view
interface doesn't require the view to be null-terminated (which we
usually need).  We would need something like cstring_view instead:

  http://open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1402r0.pdf

But in the mean time, I think `const char *` is fine.

>> +gdb_bfd_read_elf_soname (struct bfd *bfd)
>> +{
>> +  gdb_assert (bfd != nullptr);
>> +
>> +  gdb_bfd_ref_ptr abfd = gdb_bfd_open (bfd->filename, gnutarget);
>> +
>> +  if (abfd == nullptr)
>> +    return gdb::optional<std::string> ();
>> +
>> +  /* Check that bfd is an ET_DYN ELF file.  */
>> +  bfd_check_format (abfd.get (), bfd_object);

I asked this in my previous review, still applies here:

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

>> +  if (!(bfd_get_file_flags (abfd.get ()) & DYNAMIC))
>> +    return gdb::optional<std::string> ();
>> +
>> +  /* Determine soname of shared library.  If found map soname to build-id.  */
>> +  CORE_ADDR idx;
>> +  if (!gdb_bfd_scan_elf_dyntag (DT_SONAME, abfd.get (), &idx, nullptr))
>> +    return gdb::optional<std::string> ();
>> +
>> +  struct bfd_section *dynstr = bfd_get_section_by_name (abfd.get (), ".dynstr");
>> +  if (dynstr == nullptr)
>> +    return gdb::optional<std::string> ();
>> +
>> +  /* Read the soname from the string table.  */
>> +  gdb::byte_vector dynstr_buf;
>> +  if (!gdb_bfd_get_full_section_contents (abfd.get (), dynstr, &dynstr_buf))
>> +    return gdb::optional<std::string> ();
>> +
>> +  return gdb::optional<std::string> ((char *)dynstr_buf.data () + idx);

Do we need bounds checking to protect against bad ELF files?  What if
DT_SONAME points outside .dynstr's size?  And ideally that a
null-terminator is found somewhere between idx and the end of the
section.  Otherwise, I could craft a .dynstr section that does not
contain a null terminator, that would make GDB read out of bounds.

> This will not change much, but you could cast to 'const char *' (this
> is the type the std::string constructor expects).

And to shorten things up a bit, you can drop "gdb::optional":

  return std::string ((const char *) dynstr_buf.data () + idx);

While at it, instead of returning "gdb::optional<std::string> ()" on
failure, you can simply "return {}".

Simon

Re: [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles

[Aaron Merey]

Hi Lancelot and Simon,

Thanks for the reviews. The updated patch is included below.

On Tue, Aug 17, 2021 at 9:59 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
>>> +  /* Check that bfd is an ET_DYN ELF file.  */  
>>> +  bfd_check_format (abfd.get (), bfd_object);
>
> I asked this in my previous review, still applies here:
>
>   What's the point of calling bfd_check_format without checking the 
>   result?  It looks like a function without side-effects.

I included a few comments in the [PATCH 0/3] email for this series[1].
I mentioned that for some reason bfd_check_format appears to update
the bfd 'flags' field with the correct value. If bfd_check_format is not 
called here the check for the DYNAMIC flag fails even for ET_DYN files.

[1] https://sourceware.org/pipermail/gdb-patches/2021-August/181416.html

---
 gdb/arch-utils.c | 15 +++++----------
 gdb/arch-utils.h | 23 +++++++++++++----------
 gdb/build-id.h   |  2 ++
 gdb/corelow.c    | 13 ++++++++++++-
 gdb/gdbarch.c    |  2 +-
 gdb/gdbarch.h    |  4 ++--
 gdb/gdbarch.sh   |  2 +-
 gdb/linux-tdep.c | 46 ++++++++++++++++++++++++++++++++++------------
 gdb/progspace.c  | 32 ++++++++++++++++++++++++++++++++
 gdb/progspace.h  | 17 +++++++++++++++++
 gdb/solib.c      | 35 +++++++++++++++++++++++++++++++++++
 gdb/solib.h      |  5 +++++
 12 files changed, 159 insertions(+), 37 deletions(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 862f26b6cf7..ffb32cb203f 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -1075,16 +1075,11 @@ default_get_pc_address_flags (frame_info *frame, CORE_ADDR pc)
 
 /* See arch-utils.h.  */
 void
-default_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)
+default_read_core_file_mappings
+ (struct gdbarch *gdbarch,
+  struct bfd *cbfd,
+  gdb::function_view<void (ULONGEST count)> pre_loop_cb,
+  loop_cb_ftype loop_cb)
 {
 }
 
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index 03e9082f6d7..2243c3fb85b 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -294,15 +294,18 @@ extern ULONGEST default_type_align (struct gdbarch *gdbarch,
 extern std::string default_get_pc_address_flags (frame_info *frame,
 						 CORE_ADDR pc);
 
+
+using loop_cb_ftype = gdb::function_view<void (int num,
+					       ULONGEST start,
+					       ULONGEST end,
+					       ULONGEST file_ofs,
+					       const char *filename,
+					       const bfd_build_id *build_id)>;
+
 /* Default implementation of gdbarch read_core_file_mappings method.  */
-extern void default_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);
+extern void default_read_core_file_mappings
+ (struct gdbarch *gdbarch,
+  struct bfd *cbfd,
+  gdb::function_view<void (ULONGEST count)> pre_loop_cb,
+  loop_cb_ftype loop_cb);
 #endif /* ARCH_UTILS_H */
diff --git a/gdb/build-id.h b/gdb/build-id.h
index 42f8d57ede1..3c9402ee71b 100644
--- a/gdb/build-id.h
+++ b/gdb/build-id.h
@@ -20,8 +20,10 @@
 #ifndef BUILD_ID_H
 #define BUILD_ID_H
 
+#include "defs.h"
 #include "gdb_bfd.h"
 #include "gdbsupport/rsp-low.h"
+#include <string>
 
 /* Locate NT_GNU_BUILD_ID from ABFD and return its content.  */
 
diff --git a/gdb/corelow.c b/gdb/corelow.c
index eb785a08633..31af0f22584 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -214,7 +214,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.  */
@@ -282,6 +282,16 @@ core_target::build_file_mappings ()
 
 	/* Set target_section fields.  */
 	m_core_file_mappings.emplace_back (start, end, sec);
+
+	/* If this is a bfd of a shared library, record its soname
+	   and build id.  */
+	if (build_id != nullptr)
+	  {
+	    gdb::optional<std::string> soname = gdb_bfd_read_elf_soname (bfd);
+	    if (soname)
+	      current_program_space->set_cbfd_soname_build_id 
+	       (std::move (*soname), build_id);
+	  }
       });
 
   normalize_mem_ranges (&m_core_unavailable_mappings);
@@ -305,6 +315,7 @@ core_target::close ()
 	 comments in clear_solib in solib.c.  */
       clear_solib ();
 
+      current_program_space->clear_cbfd_soname_build_ids ();
       current_program_space->cbfd.reset (nullptr);
     }
 
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index f89dcc57754..35464115039 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 979159ba2f5..baf80580d60 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1710,8 +1710,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 39a99d0d5f3..c24079d34f3 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1210,7 +1210,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 ae2f7c14f6d..812ca1f99bf 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -44,6 +44,7 @@
 #include "solib-svr4.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
@@ -1097,16 +1098,11 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
    for each mapping.  */
 
 static void
-linux_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)
+linux_read_core_file_mappings
+ (struct gdbarch *gdbarch,
+  struct bfd *cbfd,
+  gdb::function_view<void (ULONGEST count)> pre_loop_cb,
+  loop_cb_ftype  loop_cb)
 {
   /* Ensure that ULONGEST is big enough for reading 64-bit core files.  */
   gdb_static_assert (sizeof (ULONGEST) >= 8);
@@ -1175,6 +1171,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++)
@@ -1188,8 +1201,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 = vma_map[start];
+
+      /* 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 != 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);
     }
 }
 
@@ -1218,7 +1240,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/progspace.c b/gdb/progspace.c
index 7080bf8ee27..8b7b949d959 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -17,6 +17,7 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#include "build-id.h"
 #include "defs.h"
 #include "gdbcmd.h"
 #include "objfiles.h"
@@ -358,6 +359,37 @@ print_program_space (struct ui_out *uiout, int requested)
     }
 }
 
+/* See progspace.h.  */
+
+void
+program_space::set_cbfd_soname_build_id (std::string soname,
+					 const bfd_build_id *build_id)
+{
+  cbfd_soname_to_build_id[std::move (soname)] = build_id_to_string (build_id);
+}
+
+/* See progspace.h.  */
+
+const char *
+program_space::get_cbfd_soname_build_id (const char *soname)
+{
+  gdb_assert (soname);
+
+  auto it = cbfd_soname_to_build_id.find (basename (soname));
+  if (it == cbfd_soname_to_build_id.end ())
+    return nullptr;
+
+  return it->second.c_str ();
+}
+
+/* See progspace.h.  */
+
+void
+program_space::clear_cbfd_soname_build_ids ()
+{
+  cbfd_soname_to_build_id.clear ();
+}
+
 /* Boolean test for an already-known program space id.  */
 
 static int
diff --git a/gdb/progspace.h b/gdb/progspace.h
index fb348ca7539..b42b3ffc4f1 100644
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -30,6 +30,7 @@
 #include "gdbsupport/safe-iterator.h"
 #include <list>
 #include <vector>
+#include <unordered_map>
 
 struct target_ops;
 struct bfd;
@@ -324,6 +325,19 @@ struct program_space
   /* Binary file diddling handle for the core file.  */
   gdb_bfd_ref_ptr cbfd;
 
+  /* Associate a core file SONAME with BUILD_ID so that it can be retrieved
+     with get_cbfd_soname_build_id.  */
+  void set_cbfd_soname_build_id (std::string soname,
+				 const bfd_build_id *build_id);
+
+  /* If a core file SONAME had a build-id associated with it by a previous
+     call to set_cbfd_soname_build_id then return the build-id as a
+     NULL-terminated hex string.  */
+  const char *get_cbfd_soname_build_id (const char *soname);
+
+  /* Clear all core file soname to build-id mappings.  */
+  void clear_cbfd_soname_build_ids ();
+
   /* The address space attached to this program space.  More than one
      program space may be bound to the same address space.  In the
      traditional unix-like debugging scenario, this will usually
@@ -378,6 +392,9 @@ struct program_space
   /* The set of target sections matching the sections mapped into
      this program space.  Managed by both exec_ops and solib.c.  */
   target_section_table m_target_sections;
+
+  /* Mapping of a core file's library sonames to their respective build-ids.  */
+  std::unordered_map<std::string, std::string> cbfd_soname_to_build_id;
 };
 
 /* An address space.  It is used for comparing if
diff --git a/gdb/solib.c b/gdb/solib.c
index e30affbb7e7..1b99c8ab985 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -23,6 +23,7 @@
 #include <fcntl.h>
 #include "symtab.h"
 #include "bfd.h"
+#include "build-id.h"
 #include "symfile.h"
 #include "objfiles.h"
 #include "gdbcore.h"
@@ -1585,6 +1586,40 @@ gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd, CORE_ADDR *ptr,
   return 0;
 }
 
+/* See solib.h.  */
+
+gdb::optional<std::string>
+gdb_bfd_read_elf_soname (struct bfd *bfd)
+{
+  gdb_assert (bfd != nullptr);
+
+  gdb_bfd_ref_ptr abfd = gdb_bfd_open (bfd->filename, gnutarget);
+
+  if (abfd == nullptr)
+    return {};
+
+  /* Check that bfd is an ET_DYN ELF file.  */
+  bfd_check_format (abfd.get (), bfd_object);
+  if (!(bfd_get_file_flags (abfd.get ()) & DYNAMIC))
+    return {};
+
+  /* Determine soname of shared library.  If found map soname to build-id.  */
+  CORE_ADDR idx;
+  if (!gdb_bfd_scan_elf_dyntag (DT_SONAME, abfd.get (), &idx, nullptr))
+    return {};
+
+  struct bfd_section *dynstr = bfd_get_section_by_name (abfd.get (), ".dynstr");
+  if (dynstr == nullptr || bfd_section_size (dynstr) <= idx)
+    return {};
+
+  /* Read the soname from the string table.  */
+  gdb::byte_vector dynstr_buf;
+  if (!gdb_bfd_get_full_section_contents (abfd.get (), dynstr, &dynstr_buf))
+    return {};
+
+  return std::string ((const char *)dynstr_buf.data () + idx);
+}
+
 /* Lookup the value for a specific symbol from symbol table.  Look up symbol
    from ABFD.  MATCH_SYM is a callback function to determine whether to pick
    up a symbol.  DATA is the input of this callback function.  Return NULL
diff --git a/gdb/solib.h b/gdb/solib.h
index c50f74e06bf..51cc047463f 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -118,6 +118,11 @@ extern CORE_ADDR gdb_bfd_lookup_symbol_from_symtab (bfd *abfd,
 extern int gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd,
 				    CORE_ADDR *ptr, CORE_ADDR *ptr_addr);
 
+/* If BFD is an ELF shared object then attempt to return the string
+   referred to by its DT_SONAME tag.   */
+
+extern gdb::optional<std::string> gdb_bfd_read_elf_soname (struct bfd *bfd);
+
 /* Enable or disable optional solib event breakpoints as appropriate.  */
 
 extern void update_solib_breakpoints (void);
-- 
2.31.1

Re: [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles

[Aaron Merey]

Ping.

Thanks,
Aaron

On Wed, Aug 18, 2021 at 10:22 PM Aaron Merey via Gdb-patches
<gdb-patches@sourceware.org> wrote:
>
> Hi Lancelot and Simon,
>
> Thanks for the reviews. The updated patch is included below.
>
> On Tue, Aug 17, 2021 at 9:59 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
> >>> +  /* Check that bfd is an ET_DYN ELF file.  */
> >>> +  bfd_check_format (abfd.get (), bfd_object);
> >
> > I asked this in my previous review, still applies here:
> >
> >   What's the point of calling bfd_check_format without checking the
> >   result?  It looks like a function without side-effects.
>
> I included a few comments in the [PATCH 0/3] email for this series[1].
> I mentioned that for some reason bfd_check_format appears to update
> the bfd 'flags' field with the correct value. If bfd_check_format is not
> called here the check for the DYNAMIC flag fails even for ET_DYN files.
>
> [1] https://sourceware.org/pipermail/gdb-patches/2021-August/181416.html
>
> ---
>  gdb/arch-utils.c | 15 +++++----------
>  gdb/arch-utils.h | 23 +++++++++++++----------
>  gdb/build-id.h   |  2 ++
>  gdb/corelow.c    | 13 ++++++++++++-
>  gdb/gdbarch.c    |  2 +-
>  gdb/gdbarch.h    |  4 ++--
>  gdb/gdbarch.sh   |  2 +-
>  gdb/linux-tdep.c | 46 ++++++++++++++++++++++++++++++++++------------
>  gdb/progspace.c  | 32 ++++++++++++++++++++++++++++++++
>  gdb/progspace.h  | 17 +++++++++++++++++
>  gdb/solib.c      | 35 +++++++++++++++++++++++++++++++++++
>  gdb/solib.h      |  5 +++++
>  12 files changed, 159 insertions(+), 37 deletions(-)
>
> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
> index 862f26b6cf7..ffb32cb203f 100644
> --- a/gdb/arch-utils.c
> +++ b/gdb/arch-utils.c
> @@ -1075,16 +1075,11 @@ default_get_pc_address_flags (frame_info *frame, CORE_ADDR pc)
>
>  /* See arch-utils.h.  */
>  void
> -default_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)
> +default_read_core_file_mappings
> + (struct gdbarch *gdbarch,
> +  struct bfd *cbfd,
> +  gdb::function_view<void (ULONGEST count)> pre_loop_cb,
> +  loop_cb_ftype loop_cb)
>  {
>  }
>
> diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
> index 03e9082f6d7..2243c3fb85b 100644
> --- a/gdb/arch-utils.h
> +++ b/gdb/arch-utils.h
> @@ -294,15 +294,18 @@ extern ULONGEST default_type_align (struct gdbarch *gdbarch,
>  extern std::string default_get_pc_address_flags (frame_info *frame,
>                                                  CORE_ADDR pc);
>
> +
> +using loop_cb_ftype = gdb::function_view<void (int num,
> +                                              ULONGEST start,
> +                                              ULONGEST end,
> +                                              ULONGEST file_ofs,
> +                                              const char *filename,
> +                                              const bfd_build_id *build_id)>;
> +
>  /* Default implementation of gdbarch read_core_file_mappings method.  */
> -extern void default_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);
> +extern void default_read_core_file_mappings
> + (struct gdbarch *gdbarch,
> +  struct bfd *cbfd,
> +  gdb::function_view<void (ULONGEST count)> pre_loop_cb,
> +  loop_cb_ftype loop_cb);
>  #endif /* ARCH_UTILS_H */
> diff --git a/gdb/build-id.h b/gdb/build-id.h
> index 42f8d57ede1..3c9402ee71b 100644
> --- a/gdb/build-id.h
> +++ b/gdb/build-id.h
> @@ -20,8 +20,10 @@
>  #ifndef BUILD_ID_H
>  #define BUILD_ID_H
>
> +#include "defs.h"
>  #include "gdb_bfd.h"
>  #include "gdbsupport/rsp-low.h"
> +#include <string>
>
>  /* Locate NT_GNU_BUILD_ID from ABFD and return its content.  */
>
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index eb785a08633..31af0f22584 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -214,7 +214,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.  */
> @@ -282,6 +282,16 @@ core_target::build_file_mappings ()
>
>         /* Set target_section fields.  */
>         m_core_file_mappings.emplace_back (start, end, sec);
> +
> +       /* If this is a bfd of a shared library, record its soname
> +          and build id.  */
> +       if (build_id != nullptr)
> +         {
> +           gdb::optional<std::string> soname = gdb_bfd_read_elf_soname (bfd);
> +           if (soname)
> +             current_program_space->set_cbfd_soname_build_id
> +              (std::move (*soname), build_id);
> +         }
>        });
>
>    normalize_mem_ranges (&m_core_unavailable_mappings);
> @@ -305,6 +315,7 @@ core_target::close ()
>          comments in clear_solib in solib.c.  */
>        clear_solib ();
>
> +      current_program_space->clear_cbfd_soname_build_ids ();
>        current_program_space->cbfd.reset (nullptr);
>      }
>
> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> index f89dcc57754..35464115039 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 979159ba2f5..baf80580d60 100644
> --- a/gdb/gdbarch.h
> +++ b/gdb/gdbarch.h
> @@ -1710,8 +1710,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 39a99d0d5f3..c24079d34f3 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -1210,7 +1210,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 ae2f7c14f6d..812ca1f99bf 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -44,6 +44,7 @@
>  #include "solib-svr4.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
> @@ -1097,16 +1098,11 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
>     for each mapping.  */
>
>  static void
> -linux_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)
> +linux_read_core_file_mappings
> + (struct gdbarch *gdbarch,
> +  struct bfd *cbfd,
> +  gdb::function_view<void (ULONGEST count)> pre_loop_cb,
> +  loop_cb_ftype  loop_cb)
>  {
>    /* Ensure that ULONGEST is big enough for reading 64-bit core files.  */
>    gdb_static_assert (sizeof (ULONGEST) >= 8);
> @@ -1175,6 +1171,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++)
> @@ -1188,8 +1201,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 = vma_map[start];
> +
> +      /* 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 != 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);
>      }
>  }
>
> @@ -1218,7 +1240,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/progspace.c b/gdb/progspace.c
> index 7080bf8ee27..8b7b949d959 100644
> --- a/gdb/progspace.c
> +++ b/gdb/progspace.c
> @@ -17,6 +17,7 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>
> +#include "build-id.h"
>  #include "defs.h"
>  #include "gdbcmd.h"
>  #include "objfiles.h"
> @@ -358,6 +359,37 @@ print_program_space (struct ui_out *uiout, int requested)
>      }
>  }
>
> +/* See progspace.h.  */
> +
> +void
> +program_space::set_cbfd_soname_build_id (std::string soname,
> +                                        const bfd_build_id *build_id)
> +{
> +  cbfd_soname_to_build_id[std::move (soname)] = build_id_to_string (build_id);
> +}
> +
> +/* See progspace.h.  */
> +
> +const char *
> +program_space::get_cbfd_soname_build_id (const char *soname)
> +{
> +  gdb_assert (soname);
> +
> +  auto it = cbfd_soname_to_build_id.find (basename (soname));
> +  if (it == cbfd_soname_to_build_id.end ())
> +    return nullptr;
> +
> +  return it->second.c_str ();
> +}
> +
> +/* See progspace.h.  */
> +
> +void
> +program_space::clear_cbfd_soname_build_ids ()
> +{
> +  cbfd_soname_to_build_id.clear ();
> +}
> +
>  /* Boolean test for an already-known program space id.  */
>
>  static int
> diff --git a/gdb/progspace.h b/gdb/progspace.h
> index fb348ca7539..b42b3ffc4f1 100644
> --- a/gdb/progspace.h
> +++ b/gdb/progspace.h
> @@ -30,6 +30,7 @@
>  #include "gdbsupport/safe-iterator.h"
>  #include <list>
>  #include <vector>
> +#include <unordered_map>
>
>  struct target_ops;
>  struct bfd;
> @@ -324,6 +325,19 @@ struct program_space
>    /* Binary file diddling handle for the core file.  */
>    gdb_bfd_ref_ptr cbfd;
>
> +  /* Associate a core file SONAME with BUILD_ID so that it can be retrieved
> +     with get_cbfd_soname_build_id.  */
> +  void set_cbfd_soname_build_id (std::string soname,
> +                                const bfd_build_id *build_id);
> +
> +  /* If a core file SONAME had a build-id associated with it by a previous
> +     call to set_cbfd_soname_build_id then return the build-id as a
> +     NULL-terminated hex string.  */
> +  const char *get_cbfd_soname_build_id (const char *soname);
> +
> +  /* Clear all core file soname to build-id mappings.  */
> +  void clear_cbfd_soname_build_ids ();
> +
>    /* The address space attached to this program space.  More than one
>       program space may be bound to the same address space.  In the
>       traditional unix-like debugging scenario, this will usually
> @@ -378,6 +392,9 @@ struct program_space
>    /* The set of target sections matching the sections mapped into
>       this program space.  Managed by both exec_ops and solib.c.  */
>    target_section_table m_target_sections;
> +
> +  /* Mapping of a core file's library sonames to their respective build-ids.  */
> +  std::unordered_map<std::string, std::string> cbfd_soname_to_build_id;
>  };
>
>  /* An address space.  It is used for comparing if
> diff --git a/gdb/solib.c b/gdb/solib.c
> index e30affbb7e7..1b99c8ab985 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -23,6 +23,7 @@
>  #include <fcntl.h>
>  #include "symtab.h"
>  #include "bfd.h"
> +#include "build-id.h"
>  #include "symfile.h"
>  #include "objfiles.h"
>  #include "gdbcore.h"
> @@ -1585,6 +1586,40 @@ gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd, CORE_ADDR *ptr,
>    return 0;
>  }
>
> +/* See solib.h.  */
> +
> +gdb::optional<std::string>
> +gdb_bfd_read_elf_soname (struct bfd *bfd)
> +{
> +  gdb_assert (bfd != nullptr);
> +
> +  gdb_bfd_ref_ptr abfd = gdb_bfd_open (bfd->filename, gnutarget);
> +
> +  if (abfd == nullptr)
> +    return {};
> +
> +  /* Check that bfd is an ET_DYN ELF file.  */
> +  bfd_check_format (abfd.get (), bfd_object);
> +  if (!(bfd_get_file_flags (abfd.get ()) & DYNAMIC))
> +    return {};
> +
> +  /* Determine soname of shared library.  If found map soname to build-id.  */
> +  CORE_ADDR idx;
> +  if (!gdb_bfd_scan_elf_dyntag (DT_SONAME, abfd.get (), &idx, nullptr))
> +    return {};
> +
> +  struct bfd_section *dynstr = bfd_get_section_by_name (abfd.get (), ".dynstr");
> +  if (dynstr == nullptr || bfd_section_size (dynstr) <= idx)
> +    return {};
> +
> +  /* Read the soname from the string table.  */
> +  gdb::byte_vector dynstr_buf;
> +  if (!gdb_bfd_get_full_section_contents (abfd.get (), dynstr, &dynstr_buf))
> +    return {};
> +
> +  return std::string ((const char *)dynstr_buf.data () + idx);
> +}
> +
>  /* Lookup the value for a specific symbol from symbol table.  Look up symbol
>     from ABFD.  MATCH_SYM is a callback function to determine whether to pick
>     up a symbol.  DATA is the input of this callback function.  Return NULL
> diff --git a/gdb/solib.h b/gdb/solib.h
> index c50f74e06bf..51cc047463f 100644
> --- a/gdb/solib.h
> +++ b/gdb/solib.h
> @@ -118,6 +118,11 @@ extern CORE_ADDR gdb_bfd_lookup_symbol_from_symtab (bfd *abfd,
>  extern int gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd,
>                                     CORE_ADDR *ptr, CORE_ADDR *ptr_addr);
>
> +/* If BFD is an ELF shared object then attempt to return the string
> +   referred to by its DT_SONAME tag.   */
> +
> +extern gdb::optional<std::string> gdb_bfd_read_elf_soname (struct bfd *bfd);
> +
>  /* Enable or disable optional solib event breakpoints as appropriate.  */
>
>  extern void update_solib_breakpoints (void);
> --
> 2.31.1
>

[PING**2][PATCH 2/3] gdb: Add soname to build-id mapping for corefiles

[Aaron Merey]

Ping

Thanks,
Aaron

On Tue, Sep 28, 2021 at 9:12 PM Aaron Merey <amerey@redhat.com> wrote:
>
> Ping.
>
> Thanks,
> Aaron
>
> On Wed, Aug 18, 2021 at 10:22 PM Aaron Merey via Gdb-patches
> <gdb-patches@sourceware.org> wrote:
> >
> > Hi Lancelot and Simon,
> >
> > Thanks for the reviews. The updated patch is included below.
> >
> > On Tue, Aug 17, 2021 at 9:59 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
> > >>> +  /* Check that bfd is an ET_DYN ELF file.  */
> > >>> +  bfd_check_format (abfd.get (), bfd_object);
> > >
> > > I asked this in my previous review, still applies here:
> > >
> > >   What's the point of calling bfd_check_format without checking the
> > >   result?  It looks like a function without side-effects.
> >
> > I included a few comments in the [PATCH 0/3] email for this series[1].
> > I mentioned that for some reason bfd_check_format appears to update
> > the bfd 'flags' field with the correct value. If bfd_check_format is not
> > called here the check for the DYNAMIC flag fails even for ET_DYN files.
> >
> > [1] https://sourceware.org/pipermail/gdb-patches/2021-August/181416.html
> >
> > ---
> >  gdb/arch-utils.c | 15 +++++----------
> >  gdb/arch-utils.h | 23 +++++++++++++----------
> >  gdb/build-id.h   |  2 ++
> >  gdb/corelow.c    | 13 ++++++++++++-
> >  gdb/gdbarch.c    |  2 +-
> >  gdb/gdbarch.h    |  4 ++--
> >  gdb/gdbarch.sh   |  2 +-
> >  gdb/linux-tdep.c | 46 ++++++++++++++++++++++++++++++++++------------
> >  gdb/progspace.c  | 32 ++++++++++++++++++++++++++++++++
> >  gdb/progspace.h  | 17 +++++++++++++++++
> >  gdb/solib.c      | 35 +++++++++++++++++++++++++++++++++++
> >  gdb/solib.h      |  5 +++++
> >  12 files changed, 159 insertions(+), 37 deletions(-)
> >
> > diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
> > index 862f26b6cf7..ffb32cb203f 100644
> > --- a/gdb/arch-utils.c
> > +++ b/gdb/arch-utils.c
> > @@ -1075,16 +1075,11 @@ default_get_pc_address_flags (frame_info *frame, CORE_ADDR pc)
> >
> >  /* See arch-utils.h.  */
> >  void
> > -default_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)
> > +default_read_core_file_mappings
> > + (struct gdbarch *gdbarch,
> > +  struct bfd *cbfd,
> > +  gdb::function_view<void (ULONGEST count)> pre_loop_cb,
> > +  loop_cb_ftype loop_cb)
> >  {
> >  }
> >
> > diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
> > index 03e9082f6d7..2243c3fb85b 100644
> > --- a/gdb/arch-utils.h
> > +++ b/gdb/arch-utils.h
> > @@ -294,15 +294,18 @@ extern ULONGEST default_type_align (struct gdbarch *gdbarch,
> >  extern std::string default_get_pc_address_flags (frame_info *frame,
> >                                                  CORE_ADDR pc);
> >
> > +
> > +using loop_cb_ftype = gdb::function_view<void (int num,
> > +                                              ULONGEST start,
> > +                                              ULONGEST end,
> > +                                              ULONGEST file_ofs,
> > +                                              const char *filename,
> > +                                              const bfd_build_id *build_id)>;
> > +
> >  /* Default implementation of gdbarch read_core_file_mappings method.  */
> > -extern void default_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);
> > +extern void default_read_core_file_mappings
> > + (struct gdbarch *gdbarch,
> > +  struct bfd *cbfd,
> > +  gdb::function_view<void (ULONGEST count)> pre_loop_cb,
> > +  loop_cb_ftype loop_cb);
> >  #endif /* ARCH_UTILS_H */
> > diff --git a/gdb/build-id.h b/gdb/build-id.h
> > index 42f8d57ede1..3c9402ee71b 100644
> > --- a/gdb/build-id.h
> > +++ b/gdb/build-id.h
> > @@ -20,8 +20,10 @@
> >  #ifndef BUILD_ID_H
> >  #define BUILD_ID_H
> >
> > +#include "defs.h"
> >  #include "gdb_bfd.h"
> >  #include "gdbsupport/rsp-low.h"
> > +#include <string>
> >
> >  /* Locate NT_GNU_BUILD_ID from ABFD and return its content.  */
> >
> > diff --git a/gdb/corelow.c b/gdb/corelow.c
> > index eb785a08633..31af0f22584 100644
> > --- a/gdb/corelow.c
> > +++ b/gdb/corelow.c
> > @@ -214,7 +214,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.  */
> > @@ -282,6 +282,16 @@ core_target::build_file_mappings ()
> >
> >         /* Set target_section fields.  */
> >         m_core_file_mappings.emplace_back (start, end, sec);
> > +
> > +       /* If this is a bfd of a shared library, record its soname
> > +          and build id.  */
> > +       if (build_id != nullptr)
> > +         {
> > +           gdb::optional<std::string> soname = gdb_bfd_read_elf_soname (bfd);
> > +           if (soname)
> > +             current_program_space->set_cbfd_soname_build_id
> > +              (std::move (*soname), build_id);
> > +         }
> >        });
> >
> >    normalize_mem_ranges (&m_core_unavailable_mappings);
> > @@ -305,6 +315,7 @@ core_target::close ()
> >          comments in clear_solib in solib.c.  */
> >        clear_solib ();
> >
> > +      current_program_space->clear_cbfd_soname_build_ids ();
> >        current_program_space->cbfd.reset (nullptr);
> >      }
> >
> > diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> > index f89dcc57754..35464115039 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 979159ba2f5..baf80580d60 100644
> > --- a/gdb/gdbarch.h
> > +++ b/gdb/gdbarch.h
> > @@ -1710,8 +1710,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 39a99d0d5f3..c24079d34f3 100755
> > --- a/gdb/gdbarch.sh
> > +++ b/gdb/gdbarch.sh
> > @@ -1210,7 +1210,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 ae2f7c14f6d..812ca1f99bf 100644
> > --- a/gdb/linux-tdep.c
> > +++ b/gdb/linux-tdep.c
> > @@ -44,6 +44,7 @@
> >  #include "solib-svr4.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
> > @@ -1097,16 +1098,11 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
> >     for each mapping.  */
> >
> >  static void
> > -linux_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)
> > +linux_read_core_file_mappings
> > + (struct gdbarch *gdbarch,
> > +  struct bfd *cbfd,
> > +  gdb::function_view<void (ULONGEST count)> pre_loop_cb,
> > +  loop_cb_ftype  loop_cb)
> >  {
> >    /* Ensure that ULONGEST is big enough for reading 64-bit core files.  */
> >    gdb_static_assert (sizeof (ULONGEST) >= 8);
> > @@ -1175,6 +1171,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++)
> > @@ -1188,8 +1201,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 = vma_map[start];
> > +
> > +      /* 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 != 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);
> >      }
> >  }
> >
> > @@ -1218,7 +1240,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/progspace.c b/gdb/progspace.c
> > index 7080bf8ee27..8b7b949d959 100644
> > --- a/gdb/progspace.c
> > +++ b/gdb/progspace.c
> > @@ -17,6 +17,7 @@
> >     You should have received a copy of the GNU General Public License
> >     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> >
> > +#include "build-id.h"
> >  #include "defs.h"
> >  #include "gdbcmd.h"
> >  #include "objfiles.h"
> > @@ -358,6 +359,37 @@ print_program_space (struct ui_out *uiout, int requested)
> >      }
> >  }
> >
> > +/* See progspace.h.  */
> > +
> > +void
> > +program_space::set_cbfd_soname_build_id (std::string soname,
> > +                                        const bfd_build_id *build_id)
> > +{
> > +  cbfd_soname_to_build_id[std::move (soname)] = build_id_to_string (build_id);
> > +}
> > +
> > +/* See progspace.h.  */
> > +
> > +const char *
> > +program_space::get_cbfd_soname_build_id (const char *soname)
> > +{
> > +  gdb_assert (soname);
> > +
> > +  auto it = cbfd_soname_to_build_id.find (basename (soname));
> > +  if (it == cbfd_soname_to_build_id.end ())
> > +    return nullptr;
> > +
> > +  return it->second.c_str ();
> > +}
> > +
> > +/* See progspace.h.  */
> > +
> > +void
> > +program_space::clear_cbfd_soname_build_ids ()
> > +{
> > +  cbfd_soname_to_build_id.clear ();
> > +}
> > +
> >  /* Boolean test for an already-known program space id.  */
> >
> >  static int
> > diff --git a/gdb/progspace.h b/gdb/progspace.h
> > index fb348ca7539..b42b3ffc4f1 100644
> > --- a/gdb/progspace.h
> > +++ b/gdb/progspace.h
> > @@ -30,6 +30,7 @@
> >  #include "gdbsupport/safe-iterator.h"
> >  #include <list>
> >  #include <vector>
> > +#include <unordered_map>
> >
> >  struct target_ops;
> >  struct bfd;
> > @@ -324,6 +325,19 @@ struct program_space
> >    /* Binary file diddling handle for the core file.  */
> >    gdb_bfd_ref_ptr cbfd;
> >
> > +  /* Associate a core file SONAME with BUILD_ID so that it can be retrieved
> > +     with get_cbfd_soname_build_id.  */
> > +  void set_cbfd_soname_build_id (std::string soname,
> > +                                const bfd_build_id *build_id);
> > +
> > +  /* If a core file SONAME had a build-id associated with it by a previous
> > +     call to set_cbfd_soname_build_id then return the build-id as a
> > +     NULL-terminated hex string.  */
> > +  const char *get_cbfd_soname_build_id (const char *soname);
> > +
> > +  /* Clear all core file soname to build-id mappings.  */
> > +  void clear_cbfd_soname_build_ids ();
> > +
> >    /* The address space attached to this program space.  More than one
> >       program space may be bound to the same address space.  In the
> >       traditional unix-like debugging scenario, this will usually
> > @@ -378,6 +392,9 @@ struct program_space
> >    /* The set of target sections matching the sections mapped into
> >       this program space.  Managed by both exec_ops and solib.c.  */
> >    target_section_table m_target_sections;
> > +
> > +  /* Mapping of a core file's library sonames to their respective build-ids.  */
> > +  std::unordered_map<std::string, std::string> cbfd_soname_to_build_id;
> >  };
> >
> >  /* An address space.  It is used for comparing if
> > diff --git a/gdb/solib.c b/gdb/solib.c
> > index e30affbb7e7..1b99c8ab985 100644
> > --- a/gdb/solib.c
> > +++ b/gdb/solib.c
> > @@ -23,6 +23,7 @@
> >  #include <fcntl.h>
> >  #include "symtab.h"
> >  #include "bfd.h"
> > +#include "build-id.h"
> >  #include "symfile.h"
> >  #include "objfiles.h"
> >  #include "gdbcore.h"
> > @@ -1585,6 +1586,40 @@ gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd, CORE_ADDR *ptr,
> >    return 0;
> >  }
> >
> > +/* See solib.h.  */
> > +
> > +gdb::optional<std::string>
> > +gdb_bfd_read_elf_soname (struct bfd *bfd)
> > +{
> > +  gdb_assert (bfd != nullptr);
> > +
> > +  gdb_bfd_ref_ptr abfd = gdb_bfd_open (bfd->filename, gnutarget);
> > +
> > +  if (abfd == nullptr)
> > +    return {};
> > +
> > +  /* Check that bfd is an ET_DYN ELF file.  */
> > +  bfd_check_format (abfd.get (), bfd_object);
> > +  if (!(bfd_get_file_flags (abfd.get ()) & DYNAMIC))
> > +    return {};
> > +
> > +  /* Determine soname of shared library.  If found map soname to build-id.  */
> > +  CORE_ADDR idx;
> > +  if (!gdb_bfd_scan_elf_dyntag (DT_SONAME, abfd.get (), &idx, nullptr))
> > +    return {};
> > +
> > +  struct bfd_section *dynstr = bfd_get_section_by_name (abfd.get (), ".dynstr");
> > +  if (dynstr == nullptr || bfd_section_size (dynstr) <= idx)
> > +    return {};
> > +
> > +  /* Read the soname from the string table.  */
> > +  gdb::byte_vector dynstr_buf;
> > +  if (!gdb_bfd_get_full_section_contents (abfd.get (), dynstr, &dynstr_buf))
> > +    return {};
> > +
> > +  return std::string ((const char *)dynstr_buf.data () + idx);
> > +}
> > +
> >  /* Lookup the value for a specific symbol from symbol table.  Look up symbol
> >     from ABFD.  MATCH_SYM is a callback function to determine whether to pick
> >     up a symbol.  DATA is the input of this callback function.  Return NULL
> > diff --git a/gdb/solib.h b/gdb/solib.h
> > index c50f74e06bf..51cc047463f 100644
> > --- a/gdb/solib.h
> > +++ b/gdb/solib.h
> > @@ -118,6 +118,11 @@ extern CORE_ADDR gdb_bfd_lookup_symbol_from_symtab (bfd *abfd,
> >  extern int gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd,
> >                                     CORE_ADDR *ptr, CORE_ADDR *ptr_addr);
> >
> > +/* If BFD is an ELF shared object then attempt to return the string
> > +   referred to by its DT_SONAME tag.   */
> > +
> > +extern gdb::optional<std::string> gdb_bfd_read_elf_soname (struct bfd *bfd);
> > +
> >  /* Enable or disable optional solib event breakpoints as appropriate.  */
> >
> >  extern void update_solib_breakpoints (void);
> > --
> > 2.31.1
> >

[PING**3][PATCH 2/3] gdb: Add soname to build-id mapping for corefiles

[Aaron Merey]

Ping.

Thanks,
Aaron

On Mon, Oct 18, 2021 at 7:06 PM Aaron Merey <amerey@redhat.com> wrote:
>
> Ping
>
> Thanks,
> Aaron
>
> On Tue, Sep 28, 2021 at 9:12 PM Aaron Merey <amerey@redhat.com> wrote:
> >
> > Ping.
> >
> > Thanks,
> > Aaron
> >
> > On Wed, Aug 18, 2021 at 10:22 PM Aaron Merey via Gdb-patches
> > <gdb-patches@sourceware.org> wrote:
> > >
> > > Hi Lancelot and Simon,
> > >
> > > Thanks for the reviews. The updated patch is included below.
> > >
> > > On Tue, Aug 17, 2021 at 9:59 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
> > > >>> +  /* Check that bfd is an ET_DYN ELF file.  */
> > > >>> +  bfd_check_format (abfd.get (), bfd_object);
> > > >
> > > > I asked this in my previous review, still applies here:
> > > >
> > > >   What's the point of calling bfd_check_format without checking the
> > > >   result?  It looks like a function without side-effects.
> > >
> > > I included a few comments in the [PATCH 0/3] email for this series[1].
> > > I mentioned that for some reason bfd_check_format appears to update
> > > the bfd 'flags' field with the correct value. If bfd_check_format is not
> > > called here the check for the DYNAMIC flag fails even for ET_DYN files.
> > >
> > > [1] https://sourceware.org/pipermail/gdb-patches/2021-August/181416.html
> > >
> > > ---
> > >  gdb/arch-utils.c | 15 +++++----------
> > >  gdb/arch-utils.h | 23 +++++++++++++----------
> > >  gdb/build-id.h   |  2 ++
> > >  gdb/corelow.c    | 13 ++++++++++++-
> > >  gdb/gdbarch.c    |  2 +-
> > >  gdb/gdbarch.h    |  4 ++--
> > >  gdb/gdbarch.sh   |  2 +-
> > >  gdb/linux-tdep.c | 46 ++++++++++++++++++++++++++++++++++------------
> > >  gdb/progspace.c  | 32 ++++++++++++++++++++++++++++++++
> > >  gdb/progspace.h  | 17 +++++++++++++++++
> > >  gdb/solib.c      | 35 +++++++++++++++++++++++++++++++++++
> > >  gdb/solib.h      |  5 +++++
> > >  12 files changed, 159 insertions(+), 37 deletions(-)
> > >
> > > diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
> > > index 862f26b6cf7..ffb32cb203f 100644
> > > --- a/gdb/arch-utils.c
> > > +++ b/gdb/arch-utils.c
> > > @@ -1075,16 +1075,11 @@ default_get_pc_address_flags (frame_info *frame, CORE_ADDR pc)
> > >
> > >  /* See arch-utils.h.  */
> > >  void
> > > -default_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)
> > > +default_read_core_file_mappings
> > > + (struct gdbarch *gdbarch,
> > > +  struct bfd *cbfd,
> > > +  gdb::function_view<void (ULONGEST count)> pre_loop_cb,
> > > +  loop_cb_ftype loop_cb)
> > >  {
> > >  }
> > >
> > > diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
> > > index 03e9082f6d7..2243c3fb85b 100644
> > > --- a/gdb/arch-utils.h
> > > +++ b/gdb/arch-utils.h
> > > @@ -294,15 +294,18 @@ extern ULONGEST default_type_align (struct gdbarch *gdbarch,
> > >  extern std::string default_get_pc_address_flags (frame_info *frame,
> > >                                                  CORE_ADDR pc);
> > >
> > > +
> > > +using loop_cb_ftype = gdb::function_view<void (int num,
> > > +                                              ULONGEST start,
> > > +                                              ULONGEST end,
> > > +                                              ULONGEST file_ofs,
> > > +                                              const char *filename,
> > > +                                              const bfd_build_id *build_id)>;
> > > +
> > >  /* Default implementation of gdbarch read_core_file_mappings method.  */
> > > -extern void default_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);
> > > +extern void default_read_core_file_mappings
> > > + (struct gdbarch *gdbarch,
> > > +  struct bfd *cbfd,
> > > +  gdb::function_view<void (ULONGEST count)> pre_loop_cb,
> > > +  loop_cb_ftype loop_cb);
> > >  #endif /* ARCH_UTILS_H */
> > > diff --git a/gdb/build-id.h b/gdb/build-id.h
> > > index 42f8d57ede1..3c9402ee71b 100644
> > > --- a/gdb/build-id.h
> > > +++ b/gdb/build-id.h
> > > @@ -20,8 +20,10 @@
> > >  #ifndef BUILD_ID_H
> > >  #define BUILD_ID_H
> > >
> > > +#include "defs.h"
> > >  #include "gdb_bfd.h"
> > >  #include "gdbsupport/rsp-low.h"
> > > +#include <string>
> > >
> > >  /* Locate NT_GNU_BUILD_ID from ABFD and return its content.  */
> > >
> > > diff --git a/gdb/corelow.c b/gdb/corelow.c
> > > index eb785a08633..31af0f22584 100644
> > > --- a/gdb/corelow.c
> > > +++ b/gdb/corelow.c
> > > @@ -214,7 +214,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.  */
> > > @@ -282,6 +282,16 @@ core_target::build_file_mappings ()
> > >
> > >         /* Set target_section fields.  */
> > >         m_core_file_mappings.emplace_back (start, end, sec);
> > > +
> > > +       /* If this is a bfd of a shared library, record its soname
> > > +          and build id.  */
> > > +       if (build_id != nullptr)
> > > +         {
> > > +           gdb::optional<std::string> soname = gdb_bfd_read_elf_soname (bfd);
> > > +           if (soname)
> > > +             current_program_space->set_cbfd_soname_build_id
> > > +              (std::move (*soname), build_id);
> > > +         }
> > >        });
> > >
> > >    normalize_mem_ranges (&m_core_unavailable_mappings);
> > > @@ -305,6 +315,7 @@ core_target::close ()
> > >          comments in clear_solib in solib.c.  */
> > >        clear_solib ();
> > >
> > > +      current_program_space->clear_cbfd_soname_build_ids ();
> > >        current_program_space->cbfd.reset (nullptr);
> > >      }
> > >
> > > diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> > > index f89dcc57754..35464115039 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 979159ba2f5..baf80580d60 100644
> > > --- a/gdb/gdbarch.h
> > > +++ b/gdb/gdbarch.h
> > > @@ -1710,8 +1710,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 39a99d0d5f3..c24079d34f3 100755
> > > --- a/gdb/gdbarch.sh
> > > +++ b/gdb/gdbarch.sh
> > > @@ -1210,7 +1210,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 ae2f7c14f6d..812ca1f99bf 100644
> > > --- a/gdb/linux-tdep.c
> > > +++ b/gdb/linux-tdep.c
> > > @@ -44,6 +44,7 @@
> > >  #include "solib-svr4.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
> > > @@ -1097,16 +1098,11 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
> > >     for each mapping.  */
> > >
> > >  static void
> > > -linux_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)
> > > +linux_read_core_file_mappings
> > > + (struct gdbarch *gdbarch,
> > > +  struct bfd *cbfd,
> > > +  gdb::function_view<void (ULONGEST count)> pre_loop_cb,
> > > +  loop_cb_ftype  loop_cb)
> > >  {
> > >    /* Ensure that ULONGEST is big enough for reading 64-bit core files.  */
> > >    gdb_static_assert (sizeof (ULONGEST) >= 8);
> > > @@ -1175,6 +1171,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++)
> > > @@ -1188,8 +1201,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 = vma_map[start];
> > > +
> > > +      /* 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 != 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);
> > >      }
> > >  }
> > >
> > > @@ -1218,7 +1240,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/progspace.c b/gdb/progspace.c
> > > index 7080bf8ee27..8b7b949d959 100644
> > > --- a/gdb/progspace.c
> > > +++ b/gdb/progspace.c
> > > @@ -17,6 +17,7 @@
> > >     You should have received a copy of the GNU General Public License
> > >     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> > >
> > > +#include "build-id.h"
> > >  #include "defs.h"
> > >  #include "gdbcmd.h"
> > >  #include "objfiles.h"
> > > @@ -358,6 +359,37 @@ print_program_space (struct ui_out *uiout, int requested)
> > >      }
> > >  }
> > >
> > > +/* See progspace.h.  */
> > > +
> > > +void
> > > +program_space::set_cbfd_soname_build_id (std::string soname,
> > > +                                        const bfd_build_id *build_id)
> > > +{
> > > +  cbfd_soname_to_build_id[std::move (soname)] = build_id_to_string (build_id);
> > > +}
> > > +
> > > +/* See progspace.h.  */
> > > +
> > > +const char *
> > > +program_space::get_cbfd_soname_build_id (const char *soname)
> > > +{
> > > +  gdb_assert (soname);
> > > +
> > > +  auto it = cbfd_soname_to_build_id.find (basename (soname));
> > > +  if (it == cbfd_soname_to_build_id.end ())
> > > +    return nullptr;
> > > +
> > > +  return it->second.c_str ();
> > > +}
> > > +
> > > +/* See progspace.h.  */
> > > +
> > > +void
> > > +program_space::clear_cbfd_soname_build_ids ()
> > > +{
> > > +  cbfd_soname_to_build_id.clear ();
> > > +}
> > > +
> > >  /* Boolean test for an already-known program space id.  */
> > >
> > >  static int
> > > diff --git a/gdb/progspace.h b/gdb/progspace.h
> > > index fb348ca7539..b42b3ffc4f1 100644
> > > --- a/gdb/progspace.h
> > > +++ b/gdb/progspace.h
> > > @@ -30,6 +30,7 @@
> > >  #include "gdbsupport/safe-iterator.h"
> > >  #include <list>
> > >  #include <vector>
> > > +#include <unordered_map>
> > >
> > >  struct target_ops;
> > >  struct bfd;
> > > @@ -324,6 +325,19 @@ struct program_space
> > >    /* Binary file diddling handle for the core file.  */
> > >    gdb_bfd_ref_ptr cbfd;
> > >
> > > +  /* Associate a core file SONAME with BUILD_ID so that it can be retrieved
> > > +     with get_cbfd_soname_build_id.  */
> > > +  void set_cbfd_soname_build_id (std::string soname,
> > > +                                const bfd_build_id *build_id);
> > > +
> > > +  /* If a core file SONAME had a build-id associated with it by a previous
> > > +     call to set_cbfd_soname_build_id then return the build-id as a
> > > +     NULL-terminated hex string.  */
> > > +  const char *get_cbfd_soname_build_id (const char *soname);
> > > +
> > > +  /* Clear all core file soname to build-id mappings.  */
> > > +  void clear_cbfd_soname_build_ids ();
> > > +
> > >    /* The address space attached to this program space.  More than one
> > >       program space may be bound to the same address space.  In the
> > >       traditional unix-like debugging scenario, this will usually
> > > @@ -378,6 +392,9 @@ struct program_space
> > >    /* The set of target sections matching the sections mapped into
> > >       this program space.  Managed by both exec_ops and solib.c.  */
> > >    target_section_table m_target_sections;
> > > +
> > > +  /* Mapping of a core file's library sonames to their respective build-ids.  */
> > > +  std::unordered_map<std::string, std::string> cbfd_soname_to_build_id;
> > >  };
> > >
> > >  /* An address space.  It is used for comparing if
> > > diff --git a/gdb/solib.c b/gdb/solib.c
> > > index e30affbb7e7..1b99c8ab985 100644
> > > --- a/gdb/solib.c
> > > +++ b/gdb/solib.c
> > > @@ -23,6 +23,7 @@
> > >  #include <fcntl.h>
> > >  #include "symtab.h"
> > >  #include "bfd.h"
> > > +#include "build-id.h"
> > >  #include "symfile.h"
> > >  #include "objfiles.h"
> > >  #include "gdbcore.h"
> > > @@ -1585,6 +1586,40 @@ gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd, CORE_ADDR *ptr,
> > >    return 0;
> > >  }
> > >
> > > +/* See solib.h.  */
> > > +
> > > +gdb::optional<std::string>
> > > +gdb_bfd_read_elf_soname (struct bfd *bfd)
> > > +{
> > > +  gdb_assert (bfd != nullptr);
> > > +
> > > +  gdb_bfd_ref_ptr abfd = gdb_bfd_open (bfd->filename, gnutarget);
> > > +
> > > +  if (abfd == nullptr)
> > > +    return {};
> > > +
> > > +  /* Check that bfd is an ET_DYN ELF file.  */
> > > +  bfd_check_format (abfd.get (), bfd_object);
> > > +  if (!(bfd_get_file_flags (abfd.get ()) & DYNAMIC))
> > > +    return {};
> > > +
> > > +  /* Determine soname of shared library.  If found map soname to build-id.  */
> > > +  CORE_ADDR idx;
> > > +  if (!gdb_bfd_scan_elf_dyntag (DT_SONAME, abfd.get (), &idx, nullptr))
> > > +    return {};
> > > +
> > > +  struct bfd_section *dynstr = bfd_get_section_by_name (abfd.get (), ".dynstr");
> > > +  if (dynstr == nullptr || bfd_section_size (dynstr) <= idx)
> > > +    return {};
> > > +
> > > +  /* Read the soname from the string table.  */
> > > +  gdb::byte_vector dynstr_buf;
> > > +  if (!gdb_bfd_get_full_section_contents (abfd.get (), dynstr, &dynstr_buf))
> > > +    return {};
> > > +
> > > +  return std::string ((const char *)dynstr_buf.data () + idx);
> > > +}
> > > +
> > >  /* Lookup the value for a specific symbol from symbol table.  Look up symbol
> > >     from ABFD.  MATCH_SYM is a callback function to determine whether to pick
> > >     up a symbol.  DATA is the input of this callback function.  Return NULL
> > > diff --git a/gdb/solib.h b/gdb/solib.h
> > > index c50f74e06bf..51cc047463f 100644
> > > --- a/gdb/solib.h
> > > +++ b/gdb/solib.h
> > > @@ -118,6 +118,11 @@ extern CORE_ADDR gdb_bfd_lookup_symbol_from_symtab (bfd *abfd,
> > >  extern int gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd,
> > >                                     CORE_ADDR *ptr, CORE_ADDR *ptr_addr);
> > >
> > > +/* If BFD is an ELF shared object then attempt to return the string
> > > +   referred to by its DT_SONAME tag.   */
> > > +
> > > +extern gdb::optional<std::string> gdb_bfd_read_elf_soname (struct bfd *bfd);
> > > +
> > >  /* Enable or disable optional solib event breakpoints as appropriate.  */
> > >
> > >  extern void update_solib_breakpoints (void);
> > > --
> > > 2.31.1
> > >

Re: [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles

[Simon Marchi]

> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
> index 862f26b6cf7..ffb32cb203f 100644
> --- a/gdb/arch-utils.c
> +++ b/gdb/arch-utils.c
> @@ -1075,16 +1075,11 @@ default_get_pc_address_flags (frame_info *frame, CORE_ADDR pc)
>  
>  /* See arch-utils.h.  */
>  void
> -default_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)
> +default_read_core_file_mappings
> + (struct gdbarch *gdbarch,
> +  struct bfd *cbfd,
> +  gdb::function_view<void (ULONGEST count)> pre_loop_cb,
> +  loop_cb_ftype loop_cb)
>  {
>  }

Two spaces before the parenthesis:

void
default_read_core_file_mappings
  (struct gdbarch *gdbarch, struct bfd *cbfd,
   gdb::function_view<void (ULONGEST count)> pre_loop_cb,
   loop_cb_ftype loop_cb)
{
}


>  
> diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
> index 03e9082f6d7..2243c3fb85b 100644
> --- a/gdb/arch-utils.h
> +++ b/gdb/arch-utils.h
> @@ -294,15 +294,18 @@ extern ULONGEST default_type_align (struct gdbarch *gdbarch,
>  extern std::string default_get_pc_address_flags (frame_info *frame,
>  						 CORE_ADDR pc);
>  
> +
> +using loop_cb_ftype = gdb::function_view<void (int num,
> +					       ULONGEST start,
> +					       ULONGEST end,
> +					       ULONGEST file_ofs,
> +					       const char *filename,
> +					       const bfd_build_id *build_id)>;

I think this could use a better name, especially since it's in a header
visible throughout GDB.  read_core_file_mappings_loop_ftype, maybe, to
follow what we use often (ftype meaning function type).  And while at
it, let's add read_core_file_mappings_pre_loop_ftype, while at it.  I
wouldn't mind if adding the named types for the callbacks was done in
its own preparatory patch, it should be fairly obvious.

> diff --git a/gdb/build-id.h b/gdb/build-id.h
> index 42f8d57ede1..3c9402ee71b 100644
> --- a/gdb/build-id.h
> +++ b/gdb/build-id.h
> @@ -20,8 +20,10 @@
>  #ifndef BUILD_ID_H
>  #define BUILD_ID_H
>  
> +#include "defs.h"
>  #include "gdb_bfd.h"
>  #include "gdbsupport/rsp-low.h"
> +#include <string>

Don't include defs.h here, see:

https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Include_Files

> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
> index 39a99d0d5f3..c24079d34f3 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -1210,7 +1210,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

Replace the long gdb::function_view types here with the new named types,
it will help readability.

> @@ -1188,8 +1201,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 = vma_map[start];

Here, use the find method of unordered_map.  Using [] will cause an
insertion if the entry does not exist, which is unnecessary work.

> diff --git a/gdb/progspace.c b/gdb/progspace.c
> index 7080bf8ee27..8b7b949d959 100644
> --- a/gdb/progspace.c
> +++ b/gdb/progspace.c
> @@ -17,6 +17,7 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
> +#include "build-id.h"
>  #include "defs.h"
>  #include "gdbcmd.h"
>  #include "objfiles.h"

Move the build-id.h include below, at least below defs.h, which must be
the first one.

> @@ -378,6 +392,9 @@ struct program_space
>    /* The set of target sections matching the sections mapped into
>       this program space.  Managed by both exec_ops and solib.c.  */
>    target_section_table m_target_sections;
> +
> +  /* Mapping of a core file's library sonames to their respective build-ids.  */
> +  std::unordered_map<std::string, std::string> cbfd_soname_to_build_id;

Prefix this member with `m_`.

Simon

[PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings

[Aaron Merey]

Add debuginfod support to core_target::build_file_mappings and
locate_exec_from_corefile_build_id to enable the downloading of
missing core file executables and shared libraries.

Also add debuginfod support to solib_map_sections so that previously
downloaded shared libraries can be retrieved from the debuginfod
cache.

When core file shared libraries are found locally, verify
that their build-ids match the corresponding build-id found in
the core file.  If there is a mismatch, print a warning and attempt
to query debuginfod for the correct build of the shared library:

warning: Build-id of /lib64/libc.so.6 does not match core file.
Downloading XY.Z MB executable for /lib64/libc.so.6
---
 gdb/corelow.c                                 | 33 +++++++++++++
 gdb/debuginfod-support.c                      | 46 +++++++++++++++++++
 gdb/debuginfod-support.h                      | 16 +++++++
 gdb/gcore.in                                  |  3 ++
 gdb/solib.c                                   | 35 ++++++++++++++
 .../gdb.debuginfod/fetch_src_and_symbols.exp  | 25 +++++++++-
 6 files changed, 156 insertions(+), 2 deletions(-)

diff --git a/gdb/corelow.c b/gdb/corelow.c
index 97eadceed84..2c9ea1044b7 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"
@@ -229,6 +231,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);
@@ -416,6 +423,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..04b39b57f6f 100644
--- a/gdb/debuginfod-support.c
+++ b/gdb/debuginfod-support.c
@@ -41,6 +41,16 @@ 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);
+}
+
 #else
 #include <elfutils/debuginfod.h>
 
@@ -194,4 +204,40 @@ 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;
+}
 #endif
diff --git a/gdb/debuginfod-support.h b/gdb/debuginfod-support.h
index 5e5aab56e74..c29a07d2abe 100644
--- a/gdb/debuginfod-support.h
+++ b/gdb/debuginfod-support.h
@@ -61,4 +61,20 @@ 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);
 #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/solib.c b/gdb/solib.c
index 8b92cf7db53..3a7bf59e64a 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -49,6 +49,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"
 
@@ -538,6 +540,39 @@ 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 ()));
+  const char *build_id_hexstr =
+    current_program_space->get_cbfd_soname_build_id (so->so_name);
+
+  /* If we already have core file build-id information for this solib,
+     verify it matches abfd's build-id.  If there is a mismatch or
+     the solib wasn't found, attempt to query debuginfod for
+     the correct solib.  */
+  if (build_id_hexstr != NULL)
+    {
+      bool mismatch = false;
+
+      if (abfd != NULL && abfd->build_id != NULL)
+	{
+	  std::string build_id = build_id_to_string (abfd->build_id);
+	  if (build_id != build_id_hexstr)
+	    {
+	      warning (_("Build-id of %ps does not match core file."),
+		       styled_string (file_name_style.style (),
+				      filename.get ()));
+	      mismatch = true;
+	    }
+	}
+
+      if (abfd == NULL || mismatch)
+	{
+	  scoped_fd fd = debuginfod_exec_query ((const unsigned char*)
+						build_id_hexstr,
+						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

Re: [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings

[Aaron Merey]

Ping.

Thanks,
Aaron

On Thu, Aug 12, 2021 at 12:25 AM Aaron Merey via Gdb-patches
<gdb-patches@sourceware.org> wrote:
>
> Add debuginfod support to core_target::build_file_mappings and
> locate_exec_from_corefile_build_id to enable the downloading of
> missing core file executables and shared libraries.
>
> Also add debuginfod support to solib_map_sections so that previously
> downloaded shared libraries can be retrieved from the debuginfod
> cache.
>
> When core file shared libraries are found locally, verify
> that their build-ids match the corresponding build-id found in
> the core file.  If there is a mismatch, print a warning and attempt
> to query debuginfod for the correct build of the shared library:
>
> warning: Build-id of /lib64/libc.so.6 does not match core file.
> Downloading XY.Z MB executable for /lib64/libc.so.6
> ---
>  gdb/corelow.c                                 | 33 +++++++++++++
>  gdb/debuginfod-support.c                      | 46 +++++++++++++++++++
>  gdb/debuginfod-support.h                      | 16 +++++++
>  gdb/gcore.in                                  |  3 ++
>  gdb/solib.c                                   | 35 ++++++++++++++
>  .../gdb.debuginfod/fetch_src_and_symbols.exp  | 25 +++++++++-
>  6 files changed, 156 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index 97eadceed84..2c9ea1044b7 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"
> @@ -229,6 +231,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);
> @@ -416,6 +423,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..04b39b57f6f 100644
> --- a/gdb/debuginfod-support.c
> +++ b/gdb/debuginfod-support.c
> @@ -41,6 +41,16 @@ 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);
> +}
> +
>  #else
>  #include <elfutils/debuginfod.h>
>
> @@ -194,4 +204,40 @@ 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;
> +}
>  #endif
> diff --git a/gdb/debuginfod-support.h b/gdb/debuginfod-support.h
> index 5e5aab56e74..c29a07d2abe 100644
> --- a/gdb/debuginfod-support.h
> +++ b/gdb/debuginfod-support.h
> @@ -61,4 +61,20 @@ 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);
>  #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/solib.c b/gdb/solib.c
> index 8b92cf7db53..3a7bf59e64a 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -49,6 +49,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"
>
> @@ -538,6 +540,39 @@ 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 ()));
> +  const char *build_id_hexstr =
> +    current_program_space->get_cbfd_soname_build_id (so->so_name);
> +
> +  /* If we already have core file build-id information for this solib,
> +     verify it matches abfd's build-id.  If there is a mismatch or
> +     the solib wasn't found, attempt to query debuginfod for
> +     the correct solib.  */
> +  if (build_id_hexstr != NULL)
> +    {
> +      bool mismatch = false;
> +
> +      if (abfd != NULL && abfd->build_id != NULL)
> +       {
> +         std::string build_id = build_id_to_string (abfd->build_id);
> +         if (build_id != build_id_hexstr)
> +           {
> +             warning (_("Build-id of %ps does not match core file."),
> +                      styled_string (file_name_style.style (),
> +                                     filename.get ()));
> +             mismatch = true;
> +           }
> +       }
> +
> +      if (abfd == NULL || mismatch)
> +       {
> +         scoped_fd fd = debuginfod_exec_query ((const unsigned char*)
> +                                               build_id_hexstr,
> +                                               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
>

[PING**2][PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings

[Aaron Merey]

Ping

Thanks,
Aaron

On Tue, Sep 28, 2021 at 9:13 PM Aaron Merey <amerey@redhat.com> wrote:
>
> Ping.
>
> Thanks,
> Aaron
>
> On Thu, Aug 12, 2021 at 12:25 AM Aaron Merey via Gdb-patches
> <gdb-patches@sourceware.org> wrote:
> >
> > Add debuginfod support to core_target::build_file_mappings and
> > locate_exec_from_corefile_build_id to enable the downloading of
> > missing core file executables and shared libraries.
> >
> > Also add debuginfod support to solib_map_sections so that previously
> > downloaded shared libraries can be retrieved from the debuginfod
> > cache.
> >
> > When core file shared libraries are found locally, verify
> > that their build-ids match the corresponding build-id found in
> > the core file.  If there is a mismatch, print a warning and attempt
> > to query debuginfod for the correct build of the shared library:
> >
> > warning: Build-id of /lib64/libc.so.6 does not match core file.
> > Downloading XY.Z MB executable for /lib64/libc.so.6
> > ---
> >  gdb/corelow.c                                 | 33 +++++++++++++
> >  gdb/debuginfod-support.c                      | 46 +++++++++++++++++++
> >  gdb/debuginfod-support.h                      | 16 +++++++
> >  gdb/gcore.in                                  |  3 ++
> >  gdb/solib.c                                   | 35 ++++++++++++++
> >  .../gdb.debuginfod/fetch_src_and_symbols.exp  | 25 +++++++++-
> >  6 files changed, 156 insertions(+), 2 deletions(-)
> >
> > diff --git a/gdb/corelow.c b/gdb/corelow.c
> > index 97eadceed84..2c9ea1044b7 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"
> > @@ -229,6 +231,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);
> > @@ -416,6 +423,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..04b39b57f6f 100644
> > --- a/gdb/debuginfod-support.c
> > +++ b/gdb/debuginfod-support.c
> > @@ -41,6 +41,16 @@ 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);
> > +}
> > +
> >  #else
> >  #include <elfutils/debuginfod.h>
> >
> > @@ -194,4 +204,40 @@ 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;
> > +}
> >  #endif
> > diff --git a/gdb/debuginfod-support.h b/gdb/debuginfod-support.h
> > index 5e5aab56e74..c29a07d2abe 100644
> > --- a/gdb/debuginfod-support.h
> > +++ b/gdb/debuginfod-support.h
> > @@ -61,4 +61,20 @@ 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);
> >  #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/solib.c b/gdb/solib.c
> > index 8b92cf7db53..3a7bf59e64a 100644
> > --- a/gdb/solib.c
> > +++ b/gdb/solib.c
> > @@ -49,6 +49,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"
> >
> > @@ -538,6 +540,39 @@ 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 ()));
> > +  const char *build_id_hexstr =
> > +    current_program_space->get_cbfd_soname_build_id (so->so_name);
> > +
> > +  /* If we already have core file build-id information for this solib,
> > +     verify it matches abfd's build-id.  If there is a mismatch or
> > +     the solib wasn't found, attempt to query debuginfod for
> > +     the correct solib.  */
> > +  if (build_id_hexstr != NULL)
> > +    {
> > +      bool mismatch = false;
> > +
> > +      if (abfd != NULL && abfd->build_id != NULL)
> > +       {
> > +         std::string build_id = build_id_to_string (abfd->build_id);
> > +         if (build_id != build_id_hexstr)
> > +           {
> > +             warning (_("Build-id of %ps does not match core file."),
> > +                      styled_string (file_name_style.style (),
> > +                                     filename.get ()));
> > +             mismatch = true;
> > +           }
> > +       }
> > +
> > +      if (abfd == NULL || mismatch)
> > +       {
> > +         scoped_fd fd = debuginfod_exec_query ((const unsigned char*)
> > +                                               build_id_hexstr,
> > +                                               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
> >

[PING**3][PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings

[Aaron Merey]

Ping

Thanks,
Aaron

On Mon, Oct 18, 2021 at 7:05 PM Aaron Merey <amerey@redhat.com> wrote:
>
> Ping
>
> Thanks,
> Aaron
>
> On Tue, Sep 28, 2021 at 9:13 PM Aaron Merey <amerey@redhat.com> wrote:
> >
> > Ping.
> >
> > Thanks,
> > Aaron
> >
> > On Thu, Aug 12, 2021 at 12:25 AM Aaron Merey via Gdb-patches
> > <gdb-patches@sourceware.org> wrote:
> > >
> > > Add debuginfod support to core_target::build_file_mappings and
> > > locate_exec_from_corefile_build_id to enable the downloading of
> > > missing core file executables and shared libraries.
> > >
> > > Also add debuginfod support to solib_map_sections so that previously
> > > downloaded shared libraries can be retrieved from the debuginfod
> > > cache.
> > >
> > > When core file shared libraries are found locally, verify
> > > that their build-ids match the corresponding build-id found in
> > > the core file.  If there is a mismatch, print a warning and attempt
> > > to query debuginfod for the correct build of the shared library:
> > >
> > > warning: Build-id of /lib64/libc.so.6 does not match core file.
> > > Downloading XY.Z MB executable for /lib64/libc.so.6
> > > ---
> > >  gdb/corelow.c                                 | 33 +++++++++++++
> > >  gdb/debuginfod-support.c                      | 46 +++++++++++++++++++
> > >  gdb/debuginfod-support.h                      | 16 +++++++
> > >  gdb/gcore.in                                  |  3 ++
> > >  gdb/solib.c                                   | 35 ++++++++++++++
> > >  .../gdb.debuginfod/fetch_src_and_symbols.exp  | 25 +++++++++-
> > >  6 files changed, 156 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/gdb/corelow.c b/gdb/corelow.c
> > > index 97eadceed84..2c9ea1044b7 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"
> > > @@ -229,6 +231,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);
> > > @@ -416,6 +423,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..04b39b57f6f 100644
> > > --- a/gdb/debuginfod-support.c
> > > +++ b/gdb/debuginfod-support.c
> > > @@ -41,6 +41,16 @@ 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);
> > > +}
> > > +
> > >  #else
> > >  #include <elfutils/debuginfod.h>
> > >
> > > @@ -194,4 +204,40 @@ 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;
> > > +}
> > >  #endif
> > > diff --git a/gdb/debuginfod-support.h b/gdb/debuginfod-support.h
> > > index 5e5aab56e74..c29a07d2abe 100644
> > > --- a/gdb/debuginfod-support.h
> > > +++ b/gdb/debuginfod-support.h
> > > @@ -61,4 +61,20 @@ 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);
> > >  #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/solib.c b/gdb/solib.c
> > > index 8b92cf7db53..3a7bf59e64a 100644
> > > --- a/gdb/solib.c
> > > +++ b/gdb/solib.c
> > > @@ -49,6 +49,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"
> > >
> > > @@ -538,6 +540,39 @@ 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 ()));
> > > +  const char *build_id_hexstr =
> > > +    current_program_space->get_cbfd_soname_build_id (so->so_name);
> > > +
> > > +  /* If we already have core file build-id information for this solib,
> > > +     verify it matches abfd's build-id.  If there is a mismatch or
> > > +     the solib wasn't found, attempt to query debuginfod for
> > > +     the correct solib.  */
> > > +  if (build_id_hexstr != NULL)
> > > +    {
> > > +      bool mismatch = false;
> > > +
> > > +      if (abfd != NULL && abfd->build_id != NULL)
> > > +       {
> > > +         std::string build_id = build_id_to_string (abfd->build_id);
> > > +         if (build_id != build_id_hexstr)
> > > +           {
> > > +             warning (_("Build-id of %ps does not match core file."),
> > > +                      styled_string (file_name_style.style (),
> > > +                                     filename.get ()));
> > > +             mismatch = true;
> > > +           }
> > > +       }
> > > +
> > > +      if (abfd == NULL || mismatch)
> > > +       {
> > > +         scoped_fd fd = debuginfod_exec_query ((const unsigned char*)
> > > +                                               build_id_hexstr,
> > > +                                               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
> > >

Re: [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings

[Simon Marchi]

On 2021-08-12 00:24, Aaron Merey wrote:
> Add debuginfod support to core_target::build_file_mappings and
> locate_exec_from_corefile_build_id to enable the downloading of
> missing core file executables and shared libraries.
> 
> Also add debuginfod support to solib_map_sections so that previously
> downloaded shared libraries can be retrieved from the debuginfod
> cache.
> 
> When core file shared libraries are found locally, verify
> that their build-ids match the corresponding build-id found in
> the core file.  If there is a mismatch, print a warning and attempt
> to query debuginfod for the correct build of the shared library:
> 
> warning: Build-id of /lib64/libc.so.6 does not match core file.
> Downloading XY.Z MB executable for /lib64/libc.so.6

Since this is getting a bit old, it doesn't apply anymore.  Can you send
a new version of the series (including fixes to patch 2)?

Thanks

Simon

[PATCH v4 0/3] Add debuginfod core file support

[Aaron Merey]

v3 of this series can be found here:
https://sourceware.org/pipermail/gdb-patches/2021-August/181416.html

Aaron Merey (3):
  gdb: Add aliases for read_core_file_mappings callbacks
  gdb: Add soname to build-id mapping for corefiles
  PR gdb/27570: missing support for debuginfod in
    core_target::build_file_mappings

 gdb/arch-utils.c                              | 15 ++--
 gdb/arch-utils.h                              | 15 ++--
 gdb/corelow.c                                 | 46 ++++++++++++-
 gdb/debuginfod-support.c                      | 44 ++++++++++++
 gdb/debuginfod-support.h                      | 17 +++++
 gdb/gcore.in                                  |  3 +
 gdb/gdbarch.c                                 |  2 +-
 gdb/gdbarch.h                                 | 16 ++++-
 gdb/gdbarch.sh                                | 14 +++-
 gdb/linux-tdep.c                              | 52 ++++++++++----
 gdb/progspace.c                               | 32 +++++++++
 gdb/progspace.h                               | 18 +++++
 gdb/solib.c                                   | 69 +++++++++++++++++++
 gdb/solib.h                                   |  5 ++
 .../gdb.debuginfod/fetch_src_and_symbols.exp  | 26 ++++++-
 15 files changed, 335 insertions(+), 39 deletions(-)

-- 
2.31.1

[PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings

[Aaron Merey]

Add debuginfod support to core_target::build_file_mappings and
locate_exec_from_corefile_build_id to enable the downloading of
missing executables and shared libraries referenced in core files.

Also add debuginfod support to solib_map_sections so that previously
downloaded shared libraries can be retrieved from the debuginfod
cache.

When core file shared libraries are found locally, verify that
their build-ids match the corresponding build-ids found in the
core file.  If there is a mismatch, attempt to query debuginfod
for the correct build and print a warning if unsuccessful:

  warning: Build-id of /lib64/libc.so.6 does not match core file.
---
 gdb/corelow.c                                 | 33 ++++++++++++++
 gdb/debuginfod-support.c                      | 44 +++++++++++++++++++
 gdb/debuginfod-support.h                      | 17 +++++++
 gdb/gcore.in                                  |  3 ++
 gdb/solib.c                                   | 34 ++++++++++++++
 .../gdb.debuginfod/fetch_src_and_symbols.exp  | 26 ++++++++++-
 6 files changed, 155 insertions(+), 2 deletions(-)

diff --git a/gdb/corelow.c b/gdb/corelow.c
index d1256a9e99b..c81ae13c804 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"
@@ -229,6 +231,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);
@@ -416,6 +423,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 a1269772b2e..07b4ac4601e 100644
--- a/gdb/debuginfod-support.c
+++ b/gdb/debuginfod-support.c
@@ -54,6 +54,15 @@ 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);
+}
+
 #define NO_IMPL _("Support for debuginfod is not compiled into GDB.")
 
 /* Stub set/show commands that indicate debuginfod is not supported.  */
@@ -396,6 +405,41 @@ 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)
+{
+  if (!debuginfod_enabled ())
+    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 (debuginfod_verbose > 0 && 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;
+}
 #endif
 
 /* Register debuginfod commands.  */
diff --git a/gdb/debuginfod-support.h b/gdb/debuginfod-support.h
index 5e5aab56e74..7815a2f6ede 100644
--- a/gdb/debuginfod-support.h
+++ b/gdb/debuginfod-support.h
@@ -61,4 +61,21 @@ 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);
 #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/solib.c b/gdb/solib.c
index c0beea19d7b..87a3fc0c462 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -49,6 +49,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"
 
@@ -539,6 +541,38 @@ 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 ()));
+  const char *build_id_hexstr =
+    current_program_space->get_cbfd_soname_build_id (so->so_name);
+
+  /* If we already know the build-id of this solib from a core file,
+     verify it matches abfd's build-id.  If there is a mismatch or
+     the solib wasn't found, attempt to query debuginfod for
+     the correct solib.  */
+  if (build_id_hexstr != nullptr)
+    {
+      bool mismatch = false;
+
+      if (abfd != nullptr && abfd->build_id != nullptr)
+	{
+	  std::string build_id = build_id_to_string (abfd->build_id);
+
+	  if (build_id != build_id_hexstr)
+	    mismatch = true;
+	}
+
+      if (abfd == nullptr || mismatch)
+	{
+	  scoped_fd fd = debuginfod_exec_query ((const unsigned char*)
+						build_id_hexstr,
+						0, so->so_name, &filename);
+
+	  if (fd.get () >= 0)
+	    abfd = ops->bfd_open (filename.get ());
+	  else if (mismatch)
+	    warning (_("Build-id of %ps does not match core file."),
+		     styled_string (file_name_style.style (), 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 31e9e4a92f0..656bcb522ba 100644
--- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
+++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
@@ -63,6 +63,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} {
@@ -114,8 +119,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 ""
 
@@ -167,10 +174,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
@@ -234,6 +251,11 @@ proc local_url { } {
     gdb_test "br main" "Breakpoint 1 at.*file.*"
     gdb_test "l" ".*This program is distributed in the hope.*"
 
+    # gdb should now find the executable file
+    clean_restart
+    gdb_test "core $corefile" ".*return 0.*" "file [file tail $corefile]" \
+	"Enable debuginfod?.*" "y"
+
     # gdb should now find the debugaltlink file
     clean_restart
     gdb_test "file ${binfile}_alt.o" \
-- 
2.31.1

Re: [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings

[Simon Marchi]

> @@ -416,6 +423,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));
> +	    }
> +	}
> +    }

The duplication of the steps taken when we have successfully found a BFD
(call exec_file_attach and symbol_file_add_main) annoy me a little bit
here.  If those steps need to change, it would be easy to update one and
forget the other.  Can you refactor the code to be something like:

  gdb_bfd_ref_ptr execbfd
    = build_id_to_exec_bfd (build_id->size, build_id->data);

  if (execbfd == nullptr)
    {
      // Try to get execbfd using debuginfod, leave execbfd nullptr if
      // not successful.
    }

  if (execbfd != nullptr)
    {
      // We found a BFD one way or another, call exec_file_attach and
      // symbol_file_add_main.
    }

> 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

I think this should be mentioned in the commit log, with an explanation
of why there may be queries and why they are unnecessary.

> @@ -114,8 +119,10 @@ proc write_dwarf_file {filename buildid {value 99}} {
>      }
>  }
>  
> +set corefile "corefile"
> +
>  proc no_url { } {
> -    global binfile outputdir debugdir
> +    global binfile corefile outputdir debugdir

Use $::corefile to refer to the global variable, rather than using the
"global" keyword.

>  
>      setenv DEBUGINFOD_URLS ""
>  
> @@ -167,10 +174,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"

Where exactly is that "corefile" file written to?  I suppose that it's
written to the current working directory, which would be
$build/gdb/testsuite.  Even though it's temporary (the file is moved
right after), the test shouldn't write outside of its output directory.
If the user does ^C exactly here, it will leave a random file there.
Also, if two tests decide to write a "corefile" file, they could step on
each other's toes when running tests in parallel.

Simon

Re: [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings

[Aaron Merey]

Hi Simon,

I've fixed the issues you pointed out.  The revised patch is below.

Thanks,
Aaron

From 357ffcc705057b4ed324cc39c2f5159693491fbe Mon Sep 17 00:00:00 2001
From: Aaron Merey <amerey@redhat.com>
Date: Tue, 16 Nov 2021 19:45:44 -0500
Subject: [PATCH 3/3] PR gdb/27570: missing support for debuginfod in
 core_target::build_file_mappings

Add debuginfod support to core_target::build_file_mappings and
locate_exec_from_corefile_build_id to enable the downloading of
missing executables and shared libraries referenced in core files.

Also add debuginfod support to solib_map_sections so that previously
downloaded shared libraries can be retrieved from the debuginfod
cache.

When core file shared libraries are found locally, verify that
their build-ids match the corresponding build-ids found in the
core file.  If there is a mismatch, attempt to query debuginfod
for the correct build and print a warning if unsuccessful:

  warning: Build-id of /lib64/libc.so.6 does not match core file.

Also unset the DEBUGINFOD_URLS environment variable in gcore.in
to disable debuginfod when gcore invokes gdb.  Debuginfo is not
needed for core file generation so debuginfod queries will slow
down gcore unnecessarily.
---
 gdb/corelow.c                                 | 27 ++++++++++++
 gdb/debuginfod-support.c                      | 44 +++++++++++++++++++
 gdb/debuginfod-support.h                      | 17 +++++++
 gdb/gcore.in                                  |  3 ++
 gdb/solib.c                                   | 34 ++++++++++++++
 .../gdb.debuginfod/fetch_src_and_symbols.exp  | 22 ++++++++++
 6 files changed, 147 insertions(+)

diff --git a/gdb/corelow.c b/gdb/corelow.c
index d1256a9e99b..8141f815e39 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"
@@ -229,6 +231,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);
@@ -410,6 +417,26 @@ locate_exec_from_corefile_build_id (bfd *abfd, int from_tty)
   gdb_bfd_ref_ptr execbfd
     = build_id_to_exec_bfd (build_id->size, build_id->data);
 
+  if (execbfd == nullptr)
+    {
+      /* Attempt to query debuginfod for the executable.  */
+      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);
+	}
+    }
+
   if (execbfd != nullptr)
     {
       exec_file_attach (bfd_get_filename (execbfd.get ()), from_tty);
diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
index 2e1837da949..c4c593148b6 100644
--- a/gdb/debuginfod-support.c
+++ b/gdb/debuginfod-support.c
@@ -68,6 +68,15 @@ 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);
+}
+
 #define NO_IMPL _("Support for debuginfod is not compiled into GDB.")
 
 #else
@@ -256,6 +265,41 @@ 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)
+{
+  if (!debuginfod_is_enabled ())
+    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 (debuginfod_verbose > 0 && 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;
+}
 #endif
 
 /* Set callback for "set debuginfod enabled".  */
diff --git a/gdb/debuginfod-support.h b/gdb/debuginfod-support.h
index 5e5aab56e74..7815a2f6ede 100644
--- a/gdb/debuginfod-support.h
+++ b/gdb/debuginfod-support.h
@@ -61,4 +61,21 @@ 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);
 #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/solib.c b/gdb/solib.c
index c0beea19d7b..87a3fc0c462 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -49,6 +49,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"
 
@@ -539,6 +541,38 @@ 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 ()));
+  const char *build_id_hexstr =
+    current_program_space->get_cbfd_soname_build_id (so->so_name);
+
+  /* If we already know the build-id of this solib from a core file,
+     verify it matches abfd's build-id.  If there is a mismatch or
+     the solib wasn't found, attempt to query debuginfod for
+     the correct solib.  */
+  if (build_id_hexstr != nullptr)
+    {
+      bool mismatch = false;
+
+      if (abfd != nullptr && abfd->build_id != nullptr)
+	{
+	  std::string build_id = build_id_to_string (abfd->build_id);
+
+	  if (build_id != build_id_hexstr)
+	    mismatch = true;
+	}
+
+      if (abfd == nullptr || mismatch)
+	{
+	  scoped_fd fd = debuginfod_exec_query ((const unsigned char*)
+						build_id_hexstr,
+						0, so->so_name, &filename);
+
+	  if (fd.get () >= 0)
+	    abfd = ops->bfd_open (filename.get ());
+	  else if (mismatch)
+	    warning (_("Build-id of %ps does not match core file."),
+		     styled_string (file_name_style.style (), 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 757bd201b17..cfe2ae7f7da 100644
--- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
+++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
@@ -63,6 +63,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} {
@@ -114,6 +119,8 @@ proc write_dwarf_file {filename buildid {value 99}} {
     }
 }
 
+set corefile [standard_output_file "corefile"]
+
 proc no_url { } {
     global binfile outputdir debugdir
 
@@ -167,6 +174,16 @@ 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 [file tail $::corefile] gen"
+    file rename -force ${binfile}2 $debugdir
+
+    clean_restart
+    gdb_test "core $::corefile" ".*in ?? ().*" "file [file tail $::corefile]"
 }
 
 proc local_url { } {
@@ -234,6 +251,11 @@ proc local_url { } {
     gdb_test "br main" "Breakpoint 1 at.*file.*"
     gdb_test "l" ".*This program is distributed in the hope.*"
 
+    # gdb should now find the executable file
+    clean_restart
+    gdb_test "core $::corefile" ".*return 0.*" "file [file tail $::corefile]" \
+	"Enable debuginfod?.*" "y"
+
     # gdb should now find the debugaltlink file
     clean_restart
     gdb_test "file ${binfile}_alt.o" \
-- 
2.31.1

Re: [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings

[Aaron Merey]

Ping

Thanks,
Aaron

On Tue, Nov 16, 2021 at 10:28 PM Aaron Merey via Gdb-patches
<gdb-patches@sourceware.org> wrote:
>
> Hi Simon,
>
> I've fixed the issues you pointed out.  The revised patch is below.
>
> Thanks,
> Aaron
>
> From 357ffcc705057b4ed324cc39c2f5159693491fbe Mon Sep 17 00:00:00 2001
> From: Aaron Merey <amerey@redhat.com>
> Date: Tue, 16 Nov 2021 19:45:44 -0500
> Subject: [PATCH 3/3] PR gdb/27570: missing support for debuginfod in
>  core_target::build_file_mappings
>
> Add debuginfod support to core_target::build_file_mappings and
> locate_exec_from_corefile_build_id to enable the downloading of
> missing executables and shared libraries referenced in core files.
>
> Also add debuginfod support to solib_map_sections so that previously
> downloaded shared libraries can be retrieved from the debuginfod
> cache.
>
> When core file shared libraries are found locally, verify that
> their build-ids match the corresponding build-ids found in the
> core file.  If there is a mismatch, attempt to query debuginfod
> for the correct build and print a warning if unsuccessful:
>
>   warning: Build-id of /lib64/libc.so.6 does not match core file.
>
> Also unset the DEBUGINFOD_URLS environment variable in gcore.in
> to disable debuginfod when gcore invokes gdb.  Debuginfo is not
> needed for core file generation so debuginfod queries will slow
> down gcore unnecessarily.
> ---
>  gdb/corelow.c                                 | 27 ++++++++++++
>  gdb/debuginfod-support.c                      | 44 +++++++++++++++++++
>  gdb/debuginfod-support.h                      | 17 +++++++
>  gdb/gcore.in                                  |  3 ++
>  gdb/solib.c                                   | 34 ++++++++++++++
>  .../gdb.debuginfod/fetch_src_and_symbols.exp  | 22 ++++++++++
>  6 files changed, 147 insertions(+)
>
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index d1256a9e99b..8141f815e39 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"
> @@ -229,6 +231,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);
> @@ -410,6 +417,26 @@ locate_exec_from_corefile_build_id (bfd *abfd, int from_tty)
>    gdb_bfd_ref_ptr execbfd
>      = build_id_to_exec_bfd (build_id->size, build_id->data);
>
> +  if (execbfd == nullptr)
> +    {
> +      /* Attempt to query debuginfod for the executable.  */
> +      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);
> +       }
> +    }
> +
>    if (execbfd != nullptr)
>      {
>        exec_file_attach (bfd_get_filename (execbfd.get ()), from_tty);
> diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
> index 2e1837da949..c4c593148b6 100644
> --- a/gdb/debuginfod-support.c
> +++ b/gdb/debuginfod-support.c
> @@ -68,6 +68,15 @@ 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);
> +}
> +
>  #define NO_IMPL _("Support for debuginfod is not compiled into GDB.")
>
>  #else
> @@ -256,6 +265,41 @@ 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)
> +{
> +  if (!debuginfod_is_enabled ())
> +    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 (debuginfod_verbose > 0 && 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;
> +}
>  #endif
>
>  /* Set callback for "set debuginfod enabled".  */
> diff --git a/gdb/debuginfod-support.h b/gdb/debuginfod-support.h
> index 5e5aab56e74..7815a2f6ede 100644
> --- a/gdb/debuginfod-support.h
> +++ b/gdb/debuginfod-support.h
> @@ -61,4 +61,21 @@ 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);
>  #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/solib.c b/gdb/solib.c
> index c0beea19d7b..87a3fc0c462 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -49,6 +49,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"
>
> @@ -539,6 +541,38 @@ 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 ()));
> +  const char *build_id_hexstr =
> +    current_program_space->get_cbfd_soname_build_id (so->so_name);
> +
> +  /* If we already know the build-id of this solib from a core file,
> +     verify it matches abfd's build-id.  If there is a mismatch or
> +     the solib wasn't found, attempt to query debuginfod for
> +     the correct solib.  */
> +  if (build_id_hexstr != nullptr)
> +    {
> +      bool mismatch = false;
> +
> +      if (abfd != nullptr && abfd->build_id != nullptr)
> +       {
> +         std::string build_id = build_id_to_string (abfd->build_id);
> +
> +         if (build_id != build_id_hexstr)
> +           mismatch = true;
> +       }
> +
> +      if (abfd == nullptr || mismatch)
> +       {
> +         scoped_fd fd = debuginfod_exec_query ((const unsigned char*)
> +                                               build_id_hexstr,
> +                                               0, so->so_name, &filename);
> +
> +         if (fd.get () >= 0)
> +           abfd = ops->bfd_open (filename.get ());
> +         else if (mismatch)
> +           warning (_("Build-id of %ps does not match core file."),
> +                    styled_string (file_name_style.style (), 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 757bd201b17..cfe2ae7f7da 100644
> --- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> +++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> @@ -63,6 +63,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} {
> @@ -114,6 +119,8 @@ proc write_dwarf_file {filename buildid {value 99}} {
>      }
>  }
>
> +set corefile [standard_output_file "corefile"]
> +
>  proc no_url { } {
>      global binfile outputdir debugdir
>
> @@ -167,6 +174,16 @@ 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 [file tail $::corefile] gen"
> +    file rename -force ${binfile}2 $debugdir
> +
> +    clean_restart
> +    gdb_test "core $::corefile" ".*in ?? ().*" "file [file tail $::corefile]"
>  }
>
>  proc local_url { } {
> @@ -234,6 +251,11 @@ proc local_url { } {
>      gdb_test "br main" "Breakpoint 1 at.*file.*"
>      gdb_test "l" ".*This program is distributed in the hope.*"
>
> +    # gdb should now find the executable file
> +    clean_restart
> +    gdb_test "core $::corefile" ".*return 0.*" "file [file tail $::corefile]" \
> +       "Enable debuginfod?.*" "y"
> +
>      # gdb should now find the debugaltlink file
>      clean_restart
>      gdb_test "file ${binfile}_alt.o" \
> --
> 2.31.1
>

Re: [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings

[Aaron Merey]

Ping ** 2

Thanks,
Aaron

On Tue, Jan 25, 2022 at 8:42 PM Aaron Merey <amerey@redhat.com> wrote:
>
> Ping
>
> Thanks,
> Aaron
>
> On Tue, Nov 16, 2021 at 10:28 PM Aaron Merey via Gdb-patches
> <gdb-patches@sourceware.org> wrote:
> >
> > Hi Simon,
> >
> > I've fixed the issues you pointed out.  The revised patch is below.
> >
> > Thanks,
> > Aaron
> >
> > From 357ffcc705057b4ed324cc39c2f5159693491fbe Mon Sep 17 00:00:00 2001
> > From: Aaron Merey <amerey@redhat.com>
> > Date: Tue, 16 Nov 2021 19:45:44 -0500
> > Subject: [PATCH 3/3] PR gdb/27570: missing support for debuginfod in
> >  core_target::build_file_mappings
> >
> > Add debuginfod support to core_target::build_file_mappings and
> > locate_exec_from_corefile_build_id to enable the downloading of
> > missing executables and shared libraries referenced in core files.
> >
> > Also add debuginfod support to solib_map_sections so that previously
> > downloaded shared libraries can be retrieved from the debuginfod
> > cache.
> >
> > When core file shared libraries are found locally, verify that
> > their build-ids match the corresponding build-ids found in the
> > core file.  If there is a mismatch, attempt to query debuginfod
> > for the correct build and print a warning if unsuccessful:
> >
> >   warning: Build-id of /lib64/libc.so.6 does not match core file.
> >
> > Also unset the DEBUGINFOD_URLS environment variable in gcore.in
> > to disable debuginfod when gcore invokes gdb.  Debuginfo is not
> > needed for core file generation so debuginfod queries will slow
> > down gcore unnecessarily.
> > ---
> >  gdb/corelow.c                                 | 27 ++++++++++++
> >  gdb/debuginfod-support.c                      | 44 +++++++++++++++++++
> >  gdb/debuginfod-support.h                      | 17 +++++++
> >  gdb/gcore.in                                  |  3 ++
> >  gdb/solib.c                                   | 34 ++++++++++++++
> >  .../gdb.debuginfod/fetch_src_and_symbols.exp  | 22 ++++++++++
> >  6 files changed, 147 insertions(+)
> >
> > diff --git a/gdb/corelow.c b/gdb/corelow.c
> > index d1256a9e99b..8141f815e39 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"
> > @@ -229,6 +231,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);
> > @@ -410,6 +417,26 @@ locate_exec_from_corefile_build_id (bfd *abfd, int from_tty)
> >    gdb_bfd_ref_ptr execbfd
> >      = build_id_to_exec_bfd (build_id->size, build_id->data);
> >
> > +  if (execbfd == nullptr)
> > +    {
> > +      /* Attempt to query debuginfod for the executable.  */
> > +      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);
> > +       }
> > +    }
> > +
> >    if (execbfd != nullptr)
> >      {
> >        exec_file_attach (bfd_get_filename (execbfd.get ()), from_tty);
> > diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
> > index 2e1837da949..c4c593148b6 100644
> > --- a/gdb/debuginfod-support.c
> > +++ b/gdb/debuginfod-support.c
> > @@ -68,6 +68,15 @@ 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);
> > +}
> > +
> >  #define NO_IMPL _("Support for debuginfod is not compiled into GDB.")
> >
> >  #else
> > @@ -256,6 +265,41 @@ 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)
> > +{
> > +  if (!debuginfod_is_enabled ())
> > +    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 (debuginfod_verbose > 0 && 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;
> > +}
> >  #endif
> >
> >  /* Set callback for "set debuginfod enabled".  */
> > diff --git a/gdb/debuginfod-support.h b/gdb/debuginfod-support.h
> > index 5e5aab56e74..7815a2f6ede 100644
> > --- a/gdb/debuginfod-support.h
> > +++ b/gdb/debuginfod-support.h
> > @@ -61,4 +61,21 @@ 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);
> >  #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/solib.c b/gdb/solib.c
> > index c0beea19d7b..87a3fc0c462 100644
> > --- a/gdb/solib.c
> > +++ b/gdb/solib.c
> > @@ -49,6 +49,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"
> >
> > @@ -539,6 +541,38 @@ 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 ()));
> > +  const char *build_id_hexstr =
> > +    current_program_space->get_cbfd_soname_build_id (so->so_name);
> > +
> > +  /* If we already know the build-id of this solib from a core file,
> > +     verify it matches abfd's build-id.  If there is a mismatch or
> > +     the solib wasn't found, attempt to query debuginfod for
> > +     the correct solib.  */
> > +  if (build_id_hexstr != nullptr)
> > +    {
> > +      bool mismatch = false;
> > +
> > +      if (abfd != nullptr && abfd->build_id != nullptr)
> > +       {
> > +         std::string build_id = build_id_to_string (abfd->build_id);
> > +
> > +         if (build_id != build_id_hexstr)
> > +           mismatch = true;
> > +       }
> > +
> > +      if (abfd == nullptr || mismatch)
> > +       {
> > +         scoped_fd fd = debuginfod_exec_query ((const unsigned char*)
> > +                                               build_id_hexstr,
> > +                                               0, so->so_name, &filename);
> > +
> > +         if (fd.get () >= 0)
> > +           abfd = ops->bfd_open (filename.get ());
> > +         else if (mismatch)
> > +           warning (_("Build-id of %ps does not match core file."),
> > +                    styled_string (file_name_style.style (), 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 757bd201b17..cfe2ae7f7da 100644
> > --- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> > +++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> > @@ -63,6 +63,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} {
> > @@ -114,6 +119,8 @@ proc write_dwarf_file {filename buildid {value 99}} {
> >      }
> >  }
> >
> > +set corefile [standard_output_file "corefile"]
> > +
> >  proc no_url { } {
> >      global binfile outputdir debugdir
> >
> > @@ -167,6 +174,16 @@ 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 [file tail $::corefile] gen"
> > +    file rename -force ${binfile}2 $debugdir
> > +
> > +    clean_restart
> > +    gdb_test "core $::corefile" ".*in ?? ().*" "file [file tail $::corefile]"
> >  }
> >
> >  proc local_url { } {
> > @@ -234,6 +251,11 @@ proc local_url { } {
> >      gdb_test "br main" "Breakpoint 1 at.*file.*"
> >      gdb_test "l" ".*This program is distributed in the hope.*"
> >
> > +    # gdb should now find the executable file
> > +    clean_restart
> > +    gdb_test "core $::corefile" ".*return 0.*" "file [file tail $::corefile]" \
> > +       "Enable debuginfod?.*" "y"
> > +
> >      # gdb should now find the debugaltlink file
> >      clean_restart
> >      gdb_test "file ${binfile}_alt.o" \
> > --
> > 2.31.1
> >