[PATCH v6 1/3] gdb: make lookup_minimal_symbol_linkage work with linker namespaces

[Guinevere Larsen <guinevere@redhat.com>]

When a symbol found by GDB may have copy relocation, GDB calculates the
address of the symbol by checking all the objfiles in the current
program space for another instance of the same symbol, and returns the
address of the first instance found.

This works well when linker namespaces are not involved, but fails when
a symbol is present in more than one namespace. That's because copy
relocations respect linker namespace boundaries, and so if we're trying
to find the symbol in namespace N, the symbol from a different namespace
M may be returned, if the SO containing the symbol was loaded in M
before N.

To make the search work correctly, lookup_minimal_symbol_linkage would
need to also respect linker namespace boundaries. However, to avoid
leaking solib knowledge to minsyms, and because of how little
information is passed to the function, this commit instead makes the
function take a vector of objfiles to search, in place of the program
space. This makes it so symbol::get_maybe_copied_address (and the
equivalent of minimal_symbol) need to request the objfiles for the
correct namespace, since they have enough context to figure out the
namespace. Creating the vector is left as a function in solib.c

Since minimal_symbol::get_maybe_copied_address only searches main_file
objfiles, if we can guarantee that only one is loaded for each program
space, we could avoid the work of constructing an std::vector that will
be mostly ignored. However, I couldn't convince myself that that is the
case, so I decided to keep the behavior exactly as is.

