gdb: add "set debug solib" and use it

Message ID 20221121193636.1479004-1-simon.marchi@polymtl.ca
State New
Headers
Series gdb: add "set debug solib" and use it |

Commit Message

Simon Marchi Nov. 21, 2022, 7:36 p.m. UTC
  From: Simon Marchi <simon.marchi@efficios.com>

I'd like to add some debug output to solib-svr4.  Add a "set debug
solib" knob, which can then be used in all solib-* files.  Then, add
the debug output I needed, related to the svr4 shared library event
breakpoint probes.

Change-Id: Ic20744f9fc80a90f196896b0829949411620c540
---
 gdb/NEWS            |  4 ++++
 gdb/doc/gdb.texinfo |  6 ++++++
 gdb/solib-svr4.c    | 23 ++++++++++++++++++++++-
 gdb/solib.c         |  8 ++------
 gdb/solib.h         | 12 ++++++++++++
 5 files changed, 46 insertions(+), 7 deletions(-)


base-commit: 1a7e622b82fa621ab505dd911bc30c1efcb4a9b0
  

Comments

Eli Zaretskii Nov. 21, 2022, 7:56 p.m. UTC | #1
> Cc: Simon Marchi <simon.marchi@efficios.com>
> Date: Mon, 21 Nov 2022 14:36:36 -0500
> From: Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
> 
> From: Simon Marchi <simon.marchi@efficios.com>
> 
> I'd like to add some debug output to solib-svr4.  Add a "set debug
> solib" knob, which can then be used in all solib-* files.  Then, add
> the debug output I needed, related to the svr4 shared library event
> breakpoint probes.

Not sure I understand: we already have "set debug aix-solib" and "set debug
solib-frv".  So this is going to be "set debug svr4-solib"?  Or do you mean
to make a single "set debug solib" command for all platforms?

Thanks.

>  gdb/NEWS            |  4 ++++
>  gdb/doc/gdb.texinfo |  6 ++++++
>  gdb/solib-svr4.c    | 23 ++++++++++++++++++++++-
>  gdb/solib.c         |  8 ++------
>  gdb/solib.h         | 12 ++++++++++++
>  5 files changed, 46 insertions(+), 7 deletions(-)

The documentation parts are okay, but the text sounds as if this command is
available on all platforms.
  
Simon Marchi Nov. 21, 2022, 8:02 p.m. UTC | #2
On 11/21/22 14:56, Eli Zaretskii wrote:
>> Cc: Simon Marchi <simon.marchi@efficios.com>
>> Date: Mon, 21 Nov 2022 14:36:36 -0500
>> From: Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
>>
>> From: Simon Marchi <simon.marchi@efficios.com>
>>
>> I'd like to add some debug output to solib-svr4.  Add a "set debug
>> solib" knob, which can then be used in all solib-* files.  Then, add
>> the debug output I needed, related to the svr4 shared library event
>> breakpoint probes.
> 
> Not sure I understand: we already have "set debug aix-solib" and "set debug
> solib-frv".  So this is going to be "set debug svr4-solib"?  Or do you mean
> to make a single "set debug solib" command for all platforms?

The new command was intended to be used by all solib implementations,
but I missed the existing commands you pointed out.  I think it would be
fine to have a single knob for all solib implementations.  In general,
only one solib implementation will be used at any given time.

If you agree, I would remove the two existing commands in favor of the
new "set debug solib", and then add my solib-svr4 changes.

> 
> Thanks.
> 
>>  gdb/NEWS            |  4 ++++
>>  gdb/doc/gdb.texinfo |  6 ++++++
>>  gdb/solib-svr4.c    | 23 ++++++++++++++++++++++-
>>  gdb/solib.c         |  8 ++------
>>  gdb/solib.h         | 12 ++++++++++++
>>  5 files changed, 46 insertions(+), 7 deletions(-)
> 
> The documentation parts are okay, but the text sounds as if this command is
> available on all platforms.

That is the intention.

Simon
  
Eli Zaretskii Nov. 21, 2022, 8:19 p.m. UTC | #3
> Date: Mon, 21 Nov 2022 15:02:08 -0500
> Cc: gdb-patches@sourceware.org
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> > Not sure I understand: we already have "set debug aix-solib" and "set debug
> > solib-frv".  So this is going to be "set debug svr4-solib"?  Or do you mean
> > to make a single "set debug solib" command for all platforms?
> 
> The new command was intended to be used by all solib implementations,
> but I missed the existing commands you pointed out.  I think it would be
> fine to have a single knob for all solib implementations.  In general,
> only one solib implementation will be used at any given time.
> 
> If you agree, I would remove the two existing commands in favor of the
> new "set debug solib", and then add my solib-svr4 changes.

