public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] On-demand loading of shlib's debuginfo
@ 2011-07-19  5:34 Sergio Durigan Junior
  2011-07-20 22:01 ` Jan Kratochvil
  0 siblings, 1 reply; 9+ messages in thread
From: Sergio Durigan Junior @ 2011-07-19  5:34 UTC (permalink / raw)
  To: gdb-patches

Hello guys,

Debugging large binaries, linked with lots of shared libraries, is
sometime slow because of the time necessary for GDB to load all the
debuginfo files related to those libraries.  This has also been
something problematic for the ABRT tool, and recently Tromey pointed me
to this bug:

   https://bugzilla.mozilla.org/show_bug.cgi?id=184013

Which shows that the Mozilla guys are using the `set auto-solib-add 0'
trick in order to avoid the immediate loading of shared libraries'
debuginfo.  With that in mind, we decided to tackle this problem
progressively, and the first part of the solution is ready for
submission.

This patch modifies the code so that the on-demand loading is possible.
What it does is:

1) Change the name of the `auto-solib-add' command to just `solib-add'
(deprecatin `auto-solib-add').  Also, make the command be a tri-state,
with `on', `off', and `lazy' options.  The default option is still `on'.

2) Implement the `lazy' option.  For that, I have created a new argument
on the function `solib_add', named `lazy_read', which informs whether we
are lazily reading or not.  If yes, then we shouldn't reinit_frame_cache
because it would invalidate all the frames (and if we're lazily reading
things, it means we are executing a command -- for example
`backtrace' -- and invalidating the frames would make the command not
work properly).

3) This patch only affects the `backtrace' command for now, so if the
user asks for the backtrace, and we are iterating over the PCs for every
frame, we call the `solib_on_demand_load' in order to know which shared
library we must load on-demand.  Given the PC, we iterate over the
shared library list and match the PC against the link map address,
loading the matched library.

4) Update the docs, citing the new `lazy' and the new `solib-add'
command.

Ok, so I think that is what this patch offers.  Comments and suggestions
are welcome, as usual.

Thank you very much,

Sergio.

2011-07-18  Sergio Durigan Junior  <sergiodj@redhat.com>

	* breakpoint.c (bpstat_what): Add `lazy_read' argument to
	`solib_add' call.
	* infcmd.c (post_create_inferior): Likewise.
	* infrun.c (handle_inferior_event): Likewise.
	* remote.c (remote_start_remote): Likewise.
	* solib-ia64-hpux.c (ia64_hpux_handle_dld_breakpoint_1): Likewise.
	* solib-irix.c (irix_solib_create_inferior_hook): Likewise.
	* solib-osf.c (osf_solib_create_inferior_hook): Likewise.
	* solib-sunos.c (sunos_solib_create_inferior_hook): Likewise.
	* solib-svr4.c (svr4_fetch_objfile_link_map): Likewise.
	* windows-nat.c (handle_unload_dll): Likewise.
	(enable_break): Likewise.
	(svr4_match_pc_solist): New function.
	(_initialize_svr4_solib): Initialize `match_pc_solist' field.
	* solib.c (solib_add_choice): New variable.
	(solib_add_options): Likewise.
	(solib_on_demand_load): New function.
	(solib_add): Add new argument `lazy_read'.  Do not call
	`reinit_frame_cache' if on-demand loading is active.
	(solib_match_pc_solist): New function.
	(sharedlibrary_command): Add `lazy_read' argument to `solib_add'
	call.
	(reload_shared_libraries_1): Treat `SOLIB_ADD_ON'.
	(reload_shared_libraries): Add `lazy_read' argument to `solib_add'
	call.
	(set_solib_add): New function.
	(_initialize_solib): Register new command `solib-add'.  Deprecate
	command `auto-solib-add'.
	* solib.h (enum solib_add_opt): New variable declaration.
	(solib_add): Change prototype.
	(solib_on_demand_load): New function.
	* solist.h (struct target_so_ops) <solib_match_pc_solist>: New
	function pointer.
	* symfile.c (auto_solib_add): Replace by...
	(solib_add_opt): ...this.  Also, update the comment to reflect the
	new lazy option.
	* symfile.h (enum solib_add_opt): New enum which contains the
	possible values for `solib-add'.
	(auto_solib_add): Replace by...
	(solib_add_opt): ...this.  Also, update the comment to reflect the
	new lazy option.
	* xxcoffsolib.c (_initialize_xcoffsolib): Register new command
	`solib-add'.
	* rs6000-nat.c (add_vmap): Treat `SOLIB_ADD_ON'.
	* frame.c (frame_unwind_pc_if_available): Call `solib_on_demand_load'
	when loading debuginfo on-demand.
	* config/rs6000/nm-rs6000.h: Update `SOLIB_ADD' macro in order to
	reflect the new `lazy_read' argument.

2011-07-18  Sergio Durigan Junior  <sergiodj@redhat.com>

	* gdb.texinfo: Update `auto-solib-add' entry, citing that
	the command has been deprecated in favor of `solib-add', and
	mentioning that it has a new option called `lazy'.  Also, explain
	this option.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 802c8a1..f1b9e30 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4593,9 +4593,9 @@ bpstat_what (bpstat bs_head)
       target_terminal_ours_for_output ();
 
 #ifdef SOLIB_ADD
-      SOLIB_ADD (NULL, 0, &current_target, auto_solib_add);
+      SOLIB_ADD (NULL, 0, &current_target, solib_add_opt, /*lazy_read=*/0);
 #else
-      solib_add (NULL, 0, &current_target, auto_solib_add);
+      solib_add (NULL, 0, &current_target, solib_add_opt, /*lazy_read=*/0);
 #endif
 
       target_terminal_inferior ();
diff --git a/gdb/config/rs6000/nm-rs6000.h b/gdb/config/rs6000/nm-rs6000.h
index 380850c..769fb9e 100644
--- a/gdb/config/rs6000/nm-rs6000.h
+++ b/gdb/config/rs6000/nm-rs6000.h
@@ -29,7 +29,7 @@
 /* When a target process or core-file has been attached, we sneak in
    and figure out where the shared libraries have got to.  */
 
-#define	SOLIB_ADD(a, b, c, d)	\
+#define	SOLIB_ADD(a, b, c, d, e)	\
   if (PIDGET (inferior_ptid))	\
     /* Attach to process.  */  \
     xcoff_relocate_symtab (PIDGET (inferior_ptid)); \
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 3a3a9fb..b46e80b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -14948,14 +14948,18 @@ will be loaded automatically when the inferior begins execution, you
 attach to an independently started inferior, or when the dynamic linker
 informs @value{GDBN} that a new library has been loaded.  If @var{mode}
 is @code{off}, symbols must be loaded manually, using the
-@code{sharedlibrary} command.  The default value is @code{on}.
+@code{sharedlibrary} command.  If @var{mode} is @code{lazy}, then
+@value{GDBN} will lazily load symbols from shared libraries, i.e., it
+will load symbols on-demand.  The default value is @code{on}.
+
+This command is now deprecated.  Use @code{set solib-add} instead.
 
 @cindex memory used for symbol tables
 If your program uses lots of shared libraries with debug info that
 takes large amounts of memory, you can decrease the @value{GDBN}
 memory footprint by preventing it from automatically loading the
 symbols from shared libraries.  To that end, type @kbd{set
-auto-solib-add off} before running the inferior, then load each
+solib-add off} before running the inferior, then load each
 library whose debug symbols you do need with @kbd{sharedlibrary
 @var{regexp}}, where @var{regexp} is a regular expression that matches
 the libraries whose symbols you want to be loaded.
@@ -14963,6 +14967,8 @@ the libraries whose symbols you want to be loaded.
 @kindex show auto-solib-add
 @item show auto-solib-add
 Display the current autoloading mode.
+
+This command is now deprecated.  Use @code{show solib-add} instead.
 @end table
 
 @cindex load shared library
diff --git a/gdb/frame.c b/gdb/frame.c
index 7a35192..43c20c8 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -45,6 +45,8 @@
 #include "block.h"
 #include "inline-frame.h"
 #include  "tracepoint.h"
+#include "solist.h"
+#include "solib.h"
 
 static struct frame_info *get_prev_frame_1 (struct frame_info *this_frame);
 static struct frame_info *get_prev_frame_raw (struct frame_info *this_frame);
@@ -677,6 +679,9 @@ frame_unwind_pc_if_available (struct frame_info *this_frame, CORE_ADDR *pc)
 				    this_frame->level,
 				    hex_string (this_frame->prev_pc.value));
 	    }
+
+	  if (solib_add_opt == SOLIB_ADD_LAZY)
+	    solib_on_demand_load (pc);
 	}
       else
 	internal_error (__FILE__, __LINE__, _("No unwind_pc method"));
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index aa3646b..29fd37d 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -442,9 +442,9 @@ post_create_inferior (struct target_ops *target, int from_tty)
 	 solib_create_inferior_hook.  */
 
 #ifdef SOLIB_ADD
-      SOLIB_ADD (NULL, 0, target, auto_solib_add);
+      SOLIB_ADD (NULL, 0, target, solib_add_opt, /*lazy_read=*/0);
 #else
-      solib_add (NULL, 0, target, auto_solib_add);
+      solib_add (NULL, 0, target, solib_add_opt, /*lazy_read=*/0);
 #endif
     }
 
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 2b4525e..036baf6 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3252,9 +3252,11 @@ handle_inferior_event (struct execution_control_state *ecs)
 	     require the table to contain all sections (including
 	     those found in shared libraries).  */
 #ifdef SOLIB_ADD
-	  SOLIB_ADD (NULL, 0, &current_target, auto_solib_add);
+	  SOLIB_ADD (NULL, 0, &current_target, solib_add_opt,
+		     /*lazy_read=*/0);
 #else
-	  solib_add (NULL, 0, &current_target, auto_solib_add);
+	  solib_add (NULL, 0, &current_target, solib_add_opt,
+		     /*lazy_read=*/0);
 #endif
 	  target_terminal_inferior ();
 
diff --git a/gdb/remote.c b/gdb/remote.c
index cbc7daa..b9c038d 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3207,7 +3207,7 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
   /* On OSs where the list of libraries is global to all
      processes, we fetch them early.  */
   if (gdbarch_has_global_solist (target_gdbarch))
-    solib_add (NULL, from_tty, target, auto_solib_add);
+    solib_add (NULL, from_tty, target, solib_add_opt, /*lazy_read=*/0);
 
   if (non_stop)
     {
diff --git a/gdb/rs6000-nat.c b/gdb/rs6000-nat.c
index 30e4c15..3cec7c7 100644
--- a/gdb/rs6000-nat.c
+++ b/gdb/rs6000-nat.c
@@ -801,7 +801,7 @@ add_vmap (LdInfo *ldi)
   vp->objfile = obj;
 
   /* Always add symbols for the main objfile.  */
-  if (vp == vmap || auto_solib_add)
+  if (vp == vmap || solib_add_opt == SOLIB_ADD_ON)
     vmap_add_symbols (vp);
   return vp;
 }
diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index 7299cef..a791c50 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -1302,7 +1302,7 @@ frv_fetch_objfile_link_map (struct objfile *objfile)
 
   /* Cause frv_current_sos() to be run if it hasn't been already.  */
   if (main_lm_addr == 0)
-    solib_add (0, 0, 0, 1);
+    solib_add (0, 0, 0, 1, /*lazy_read=*/0);
 
   /* frv_current_sos() will set main_lm_addr for the main executable.  */
   if (objfile == symfile_objfile)
diff --git a/gdb/solib-ia64-hpux.c b/gdb/solib-ia64-hpux.c
index f6dd2dd..8b16b58 100644
--- a/gdb/solib-ia64-hpux.c
+++ b/gdb/solib-ia64-hpux.c
@@ -257,7 +257,7 @@ ia64_hpux_handle_dld_breakpoint_1 (ptid_t ptid)
 	break;
       case BREAK_DE_LIB_LOADED:
 	ia64_hpux_handle_load_event (regcache);
-	solib_add (NULL, 0, &current_target, auto_solib_add);
+	solib_add (NULL, 0, &current_target, solib_add_opt, /*lazy_read=*/0);
 	break;
       case BREAK_DE_LIB_UNLOADED:
       case BREAK_DE_LOAD_COMPLETE:
@@ -567,7 +567,7 @@ chatr +dbg enable a.out"));
       break;  /* End of list.  */
 
   /* Resync the library list at the core level.  */
-  solib_add (NULL, 1, &current_target, auto_solib_add);
+  solib_add (NULL, 1, &current_target, solib_add_opt, /*lazy_read=*/0);
 }
 
 /* The "create_inferior_hook" target_so_ops routine for ia64-hpux.  */
diff --git a/gdb/solib-irix.c b/gdb/solib-irix.c
index 7bb528a..3d5657c 100644
--- a/gdb/solib-irix.c
+++ b/gdb/solib-irix.c
@@ -495,7 +495,8 @@ irix_solib_create_inferior_hook (int from_tty)
      and will put out an annoying warning.
      Delaying the resetting of stop_soon until after symbol loading
      suppresses the warning.  */
-  solib_add ((char *) 0, 0, (struct target_ops *) 0, auto_solib_add);
+  solib_add ((char *) 0, 0, (struct target_ops *) 0, solib_add_opt,
+	     /*lazy_read=*/0);
   inf->control.stop_soon = NO_STOP_QUIETLY;
 }
 
diff --git a/gdb/solib-osf.c b/gdb/solib-osf.c
index 0905eb6..94b8375 100644
--- a/gdb/solib-osf.c
+++ b/gdb/solib-osf.c
@@ -356,7 +356,8 @@ osf_solib_create_inferior_hook (int from_tty)
      and will put out an annoying warning.
      Delaying the resetting of stop_soon until after symbol loading
      suppresses the warning.  */
-  solib_add ((char *) 0, 0, (struct target_ops *) 0, auto_solib_add);
+  solib_add ((char *) 0, 0, (struct target_ops *) 0, solib_add_opt,
+	     /*lazy_read=*/0);
   inf->control.stop_soon = NO_STOP_QUIETLY;
 }
 
diff --git a/gdb/solib-sunos.c b/gdb/solib-sunos.c
index 9936038..463f3eb 100644
--- a/gdb/solib-sunos.c
+++ b/gdb/solib-sunos.c
@@ -805,7 +805,8 @@ sunos_solib_create_inferior_hook (int from_tty)
       warning (_("shared library handler failed to disable breakpoint"));
     }
 
-  solib_add ((char *) 0, 0, (struct target_ops *) 0, auto_solib_add);
+  solib_add ((char *) 0, 0, (struct target_ops *) 0, solib_add_opt,
+	     /*lazy_read=*/0);
 }
 
 static void
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index d92a83c..14c8f07 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1252,7 +1252,7 @@ svr4_fetch_objfile_link_map (struct objfile *objfile)
 
   /* Cause svr4_current_sos() to be run if it hasn't been already.  */
   if (info->main_lm_addr == 0)
-    solib_add (NULL, 0, &current_target, auto_solib_add);
+    solib_add (NULL, 0, &current_target, solib_add_opt, /*lazy_read=*/0);
 
   /* svr4_current_sos() will set main_lm_addr for the main executable.  */
   if (objfile == symfile_objfile)
@@ -1381,7 +1381,8 @@ enable_break (struct svr4_info *info, int from_tty)
      mean r_brk has already been relocated.  Assume the dynamic linker
      is the object containing r_brk.  */
 
-  solib_add (NULL, from_tty, &current_target, auto_solib_add);
+  solib_add (NULL, from_tty, &current_target, solib_add_opt,
+	     /*lazy_read=*/0);
   sym_addr = 0;
   if (info->debug_base && solib_svr4_r_map (info) != 0)
     sym_addr = solib_svr4_r_brk (info);
@@ -1553,7 +1554,8 @@ enable_break (struct svr4_info *info, int from_tty)
 	  info->debug_loader_name = xstrdup (interp_name);
 	  info->debug_loader_offset_p = 1;
 	  info->debug_loader_offset = load_addr;
-	  solib_add (NULL, from_tty, &current_target, auto_solib_add);
+	  solib_add (NULL, from_tty, &current_target, solib_add_opt,
+		     /*lazy_read=*/0);
 	}
 
       /* Record the relocated start and end address of the dynamic linker
@@ -2449,6 +2451,29 @@ elf_lookup_lib_symbol (const struct objfile *objfile,
   return lookup_global_symbol_from_objfile (objfile, name, domain);
 }
 
+/* Given PC, try to match a so_list which contains it.
+   See match_pc_solist in solist.h.  */
+
+static struct so_list *
+svr4_match_pc_solist (CORE_ADDR pc, struct so_list *so)
+{
+  struct so_list *iter, *res = NULL;
+  CORE_ADDR cur = CORE_ADDR_MAX;
+
+  for (iter = so; iter; iter = iter->next)
+    {
+      CORE_ADDR addr = lm_dynamic_from_link_map (iter);
+
+      if (addr >= pc && addr < cur)
+	{
+	  cur = addr;
+	  res = iter;
+	}
+    }
+
+  return res;
+}
+
 extern initialize_file_ftype _initialize_svr4_solib; /* -Wmissing-prototypes */
 
 void
@@ -2470,4 +2495,5 @@ _initialize_svr4_solib (void)
   svr4_so_ops.lookup_lib_global_symbol = elf_lookup_lib_symbol;
   svr4_so_ops.same = svr4_same;
   svr4_so_ops.keep_data_in_core = svr4_keep_data_in_core;
+  svr4_so_ops.match_pc_solist = svr4_match_pc_solist;
 }
diff --git a/gdb/solib.c b/gdb/solib.c
index 94f8f13..149026b 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -54,6 +54,18 @@
 /* Per-architecture data key.  */
 static struct gdbarch_data *solib_data;
 
+/* Used to store the user-defined setting for `auto-solib-add'.  */
+
+static const char *solib_add_choice = "on";
+
+static const char *solib_add_options[] =
+  {
+    "on",
+    "off",
+    "lazy",
+    NULL
+  };
+
 static void *
 solib_init (struct obstack *obstack)
 {
@@ -888,6 +900,19 @@ libpthread_solib_p (struct so_list *so)
   return libpthread_name_p (so->so_name);
 }
 
+void
+solib_on_demand_load (CORE_ADDR pc)
+{
+  struct so_list *solib;
+
+  /* On-demand loading of shared libraries' debuginfo.  */
+  solib = solib_match_pc_solist (pc);
+
+  if (solib && !solib->symbols_loaded)
+    solib_add (solib->so_name, 0, &current_target, 1,
+	       /*lazy_read=*/1);
+}
+
 /* GLOBAL FUNCTION
 
    solib_add -- read in symbol info for newly added shared libraries
@@ -895,7 +920,7 @@ libpthread_solib_p (struct so_list *so)
    SYNOPSIS
 
    void solib_add (char *pattern, int from_tty, struct target_ops
-   *TARGET, int readsyms)
+   *TARGET, int readsyms, int lazy_read)
 
    DESCRIPTION
 
@@ -906,11 +931,25 @@ libpthread_solib_p (struct so_list *so)
    If READSYMS is 0, defer reading symbolic information until later
    but still do any needed low level processing.
 
+   If LAZY_READ is 1, it means we are lazily reading debuginfo files
+   and should not reinit the frame cache.  reinit_frame_cache invalidates
+   all the frames and reinitializes the cache (obviously), so if the user
+   is doing normal shared library loading (without LAZY_READ set), we
+   can safely call reinit_frame_cache because GDB will not be performing
+   any action other than load the shared library.  However, if the user
+   has set LAZY_READ, it means we will be loading debuginfo from shared
+   libraries on-demand, i.e., the user will ask for something (like a
+   backtrace), and GDB will load the debuginfo *while* executing the
+   backtrace command.  In this scenario, we cannot reinitialize the frame
+   cache otherwise it will invalidate all the frames and GDB will lose
+   necessary information to reconstruct the backtrace.
+
    FROM_TTY and TARGET are as described for update_solib_list, above.  */
 
 void
 solib_add (char *pattern, int from_tty,
-	   struct target_ops *target, int readsyms)
+	   struct target_ops *target, enum solib_add_opt readsyms,
+	   int lazy_read)
 {
   struct so_list *gdb;
 
@@ -942,7 +981,7 @@ solib_add (char *pattern, int from_tty,
              need the library symbols to be loaded in order to provide
              thread support (x86-linux for instance).  */
           const int add_this_solib =
-            (readsyms || libpthread_solib_p (gdb));
+            (readsyms == SOLIB_ADD_ON || libpthread_solib_p (gdb));
 
 	  any_matches = 1;
 	  if (add_this_solib)
@@ -967,7 +1006,7 @@ solib_add (char *pattern, int from_tty,
       printf_unfiltered
 	("No loaded shared libraries match the pattern `%s'.\n", pattern);
 
-    if (loaded_any_symbols)
+    if (loaded_any_symbols && !lazy_read)
       {
 	struct target_so_ops *ops = solib_ops (target_gdbarch);
 
@@ -1265,6 +1304,28 @@ in_solib_dynsym_resolve_code (CORE_ADDR pc)
   return ops->in_dynsym_resolve_code (pc);
 }
 
+/* GLOBAL FUNCTION
+
+   solib_match_pc_solist -- check to see to which so_list PC belongs.
+
+   SYNOPSIS
+
+   struct so_list *solib_match_pc_solist (CORE_ADDR pc)
+
+   DESCRIPTION
+
+   Determine to which so_list the given PC belongs.  Returns the
+   so_list if found, NULL otherwise.
+*/
+
+struct so_list *
+solib_match_pc_solist (CORE_ADDR pc)
+{
+  struct target_so_ops *ops = solib_ops (target_gdbarch);
+
+  return ops->match_pc_solist (pc, so_list_head);
+}
+
 /*
 
    LOCAL FUNCTION
@@ -1283,7 +1344,7 @@ static void
 sharedlibrary_command (char *args, int from_tty)
 {
   dont_repeat ();
-  solib_add (args, from_tty, (struct target_ops *) 0, 1);
+  solib_add (args, from_tty, (struct target_ops *) 0, 1, /*lazy_read=*/0);
 }
 
 /* LOCAL FUNCTION
@@ -1364,7 +1425,8 @@ reload_shared_libraries_1 (int from_tty)
 	    exception_fprintf (gdb_stderr, e,
 			       _("Error while mapping "
 				 "shared library sections:\n"));
-	  else if (auto_solib_add || was_loaded || libpthread_solib_p (so))
+	  else if (solib_add_opt == SOLIB_ADD_ON
+		   || was_loaded || libpthread_solib_p (so))
 	    solib_read_symbols (so, flags);
 	}
     }
@@ -1416,7 +1478,7 @@ reload_shared_libraries (char *ignored, int from_tty,
      removed.  Call it only after the solib target has been initialized by
      solib_create_inferior_hook.  */
 
-  solib_add (NULL, 0, NULL, auto_solib_add);
+  solib_add (NULL, 0, NULL, solib_add_opt, /*lazy_read=*/0);
 
   breakpoint_re_set ();
 
@@ -1431,11 +1493,24 @@ reload_shared_libraries (char *ignored, int from_tty,
 }
 
 static void
-show_auto_solib_add (struct ui_file *file, int from_tty,
+set_solib_add (char *ignore_args, int from_tty,
+		    struct cmd_list_element *c)
+{
+  if (strcmp (solib_add_choice, "on") == 0)
+    solib_add_opt = SOLIB_ADD_ON;
+  else if (strcmp (solib_add_choice, "off") == 0)
+    solib_add_opt = SOLIB_ADD_OFF;
+  else if (strcmp (solib_add_choice, "lazy") == 0)
+    solib_add_opt = SOLIB_ADD_LAZY;
+}
+
+static void
+show_solib_add (struct ui_file *file, int from_tty,
 		     struct cmd_list_element *c, const char *value)
 {
-  fprintf_filtered (file, _("Autoloading of shared library symbols is %s.\n"),
-		    value);
+  fprintf_filtered (file,
+		    _("Autoloading of shared library symbols is `%s'.\n"),
+		    solib_add_choice);
 }
 
 
@@ -1460,6 +1535,10 @@ extern initialize_file_ftype _initialize_solib; /* -Wmissing-prototypes */
 void
 _initialize_solib (void)
 {
+  /* Used to deprecate `auto-solib-add' command.  */
+  struct cmd_list_element *c;
+  char *cmd_name, *old_cmdname;
+
   solib_data = gdbarch_data_register_pre_init (solib_init);
 
   add_com ("sharedlibrary", class_files, sharedlibrary_command,
@@ -1469,18 +1548,49 @@ _initialize_solib (void)
   add_com ("nosharedlibrary", class_files, no_shared_libraries,
 	   _("Unload all shared object library symbols."));
 
-  add_setshow_boolean_cmd ("auto-solib-add", class_support,
-			   &auto_solib_add, _("\
+  add_setshow_enum_cmd ("auto-solib-add", class_support,
+			solib_add_options,
+			&solib_add_choice, _("\
+Set autoloading of shared library symbols."), _("\
+Show autoloading of shared library symbols."), _("\
+If \"on\", symbols from all shared object libraries will be loaded\n\
+automatically when the inferior begins execution, when the dynamic linker\n\
+informs gdb that a new library has been loaded, or when attaching to the\n\
+inferior.  If \"lazy\", symbols from shared object libraries will be loaded\n\
+only when necessary.  If \"off\", symbols must be loaded manually, using\n\
+`sharedlibrary'."),
+			set_solib_add,
+			show_solib_add,
+			&setlist, &showlist);
+
+  /* Deprecate the command `auto-solib-add'.  We use `solib-add' now.  */
+  cmd_name = xstrdup ("auto-solib-add");
+  old_cmdname = cmd_name;
+
+  c = lookup_cmd (&cmd_name, setlist, "", -1, 0);
+  deprecate_cmd (c, "set solib-add");
+
+  /* Reset the command name.  */
+  cmd_name = old_cmdname;
+
+  c = lookup_cmd (&cmd_name, showlist, "", -1, 0);
+  deprecate_cmd (c, "show solib-add");
+  xfree (old_cmdname);
+
+  add_setshow_enum_cmd ("solib-add", class_support,
+			solib_add_options,
+			&solib_add_choice, _("\
 Set autoloading of shared library symbols."), _("\
 Show autoloading of shared library symbols."), _("\
 If \"on\", symbols from all shared object libraries will be loaded\n\
 automatically when the inferior begins execution, when the dynamic linker\n\
 informs gdb that a new library has been loaded, or when attaching to the\n\
-inferior.  Otherwise, symbols must be loaded manually, using \
+inferior.  If \"lazy\", symbols from shared object libraries will be loaded\n\
+only when necessary.  If \"off\", symbols must be loaded manually, using\n\
 `sharedlibrary'."),
-			   NULL,
-			   show_auto_solib_add,
-			   &setlist, &showlist);
+			set_solib_add,
+			show_solib_add,
+			&setlist, &showlist);
 
   add_setshow_filename_cmd ("sysroot", class_support,
 			    &gdb_sysroot, _("\
diff --git a/gdb/solib.h b/gdb/solib.h
index c473d85..ed2be66 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -26,6 +26,7 @@ struct so_list;
 struct target_ops;
 struct target_so_ops;
 struct program_space;
+enum solib_add_opt;
 
 /* Called when we free all symtabs, to free the shared library information
    as well.  */
@@ -34,7 +35,8 @@ extern void clear_solib (void);
 
 /* Called to add symbols from a shared library to gdb's symbol table.  */
 
-extern void solib_add (char *, int, struct target_ops *, int);
+extern void solib_add (char *, int, struct target_ops *,
+		       enum solib_add_opt, int);
 extern int solib_read_symbols (struct so_list *, int);
 
 /* Function to be called when the inferior starts up, to discover the
@@ -78,4 +80,8 @@ extern void set_solib_ops (struct gdbarch *gdbarch,
 
 extern int libpthread_name_p (const char *name);
 
+/* On-demand loading of shared libraries.  */
+
+extern void solib_on_demand_load (CORE_ADDR pc);
+
 #endif /* SOLIB_H */
diff --git a/gdb/solist.h b/gdb/solist.h
index dad11be..83bdcac 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -110,6 +110,10 @@ struct target_so_ops
        the run time loader.  */
     int (*in_dynsym_resolve_code) (CORE_ADDR pc);
 
+    /* Given PC and the current shared objects' list SO, iterate over SO
+       and return the so_list which contains PC, or NULL otherwise.  */
+    struct so_list *(*match_pc_solist) (CORE_ADDR pc, struct so_list *so);
+
     /* Find and open shared library binary file.  */
     bfd *(*bfd_open) (char *pathname);
 
@@ -154,6 +158,10 @@ extern bfd *solib_bfd_fopen (char *pathname, int fd);
 /* Find solib binary file and open it.  */
 extern bfd *solib_bfd_open (char *in_pathname);
 
+/* Iterate over current shared objects' list, and return the so_list
+   which contains PC, or NULL otherwise.  */
+extern struct so_list *solib_match_pc_solist (CORE_ADDR pc);
+
 /* FIXME: gdbarch needs to control this variable.  */
 extern struct target_so_ops *current_target_so_ops;
 
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 4689e0e..9e8da46 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -165,17 +165,19 @@ show_symbol_reloading (struct ui_file *file, int from_tty,
 		    value);
 }
 
-/* If non-zero, shared library symbols will be added automatically
+/* If `ON', shared library symbols will be added automatically
    when the inferior is created, new libraries are loaded, or when
    attaching to the inferior.  This is almost always what users will
    want to have happen; but for very large programs, the startup time
-   will be excessive, and so if this is a problem, the user can clear
-   this flag and then add the shared library symbols as needed.  Note
+   will be excessive, and so if this is a problem, the user can set this
+   flag to `OFF' and then add the shared library symbols as needed.  Note
    that there is a potential for confusion, since if the shared
    library symbols are not loaded, commands like "info fun" will *not*
-   report all the functions that are actually present.  */
+   report all the functions that are actually present.  The user also has
+   the option to set this flag to `AUTO'; this will make GDB load shared
+   library symbols on-demand.  */
 
-int auto_solib_add = 1;
+enum solib_add_opt solib_add_opt = SOLIB_ADD_ON;
 \f
 
 /* Make a null terminated copy of the string at PTR with SIZE characters in
diff --git a/gdb/symfile.h b/gdb/symfile.h
index cc65a14..9003ac0 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -472,17 +472,31 @@ extern char *obconcat (struct obstack *obstackp, ...) ATTRIBUTE_SENTINEL;
 
 			/*   Variables   */
 
-/* If non-zero, shared library symbols will be added automatically
+enum solib_add_opt
+  {
+    /* Disable auto-loading of debuginfo from shared libraries.  */
+    SOLIB_ADD_OFF = 0,
+
+    /* Enable auto-loading of debuginfo from shared libraries.  */
+    SOLIB_ADD_ON,
+
+    /* Lazily (on-demand) read debuginfo from shared libraries.  */
+    SOLIB_ADD_LAZY,
+  };
+
+/* If `ON', shared library symbols will be added automatically
    when the inferior is created, new libraries are loaded, or when
    attaching to the inferior.  This is almost always what users will
    want to have happen; but for very large programs, the startup time
-   will be excessive, and so if this is a problem, the user can clear
-   this flag and then add the shared library symbols as needed.  Note
+   will be excessive, and so if this is a problem, the user can set this
+   flag to `OFF' and then add the shared library symbols as needed.  Note
    that there is a potential for confusion, since if the shared
    library symbols are not loaded, commands like "info fun" will *not*
-   report all the functions that are actually present.  */
+   report all the functions that are actually present.  The user also has
+   the option to set this flag to `AUTO'; this will make GDB load shared
+   library symbols on-demand.  */
 
-extern int auto_solib_add;
+extern enum solib_add_opt solib_add_opt;
 
 /* From symfile.c */
 
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 8096f95..a613f8f 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -862,7 +862,7 @@ handle_unload_dll (void *dummy)
 	DEBUG_EVENTS (("gdb: Unloading dll \"%s\".\n", sodel->so_name));
 
 	windows_free_so (sodel);
-	solib_add (NULL, 0, NULL, auto_solib_add);
+	solib_add (NULL, 0, NULL, solib_add_opt, /*lazy_read=*/0);
 	return 1;
       }
 
diff --git a/gdb/xcoffsolib.c b/gdb/xcoffsolib.c
index 0915439..3f19002 100644
--- a/gdb/xcoffsolib.c
+++ b/gdb/xcoffsolib.c
@@ -166,16 +166,18 @@ _initialize_xcoffsolib (void)
   add_info ("sharedlibrary", solib_info,
 	    _("Status of loaded shared object libraries"));
 
-  add_setshow_boolean_cmd ("auto-solib-add", class_support,
-			   &auto_solib_add, _("\
+  add_setshow_enum_cmd ("solib-add", class_support,
+			solib_add_options,
+			&solib_add_choice, _("\
 Set autoloading of shared library symbols."), _("\
 Show autoloading of shared library symbols."), _("\
 If \"on\", symbols from all shared object libraries will be loaded\n\
 automatically when the inferior begins execution, when the dynamic linker\n\
 informs gdb that a new library has been loaded, or when attaching to the\n\
-inferior.  Otherwise, symbols must be loaded manually, using \
+inferior.  If \"lazy\", symbols from shared object libraries will be loaded\n\
+only when necessary.  If \"off\", symbols must be loaded manually, using\n\
 `sharedlibrary'."),
-			   NULL,
-			   NULL, /* FIXME: i18n: */
-			   &setlist, &showlist);
+			set_solib_add,
+			show_solib_add,
+			&setlist, &showlist);
 }

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

* Re: [PATCH] On-demand loading of shlib's debuginfo
  2011-07-19  5:34 [PATCH] On-demand loading of shlib's debuginfo Sergio Durigan Junior
@ 2011-07-20 22:01 ` Jan Kratochvil
  2011-07-21 11:13   ` Pedro Alves
  0 siblings, 1 reply; 9+ messages in thread
From: Jan Kratochvil @ 2011-07-20 22:01 UTC (permalink / raw)
  To: Sergio Durigan Junior; +Cc: gdb-patches

Hi Sergio,

it crashes for me on fedora-rawhide-x86_64:
$ echo 'main(){pause();}'|gcc -g -x c -;./gdb -nx ./a.out -ex 'set solib-add lazy' -ex start -ex 'list pause'
[...]
#3  in extract_typed_address (buf=0x10 <Address 0x10 out of bounds>, type=0x2000d60) at findvar.c:180
#4  in lm_dynamic_from_link_map (so=0x2009c00) at solib-svr4.c:169
[...]


On Tue, 19 Jul 2011 00:23:49 +0200, Sergio Durigan Junior wrote:
> With that in mind, we decided to tackle this problem progressively, and the
> first part of the solution is ready for submission.

That is currently it still touches the solib files on disk for the purpose of
solib_map_sections so that will be a different patch, looking forward.


> 1) Change the name of the `auto-solib-add' command to just `solib-add'
> (deprecatin `auto-solib-add').  Also, make the command be a tri-state,
> with `on', `off', and `lazy' options.  The default option is still `on'.

I do not understand why you haven't kept the original name, just extending it
to be tri-state.  Otherwise the auto_solib_add -> solib_add_opt rename could
be in a separate mechanical patch.


> 3) This patch only affects the `backtrace' command for now,

Do you have some benchmark of the benefits?  But without the
solib_map_sections part it may not yet been so significant I guess.


> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -4593,9 +4593,9 @@ bpstat_what (bpstat bs_head)
>        target_terminal_ours_for_output ();
>  
>  #ifdef SOLIB_ADD
> -      SOLIB_ADD (NULL, 0, &current_target, auto_solib_add);
> +      SOLIB_ADD (NULL, 0, &current_target, solib_add_opt, /*lazy_read=*/0);

nitpick: I find - except one such case - in GDB the style: , 0 /* lazy_read */);
+everywhere.


> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -14948,14 +14948,18 @@ will be loaded automatically when the inferior begins execution, you
>  attach to an independently started inferior, or when the dynamic linker
>  informs @value{GDBN} that a new library has been loaded.  If @var{mode}
>  is @code{off}, symbols must be loaded manually, using the
> -@code{sharedlibrary} command.  The default value is @code{on}.
> +@code{sharedlibrary} command.  If @var{mode} is @code{lazy}, then
> +@value{GDBN} will lazily load symbols from shared libraries, i.e., it
> +will load symbols on-demand.  The default value is @code{on}.

The crash reproducer at the top should have shown the "lazy" mode has limited
applicability now - which is also probably why it is not a default.

The lazy hook is now there only for backtrace so the documentation should
describe it works only for backtraces.  It could be applied to /usr/bin/gstack
but that is a Fedora only command/patch so far.


> +This command is now deprecated.  Use @code{set solib-add} instead.

This `auto-solib-add' gets deprecated but I do not see the new `solib-add'
setting described in the doc.


> --- a/gdb/solib-frv.c
> +++ b/gdb/solib-frv.c
> @@ -1302,7 +1302,7 @@ frv_fetch_objfile_link_map (struct objfile *objfile)
>  
>    /* Cause frv_current_sos() to be run if it hasn't been already.  */
>    if (main_lm_addr == 0)
> -    solib_add (0, 0, 0, 1);
> +    solib_add (0, 0, 0, 1, /*lazy_read=*/0);

1 should be SOLIB_ADD_ON instead.

 
> --- a/gdb/solib-svr4.c
> +++ b/gdb/solib-svr4.c
> @@ -2449,6 +2451,29 @@ elf_lookup_lib_symbol (const struct objfile *objfile,
>    return lookup_global_symbol_from_objfile (objfile, name, domain);
>  }
>  
> +/* Given PC, try to match a so_list which contains it.
> +   See match_pc_solist in solist.h.  */
> +
> +static struct so_list *
> +svr4_match_pc_solist (CORE_ADDR pc, struct so_list *so)
> +{
> +  struct so_list *iter, *res = NULL;
> +  CORE_ADDR cur = CORE_ADDR_MAX;
> +
> +  for (iter = so; iter; iter = iter->next)
> +    {
> +      CORE_ADDR addr = lm_dynamic_from_link_map (iter);
> +
> +      if (addr >= pc && addr < cur)

Maybe a note "cur" is in the DYNAMIC segment while PC is in the LOAD segment
and normally LOAD segment is under DYNAMIC segment; which is why the
conditional looks like reversed.


> +	{
> +	  cur = addr;
> +	  res = iter;
> +	}
> +    }
> +
> +  return res;
> +}
> +

> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -888,6 +900,19 @@ libpthread_solib_p (struct so_list *so)
>    return libpthread_name_p (so->so_name);
>  }
>  
> +void
> +solib_on_demand_load (CORE_ADDR pc)

Missing function comment.


> +{
> +  struct so_list *solib;
> +
> +  /* On-demand loading of shared libraries' debuginfo.  */
> +  solib = solib_match_pc_solist (pc);
> +
> +  if (solib && !solib->symbols_loaded)
> +    solib_add (solib->so_name, 0, &current_target, 1,
> +	       /*lazy_read=*/1);

1 should be SOLIB_ADD_ON instead.


> +}
> +
>  /* GLOBAL FUNCTION
>  
>     solib_add -- read in symbol info for newly added shared libraries
> @@ -967,7 +1006,7 @@ solib_add (char *pattern, int from_tty,
>        printf_unfiltered
>  	("No loaded shared libraries match the pattern `%s'.\n", pattern);
>  
> -    if (loaded_any_symbols)
> +    if (loaded_any_symbols && !lazy_read)

I think it can break sunos_special_symbol_handling, that should still be
called there or to disable lazy_read functionality on sunos or so.


>        {
>  	struct target_so_ops *ops = solib_ops (target_gdbarch);
>  
> @@ -1265,6 +1304,28 @@ in_solib_dynsym_resolve_code (CORE_ADDR pc)
>    return ops->in_dynsym_resolve_code (pc);
>  }
>  
> +/* GLOBAL FUNCTION
> +
> +   solib_match_pc_solist -- check to see to which so_list PC belongs.
> +
> +   SYNOPSIS
> +
> +   struct so_list *solib_match_pc_solist (CORE_ADDR pc)
> +
> +   DESCRIPTION
> +
> +   Determine to which so_list the given PC belongs.  Returns the
> +   so_list if found, NULL otherwise.
> +*/
> +
> +struct so_list *
> +solib_match_pc_solist (CORE_ADDR pc)
> +{
> +  struct target_so_ops *ops = solib_ops (target_gdbarch);
> +
> +  return ops->match_pc_solist (pc, so_list_head);

On solib platforms where match_pc_solist is not defined it will crash.


> +}
> +
>  /*
>  
>     LOCAL FUNCTION
> @@ -1283,7 +1344,7 @@ static void
>  sharedlibrary_command (char *args, int from_tty)
>  {
>    dont_repeat ();
> -  solib_add (args, from_tty, (struct target_ops *) 0, 1);
> +  solib_add (args, from_tty, (struct target_ops *) 0, 1, /*lazy_read=*/0);