Ideally, lookup_minimal_symbol_linkage would receive an iterator instead
of a vector, but as of now it isn't possible to do forward declarations
of nested types (we'd need program_space::objfiles_range) and
progspace.h can't be included into minsyms.h, so we're stuck
constructing a vector for it.

Finally, as some minor refactoring, find_solib_for_objfile is moved to
solib.c to simplify looking for which namespace contains the objfile
where we found the symbol the user was looking for.
---
 gdb/dwarf2/ada-imported.c |  8 +++-
 gdb/minsyms.c             | 15 ++++---
 gdb/minsyms.h             |  9 ++--
 gdb/solib-svr4.c          | 20 ---------
 gdb/solib.c               | 86 ++++++++++++++++++++++++++++++++++++++-
 gdb/solib.h               | 19 +++++++++
 gdb/symtab.c              | 13 +++++-
 7 files changed, 136 insertions(+), 34 deletions(-)

diff --git a/gdb/dwarf2/ada-imported.c b/gdb/dwarf2/ada-imported.c
index 48e6fccdb73..15ecfdaf6ce 100644
--- a/gdb/dwarf2/ada-imported.c
+++ b/gdb/dwarf2/ada-imported.c
@@ -35,9 +35,13 @@ static struct value *
 ada_imported_read_variable (struct symbol *symbol, const frame_info_ptr &frame)
 {
   const char *name = get_imported_name (symbol);
+
+  std::vector<objfile *> objfiles_to_search;
+  for (objfile &objf : symbol->objfile ()->pspace ()->objfiles ())
+    objfiles_to_search.push_back (&objf);
+
   bound_minimal_symbol minsym
-    = lookup_minimal_symbol_linkage (symbol->objfile ()->pspace (), name,
-				     true, false);
+    = lookup_minimal_symbol_linkage (objfiles_to_search, name, true, false);
   if (minsym.minsym == nullptr)
     error (_("could not find imported name %s"), name);
   return value_at (symbol->type (), minsym.value_address ());
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 90a0c601c59..7e4f47b8c3f 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -54,6 +54,7 @@
 #include "gdbsupport/cxx-thread.h"
 #include "gdbsupport/parallel-for.h"
 #include "inferior.h"
+#include "solib.h"
 
 /* Return true if MINSYM is a cold clone symbol.
    Recognize f.i. these symbols (mangled/demangled):
@@ -584,19 +585,21 @@ lookup_minimal_symbol_linkage (const char *name, struct objfile *objf,
 /* See minsyms.h.  */
 
 bound_minimal_symbol
-lookup_minimal_symbol_linkage (program_space *pspace, const char *name,
-			       bool match_static_type, bool only_main)
+lookup_minimal_symbol_linkage (gdb::array_view<objfile *> objfiles_to_search,
+			       const char *name, bool match_static_type,
+			       bool only_main)
 {
-  for (objfile &objfile : pspace->objfiles ())
+  for (objfile *objfile : objfiles_to_search)
     {
-      if (objfile.separate_debug_objfile_backlink != nullptr)
+      if (objfile == nullptr
+	  || objfile->separate_debug_objfile_backlink != nullptr)
 	continue;
 
-      if (only_main && (objfile.flags & OBJF_MAINLINE) == 0)
+      if (only_main && (objfile->flags & OBJF_MAINLINE) == 0)
 	continue;
 
       bound_minimal_symbol minsym
-	= lookup_minimal_symbol_linkage (name, &objfile, match_static_type);
+	= lookup_minimal_symbol_linkage (name, objfile, match_static_type);
       if (minsym.minsym != nullptr)
 	return minsym;
     }
diff --git a/gdb/minsyms.h b/gdb/minsyms.h
index ed38044a38c..4aa8a428470 100644
--- a/gdb/minsyms.h
+++ b/gdb/minsyms.h
@@ -237,11 +237,14 @@ extern bound_minimal_symbol lookup_minimal_symbol_linkage
 
 /* A variant of lookup_minimal_symbol_linkage that iterates over all
    objfiles of PSPACE.  If ONLY_MAIN is true, then only an objfile with
-   OBJF_MAINLINE will be considered.  */
+   OBJF_MAINLINE will be considered.  This function should receive a
+   program_space::objfile_ranges instead, but we can't include progspace.h
+   here, nor can we do forward declarations of nested types, so std::vector
+   will waste a bit of memory to work around that issue.  */
 
 extern bound_minimal_symbol lookup_minimal_symbol_linkage
-  (program_space *pspace, const char *name, bool match_static_type,
-   bool only_main) ATTRIBUTE_NONNULL (1);
+  (gdb::array_view<objfile *> objfiles_to_search, const char *name,
+   bool match_static_type, bool only_main);
 
 /* Look through all the current minimal symbol tables and find the
    first minimal symbol that matches NAME and PC.  If OBJF is non-NULL,
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index c91e0b2c37b..49d2b207c36 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -3595,26 +3595,6 @@ lp64_svr4_solib_ops::fetch_link_map_offsets () const
 }
 \f
 
-/* Return the DSO matching OBJFILE or nullptr if none can be found.  */
-
-static const solib *
-find_solib_for_objfile (struct objfile *objfile)
-{
-  if (objfile == nullptr)
-    return nullptr;
-
-  /* If OBJFILE is a separate debug object file, look for the original
-     object file.  */
-  if (objfile->separate_debug_objfile_backlink != nullptr)
-    objfile = objfile->separate_debug_objfile_backlink;
-
-  for (const solib &so : current_program_space->solibs ())
-    if (so.objfile == objfile)
-      return &so;
-
-  return nullptr;
-}
-
 /* Return the address of the r_debug object for the namespace containing
    SOLIB or zero if it cannot be found.  This may happen when symbol files
    are added manually, for example, or with the main executable.
diff --git a/gdb/solib.c b/gdb/solib.c
index dade2a76a48..34b0d6b3f9c 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -1825,8 +1825,92 @@ solib_linker_namespace_count (program_space *pspace)
   return 0;
 }
 
-/* Implementation of the linker_namespace convenience variable.
+/* See solib.h.  */
+
+const solib *
+find_solib_for_objfile (struct objfile *objfile)
+{
+  if (objfile == nullptr)
+    return nullptr;
+
+  /* If OBJFILE is a separate debug object file, look for the original
+     object file.  */
+  if (objfile->separate_debug_objfile_backlink != nullptr)
+    objfile = objfile->separate_debug_objfile_backlink;
+
+  for (const solib &so : objfile->pspace ()->solibs ())
+    if (so.objfile == objfile)
+      return &so;
+
+  return nullptr;
+}
+
+/* See solib.h.  */
+
+std::vector<objfile *>
+get_objfiles_in_linker_namespace (int nsid, program_space *pspace)
+{
+  std::vector<objfile *> objfiles_in_ns;
+  const solib_ops *ops = pspace->solib_ops ();
+
+  gdb_assert (ops->supports_namespaces ());
+
+  /* If we're looking at the default namespace, we also need
+     to add the mainline objfiles by hand, since there is no
+     solib associated with those files.  We add them first
+     because if we're searching for copy relocations, they
+     should be in the main file, so we'll find it faster.  */
+  if (nsid == 0)
+    for (objfile &objf : pspace->objfiles ())
+      if ((objf.flags & OBJF_MAINLINE) != 0)
+	objfiles_in_ns.push_back (&objf);
+
+  std::vector<const solib *> solibs = ops->get_solibs_in_ns (nsid);
+  /* Reserve for efficiency.  */
+  objfiles_in_ns.reserve (solibs.size () + objfiles_in_ns.size ());
+  for (const solib *so : solibs)
+    objfiles_in_ns.push_back (so->objfile);
+
+  return objfiles_in_ns;
+}
+
+/* See solib.h.  */
+
+std::vector<objfile *>
+get_objfiles_in_linker_namespace (objfile *objfile)
+{
+  program_space *pspace = objfile->pspace ();
+  const solib_ops *ops = pspace->solib_ops ();
+  const solib *so = find_solib_for_objfile (objfile);
+
+  /* If the inferior hasn't started yet, the solib_ops won't be
+     set for the program space.  Since namespaces only make sense
+     when the inferior has already created some, we can just skip
+     it here.  */
+  if (ops != nullptr && ops->supports_namespaces ()
+      /* If we're searching for a symbol from the linker, we'll reach here
+	 before having any namespaces.  Return all objfiles since the
+	 boundaries haven't been setup yet.  */
+      && ops->num_active_namespaces () > 0
+      /* When trying to load libthread_db, we can search for a symbol in an
+	 objfile with no associated solib.  In that case, again, we should
+	 return all objfiles.  */
+      && so != nullptr)
+    {
+      return get_objfiles_in_linker_namespace (ops->find_solib_ns (*so),
+					       pspace);
+    }
+
+  /* If any of the previous conditions isn't satisfied, we return
+     the full list of objfiles in the inferior.  */
+  std::vector<struct objfile *> found_objfiles;
+  for (struct objfile &objf : objfile->pspace ()->objfiles ())
+    found_objfiles.push_back (&objf);
 
+  return found_objfiles;
+}
+
+/* Implementation of the linker_namespace convenience variable.
    This returns the GDB internal identifier of the linker namespace,
    for the selected frame, as an integer.  If the inferior doesn't support
    linker namespaces, this always returns 0.  */
diff --git a/gdb/solib.h b/gdb/solib.h
index 85ea6675b79..81389da6267 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -20,6 +20,9 @@
 #ifndef GDB_SOLIB_H
 #define GDB_SOLIB_H
 
+/* Forward decl's for prototypes */
+struct objfile;
+
 #include "gdb_bfd.h"
 #include "gdbsupport/function-view.h"
 #include "gdbsupport/intrusive_list.h"
@@ -409,4 +412,20 @@ extern void handle_solib_event (void);
 
 extern int solib_linker_namespace_count (program_space *pspace);
 
+/* Return a vector with pointers of all objfiles in the namespace
+   NSID.  This version assumes that the inferior supports namespaces.  */
+
+std::vector<objfile *> get_objfiles_in_linker_namespace
+  (int nsid, program_space *pspace);
+
+/* Return a vector with pointers of all objfiles in the same namespace
+   as OBJFILE.  If the inferior doesn't support namespaces, return all
+   objfiles in the program_space.  */
+
+std::vector<objfile *> get_objfiles_in_linker_namespace (objfile *objfile);
+
+/* Return the DSO matching OBJFILE or nullptr if none can be found.  */
+
+const solib *find_solib_for_objfile (struct objfile *objfile);
+
 #endif /* GDB_SOLIB_H */
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 3b0687c0750..786dd9f56ba 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -74,6 +74,7 @@
 #include "gdbsupport/common-utils.h"
 #include <optional>
 #include "gdbsupport/unordered_set.h"
+#include "solib.h"
 
 /* Forward declarations for local functions.  */
 
@@ -6575,8 +6576,12 @@ symbol::get_maybe_copied_address () const
   gdb_assert (this->loc_class () == LOC_STATIC);
 
   const char *linkage_name = this->linkage_name ();
+
+  std::vector<struct objfile *> objfiles_to_search
+    (get_objfiles_in_linker_namespace (this->objfile ()));
+
   bound_minimal_symbol minsym
-    = lookup_minimal_symbol_linkage (this->objfile ()->pspace (), linkage_name,
+    = lookup_minimal_symbol_linkage (objfiles_to_search, linkage_name,
 				     false, false);
   if (minsym.minsym != nullptr)
     return minsym.value_address ();
@@ -6593,8 +6598,12 @@ minimal_symbol::get_maybe_copied_address (objfile *objf) const
   gdb_assert ((objf->flags & OBJF_MAINLINE) == 0);
 
   const char *linkage_name = this->linkage_name ();
+
+  std::vector<struct objfile *> objfiles_to_search
+    (get_objfiles_in_linker_namespace (objf));
+
   bound_minimal_symbol found
-    = lookup_minimal_symbol_linkage (objf->pspace (), linkage_name,
+    = lookup_minimal_symbol_linkage (objfiles_to_search, linkage_name,
 				     false, true);
   if (found.minsym != nullptr)
     return found.value_address ();
-- 
2.51.0