I have no use for those two commands, but I guess users of AIX and FRV do?
Can we just delete the commands from under their feet?
  
Simon Marchi Nov. 21, 2022, 8:24 p.m. UTC | #4
On 11/21/22 15:19, Eli Zaretskii wrote:
>> Date: Mon, 21 Nov 2022 15:02:08 -0500
>> Cc: gdb-patches@sourceware.org
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>>
>>> Not sure I understand: we already have "set debug aix-solib" and "set debug
>>> solib-frv".  So this is going to be "set debug svr4-solib"?  Or do you mean
>>> to make a single "set debug solib" command for all platforms?
>>
>> The new command was intended to be used by all solib implementations,
>> but I missed the existing commands you pointed out.  I think it would be
>> fine to have a single knob for all solib implementations.  In general,
>> only one solib implementation will be used at any given time.
>>
>> If you agree, I would remove the two existing commands in favor of the
>> new "set debug solib", and then add my solib-svr4 changes.
> 
> I have no use for those two commands, but I guess users of AIX and FRV do?
> Can we just delete the commands from under their feet?

I tend to view "set debug" commands, as internal commands (maybe they
should be maintenance commands, really), I wouldn't feel bad to just
remove them.  Or we could keep them as deprecated aliases of the new
command.  Or, I could just add "set debug solib-svr", if that makes
everyone happier.

Simon
  
Simon Marchi Nov. 21, 2022, 8:24 p.m. UTC | #5
On 11/21/22 15:24, Simon Marchi wrote:
> 
> 
> On 11/21/22 15:19, Eli Zaretskii wrote:
>>> Date: Mon, 21 Nov 2022 15:02:08 -0500
>>> Cc: gdb-patches@sourceware.org
>>> From: Simon Marchi <simon.marchi@polymtl.ca>
>>>
>>>> Not sure I understand: we already have "set debug aix-solib" and "set debug
>>>> solib-frv".  So this is going to be "set debug svr4-solib"?  Or do you mean
>>>> to make a single "set debug solib" command for all platforms?
>>>
>>> The new command was intended to be used by all solib implementations,
>>> but I missed the existing commands you pointed out.  I think it would be
>>> fine to have a single knob for all solib implementations.  In general,
>>> only one solib implementation will be used at any given time.
>>>
>>> If you agree, I would remove the two existing commands in favor of the
>>> new "set debug solib", and then add my solib-svr4 changes.
>>
>> I have no use for those two commands, but I guess users of AIX and FRV do?
>> Can we just delete the commands from under their feet?
> 
> I tend to view "set debug" commands, as internal commands (maybe they
> should be maintenance commands, really), I wouldn't feel bad to just
> remove them.  Or we could keep them as deprecated aliases of the new
> command.  Or, I could just add "set debug solib-svr", if that makes

solib-svr4, sorry.

Simon
  
Tom Tromey Nov. 28, 2022, 3:27 p.m. UTC | #6
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> I tend to view "set debug" commands, as internal commands (maybe they
Simon> should be maintenance commands, really), I wouldn't feel bad to just
Simon> remove them.

Yes, I think that's fine.

And I also agree that we should probably move these to "maint" someday.

Tom
  
Simon Marchi Nov. 28, 2022, 4:57 p.m. UTC | #7
On 11/21/22 14:56, Eli Zaretskii wrote:
> Not sure I understand: we already have "set debug aix-solib" and "set debug
> solib-frv".  So this is going to be "set debug svr4-solib"?  Or do you mean
> to make a single "set debug solib" command for all platforms?

I just realized why this was confusing.  I added the command to
solib-svr4.c instead of solib.c, even though it was meant to affect all
solib implementations.  It will be fixed in v2.

Simon
  

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index dddef6525de8..d36aa0707629 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -145,6 +145,10 @@  set debug infcall on|off
 show debug infcall
   Print additional debug messages about inferior function calls.
 
+set debug solib on|off
+show debug solib
+  Print additional debug messages about shared library handling.
+
 set style tui-current-position [on|off]
   Whether to style the source and assembly code highlighted by the
   TUI's current position indicator.  The default is off.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index bababf3c7ff8..ce4dae11f0d2 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -27812,6 +27812,12 @@  A value greater than 1 provides more verbose information.
 @item show debug symbol-lookup
 Show the current state of symbol lookup debugging messages.
 
