From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 88050 invoked by alias); 10 Apr 2019 02:01:41 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 88038 invoked by uid 89); 10 Apr 2019 02:01:41 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.3 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 10 Apr 2019 02:01:39 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 361231E471; Tue, 9 Apr 2019 22:01:37 -0400 (EDT) Subject: Re: [PATCH 3/3] Introduce a separate debug objfile iterator To: Tom Tromey , gdb-patches@sourceware.org References: <20190409180945.21621-1-tom@tromey.com> <20190409180945.21621-4-tom@tromey.com> From: Simon Marchi Message-ID: Date: Wed, 10 Apr 2019 02:01:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.3 MIME-Version: 1.0 In-Reply-To: <20190409180945.21621-4-tom@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-04/txt/msg00172.txt.bz2 On 2019-04-09 2:09 p.m., Tom Tromey wrote: > This introduces a new iterator and range adapter for iteration over > the separate debug files of a given objfile. As in the current > approach, the requested objfile is returned first, followed by the > separate debug objfiles. > > gdb/ChangeLog > 2019-04-09 Tom Tromey > > * symtab.c (lookup_global_symbol_from_objfile) > (lookup_symbol_in_objfile_from_linkage_name): Use the iterator. > * objfiles.h (class separate_debug_iterator): New. > (class separate_debug_range): New. > (struct objfile) : New method. > (objfile_separate_debug_iterate): Don't declare. > * objfiles.c (separate_debug_iterator::operator++): Rename from > objfile_separate_debug_iterate. > (objfile_relocate, objfile_rebase, objfile_has_symbols): Use the > iterator. > * minsyms.c (lookup_minimal_symbol_by_pc_section): Use the > iterator. > --- > gdb/ChangeLog | 15 +++++++++++ > gdb/minsyms.c | 5 +--- > gdb/objfiles.c | 67 ++++++++++++++++++++++++------------------------- > gdb/objfiles.h | 68 +++++++++++++++++++++++++++++++++++++++++++++++--- > gdb/symtab.c | 12 +++------ > 5 files changed, 117 insertions(+), 50 deletions(-) > > diff --git a/gdb/minsyms.c b/gdb/minsyms.c > index 34198d122dc..8037329a862 100644 > --- a/gdb/minsyms.c > +++ b/gdb/minsyms.c > @@ -696,7 +696,6 @@ lookup_minimal_symbol_by_pc_section (CORE_ADDR pc_in, struct obj_section *sectio > int lo; > int hi; > int newobj; > - struct objfile *objfile; > struct minimal_symbol *msymbol; > struct minimal_symbol *best_symbol = NULL; > struct objfile *best_objfile = NULL; > @@ -722,9 +721,7 @@ lookup_minimal_symbol_by_pc_section (CORE_ADDR pc_in, struct obj_section *sectio > > gdb_assert (section != NULL); > > - for (objfile = section->objfile; > - objfile != NULL; > - objfile = objfile_separate_debug_iterate (section->objfile, objfile)) > + for (struct objfile *objfile : section->objfile->separate_debug_objfiles ()) > { > CORE_ADDR pc = pc_in; > > diff --git a/gdb/objfiles.c b/gdb/objfiles.c > index ada5edc42fa..71518ea5b24 100644 > --- a/gdb/objfiles.c > +++ b/gdb/objfiles.c > @@ -446,44 +446,50 @@ entry_point_address (void) > return retval; > } > > -/* Iterator on PARENT and every separate debug objfile of PARENT. > - The usage pattern is: > - for (objfile = parent; > - objfile; > - objfile = objfile_separate_debug_iterate (parent, objfile)) > - ... > -*/ > - > -struct objfile * > -objfile_separate_debug_iterate (const struct objfile *parent, > - const struct objfile *objfile) > +separate_debug_iterator & > +separate_debug_iterator::operator++ () > { > + gdb_assert (m_objfile != nullptr); > + > struct objfile *res; > > /* If any, return the first child. */ > - res = objfile->separate_debug_objfile; > + res = m_objfile->separate_debug_objfile; > if (res) > - return res; > + { > + m_objfile = res; > + return *this; > + } > > /* Common case where there is no separate debug objfile. */ > - if (objfile == parent) > - return NULL; > + if (m_objfile == m_parent) > + { > + m_objfile = nullptr; > + return *this; > + } > > /* Return the brother if any. Note that we don't iterate on brothers of > the parents. */ > - res = objfile->separate_debug_objfile_link; > + res = m_objfile->separate_debug_objfile_link; > if (res) > - return res; > + { > + m_objfile = res; > + return *this; > + } > > - for (res = objfile->separate_debug_objfile_backlink; > - res != parent; > + for (res = m_objfile->separate_debug_objfile_backlink; > + res != m_parent; > res = res->separate_debug_objfile_backlink) > { > gdb_assert (res != NULL); > if (res->separate_debug_objfile_link) > - return res->separate_debug_objfile_link; > + { > + m_objfile = res->separate_debug_objfile_link; > + return *this; > + } > } > - return NULL; > + m_objfile = nullptr; > + return *this; > } > > /* Put one object file before a specified on in the global list. > @@ -860,15 +866,15 @@ void > objfile_relocate (struct objfile *objfile, > const struct section_offsets *new_offsets) > { > - struct objfile *debug_objfile; > int changed = 0; > > changed |= objfile_relocate1 (objfile, new_offsets); > > - for (debug_objfile = objfile->separate_debug_objfile; > - debug_objfile; > - debug_objfile = objfile_separate_debug_iterate (objfile, debug_objfile)) > + for (struct objfile *debug_objfile : objfile->separate_debug_objfiles ()) > { > + if (debug_objfile == objfile) > + continue; > + > section_addr_info objfile_addrs > = build_section_addr_info_from_objfile (objfile); > > @@ -917,14 +923,9 @@ objfile_rebase1 (struct objfile *objfile, CORE_ADDR slide) > void > objfile_rebase (struct objfile *objfile, CORE_ADDR slide) > { > - struct objfile *debug_objfile; > int changed = 0; > > - changed |= objfile_rebase1 (objfile, slide); > - > - for (debug_objfile = objfile->separate_debug_objfile; > - debug_objfile; > - debug_objfile = objfile_separate_debug_iterate (objfile, debug_objfile)) > + for (struct objfile *debug_objfile : objfile->separate_debug_objfiles ()) > changed |= objfile_rebase1 (debug_objfile, slide); > > /* Relocate breakpoints as necessary, after things are relocated. */ > @@ -965,9 +966,7 @@ objfile_has_full_symbols (struct objfile *objfile) > int > objfile_has_symbols (struct objfile *objfile) > { > - struct objfile *o; > - > - for (o = objfile; o; o = objfile_separate_debug_iterate (objfile, o)) > + for (struct objfile *o : objfile->separate_debug_objfiles ()) > if (objfile_has_partial_symbols (o) || objfile_has_full_symbols (o)) > return 1; > return 0; > diff --git a/gdb/objfiles.h b/gdb/objfiles.h > index 368d9f3abe2..168f7fc275b 100644 > --- a/gdb/objfiles.h > +++ b/gdb/objfiles.h > @@ -318,6 +318,63 @@ struct objfile_per_bfd_storage > std::bitset demangled_hash_languages; > }; > > +/* An iterator that first returns a parent objfile, and then each > + separate debug objfile. */ > + > +class separate_debug_iterator > +{ > +public: > + > + explicit separate_debug_iterator (struct objfile *objfile) > + : m_objfile (objfile), > + m_parent (objfile) > + { > + } > + > + bool operator!= (const separate_debug_iterator &other) > + { > + return m_objfile != other.m_objfile; > + } > + > + separate_debug_iterator &operator++ (); > + > + struct objfile *operator* () > + { > + return m_objfile; > + } > + > +private: > + > + struct objfile *m_objfile; > + struct objfile *m_parent; > +}; > + > +/* A range adapter wrapping separate_debug_iterator. */ > + > +class separate_debug_range > +{ > +public: > + > + explicit separate_debug_range (struct objfile *objfile) > + : m_objfile (objfile) > + { > + } > + > + separate_debug_iterator begin () > + { > + return separate_debug_iterator (m_objfile); > + } > + > + separate_debug_iterator end () > + { > + return separate_debug_iterator (nullptr); > + } > + > +private: > + > + struct objfile *m_objfile; > +}; > + > /* Master structure for keeping track of each file from which > gdb reads symbols. There are several ways these get allocated: 1. > The main symbol file, symfile_objfile, set by the symbol-file command, > @@ -396,6 +453,14 @@ struct objfile > return msymbols_range (this); > } > > + /* Return a range adapter for iterating over all the separate debug > + objfiles of this objfile. */ > + > + separate_debug_range separate_debug_objfiles () > + { > + return separate_debug_range (this); > + } > + > > /* All struct objfile's are chained together by their next pointers. > The program space field "objfiles" (frequently referenced via > @@ -563,9 +628,6 @@ extern CORE_ADDR entry_point_address (void); > > extern void build_objfile_section_table (struct objfile *); > > -extern struct objfile *objfile_separate_debug_iterate (const struct objfile *, > - const struct objfile *); > - > extern void put_objfile_before (struct objfile *, struct objfile *); > > extern void add_separate_debug_objfile (struct objfile *, struct objfile *); > diff --git a/gdb/symtab.c b/gdb/symtab.c > index d25f560f084..16e641a830b 100644 > --- a/gdb/symtab.c > +++ b/gdb/symtab.c > @@ -2246,11 +2246,7 @@ lookup_global_symbol_from_objfile (struct objfile *main_objfile, > const char *name, > const domain_enum domain) > { > - struct objfile *objfile; > - > - for (objfile = main_objfile; > - objfile; > - objfile = objfile_separate_debug_iterate (main_objfile, objfile)) > + for (struct objfile *objfile : main_objfile->separate_debug_objfiles ()) > { > struct block_symbol result > = lookup_symbol_in_objfile (objfile, GLOBAL_BLOCK, name, domain); > @@ -2327,7 +2323,7 @@ lookup_symbol_in_objfile_from_linkage_name (struct objfile *objfile, > domain_enum domain) > { > enum language lang = current_language->la_language; > - struct objfile *main_objfile, *cur_objfile; > + struct objfile *main_objfile; > > demangle_result_storage storage; > const char *modified_name = demangle_for_lookup (linkage_name, lang, storage); > @@ -2337,9 +2333,7 @@ lookup_symbol_in_objfile_from_linkage_name (struct objfile *objfile, > else > main_objfile = objfile; > > - for (cur_objfile = main_objfile; > - cur_objfile; > - cur_objfile = objfile_separate_debug_iterate (main_objfile, cur_objfile)) > + for (struct objfile *cur_objfile : main_objfile->separate_debug_objfiles ()) > { > struct block_symbol result; > > Thanks, this LGTM. While touching this code, could you fixup the various? - if (res) + if (res != NULL) Simon