Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH 1/1] gdb: Print linker namespace when showing a frame
@ 2025-08-12 20:21 Guinevere Larsen
  2025-08-13 19:28 ` Simon Marchi
  0 siblings, 1 reply; 3+ messages in thread
From: Guinevere Larsen @ 2025-08-12 20:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: Guinevere Larsen

When a user is stopped in a private linker namespace, the only way for
them to realize that is using the _linker_namespace convenience
variable. While serviceable, this is a sub-optimal solution, as most
users are unaware of convenience variables.

This commit introduces a new way for users to be informed of the linker
namespace of a function, by printing it along with the function name.
This is done by using the proposed syntax for symbols and locations,
like so:

  #0  [[0]]::main ()

This is done by exporting part of the functionality behind the
_linker_namespace variable, namely, find the linker namespace that
contains the given address on the current program space. Since this
change also touched the convenience variable code, this commit fixes a
formatting error in that function.

The namespace ID is only printed if multiple namespaces are active,
otherwise no change in behavior is expected. This commit also updates
the test gdb.base/dlmopen-ns-ids.exp to test this functionality.
---
 gdb/solib.c                               | 28 ++++++++++++++---------
 gdb/solib.h                               |  5 ++++
 gdb/stack.c                               | 13 +++++++++++
 gdb/testsuite/gdb.base/dlmopen-ns-ids.exp | 12 +++++++++-
 gdb/testsuite/gdb.mi/mi-dlmopen.exp       |  2 +-
 5 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/gdb/solib.c b/gdb/solib.c
index 3ec2032f012..2f32f08b645 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -1805,6 +1805,21 @@ remove_user_added_objfile (struct objfile *objfile)
     }
 }
 
+/* See solib.h.  */
+
+int
+linker_namespace_contains_addr (CORE_ADDR addr)
+{
+  for (const solib &so : current_program_space->solibs ())
+    if (solib_contains_address_p (so, addr))
+      {
+	if (so.ops ().supports_namespaces ())
+	  return so.ops ().find_solib_ns (so);
+      }
+
+  return 0;
+}
+
 /* Implementation of the linker_namespace convenience variable.
 
    This returns the GDB internal identifier of the linker namespace,
@@ -1813,19 +1828,10 @@ remove_user_added_objfile (struct objfile *objfile)
 
 static value *
 linker_namespace_make_value (gdbarch *gdbarch, internalvar *var,
-				     void *ignore)
+			     void *ignore)
 {
-  int nsid = 0;
   CORE_ADDR curr_pc = get_frame_pc (get_selected_frame ());
-
-  for (const solib &so : current_program_space->solibs ())
-    if (solib_contains_address_p (so, curr_pc))
-      {
-	if (so.ops ().supports_namespaces ())
-	  nsid = so.ops ().find_solib_ns (so);
-
-	break;
-      }
+  int nsid = linker_namespace_contains_addr (curr_pc);
 
   /* If the PC is not in an SO, or the solib_ops doesn't support
      linker namespaces, the inferior is in the default namespace.  */
diff --git a/gdb/solib.h b/gdb/solib.h
index b9465e103bd..213f66c7ae4 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -303,6 +303,11 @@ extern const char *solib_name_from_address (struct program_space *, CORE_ADDR);
 
 extern bool solib_contains_address_p (const solib &, CORE_ADDR);
 
+/* Given the address ADDR, return which linker namespace contains
+   this address in the current program space.  */
+
+int linker_namespace_contains_addr (CORE_ADDR addr);
+
 /* Return whether the data starting at VADDR, size SIZE, must be kept
    in a core file for shared libraries loaded before "gcore" is used
    to be handled correctly when the core file is loaded.  This only
diff --git a/gdb/stack.c b/gdb/stack.c
index e6335669531..10078e06766 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1363,6 +1363,19 @@ print_frame (struct ui_out *uiout,
     annotate_frame_function_name ();
 
     string_file stb;
+
+    /* Print the linker namespace containing the frame, if there
+       are multiple namespaces active.  */
+    LONGEST num_linker_namespaces;
+    get_internalvar_integer (lookup_internalvar ("_active_linker_namespaces"),
+			     &num_linker_namespaces);
+    if (pc_p && num_linker_namespaces > 1)
+      {
+	LONGEST curr_namespace = linker_namespace_contains_addr (pc);
+	std::string s = string_printf ("[[%ld]]::", curr_namespace);
+	gdb_puts (s.c_str (), &stb);
+      }
+
     gdb_puts (funname ? funname.get () : "??", &stb);
     uiout->field_stream ("func", stb, function_name_style.style ());
     uiout->wrap_hint (3);
diff --git a/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
index 4d3e8eba2ab..a88ad817dc1 100644
--- a/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
+++ b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
@@ -107,6 +107,8 @@ proc test_info_shared {} {
 
 # Run all tests related to the linkage namespaces convenience
 # variables, _active_namespaces and _current_namespaces.
+# Also tests that the namespace ID is only printed at the correct
+# times.
 proc_with_prefix test_conv_vars {} {
     clean_restart $::binfile
 
@@ -124,6 +126,11 @@ proc_with_prefix test_conv_vars {} {
     gdb_test "print \$_linker_namespace" ".* = 0" \
 	"Still in the default namespace"
 
+    # There should be no namespace ID visible, since there's
+    # only one namespace loaded.
+    gdb_test "backtrace" "\#0\\s+main .*" \
+	"No namespace ID in backtrace"
+
     gdb_breakpoint "inc" allow-pending
     gdb_breakpoint [gdb_get_line_number "TAG: first dlclose"]
 
@@ -132,11 +139,14 @@ proc_with_prefix test_conv_vars {} {
 
 	gdb_test "print \$_linker_namespace" ".* = $dl" \
 	    "Verify we're in namespace $dl"
+
+	gdb_test "frame" "\#0\\s+\\\[\\\[$dl\\\]\\\]::inc.*" \
+	    "Namespace ID in the frame"
     }
 
     # Check that we display the namespace of the selected
     # frame, not the lowermost one.
-    gdb_test "up" "\#1.*in main.*"
+    gdb_test "up" "\#1.*in \\\[\\\[0\\\]\\\]::main.*"
     gdb_test "print \$_linker_namespace" ".* = 0" \
 	"print namespace of selected frame"
 
diff --git a/gdb/testsuite/gdb.mi/mi-dlmopen.exp b/gdb/testsuite/gdb.mi/mi-dlmopen.exp
index c0208ebcc51..3070f879d17 100644
--- a/gdb/testsuite/gdb.mi/mi-dlmopen.exp
+++ b/gdb/testsuite/gdb.mi/mi-dlmopen.exp
@@ -146,7 +146,7 @@ proc check_solib_unload_events {} {
 	-disp keep -func main -file ".*$::srcfile" -line $::bp_main
 
     # Run past all the dlopen and dlmopen calls.
-    mi_execute_to "exec-continue" "breakpoint-hit" main "" ".*" $::bp_loaded \
+    mi_execute_to "exec-continue" "breakpoint-hit" {\[\[0\]\]::main} "" ".*" $::bp_loaded \
 	{"" "disp=\"keep\""} "continue until all libraries are loaded"
 
     # Check that the dynamic linker has now been loaded multiple times.
-- 
2.50.1


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

* Re: [PATCH 1/1] gdb: Print linker namespace when showing a frame
  2025-08-12 20:21 [PATCH 1/1] gdb: Print linker namespace when showing a frame Guinevere Larsen
@ 2025-08-13 19:28 ` Simon Marchi
  2025-08-13 20:18   ` Guinevere Larsen
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2025-08-13 19:28 UTC (permalink / raw)
  To: Guinevere Larsen, gdb-patches



On 2025-08-12 16:21, Guinevere Larsen wrote:
> When a user is stopped in a private linker namespace, the only way for
> them to realize that is using the _linker_namespace convenience
> variable. While serviceable, this is a sub-optimal solution, as most
> users are unaware of convenience variables.
> 
> This commit introduces a new way for users to be informed of the linker
> namespace of a function, by printing it along with the function name.
> This is done by using the proposed syntax for symbols and locations,
> like so:
> 
>   #0  [[0]]::main ()
> 
> This is done by exporting part of the functionality behind the
> _linker_namespace variable, namely, find the linker namespace that
> contains the given address on the current program space. Since this
> change also touched the convenience variable code, this commit fixes a
> formatting error in that function.
> 
> The namespace ID is only printed if multiple namespaces are active,
> otherwise no change in behavior is expected. This commit also updates
> the test gdb.base/dlmopen-ns-ids.exp to test this functionality.

Just bear in mind that an address could be in a library that is part of
multiple namespaces.  The only real occurence I know of today is the
dynamic linker itself.  If you're stopped inside the dynamic linker,
with this patch, I think we would end up showing one arbitrary
namespace id.

The idea of sharing libraries between namespaces was also pitched in the
past, although it wasn't merged:

https://sourceware.org/pipermail/libc-alpha/2021-October/131818.html

I guess the right behavior would be to not show a namespace ID if the
piece of code is globally present in all namespaces, or show a list of
namespace IDs if the piece of code is visible in only some namespaces.
Perhaps it's not worth finding a solution for this right now, but I
thought I'd mention it so that we are aware of the limitations.

> ---
>  gdb/solib.c                               | 28 ++++++++++++++---------
>  gdb/solib.h                               |  5 ++++
>  gdb/stack.c                               | 13 +++++++++++
>  gdb/testsuite/gdb.base/dlmopen-ns-ids.exp | 12 +++++++++-
>  gdb/testsuite/gdb.mi/mi-dlmopen.exp       |  2 +-
>  5 files changed, 47 insertions(+), 13 deletions(-)
> 
> diff --git a/gdb/solib.c b/gdb/solib.c
> index 3ec2032f012..2f32f08b645 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -1805,6 +1805,21 @@ remove_user_added_objfile (struct objfile *objfile)
>      }
>  }
>  
> +/* See solib.h.  */
> +
> +int
> +linker_namespace_contains_addr (CORE_ADDR addr)

This function name makes it sound like it's a function to check whether
a given namespace contains a given address.  I would suggest
"linker_namespace_for_addr" or something like that.

> +{
> +  for (const solib &so : current_program_space->solibs ())
> +    if (solib_contains_address_p (so, addr))
> +      {
> +	if (so.ops ().supports_namespaces ())
> +	  return so.ops ().find_solib_ns (so);
> +      }

Please combine the two ifs:

  for (const solib &so : current_program_space->solibs ())
    if (solib_contains_address_p (so, addr)
	&& so.ops ().supports_namespaces ())
      return so.ops ().find_solib_ns (so);

> diff --git a/gdb/solib.h b/gdb/solib.h
> index b9465e103bd..213f66c7ae4 100644
> --- a/gdb/solib.h
> +++ b/gdb/solib.h
> @@ -303,6 +303,11 @@ extern const char *solib_name_from_address (struct program_space *, CORE_ADDR);
>  
>  extern bool solib_contains_address_p (const solib &, CORE_ADDR);
>  
> +/* Given the address ADDR, return which linker namespace contains
> +   this address in the current program space.  */
> +
> +int linker_namespace_contains_addr (CORE_ADDR addr);

If nothing in the call tree relies on the current program space, I would
prefer if the program space was passed as a parameter.

> +
>  /* Return whether the data starting at VADDR, size SIZE, must be kept
>     in a core file for shared libraries loaded before "gcore" is used
>     to be handled correctly when the core file is loaded.  This only
> diff --git a/gdb/stack.c b/gdb/stack.c
> index e6335669531..10078e06766 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -1363,6 +1363,19 @@ print_frame (struct ui_out *uiout,
>      annotate_frame_function_name ();
>  
>      string_file stb;
> +
> +    /* Print the linker namespace containing the frame, if there
> +       are multiple namespaces active.  */
> +    LONGEST num_linker_namespaces;
> +    get_internalvar_integer (lookup_internalvar ("_active_linker_namespaces"),
> +			     &num_linker_namespaces);

I would really prefer some direct (C++) function call, instead of going
through the internalval mechanism.  It makes it much easier to navigate
the code, find references, etc.

And because I looked at _active_linker_namespaces, I have some thoughts
about that.  It's not related to this patch, but should perhaps be
addressed before the release.

First, the name "_active_linker_namespaces" insinuates that it contains
something like a list of all the active linker namespaces.  It should
perhaps be "_active_linker_namespaces_count" or
"_num_active_linker_namespaces"?  We already have
$_inferior_thread_count, so I guess _active_linker_namespaces_count
would be good for consistency.

Then, _active_linker_namespaces is set in
svr4_solib_ops::maybe_add_namespace, which is called when
fetching/updating/listing shared libraries.  That doesn't work well with
multi-inferior for instance.  Imagine you stop at a breakpoint in
inferior 1, which has 2 namespaces, _active_linker_namespaces is set to
2.  Then you run inferior 2, which has 2 namespaces, stop at a
breakpoint, _active_linker_namespaces is set to 3.  If you focus back to
inferior 1, _active_linker_namespaces will still have the value for
inferior 2.  My feeling is that the variable should be sensitive to the
user's focus.  That would probably mean calling
solib_ops::num_active_namespaces on the current program space's
solib_ops.

And then, since we're talking about that, I was wondering how to handle
_active_linker_namespaces in my "multiple solib ops" series.  Imagine
you have a program space with one svr4_solib_ops and one rocm_solib_ops,
which is how I envision it will work with my patch, when debugging a
ROCm program.  How would I implement _active_linker_namespaces then
(assuming you first made the change described above).  Should I iterate
on all the solib_ops of the program space and sum up their
solib_ops::num_active_namespaces?  In a normal scenario, that would give
me 2, which is kinda right because the linker namespace on the host side
is distinct from the linker namespace on the device side, so you
effectively have 2 of them in the program space.  But is it going to
work with the uses this convenience variable is intended for?  I don't
really what the intended uses are.

Simon

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

* Re: [PATCH 1/1] gdb: Print linker namespace when showing a frame
  2025-08-13 19:28 ` Simon Marchi
