[PATCH v2] Replace solib_global_lookup with gdbarch_iterate_over_objfiles_in_search_order

[PATCH v2] Replace solib_global_lookup with gdbarch_iterate_over_objfiles_in_search_order

[Christian Biesinger via gdb-patches]

[v2 of https://sourceware.org/ml/gdb-patches/2019-08/msg00030.html,
but not reusing that thread because that was an RFC, this is proposed for
submitting]

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   | 53 +++++++++++++++++++++++++++++-----------------
 gdb/solib-svr4.h   |  5 +++++
 gdb/solib.c        | 15 -------------
 gdb/solist.h       | 11 ----------
 gdb/symtab.c       |  6 +-----
 7 files changed, 55 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;
 }
 \f
-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..ad10e6acd0 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,46 @@ 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 == symfile_objfile)
+	abfd = exec_bfd;
+      else
+	{
+	  /* OBJFILE should have been passed as the non-debug one.  */
+	  gdb_assert (current_objfile->separate_debug_objfile_backlink == nullptr);
 
-  if (abfd == NULL || scan_dyntag (DT_SYMBOLIC, abfd, NULL, NULL) != 1)
-    return {};
+	  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 (objfile != current_objfile)
+	{
+	  if (cb (objfile, cb_data) != 0)
+	    return;
+	}
+    }
 }
 
 void
@@ -3244,7 +3260,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

[PATCH v3] Replace solib_global_lookup with gdbarch_iterate_over_objfiles_in_search_order

[Christian Biesinger via gdb-patches]

[Fixes a bug in the SVR4 impl. Buildbot does have a failure in
gdb.base/solib-corrupted.exp which I am investigating.]

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   | 50 ++++++++++++++++++++++++++++------------------
 gdb/solib-svr4.h   |  5 +++++
 gdb/solib.c        | 15 --------------
 gdb/solist.h       | 11 ----------
 gdb/symtab.c       |  6 +-----
 7 files changed, 52 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;
 }
 \f
-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..ebab4e6b87 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,43 @@ 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 == symfile_objfile)
+	abfd = exec_bfd;
+      else
+	{
+	  /* OBJFILE should have been passed as the non-debug one.  */
+	  gdb_assert (current_objfile->separate_debug_objfile_backlink == nullptr);
 
-  if (abfd == NULL || scan_dyntag (DT_SYMBOLIC, abfd, NULL, NULL) != 1)
-    return {};
+	  abfd = current_objfile->obfd;
+	}
 
-  return lookup_global_symbol_from_objfile (objfile, GLOBAL_BLOCK, name,
-					    domain);
+      if (abfd != nullptr &&
+	  scan_dyntag (DT_SYMBOLIC, abfd, nullptr, nullptr) == 1)
+	{
+	  if (cb (current_objfile, cb_data) != 0)
+	    return;
+	}
+    }
+
+  for (objfile *objfile : current_program_space->objfiles ())
+    {
+      if (cb (objfile, cb_data) != 0)
+	return;
+    }
 }
 
 void
@@ -3244,7 +3257,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

[PATCH v3] Replace solib_global_lookup with gdbarch_iterate_over_objfiles_in_search_order

[Christian Biesinger via gdb-patches]

[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;
 }
 \f
-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] [PATCH v3] Replace solib_global_lookup with gdbarch_iterate_over_objfiles_in_search_order

[Christian Biesinger via gdb-patches]

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
>

Re: [ping] [PATCH v3] Replace solib_global_lookup with gdbarch_iterate_over_objfiles_in_search_order

[Christian Biesinger via gdb-patches]

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
> >

Re: [PATCH v3] Replace solib_global_lookup with gdbarch_iterate_over_objfiles_in_search_order

[Simon Marchi]

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

[PUSHED/OBVIOUS] Don't duplicate comment in symfile.c and .h

[Christian Biesinger via gdb-patches]

This just replaces the comment in the .c file with
"See symfile.h.".

gdb/ChangeLog:

2019-09-16  Christian Biesinger  <cbiesinger@google.com>

	* symfile.c (auto_solib_add): Replace comment with a reference
	to the header file.
---
 gdb/symfile.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 3cd514409b..259a30aa04 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -141,15 +141,7 @@ static const char *print_symbol_loading_enums[] =
 };
 static const char *print_symbol_loading = print_symbol_loading_full;
 
