[PATCH] gdb: Introduce user-friendly namespace identifier for "info shared"

[Guinevere Larsen <guinevere@redhat.com>]

GDB has had basic support for linkage namespaces for some time already,
but only in the sense of managing multiple copies of the same shared
object being loaded, and a very fragile way to find the correct copy of
a symbol (see PR shlibs/32054).

This commit is the first step in improving the user experience around
multiple namespace support. It introduces a user-friendly identifier for
namespaces, in the format #<number>, that will keep consistent between
dlmopen and dlclose calls. The plan is for this identifier to be usable
in expressions like `print #1::var` to find a specific instance of a
symbol, and so the identifier must not be a valid C++ or Ada namespace
identifier, otherwise disambiguation becomes a problem. Support for
those expressions has not been implemented yet, it is only mentioned to
explain why the identifier looks like this.

The namespace identifiers are stored in the svr4_info object, in both a
vector and an unordered_map. We use the vector because it makes it
trivial to just grow the identifier and number of namespaces, and we use
the map because we must remember what was the identifier of a given
address, since a dlclose call causes GDB to remove all SOs from its list
and reload them all; if the dlclose caused a namespace to be closed,
alll future namespaces would be assigned the id n-1.

Finally, a new solib_ops function pointer was added, find_solib_ns, to
allow code outside of solib-svr4 to find and use the namespace
identifiers. As a sanity check, the command `info sharedlibraries` has
been changed to display the namespace identifier. Plus, a couple of tests
had to be tweaked to handle the possible new column.
---

Hi all!

This is the first of a long running series of improvements for how GDB
will handle linkage namespaces. The hope is to eventually be able to use
namespace identifiers to print a symbol, set a NS-specific breakpoint,
and be able to analyse some things about namespaces themselves, such as
how many are open or in which one the inferior is stopped.

This first patch is mostly about how the support should be implemented,
I expect that the changes to `info sharedlibraries` would be pretty
simple.

---
 gdb/NEWS                                     |   4 +
 gdb/doc/gdb.texinfo                          |   4 +
 gdb/solib-svr4.c                             | 118 +++++++++++++++++--
 gdb/solib.c                                  |  23 +++-
 gdb/solist.h                                 |  11 ++
 gdb/testsuite/gdb.base/attach-pie-noexec.exp |   4 +-
 gdb/testsuite/gdb.base/dlmopen.exp           |   6 +-
 gdb/testsuite/gdb.mi/mi-dlmopen.exp          |   6 +-
 8 files changed, 160 insertions(+), 16 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index f5dbf5c3350..67794f4497c 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -26,6 +26,10 @@
 
 * Linux checkpoint code has been updated to work with multiple inferiors.
 
+* In systems that support linkage namespaces, the output of the command
+  "info sharedlibraries" will add one more column, NS, which identifies the
+  namespace into which the library was loaded.
+
 * New commands
 
 maintenance check psymtabs
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 95a881e9dff..19b9e6631e5 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22166,6 +22166,10 @@ Print the names of the shared libraries which are currently loaded
 that match @var{regex}.  If @var{regex} is omitted then print
 all shared libraries that are loaded.
 
+In systems that support linkage namespaces, the column @code{NS} is
+added to the output.  This column the linkage namespace that the
+shared library was loaded into.
+
 @kindex info dll
 @item info dll @var{regex}
 This is an alias of @code{info sharedlibrary}.
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 8378ecaff40..dabbd68d02e 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -405,11 +405,59 @@ struct svr4_info
      The special entry zero is reserved for a linear list to support
      gdbstubs that do not support namespaces.  */
   std::map<CORE_ADDR, std::vector<svr4_so>> solib_lists;
