[PATCH v2 1/2] gdb: merge solib-frv aix-solib debug options into "set/show debug solib"

[PATCH v2 1/2] gdb: merge solib-frv aix-solib debug options into "set/show debug solib"

[Simon Marchi]

solib implementations are typically used one at a time.  So it will be
rare that you will want to enable debug for one solib kind, and
absolutely want to keep the others disabled.  To make things simpler,
instead of adding separate variables / macros / commands for each solib
implementation, merge the existing ones (frv and aix) into a unified
"set/show debug solib", with the solib_debug_printf macro.

Change-Id: I6e18bbc7401724f37ae66681badb079d75ecf7fa
---
 gdb/NEWS            | 12 ++++++
 gdb/doc/gdb.texinfo | 18 +++------
 gdb/solib-aix.c     | 39 +++----------------
 gdb/solib-frv.c     | 95 +++++++++++----------------------------------
 gdb/solib.c         | 16 +++++---
 gdb/solib.h         | 12 ++++++
 6 files changed, 68 insertions(+), 124 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index dddef6525de8..c4ccfcc9e327 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.
@@ -166,6 +170,14 @@ maintenance info line-table
   entry corresponds to an address where a breakpoint should be placed
   to be at the first instruction past a function's prologue.
 
+* Removed commands
+
+set debug aix-solib on|off
+show debug aix-solib
+set debug solib-frv on|off
+show debug solib-frv
+  Removed in favor of "set/show debug solib".
+
 * New targets
 
 GNU/Linux/LoongArch (gdbserver)	loongarch*-*-linux*
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index bababf3c7ff8..5b5666699750 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -27556,13 +27556,6 @@ Turns on or off display of gdbarch debugging info.  The default is off
 @item show debug arch
 Displays the current state of displaying gdbarch debugging info.
 
-@item set debug aix-solib
-@cindex AIX shared library debugging
-Control display of debugging messages from the AIX shared library
-support module.  The default is off.
-@item show debug aix-solib
-Show the current state of displaying AIX shared library debugging messages.
-
 @item set debug aix-thread
 @cindex AIX threads
 Display debugging messages about inner workings of the AIX thread
@@ -27796,12 +27789,11 @@ default is off.
 Displays the current state of displaying @value{GDBN} serial debugging
 info.
 
-@item set debug solib-frv
-@cindex FR-V shared-library debugging
-Turn on or off debugging messages for FR-V shared-library code.
-@item show debug solib-frv
-Display the current state of FR-V shared-library code 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 symbol-lookup
 @cindex symbol lookup
diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index f483f54de13a..77ddb163d8c1 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -17,6 +17,7 @@
 
 #include "defs.h"
 #include "solib-aix.h"
+#include "solib.h"
 #include "solist.h"
 #include "inferior.h"
 #include "gdb_bfd.h"
@@ -28,15 +29,6 @@
 #include "gdbcmd.h"
 #include "gdbsupport/scope-exit.h"
 
-/* Variable controlling the output of the debugging traces for
-   this module.  */
-static bool solib_aix_debug;
-
-/* Print an "aix-solib" debug statement.  */
-
-#define solib_aix_debug_printf(fmt, ...) \
-  debug_prefixed_printf_cond (solib_aix_debug, "aix-solib",fmt, ##__VA_ARGS__)
-
 /* Our private data in struct so_list.  */
 
 struct lm_info_aix : public lm_info_base
@@ -257,8 +249,8 @@ solib_aix_get_library_list (struct inferior *inf, const char *warning_msg)
       return data->library_list;
     }
 
-  solib_aix_debug_printf ("TARGET_OBJECT_LIBRARIES_AIX = %s",
-			  library_document->data ());
+  solib_debug_printf ("TARGET_OBJECT_LIBRARIES_AIX = %s",
+		      library_document->data ());
 
   data->library_list = solib_aix_parse_libraries (library_document->data ());
   if (!data->library_list.has_value () && warning_msg != NULL)
@@ -379,7 +371,7 @@ solib_aix_free_so (struct so_list *so)
 {
   lm_info_aix *li = (lm_info_aix *) so->lm_info;
 
-  solib_aix_debug_printf ("%s", so->so_name);
+  solib_debug_printf ("%s", so->so_name);
 
   delete li;
 }
@@ -688,8 +680,8 @@ solib_aix_get_toc_value (CORE_ADDR pc)
 
   result = data_osect->addr () + xcoff_get_toc_offset (pc_osect->objfile);
 
-  solib_aix_debug_printf ("pc=%s -> %s", core_addr_to_string (pc),
-			  core_addr_to_string (result));
+  solib_debug_printf ("pc=%s -> %s", core_addr_to_string (pc),
+		      core_addr_to_string (result));
 
   return result;
 }
@@ -708,15 +700,6 @@ solib_aix_normal_stop_observer (struct bpstat *unused_1, int unused_2)
   data->library_list.reset ();
 }
 
-/* Implements the "show debug aix-solib" command.  */
-
-static void
-show_solib_aix_debug (struct ui_file *file, int from_tty,
-		      struct cmd_list_element *c, const char *value)
-{
-  gdb_printf (file, _("solib-aix debugging is %s.\n"), value);
-}
-
 /* The target_so_ops for AIX targets.  */
 const struct target_so_ops solib_aix_so_ops =
 {
@@ -737,14 +720,4 @@ _initialize_solib_aix ()
 {
   gdb::observers::normal_stop.attach (solib_aix_normal_stop_observer,
 				      "solib-aix");
-
-  /* Debug this file's internals.  */
-  add_setshow_boolean_cmd ("aix-solib", class_maintenance,
-			   &solib_aix_debug, _("\
-Control the debugging traces for the solib-aix module."), _("\
-Show whether solib-aix debugging traces are enabled."), _("\
-When on, solib-aix debugging traces are enabled."),
-			    NULL,
-			    show_solib_aix_debug,
-			    &setdebuglist, &showdebuglist);
 }
diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index 6ca303c35662..5a8e6874398e 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -31,9 +31,6 @@
 #include "elf/frv.h"
 #include "gdb_bfd.h"
 
-/* Flag which indicates whether internal debug messages should be printed.  */
-static unsigned int solib_frv_debug;
-
 /* FR-V pointers are four bytes wide.  */
 enum { FRV_PTR_SIZE = 4 };
 
@@ -290,27 +287,21 @@ lm_base (void)
 				   current_program_space->symfile_object_file);
   if (got_sym.minsym == 0)
     {
-      if (solib_frv_debug)
-	gdb_printf (gdb_stdlog,
-		    "lm_base: _GLOBAL_OFFSET_TABLE_ not found.\n");
+      solib_debug_printf ("_GLOBAL_OFFSET_TABLE_ not found.");
       return 0;
     }
 
   addr = got_sym.value_address () + 8;
 
-  if (solib_frv_debug)
-    gdb_printf (gdb_stdlog,
-		"lm_base: _GLOBAL_OFFSET_TABLE_ + 8 = %s\n",
-		hex_string_custom (addr, 8));
+  solib_debug_printf ("_GLOBAL_OFFSET_TABLE_ + 8 = %s",
+		      hex_string_custom (addr, 8));
 
   if (target_read_memory (addr, buf, sizeof buf) != 0)
     return 0;
   lm_base_cache = extract_unsigned_integer (buf, sizeof buf, byte_order);
 
-  if (solib_frv_debug)
-    gdb_printf (gdb_stdlog,
-		"lm_base: lm_base_cache = %s\n",
-		hex_string_custom (lm_base_cache, 8));
+  solib_debug_printf ("lm_base_cache = %s",
+		      hex_string_custom (lm_base_cache, 8));
 
   return lm_base_cache;
 }
@@ -354,10 +345,8 @@ frv_current_sos (void)
       struct ext_link_map lm_buf;
       CORE_ADDR got_addr;
 
-      if (solib_frv_debug)
-	gdb_printf (gdb_stdlog,
-		    "current_sos: reading link_map entry at %s\n",
-		    hex_string_custom (lm_addr, 8));
+      solib_debug_printf ("reading link_map entry at %s",
+			  hex_string_custom (lm_addr, 8));
 
       if (target_read_memory (lm_addr, (gdb_byte *) &lm_buf,
 			      sizeof (lm_buf)) != 0)
@@ -405,10 +394,8 @@ frv_current_sos (void)
 	  gdb::unique_xmalloc_ptr<char> name_buf
 	    = target_read_string (addr, SO_NAME_MAX_PATH_SIZE - 1);
 
-	  if (solib_frv_debug)
-	    gdb_printf (gdb_stdlog, "current_sos: name = %s\n",
-			name_buf.get ());
-	  
+	  solib_debug_printf ("name = %s", name_buf.get ());
+
 	  if (name_buf == nullptr)
 	    warning (_("Can't read pathname for link map entry."));
 	  else
@@ -582,10 +569,8 @@ enable_break2 (void)
 	  return 0;
 	}
 
-      if (solib_frv_debug)
-	gdb_printf (gdb_stdlog,
-		    "enable_break: interp_loadmap_addr = %s\n",
-		    hex_string_custom (interp_loadmap_addr, 8));
+      solib_debug_printf ("interp_loadmap_addr = %s",
+			  hex_string_custom (interp_loadmap_addr, 8));
 
       ldm = fetch_loadmap (interp_loadmap_addr);
       if (ldm == NULL)
@@ -627,19 +612,13 @@ enable_break2 (void)
 	  return 0;
 	}
 
-      if (solib_frv_debug)
-	gdb_printf (gdb_stdlog,
-		    "enable_break: _dl_debug_addr "
-		    "(prior to relocation) = %s\n",
-		    hex_string_custom (addr, 8));
+      solib_debug_printf ("_dl_debug_addr (prior to relocation) = %s",
+			  hex_string_custom (addr, 8));
 
       addr += displacement_from_map (ldm, addr);
 
-      if (solib_frv_debug)
-	gdb_printf (gdb_stdlog,
-		    "enable_break: _dl_debug_addr "
-		    "(after relocation) = %s\n",
-		    hex_string_custom (addr, 8));
+      solib_debug_printf ("_dl_debug_addr (after relocation) = %s",
+			  hex_string_custom (addr, 8));
 
       /* Fetch the address of the r_debug struct.  */
       if (target_read_memory (addr, addr_buf, sizeof addr_buf) != 0)
@@ -650,18 +629,14 @@ enable_break2 (void)
 	}
       addr = extract_unsigned_integer (addr_buf, sizeof addr_buf, byte_order);
 
-      if (solib_frv_debug)
-	gdb_printf (gdb_stdlog,
-		    "enable_break: _dl_debug_addr[0..3] = %s\n",
-		    hex_string_custom (addr, 8));
+      solib_debug_printf ("_dl_debug_addr[0..3] = %s",
+			  hex_string_custom (addr, 8));
 
       /* If it's zero, then the ldso hasn't initialized yet, and so
 	 there are no shared libs yet loaded.  */
       if (addr == 0)
 	{
-	  if (solib_frv_debug)
-	    gdb_printf (gdb_stdlog,
-			"enable_break: ldso not yet initialized\n");
+	  solib_debug_printf ("ldso not yet initialized");
 	  /* Do not warn, but mark to run again.  */
 	  return 0;
 	}
@@ -719,17 +694,13 @@ enable_break (void)
 
   if (current_program_space->symfile_object_file == NULL)
     {
-      if (solib_frv_debug)
-	gdb_printf (gdb_stdlog,
-		    "enable_break: No symbol file found.\n");
+      solib_debug_printf ("No symbol file found.");
       return 0;
     }
 
   if (!entry_point_address_query (&entry_point))
     {
-      if (solib_frv_debug)
-	gdb_printf (gdb_stdlog,
-		    "enable_break: Symbol file has no entry point.\n");
+      solib_debug_printf ("Symbol file has no entry point.");
       return 0;
     }
 
@@ -741,19 +712,14 @@ enable_break (void)
 
   if (interp_sect == NULL)
     {
-      if (solib_frv_debug)
-	gdb_printf (gdb_stdlog,
-		    "enable_break: No .interp section found.\n");
+      solib_debug_printf ("No .interp section found.");
       return 0;
     }
 
   create_solib_event_breakpoint (target_gdbarch (), entry_point);
 
-  if (solib_frv_debug)
-    gdb_printf (gdb_stdlog,
-		"enable_break: solib event breakpoint "
-		"placed at entry point: %s\n",
-		hex_string_custom (entry_point, 8));
+  solib_debug_printf ("solib event breakpoint placed at entry point: %s",
+		      hex_string_custom (entry_point, 8));
   return 1;
 }
 
@@ -1141,18 +1107,3 @@ const struct target_so_ops frv_so_ops =
   frv_in_dynsym_resolve_code,
   solib_bfd_open,
 };
-
-void _initialize_frv_solib ();
-void
-_initialize_frv_solib ()
-{
-  /* Debug this file's internals.  */
-  add_setshow_zuinteger_cmd ("solib-frv", class_maintenance,
-			     &solib_frv_debug, _("\
-Set internal debugging of shared library code for FR-V."), _("\
-Show internal debugging of shared library code for FR-V."), _("\
-When non-zero, FR-V solib specific internal debugging is enabled."),
-			     NULL,
-			     NULL, /* FIXME: i18n: */
-			     &setdebuglist, &showdebuglist);
-}
diff --git a/gdb/solib.c b/gdb/solib.c
index 7cfdd81114c4..59fd866b6529 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.  */
-
-\f
-
-/* 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
@@ -1808,4 +1804,12 @@ PATH and LD_LIBRARY_PATH."),
 				     reload_shared_libraries,
 				     show_solib_search_path,
 				     &setlist, &showlist);
+
+  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);
 }
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.  */
 

base-commit: 4a6bdfb9baa27e29151c7e97ae2abbe902f53638
-- 
2.38.1

[PATCH v2 2/2] gdb: add some debug statements to solib-svr4.c

[Simon Marchi]

From: Simon Marchi <simon.marchi@efficios.com>