-/* If non-zero, shared library symbols will be added automatically
-   when the inferior is created, new libraries are loaded, or when
-   attaching to the inferior.  This is almost always what users will
-   want to have happen; but for very large programs, the startup time
-   will be excessive, and so if this is a problem, the user can clear
-   this flag and then add the shared library symbols as needed.  Note
-   that there is a potential for confusion, since if the shared
-   library symbols are not loaded, commands like "info fun" will *not*
-   report all the functions that are actually present.  */
+/* See symfile.h.  */
 
 int auto_solib_add = 1;
 \f
-- 
2.23.0.237.gc6a4ce50a0-goog

Re: [PATCH v3] Replace solib_global_lookup with gdbarch_iterate_over_objfiles_in_search_order

[Christian Biesinger via gdb-patches]

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

[PATCH v4] Replace solib_global_lookup with gdbarch_iterate_over_objfiles_in_search_order

[Christian Biesinger via gdb-patches]

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-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.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-svr4.c   | 56 ++++++++++++++++++++++++++++++----------------
 gdb/solib.c        | 15 -------------
 gdb/solist.h       | 11 ---------
 gdb/symtab.c       |  6 +----
 5 files changed, 38 insertions(+), 59 deletions(-)

diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index 778c6d9f61..286e38558f 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;
 }
 \f
-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)
 {
@@ -687,6 +679,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-svr4.c b/gdb/solib-svr4.c
index e04fde942d..dc63549105 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -51,6 +51,10 @@ static int svr4_have_link_map_offsets (void);
 static void svr4_relocate_main_executable (void);
 static void svr4_free_library_list (void *p_list);
 static void probes_table_remove_objfile_probes (struct objfile *objfile);
+static 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);
+
 
 /* On SVR4 systems, a list of symbols in the dynamic linker where
    GDB can try to place a breakpoint to monitor shared library
@@ -3102,6 +3106,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
@@ -3200,32 +3206,45 @@ 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)
+static 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
+  bool checked_current_objfile = false;
+  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)
+	{
+	  checked_current_objfile = true;
+	  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 (checked_current_objfile && objfile == current_objfile)
+	continue;
+      if (cb (objfile, cb_data) != 0)
+	return;
+    }
 }
 
 void
@@ -3242,7 +3261,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.c b/gdb/solib.c
index 86000f6d61..60cad2122f 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 35eab08cb3..54c7ed9c7c 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2634,11 +2634,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.351.gc4317032e6-goog

Re: [PUSHED/OBVIOUS] Don't duplicate comment in symfile.c and .h

[Christian Biesinger via gdb-patches]

argh, accidentally sent the wrong file. please disregard (this is a duplicate)

On Fri, Sep 20, 2019 at 11:57 AM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> This just replaces the comment in the .c file with
> "See symfile.h.".
>
> gdb/ChangeLog:
>
> 2019-09-16  Christian Biesinger  <cbiesinger@google.com>
>
>         * symfile.c (auto_solib_add): Replace comment with a reference
>         to the header file.
> ---
>  gdb/symfile.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 3cd514409b..259a30aa04 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -141,15 +141,7 @@ static const char *print_symbol_loading_enums[] =
>  };
>  static const char *print_symbol_loading = print_symbol_loading_full;
>
> -/* If non-zero, shared library symbols will be added automatically
> -   when the inferior is created, new libraries are loaded, or when
> -   attaching to the inferior.  This is almost always what users will
> -   want to have happen; but for very large programs, the startup time
> -   will be excessive, and so if this is a problem, the user can clear
> -   this flag and then add the shared library symbols as needed.  Note
> -   that there is a potential for confusion, since if the shared
> -   library symbols are not loaded, commands like "info fun" will *not*
> -   report all the functions that are actually present.  */
> +/* See symfile.h.  */
>
>  int auto_solib_add = 1;
>
> --
> 2.23.0.237.gc6a4ce50a0-goog
>

Re: [PATCH v4] Replace solib_global_lookup with gdbarch_iterate_over_objfiles_in_search_order

[Simon Marchi]

On 2019-09-19 10:58 p.m., Christian Biesinger via gdb-patches wrote:
> 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,

This LGTM, it is fine to push after the SPU removal patch.

Thanks,

Simon

Re: [PATCH v4] Replace solib_global_lookup with gdbarch_iterate_over_objfiles_in_search_order

[Christian Biesinger via gdb-patches]

On Fri, Sep 20, 2019 at 7:56 PM Simon Marchi <simark@simark.ca> wrote:
>
> On 2019-09-19 10:58 p.m., Christian Biesinger via gdb-patches wrote:
> > 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,
>
> This LGTM, it is fine to push after the SPU removal patch.

Thanks, pushed now.

Christian