+
+  /* Mapping between r_debug[_ext] addresses and a user-friendly
+     identifier for the namespace.  We use both a vector and an unordered
+     map for a couple of reasons.  The vector is used to make it
+     easy to assign new internal IDs to namespaces, and to double check
+     which namespaces are still active.  The map is used when we're
+     rewriting the solib_lists, to make sure that IDs remain consistent
+     as SOs get dlclosed and deactivate namespaces.
+
+     For gdbservers that don't support namespaces, the first entry of the
+     vector will be nullptr, and the map will be empty.
+
+     A note on consistency. We can't make the IDs be consistent before
+     and after the initial relocation of the inferior (when the global
+     _r_debug is relocated, as mentioned in the previous comment).  It is
+     likely that this is a non-issue, since the inferior can't have called
+     dlmopen yet, but I think it is worth noting.
+
+     The only issue I am aware at this point is that, if when parsing an
+     XML file, we read an LMID that given by an XML file (and read in
+     library_list_start_library) is the identifier obtained with dlinfo
+     instead of the address of r_debug[_ext], and after attaching the
+     inferior adds another SO to that namespace, we might double-count it
+     since we won't have access to the LMID later on.  However, this is
+     already a problem with the existing solib_lists code.  */
+  std::vector<CORE_ADDR> namespace_id_vec;
+  std::unordered_map<CORE_ADDR, size_t> namespace_id_map;
 };
 
 /* Per-program-space data key.  */
 static const registry<program_space>::key<svr4_info> solib_svr4_pspace_data;
 
+/* Check if the lmid address is already assigned an ID in the svr4_info,
+   and if not, assign it one and add it to the list of known namespaces.  */
+static void
+svr4_maybe_add_namespace (svr4_info *info, CORE_ADDR lmid)
+{
+  if (info->namespace_id_map.count (lmid) > 0)
+    {
+      size_t i = info->namespace_id_map[lmid];
+      gdb_assert (info->namespace_id_map[i] == 0
+		  || info->namespace_id_vec[i] == lmid);
+      info->namespace_id_vec[i] = lmid;
+    }
+  else
+    {
+      /* If we reach this part, this is a new (to us) namespace, add it to the
+	 list.  */
+      info->namespace_id_map[lmid] = info->namespace_id_vec.size ();
+      info->namespace_id_vec.push_back (lmid);
+    }
+}
+
 /* Return whether DEBUG_BASE is the default namespace of INFO.  */
 
 static bool
@@ -1041,14 +1089,18 @@ library_list_start_library (struct gdb_xml_parser *parser,
   /* Older versions did not supply lmid.  Put the element into the flat
      list of the special namespace zero in that case.  */
   gdb_xml_value *at_lmid = xml_find_attribute (attributes, "lmid");
+  svr4_info *info = get_svr4_info (current_program_space);
   if (at_lmid == nullptr)
-    solist = list->cur_list;
+    {
+      solist = list->cur_list;
+      svr4_maybe_add_namespace (info, 0);
+    }
   else
     {
       ULONGEST lmid = *(ULONGEST *) at_lmid->value.get ();
       solist = &list->solib_lists[lmid];
+      svr4_maybe_add_namespace (info, lmid);
     }
-
   solist->emplace_back (name, std::move (li));
 }
 
@@ -1286,6 +1338,19 @@ svr4_current_sos_direct (struct svr4_info *info)
   /* Remove any old libraries.  We're going to read them back in again.  */
   info->solib_lists.clear ();
 
