Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared"
@ 2025-03-13 17:00 Guinevere Larsen
  2025-03-13 19:19 ` Eli Zaretskii
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Guinevere Larsen @ 2025-03-13 17:00 UTC (permalink / raw)
  To: gdb-patches; +Cc: Guinevere Larsen

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-03-13 17:00 [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared" Guinevere Larsen
@ 2025-03-13 19:19 ` Eli Zaretskii
  2025-03-13 19:27   ` Guinevere Larsen
  2025-03-15  2:51 ` Kevin Buettner
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2025-03-13 19:19 UTC (permalink / raw)
  To: Guinevere Larsen; +Cc: gdb-patches

> From: Guinevere Larsen <guinevere@redhat.com>
> Cc: Guinevere Larsen <guinevere@redhat.com>
> Date: Thu, 13 Mar 2025 14:00:05 -0300
> 
>  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(-)

Thanks.

> 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
     ^^^^^^^^^^
"On systems"

> +In systems that support linkage namespaces, the column @code{NS} is
> +added to the output.

Same here, and in addition this sentence uses passive voice, so I'd
rephrase

 On systems that support linkage namespaces, the output includes an
 additional column @code{NS}.

The documentation parts are okay with these nits fixed.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-03-13 19:19 ` Eli Zaretskii
@ 2025-03-13 19:27   ` Guinevere Larsen
  0 siblings, 0 replies; 17+ messages in thread
From: Guinevere Larsen @ 2025-03-13 19:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 3/13/25 4:19 PM, Eli Zaretskii wrote:
>> From: Guinevere Larsen <guinevere@redhat.com>
>> Cc: Guinevere Larsen <guinevere@redhat.com>
>> Date: Thu, 13 Mar 2025 14:00:05 -0300
>>
>>   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(-)
> Thanks.
>
>> 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
>       ^^^^^^^^^^
> "On systems"
>
>> +In systems that support linkage namespaces, the column @code{NS} is
>> +added to the output.
> Same here, and in addition this sentence uses passive voice, so I'd
> rephrase
>
>   On systems that support linkage namespaces, the output includes an
>   additional column @code{NS}.
>
> The documentation parts are okay with these nits fixed.
>
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
>
Thanks for the review, I have applied the fixes locally!

