From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 0B9673858C3A for ; Thu, 4 Nov 2021 01:33:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0B9673858C3A Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 1A41W1qw008672 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 3 Nov 2021 21:32:07 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 1A41W1qw008672 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 8C0761E940; Wed, 3 Nov 2021 21:32:01 -0400 (EDT) Message-ID: <981d6c48-9464-16bd-a2e1-d56476ebb267@polymtl.ca> Date: Wed, 3 Nov 2021 21:32:01 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.1 Subject: Re: [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles Content-Language: en-US To: Aaron Merey , lsix@lancelotsix.com Cc: gdb-patches@sourceware.org, tom@tromey.com References: <6d314528-224a-3e5e-2d4b-070ea776823d@polymtl.ca> <20210819022227.62623-1-amerey@redhat.com> From: Simon Marchi In-Reply-To: <20210819022227.62623-1-amerey@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Thu, 4 Nov 2021 01:32:01 +0000 X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, 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, 04 Nov 2021 01:33:13 -0000 > 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 start, > - ULONGEST end, > - ULONGEST file_ofs, > - const char *filename)> > - loop_cb) > +default_read_core_file_mappings > + (struct gdbarch *gdbarch, > + struct bfd *cbfd, > + gdb::function_view pre_loop_cb, > + loop_cb_ftype loop_cb) > { > } Two spaces before the parenthesis: void 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 + ULONGEST start, > + ULONGEST end, > + ULONGEST file_ofs, > + const char *filename, > + const bfd_build_id *build_id)>; I think this could use a better name, especially since it's in a header visible throughout GDB. read_core_file_mappings_loop_ftype, maybe, to follow what we use often (ftype meaning function type). And while at it, let's add read_core_file_mappings_pre_loop_ftype, while at it. I wouldn't mind if adding the named types for the callbacks was done in its own preparatory patch, it should be fairly obvious. > 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 Don't include defs.h here, see: https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Include_Files > 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 Replace the long gdb::function_view types here with the new named types, it will help readability. > @@ -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]; Here, use the find method of unordered_map. Using [] will cause an insertion if the entry does not exist, which is unnecessary work. > 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" Move the build-id.h include below, at least below defs.h, which must be the first one. > @@ -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; Prefix this member with `m_`. Simon