+  /* For namespace IDs, we can't just clear the array and re-start from
+     scratch.  That's because, if this full reading was triggered by a
+     dlclose call, which fully closed a namespace, we could end up changing
+     the numbers of other, still loaded, namespaces.
+
+     We instead just zero out all the vector values, to preemptively mark
+     them as deactivated, and then as we read the libraries we'll reactivate
+     the mappings.  */
+  for (size_t i = 0; i < info->namespace_id_vec.size (); i++)
+    {
+      info->namespace_id_vec[i] = 0;
+    }
+
   /* Fall back to manual examination of the target if the packet is not
      supported or gdbserver failed to find DT_DEBUG.  gdb.server/solib-list.exp
      tests a case where gdbserver cannot find the shared libraries list while
@@ -1333,7 +1398,11 @@ svr4_current_sos_direct (struct svr4_info *info)
     ignore_first = true;
 
   auto cleanup = make_scope_exit ([info] ()
-    { info->solib_lists.clear (); });
+    {
+      info->solib_lists.clear ();
+      info->namespace_id_vec.clear ();
+      info->namespace_id_map.clear ();
+    });
 
   /* Collect the sos in each namespace.  */
   CORE_ADDR debug_base = info->debug_base;
@@ -1343,8 +1412,11 @@ svr4_current_sos_direct (struct svr4_info *info)
       /* Walk the inferior's link map list, and build our so_list list.  */
       lm = solib_svr4_r_map (debug_base);
       if (lm != 0)
-	svr4_read_so_list (info, lm, 0, info->solib_lists[debug_base],
-			   ignore_first);
+	{
+	  svr4_maybe_add_namespace (info, debug_base);
+	  svr4_read_so_list (info, lm, 0, info->solib_lists[debug_base],
+			     ignore_first);
+	}
     }
 
   /* On Solaris, the dynamic linker is not in the normal list of
@@ -1361,8 +1433,11 @@ svr4_current_sos_direct (struct svr4_info *info)
     {
       /* Add the dynamic linker's namespace unless we already did.  */
       if (info->solib_lists.find (debug_base) == info->solib_lists.end ())
-	svr4_read_so_list (info, debug_base, 0, info->solib_lists[debug_base],
-			   0);
+	{
+	  svr4_maybe_add_namespace (info, debug_base);
+	  svr4_read_so_list (info, debug_base, 0, info->solib_lists[debug_base],
+			     0);
+	}
     }
 
   cleanup.release ();
@@ -1778,6 +1853,10 @@ solist_update_incremental (svr4_info *info, CORE_ADDR debug_base,
 	return 0;
 
       prev_lm = 0;
+
+      /* If the list is empty, we are seeing a new namespace for the
+	 first time, so assign it an internal ID.  */
+      svr4_maybe_add_namespace (info, debug_base);
     }
   else
     prev_lm = solist.back ().lm_info->lm_addr;
@@ -1845,6 +1924,8 @@ disable_probes_interface (svr4_info *info)
 
   free_probes_table (info);
   info->solib_lists.clear ();
+  info->namespace_id_vec.clear ();
+  info->namespace_id_map.clear ();
 }
 
 /* Update the solib list as appropriate when using the
@@ -3042,6 +3123,8 @@ svr4_solib_create_inferior_hook (int from_tty)
   /* Clear the probes-based interface's state.  */
   free_probes_table (info);
   info->solib_lists.clear ();
+  info->namespace_id_vec.clear ();
+  info->namespace_id_map.clear ();
 
   /* Relocate the main executable if necessary.  */
   svr4_relocate_main_executable ();
@@ -3361,6 +3444,26 @@ svr4_find_solib_addr (solib &so)
   return li->l_addr_inferior;
 }
 
+/* See solib_ops::find_solib_ns in solist.h.  */
+
+static int
+svr4_find_solib_ns (const solib &so)
+{
+  CORE_ADDR debug_base = find_debug_base_for_solib (&so);
+  svr4_info *info = get_svr4_info (current_program_space);
+  if (info->namespace_id_map.count (debug_base) == 1)
+    {
+      /* The only way for a mapping to be in the map but not
+	 in the vector is if the namespace was deactivated.  There
+	 should be no way for an SO to be loaded in a deactivated
+	 namespace.  */
+      gdb_assert
+	(info->namespace_id_vec[info->namespace_id_map[debug_base]] > 0);
+      return info->namespace_id_map[debug_base];
+    }
+  error (_("No namespace found"));
+}
+
 const struct solib_ops svr4_so_ops =
 {
   svr4_relocate_section_addresses,
@@ -3376,6 +3479,7 @@ const struct solib_ops svr4_so_ops =
   svr4_update_solib_event_breakpoints,
   svr4_handle_solib_event,
   svr4_find_solib_addr,
+  svr4_find_solib_ns,
 };
 
 void _initialize_svr4_solib ();
