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 ESMTPS id 785523858C27 for ; Mon, 18 Oct 2021 23:06:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 785523858C27 Received: from mail-vk1-f197.google.com (mail-vk1-f197.google.com [209.85.221.197]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-467-0kgpr81yPuCQSVgYzTOnPg-1; Mon, 18 Oct 2021 19:06:46 -0400 X-MC-Unique: 0kgpr81yPuCQSVgYzTOnPg-1 Received: by mail-vk1-f197.google.com with SMTP id m187-20020a1f3fc4000000b002a2f4c18545so6413826vka.13 for ; Mon, 18 Oct 2021 16:06:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=Bo+3NHTo34yOoGago8WHv94Jjo6pHyVBD+LeXGaOFSk=; b=Aesn8bWF+PnhWoy8Yba7tMPG4+uFkD2I9FTP/9wvN7Osc3HdU6cbMbrJuF3pxo2XKF zYN+NxR48wA+QoPtkHRLw2oWTS6w9W2u7PZd49ePEVsHaeohT5egshLmRGDwtcc47JkN c/15t3h13yb/KYRAvAtaTxvZdIR2jPmc9djR/+csredqEUQvRbNp+JrQddwwocJjG03U TQLucjib29+JPw/RGqtDmwW0K40cqCae5uzMcZaAJr1fgoOa/lsTsqjUc4jNHEnmu2JA 9hC37IMBJU7A2F14nh7X7bVj8W/ZX8fbLurIKYIgVZz5yNYHI9bniTOmMACmmxiJMDTe f3Tw== X-Gm-Message-State: AOAM5325vdpUrOzptn0gY/JbjP4ZAfjut6SJ0nDGVzuFJDoeqEcb+aiM hwDFujTvs86xLT41kt7DohE433r4mFCaJ7U9K+IbXKYZlrs5Y5VtnWXChV6cxbGp0H6KGqTrvcg NtMoYozHZAR4liw9ZNyNgk3E6TAk5C4b6N1rB X-Received: by 2002:a05:6102:2913:: with SMTP id cz19mr10131603vsb.36.1634598406132; Mon, 18 Oct 2021 16:06:46 -0700 (PDT) X-Google-Smtp-Source: ABdhPJza1Gim6uAm6wpDRP0/NsWAtDsopzb90CiDPzp8SrFb4gZWVQMTJr+gtNqapCMOO/+oO1H5A+MaXADa807hPaw= X-Received: by 2002:a05:6102:2913:: with SMTP id cz19mr10131569vsb.36.1634598405665; Mon, 18 Oct 2021 16:06:45 -0700 (PDT) MIME-Version: 1.0 References: <6d314528-224a-3e5e-2d4b-070ea776823d@polymtl.ca> <20210819022227.62623-1-amerey@redhat.com> In-Reply-To: From: Aaron Merey Date: Mon, 18 Oct 2021 19:06:35 -0400 Message-ID: Subject: [PING**2][PATCH 2/3] gdb: Add soname to build-id mapping for corefiles To: Aaron Merey Cc: lsix@lancelotsix.com, Simon Marchi , Tom Tromey , gdb-patches@sourceware.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-12.2 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: Mon, 18 Oct 2021 23:06:52 -0000 Ping Thanks, Aaron On Tue, Sep 28, 2021 at 9:12 PM Aaron Merey wrote: > > Ping. > > Thanks, > Aaron > > On Wed, Aug 18, 2021 at 10:22 PM Aaron Merey via Gdb-patches > wrote: > > > > Hi Lancelot and Simon, > > > > Thanks for the reviews. The updated patch is included below. > > > > On Tue, Aug 17, 2021 at 9:59 AM Simon Marchi = 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 no= t > > called here the check for the DYNAMIC flag fails even for ET_DYN files. > > > > [1] https://sourceware.org/pipermail/gdb-patches/2021-August/181416.htm= l > > > > --- > > 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 > - ULONGEST star= t, > > - ULONGEST end, > > - ULONGEST file= _ofs, > > - const char *f= ilename)> > > - 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 gdbarc= h *gdbarch, > > extern std::string default_get_pc_address_flags (frame_info *frame, > > CORE_ADDR pc); > > > > + > > +using loop_cb_ftype =3D gdb::function_view > + ULONGEST start, > > + ULONGEST end, > > + ULONGEST file_ofs, > > + const char *filename, > > + const bfd_build_id *buil= d_id)>; > > + > > /* Default implementation of gdbarch read_core_file_mappings method. = */ > > -extern void default_read_core_file_mappings (struct gdbarch *gdbarch, > > - struct bfd *cbfd, > > - gdb::function_view > > - pre_loop_cb, > > - gdb::function_view > - U= LONGEST start, > > - U= LONGEST end, > > - U= LONGEST file_ofs, > > - c= onst char *filename)> > > - 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 mappin= g > > 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 !=3D nullptr) > > + { > > + gdb::optional soname =3D gdb_bfd_read_elf_sona= me (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 !=3D NULL); > > gdb_assert (gdbarch->read_core_file_mappings !=3D 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 (str= uct 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_lo= op_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, gd= b::function_view loop_cb); > > +typedef void (gdbarch_read_core_file_mappings_ftype) (struct gdbarch *= gdbarch, struct bfd *cbfd, gdb::function_view pre_lo= op_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, gd= b::function_view loop_cb); > > extern void set_gdbarch_read_core_file_mappings (struct gdbarch *gdbar= ch, 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;;def= ault_type_align;;0 > > f;std::string;get_pc_address_flags;frame_info *frame, CORE_ADDR pc;fra= me, 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;cb= fd, 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_f= ile_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 > - ULONGEST start, > > - ULONGEST end, > > - ULONGEST file_o= fs, > > - const char *fil= ename)> > > - 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) >=3D 8); > > @@ -1175,6 +1171,23 @@ linux_read_core_file_mappings (struct gdbarch *g= dbarch, > > if (f !=3D descend) > > warning (_("malformed note - filename area is too big")); > > > > + const bfd_build_id *orig_build_id =3D 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 fo= und, > > + map the start vma of the corresponding elf header to the build-id= . */ > > + for (bfd_section *sec =3D cbfd->sections; sec !=3D nullptr; sec =3D = sec->next) > > + { > > + cbfd->build_id =3D nullptr; > > + > > + if (sec->flags & SEC_LOAD > > + && get_elf_backend_data (cbfd)->elf_backend_core_find_build_i= d > > + (cbfd, (bfd_vma) sec->filepos)) > > + vma_map[sec->vma] =3D cbfd->build_id; > > + } > > + > > + cbfd->build_id =3D orig_build_id; > > pre_loop_cb (count); > > > > for (int i =3D 0; i < count; i++) > > @@ -1188,8 +1201,17 @@ linux_read_core_file_mappings (struct gdbarch *g= dbarch, > > descdata +=3D addr_size; > > char * filename =3D filenames; > > filenames +=3D strlen ((char *) filenames) + 1; > > + const bfd_build_id *build_id =3D 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 !=3D nullptr) > > + filename_map[filename] =3D build_id; > > + else > > + build_id =3D 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 *gd= barch, const char *args) > > } > > }, > > [=3D] (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) =3D=3D 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 req= uested) > > } > > } > > > > +/* 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)] =3D 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 =3D cbfd_soname_to_build_id.find (basename (soname)); > > + if (it =3D=3D 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 retr= ieved > > + 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 prev= ious > > + 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 buil= d-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_dynta= g, bfd *abfd, CORE_ADDR *ptr, > > return 0; > > } > > > > +/* See solib.h. */ > > + > > +gdb::optional > > +gdb_bfd_read_elf_soname (struct bfd *bfd) > > +{ > > + gdb_assert (bfd !=3D nullptr); > > + > > + gdb_bfd_ref_ptr abfd =3D gdb_bfd_open (bfd->filename, gnutarget); > > + > > + if (abfd =3D=3D 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 =3D bfd_get_section_by_name (abfd.get (),= ".dynstr"); > > + if (dynstr =3D=3D nullptr || bfd_section_size (dynstr) <=3D 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 t= o 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 *abf= d, > > 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 > >