1 should be SOLIB_ADD_ON instead.



Thanks,
Jan

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

* Re: [PATCH] On-demand loading of shlib's debuginfo
  2011-07-20 22:01 ` Jan Kratochvil
@ 2011-07-21 11:13   ` Pedro Alves
  2011-07-21 14:03     ` Jan Kratochvil
  2011-07-21 18:43     ` Sergio Durigan Junior
  0 siblings, 2 replies; 9+ messages in thread
From: Pedro Alves @ 2011-07-21 11:13 UTC (permalink / raw)
  To: gdb-patches; +Cc: Jan Kratochvil, Sergio Durigan Junior

On Wednesday 20 July 2011 21:53:14, Jan Kratochvil wrote:
> On Tue, 19 Jul 2011 00:23:49 +0200, Sergio Durigan Junior wrote:
> > With that in mind, we decided to tackle this problem progressively, and the
> > first part of the solution is ready for submission.
> 
> That is currently it still touches the solib files on disk for the purpose of
> solib_map_sections so that will be a different patch, looking forward.

I had skimmed the patch only, and reserved commenting until I had
a chance of reading the patch carefuly and figuring it out
myself, but since you raise this...

Is that the reason then that svr4_match_pc_solist goes through
the link map to map a PC to a so_list, instead of having a
generic implementation that matches the PC to the so_list's
loaded (bfd) sections (that is, just call solib_contains_address_p) ?
If so, it'd make more sense IMO to remove that from this patch,
and add it only along with a change that lazies mapping in the bfd
and its sections as well.  I'm not sure how safe that will be.

