From: Aaron Merey <amerey@redhat.com>
To: tom@tromey.com
Cc: gdb-patches@sourceware.org, aburgess@redhat.com,
lsix@lancelot.com, Aaron Merey <amerey@redhat.com>
Subject: Re: [PATCH 1/2] gdb: Add soname to build-id mapping for core files
Date: Fri, 11 Mar 2022 16:44:38 -0500 [thread overview]
Message-ID: <20220311214438.120149-1-amerey@redhat.com> (raw)
In-Reply-To: <877d90ms1o.fsf@tromey.com>
On Fri, Mar 11, 2022 at 10:23 AM Tom Tromey <tom@tromey.com> wrote:
> > + if (soname.get () != nullptr)
>
> I don't think you need the .get here.
Fixed.
> > + std::unordered_map<char *, const bfd_build_id *> filename_map;
>
> I suspect this code is intended to use the contents of the string as the
> key -- but this will actually use the pointer value as the key.
>
> If the pointer value is intended, some comment somewhere to that effect
> -- explaining why it's an ok approach -- would be good to have.
After taking another look at this I don't think filename_map is needed
at all. The idea behind it was to use build-ids to reaquire downloaded
files in core_target::build_file_mappings from the debuginfod cache.
However this is unnecessary since bfd_map in build_file_mappings already
makes previously opened files (including those downloaded) available
using just a filename.
A build-id is only needed when trying to aquire the file for the first
time. This is facilitated by vma_map in linux_read_core_file_mappings.
> > + if (sec->flags & SEC_LOAD
> > + && get_elf_backend_data (cbfd)->elf_backend_core_find_build_id
> > + (cbfd, (bfd_vma) sec->filepos))
>
> The get_elf_backend_data call has to have parens around it according to
> GNU / gdb style.
Fixed.
> > + /* 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 make_unique_xstrdup ((char *) dynstr_buf.data () + idx);
>
> What if the data isn't \0-terminated? Maybe some size checking (not
> sure if needed) and make_unique_xstrndup would be safer.
Added a check to make sure \0 exists between the start of the soname
and the end of the section.
I also replaced basename with lbasename in get_cbfd_soname_build_id.
Aaron
---
gdb/corelow.c | 11 ++++++
gdb/linux-tdep.c | 24 ++++++++++++-
gdb/solib.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++
gdb/solib.h | 20 +++++++++++
4 files changed, 141 insertions(+), 1 deletion(-)
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 1579e6bc2b8..9876f4d920e 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -282,6 +282,17 @@ 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::unique_xmalloc_ptr<char> soname
+ = gdb_bfd_read_elf_soname (bfd->filename);
+ if (soname != nullptr)
+ set_cbfd_soname_build_id (current_program_space->cbfd,
+ soname.get (), build_id);
+ }
});
normalize_mem_ranges (&m_core_unavailable_mappings);
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index d4868902ac3..52cdaae034b 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
@@ -1185,6 +1186,22 @@ 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;
+
+ /* 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++)
@@ -1198,8 +1215,13 @@ 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);
+
+ if (vma_map_it != vma_map.end ())
+ build_id = vma_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/solib.c b/gdb/solib.c
index 8bcbfa22df1..8ecea0f89e7 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"
@@ -519,6 +520,55 @@ solib_bfd_open (const char *pathname)
return abfd;
}
+/* Mapping of a core file's shared library sonames to their respective
+ build-ids. Added to the registries of core file bfds. */
+
+typedef std::unordered_map<std::string, std::string> soname_build_id_map;
+
+/* Key used to associate a soname_build_id_map to a core file bfd. */
+
+static const struct bfd_key<soname_build_id_map> cbfd_soname_build_id_data_key;
+
+/* See solib.h. */
+
+void
+set_cbfd_soname_build_id (gdb_bfd_ref_ptr abfd,
+ const char *soname,
+ const bfd_build_id *build_id)
+{
+ gdb_assert (abfd.get () != nullptr);
+ gdb_assert (soname != nullptr);
+ gdb_assert (build_id != nullptr);
+
+ soname_build_id_map *mapptr = cbfd_soname_build_id_data_key.get (abfd.get ());
+
+ if (mapptr == nullptr)
+ mapptr = cbfd_soname_build_id_data_key.emplace (abfd.get ());
+
+ (*mapptr)[soname] = build_id_to_string (build_id);
+}
+
+/* See solib.h. */
+
+gdb::unique_xmalloc_ptr<char>
+get_cbfd_soname_build_id (gdb_bfd_ref_ptr abfd, const char *soname)
+{
+ if (abfd.get () == nullptr || soname == nullptr)
+ return {};
+
+ soname_build_id_map *mapptr
+ = cbfd_soname_build_id_data_key.get (abfd.get ());
+
+ if (mapptr == nullptr)
+ return {};
+
+ auto it = mapptr->find (lbasename (soname));
+ if (it == mapptr->end ())
+ return {};
+
+ return make_unique_xstrdup (it->second.c_str ());
+}
+
/* Given a pointer to one of the shared objects in our list of mapped
objects, use the recorded name to open a bfd descriptor for the
object, build a section table, relocate all the section addresses
@@ -1586,6 +1636,43 @@ gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd, CORE_ADDR *ptr,
return 0;
}
+/* See solib.h. */
+
+gdb::unique_xmalloc_ptr<char>
+gdb_bfd_read_elf_soname (const char *filename)
+{
+ gdb_bfd_ref_ptr abfd = gdb_bfd_open (filename, gnutarget);
+
+ if (abfd == nullptr)
+ return {};
+
+ /* Check that ABFD is an ET_DYN ELF file. */
+ if (!bfd_check_format (abfd.get (), bfd_object)
+ || !(bfd_get_file_flags (abfd.get ()) & DYNAMIC))
+ return {};
+
+ 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");
+ int sect_size = bfd_section_size (dynstr);
+ if (dynstr == nullptr || sect_size <= idx)
+ return {};
+
+ /* Read soname from the string table. */
+ gdb::byte_vector dynstr_buf;
+ if (!gdb_bfd_get_full_section_contents (abfd.get (), dynstr, &dynstr_buf))
+ return {};
+
+ /* Ensure soname is null-terminated before returning a copy. */
+ char *soname = (char *) dynstr_buf.data () + idx;
+ if (strnlen (soname, sect_size - idx) == sect_size - idx)
+ return {};
+
+ return make_unique_xstrdup (soname);
+}
+
/* 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 2258b0ba6a0..cd6c8a81104 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -118,6 +118,12 @@ 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 FILENAME refers to an ELF shared object then attempt to return the
+ string referred to by its DT_SONAME tag. */
+
+extern gdb::unique_xmalloc_ptr<char> gdb_bfd_read_elf_soname
+ (const char *filename);
+
/* Enable or disable optional solib event breakpoints as appropriate. */
extern void update_solib_breakpoints (void);
@@ -126,4 +132,18 @@ extern void update_solib_breakpoints (void);
extern void handle_solib_event (void);
+/* Associate SONAME with BUILD_ID in ABFD's registry so that it can be
+ retrieved with get_cbfd_soname_build_id. */
+
+extern void set_cbfd_soname_build_id (gdb_bfd_ref_ptr abfd,
+ const char *soname,
+ const bfd_build_id *build_id);
+
+/* If SONAME had a build-id associated with it in ABFD's registry by a
+ previous call to set_cbfd_soname_build_id then return the build-id
+ as a NULL-terminated hex string. */
+
+extern gdb::unique_xmalloc_ptr<char> get_cbfd_soname_build_id
+ (gdb_bfd_ref_ptr abfd, const char *soname);
+
#endif /* SOLIB_H */
--
2.35.1
next prev parent reply other threads:[~2022-03-11 21:44 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-03 1:34 [PATCH 0/2] Add debuginfod core file support Aaron Merey
2022-03-03 1:35 ` [PATCH 1/2] gdb: Add soname to build-id mapping for core files Aaron Merey
2022-03-04 14:53 ` Tom Tromey
2022-03-08 0:30 ` Aaron Merey
2022-03-11 15:23 ` Tom Tromey
2022-03-11 21:44 ` Aaron Merey [this message]
2022-03-18 19:03 ` Tom Tromey
2022-03-21 18:21 ` Aaron Merey
2022-03-03 1:35 ` [PATCH 2/2] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings Aaron Merey
2022-03-04 15:20 ` Tom Tromey
2022-03-08 0:33 ` Aaron Merey
2022-03-11 15:27 ` Tom Tromey
2022-03-11 21:49 ` Aaron Merey
2022-03-21 18:22 ` Aaron Merey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220311214438.120149-1-amerey@redhat.com \
--to=amerey@redhat.com \
--cc=aburgess@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=lsix@lancelot.com \
--cc=tom@tromey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).