-- 
Cheers,
Guinevere Larsen
She/Her/Hers


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-03-13 17:00 [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared" Guinevere Larsen
  2025-03-13 19:19 ` Eli Zaretskii
@ 2025-03-15  2:51 ` Kevin Buettner
  2025-03-15  3:11   ` Kevin Buettner
  2025-03-17 15:36 ` Simon Marchi
  2025-03-19 12:30 ` [PATCH v2] " Guinevere Larsen
  3 siblings, 1 reply; 17+ messages in thread
From: Kevin Buettner @ 2025-03-15  2:51 UTC (permalink / raw)
  To: Guinevere Larsen; +Cc: gdb-patches

Hi Gwen,

First, I'll note that I ran into a conflict in gdb/NEWS when applying
this commit when using "git am".  (It was easy to fix though.)

On Thu, 13 Mar 2025 14:00:05 -0300
Guinevere Larsen <guinevere@redhat.com> wrote:

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

I applied the patch, built GDB, and ran the tests.  But I didn't
see the new NS column in any of the "info shared"/"info sharedlibrary"
output.  Should I have seen this column?  (I.e, did I do something
wrong?)  Assuming that I didn't mess up, I think it'd be good to add
some tests which show the new column.

Kevin


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-03-15  2:51 ` Kevin Buettner
@ 2025-03-15  3:11   ` Kevin Buettner
  2025-03-17 11:55     ` Guinevere Larsen
  0 siblings, 1 reply; 17+ messages in thread
From: Kevin Buettner @ 2025-03-15  3:11 UTC (permalink / raw)
  To: Guinevere Larsen; +Cc: gdb-patches

On Fri, 14 Mar 2025 19:51:08 -0700
Kevin Buettner <kevinb@redhat.com> wrote:

> On Thu, 13 Mar 2025 14:00:05 -0300
> Guinevere Larsen <guinevere@redhat.com> wrote:
> 
> > 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.  
> 
> I applied the patch, built GDB, and ran the tests.  But I didn't
> see the new NS column in any of the "info shared"/"info sharedlibrary"
> output.  Should I have seen this column?  (I.e, did I do something
> wrong?)  Assuming that I didn't mess up, I think it'd be good to add
> some tests which show the new column.

So... I screwed up when attempting to resolve the "git am" problem.
I guess I need to figure out how to correctly use "git am --continue".

I now see the NS column in the "info shared" output.  But, now, I *always*
see it in the "info shared" output, even when there's only one linker
namespace.  Is that desirable?

My opinion is that we should suppress the NS column when there's
only one linker namespace.

Kevin


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-03-15  3:11   ` Kevin Buettner
@ 2025-03-17 11:55     ` Guinevere Larsen
  2025-03-17 15:36       ` Simon Marchi
  2025-03-18  1:07       ` Kevin Buettner
  0 siblings, 2 replies; 17+ messages in thread
From: Guinevere Larsen @ 2025-03-17 11:55 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

On 3/15/25 12:11 AM, Kevin Buettner wrote:
> On Fri, 14 Mar 2025 19:51:08 -0700
> Kevin Buettner <kevinb@redhat.com> wrote:
>
>> On Thu, 13 Mar 2025 14:00:05 -0300
>> Guinevere Larsen <guinevere@redhat.com> wrote:
>>
>>> 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.
>> I applied the patch, built GDB, and ran the tests.  But I didn't
>> see the new NS column in any of the "info shared"/"info sharedlibrary"
>> output.  Should I have seen this column?  (I.e, did I do something
>> wrong?)  Assuming that I didn't mess up, I think it'd be good to add
>> some tests which show the new column.
> So... I screwed up when attempting to resolve the "git am" problem.
> I guess I need to figure out how to correctly use "git am --continue".
>
> I now see the NS column in the "info shared" output.  But, now, I *always*
> see it in the "info shared" output, even when there's only one linker
> namespace.  Is that desirable?
I did that on purpose, as long as you're using solib-svr4 (which is the 
only namespace-capable solib handler that I'm aware of) you'd see the 
column. My reasoning was that when I didn't see part of an output that i 
didn't understand, I personally would just ignore it, but I wasn't sure 
my approach was the best.
>
> My opinion is that we should suppress the NS column when there's
> only one linker namespace.

I can give this a shot, but in my head there are 2 ways of implementing it:

1. If there is more than one linker namespace *active* when the command 
is run;
2. If there is more than one linker namespace *registered* when the 
command is run;

The main difference is: if the inferior has opened an SO in a namespace, 
but has since closed it and everything loaded is in the default 
namespace, should we still have the NS column? The first is closer to a 
future plan of having an "active namespaces" convenience variable, while 
the second is brand new code that I don't expect to use anywhere else, 
but I can see an argument for it, so which do you prefer?

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
> Kevin
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-03-17 11:55     ` Guinevere Larsen
@ 2025-03-17 15:36       ` Simon Marchi
  2025-03-18  1:07       ` Kevin Buettner
  1 sibling, 0 replies; 17+ messages in thread
From: Simon Marchi @ 2025-03-17 15:36 UTC (permalink / raw)
  To: Guinevere Larsen, Kevin Buettner; +Cc: gdb-patches

On 3/17/25 7:55 AM, Guinevere Larsen wrote:
> I can give this a shot, but in my head there are 2 ways of implementing it:
> 
> 1. If there is more than one linker namespace *active* when the command is run;
> 2. If there is more than one linker namespace *registered* when the command is run;
> 
> The main difference is: if the inferior has opened an SO in a namespace, but has since closed it and everything loaded is in the default namespace, should we still have the NS column? The first is closer to a future plan of having an "active namespaces" convenience variable, while the second is brand new code that I don't expect to use anywhere else, but I can see an argument for it, so which do you prefer?

I don't know much about linker namespaces, so I don't know which option
is better, but I agree that not showing the column when the program
doesn't deal with non-default namespaces would be nice.  We do something
similar for instance with info threads: if there is a single inferior,
we don't show the inferior number in the thread id, otherwise we do.

Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-03-13 17:00 [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared" Guinevere Larsen
  2025-03-13 19:19 ` Eli Zaretskii
  2025-03-15  2:51 ` Kevin Buettner
@ 2025-03-17 15:36 ` Simon Marchi
  2025-03-17 17:07   ` Guinevere Larsen
  2025-03-19 12:30 ` [PATCH v2] " Guinevere Larsen
  3 siblings, 1 reply; 17+ messages in thread
From: Simon Marchi @ 2025-03-17 15:36 UTC (permalink / raw)
  To: Guinevere Larsen, gdb-patches

I didn't do a full review, I can review more in depth a future version
(since you'll probably change the behavior of the "info shared" column
anyway).  I noted some high level things:

On 3/13/25 1:00 PM, Guinevere Larsen wrote:
> 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;

Since the count of namespaces is always expected to be small, you could
consider just maintaining just a vector.  For example, glibc only
supports a max of 16 namespaces today.  Maintaining one data structure
is usually easier than maintaining two in parallel.

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

"the current so", or just "SO" (... which linker namespace contains 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.

I would not put a statement like "which is basically all of them but
solib" in the interface here.  It is true until it isn't.  I think it's
fine to sometimes mention a specific implementation in the interface
documentation to explain why a certain method exists or to give an
example, which helps the reader understand what the method does.  But in
this case I don't think mentioning solib-svr4 helps understand what
find_solib_ns does.

Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-03-17 15:36 ` Simon Marchi
@ 2025-03-17 17:07   ` Guinevere Larsen
  2025-03-17 17:54     ` Simon Marchi
  0 siblings, 1 reply; 17+ messages in thread
From: Guinevere Larsen @ 2025-03-17 17:07 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 3/17/25 12:36 PM, Simon Marchi wrote:
> I didn't do a full review, I can review more in depth a future version
> (since you'll probably change the behavior of the "info shared" column
> anyway).  I noted some high level things:
>
> On 3/13/25 1:00 PM, Guinevere Larsen wrote:
>> 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;
> Since the count of namespaces is always expected to be small, you could
> consider just maintaining just a vector.  For example, glibc only
> supports a max of 16 namespaces today.  Maintaining one data structure
> is usually easier than maintaining two in parallel.

The issue here is handling dlclose calls. When svr4 detects a dlclose 
call, it will clear the solib_lists object, and read all the SOs again. 
So if the dlclose call has deactivated a namespace, we don't have an 
easy way to identify which namespaces were deactivated, and the way to 
handle this is to deactivate them all and reactivate them in 
svr4_maybe_add_namespace.

And we need all the data to reactivate namespaces to be stored in 
svr4_info, because if the target is a gdbserver, we're reading 
everything through svr4_current_sos_via_xfer_libraries, and it doesn't 
sound like a good idea to modify the xml parsing code to pass some 
context that would only be useful for solib-svr4.

While writing this, I realized I might be able to simplify this, having 
just a set of namespace IDs that are active, which will make the 
synchronization much simpler.

>
>> 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.
> "the current so", or just "SO" (... which linker namespace contains 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.
> I would not put a statement like "which is basically all of them but
> solib" in the interface here.  It is true until it isn't.  I think it's
> fine to sometimes mention a specific implementation in the interface
> documentation to explain why a certain method exists or to give an
> example, which helps the reader understand what the method does.  But in
> this case I don't think mentioning solib-svr4 helps understand what
> find_solib_ns does.
>
> Simon
>
I'll fix these other comments.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-03-17 17:07   ` Guinevere Larsen
@ 2025-03-17 17:54     ` Simon Marchi
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Marchi @ 2025-03-17 17:54 UTC (permalink / raw)
  To: Guinevere Larsen, gdb-patches

> The issue here is handling dlclose calls. When svr4 detects a dlclose
> call, it will clear the solib_lists object, and read all the SOs
> again. So if the dlclose call has deactivated a namespace, we don't
> have an easy way to identify which namespaces were deactivated, and
> the way to handle this is to deactivate them all and reactivate them
> in svr4_maybe_add_namespace.
> 
> And we need all the data to reactivate namespaces to be stored in
> svr4_info, because if the target is a gdbserver, we're reading
> everything through svr4_current_sos_via_xfer_libraries, and it doesn't
> sound like a good idea to modify the xml parsing code to pass some
> context that would only be useful for solib-svr4.
> 
> While writing this, I realized I might be able to simplify this,
> having just a set of namespace IDs that are active, which will make
> the synchronization much simpler.

Ok you lost me there :).  I'll see in the next patch version.

Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-03-17 11:55     ` Guinevere Larsen
  2025-03-17 15:36       ` Simon Marchi
@ 2025-03-18  1:07       ` Kevin Buettner
  1 sibling, 0 replies; 17+ messages in thread
From: Kevin Buettner @ 2025-03-18  1:07 UTC (permalink / raw)
  To: Guinevere Larsen; +Cc: gdb-patches

On Mon, 17 Mar 2025 08:55:25 -0300
Guinevere Larsen <guinevere@redhat.com> wrote:

> > My opinion is that we should suppress the NS column when there's
> > only one linker namespace.  
> 
> I can give this a shot, but in my head there are 2 ways of implementing it:
> 
> 1. If there is more than one linker namespace *active* when the command 
> is run;
> 2. If there is more than one linker namespace *registered* when the 
> command is run;
> 
> The main difference is: if the inferior has opened an SO in a namespace, 
> but has since closed it and everything loaded is in the default 
> namespace, should we still have the NS column? The first is closer to a 
> future plan of having an "active namespaces" convenience variable, while 
> the second is brand new code that I don't expect to use anywhere else, 
> but I can see an argument for it, so which do you prefer?

I prefer option 1, in which the NS column is shown when there is more than
one active namespace.

Kevin


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-03-13 17:00 [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared" Guinevere Larsen
                   ` (2 preceding siblings ...)
  2025-03-17 15:36 ` Simon Marchi
@ 2025-03-19 12:30 ` Guinevere Larsen
  2025-03-21 17:55   ` Guinevere Larsen
  2025-03-27 17:13   ` [PATCH v3] " Guinevere Larsen
  3 siblings, 2 replies; 17+ messages in thread
From: Guinevere Larsen @ 2025-03-19 12:30 UTC (permalink / raw)
  To: gdb-patches; +Cc: Guinevere Larsen

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 via a vector inside svr4_info
object. The vector stores the address of the r_debug objects used by
glibc to identify each namespace, and the user-friendly ID is the index
of the r_debug in the vector. This commit also introduces a set storing
the indices of active namespaces. The glibc I used to develop this patch
(glibc 2.40 on Fedora 41) doesn't allow an SO to be loaded into a
deactivated namespace, and requesting a new namespace when a namespace
was previously closed will reuse that namespace. Because of how this is
implemented, this patch lets GDB easily track the exact namespace IDs
that the inferior will see.

Finally, two new solib_ops function pointers were added, find_solib_ns
and num_active_namespaces, to allow code outside of solib-svr4 to find
and use the namespace identifiers and the number of namespaces,
respectively. As a sanity check, the command `info sharedlibrary` has
been changed to display the namespace identifier when the inferior has
more than one active namespace. With this final change, a couple of tests
had to be tweaked to handle the possible new column, and a new test has
been created to make sure that the column appears and disappears as
needed, and that GDB can track the value of the LMID for namespaces.

---

Changes for v2:
* made it so the new column in "info shared" only shows up when multiple
  namespaces are active
* changes NEWS and docs to reflect that
* Added a test for this functionality

---
 gdb/NEWS                                     |   5 +
 gdb/doc/gdb.texinfo                          |   5 +
 gdb/solib-svr4.c                             | 108 +++++++++++++++++--
 gdb/solib.c                                  |  27 ++++-
 gdb/solist.h                                 |  14 +++
 gdb/testsuite/gdb.base/attach-pie-noexec.exp |   4 +-
 gdb/testsuite/gdb.base/dlmopen-ns-ids-lib.c  |  28 +++++
 gdb/testsuite/gdb.base/dlmopen-ns-ids-main.c |  54 ++++++++++
 gdb/testsuite/gdb.base/dlmopen-ns-ids.exp    | 108 +++++++++++++++++++
 gdb/testsuite/gdb.base/dlmopen.exp           |   8 +-
 gdb/testsuite/gdb.mi/mi-dlmopen.exp          |   6 +-
 11 files changed, 350 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/dlmopen-ns-ids-lib.c
 create mode 100644 gdb/testsuite/gdb.base/dlmopen-ns-ids-main.c
 create mode 100644 gdb/testsuite/gdb.base/dlmopen-ns-ids.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 0aac7a7b13a..33918b142de 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -31,6 +31,11 @@
   a -h or --help option, which prints each options and a brief
   description.
 
+* On systems that support linkage namespaces, the output of the command
+  "info sharedlibraries" may add one more column, NS, which identifies the
+  namespace into which the library was loaded, if more than one namespace
+  is active.
+
 * New commands
 
 maintenance check psymtabs
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 473431011d1..2350d3a238f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22166,6 +22166,11 @@ 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.
 
+On systems that support linkage namespaces, the output includes an
+additional column @code{NS} if the inferior has more than one active
+namespace when the command is used.  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..c631eb4c770 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -405,11 +405,54 @@ 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.  A vector is used to make it
+     easy to assign new internal IDs to namespaces.
+
+     For gdbservers that don't support namespaces, the first (and only)
+     entry of the vector will be 0.
+
+     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;
+
+  /* This identifies which namespaces are active.  A namespace is considered
+     active when there is at least one shared object loaded into it.  */
+  std::set<size_t> active_namespaces;
 };
 
 /* 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)
+{
+  int i;
+  for (i = 0; i < info->namespace_id.size (); i++)
+    {
+      if (info->namespace_id[i] == lmid)
+	break;
+    }
+  if (i == info->namespace_id.size ())
+    info->namespace_id.push_back (lmid);
+
+  info->active_namespaces.insert (i);
+}
+
 /* Return whether DEBUG_BASE is the default namespace of INFO.  */
 
 static bool
@@ -1041,14 +1084,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 +1333,8 @@ 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 ();
 
+  info->active_namespaces.clear ();
+
   /* 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 +1382,10 @@ 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->active_namespaces.clear ();
+    });
 
   /* Collect the sos in each namespace.  */
   CORE_ADDR debug_base = info->debug_base;
@@ -1343,8 +1395,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 +1416,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 +1836,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 +1907,8 @@ disable_probes_interface (svr4_info *info)
 
   free_probes_table (info);
   info->solib_lists.clear ();
+  info->namespace_id.clear ();
+  info->active_namespaces.clear ();
 }
 
 /* Update the solib list as appropriate when using the
@@ -3042,6 +3106,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.clear ();
+  info->active_namespaces.clear ();
 
   /* Relocate the main executable if necessary.  */
   svr4_relocate_main_executable ();
@@ -3361,6 +3427,32 @@ 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);
+  for (int i = 0; i < info->namespace_id.size (); i++)
+    {
+      if (info->namespace_id[i] == debug_base)
+	{
+	  gdb_assert (info->active_namespaces.count (i) == 1);
+	  return i;
+	}
+    }
+  error (_("No namespace found"));
+}
+
+/* see solib_ops::num_active_namespaces in solist.h.  */
+static int
+svr4_num_active_namespaces ()
+{
+  svr4_info *info = get_svr4_info (current_program_space);
+  return info->active_namespaces.size ();
+}
+
 const struct solib_ops svr4_so_ops =
 {
   svr4_relocate_section_addresses,
@@ -3376,6 +3468,8 @@ const struct solib_ops svr4_so_ops =
   svr4_update_solib_event_breakpoints,
   svr4_handle_solib_event,
   svr4_find_solib_addr,
+  svr4_find_solib_ns,
+  svr4_num_active_namespaces,
 };
 
 void _initialize_svr4_solib ();
diff --git a/gdb/solib.c b/gdb/solib.c
index 7782c8d699e..32dfa887ac7 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -1048,12 +1048,24 @@ info_sharedlibrary_command (const char *pattern, int from_tty)
 	}
     }
 
+  /* How many columns the table should have.  If the inferior has
+     more than one namespace active, we need a column to show that.  */
+  int num_cols = 4;
+  const solib_ops *ops = gdbarch_so_ops (gdbarch);
+  if (ops->num_active_namespaces != nullptr
+      && ops->num_active_namespaces () > 1)
+    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->num_active_namespaces != nullptr
+	&& ops->num_active_namespaces () > 1)
+      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 +1092,19 @@ info_sharedlibrary_command (const char *pattern, int from_tty)
 	    uiout->field_skip ("to");
 	  }
 
+	if (ops->num_active_namespaces != nullptr
+	    && ops->num_active_namespaces ()> 1)
+	  {
+	    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..03d2392b19d 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -180,6 +180,20 @@ 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);
+
+  /* Returns the number of active namespaces in the inferior.  */
+  int (*num_active_namespaces) ();
 };
 
 /* 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-ns-ids-lib.c b/gdb/testsuite/gdb.base/dlmopen-ns-ids-lib.c
new file mode 100644
index 00000000000..86cbb0f4e38
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dlmopen-ns-ids-lib.c
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2025 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+int gdb_dlmopen_glob = 0;
+
+__attribute__((visibility ("default")))
+int
+inc (int n)
+{
+  int amount = gdb_dlmopen_glob;
+  return n + amount;  /* bp.inc.  */
+}
diff --git a/gdb/testsuite/gdb.base/dlmopen-ns-ids-main.c b/gdb/testsuite/gdb.base/dlmopen-ns-ids-main.c
new file mode 100644
index 00000000000..3bcd8196483
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dlmopen-ns-ids-main.c
@@ -0,0 +1,54 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2025 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+#define _GNU_SOURCE
+#include <dlfcn.h>
+#include <stddef.h>
+#include <assert.h>
+#include <unistd.h>
+#include <stdio.h>
+
+int
+main (void)
+{
+  void *handle[4];
+  int (*fun) (int);
+  Lmid_t lmid;
+  int dl;
+
+  handle[0] = dlmopen (LM_ID_NEWLM, DSO_NAME, RTLD_LAZY | RTLD_LOCAL);
+  assert (handle[0] != NULL);
+
+  handle[1] = dlmopen (LM_ID_NEWLM, DSO_NAME, RTLD_LAZY | RTLD_LOCAL);
+  assert (handle[1] != NULL);
+
+  handle[2] = dlmopen (LM_ID_NEWLM, DSO_NAME, RTLD_LAZY | RTLD_LOCAL);
+  assert (handle[2] != NULL);
+
+  dlclose (handle[0]); /* TAG: first dlclose */
+  dlclose (handle[1]); /* TAG: second dlclose */
+  dlclose (handle[2]); /* TAG: third dlclose */
+
+  handle[3] = dlmopen (LM_ID_NEWLM, DSO_NAME, RTLD_LAZY | RTLD_LOCAL);
+  dlinfo (handle[3], RTLD_DI_LMID, &lmid);
+
+  dlclose (handle[3]); /* TAG: fourth dlclose */
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
new file mode 100644
index 00000000000..890ee091be9
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
@@ -0,0 +1,108 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2025 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+#
+# Test several things related to handling linker namespaces:
+# * That the user-facing namespace ID is consistent;
+
+require allow_dlmopen_tests
+
+standard_testfile -main.c -lib.c
+
+set srcfile_lib $srcfile2
+set binfile_lib [standard_output_file dlmopen-lib.so]
+
+if { [build_executable "build shlib" $binfile_lib $srcfile_lib \
+	  [list debug shlib]] == -1 } {
+    return
+}
+
+if { [build_executable "failed to build" $testfile $srcfile \
+	  [list additional_flags=-DDSO_NAME=\"$binfile_lib\" \
+	       shlib_load debug]] } {
+    return
+}
+
+# Run the command "info sharedlibrary" and get the first namespace
+# for the so
+proc get_first_so_ns {} {
+    set ns -1
+    gdb_test_multiple "info sharedlibrary" "get SO namespace" -lbl {
+	-re "From\\s+To\\s+\(NS\\s+\)?Syms\\s+Read\\s+Shared Object Library\r\n" {
+	    exp_continue
+	}
+	-re "^$::hex\\s+$::hex\\s+\#($::decimal)\\s+\[^\r\n]+$::binfile_lib.*" {
+	    set ns $expect_out(1,string)
+	}
+	-re "^$::gdb_prompt $" {
+	}
+	-re "^\[^\r\n\]+\r\n" {
+	    exp_continue
+	}
+    }
+    return $ns
+}
+
+# Run the tests relating to the command "info sharedlibrary", to
+# verify that the namespace ID is consistent.
+proc test_info_shared {} {
+    clean_restart $::binfile
+
+    if { ![runto_main] } {
+	return
+    }
+
+    # First test that we don't print a namespace column at the start.
+    gdb_test "info sharedlibrary" \
+	"From\\s+To\\s+Syms\\s+Read\\s+Shared Object Library.*" \
+	"before loading anything"
+
+    gdb_test_no_output "set wait_for_gdb = 0"
+
+    gdb_breakpoint [gdb_get_line_number "TAG: first dlclose"]
+    gdb_continue_to_breakpoint "TAG: first dlclose"
+
+    # Next, test that we *do* print a namespace column after loading SOs.
+    gdb_test "info sharedlibrary" \
+	"From\\s+To\\s+NS\\s+Syms\\s+Read\\s+Shared Object Library.*" \
+	"after loading everything"
+
+    gdb_assert {[get_first_so_ns] == 1} "before closing any library"
+
+    gdb_test "next" ".*second dlclose.*" "close first library"
+    gdb_assert {[get_first_so_ns] == 2} "after closing one library"
+
+    gdb_test "next" ".*third dlclose.*" "close second library"
+    gdb_assert {[get_first_so_ns] == 3} "before closing two libraries"
+
+    gdb_breakpoint [gdb_get_line_number "TAG: fourth dlclose"]
+    gdb_continue_to_breakpoint "TAG: fourth dlclose"
+    # As of writing this test, glibc's LMID is just an index on an array of
+    # namespaces.  After closing a namespace, requesting a new one will
+    # return the index of the lowest-closed namespace, so this will likely
+    # be namespace 1, and because of glibc's reuse of the r_debug object,
+    # GDB should be able to assign the same number.
+    gdb_assert {[get_first_so_ns] == [get_integer_valueof "lmid" "-1"]} \
+	"reopen a namespace"
+
+    gdb_test "next" ".*return 0.*" "final namespace inactive"
+    gdb_test "info sharedlibrary" \
+	"From\\s+To\\s+Syms\\s+Read\\s+Shared Object Library.*" \
+	"after unloading everything"
+}
+
+test_info_shared
diff --git a/gdb/testsuite/gdb.base/dlmopen.exp b/gdb/testsuite/gdb.base/dlmopen.exp
index a8e3b08c016..6f8a3bc9de4 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,12 +233,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\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)
+	    set lib $expect_out(3,string)
 
 	    if { [is_dyln $lib] } {
 		# This looks like it might be the dynamic linker.
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.

base-commit: 82455bdab80f9a4d4f94a6e8590a1bdc58c4010e
-- 
2.48.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-03-19 12:30 ` [PATCH v2] " Guinevere Larsen
@ 2025-03-21 17:55   ` Guinevere Larsen
  2025-03-27 17:13   ` [PATCH v3] " Guinevere Larsen
  1 sibling, 0 replies; 17+ messages in thread
From: Guinevere Larsen @ 2025-03-21 17:55 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 9646 bytes --]

On 3/19/25 9:30 AM, Guinevere Larsen wrote:
> 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.

While doing some more development on the next patch for this series, it 
occurred to me that we also identify frames with #N, so this would be 
overlap of the meaning of #. This will eventually be somewhat confusing 
as we could possibly have the following:

(gdb) bt
#0  #1::roll () at random.c:6
#1  0x00000000004011b8 in #1::hit (chance=0, acc_boost=0, 
evasion_boost=0) at random.c:9
#2  0x000000000040126c in #0::main () at random.c:20

My question is: is this confusing enough that we should figure out a 
different syntax? and if so, what other invalid character should I use 
for a namespace?

Also I have an unrelated note about my patch inlined.

>
> The namespace identifiers are stored via a vector inside svr4_info
> object. The vector stores the address of the r_debug objects used by
> glibc to identify each namespace, and the user-friendly ID is the index
> of the r_debug in the vector. This commit also introduces a set storing
> the indices of active namespaces. The glibc I used to develop this patch
> (glibc 2.40 on Fedora 41) doesn't allow an SO to be loaded into a
> deactivated namespace, and requesting a new namespace when a namespace
> was previously closed will reuse that namespace. Because of how this is
> implemented, this patch lets GDB easily track the exact namespace IDs
> that the inferior will see.
>
> Finally, two new solib_ops function pointers were added, find_solib_ns
> and num_active_namespaces, to allow code outside of solib-svr4 to find
> and use the namespace identifiers and the number of namespaces,
> respectively. As a sanity check, the command `info sharedlibrary` has
> been changed to display the namespace identifier when the inferior has
> more than one active namespace. With this final change, a couple of tests
> had to be tweaked to handle the possible new column, and a new test has
> been created to make sure that the column appears and disappears as
> needed, and that GDB can track the value of the LMID for namespaces.
>
> ---
<snipping for convenience>
> diff --git a/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
> new file mode 100644
> index 00000000000..890ee091be9
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
> @@ -0,0 +1,108 @@
> +# This testcase is part of GDB, the GNU debugger.
> +#
> +# Copyright 2025 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see<http://www.gnu.org/licenses/>.
> +#
> +#
> +# Test several things related to handling linker namespaces:
> +# * That the user-facing namespace ID is consistent;
> +
> +require allow_dlmopen_tests
> +
> +standard_testfile -main.c -lib.c
> +
> +set srcfile_lib $srcfile2
> +set binfile_lib [standard_output_file dlmopen-lib.so]
> +
> +if { [build_executable "build shlib" $binfile_lib $srcfile_lib \
> +	  [list debug shlib]] == -1 } {
> +    return
> +}
> +
> +if { [build_executable "failed to build" $testfile $srcfile \
> +	  [list additional_flags=-DDSO_NAME=\"$binfile_lib\" \
> +	       shlib_load debug]] } {
> +    return
> +}
> +
> +# Run the command "info sharedlibrary" and get the first namespace
> +# for the so
> +proc get_first_so_ns {} {
> +    set ns -1
> +    gdb_test_multiple "info sharedlibrary" "get SO namespace" -lbl {
> +	-re "From\\s+To\\s+\(NS\\s+\)?Syms\\s+Read\\s+Shared Object Library\r\n" {
> +	    exp_continue
> +	}
> +	-re "^$::hex\\s+$::hex\\s+\#($::decimal)\\s+\[^\r\n]+$::binfile_lib.*" {
> +	    set ns $expect_out(1,string)
> +	}
> +	-re "^$::gdb_prompt $" {
> +	}
> +	-re "^\[^\r\n\]+\r\n" {
> +	    exp_continue
> +	}
> +    }
> +    return $ns
> +}
> +
> +# Run the tests relating to the command "info sharedlibrary", to
> +# verify that the namespace ID is consistent.
> +proc test_info_shared {} {
> +    clean_restart $::binfile
> +
> +    if { ![runto_main] } {
> +	return
> +    }
> +
> +    # First test that we don't print a namespace column at the start.
> +    gdb_test "info sharedlibrary" \
> +	"From\\s+To\\s+Syms\\s+Read\\s+Shared Object Library.*" \
> +	"before loading anything"
> +
> +    gdb_test_no_output "set wait_for_gdb = 0"

This is leftover from an earlier version where I thought about using 
attach, but decided against it since dlmopen already tests it.

I've removed this from my local branch

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

> +
> +    gdb_breakpoint [gdb_get_line_number "TAG: first dlclose"]
> +    gdb_continue_to_breakpoint "TAG: first dlclose"
> +
> +    # Next, test that we *do* print a namespace column after loading SOs.
> +    gdb_test "info sharedlibrary" \
> +	"From\\s+To\\s+NS\\s+Syms\\s+Read\\s+Shared Object Library.*" \
> +	"after loading everything"
> +
> +    gdb_assert {[get_first_so_ns] == 1} "before closing any library"
> +
> +    gdb_test "next" ".*second dlclose.*" "close first library"
> +    gdb_assert {[get_first_so_ns] == 2} "after closing one library"
> +
> +    gdb_test "next" ".*third dlclose.*" "close second library"
> +    gdb_assert {[get_first_so_ns] == 3} "before closing two libraries"
> +
> +    gdb_breakpoint [gdb_get_line_number "TAG: fourth dlclose"]
> +    gdb_continue_to_breakpoint "TAG: fourth dlclose"
> +    # As of writing this test, glibc's LMID is just an index on an array of
> +    # namespaces.  After closing a namespace, requesting a new one will
> +    # return the index of the lowest-closed namespace, so this will likely
> +    # be namespace 1, and because of glibc's reuse of the r_debug object,
> +    # GDB should be able to assign the same number.
> +    gdb_assert {[get_first_so_ns] == [get_integer_valueof "lmid" "-1"]} \
> +	"reopen a namespace"
> +
> +    gdb_test "next" ".*return 0.*" "final namespace inactive"
> +    gdb_test "info sharedlibrary" \
> +	"From\\s+To\\s+Syms\\s+Read\\s+Shared Object Library.*" \
> +	"after unloading everything"
> +}
> +
> +test_info_shared
> diff --git a/gdb/testsuite/gdb.base/dlmopen.exp b/gdb/testsuite/gdb.base/dlmopen.exp
> index a8e3b08c016..6f8a3bc9de4 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,12 +233,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\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)
> +	    set lib $expect_out(3,string)
>   
>   	    if { [is_dyln $lib] } {
>   		# This looks like it might be the dynamic linker.
> 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.
>
> base-commit: 82455bdab80f9a4d4f94a6e8590a1bdc58c4010e

[-- Attachment #2: Type: text/html, Size: 10393 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-03-19 12:30 ` [PATCH v2] " Guinevere Larsen
  2025-03-21 17:55   ` Guinevere Larsen
@ 2025-03-27 17:13   ` Guinevere Larsen
  2025-03-31 19:34     ` Guinevere Larsen
  2025-04-05 20:17     ` Kevin Buettner
  1 sibling, 2 replies; 17+ messages in thread
From: Guinevere Larsen @ 2025-03-27 17:13 UTC (permalink / raw)
  To: gdb-patches; +Cc: Guinevere Larsen

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.

This syntax was chosen based on the C attributes, since nothing in GDB
uses a similar syntax that could confuse users. Other syntax options
that were explored were "#<number>" and "@<number>". The former was
abandoned because when printing a frame, the frame number is also
printed with #<number>, so in a lot of the context in which that the
identifier would show up, it appears in a confusing way. The latter
clashes with the array printing syntax, and I believe that the having
"@N::foo" working completely differently to "foo@2" would also lead to a
bad user experience.

The namespace identifiers are stored via a vector inside svr4_info
object. The vector stores the address of the r_debug objects used by
glibc to identify each namespace, and the user-friendly ID is the index
of the r_debug in the vector. This commit also introduces a set storing
the indices of active namespaces. The glibc I used to develop this patch
(glibc 2.40 on Fedora 41) doesn't allow an SO to be loaded into a
deactivated namespace, and requesting a new namespace when a namespace
was previously closed will reuse that namespace. Because of how this is
implemented, this patch lets GDB easily track the exact namespace IDs
that the inferior will see.

Finally, two new solib_ops function pointers were added, find_solib_ns
and num_active_namespaces, to allow code outside of solib-svr4 to find
and use the namespace identifiers and the number of namespaces,
respectively. As a sanity check, the command `info sharedlibrary` has
been changed to display the namespace identifier when the inferior has
more than one active namespace. With this final change, a couple of tests
had to be tweaked to handle the possible new column, and a new test has
been created to make sure that the column appears and disappears as
needed, and that GDB can track the value of the LMID for namespaces.

---
Changes for v3:
* Changed the namespace identifier display from #N to [[N]]

Changes for v2:
* made it so the new column in "info shared" only shows up when multiple
  namespaces are active
* changes NEWS and docs to reflect that
* Added a test for this functionality

---
 gdb/NEWS                                     |   5 +
 gdb/doc/gdb.texinfo                          |   5 +
 gdb/solib-svr4.c                             | 108 +++++++++++++++++--
 gdb/solib.c                                  |  27 ++++-
 gdb/solist.h                                 |  14 +++
 gdb/testsuite/gdb.base/attach-pie-noexec.exp |   4 +-
 gdb/testsuite/gdb.base/dlmopen-ns-ids-lib.c  |  28 +++++
 gdb/testsuite/gdb.base/dlmopen-ns-ids-main.c |  54 ++++++++++
 gdb/testsuite/gdb.base/dlmopen-ns-ids.exp    | 106 ++++++++++++++++++
 gdb/testsuite/gdb.base/dlmopen.exp           |   8 +-
 gdb/testsuite/gdb.mi/mi-dlmopen.exp          |   6 +-
 11 files changed, 348 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/dlmopen-ns-ids-lib.c
 create mode 100644 gdb/testsuite/gdb.base/dlmopen-ns-ids-main.c
 create mode 100644 gdb/testsuite/gdb.base/dlmopen-ns-ids.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 2fdb849a81b..77fe7a16997 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -31,6 +31,11 @@
   a -h or --help option, which prints each options and a brief
   description.
 
+* On systems that support linkage namespaces, the output of the command
+  "info sharedlibraries" may add one more column, NS, which identifies the
+  namespace into which the library was loaded, if more than one namespace
+  is active.
+
 * New commands
 
 maintenance check psymtabs
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e034ac53295..4e4509a4649 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22172,6 +22172,11 @@ be determined then the address range for the @code{.text} section from
 the library will be listed.  If the @code{.text} section cannot be
 found then no addresses will be listed.
 
+On systems that support linkage namespaces, the output includes an
+additional column @code{NS} if the inferior has more than one active
+namespace when the command is used.  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 398123f7a52..2f839bd7b0b 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -405,11 +405,54 @@ 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.  A vector is used to make it
+     easy to assign new internal IDs to namespaces.
+
+     For gdbservers that don't support namespaces, the first (and only)
+     entry of the vector will be 0.
+
+     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;
+
+  /* This identifies which namespaces are active.  A namespace is considered
+     active when there is at least one shared object loaded into it.  */
+  std::set<size_t> active_namespaces;
 };
 
 /* 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)
+{
+  int i;
+  for (i = 0; i < info->namespace_id.size (); i++)
+    {
+      if (info->namespace_id[i] == lmid)
+	break;
+    }
+  if (i == info->namespace_id.size ())
+    info->namespace_id.push_back (lmid);
+
+  info->active_namespaces.insert (i);
+}
+
 /* Return whether DEBUG_BASE is the default namespace of INFO.  */
 
 static bool
@@ -1041,14 +1084,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 +1333,8 @@ 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 ();
 
+  info->active_namespaces.clear ();
+
   /* 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 +1382,10 @@ 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->active_namespaces.clear ();
+    });
 
   /* Collect the sos in each namespace.  */
   CORE_ADDR debug_base = info->debug_base;
@@ -1343,8 +1395,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 +1416,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 +1836,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 +1907,8 @@ disable_probes_interface (svr4_info *info)
 
   free_probes_table (info);
   info->solib_lists.clear ();
+  info->namespace_id.clear ();
+  info->active_namespaces.clear ();
 }
 
 /* Update the solib list as appropriate when using the
@@ -3042,6 +3106,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.clear ();
+  info->active_namespaces.clear ();
 
   /* Relocate the main executable if necessary.  */
   svr4_relocate_main_executable ();
@@ -3460,6 +3526,32 @@ 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);
+  for (int i = 0; i < info->namespace_id.size (); i++)
+    {
+      if (info->namespace_id[i] == debug_base)
+	{
+	  gdb_assert (info->active_namespaces.count (i) == 1);
+	  return i;
+	}
+    }
+  error (_("No namespace found"));
+}
+
+/* see solib_ops::num_active_namespaces in solist.h.  */
+static int
+svr4_num_active_namespaces ()
+{
+  svr4_info *info = get_svr4_info (current_program_space);
+  return info->active_namespaces.size ();
+}
+
 const struct solib_ops svr4_so_ops =
 {
   svr4_relocate_section_addresses,
@@ -3475,6 +3567,8 @@ const struct solib_ops svr4_so_ops =
   svr4_update_solib_event_breakpoints,
   svr4_handle_solib_event,
   svr4_find_solib_addr,
+  svr4_find_solib_ns,
+  svr4_num_active_namespaces,
 };
 
 void _initialize_svr4_solib ();
