From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3680 invoked by alias); 17 Sep 2019 12:03:24 -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 3669 invoked by uid 89); 17 Sep 2019 12:03:24 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-30.7 required=5.0 tests=AWL,BAYES_00,ENV_AND_HDR_SPF_MATCH,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS,USER_IN_DEF_SPF_WL autolearn=ham version=3.3.1 spammy=HX-Google-Smtp-Source:APXvYqy, DATA, 1396 X-HELO: mail-oi1-f195.google.com Received: from mail-oi1-f195.google.com (HELO mail-oi1-f195.google.com) (209.85.167.195) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 17 Sep 2019 12:03:21 +0000 Received: by mail-oi1-f195.google.com with SMTP id w17so2577054oiw.8 for ; Tue, 17 Sep 2019 05:03:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=5XbXjRbYD0xolX8EXBjbF6E/9OIh6cBpfiC1+GcVnKg=; b=PCmXIJeiADqkia+em0BdUHa+XQvqn0IgZSpZ+4eAURzX/Rm6/WerzBjZheDeVlinQG SkZhzs7VaPb7Mj7VOQu3r9Q4sMsX6i5azs6BZHmPXdlLWXVrES5bxfaY3PaHCcujJnW+ 1/QBa5uMdg8RljK9UV/mHXz8CiS6AwTp1qw/mAyGJmRTf4qksk9I7LkOz+TsqM9pmCEP JegtpMKNkvjgArCDQfl4AwZZb/adU9gAltr8fwGG+qgL9LwInSxj+yQkvs0saTAnXx1C DGOj1O9rhIjCr64aD66qQe4zBoGlof6YBSM9nGm1MNFlvS8DwAi+AIqkr0uaOm+K4aC/ v8yA== MIME-Version: 1.0 References: <20190827213752.204025-1-cbiesinger@google.com> <20190827230330.89757-1-cbiesinger@google.com> In-Reply-To: From: "Christian Biesinger via gdb-patches" Reply-To: Christian Biesinger Date: Tue, 17 Sep 2019 12:03:00 -0000 Message-ID: Subject: Re: [ping] [PATCH v3] Replace solib_global_lookup with gdbarch_iterate_over_objfiles_in_search_order To: gdb-patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2019-09/txt/msg00311.txt.bz2 ping On Tue, Sep 10, 2019 at 11:35 PM Christian Biesinger wrote: > > ping > > On Tue, Aug 27, 2019 at 6:03 PM Christian Biesinger > wrote: > > > > [Test solib-corrupted.exp is now fixed] > > > > All implementations of either function use it for the same purpose (except > > Darwin, which is a no-op): to prefer a symbol in the current objfile over > > symbols with the same name in other objfiles. There does not seem to be a > > reason to have both mechanisms for that purpose. > > > > gdb/ChangeLog: > > > > 2019-08-27 Christian Biesinger > > > > * solib-darwin.c (darwin_lookup_lib_symbol): Remove. > > (_initialize_darwin_solib): Don't set > > darwin_so_ops.lookup_lib_global_symbol. > > * solib-spu.c (spu_lookup_lib_symbol): Rename to... > > (spu_iterate_over_objfiles_in_search_order): ...this, > > and update to the iterate semantics. > > (set_spu_solib_ops): Instead of setting lookup_lib_global_symbol, > > call set_gdbarch_iterate_over_objfiles_in_search_order. > > * solib-svr4.c (set_solib_svr4_fetch_link_map_offsets): Call > > set_gdbarch_iterate_over_objfiles_in_search_order. > > (elf_lookup_lib_symbol): Rename to... > > (svr4_iterate_over_objfiles_in_search_order): this, and update > > to iterate semantics. > > (_initialize_svr4_solib): Don't set lookup_lib_global_symbol. > > * solib-svr4.h: Declare svr4_iterate_over_objfiles_in_search_order. > > * solib.c (solib_global_lookup): Remove. > > * solist.h (struct target_so_ops): Remove lookup_lib_global_symbol. > > (solib_global_lookup): Remove. > > * symtab.c (lookup_global_or_static_symbol): Remove call to > > solib_global_lookup. > > --- > > gdb/solib-darwin.c | 9 --------- > > gdb/solib-spu.c | 27 ++++++++++++++------------ > > gdb/solib-svr4.c | 48 ++++++++++++++++++++++++++++------------------ > > gdb/solib-svr4.h | 5 +++++ > > gdb/solib.c | 15 --------------- > > gdb/solist.h | 11 ----------- > > gdb/symtab.c | 6 +----- > > 7 files changed, 50 insertions(+), 71 deletions(-) > > > > diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c > > index 443ebb64a4..b386cd35ed 100644 > > --- a/gdb/solib-darwin.c > > +++ b/gdb/solib-darwin.c > > @@ -638,14 +638,6 @@ darwin_relocate_section_addresses (struct so_list *so, > > so->addr_low = sec->addr; > > } > > > > -static struct block_symbol > > -darwin_lookup_lib_symbol (struct objfile *objfile, > > - const char *name, > > - const domain_enum domain) > > -{ > > - return {}; > > -} > > - > > static gdb_bfd_ref_ptr > > darwin_bfd_open (const char *pathname) > > { > > @@ -688,6 +680,5 @@ _initialize_darwin_solib (void) > > darwin_so_ops.current_sos = darwin_current_sos; > > darwin_so_ops.open_symbol_file_object = open_symbol_file_object; > > darwin_so_ops.in_dynsym_resolve_code = darwin_in_dynsym_resolve_code; > > - darwin_so_ops.lookup_lib_global_symbol = darwin_lookup_lib_symbol; > > darwin_so_ops.bfd_open = darwin_bfd_open; > > } > > diff --git a/gdb/solib-spu.c b/gdb/solib-spu.c > > index 5b97b9bcf6..ed33a0adf2 100644 > > --- a/gdb/solib-spu.c > > +++ b/gdb/solib-spu.c > > @@ -385,19 +385,21 @@ spu_bfd_open (const char *pathname) > > return abfd; > > } > > > > -/* Lookup global symbol in a SPE executable. */ > > -static struct block_symbol > > -spu_lookup_lib_symbol (struct objfile *objfile, > > - const char *name, > > - const domain_enum domain) > > +/* Search order prefers the current objfile for SPE executables. */ > > +static void > > +spu_iterate_over_objfiles_in_search_order > > + (struct gdbarch *gdbarch, > > + iterate_over_objfiles_in_search_order_cb_ftype *cb, > > + void *cb_data, struct objfile *objfile) > > { > > - if (bfd_get_arch (objfile->obfd) == bfd_arch_spu) > > - return lookup_global_symbol_from_objfile (objfile, GLOBAL_BLOCK, name, > > - domain); > > + if (objfile != nullptr && bfd_get_arch (objfile->obfd) == bfd_arch_spu) > > + { > > + if (cb (objfile, cb_data) != 0) > > + return; > > + } > > > > - if (svr4_so_ops.lookup_lib_global_symbol != NULL) > > - return svr4_so_ops.lookup_lib_global_symbol (objfile, name, domain); > > - return {}; > > + return svr4_iterate_over_objfiles_in_search_order( > > + gdbarch, cb, cb_data, objfile); > > } > > > > /* Enable shared library breakpoint. */ > > @@ -515,9 +517,10 @@ set_spu_solib_ops (struct gdbarch *gdbarch) > > spu_so_ops.free_so = spu_free_so; > > spu_so_ops.current_sos = spu_current_sos; > > spu_so_ops.bfd_open = spu_bfd_open; > > - spu_so_ops.lookup_lib_global_symbol = spu_lookup_lib_symbol; > > } > > > > + set_gdbarch_iterate_over_objfiles_in_search_order > > + (gdbarch, spu_iterate_over_objfiles_in_search_order); > > set_solib_ops (gdbarch, &spu_so_ops); > > } > > > > diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c > > index c0c505acaa..e595a3572b 100644 > > --- a/gdb/solib-svr4.c > > +++ b/gdb/solib-svr4.c > > @@ -3104,6 +3104,8 @@ set_solib_svr4_fetch_link_map_offsets (struct gdbarch *gdbarch, > > ops->fetch_link_map_offsets = flmo; > > > > set_solib_ops (gdbarch, &svr4_so_ops); > > + set_gdbarch_iterate_over_objfiles_in_search_order > > + (gdbarch, svr4_iterate_over_objfiles_in_search_order); > > } > > > > /* Fetch a link_map_offsets structure using the architecture-specific > > @@ -3202,32 +3204,41 @@ svr4_lp64_fetch_link_map_offsets (void) > > > > struct target_so_ops svr4_so_ops; > > > > -/* Lookup global symbol for ELF DSOs linked with -Bsymbolic. Those DSOs have a > > +/* Search order for ELF DSOs linked with -Bsymbolic. Those DSOs have a > > different rule for symbol lookup. The lookup begins here in the DSO, not in > > the main executable. */ > > > > -static struct block_symbol > > -elf_lookup_lib_symbol (struct objfile *objfile, > > - const char *name, > > - const domain_enum domain) > > +void > > +svr4_iterate_over_objfiles_in_search_order > > + (struct gdbarch *gdbarch, > > + iterate_over_objfiles_in_search_order_cb_ftype *cb, > > + void *cb_data, struct objfile *current_objfile) > > { > > - bfd *abfd; > > - > > - if (objfile == symfile_objfile) > > - abfd = exec_bfd; > > - else > > + if (current_objfile != nullptr) > > { > > - /* OBJFILE should have been passed as the non-debug one. */ > > - gdb_assert (objfile->separate_debug_objfile_backlink == NULL); > > + bfd *abfd; > > > > - abfd = objfile->obfd; > > - } > > + if (current_objfile->separate_debug_objfile_backlink != nullptr) > > + current_objfile = current_objfile->separate_debug_objfile_backlink; > > > > - if (abfd == NULL || scan_dyntag (DT_SYMBOLIC, abfd, NULL, NULL) != 1) > > - return {}; > > + if (current_objfile == symfile_objfile) > > + abfd = exec_bfd; > > + else > > + abfd = current_objfile->obfd; > > + > > + if (abfd != nullptr && > > + scan_dyntag (DT_SYMBOLIC, abfd, nullptr, nullptr) == 1) > > + { > > + if (cb (current_objfile, cb_data) != 0) > > + return; > > + } > > + } > > > > - return lookup_global_symbol_from_objfile (objfile, GLOBAL_BLOCK, name, > > - domain); > > + for (objfile *objfile : current_program_space->objfiles ()) > > + { > > + if (cb (objfile, cb_data) != 0) > > + return; > > + } > > } > > > > void > > @@ -3244,7 +3255,6 @@ _initialize_svr4_solib (void) > > svr4_so_ops.open_symbol_file_object = open_symbol_file_object; > > svr4_so_ops.in_dynsym_resolve_code = svr4_in_dynsym_resolve_code; > > svr4_so_ops.bfd_open = solib_bfd_open; > > - svr4_so_ops.lookup_lib_global_symbol = elf_lookup_lib_symbol; > > svr4_so_ops.same = svr4_same; > > svr4_so_ops.keep_data_in_core = svr4_keep_data_in_core; > > svr4_so_ops.update_breakpoints = svr4_update_solib_event_breakpoints; > > diff --git a/gdb/solib-svr4.h b/gdb/solib-svr4.h > > index a051e70b79..b99b8e2e3a 100644 > > --- a/gdb/solib-svr4.h > > +++ b/gdb/solib-svr4.h > > @@ -21,6 +21,7 @@ > > #define SOLIB_SVR4_H > > > > #include "solist.h" > > +#include "gdbarch.h" > > > > struct objfile; > > struct target_so_ops; > > @@ -107,4 +108,8 @@ extern struct link_map_offsets *svr4_lp64_fetch_link_map_offsets (void); > > SVR4 run time loader. */ > > int svr4_in_dynsym_resolve_code (CORE_ADDR pc); > > > > +extern void svr4_iterate_over_objfiles_in_search_order ( > > + struct gdbarch *gdbarch, iterate_over_objfiles_in_search_order_cb_ftype *cb, > > + void *cb_data, struct objfile *objfile); > > + > > #endif /* solib-svr4.h */ > > diff --git a/gdb/solib.c b/gdb/solib.c > > index 29a17ad5d4..ec3bf06c50 100644 > > --- a/gdb/solib.c > > +++ b/gdb/solib.c > > @@ -1441,21 +1441,6 @@ show_auto_solib_add (struct ui_file *file, int from_tty, > > } > > > > > > -/* Handler for library-specific lookup of global symbol NAME in OBJFILE. Call > > - the library-specific handler if it is installed for the current target. */ > > - > > -struct block_symbol > > -solib_global_lookup (struct objfile *objfile, > > - const char *name, > > - const domain_enum domain) > > -{ > > - const struct target_so_ops *ops = solib_ops (target_gdbarch ()); > > - > > - if (ops->lookup_lib_global_symbol != NULL) > > - return ops->lookup_lib_global_symbol (objfile, name, domain); > > - return {}; > > -} > > - > > /* Lookup the value for a specific symbol from dynamic 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 > > diff --git a/gdb/solist.h b/gdb/solist.h > > index e1410c2b79..ee99fc815b 100644 > > --- a/gdb/solist.h > > +++ b/gdb/solist.h > > @@ -139,12 +139,6 @@ struct target_so_ops > > unsigned o_flags, > > gdb::unique_xmalloc_ptr *temp_pathname); > > > > - /* Hook for looking up global symbols in a library-specific way. */ > > - struct block_symbol (*lookup_lib_global_symbol) > > - (struct objfile *objfile, > > - const char *name, > > - const domain_enum domain); > > - > > /* Given two so_list objects, one from the GDB thread list > > and another from the list returned by current_sos, return 1 > > if they represent the same library. > > @@ -209,9 +203,4 @@ extern gdb_bfd_ref_ptr solib_bfd_open (const char *in_pathname); > > /* FIXME: gdbarch needs to control this variable. */ > > extern struct target_so_ops *current_target_so_ops; > > > > -/* Handler for library-specific global symbol lookup in solib.c. */ > > -struct block_symbol solib_global_lookup (struct objfile *objfile, > > - const char *name, > > - const domain_enum domain); > > - > > #endif > > diff --git a/gdb/symtab.c b/gdb/symtab.c > > index 7762c85708..c74f39c3cd 100644 > > --- a/gdb/symtab.c > > +++ b/gdb/symtab.c > > @@ -2637,11 +2637,7 @@ lookup_global_or_static_symbol (const char *name, > > return result; > > } > > > > - /* Call library-specific lookup procedure. */ > > - if (objfile != NULL) > > - result = solib_global_lookup (objfile, name, domain); > > - > > - /* If that didn't work go a global search (of global blocks, heh). */ > > + /* Do a global search (of global blocks, heh). */ > > if (result.symbol == NULL) > > { > > memset (&lookup_data, 0, sizeof (lookup_data)); > > -- > > 2.23.0.187.g17f5b7556c-goog > >