+@item set debug solib
+Turns on or off display of debugging messages related to shared libraries.
+The default is off.
+@item show debug solib
+Show the current state of solib debugging messages.
+
 @item set debug symfile
 @cindex symbol file functions
 Turns on or off display of debugging messages related to symbol file functions.
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 6acaf87960bf..d2c1cfa7851e 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -34,6 +34,7 @@ 
 #include "regcache.h"
 #include "gdbthread.h"
 #include "observable.h"
+#include "cli/cli-cmds.h"
 
 #include "solist.h"
 #include "solib.h"
@@ -2168,6 +2169,9 @@  svr4_create_probe_breakpoints (svr4_info *info, struct gdbarch *gdbarch,
 	{
 	  CORE_ADDR address = p->get_relocated_address (objfile);
 
+	  solib_debug_printf ("name=%s, addr=%s", probe_info[i].name,
+			      paddress (gdbarch, address));
+
 	  create_solib_event_breakpoint (gdbarch, address);
 	  register_solib_event_probe (info, objfile, p, address, action);
 	}
@@ -2185,6 +2189,9 @@  svr4_find_and_create_probe_breakpoints (svr4_info *info,
 					struct obj_section *os,
 					bool with_prefix)
 {
+  SOLIB_SCOPED_DEBUG_START_END ("objfile=%s, with_prefix=%d",
+				os->objfile->original_name, with_prefix);
+
   std::vector<probe *> probes[NUM_PROBES];
 
   for (int i = 0; i < NUM_PROBES; i++)
@@ -2204,6 +2211,7 @@  svr4_find_and_create_probe_breakpoints (svr4_info *info,
 	}
 
       probes[i] = find_probes_in_objfile (os->objfile, "rtld", name);
+      solib_debug_printf ("probe=%s, num found=%zu", name, probes[i].size ());
 
       /* The "map_failed" probe did not exist in early
 	 versions of the probes code in which the probes'
@@ -2237,6 +2245,7 @@  svr4_find_and_create_probe_breakpoints (svr4_info *info,
     }
 
   /* All probes found.  Now create them.  */
+  solib_debug_printf ("using probes interface");
   svr4_create_probe_breakpoints (info, gdbarch, probes, os->objfile);
   return true;
 }
@@ -2262,7 +2271,11 @@  svr4_create_solib_event_breakpoints (svr4_info *info, struct gdbarch *gdbarch,
   if (os == nullptr
       || (!svr4_find_and_create_probe_breakpoints (info, gdbarch, os, false)
 	  && !svr4_find_and_create_probe_breakpoints (info, gdbarch, os, true)))
-    create_solib_event_breakpoint (gdbarch, address);
+    {
+      solib_debug_printf ("falling back to r_brk breakpoint: addr=%s",
+			  paddress (gdbarch, address));
+      create_solib_event_breakpoint (gdbarch, address);
+    }
 }
 
 /* Helper function for gdb_bfd_lookup_symbol.  */
@@ -3449,6 +3462,14 @@  void _initialize_svr4_solib ();
 void
 _initialize_svr4_solib ()
 {
+  add_setshow_boolean_cmd ("solib", class_maintenance,
+			   &debug_solib, _("\
+Set solib debugging."), _("\
+Show solib debugging."), _("\
+When true, solib-related debugging output is enabled."),
+			    nullptr, nullptr,
+			    &setdebuglist, &showdebuglist);
+
   gdb::observers::free_objfile.attach (svr4_free_objfile_observer,
 				       "solib-svr4");
 }
diff --git a/gdb/solib.c b/gdb/solib.c
index 7cfdd81114c4..aafdb85db3be 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -55,13 +55,9 @@ 
 #include "cli/cli-style.h"
 #include "solib-target.h"
 
-/* Architecture-specific operations.  */
-
-
-
-/* external data declarations */
+/* See solib.h.  */
 
-/* Local function prototypes */
+bool debug_solib;
 
 /* If non-empty, this is a search path for loading non-absolute shared library
    symbol files.  This takes precedence over the environment variables PATH
diff --git a/gdb/solib.h b/gdb/solib.h
index a7e751ed9b31..74066680ef9c 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -29,6 +29,18 @@  struct program_space;
 #include "gdb_bfd.h"
 #include "symfile-add-flags.h"
 
+/* Value of the 'set debug solib' configuration variable.  */
+
+extern bool debug_solib;
+
+/* Print an "solib" debug statement.  */
+
+#define solib_debug_printf(fmt, ...) \
+  debug_prefixed_printf_cond (debug_solib, "solib", fmt, ##__VA_ARGS__)
+
+#define SOLIB_SCOPED_DEBUG_START_END(fmt, ...) \
+  scoped_debug_start_end (debug_solib, "solib", fmt, ##__VA_ARGS__)
+
 /* Called when we free all symtabs, to free the shared library information
    as well.  */