diff --git a/gdb/solib.c b/gdb/solib.c
index b1fdea9ad14..bb9c87d2237 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -1048,12 +1048,24 @@ info_sharedlibrary_command (const char *pattern, int from_tty)
 	}
     }
 
+  /* How many columns the table should have.  If the inferior has
+     more than one namespace active, we need a column to show that.  */
+  int num_cols = 4;
+  const solib_ops *ops = gdbarch_so_ops (gdbarch);
+  if (ops->num_active_namespaces != nullptr
+      && ops->num_active_namespaces () > 1)
+    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->num_active_namespaces != nullptr
+	&& ops->num_active_namespaces () > 1)
+      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 +1092,19 @@ info_sharedlibrary_command (const char *pattern, int from_tty)
 	    uiout->field_skip ("to");
 	  }
 
+	if (ops->num_active_namespaces != nullptr
+	    && ops->num_active_namespaces ()> 1)
+	  {
+	    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..03d2392b19d 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -180,6 +180,20 @@ 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);
+
+  /* Returns the number of active namespaces in the inferior.  */
+  int (*num_active_namespaces) ();
 };
 
 /* 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-ns-ids-lib.c b/gdb/testsuite/gdb.base/dlmopen-ns-ids-lib.c
new file mode 100644
index 00000000000..86cbb0f4e38
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dlmopen-ns-ids-lib.c
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2025 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+int gdb_dlmopen_glob = 0;
+
+__attribute__((visibility ("default")))
+int
+inc (int n)
+{
+  int amount = gdb_dlmopen_glob;
+  return n + amount;  /* bp.inc.  */
+}
diff --git a/gdb/testsuite/gdb.base/dlmopen-ns-ids-main.c b/gdb/testsuite/gdb.base/dlmopen-ns-ids-main.c
new file mode 100644
index 00000000000..3bcd8196483
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dlmopen-ns-ids-main.c
@@ -0,0 +1,54 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2025 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+#define _GNU_SOURCE
+#include <dlfcn.h>
+#include <stddef.h>
+#include <assert.h>
+#include <unistd.h>
+#include <stdio.h>
+
+int
+main (void)
+{
+  void *handle[4];
+  int (*fun) (int);
+  Lmid_t lmid;
+  int dl;
+
+  handle[0] = dlmopen (LM_ID_NEWLM, DSO_NAME, RTLD_LAZY | RTLD_LOCAL);
+  assert (handle[0] != NULL);
+
+  handle[1] = dlmopen (LM_ID_NEWLM, DSO_NAME, RTLD_LAZY | RTLD_LOCAL);
+  assert (handle[1] != NULL);
+
+  handle[2] = dlmopen (LM_ID_NEWLM, DSO_NAME, RTLD_LAZY | RTLD_LOCAL);
+  assert (handle[2] != NULL);
+
+  dlclose (handle[0]); /* TAG: first dlclose */
+  dlclose (handle[1]); /* TAG: second dlclose */
+  dlclose (handle[2]); /* TAG: third dlclose */
+
+  handle[3] = dlmopen (LM_ID_NEWLM, DSO_NAME, RTLD_LAZY | RTLD_LOCAL);
+  dlinfo (handle[3], RTLD_DI_LMID, &lmid);
+
+  dlclose (handle[3]); /* TAG: fourth dlclose */
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
new file mode 100644
index 00000000000..18856dca7cc
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
@@ -0,0 +1,106 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2025 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+#
+# Test several things related to handling linker namespaces:
+# * That the user-facing namespace ID is consistent;
+
+require allow_dlmopen_tests
+
+standard_testfile -main.c -lib.c
+
+set srcfile_lib $srcfile2
+set binfile_lib [standard_output_file dlmopen-lib.so]
+
+if { [build_executable "build shlib" $binfile_lib $srcfile_lib \
+	  [list debug shlib]] == -1 } {
+    return
+}
+
+if { [build_executable "failed to build" $testfile $srcfile \
+	  [list additional_flags=-DDSO_NAME=\"$binfile_lib\" \
+	       shlib_load debug]] } {
+    return
+}
+
+# Run the command "info sharedlibrary" and get the first namespace
+# for the so
+proc get_first_so_ns {} {
+    set ns -1
+    gdb_test_multiple "info sharedlibrary" "get SO namespace" -lbl {
+	-re "From\\s+To\\s+\(NS\\s+\)?Syms\\s+Read\\s+Shared Object Library\r\n" {
+	    exp_continue
+	}
+	-re "^$::hex\\s+$::hex\\s+\[\[($::decimal)\]\]\\s+\[^\r\n]+$::binfile_lib.*" {
+	    set ns $expect_out(1,string)
+	}
+	-re "^$::gdb_prompt $" {
+	}
+	-re "^\[^\r\n\]+\r\n" {
+	    exp_continue
+	}
+    }
+    return $ns
+}
+
+# Run the tests relating to the command "info sharedlibrary", to
+# verify that the namespace ID is consistent.
+proc test_info_shared {} {
+    clean_restart $::binfile
+
+    if { ![runto_main] } {
+	return
+    }
+
+    # First test that we don't print a namespace column at the start.
+    gdb_test "info sharedlibrary" \
+	"From\\s+To\\s+Syms\\s+Read\\s+Shared Object Library.*" \
+	"before loading anything"
+
+    gdb_breakpoint [gdb_get_line_number "TAG: first dlclose"]
+    gdb_continue_to_breakpoint "TAG: first dlclose"
+
+    # Next, test that we *do* print a namespace column after loading SOs.
+    gdb_test "info sharedlibrary" \
+	"From\\s+To\\s+NS\\s+Syms\\s+Read\\s+Shared Object Library.*" \
+	"after loading everything"
+
+    gdb_assert {[get_first_so_ns] == 1} "before closing any library"
+
+    gdb_test "next" ".*second dlclose.*" "close first library"
+    gdb_assert {[get_first_so_ns] == 2} "after closing one library"
+
+    gdb_test "next" ".*third dlclose.*" "close second library"
+    gdb_assert {[get_first_so_ns] == 3} "before closing two libraries"
+
+    gdb_breakpoint [gdb_get_line_number "TAG: fourth dlclose"]
+    gdb_continue_to_breakpoint "TAG: fourth dlclose"
+    # As of writing this test, glibc's LMID is just an index on an array of
+    # namespaces.  After closing a namespace, requesting a new one will
+    # return the index of the lowest-closed namespace, so this will likely
+    # be namespace 1, and because of glibc's reuse of the r_debug object,
+    # GDB should be able to assign the same number.
+    gdb_assert {[get_first_so_ns] == [get_integer_valueof "lmid" "-1"]} \
+	"reopen a namespace"
+
+    gdb_test "next" ".*return 0.*" "final namespace inactive"
+    gdb_test "info sharedlibrary" \
+	"From\\s+To\\s+Syms\\s+Read\\s+Shared Object Library.*" \
+	"after unloading everything"
+}
+
+test_info_shared
diff --git a/gdb/testsuite/gdb.base/dlmopen.exp b/gdb/testsuite/gdb.base/dlmopen.exp
index a8e3b08c016..084c5bc193c 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,12 +233,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\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)
+	    set lib $expect_out(3,string)
 
 	    if { [is_dyln $lib] } {
 		# This looks like it might be the dynamic linker.
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.

base-commit: ea10be6e89a4569d03d61e5cf7418dc9a4015a24
-- 
2.49.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-03-27 17:13   ` [PATCH v3] " Guinevere Larsen
@ 2025-03-31 19:34     ` Guinevere Larsen
  2025-04-05 20:17     ` Kevin Buettner
  1 sibling, 0 replies; 17+ messages in thread
From: Guinevere Larsen @ 2025-03-31 19:34 UTC (permalink / raw)
  To: gdb-patches

Linaro CI told me there was a problem in the test case in here (thanks 
again, linaro!). More inlined
> diff --git a/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
> new file mode 100644
> index 00000000000..18856dca7cc
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
> @@ -0,0 +1,106 @@
> +# This testcase is part of GDB, the GNU debugger.
> +#
> +# Copyright 2025 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +#
> +# Test several things related to handling linker namespaces:
> +# * That the user-facing namespace ID is consistent;
> +
> +require allow_dlmopen_tests
> +
> +standard_testfile -main.c -lib.c
> +
> +set srcfile_lib $srcfile2
> +set binfile_lib [standard_output_file dlmopen-lib.so]
> +
> +if { [build_executable "build shlib" $binfile_lib $srcfile_lib \
> +	  [list debug shlib]] == -1 } {
> +    return
> +}
> +
> +if { [build_executable "failed to build" $testfile $srcfile \
> +	  [list additional_flags=-DDSO_NAME=\"$binfile_lib\" \
> +	       shlib_load debug]] } {
> +    return
> +}
> +
> +# Run the command "info sharedlibrary" and get the first namespace
> +# for the so
> +proc get_first_so_ns {} {
> +    set ns -1
> +    gdb_test_multiple "info sharedlibrary" "get SO namespace" -lbl {
> +	-re "From\\s+To\\s+\(NS\\s+\)?Syms\\s+Read\\s+Shared Object Library\r\n" {
> +	    exp_continue
> +	}
> +	-re "^$::hex\\s+$::hex\\s+\[\[($::decimal)\]\]\\s+\[^\r\n]+$::binfile_lib.*" {

Turns out the literal [ and ] should be triple escaped here. I've fixed 
thios locally

> +	    set ns $expect_out(1,string)
> +	}
> +	-re "^$::gdb_prompt $" {
> +	}
> +	-re "^\[^\r\n\]+\r\n" {
> +	    exp_continue
> +	}
> +    }
> +    return $ns
> +}
> +
> +# Run the tests relating to the command "info sharedlibrary", to
> +# verify that the namespace ID is consistent.
> +proc test_info_shared {} {
> +    clean_restart $::binfile
> +
> +    if { ![runto_main] } {
> +	return
> +    }
> +
> +    # First test that we don't print a namespace column at the start.
> +    gdb_test "info sharedlibrary" \
> +	"From\\s+To\\s+Syms\\s+Read\\s+Shared Object Library.*" \
> +	"before loading anything"
> +
> +    gdb_breakpoint [gdb_get_line_number "TAG: first dlclose"]
> +    gdb_continue_to_breakpoint "TAG: first dlclose"
> +
> +    # Next, test that we *do* print a namespace column after loading SOs.
> +    gdb_test "info sharedlibrary" \
> +	"From\\s+To\\s+NS\\s+Syms\\s+Read\\s+Shared Object Library.*" \
> +	"after loading everything"
> +
> +    gdb_assert {[get_first_so_ns] == 1} "before closing any library"
> +
> +    gdb_test "next" ".*second dlclose.*" "close first library"
> +    gdb_assert {[get_first_so_ns] == 2} "after closing one library"
> +
> +    gdb_test "next" ".*third dlclose.*" "close second library"
> +    gdb_assert {[get_first_so_ns] == 3} "before closing two libraries"
> +
> +    gdb_breakpoint [gdb_get_line_number "TAG: fourth dlclose"]
> +    gdb_continue_to_breakpoint "TAG: fourth dlclose"
> +    # As of writing this test, glibc's LMID is just an index on an array of
> +    # namespaces.  After closing a namespace, requesting a new one will
> +    # return the index of the lowest-closed namespace, so this will likely
> +    # be namespace 1, and because of glibc's reuse of the r_debug object,
> +    # GDB should be able to assign the same number.
> +    gdb_assert {[get_first_so_ns] == [get_integer_valueof "lmid" "-1"]} \
> +	"reopen a namespace"
> +
> +    gdb_test "next" ".*return 0.*" "final namespace inactive"
> +    gdb_test "info sharedlibrary" \
> +	"From\\s+To\\s+Syms\\s+Read\\s+Shared Object Library.*" \
> +	"after unloading everything"
> +}
> +
> +test_info_shared

-- 
Cheers,
Guinevere Larsen
She/Her/Hers


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-03-27 17:13   ` [PATCH v3] " Guinevere Larsen
  2025-03-31 19:34     ` Guinevere Larsen
@ 2025-04-05 20:17     ` Kevin Buettner
  2025-04-07 13:07       ` Guinevere Larsen
  1 sibling, 1 reply; 17+ messages in thread
From: Kevin Buettner @ 2025-04-05 20:17 UTC (permalink / raw)
  To: Guinevere Larsen; +Cc: gdb-patches

On Thu, 27 Mar 2025 14:13:13 -0300
Guinevere Larsen <guinevere@redhat.com> wrote:

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

This syntax seems okay to me.  (I don't have a better suggestion.)

> This syntax was chosen based on the C attributes, since nothing in GDB
> uses a similar syntax that could confuse users. Other syntax options
> that were explored were "#<number>" and "@<number>". The former was
> abandoned because when printing a frame, the frame number is also
> printed with #<number>, so in a lot of the context in which that the
> identifier would show up, it appears in a confusing way. The latter
> clashes with the array printing syntax, and I believe that the having
> "@N::foo" working completely differently to "foo@2" would also lead to a
> bad user experience.

Thanks for explaining the other options considered.

> The namespace identifiers are stored via a vector inside svr4_info
> object. The vector stores the address of the r_debug objects used by
> glibc to identify each namespace, and the user-friendly ID is the index
> of the r_debug in the vector. This commit also introduces a set storing
> the indices of active namespaces. The glibc I used to develop this patch
> (glibc 2.40 on Fedora 41) doesn't allow an SO to be loaded into a
> deactivated namespace, and requesting a new namespace when a namespace
> was previously closed will reuse that namespace. Because of how this is
> implemented, this patch lets GDB easily track the exact namespace IDs
> that the inferior will see.
> 
> Finally, two new solib_ops function pointers were added, find_solib_ns
> and num_active_namespaces, to allow code outside of solib-svr4 to find
> and use the namespace identifiers and the number of namespaces,
> respectively. As a sanity check, the command `info sharedlibrary` has
> been changed to display the namespace identifier when the inferior has
> more than one active namespace. With this final change, a couple of tests
> had to be tweaked to handle the possible new column, and a new test has
> been created to make sure that the column appears and disappears as
> needed, and that GDB can track the value of the LMID for namespaces.
> 
> ---
> Changes for v3:
> * Changed the namespace identifier display from #N to [[N]]
> 
> Changes for v2:
> * made it so the new column in "info shared" only shows up when multiple
>   namespaces are active
> * changes NEWS and docs to reflect that
> * Added a test for this functionality

Your solib related changes and the new tests look good to me.  The only
(slight) misgiving I have is related to identifier being printed in
the NS column...

(gdb) info sharedlibrary
From                To                  NS    Syms Read   Shared Object Library
0x00007ffff7fc7000  0x00007ffff7fff000  [[1]] Yes         /lib64/ld-linux-x86-64.so.2
0x00007ffff7ea2000  0x00007ffff7f90000  [[0]] Yes (*)     /lib64/libm.so.6
0x00007ffff7caf000  0x00007ffff7ea2000  [[0]] Yes (*)     /lib64/libc.so.6
0x00007ffff7fbb000  0x00007ffff7fbf000  [[1]] Yes         /mesquite2/sourceware-git/f42-review/bld/gdb/testsuite/outputs/gdb.base/dlmopen-ns-ids/dlmopen-lib.so
0x00007ffff7b8f000  0x00007ffff7c7d000  [[1]] Yes (*)     /lib64/libm.so.6
0x00007ffff799c000  0x00007ffff7b8f000  [[1]] Yes (*)     /lib64/libc.so.6
0x00007ffff7fc7000  0x00007ffff7fff000  [[1]] Yes         /lib64/ld-linux-x86-64.so.2
0x00007ffff7fb7000  0x00007ffff7fbb000  [[2]] Yes         /mesquite2/sourceware-git/f42-review/bld/gdb/testsuite/outputs/gdb.base/dlmopen-ns-ids/dlmopen-lib.so
0x00007ffff78ae000  0x00007ffff799c000  [[2]] Yes (*)     /lib64/libm.so.6
0x00007ffff76bb000  0x00007ffff78ae000  [[2]] Yes (*)     /lib64/libc.so.6
0x00007ffff7fc7000  0x00007ffff7fff000  [[1]] Yes         /lib64/ld-linux-x86-64.so.2
0x00007ffff7fb3000  0x00007ffff7fb7000  [[3]] Yes         /mesquite2/sourceware-git/f42-review/bld/gdb/testsuite/outputs/gdb.base/dlmopen-ns-ids/dlmopen-lib.so
0x00007ffff75cd000  0x00007ffff76bb000  [[3]] Yes (*)     /lib64/libm.so.6
0x00007ffff73da000  0x00007ffff75cd000  [[3]] Yes (*)     /lib64/libc.so.6
0x00007ffff7fc7000  0x00007ffff7fff000  [[1]] Yes         /lib64/ld-linux-x86-64.so.2


I'm wondering if it might be better to print just (e.g.) "1" instead
of "[[1]]".  The argument in favor of the way you're doing it is that
it may be a reminder of the syntax that'll (eventually) be used to
specify linker namespaces in expressions.  Since it is kind of odd
and may be difficult to remember, especially since this is such
a niche feature, I think it's okay.

I think this patch should go in so that further progress can be made
on linker namespace support.  Quibbles regarding whether the NS
column should have the square brackets or not can be addressed in
later patches if necessary.

The approval give below assumes that you've fixed the testcase as
mentioned in reply.  (I ran into this and added your fix to my local
sources for testing.  FWIW, I tested on x86_64, aarch64, and riscv.)

Approved-by: Kevin Buettner <kevinb@redhat.com>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] gdb: Introduce user-friendly namespace identifier for "info shared"
  2025-04-05 20:17     ` Kevin Buettner
@ 2025-04-07 13:07       ` Guinevere Larsen
  0 siblings, 0 replies; 17+ messages in thread
From: Guinevere Larsen @ 2025-04-07 13:07 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

On 4/5/25 5:17 PM, Kevin Buettner wrote:
> On Thu, 27 Mar 2025 14:13:13 -0300
> Guinevere Larsen <guinevere@redhat.com> wrote:
>
>> 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.
> This syntax seems okay to me.  (I don't have a better suggestion.)
>
>> This syntax was chosen based on the C attributes, since nothing in GDB
>> uses a similar syntax that could confuse users. Other syntax options
>> that were explored were "#<number>" and "@<number>". The former was
>> abandoned because when printing a frame, the frame number is also
>> printed with #<number>, so in a lot of the context in which that the
>> identifier would show up, it appears in a confusing way. The latter
>> clashes with the array printing syntax, and I believe that the having
>> "@N::foo" working completely differently to "foo@2" would also lead to a
>> bad user experience.
> Thanks for explaining the other options considered.
>
>> The namespace identifiers are stored via a vector inside svr4_info
>> object. The vector stores the address of the r_debug objects used by
>> glibc to identify each namespace, and the user-friendly ID is the index
>> of the r_debug in the vector. This commit also introduces a set storing
>> the indices of active namespaces. The glibc I used to develop this patch
>> (glibc 2.40 on Fedora 41) doesn't allow an SO to be loaded into a
>> deactivated namespace, and requesting a new namespace when a namespace
>> was previously closed will reuse that namespace. Because of how this is
>> implemented, this patch lets GDB easily track the exact namespace IDs
>> that the inferior will see.
>>
>> Finally, two new solib_ops function pointers were added, find_solib_ns
>> and num_active_namespaces, to allow code outside of solib-svr4 to find
>> and use the namespace identifiers and the number of namespaces,
>> respectively. As a sanity check, the command `info sharedlibrary` has
>> been changed to display the namespace identifier when the inferior has
>> more than one active namespace. With this final change, a couple of tests
>> had to be tweaked to handle the possible new column, and a new test has
>> been created to make sure that the column appears and disappears as
>> needed, and that GDB can track the value of the LMID for namespaces.
>>
>> ---
>> Changes for v3:
>> * Changed the namespace identifier display from #N to [[N]]
>>
>> Changes for v2:
>> * made it so the new column in "info shared" only shows up when multiple
>>    namespaces are active
>> * changes NEWS and docs to reflect that
>> * Added a test for this functionality
> Your solib related changes and the new tests look good to me.  The only
> (slight) misgiving I have is related to identifier being printed in
> the NS column...
>
> (gdb) info sharedlibrary
>  From                To                  NS    Syms Read   Shared Object Library
> 0x00007ffff7fc7000  0x00007ffff7fff000  [[1]] Yes         /lib64/ld-linux-x86-64.so.2
> 0x00007ffff7ea2000  0x00007ffff7f90000  [[0]] Yes (*)     /lib64/libm.so.6
> 0x00007ffff7caf000  0x00007ffff7ea2000  [[0]] Yes (*)     /lib64/libc.so.6
> 0x00007ffff7fbb000  0x00007ffff7fbf000  [[1]] Yes         /mesquite2/sourceware-git/f42-review/bld/gdb/testsuite/outputs/gdb.base/dlmopen-ns-ids/dlmopen-lib.so
> 0x00007ffff7b8f000  0x00007ffff7c7d000  [[1]] Yes (*)     /lib64/libm.so.6
> 0x00007ffff799c000  0x00007ffff7b8f000  [[1]] Yes (*)     /lib64/libc.so.6
> 0x00007ffff7fc7000  0x00007ffff7fff000  [[1]] Yes         /lib64/ld-linux-x86-64.so.2
> 0x00007ffff7fb7000  0x00007ffff7fbb000  [[2]] Yes         /mesquite2/sourceware-git/f42-review/bld/gdb/testsuite/outputs/gdb.base/dlmopen-ns-ids/dlmopen-lib.so
> 0x00007ffff78ae000  0x00007ffff799c000  [[2]] Yes (*)     /lib64/libm.so.6
> 0x00007ffff76bb000  0x00007ffff78ae000  [[2]] Yes (*)     /lib64/libc.so.6
> 0x00007ffff7fc7000  0x00007ffff7fff000  [[1]] Yes         /lib64/ld-linux-x86-64.so.2
> 0x00007ffff7fb3000  0x00007ffff7fb7000  [[3]] Yes         /mesquite2/sourceware-git/f42-review/bld/gdb/testsuite/outputs/gdb.base/dlmopen-ns-ids/dlmopen-lib.so
> 0x00007ffff75cd000  0x00007ffff76bb000  [[3]] Yes (*)     /lib64/libm.so.6
> 0x00007ffff73da000  0x00007ffff75cd000  [[3]] Yes (*)     /lib64/libc.so.6
> 0x00007ffff7fc7000  0x00007ffff7fff000  [[1]] Yes         /lib64/ld-linux-x86-64.so.2
>
>
> I'm wondering if it might be better to print just (e.g.) "1" instead
> of "[[1]]".  The argument in favor of the way you're doing it is that
> it may be a reminder of the syntax that'll (eventually) be used to
> specify linker namespaces in expressions.  Since it is kind of odd
> and may be difficult to remember, especially since this is such
> a niche feature, I think it's okay.
>
> I think this patch should go in so that further progress can be made
> on linker namespace support.  Quibbles regarding whether the NS
> column should have the square brackets or not can be addressed in
> later patches if necessary.

I agree it makes sense to push this and leave final discussion of how to 
print the namespace, but I'll leave my thoughts here.

It was a conscious decision to always display the linker namespace with 
the same look as how the user should input it. That's because I believe 
that, as much as its reasonable, we should try to make it possible for a 
user to figure out how to use commands without requiring that they look 
into the documentation. So if a user ends up dealing with an inferior 
with multiple namespaces, how to interact with that part of the inferior 
should be apparent from the moment the user realizes this is happening. 
In other words, we probably should print the full syntax every time.

Additionally, if we print a stop location like "1::foo", a user could be 
confused about whether that refers to a c++ namespace named 1, or the 
linker namespace, so it is clear to me that we should always print 
"[[1]]:foo". Keeping the NS column as printing the [[]] will also help 
the user read that identifier and keep things consistent throughout GDB.

>
> The approval give below assumes that you've fixed the testcase as
> mentioned in reply.  (I ran into this and added your fix to my local
> sources for testing.  FWIW, I tested on x86_64, aarch64, and riscv.)
>
> Approved-by: Kevin Buettner <kevinb@redhat.com>
>
Thanks for the approval. I've pushed it

-- 
Cheers,
Guinevere Larsen
She/Her/Hers


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2025-04-07 13:08 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-03-13 17:00 [PATCH] gdb: Introduce user-friendly namespace identifier for "info shared" Guinevere Larsen
2025-03-13 19:19 ` Eli Zaretskii
2025-03-13 19:27   ` Guinevere Larsen
2025-03-15  2:51 ` Kevin Buettner
2025-03-15  3:11   ` Kevin Buettner
2025-03-17 11:55     ` Guinevere Larsen
2025-03-17 15:36       ` Simon Marchi
2025-03-18  1:07       ` Kevin Buettner
2025-03-17 15:36 ` Simon Marchi
2025-03-17 17:07   ` Guinevere Larsen
2025-03-17 17:54     ` Simon Marchi
2025-03-19 12:30 ` [PATCH v2] " Guinevere Larsen
2025-03-21 17:55   ` Guinevere Larsen
2025-03-27 17:13   ` [PATCH v3] " Guinevere Larsen
2025-03-31 19:34     ` Guinevere Larsen
2025-04-05 20:17     ` Kevin Buettner
2025-04-07 13:07       ` Guinevere Larsen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox