* [PATCH 1/3] gdb: Add aliases for read_core_file_mappings callbacks
2021-11-10 1:47 [PATCH v4 0/3] Add debuginfod core file support Aaron Merey
@ 2021-11-10 1:47 ` Aaron Merey
2021-11-14 2:20 ` Simon Marchi
2021-11-10 1:47 ` [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles Aaron Merey
2021-11-10 1:47 ` [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings Aaron Merey
2 siblings, 1 reply; 17+ messages in thread
From: Aaron Merey @ 2021-11-10 1:47 UTC (permalink / raw)
To: gdb-patches; +Cc: simon.marchi, tom, lsix
Add aliases read_core_file_mappings_loop_ftype and
read_core_file_mappings_pre_loop_ftype. Intended for use with
read_core_file_mappings.
Also add build_id parameter to read_core_file_mappings_loop_ftype.
---
gdb/arch-utils.c | 15 +++++----------
gdb/arch-utils.h | 15 +++++----------
gdb/corelow.c | 2 +-
gdb/gdbarch.c | 2 +-
gdb/gdbarch.h | 16 ++++++++++++++--
gdb/gdbarch.sh | 14 +++++++++++++-
gdb/linux-tdep.c | 19 +++++++------------
7 files changed, 46 insertions(+), 37 deletions(-)
diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 862f26b6cf7..6c6dca22d63 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,
+ read_core_file_mappings_pre_loop_ftype pre_loop_cb,
+ read_core_file_mappings_loop_ftype loop_cb)
{
}
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index 03e9082f6d7..07cc496a02a 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -295,14 +295,9 @@ 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,
+ read_core_file_mappings_pre_loop_ftype pre_loop_cb,
+ read_core_file_mappings_loop_ftype loop_cb);
#endif /* ARCH_UTILS_H */
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 5f48d96aa12..10942e6af01 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. */
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index f89dcc57754..689187f8b1e 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, read_core_file_mappings_pre_loop_ftype pre_loop_cb, read_core_file_mappings_loop_ftype 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..f7d624fcfdc 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -129,6 +129,18 @@ enum class memtag_type
allocation,
};
+/* Callback types for 'read_core_file_mappings' gdbarch method. */
+
+using read_core_file_mappings_pre_loop_ftype =
+ gdb::function_view<void (ULONGEST count)>;
+
+using read_core_file_mappings_loop_ftype =
+ gdb::function_view<void (int num,
+ ULONGEST start,
+ ULONGEST end,
+ ULONGEST file_ofs,
+ const char *filename,
+ const bfd_build_id *build_id)>;
/* The following are pre-initialized by GDBARCH. */
@@ -1710,8 +1722,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, read_core_file_mappings_pre_loop_ftype pre_loop_cb, read_core_file_mappings_loop_ftype loop_cb);
+extern void gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, read_core_file_mappings_pre_loop_ftype pre_loop_cb, read_core_file_mappings_loop_ftype 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..06fd71304ea 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, read_core_file_mappings_pre_loop_ftype pre_loop_cb, read_core_file_mappings_loop_ftype loop_cb;cbfd, pre_loop_cb, loop_cb;;default_read_core_file_mappings;;0
EOF
}
@@ -1400,6 +1400,18 @@ enum class memtag_type
allocation,
};
+/* Callback types for 'read_core_file_mappings' gdbarch method. */
+
+using read_core_file_mappings_pre_loop_ftype =
+ gdb::function_view<void (ULONGEST count)>;
+
+using read_core_file_mappings_loop_ftype =
+ gdb::function_view<void (int num,
+ ULONGEST start,
+ ULONGEST end,
+ ULONGEST file_ofs,
+ const char *filename,
+ const bfd_build_id *build_id)>;
EOF
# function typedef's
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index ae2f7c14f6d..e2cff83086a 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1097,16 +1097,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,
+ read_core_file_mappings_pre_loop_ftype pre_loop_cb,
+ read_core_file_mappings_loop_ftype loop_cb)
{
/* Ensure that ULONGEST is big enough for reading 64-bit core files. */
gdb_static_assert (sizeof (ULONGEST) >= 8);
@@ -1189,7 +1184,7 @@ linux_read_core_file_mappings (struct gdbarch *gdbarch,
char * filename = filenames;
filenames += strlen ((char *) filenames) + 1;
- loop_cb (i, start, end, file_ofs, filename);
+ loop_cb (i, start, end, file_ofs, filename, nullptr);
}
}
@@ -1218,7 +1213,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",
--
2.31.1
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 1/3] gdb: Add aliases for read_core_file_mappings callbacks
2021-11-10 1:47 ` [PATCH 1/3] gdb: Add aliases for read_core_file_mappings callbacks Aaron Merey
@ 2021-11-14 2:20 ` Simon Marchi
2021-11-17 3:39 ` Aaron Merey
0 siblings, 1 reply; 17+ messages in thread
From: Simon Marchi @ 2021-11-14 2:20 UTC (permalink / raw)
To: Aaron Merey, gdb-patches; +Cc: tom, lsix
On 2021-11-09 20:47, Aaron Merey wrote:
> Add aliases read_core_file_mappings_loop_ftype and
> read_core_file_mappings_pre_loop_ftype. Intended for use with
> read_core_file_mappings.
>
> Also add build_id parameter to read_core_file_mappings_loop_ftype.
This patch LGTM, thanks.
Simon
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] gdb: Add aliases for read_core_file_mappings callbacks
2021-11-14 2:20 ` Simon Marchi
@ 2021-11-17 3:39 ` Aaron Merey
0 siblings, 0 replies; 17+ messages in thread
From: Aaron Merey @ 2021-11-17 3:39 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches, Tom Tromey, lsix
On Sat, Nov 13, 2021 at 9:21 PM Simon Marchi <simon.marchi@polymtl.ca> wrote:
>
> On 2021-11-09 20:47, Aaron Merey wrote:
> > Add aliases read_core_file_mappings_loop_ftype and
> > read_core_file_mappings_pre_loop_ftype. Intended for use with
> > read_core_file_mappings.
> >
> > Also add build_id parameter to read_core_file_mappings_loop_ftype.
>
> This patch LGTM, thanks.
Thanks Simon. Pushed as commit aa95b2d4388.
Aaron
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles
2021-11-10 1:47 [PATCH v4 0/3] Add debuginfod core file support Aaron Merey
2021-11-10 1:47 ` [PATCH 1/3] gdb: Add aliases for read_core_file_mappings callbacks Aaron Merey
@ 2021-11-10 1:47 ` Aaron Merey
2021-11-14 2:36 ` Simon Marchi
2021-11-10 1:47 ` [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings Aaron Merey
2 siblings, 1 reply; 17+ messages in thread
From: Aaron Merey @ 2021-11-10 1:47 UTC (permalink / raw)
To: gdb-patches; +Cc: simon.marchi, tom, lsix
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
soname to build-id 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/corelow.c | 11 +++++++++++
gdb/linux-tdep.c | 35 ++++++++++++++++++++++++++++++++++-
gdb/progspace.c | 32 ++++++++++++++++++++++++++++++++
gdb/progspace.h | 18 ++++++++++++++++++
gdb/solib.c | 35 +++++++++++++++++++++++++++++++++++
gdb/solib.h | 5 +++++
6 files changed, 135 insertions(+), 1 deletion(-)
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 10942e6af01..d1256a9e99b 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -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/linux-tdep.c b/gdb/linux-tdep.c
index e2cff83086a..a86ede6623e 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
@@ -1170,6 +1171,23 @@ linux_read_core_file_mappings
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++)
@@ -1183,8 +1201,23 @@ linux_read_core_file_mappings
descdata += addr_size;
char * filename = filenames;
filenames += strlen ((char *) filenames) + 1;
+ const bfd_build_id *build_id = nullptr;
+ auto vma_map_it = vma_map.find (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 (vma_map_it != vma_map.end ())
+ filename_map[filename] = build_id = vma_map_it->second;
+ else
+ {
+ auto filename_map_it = filename_map.find (filename);
+
+ if (filename_map_it != filename_map.end ())
+ build_id = filename_map_it->second;
+ }
- loop_cb (i, start, end, file_ofs, filename, nullptr);
+ loop_cb (i, start, end, file_ofs, filename, build_id);
}
}
diff --git a/gdb/progspace.c b/gdb/progspace.c
index 7080bf8ee27..680c5ba8f0a 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -18,6 +18,7 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include "defs.h"
+#include "build-id.h"
#include "gdbcmd.h"
#include "objfiles.h"
#include "arch-utils.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)
+{
+ m_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 = m_cbfd_soname_to_build_id.find (basename (soname));
+ if (it == m_cbfd_soname_to_build_id.end ())
+ return nullptr;
+
+ return it->second.c_str ();
+}
+
+/* See progspace.h. */
+
+void
+program_space::clear_cbfd_soname_build_ids ()
+{
+ m_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..01af5a387e3 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,10 @@ 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 shared library sonames to their
+ respective build-ids. */
+ std::unordered_map<std::string, std::string> m_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 3947c2d1d2e..c0beea19d7b 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"
@@ -1586,6 +1587,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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles
2021-11-10 1:47 ` [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles Aaron Merey
@ 2021-11-14 2:36 ` Simon Marchi
2021-11-17 3:24 ` Aaron Merey
0 siblings, 1 reply; 17+ messages in thread
From: Simon Marchi @ 2021-11-14 2:36 UTC (permalink / raw)
To: Aaron Merey, gdb-patches; +Cc: tom, lsix
> @@ -1183,8 +1201,23 @@ linux_read_core_file_mappings
> descdata += addr_size;
> char * filename = filenames;
> filenames += strlen ((char *) filenames) + 1;
> + const bfd_build_id *build_id = nullptr;
> + auto vma_map_it = vma_map.find (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 (vma_map_it != vma_map.end ())
> + filename_map[filename] = build_id = vma_map_it->second;
Please avoid doing two assignments in one statement.
> @@ -1586,6 +1587,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);
Can you explain why we are opening bfd->filename? Doesn't "abfd"
represent the exact same thing than "bfd"? So why can't we use "bfd"
directly to look up the soname?
Simon
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles
2021-11-14 2:36 ` Simon Marchi
@ 2021-11-17 3:24 ` Aaron Merey
2021-11-17 14:17 ` Tom Tromey
0 siblings, 1 reply; 17+ messages in thread
From: Aaron Merey @ 2021-11-17 3:24 UTC (permalink / raw)
To: simon.marchi; +Cc: gdb-patches, tom, lsix
Hi Simon,
On Sat, Nov 13, 2021 at 9:37 PM Simon Marchi <simon.marchi@polymtl.ca> wrote:
> > + filename_map[filename] = build_id = vma_map_it->second;
>
> Please avoid doing two assignments in one statement.
Fixed.
> > +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);
>
> Can you explain why we are opening bfd->filename? Doesn't "abfd"
> represent the exact same thing than "bfd"? So why can't we use "bfd"
> directly to look up the soname?
bfd->flags does not contain the correct value. This causes the check
for ET_DYN to fail even for shared libraries. Also bfd->build_id always
seems to be NULL. Opening "abfd" with gdb_bfd_open and passing it to
bfd_check_format populates flags and build_id with the correct values.
Passing "bfd" to bfd_check_format still does not populate these fields
with the correct values.
It looks like this happens because the format of "abfd" is initially
set to bfd_unknown. In this case bfd_check_format rereads the file and
updates some fields with the correct values, including flags and build_id.
Since format of "bfd" is instead bfd_object, bfd_check_format returns
early and skips updating any fields.
Thanks,
Aaron
From 275e69ce65aa3d5ee3b017175c73f8536a1fb559 Mon Sep 17 00:00:00 2001
From: Aaron Merey <amerey@redhat.com>
Date: Tue, 16 Nov 2021 17:33:52 -0500
Subject: [PATCH 2/3] gdb: Add soname to build-id mapping for core files
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
soname to build-id 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/corelow.c | 11 +++++++++++
gdb/linux-tdep.c | 38 +++++++++++++++++++++++++++++++++++++-
gdb/progspace.c | 32 ++++++++++++++++++++++++++++++++
gdb/progspace.h | 18 ++++++++++++++++++
gdb/solib.c | 35 +++++++++++++++++++++++++++++++++++
gdb/solib.h | 5 +++++
6 files changed, 138 insertions(+), 1 deletion(-)
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 10942e6af01..d1256a9e99b 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -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/linux-tdep.c b/gdb/linux-tdep.c
index e2cff83086a..7325ba67208 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
@@ -1170,6 +1171,23 @@ linux_read_core_file_mappings
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++)
@@ -1183,8 +1201,26 @@ linux_read_core_file_mappings
descdata += addr_size;
char * filename = filenames;
filenames += strlen ((char *) filenames) + 1;
+ const bfd_build_id *build_id = nullptr;
+ auto vma_map_it = vma_map.find (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 (vma_map_it != vma_map.end ())
+ {
+ build_id = vma_map_it->second;
+ filename_map[filename] = build_id;
+ }
+ else
+ {
+ auto filename_map_it = filename_map.find (filename);
+
+ if (filename_map_it != filename_map.end ())
+ build_id = filename_map_it->second;
+ }
- loop_cb (i, start, end, file_ofs, filename, nullptr);
+ loop_cb (i, start, end, file_ofs, filename, build_id);
}
}
diff --git a/gdb/progspace.c b/gdb/progspace.c
index 7080bf8ee27..680c5ba8f0a 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -18,6 +18,7 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include "defs.h"
+#include "build-id.h"
#include "gdbcmd.h"
#include "objfiles.h"
#include "arch-utils.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)
+{
+ m_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 = m_cbfd_soname_to_build_id.find (basename (soname));
+ if (it == m_cbfd_soname_to_build_id.end ())
+ return nullptr;
+
+ return it->second.c_str ();
+}
+
+/* See progspace.h. */
+
+void
+program_space::clear_cbfd_soname_build_ids ()
+{
+ m_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..01af5a387e3 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,10 @@ 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 shared library sonames to their
+ respective build-ids. */
+ std::unordered_map<std::string, std::string> m_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 3947c2d1d2e..c0beea19d7b 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"
@@ -1586,6 +1587,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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles
2021-11-17 3:24 ` Aaron Merey
@ 2021-11-17 14:17 ` Tom Tromey
2021-11-17 21:16 ` Aaron Merey
0 siblings, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2021-11-17 14:17 UTC (permalink / raw)
To: Aaron Merey via Gdb-patches; +Cc: simon.marchi, Aaron Merey, tom, lsix
>>>>> "Aaron" == Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> writes:
>> Can you explain why we are opening bfd->filename? Doesn't "abfd"
>> represent the exact same thing than "bfd"? So why can't we use "bfd"
>> directly to look up the soname?
Aaron> bfd->flags does not contain the correct value. This causes the check
Aaron> for ET_DYN to fail even for shared libraries. Also bfd->build_id always
Aaron> seems to be NULL. Opening "abfd" with gdb_bfd_open and passing it to
Aaron> bfd_check_format populates flags and build_id with the correct values.
Aaron> Passing "bfd" to bfd_check_format still does not populate these fields
Aaron> with the correct values.
Aaron> It looks like this happens because the format of "abfd" is initially
Aaron> set to bfd_unknown. In this case bfd_check_format rereads the file and
Aaron> updates some fields with the correct values, including flags and build_id.
Aaron> Since format of "bfd" is instead bfd_object, bfd_check_format returns
Aaron> early and skips updating any fields.
This seems strange to me.
At first I was surprised that re-opening the BFD like this wouldn't
simply return the same BFD, since gdb uses a BFD cache. However, now I
see that corelow isn't using the cache:
bfd = bfd_map[filename] = bfd_openr (expanded_fname.get (),
"binary");
(as opposed to using gdb_bfd_openr here.)
If re-opening and then calling bfd_check_format sets the flags
correctly, then I suppose the question is just why the call in
build_file_mappings isn't doing that:
if (bfd == nullptr || !bfd_check_format (bfd, bfd_object))
Possibly the answer is that corelow passes "binary" as the target, but
your code does:
Aaron> + gdb_bfd_ref_ptr abfd = gdb_bfd_open (bfd->filename, gnutarget);
But then I wonder why "binary" is appropriate in corelow.
Tom
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles
2021-11-17 14:17 ` Tom Tromey
@ 2021-11-17 21:16 ` Aaron Merey
2022-01-26 1:40 ` Aaron Merey
2022-02-17 16:01 ` Andrew Burgess
0 siblings, 2 replies; 17+ messages in thread
From: Aaron Merey @ 2021-11-17 21:16 UTC (permalink / raw)
To: Tom Tromey; +Cc: Aaron Merey via Gdb-patches, Simon Marchi, lsix
Hi Tom,
On Wed, Nov 17, 2021 at 9:29 AM Tom Tromey <tom@tromey.com> wrote:
> If re-opening and then calling bfd_check_format sets the flags
> correctly, then I suppose the question is just why the call in
> build_file_mappings isn't doing that:
>
> if (bfd == nullptr || !bfd_check_format (bfd, bfd_object))
>
> Possibly the answer is that corelow passes "binary" as the target, but
> your code does:
>
> Aaron> + gdb_bfd_ref_ptr abfd = gdb_bfd_open (bfd->filename, gnutarget);
>
> But then I wonder why "binary" is appropriate in corelow.
Yes it would be much better if "bfd" was simply initialised with the proper
fields. Corelow treats bfds in a target-agnostic manner but this shouldn't
preclude having accurate information in "bfd".
We could force "bfd" to have a format of bfd_unknown and let
bfd_check_format figure out the correct format and fields. This is
basically what I do in gdb_bfd_read_elf_soname. But really there
should be a way to just initialise "bfd" with completely accurate
contents at the time of creation. If this doesn't exist maybe now
is the time to add it.
Aaron
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles
2021-11-17 21:16 ` Aaron Merey
@ 2022-01-26 1:40 ` Aaron Merey
2022-02-09 2:31 ` Aaron Merey
2022-02-17 16:01 ` Andrew Burgess
1 sibling, 1 reply; 17+ messages in thread
From: Aaron Merey @ 2022-01-26 1:40 UTC (permalink / raw)
To: Tom Tromey; +Cc: Aaron Merey via Gdb-patches, Simon Marchi, lsix
Ping
Thanks,
Aaron
On Wed, Nov 17, 2021 at 4:16 PM Aaron Merey <amerey@redhat.com> wrote:
>
> Hi Tom,
>
> On Wed, Nov 17, 2021 at 9:29 AM Tom Tromey <tom@tromey.com> wrote:
> > If re-opening and then calling bfd_check_format sets the flags
> > correctly, then I suppose the question is just why the call in
> > build_file_mappings isn't doing that:
> >
> > if (bfd == nullptr || !bfd_check_format (bfd, bfd_object))
> >
> > Possibly the answer is that corelow passes "binary" as the target, but
> > your code does:
> >
> > Aaron> + gdb_bfd_ref_ptr abfd = gdb_bfd_open (bfd->filename, gnutarget);
> >
> > But then I wonder why "binary" is appropriate in corelow.
>
> Yes it would be much better if "bfd" was simply initialised with the proper
> fields. Corelow treats bfds in a target-agnostic manner but this shouldn't
> preclude having accurate information in "bfd".
>
> We could force "bfd" to have a format of bfd_unknown and let
> bfd_check_format figure out the correct format and fields. This is
> basically what I do in gdb_bfd_read_elf_soname. But really there
> should be a way to just initialise "bfd" with completely accurate
> contents at the time of creation. If this doesn't exist maybe now
> is the time to add it.
>
> Aaron
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles
2022-01-26 1:40 ` Aaron Merey
@ 2022-02-09 2:31 ` Aaron Merey
0 siblings, 0 replies; 17+ messages in thread
From: Aaron Merey @ 2022-02-09 2:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Simon Marchi, lsix
Ping ** 2
Thanks,
Aaron
On Tue, Jan 25, 2022 at 8:40 PM Aaron Merey <amerey@redhat.com> wrote:
>
> Ping
>
> Thanks,
> Aaron
>
> On Wed, Nov 17, 2021 at 4:16 PM Aaron Merey <amerey@redhat.com> wrote:
> >
> > Hi Tom,
> >
> > On Wed, Nov 17, 2021 at 9:29 AM Tom Tromey <tom@tromey.com> wrote:
> > > If re-opening and then calling bfd_check_format sets the flags
> > > correctly, then I suppose the question is just why the call in
> > > build_file_mappings isn't doing that:
> > >
> > > if (bfd == nullptr || !bfd_check_format (bfd, bfd_object))
> > >
> > > Possibly the answer is that corelow passes "binary" as the target, but
> > > your code does:
> > >
> > > Aaron> + gdb_bfd_ref_ptr abfd = gdb_bfd_open (bfd->filename, gnutarget);
> > >
> > > But then I wonder why "binary" is appropriate in corelow.
> >
> > Yes it would be much better if "bfd" was simply initialised with the proper
> > fields. Corelow treats bfds in a target-agnostic manner but this shouldn't
> > preclude having accurate information in "bfd".
> >
> > We could force "bfd" to have a format of bfd_unknown and let
> > bfd_check_format figure out the correct format and fields. This is
> > basically what I do in gdb_bfd_read_elf_soname. But really there
> > should be a way to just initialise "bfd" with completely accurate
> > contents at the time of creation. If this doesn't exist maybe now
> > is the time to add it.
> >
> > Aaron
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles
2021-11-17 21:16 ` Aaron Merey
2022-01-26 1:40 ` Aaron Merey
@ 2022-02-17 16:01 ` Andrew Burgess
1 sibling, 0 replies; 17+ messages in thread
From: Andrew Burgess @ 2022-02-17 16:01 UTC (permalink / raw)
To: Aaron Merey; +Cc: Tom Tromey, lsix, Aaron Merey via Gdb-patches
* Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> [2021-11-17 16:16:13 -0500]:
> Hi Tom,
>
> On Wed, Nov 17, 2021 at 9:29 AM Tom Tromey <tom@tromey.com> wrote:
> > If re-opening and then calling bfd_check_format sets the flags
> > correctly, then I suppose the question is just why the call in
> > build_file_mappings isn't doing that:
> >
> > if (bfd == nullptr || !bfd_check_format (bfd, bfd_object))
> >
> > Possibly the answer is that corelow passes "binary" as the target, but
> > your code does:
> >
> > Aaron> + gdb_bfd_ref_ptr abfd = gdb_bfd_open (bfd->filename, gnutarget);
> >
> > But then I wonder why "binary" is appropriate in corelow.
>
> Yes it would be much better if "bfd" was simply initialised with the proper
> fields. Corelow treats bfds in a target-agnostic manner but this shouldn't
> preclude having accurate information in "bfd".
>
> We could force "bfd" to have a format of bfd_unknown and let
> bfd_check_format figure out the correct format and fields. This is
> basically what I do in gdb_bfd_read_elf_soname. But really there
> should be a way to just initialise "bfd" with completely accurate
> contents at the time of creation. If this doesn't exist maybe now
> is the time to add it.
As I understand it, the code in corelow is making a deliberate choice
_not_ to initialise the bfd with any of the "correct" information.
We want something that can handle both structured files, like shared
libraries, but also unstructured, raw data files that are mapped into
memory.
Once we've opened the bfd in core_target::build_file_mappings, we then
add sections to the bfd to cover those regions that were mapped into
memory.
One option would be to modify core_target::build_file_mappings so that
we first try to open the bfd using `gnutarget` as the type, and only
if that fails (returns nullptr), try opening the bfd with type
binary. I think in that way you'd find the bfd had all the data you
needed.
The other option, which I might be inclined to go with, would be to
change the API for the new gdb_bfd_read_elf_soname function, make it
instead something like:
gdb::optional<std::string>
gdb_bfd_read_elf_soname (const char *filename)
{ ... }
Then you remove the whole ugly, pull the filename from the bfd and
reopen the bfd logic, and instead, you're just opening a file as a
bfd, and reading content from it. You already have the filename
available in core_target::build_file_mappings.
I have some minor style issues which I've highlighted below. But I've
only really had a good look at the one part of the patch I discuss
above, (around gdb_bfd_read_elf_soname). I can try to look at the
rest next week, if nobody else picks this up.
Thanks,
Andrew
> From 275e69ce65aa3d5ee3b017175c73f8536a1fb559 Mon Sep 17 00:00:00 2001
> From: Aaron Merey <amerey@redhat.com>
> Date: Tue, 16 Nov 2021 17:33:52 -0500
> Subject: [PATCH 2/3] gdb: Add soname to build-id mapping for core files
>
> 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
> soname to build-id 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/corelow.c | 11 +++++++++++
> gdb/linux-tdep.c | 38 +++++++++++++++++++++++++++++++++++++-
> gdb/progspace.c | 32 ++++++++++++++++++++++++++++++++
> gdb/progspace.h | 18 ++++++++++++++++++
> gdb/solib.c | 35 +++++++++++++++++++++++++++++++++++
> gdb/solib.h | 5 +++++
> 6 files changed, 138 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index 10942e6af01..d1256a9e99b 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -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)
if (soname.has_value ())
> + 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/linux-tdep.c b/gdb/linux-tdep.c
> index e2cff83086a..7325ba67208 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
> @@ -1170,6 +1171,23 @@ linux_read_core_file_mappings
> 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++)
> @@ -1183,8 +1201,26 @@ linux_read_core_file_mappings
> descdata += addr_size;
> char * filename = filenames;
> filenames += strlen ((char *) filenames) + 1;
> + const bfd_build_id *build_id = nullptr;
> + auto vma_map_it = vma_map.find (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. */
Two spaces after the '.'.
> + if (vma_map_it != vma_map.end ())
> + {
> + build_id = vma_map_it->second;
> + filename_map[filename] = build_id;
> + }
> + else
> + {
> + auto filename_map_it = filename_map.find (filename);
> +
> + if (filename_map_it != filename_map.end ())
> + build_id = filename_map_it->second;
> + }
>
> - loop_cb (i, start, end, file_ofs, filename, nullptr);
> + loop_cb (i, start, end, file_ofs, filename, build_id);
> }
> }
>
> diff --git a/gdb/progspace.c b/gdb/progspace.c
> index 7080bf8ee27..680c5ba8f0a 100644
> --- a/gdb/progspace.c
> +++ b/gdb/progspace.c
> @@ -18,6 +18,7 @@
> along with this program. If not, see <http://www.gnu.org/licenses/>. */
>
> #include "defs.h"
> +#include "build-id.h"
> #include "gdbcmd.h"
> #include "objfiles.h"
> #include "arch-utils.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)
> +{
> + m_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);
gdb_assert (soname != nullptr);
> +
> + auto it = m_cbfd_soname_to_build_id.find (basename (soname));
> + if (it == m_cbfd_soname_to_build_id.end ())
> + return nullptr;
> +
> + return it->second.c_str ();
> +}
> +
> +/* See progspace.h. */
> +
> +void
> +program_space::clear_cbfd_soname_build_ids ()
> +{
> + m_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..01af5a387e3 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,10 @@ 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 shared library sonames to their
> + respective build-ids. */
> + std::unordered_map<std::string, std::string> m_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 3947c2d1d2e..c0beea19d7b 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"
> @@ -1586,6 +1587,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);
Missing a space after the cast.
> +}
> +
> /* 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);
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings
2021-11-10 1:47 [PATCH v4 0/3] Add debuginfod core file support Aaron Merey
2021-11-10 1:47 ` [PATCH 1/3] gdb: Add aliases for read_core_file_mappings callbacks Aaron Merey
2021-11-10 1:47 ` [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles Aaron Merey
@ 2021-11-10 1:47 ` Aaron Merey
2021-11-14 2:56 ` Simon Marchi
2 siblings, 1 reply; 17+ messages in thread
From: Aaron Merey @ 2021-11-10 1:47 UTC (permalink / raw)
To: gdb-patches; +Cc: simon.marchi, tom, lsix
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings
2021-11-10 1:47 ` [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings Aaron Merey
@ 2021-11-14 2:56 ` Simon Marchi
2021-11-17 3:28 ` Aaron Merey
0 siblings, 1 reply; 17+ messages in thread
From: Simon Marchi @ 2021-11-14 2:56 UTC (permalink / raw)
To: Aaron Merey, gdb-patches; +Cc: tom, lsix
> @@ -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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings
2021-11-14 2:56 ` Simon Marchi
@ 2021-11-17 3:28 ` Aaron Merey
2022-01-26 1:42 ` Aaron Merey
0 siblings, 1 reply; 17+ messages in thread
From: Aaron Merey @ 2021-11-17 3:28 UTC (permalink / raw)
To: simon.marchi; +Cc: gdb-patches, tom, lsix
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings
2021-11-17 3:28 ` Aaron Merey
@ 2022-01-26 1:42 ` Aaron Merey
2022-02-09 2:31 ` Aaron Merey
0 siblings, 1 reply; 17+ messages in thread
From: Aaron Merey @ 2022-01-26 1:42 UTC (permalink / raw)
To: Aaron Merey; +Cc: Simon Marchi, Tom Tromey, lsix, gdb-patches
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings
2022-01-26 1:42 ` Aaron Merey
@ 2022-02-09 2:31 ` Aaron Merey
0 siblings, 0 replies; 17+ messages in thread
From: Aaron Merey @ 2022-02-09 2:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi, Tom Tromey, lsix
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
> >
^ permalink raw reply [flat|nested] 17+ messages in thread