Add a few debug statements that were useful to me when debugging why the
glibc probes interface wasn't getting used.

Change-Id: Ic20744f9fc80a90f196896b0829949411620c540
---
 gdb/solib-svr4.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 6acaf87960bf..357059c09486 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -2168,6 +2168,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 +2188,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 +2210,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 +2244,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 +2270,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.  */
-- 
2.38.1

Re: [PATCH v2 1/2] gdb: merge solib-frv aix-solib debug options into "set/show debug solib"

[Eli Zaretskii]

> Cc: Simon Marchi <simon.marchi@polymtl.ca>
> Date: Mon, 28 Nov 2022 12:00:48 -0500
> From: Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
> 
> solib implementations are typically used one at a time.  So it will be
> rare that you will want to enable debug for one solib kind, and
> absolutely want to keep the others disabled.  To make things simpler,
> instead of adding separate variables / macros / commands for each solib
> implementation, merge the existing ones (frv and aix) into a unified
> "set/show debug solib", with the solib_debug_printf macro.
> 
> Change-Id: I6e18bbc7401724f37ae66681badb079d75ecf7fa
> ---
>  gdb/NEWS            | 12 ++++++
>  gdb/doc/gdb.texinfo | 18 +++------
>  gdb/solib-aix.c     | 39 +++----------------
>  gdb/solib-frv.c     | 95 +++++++++++----------------------------------
>  gdb/solib.c         | 16 +++++---
>  gdb/solib.h         | 12 ++++++
>  6 files changed, 68 insertions(+), 124 deletions(-)

OK for the documentation parts, thanks.

I wonder what the others think about removing those two commands?  Do we
have any policy regarding that?  Maybe leave them as aliases for the new
command?

Re: [PATCH v2 1/2] gdb: merge solib-frv aix-solib debug options into "set/show debug solib"

[Simon Marchi]

On 11/28/22 12:24, Eli Zaretskii wrote:
>> Cc: Simon Marchi <simon.marchi@polymtl.ca>
>> Date: Mon, 28 Nov 2022 12:00:48 -0500
>> From: Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
>>
>> solib implementations are typically used one at a time.  So it will be
>> rare that you will want to enable debug for one solib kind, and
>> absolutely want to keep the others disabled.  To make things simpler,
>> instead of adding separate variables / macros / commands for each solib
>> implementation, merge the existing ones (frv and aix) into a unified
>> "set/show debug solib", with the solib_debug_printf macro.
>>
>> Change-Id: I6e18bbc7401724f37ae66681badb079d75ecf7fa
>> ---
>>  gdb/NEWS            | 12 ++++++
>>  gdb/doc/gdb.texinfo | 18 +++------
>>  gdb/solib-aix.c     | 39 +++----------------
>>  gdb/solib-frv.c     | 95 +++++++++++----------------------------------
>>  gdb/solib.c         | 16 +++++---
>>  gdb/solib.h         | 12 ++++++
>>  6 files changed, 68 insertions(+), 124 deletions(-)
> 
> OK for the documentation parts, thanks.
> 
> I wonder what the others think about removing those two commands?  Do we
> have any policy regarding that?  Maybe leave them as aliases for the new
> command?

Tom mentioned he's fine with it here:

  https://inbox.sourceware.org/gdb-patches/94b7d01e-509f-eeaa-ff04-90be8d860868@polymtl.ca/T/#m7f63f08de9db666a51ebbe165a916c51000736c3

Like I said earlier, I believe it's fine to change these debug commands.
They exist essentially for people debugging some trouble with GDB
itself, not for the everyday user.  I think it's fine to consider
them as maintenance commands (and even move them to the maint prefix one
day, as Tom mentioned).

Given I see them as maintenance commands, I'd rather not add aliases,
unless you have strong feelings about it.

Simon

Re: [PATCH v2 1/2] gdb: merge solib-frv aix-solib debug options into "set/show debug solib"

[Eli Zaretskii]

> Date: Mon, 28 Nov 2022 13:29:54 -0500
> Cc: gdb-patches@sourceware.org
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> Given I see them as maintenance commands, I'd rather not add aliases,
> unless you have strong feelings about it.

If everyone else is fine with the deletion, I'm fine as well.

Thanks.

Re: [PATCH v2 1/2] gdb: merge solib-frv aix-solib debug options into "set/show debug solib"

[Simon Marchi]

On 11/28/22 13:38, Eli Zaretskii wrote:
>> Date: Mon, 28 Nov 2022 13:29:54 -0500
>> Cc: gdb-patches@sourceware.org
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>>
>> Given I see them as maintenance commands, I'd rather not add aliases,
>> unless you have strong feelings about it.
> 
> If everyone else is fine with the deletion, I'm fine as well.
> 
> Thanks.

Ok, thanks, I pushed both patches.

Simon