[v3] Replace solib_global_lookup with gdbarch_iterate_over_objfiles_in_search_order
Commit Message
[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 <cbiesinger@google.com>
* 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(-)
Comments
ping
On Tue, Aug 27, 2019 at 6:03 PM Christian Biesinger
<cbiesinger@google.com> 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 <cbiesinger@google.com>
>
> * 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<char> *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
>
ping
On Tue, Sep 10, 2019 at 11:35 PM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> ping
>
> On Tue, Aug 27, 2019 at 6:03 PM Christian Biesinger
> <cbiesinger@google.com> 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 <cbiesinger@google.com>
> >
> > * 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<char> *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
> >
On 2019-08-27 7:03 p.m., Christian Biesinger via gdb-patches 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.
Hi Christian,
The patch LGTM, I'm also confident that it doesn't change the current
behavior. I just have two very minor comments below.
Note that SPU support is about to be removed [1]. If you wait until it is
removed, it would simplify a bit this patch, as you wish.
[1] https://sourceware.org/ml/gdb-patches/2019-09/msg00067.html
> @@ -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;
> + }
> }
If we do search the current objfile in the first part of that
function, we could skip it in the second part.
> 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"
Could this be a forward declaration of `struct gdbarch` instead?
Though if you wait until SPU is removed, the function
svr4_iterate_over_objfiles_in_search_order won't need to be exported, it should
be made static.
Simon
On Fri, Sep 20, 2019 at 10:00 AM Simon Marchi <simark@simark.ca> wrote:
>
> On 2019-08-27 7:03 p.m., Christian Biesinger via gdb-patches 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.
>
> Hi Christian,
>
> The patch LGTM, I'm also confident that it doesn't change the current
> behavior. I just have two very minor comments below.
>
> Note that SPU support is about to be removed [1]. If you wait until it is
> removed, it would simplify a bit this patch, as you wish.
>
> [1] https://sourceware.org/ml/gdb-patches/2019-09/msg00067.html
Will do.
> > @@ -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;
> > + }
> > }
>
> If we do search the current objfile in the first part of that
> function, we could skip it in the second part.
Done.
> > 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"
>
> Could this be a forward declaration of `struct gdbarch` instead?
>
> Though if you wait until SPU is removed, the function
> svr4_iterate_over_objfiles_in_search_order won't need to be exported, it should
> be made static.
Done (made static).
Will send a new version of the patch and push when SPU is removed.
Christian
@@ -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;
}
@@ -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);
}
@@ -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;
@@ -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 */
@@ -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
@@ -139,12 +139,6 @@ struct target_so_ops
unsigned o_flags,
gdb::unique_xmalloc_ptr<char> *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
@@ -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));