-- 
Pedro Alves

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

* Re: [PATCH] On-demand loading of shlib's debuginfo
  2011-07-21 11:13   ` Pedro Alves
@ 2011-07-21 14:03     ` Jan Kratochvil
  2011-07-21 15:19       ` Pedro Alves
  2011-07-21 18:43     ` Sergio Durigan Junior
  1 sibling, 1 reply; 9+ messages in thread
From: Jan Kratochvil @ 2011-07-21 14:03 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Sergio Durigan Junior

On Thu, 21 Jul 2011 12:58:44 +0200, Pedro Alves wrote:
> Is that the reason then that svr4_match_pc_solist goes through
> the link map to map a PC to a so_list, instead of having a
> generic implementation that matches the PC to the so_list's
> loaded (bfd) sections (that is, just call solib_contains_address_p) ?
> If so, it'd make more sense IMO to remove that from this patch,
> and add it only along with a change that lazies mapping in the bfd
> and its sections as well.  I'm not sure how safe that will be.

I agree the patch as is does not make much sense on its own IMHO, maybe it
could be pre-approved but checked-in only later with the bfd part of it.


Thanks,
Jan

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

* Re: [PATCH] On-demand loading of shlib's debuginfo
  2011-07-21 14:03     ` Jan Kratochvil
@ 2011-07-21 15:19       ` Pedro Alves
  2011-07-21 20:38         ` Tom Tromey
  0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2011-07-21 15:19 UTC (permalink / raw)
  To: gdb-patches; +Cc: Jan Kratochvil, Sergio Durigan Junior

On Thursday 21 July 2011 13:21:04, Jan Kratochvil wrote:
> On Thu, 21 Jul 2011 12:58:44 +0200, Pedro Alves wrote:
> > Is that the reason then that svr4_match_pc_solist goes through
> > the link map to map a PC to a so_list, instead of having a
> > generic implementation that matches the PC to the so_list's
> > loaded (bfd) sections (that is, just call solib_contains_address_p) ?
> > If so, it'd make more sense IMO to remove that from this patch,
> > and add it only along with a change that lazies mapping in the bfd
> > and its sections as well.  I'm not sure how safe that will be.
> 
> I agree the patch as is does not make much sense on its own IMHO, maybe it
> could be pre-approved but checked-in only later with the bfd part of it.

I was under the impression that the debug info loading would be
the major hog, and lazying that alone would stand on its own (as this
patch, but with a solib_contains_address_pc_p call in solib_on_demand_load
instead of the new match_pc_solist stuff).  Do you expect that lazying
opening the bfd and mapping its target sections to save a fair amount of
additional time?

Some benchmarks would indeed by nice...

-- 
Pedro Alves

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

* Re: [PATCH] On-demand loading of shlib's debuginfo
  2011-07-21 11:13   ` Pedro Alves
  2011-07-21 14:03     ` Jan Kratochvil
@ 2011-07-21 18:43     ` Sergio Durigan Junior
  2011-07-21 19:21       ` Pedro Alves
  1 sibling, 1 reply; 9+ messages in thread
From: Sergio Durigan Junior @ 2011-07-21 18:43 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Jan Kratochvil

Pedro Alves <pedro@codesourcery.com> writes:

> On Wednesday 20 July 2011 21:53:14, Jan Kratochvil wrote:
>> On Tue, 19 Jul 2011 00:23:49 +0200, Sergio Durigan Junior wrote:
>> > With that in mind, we decided to tackle this problem progressively, and the
>> > first part of the solution is ready for submission.
>> 
>> That is currently it still touches the solib files on disk for the purpose of
>> solib_map_sections so that will be a different patch, looking forward.
>
> I had skimmed the patch only, and reserved commenting until I had
> a chance of reading the patch carefuly and figuring it out
> myself, but since you raise this...
>
> Is that the reason then that svr4_match_pc_solist goes through
> the link map to map a PC to a so_list, instead of having a
> generic implementation that matches the PC to the so_list's
> loaded (bfd) sections (that is, just call solib_contains_address_p) ?
> If so, it'd make more sense IMO to remove that from this patch,
> and add it only along with a change that lazies mapping in the bfd
> and its sections as well.  I'm not sure how safe that will be.

Thanks for the comments, both of you.  After some chat with Jan, I have
decided that I will work on the second part of this task, i.e., take
care of solib_map_sections inside update_solib_list, and will only
submit a new patch when it's done.

Thank you,

Sergio.

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

* Re: [PATCH] On-demand loading of shlib's debuginfo
  2011-07-21 18:43     ` Sergio Durigan Junior
