This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA]: Fix for pending breakpoints in manually loaded/unloadedshlibs


The updated patch is attached. The testcase had to be slightly modified to match the new warning message format.

2004-08-23 Jeff Johnston <jjohnstn@redhat.com>

        * observer.sh: Add struct so_list declaration.
        * Makefile.in: Add dependencies on observer.h for solib.c and
        breakpoint.c.
        * breakpoint.c (disable_breakpoints_in_unloaded_shlib): New
        function.
        (_initialize_breakpoint): Register
        disable_breakpoints_in_unloaded_shlib as an observer of the
        "solib unloaded" observation event.
        (re_enable_breakpoints_in_shlibs): For bp_shlib_disabled breakpoints,
        call decode_line_1 so unfound breakpoint errors are silent.
        * solib.c (update_solib_list): When a solib is discovered to have
        been unloaded by the program, notify all observers of the
        "solib unloaded" observation event.


2004-08-23 Jeff Johnston <jjohnstn@redhat.com>


        * gdb.base/unload.exp: Fix expected warning message to match
        latest format.


Ok to commit?


-- Jeff J.

Jeff Johnston wrote:
Daniel Jacobowitz wrote:

On Wed, Aug 18, 2004 at 03:22:22PM -0400, Jeff Johnston wrote:

Daniel Jacobowitz wrote:

On Wed, Aug 11, 2004 at 04:12:07PM -0400, Jeff Johnston wrote:

+ if (so_name + && !strcmp (so_name, solib->so_name))
+ {
+ b->enable_state = bp_shlib_disabled;
+ /* At this point, we cannot rely on remove_breakpoint
+ succeeding so we must mark the breakpoint as not inserted
+ to prevent future errors occurring in remove_breakpoints. */
+ b->loc->inserted = 0;
+ if (!disabled_shlib_breaks)
+ {
+ target_terminal_ours_for_output ();
+ warning ("Temporarily disabling unloaded shared library breakpoints:");
+ }
+ disabled_shlib_breaks = 1;
+ warning ("breakpoint #%d ", b->number);



I think you're missing a space after the colon, in the first warning. Also, this use of multiple warning() statements is neither i18n friendly nor MI/GUI friendly - you may get a separate dialog box for each. I believe other places do this with sprintf; still not 100% i18n friendly, but avoids the MI/GUI problems. I can't find an example offhand.


What you do want to see so I don't waste my time on this. As you already know, this routine was copied from the routine which disables shared library breakpoints in breakpoint.c. Is it sufficient to just issue the warning that I am temporarily disabling unloaded shared library breakpoints and not spell out each breakpoint in turn? I can see this as really annoying and pointless to an end-user if there are hundreds or thousands of breakpoints.



That's a good idea. How about this?


target_terminal_ours_for_output ();
warning ("Temporarily disabling breakpoints for unloaded shared library \"%s\",
so_name);



Yes, that's exactly what I had in mind. Consider it done plus your other comments. Eli, in light of what Daniel and Andrew have said regarding the value of having an observer, may I repost with changes and check the code in?


-- Jeff J.


Index: doc/observer.texi
===================================================================
RCS file: /cvs/src/src/gdb/doc/observer.texi,v
retrieving revision 1.7
diff -u -p -r1.7 observer.texi
--- doc/observer.texi	21 May 2004 16:04:03 -0000	1.7
+++ doc/observer.texi	23 Aug 2004 21:27:18 -0000
@@ -90,3 +90,8 @@ at the entry-point instruction.  For @sa
 @value{GDBN} calls this observer immediately after connecting to the
 inferior, and before any information on the inferior has been printed.
 @end deftypefun
+
+@deftypefun void solib_unloaded (struct so_list *@var{solib})
+The specified shared library has been discovered to be unloaded.
+@end deftypefun
+
Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.607
diff -u -p -r1.607 Makefile.in
--- Makefile.in	8 Aug 2004 19:27:09 -0000	1.607
+++ Makefile.in	23 Aug 2004 21:27:19 -0000
@@ -1718,7 +1718,7 @@ breakpoint.o: breakpoint.c $(defs_h) $(s
 	$(gdb_string_h) $(demangle_h) $(annotate_h) $(symfile_h) \
 	$(objfiles_h) $(source_h) $(linespec_h) $(completer_h) $(gdb_h) \
 	$(ui_out_h) $(cli_script_h) $(gdb_assert_h) $(block_h) \
-	$(gdb_events_h)
+	$(gdb_events_h) $(observer_h) $(solist_h)
 bsd-kvm.o: bsd-kvm.c $(defs_h) $(cli_cmds_h) $(command_h) $(frame_h) \
 	$(regcache_h) $(target_h) $(value_h) $(gdb_assert_h) $(readline_h) \
 	$(bsd_kvm_h)
@@ -2450,7 +2450,8 @@ solib-aix5.o: solib-aix5.c $(defs_h) $(g
 solib.o: solib.c $(defs_h) $(gdb_string_h) $(symtab_h) $(bfd_h) $(symfile_h) \
 	$(objfiles_h) $(gdbcore_h) $(command_h) $(target_h) $(frame_h) \
 	$(gdb_regex_h) $(inferior_h) $(environ_h) $(language_h) $(gdbcmd_h) \
-	$(completer_h) $(filenames_h) $(exec_h) $(solist_h) $(readline_h)
+	$(completer_h) $(filenames_h) $(exec_h) $(solist_h) $(readline_h) \
+	$(observer_h)
 solib-frv.o: solib-frv.c $(defs_h) $(gdb_string_h) $(inferior_h) \
 	$(gdbcore_h) $(solist_h) $(frv_tdep_h) $(objfiles_h) $(symtab_h) \
 	$(language_h) $(command_h) $(gdbcmd_h) $(elf_frv_h)
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.179
diff -u -p -r1.179 breakpoint.c
--- breakpoint.c	28 Jul 2004 17:26:26 -0000	1.179
+++ breakpoint.c	23 Aug 2004 21:27:19 -0000
@@ -49,6 +49,8 @@
 #include "cli/cli-script.h"
 #include "gdb_assert.h"
 #include "block.h"
+#include "solist.h"
+#include "observer.h"
 
 #include "gdb-events.h"
 
@@ -4388,6 +4390,46 @@ disable_breakpoints_in_shlibs (int silen
   }
 }
 
+/* Disable any breakpoints that are in in an unloaded shared library.  Only
+   apply to enabled breakpoints, disabled ones can just stay disabled.  */
+
+void
+disable_breakpoints_in_unloaded_shlib (struct so_list *solib)
+{
+  struct breakpoint *b;
+  int disabled_shlib_breaks = 0;
+
+#if defined (PC_SOLIB)
+  /* See also: insert_breakpoints, under DISABLE_UNSETTABLE_BREAK.  */
+  ALL_BREAKPOINTS (b)
+  {
+    if ((b->loc->loc_type == bp_loc_hardware_breakpoint
+	|| b->loc->loc_type == bp_loc_software_breakpoint)
+	&& breakpoint_enabled (b) 
+	&& !b->loc->duplicate)
+      {
+	char *so_name = PC_SOLIB (b->loc->address);
+	if (so_name 
+	    && !strcmp (so_name, solib->so_name))
+          {
+	    b->enable_state = bp_shlib_disabled;
+	    /* At this point, we cannot rely on remove_breakpoint
+	       succeeding so we must mark the breakpoint as not inserted
+	       to prevent future errors occurring in remove_breakpoints.  */
+	    b->loc->inserted = 0;
+	    if (!disabled_shlib_breaks)
+	      {
+		target_terminal_ours_for_output ();
+		warning ("Temporarily disabling breakpoints for unloaded shared library \"%s\"",
+			  so_name);
+	      }
+	    disabled_shlib_breaks = 1;
+	  }
+      }
+  }
+#endif
+}
+
 /* Try to reenable any breakpoints in shared libraries.  */
 void
 re_enable_breakpoints_in_shlibs (void)
@@ -7100,6 +7142,8 @@ breakpoint_re_set_one (void *bint)
   struct breakpoint *b = (struct breakpoint *) bint;
   struct value *mark;
   int i;
+  int not_found;
+  int *not_found_ptr = NULL;
   struct symtabs_and_lines sals;
   char *s;
   enum enable_state save_enable;
@@ -7150,11 +7194,19 @@ breakpoint_re_set_one (void *bint)
       save_enable = b->enable_state;
       if (b->enable_state != bp_shlib_disabled)
         b->enable_state = bp_disabled;
+      else
+	/* If resetting a shlib-disabled breakpoint, we don't want to
+	   see an error message if it is not found since we will expect
+	   this to occur until the shared library is finally reloaded.
+	   We accomplish this by giving decode_line_1 a pointer to use
+	   for silent notification that the symbol is not found.  */
+	not_found_ptr = &not_found;
 
       set_language (b->language);
       input_radix = b->input_radix;
       s = b->addr_string;
-      sals = decode_line_1 (&s, 1, (struct symtab *) NULL, 0, (char ***) NULL, NULL);
+      sals = decode_line_1 (&s, 1, (struct symtab *) NULL, 0, (char ***) NULL,
+		            not_found_ptr);
       for (i = 0; i < sals.nelts; i++)
 	{
 	  resolve_sal_pc (&sals.sals[i]);
@@ -7755,6 +7807,10 @@ _initialize_breakpoint (void)
   static struct cmd_list_element *breakpoint_show_cmdlist;
   struct cmd_list_element *c;
 
+#ifdef SOLIB_ADD
+  observer_attach_solib_unloaded (disable_breakpoints_in_unloaded_shlib);
+#endif
+
   breakpoint_chain = 0;
   /* Don't bother to call set_breakpoint_count.  $bpnum isn't useful
      before a breakpoint is set.  */
Index: observer.sh
===================================================================
RCS file: /cvs/src/src/gdb/observer.sh,v
retrieving revision 1.3
diff -u -p -r1.3 observer.sh
--- observer.sh	7 May 2004 22:51:52 -0000	1.3
+++ observer.sh	23 Aug 2004 21:27:19 -0000
@@ -52,6 +52,7 @@ case $lang in
 
 struct observer;
 struct bpstats;
+struct so_list;
 EOF
         ;;
 esac
Index: solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.66
diff -u -p -r1.66 solib.c
--- solib.c	30 Jul 2004 19:17:19 -0000	1.66
+++ solib.c	23 Aug 2004 21:27:19 -0000
@@ -42,6 +42,7 @@
 #include "filenames.h"		/* for DOSish file names */
 #include "exec.h"
 #include "solist.h"
+#include "observer.h"
 #include "readline/readline.h"
 
 /* external data declarations */
@@ -478,6 +479,10 @@ update_solib_list (int from_tty, struct 
       /* If it's not on the inferior's list, remove it from GDB's tables.  */
       else
 	{
+	  /* Notify any observer that the SO has been unloaded
+	     before we remove it from the gdb tables.  */
+	  observer_notify_solib_unloaded (gdb);
+
 	  *gdb_link = gdb->next;
 
 	  /* Unless the user loaded it explicitly, free SO's objfile.  */
Index: gdb.base/unload.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/unload.exp,v
retrieving revision 1.1
diff -u -p -r1.1 unload.exp
--- gdb.base/unload.exp	12 Aug 2004 20:22:59 -0000	1.1
+++ gdb.base/unload.exp	23 Aug 2004 21:30:34 -0000
@@ -126,7 +126,7 @@ Breakpoint.*, shrfunc1 \\\(x=3\\\).*unlo
 "running program"
 
 gdb_test "continue" \
-"Continuing.*y is 7.*warning: Temporarily disabling unloaded shared library breakpoints.*warning: breakpoint #.*Program exited normally." \
+"Continuing.*y is 7.*warning: Temporarily disabling breakpoints for.*unloadshr.sl.*Program exited normally." \
 "continuing to end of program"
 
 #
@@ -138,5 +138,6 @@ gdb_test "run" \
 "rerun to shared library breakpoint"
 
 gdb_test "continue" \
-"Continuing.*y is 7.*warning: Temporarily disabling unloaded shared library breakpoints.*warning: breakpoint #.*Program exited normally." \
-"continuing to end of program second time"
+"Continuing.*y is 7.*warning: Temporarily disabling breakpoints for.*unloadshr.sl.*Program exited normally." \
+"continuing to end of program"
+

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]