From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 44CA8385DC23 for ; Thu, 19 Aug 2021 02:22:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 44CA8385DC23 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-331-3q_D1yapOtyrW3_U2vzDow-1; Wed, 18 Aug 2021 22:22:30 -0400 X-MC-Unique: 3q_D1yapOtyrW3_U2vzDow-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 71A2E1008060; Thu, 19 Aug 2021 02:22:29 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-112-21.phx2.redhat.com [10.3.112.21]) by smtp.corp.redhat.com (Postfix) with ESMTP id CDF02687D5; Thu, 19 Aug 2021 02:22:28 +0000 (UTC) From: Aaron Merey To: lsix@lancelotsix.com, simon.marchi@polymtl.ca Cc: gdb-patches@sourceware.org, tom@tromey.com Subject: Re: [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles Date: Wed, 18 Aug 2021 22:22:27 -0400 Message-Id: <20210819022227.62623-1-amerey@redhat.com> In-Reply-To: <6d314528-224a-3e5e-2d4b-070ea776823d@polymtl.ca> References: <6d314528-224a-3e5e-2d4b-070ea776823d@polymtl.ca> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-13.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Aug 2021 02:22:44 -0000 Hi Lancelot and Simon, Thanks for the reviews. The updated patch is included below. On Tue, Aug 17, 2021 at 9:59 AM Simon Marchi wrote: >>> + /* Check that bfd is an ET_DYN ELF file. */ >>> + bfd_check_format (abfd.get (), bfd_object); > > I asked this in my previous review, still applies here: > >   What's the point of calling bfd_check_format without checking the >   result?  It looks like a function without side-effects. I included a few comments in the [PATCH 0/3] email for this series[1]. I mentioned that for some reason bfd_check_format appears to update the bfd 'flags' field with the correct value. If bfd_check_format is not called here the check for the DYNAMIC flag fails even for ET_DYN files. [1] https://sourceware.org/pipermail/gdb-patches/2021-August/181416.html --- gdb/arch-utils.c | 15 +++++---------- gdb/arch-utils.h | 23 +++++++++++++---------- gdb/build-id.h | 2 ++ gdb/corelow.c | 13 ++++++++++++- gdb/gdbarch.c | 2 +- gdb/gdbarch.h | 4 ++-- gdb/gdbarch.sh | 2 +- gdb/linux-tdep.c | 46 ++++++++++++++++++++++++++++++++++------------ gdb/progspace.c | 32 ++++++++++++++++++++++++++++++++ gdb/progspace.h | 17 +++++++++++++++++ gdb/solib.c | 35 +++++++++++++++++++++++++++++++++++ gdb/solib.h | 5 +++++ 12 files changed, 159 insertions(+), 37 deletions(-) diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c index 862f26b6cf7..ffb32cb203f 100644 --- a/gdb/arch-utils.c +++ b/gdb/arch-utils.c @@ -1075,16 +1075,11 @@ default_get_pc_address_flags (frame_info *frame, CORE_ADDR pc) /* See arch-utils.h. */ void -default_read_core_file_mappings (struct gdbarch *gdbarch, - struct bfd *cbfd, - gdb::function_view - pre_loop_cb, - gdb::function_view - loop_cb) +default_read_core_file_mappings + (struct gdbarch *gdbarch, + struct bfd *cbfd, + gdb::function_view pre_loop_cb, + loop_cb_ftype loop_cb) { } diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h index 03e9082f6d7..2243c3fb85b 100644 --- a/gdb/arch-utils.h +++ b/gdb/arch-utils.h @@ -294,15 +294,18 @@ extern ULONGEST default_type_align (struct gdbarch *gdbarch, extern std::string default_get_pc_address_flags (frame_info *frame, CORE_ADDR pc); + +using loop_cb_ftype = gdb::function_view; + /* 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 - pre_loop_cb, - gdb::function_view - loop_cb); +extern void default_read_core_file_mappings + (struct gdbarch *gdbarch, + struct bfd *cbfd, + gdb::function_view pre_loop_cb, + loop_cb_ftype loop_cb); #endif /* ARCH_UTILS_H */ diff --git a/gdb/build-id.h b/gdb/build-id.h index 42f8d57ede1..3c9402ee71b 100644 --- a/gdb/build-id.h +++ b/gdb/build-id.h @@ -20,8 +20,10 @@ #ifndef BUILD_ID_H #define BUILD_ID_H +#include "defs.h" #include "gdb_bfd.h" #include "gdbsupport/rsp-low.h" +#include /* Locate NT_GNU_BUILD_ID from ABFD and return its content. */ diff --git a/gdb/corelow.c b/gdb/corelow.c index eb785a08633..31af0f22584 100644 --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -214,7 +214,7 @@ core_target::build_file_mappings () /* read_core_file_mappings will invoke this lambda for each mapping that it finds. */ [&] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, - const char *filename) + const char *filename, const bfd_build_id *build_id) { /* Architecture-specific read_core_mapping methods are expected to weed out non-file-backed mappings. */ @@ -282,6 +282,16 @@ core_target::build_file_mappings () /* Set target_section fields. */ m_core_file_mappings.emplace_back (start, end, sec); + + /* If this is a bfd of a shared library, record its soname + and build id. */ + if (build_id != nullptr) + { + gdb::optional soname = gdb_bfd_read_elf_soname (bfd); + if (soname) + current_program_space->set_cbfd_soname_build_id + (std::move (*soname), build_id); + } }); normalize_mem_ranges (&m_core_unavailable_mappings); @@ -305,6 +315,7 @@ core_target::close () comments in clear_solib in solib.c. */ clear_solib (); + current_program_space->clear_cbfd_soname_build_ids (); current_program_space->cbfd.reset (nullptr); } diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index f89dcc57754..35464115039 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -5411,7 +5411,7 @@ set_gdbarch_get_pc_address_flags (struct gdbarch *gdbarch, } void -gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view pre_loop_cb, gdb::function_view loop_cb) +gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view pre_loop_cb, gdb::function_view loop_cb) { gdb_assert (gdbarch != NULL); gdb_assert (gdbarch->read_core_file_mappings != NULL); diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h index 979159ba2f5..baf80580d60 100644 --- a/gdb/gdbarch.h +++ b/gdb/gdbarch.h @@ -1710,8 +1710,8 @@ extern void set_gdbarch_get_pc_address_flags (struct gdbarch *gdbarch, gdbarch_g /* Read core file mappings */ -typedef void (gdbarch_read_core_file_mappings_ftype) (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view pre_loop_cb, gdb::function_view loop_cb); -extern void gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view pre_loop_cb, gdb::function_view loop_cb); +typedef void (gdbarch_read_core_file_mappings_ftype) (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view pre_loop_cb, gdb::function_view loop_cb); +extern void gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, gdb::function_view pre_loop_cb, gdb::function_view loop_cb); extern void set_gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, gdbarch_read_core_file_mappings_ftype *read_core_file_mappings); extern struct gdbarch_tdep *gdbarch_tdep (struct gdbarch *gdbarch); diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh index 39a99d0d5f3..c24079d34f3 100755 --- a/gdb/gdbarch.sh +++ b/gdb/gdbarch.sh @@ -1210,7 +1210,7 @@ m;ULONGEST;type_align;struct type *type;type;;default_type_align;;0 f;std::string;get_pc_address_flags;frame_info *frame, CORE_ADDR pc;frame, pc;;default_get_pc_address_flags;;0 # Read core file mappings -m;void;read_core_file_mappings;struct bfd *cbfd, gdb::function_view pre_loop_cb, gdb::function_view loop_cb;cbfd, pre_loop_cb, loop_cb;;default_read_core_file_mappings;;0 +m;void;read_core_file_mappings;struct bfd *cbfd, gdb::function_view pre_loop_cb, gdb::function_view loop_cb;cbfd, pre_loop_cb, loop_cb;;default_read_core_file_mappings;;0 EOF } diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c index ae2f7c14f6d..812ca1f99bf 100644 --- a/gdb/linux-tdep.c +++ b/gdb/linux-tdep.c @@ -44,6 +44,7 @@ #include "solib-svr4.h" #include +#include /* This enum represents the values that the user can choose when informing the Linux kernel about which memory mappings will be @@ -1097,16 +1098,11 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args, for each mapping. */ static void -linux_read_core_file_mappings (struct gdbarch *gdbarch, - struct bfd *cbfd, - gdb::function_view - pre_loop_cb, - gdb::function_view - loop_cb) +linux_read_core_file_mappings + (struct gdbarch *gdbarch, + struct bfd *cbfd, + gdb::function_view pre_loop_cb, + loop_cb_ftype loop_cb) { /* Ensure that ULONGEST is big enough for reading 64-bit core files. */ gdb_static_assert (sizeof (ULONGEST) >= 8); @@ -1175,6 +1171,23 @@ linux_read_core_file_mappings (struct gdbarch *gdbarch, if (f != descend) warning (_("malformed note - filename area is too big")); + const bfd_build_id *orig_build_id = cbfd->build_id; + std::unordered_map vma_map; + std::unordered_map filename_map; + + /* Search for solib build-ids in the core file. Each time one is found, + map the start vma of the corresponding elf header to the build-id. */ + for (bfd_section *sec = cbfd->sections; sec != nullptr; sec = sec->next) + { + cbfd->build_id = nullptr; + + if (sec->flags & SEC_LOAD + && get_elf_backend_data (cbfd)->elf_backend_core_find_build_id + (cbfd, (bfd_vma) sec->filepos)) + vma_map[sec->vma] = cbfd->build_id; + } + + cbfd->build_id = orig_build_id; pre_loop_cb (count); for (int i = 0; i < count; i++) @@ -1188,8 +1201,17 @@ linux_read_core_file_mappings (struct gdbarch *gdbarch, descdata += addr_size; char * filename = filenames; filenames += strlen ((char *) filenames) + 1; + const bfd_build_id *build_id = vma_map[start]; + + /* Map filename to the build-id associated with this start vma, + if such a build-id was found. Otherwise use the build-id + already associated with this filename if it exists. */ + if (build_id != nullptr) + filename_map[filename] = build_id; + else + build_id = filename_map[filename]; - loop_cb (i, start, end, file_ofs, filename); + loop_cb (i, start, end, file_ofs, filename, build_id); } } @@ -1218,7 +1240,7 @@ linux_core_info_proc_mappings (struct gdbarch *gdbarch, const char *args) } }, [=] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, - const char *filename) + const char *filename, const bfd_build_id *build_id) { if (gdbarch_addr_bit (gdbarch) == 32) printf_filtered ("\t%10s %10s %10s %10s %s\n", diff --git a/gdb/progspace.c b/gdb/progspace.c index 7080bf8ee27..8b7b949d959 100644 --- a/gdb/progspace.c +++ b/gdb/progspace.c @@ -17,6 +17,7 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . */ +#include "build-id.h" #include "defs.h" #include "gdbcmd.h" #include "objfiles.h" @@ -358,6 +359,37 @@ print_program_space (struct ui_out *uiout, int requested) } } +/* See progspace.h. */ + +void +program_space::set_cbfd_soname_build_id (std::string soname, + const bfd_build_id *build_id) +{ + cbfd_soname_to_build_id[std::move (soname)] = build_id_to_string (build_id); +} + +/* See progspace.h. */ + +const char * +program_space::get_cbfd_soname_build_id (const char *soname) +{ + gdb_assert (soname); + + auto it = cbfd_soname_to_build_id.find (basename (soname)); + if (it == cbfd_soname_to_build_id.end ()) + return nullptr; + + return it->second.c_str (); +} + +/* See progspace.h. */ + +void +program_space::clear_cbfd_soname_build_ids () +{ + cbfd_soname_to_build_id.clear (); +} + /* Boolean test for an already-known program space id. */ static int diff --git a/gdb/progspace.h b/gdb/progspace.h index fb348ca7539..b42b3ffc4f1 100644 --- a/gdb/progspace.h +++ b/gdb/progspace.h @@ -30,6 +30,7 @@ #include "gdbsupport/safe-iterator.h" #include #include +#include struct target_ops; struct bfd; @@ -324,6 +325,19 @@ struct program_space /* Binary file diddling handle for the core file. */ gdb_bfd_ref_ptr cbfd; + /* Associate a core file SONAME with BUILD_ID so that it can be retrieved + with get_cbfd_soname_build_id. */ + void set_cbfd_soname_build_id (std::string soname, + const bfd_build_id *build_id); + + /* If a core file SONAME had a build-id associated with it by a previous + call to set_cbfd_soname_build_id then return the build-id as a + NULL-terminated hex string. */ + const char *get_cbfd_soname_build_id (const char *soname); + + /* Clear all core file soname to build-id mappings. */ + void clear_cbfd_soname_build_ids (); + /* The address space attached to this program space. More than one program space may be bound to the same address space. In the traditional unix-like debugging scenario, this will usually @@ -378,6 +392,9 @@ struct program_space /* The set of target sections matching the sections mapped into this program space. Managed by both exec_ops and solib.c. */ target_section_table m_target_sections; + + /* Mapping of a core file's library sonames to their respective build-ids. */ + std::unordered_map cbfd_soname_to_build_id; }; /* An address space. It is used for comparing if diff --git a/gdb/solib.c b/gdb/solib.c index e30affbb7e7..1b99c8ab985 100644 --- a/gdb/solib.c +++ b/gdb/solib.c @@ -23,6 +23,7 @@ #include #include "symtab.h" #include "bfd.h" +#include "build-id.h" #include "symfile.h" #include "objfiles.h" #include "gdbcore.h" @@ -1585,6 +1586,40 @@ gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd, CORE_ADDR *ptr, return 0; } +/* See solib.h. */ + +gdb::optional +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 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