@ 2011-07-21 19:21       ` Pedro Alves
  2011-07-21 20:10         ` Jan Kratochvil
  0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2011-07-21 19:21 UTC (permalink / raw)
  To: Sergio Durigan Junior; +Cc: gdb-patches, Jan Kratochvil

On Thursday 21 July 2011 19:14:50, Sergio Durigan Junior wrote:
> Pedro Alves <pedro@codesourcery.com> writes:
> 
> > On Wednesday 20 July 2011 21:53:14, Jan Kratochvil wrote:
> >> On Tue, 19 Jul 2011 00:23:49 +0200, Sergio Durigan Junior wrote:
> >> > With that in mind, we decided to tackle this problem progressively, and the
> >> > first part of the solution is ready for submission.
> >> 
> >> That is currently it still touches the solib files on disk for the purpose of
> >> solib_map_sections so that will be a different patch, looking forward.
> >
> > I had skimmed the patch only, and reserved commenting until I had
> > a chance of reading the patch carefuly and figuring it out
> > myself, but since you raise this...
> >
> > Is that the reason then that svr4_match_pc_solist goes through
> > the link map to map a PC to a so_list, instead of having a
> > generic implementation that matches the PC to the so_list's
> > loaded (bfd) sections (that is, just call solib_contains_address_p) ?
> > If so, it'd make more sense IMO to remove that from this patch,
> > and add it only along with a change that lazies mapping in the bfd
> > and its sections as well.  I'm not sure how safe that will be.
> 
> Thanks for the comments, both of you.  After some chat with Jan, I have
> decided that I will work on the second part of this task, i.e., take
> care of solib_map_sections inside update_solib_list, and will only
> submit a new patch when it's done.

