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 [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 324613858D3C for ; Fri, 11 Mar 2022 21:44:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 324613858D3C Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-333-kaTZNsLbPjWVkCundeMyhQ-1; Fri, 11 Mar 2022 16:44:52 -0500 X-MC-Unique: kaTZNsLbPjWVkCundeMyhQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9329A520E; Fri, 11 Mar 2022 21:44:51 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.22.34.124]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8A2CC794A1; Fri, 11 Mar 2022 21:44:46 +0000 (UTC) From: Aaron Merey To: tom@tromey.com Cc: gdb-patches@sourceware.org, aburgess@redhat.com, lsix@lancelot.com, Aaron Merey Subject: Re: [PATCH 1/2] gdb: Add soname to build-id mapping for core files Date: Fri, 11 Mar 2022 16:44:38 -0500 Message-Id: <20220311214438.120149-1-amerey@redhat.com> In-Reply-To: <877d90ms1o.fsf@tromey.com> References: <877d90ms1o.fsf@tromey.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE 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: Fri, 11 Mar 2022 21:44:58 -0000 On Fri, Mar 11, 2022 at 10:23 AM Tom Tromey wrote: > > + if (soname.get () != nullptr) > > I don't think you need the .get here. Fixed. > > + std::unordered_map 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 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 +#include /* 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 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 #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 soname_build_id_map; + +/* Key used to associate a soname_build_id_map to a core file bfd. */ + +static const struct bfd_key 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 +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 +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 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 get_cbfd_soname_build_id + (gdb_bfd_ref_ptr abfd, const char *soname); + #endif /* SOLIB_H */ -- 2.35.1