diff --git a/gdb/solib.c b/gdb/solib.c
index 7782c8d699e..6464075f319 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -1048,12 +1048,21 @@ info_sharedlibrary_command (const char *pattern, int from_tty)
 	}
     }
 
+  /* How many columns the table should have.  If the current arch
+     supports namespaces, we should add one for it.  */
+  int num_cols = 4;
+  const solib_ops *ops = gdbarch_so_ops (gdbarch);
+  if (ops->find_solib_ns != nullptr)
+    num_cols++;
+
   {
-    ui_out_emit_table table_emitter (uiout, 4, nr_libs, "SharedLibraryTable");
+    ui_out_emit_table table_emitter (uiout, num_cols, nr_libs, "SharedLibraryTable");
 
     /* The "- 1" is because ui_out adds one space between columns.  */
     uiout->table_header (addr_width - 1, ui_left, "from", "From");
     uiout->table_header (addr_width - 1, ui_left, "to", "To");
+    if (ops->find_solib_ns != nullptr)
+      uiout->table_header (5, ui_left, "namespace", "NS");
     uiout->table_header (12 - 1, ui_left, "syms-read", "Syms Read");
     uiout->table_header (0, ui_noalign, "name", "Shared Object Library");
 
@@ -1080,6 +1089,18 @@ info_sharedlibrary_command (const char *pattern, int from_tty)
 	    uiout->field_skip ("to");
 	  }
 
+	if (ops->find_solib_ns != nullptr)
+	  {
+	    try
+	      {
+		uiout->field_fmt ("namespace", "#%d", ops->find_solib_ns (so));
+	      }
+	    catch (const gdb_exception_error &er)
+	      {
+		uiout->field_skip ("namespace");
+	      }
+	  }
+
 	if (!top_level_interpreter ()->interp_ui_out ()->is_mi_like_p ()
 	    && so.symbols_loaded && !objfile_has_symbols (so.objfile))
 	  {
diff --git a/gdb/solist.h b/gdb/solist.h
index 9a157a4bbae..a6822877770 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -180,6 +180,17 @@ struct solib_ops
      name).  */
 
   std::optional<CORE_ADDR> (*find_solib_addr) (solib &so);
+
+  /* Return which linker namespace contains the current so.
+     If the linker or libc does not support linkage namespaces at all
+     (which is basically all of them but solib-svr4), this function should
+     be set to nullptr, so that "info shared" won't add an unnecessary
+     column.
+
+     If the namespace can not be determined (such as when we're stepping
+     though the dynamic linker), this function should throw a
+     gdb_exception_error.  */
+  int (*find_solib_ns) (const solib &so);
 };
 
 /* A unique pointer to a so_list.  */