Oh, well:-)  I thought the separation was good, as:

 - target sections are used for "set trust-readonly-sections on"
   and similar fallbacks to reading memory from the exec, which
   requires separate dso lazy load points (disassembly? printing
   some address?).

 - its not clear to me the pc -> dso mapping from the link map is
   faster than from the bfd in all scenarios.  E.g., on remote
   targets, it may be faster to get at the bfd info on the host, than
   to remote read memory from the target.

 - I was curious on how much lazing debug info alone was helping,
   vs lazing the bfd reads.

-- 
Pedro Alves

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

* Re: [PATCH] On-demand loading of shlib's debuginfo
  2011-07-21 19:21       ` Pedro Alves
@ 2011-07-21 20:10         ` Jan Kratochvil
  0 siblings, 0 replies; 9+ messages in thread
From: Jan Kratochvil @ 2011-07-21 20:10 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Sergio Durigan Junior, gdb-patches

On Thu, 21 Jul 2011 20:36:04 +0200, Pedro Alves wrote:
> Oh, well:-)  I thought the separation was good, as:
> 
>  - target sections are used for "set trust-readonly-sections on"
>    and similar fallbacks to reading memory from the exec, which
>    requires separate dso lazy load points (disassembly? printing
>    some address?).
> 
>  - its not clear to me the pc -> dso mapping from the link map is
>    faster than from the bfd in all scenarios.  E.g., on remote
>    targets, it may be faster to get at the bfd info on the host, than
>    to remote read memory from the target.