@ 2025-08-13 20:18   ` Guinevere Larsen
  0 siblings, 0 replies; 3+ messages in thread
From: Guinevere Larsen @ 2025-08-13 20:18 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 8/13/25 4:28 PM, Simon Marchi wrote:
>
> On 2025-08-12 16:21, Guinevere Larsen wrote:
>> When a user is stopped in a private linker namespace, the only way for
>> them to realize that is using the _linker_namespace convenience
>> variable. While serviceable, this is a sub-optimal solution, as most
>> users are unaware of convenience variables.
>>
>> This commit introduces a new way for users to be informed of the linker
>> namespace of a function, by printing it along with the function name.
>> This is done by using the proposed syntax for symbols and locations,
>> like so:
>>
>>    #0  [[0]]::main ()
>>
>> This is done by exporting part of the functionality behind the
>> _linker_namespace variable, namely, find the linker namespace that
>> contains the given address on the current program space. Since this
>> change also touched the convenience variable code, this commit fixes a
>> formatting error in that function.
>>
>> The namespace ID is only printed if multiple namespaces are active,
>> otherwise no change in behavior is expected. This commit also updates
>> the test gdb.base/dlmopen-ns-ids.exp to test this functionality.
> Just bear in mind that an address could be in a library that is part of
> multiple namespaces.  The only real occurence I know of today is the
> dynamic linker itself.  If you're stopped inside the dynamic linker,
> with this patch, I think we would end up showing one arbitrary
> namespace id.
>
> The idea of sharing libraries between namespaces was also pitched in the
> past, although it wasn't merged:
>
> https://sourceware.org/pipermail/libc-alpha/2021-October/131818.html
>
> I guess the right behavior would be to not show a namespace ID if the
> piece of code is globally present in all namespaces, or show a list of
> namespace IDs if the piece of code is visible in only some namespaces.
> Perhaps it's not worth finding a solution for this right now, but I
> thought I'd mention it so that we are aware of the limitations.
Yeah, I don't think it would be too hard to solve this problem, but 
considering that only the linker namespace can do it for now, I don't 
think it's worth implementing a solution.
>
>> ---
>>   gdb/solib.c                               | 28 ++++++++++++++---------
>>   gdb/solib.h                               |  5 ++++
>>   gdb/stack.c                               | 13 +++++++++++
>>   gdb/testsuite/gdb.base/dlmopen-ns-ids.exp | 12 +++++++++-
>>   gdb/testsuite/gdb.mi/mi-dlmopen.exp       |  2 +-
>>   5 files changed, 47 insertions(+), 13 deletions(-)
>>
>> diff --git a/gdb/solib.c b/gdb/solib.c
>> index 3ec2032f012..2f32f08b645 100644
>> --- a/gdb/solib.c
>> +++ b/gdb/solib.c
>> @@ -1805,6 +1805,21 @@ remove_user_added_objfile (struct objfile *objfile)
>>       }
>>   }
>>   
>> +/* See solib.h.  */
>> +
>> +int
>> +linker_namespace_contains_addr (CORE_ADDR addr)
> This function name makes it sound like it's a function to check whether
> a given namespace contains a given address.  I would suggest
> "linker_namespace_for_addr" or something like that.
I couldn't think of a better name, thank you!
>> +{
>> +  for (const solib &so : current_program_space->solibs ())
>> +    if (solib_contains_address_p (so, addr))
>> +      {
>> +	if (so.ops ().supports_namespaces ())
>> +	  return so.ops ().find_solib_ns (so);
>> +      }
> Please combine the two ifs:
>
>    for (const solib &so : current_program_space->solibs ())
>      if (solib_contains_address_p (so, addr)
> 	&& so.ops ().supports_namespaces ())
>        return so.ops ().find_solib_ns (so);
fixed
>> diff --git a/gdb/solib.h b/gdb/solib.h
>> index b9465e103bd..213f66c7ae4 100644
>> --- a/gdb/solib.h
>> +++ b/gdb/solib.h
>> @@ -303,6 +303,11 @@ extern const char *solib_name_from_address (struct program_space *, CORE_ADDR);
>>   
>>   extern bool solib_contains_address_p (const solib &, CORE_ADDR);
>>   
>> +/* Given the address ADDR, return which linker namespace contains
>> +   this address in the current program space.  */
>> +
>> +int linker_namespace_contains_addr (CORE_ADDR addr);
> If nothing in the call tree relies on the current program space, I would
> prefer if the program space was passed as a parameter.
sure
>
>> +
>>   /* Return whether the data starting at VADDR, size SIZE, must be kept
>>      in a core file for shared libraries loaded before "gcore" is used
>>      to be handled correctly when the core file is loaded.  This only
>> diff --git a/gdb/stack.c b/gdb/stack.c
>> index e6335669531..10078e06766 100644
>> --- a/gdb/stack.c
>> +++ b/gdb/stack.c
>> @@ -1363,6 +1363,19 @@ print_frame (struct ui_out *uiout,
>>       annotate_frame_function_name ();
>>   
>>       string_file stb;
>> +
>> +    /* Print the linker namespace containing the frame, if there
>> +       are multiple namespaces active.  */
>> +    LONGEST num_linker_namespaces;
>> +    get_internalvar_integer (lookup_internalvar ("_active_linker_namespaces"),
>> +			     &num_linker_namespaces);
> I would really prefer some direct (C++) function call, instead of going
> through the internalval mechanism.  It makes it much easier to navigate
> the code, find references, etc.
Sure, makes sense.
>
> And because I looked at _active_linker_namespaces, I have some thoughts
> about that.  It's not related to this patch, but should perhaps be
> addressed before the release.
>
> First, the name "_active_linker_namespaces" insinuates that it contains
> something like a list of all the active linker namespaces.  It should
> perhaps be "_active_linker_namespaces_count" or
> "_num_active_linker_namespaces"?  We already have
> $_inferior_thread_count, so I guess _active_linker_namespaces_count
> would be good for consistency.

Well, since we only ever keep track of active namespaces, we could also 
simplify it to _linker_namespace_count, which follows closer to 
_inferior_thread_count anyway.

>
> Then, _active_linker_namespaces is set in
> svr4_solib_ops::maybe_add_namespace, which is called when
> fetching/updating/listing shared libraries.  That doesn't work well with
> multi-inferior for instance.  Imagine you stop at a breakpoint in
> inferior 1, which has 2 namespaces, _active_linker_namespaces is set to
> 2.  Then you run inferior 2, which has 2 namespaces, stop at a
> breakpoint, _active_linker_namespaces is set to 3.  If you focus back to
> inferior 1, _active_linker_namespaces will still have the value for
> inferior 2.  My feeling is that the variable should be sensitive to the
> user's focus.  That would probably mean calling
> solib_ops::num_active_namespaces on the current program space's
> solib_ops.

You're right, I didn't think about the multi-inferior case.

But I'm not sure if the answer should be in solib_ops or in the 
inferior. With that, svr4_solib_ops::maybe_add_namespace updated a value 
held by the inferior, and when you had multiple solib_ops in one 
inferior, the addition of the rocm solib_ops could also increment the 
count of namespaces. This would also solve the problem below, while 
leaking minimal solib information to inferior.

What do you think?

-- 
Cheers,
Guinevere Larsen
She/it

>
> And then, since we're talking about that, I was wondering how to handle
> _active_linker_namespaces in my "multiple solib ops" series.  Imagine
> you have a program space with one svr4_solib_ops and one rocm_solib_ops,
> which is how I envision it will work with my patch, when debugging a
> ROCm program.  How would I implement _active_linker_namespaces then
> (assuming you first made the change described above).  Should I iterate
> on all the solib_ops of the program space and sum up their
> solib_ops::num_active_namespaces?  In a normal scenario, that would give
> me 2, which is kinda right because the linker namespace on the host side
> is distinct from the linker namespace on the device side, so you
> effectively have 2 of them in the program space.  But is it going to
> work with the uses this convenience variable is intended for?  I don't
> really what the intended uses are.
>
> Simon
>


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

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

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-12 20:21 [PATCH 1/1] gdb: Print linker namespace when showing a frame Guinevere Larsen
2025-08-13 19:28 ` Simon Marchi
2025-08-13 20:18   ` Guinevere Larsen

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