diff --git a/gdb/testsuite/gdb.base/attach-pie-noexec.exp b/gdb/testsuite/gdb.base/attach-pie-noexec.exp
index 4e6ede12615..20c93b5599a 100644
--- a/gdb/testsuite/gdb.base/attach-pie-noexec.exp
+++ b/gdb/testsuite/gdb.base/attach-pie-noexec.exp
@@ -35,7 +35,7 @@ if ![runto_main] {
 }
 set test "sanity check info shared"
 gdb_test_multiple "info shared" $test {
-    -re "From\[ \t\]+To\[ \t\]+Syms Read\[ \t\]+Shared Object Library\r\n0x.*\r\n$gdb_prompt $" {
+    -re "From\[ \t\]+To(\\s+NS)?\[ \t\]+Syms Read\[ \t\]+Shared Object Library\r\n0x.*\r\n$gdb_prompt $" {
 	pass $test
     }
     -re "No shared libraries loaded at this time\\.\r\n$gdb_prompt $" {
@@ -62,6 +62,6 @@ if { ![gdb_attach $testpid] } {
     return
 }
 gdb_test "set architecture $arch" "The target architecture is set to \"$arch\"\\."
-gdb_test "info shared" "From\[ \t\]+To\[ \t\]+Syms Read\[ \t\]+Shared Object Library\r\n0x.*"
+gdb_test "info shared" "From\[ \t\]+To(\\s+NS)?\[ \t\]+Syms Read\[ \t\]+Shared Object Library\r\n0x.*"
 
 kill_wait_spawned_process $test_spawn_id
diff --git a/gdb/testsuite/gdb.base/dlmopen.exp b/gdb/testsuite/gdb.base/dlmopen.exp
index a8e3b08c016..f4882015f28 100644
--- a/gdb/testsuite/gdb.base/dlmopen.exp
+++ b/gdb/testsuite/gdb.base/dlmopen.exp
@@ -106,7 +106,7 @@ proc check_dso_count { dso num } {
 
     set count 0
     gdb_test_multiple "info shared" "info shared" {
-	-re "$hex  $hex  Yes \[^\r\n\]*$dso\r\n" {
+	-re "$hex  $hex  \(\#$::decimal\\s+\)\?Yes \[^\r\n\]*$dso\r\n" {
 	    # use longer form so debug remote does not interfere
 	    set count [expr $count + 1]
 	    exp_continue
@@ -233,10 +233,10 @@ proc get_dyld_info {} {
     set dyld_count 0
     set dyld_start_addr ""
     gdb_test_multiple "info sharedlibrary" "" {
-	-re "From\\s+To\\s+Syms\\s+Read\\s+Shared Object Library\r\n" {
+	-re "From\\s+To\\s+NS\\s+Syms\\s+Read\\s+Shared Object Library\r\n" {
 	    exp_continue
 	}
-	-re "^($::hex)\\s+$::hex\\s+\[^/\]+(/\[^\r\n\]+)\r\n" {
+	-re "^($::hex)\\s+$::hex\\s+\#$::decimal\\s+\[^/\]+(/\[^\r\n\]+)\r\n" {
 	    set addr $expect_out(1,string)
 	    set lib $expect_out(2,string)
 
diff --git a/gdb/testsuite/gdb.mi/mi-dlmopen.exp b/gdb/testsuite/gdb.mi/mi-dlmopen.exp
index a5743f83bb8..c0208ebcc51 100644
--- a/gdb/testsuite/gdb.mi/mi-dlmopen.exp
+++ b/gdb/testsuite/gdb.mi/mi-dlmopen.exp
@@ -81,12 +81,12 @@ proc get_dyld_info {} {
     set dyld_count 0
     set dyld_start_addr ""
     gdb_test_multiple "info sharedlibrary" "" {
-	-re "~\"From\\s+To\\s+Syms\\s+Read\\s+Shared Object Library\\\\n\"\r\n" {
+	-re "~\"From\\s+To(\\s+NS)?\\s+Syms\\s+Read\\s+Shared Object Library\\\\n\"\r\n" {
 	    exp_continue
 	}
-	-re "^~\"($::hex)\\s+$::hex\\s+\[^/\]+(/\[^\r\n\]+)\\\\n\"\r\n" {
+	-re "^~\"($::hex)\\s+${::hex}(\\s+$::decimal)?\\s+\[^/\]+(/\[^\r\n\]+)\\\\n\"\r\n" {
 	    set addr $expect_out(1,string)
-	    set lib $expect_out(2,string)
+	    set lib $expect_out(3,string)
 
 	    if { [is_dyln $lib] } {
 		# This looks like it might be the dynamic linker.
-- 
2.48.1