I agree the remote targets may give opposite performance benefits.

I think it would be good to have both parts at hand to play with pros and
cons.  So far this patch needs explicit enable anyway as it breaks most of the
non-backtrace commands.


>  - I was curious on how much lazing debug info alone was helping,
>    vs lazing the bfd reads.

I had some very brief benchmark:
	0m24.824s -> 0m12.885s (cold disk cache with warm cache for gdb)
	F15.x86_64, therefore with .gdb_index.

This is by a variant of this patch - that is still with mapping bfds for
target sections, on HDD (not SSD).  And sure everything with linux-nat.


Thanks,
Jan

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

* Re: [PATCH] On-demand loading of shlib's debuginfo
  2011-07-21 15:19       ` Pedro Alves
@ 2011-07-21 20:38         ` Tom Tromey
  0 siblings, 0 replies; 9+ messages in thread
From: Tom Tromey @ 2011-07-21 20:38 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Jan Kratochvil, Sergio Durigan Junior

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> I was under the impression that the debug info loading would be
Pedro> the major hog, and lazying that alone would stand on its own (as
Pedro> this patch, but with a solib_contains_address_pc_p call in
Pedro> solib_on_demand_load instead of the new match_pc_solist stuff).

There's also this, which I still haven't checked in:

    http://sourceware.org/ml/gdb-patches/2011-03/msg00606.html

This defers reading debuginfo for new inferiors until some event happens
that requires it.

It turns out that Gary's recent patches will (most likely -- I didn't
try it yet) fix the problem pointed out in that message.

Anyway, this approach does help quite a bit.

Tom

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

end of thread, other threads:[~2011-07-21 20:27 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-19  5:34 [PATCH] On-demand loading of shlib's debuginfo Sergio Durigan Junior
2011-07-20 22:01 ` Jan Kratochvil
2011-07-21 11:13   ` Pedro Alves
2011-07-21 14:03     ` Jan Kratochvil
2011-07-21 15:19       ` Pedro Alves
2011-07-21 20:38         ` Tom Tromey
2011-07-21 18:43     ` Sergio Durigan Junior
2011-07-21 19:21       ` Pedro Alves
2011-07-21 20:10         ` Jan Kratochvil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).