[RFA]: Fix for pending breakpoints in manually loaded/unloaded shlibs

[Jeff Johnston <jjohnstn@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2689 bytes --]

The following patch fixes a problem with breakpoints set in shlibs that are 
manually loaded/unloaded by the program.  What currently happens is that pending 
breakpoints work properly for the first run of the program.  On the 2nd run, the 
resolved breakpoint(s) can end up at the start of the breakpoint list and is 
marked bp_shlib_disabled.  This is fine for a bit and we reach the breakpoint 
again when the shared library is loaded.  However, when we unload the 2nd time, 
there is trouble.  We eventually get a shlib_event from the dlclose() and we 
attempt to remove the breakpoint to step over it.  Unfortunately, we try and 
remove all breakpoints and we end attempting to remove a breakpoint that no 
longer exists (remember the breakpoint for the shared library routine is now at 
the start of the breakpoint list).  We fail trying to remove the first 
breakpoint and end up failing remove_breakpoints.  We subsequently keep running 
into the shlib_event breakpoint over and over again ad-infinitum.

To fix this, I have added an observer for a new event: solib_unloaded.  When 
update_solib_list discovers a shared library has been unloaded, it notifies all 
observers (initially this is just breakpoint.c).  Breakpoint.c sets up an 
observer to find all breakpoints in the removed shlib and mark them as 
non-inserted and bp_shlib_disabled.  This solves the problem.

I also added code to re_enable_breakpoints_in_shlibs to remove the error 
messages we get when we go and rerun the program (it attempts to find shlib 
breakpoints in every shared library that gets loaded).

I have included a new test case which exercises the scenario.

Ok to commit?

-- Jeff J.

2004-08-10  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-10  Jeff Johnston  <jjohnstn@redhat.com>

         * gdb.base/unload.exp: New test for breakpoints in dynamically
         loaded libraries.
         * gdb.base/unload.c: Ditto.
         * gdb.base/unloadshr.c: Ditto.



[-- Attachment #2: unload.patch --]
[-- Type: text/plain, Size: 6500 bytes --]

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	10 Aug 2004 18:47:47 -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	10 Aug 2004 18:47:48 -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)
 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	10 Aug 2004 18:47:48 -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,42 @@ 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;
+
+  /* See also: insert_breakpoints, under DISABLE_UNSETTABLE_BREAK. */
+  ALL_BREAKPOINTS (b)
+  {
+#if defined (PC_SOLIB)
+    if (((b->type == bp_breakpoint) ||
+	 (b->type == bp_hardware_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;
+	    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);
+	  }
+      }
+#endif
+  }
+}
 /* Try to reenable any breakpoints in shared libraries.  */
 void
 re_enable_breakpoints_in_shlibs (void)
@@ -7100,6 +7138,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 +7190,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 +7803,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	10 Aug 2004 18:47:48 -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	10 Aug 2004 18:47:48 -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.  */

[-- Attachment #3: unloadtest.patch --]
[-- Type: text/plain, Size: 8288 bytes --]

Index: gdb.base/unload.c
===================================================================
RCS file: gdb.base/unload.c
diff -N gdb.base/unload.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb.base/unload.c	10 Aug 2004 18:40:26 -0000
@@ -0,0 +1,60 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+ 
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+
+   Please email any bugs, comments, and/or additions to this file to:
+   bug-gdb@prep.ai.mit.edu  */
+
+#include <stdio.h>
+#include <dlfcn.h>
+
+int k = 0;
+
+#define SHLIB_NAME SHLIB_DIR "/unloadshr.sl"
+
+int main()
+{
+  void *handle;
+  int (*unloadshr) (int);
+  int y;
+  char *msg;
+
+  handle = dlopen (SHLIB_NAME, RTLD_LAZY);
+  msg = dlerror ();
+  
+  if (!handle)
+    {
+      fprintf (stderr, msg);
+      exit (1);
+    }
+
+  unloadshr = (int (*)(int))dlsym (handle, "shrfunc1");
+
+  if (!unloadshr)
+    {
+      fprintf (stderr, dlerror ());
+      exit (1);
+    }
+
+  y = (*unloadshr)(3);
+
+  printf ("y is %d\n", y);
+
+  dlclose (handle);
+
+  return 0;
+}
Index: gdb.base/unload.exp
===================================================================
RCS file: gdb.base/unload.exp
diff -N gdb.base/unload.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb.base/unload.exp	10 Aug 2004 18:40:26 -0000
@@ -0,0 +1,148 @@
+#   Copyright 2003, 2004
+#   Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+# 
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@prep.ai.mit.edu
+
+# This file was created by Jeff Johnston. (jjohnstn@redhat.com)
+# The shared library compilation portion was copied from shlib-call.exp which was
+# written by Elena Zannoni (ezannoni@redhat.com).
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+#
+# test running programs
+#
+set prms_id 0
+set bug_id 0
+
+# are we on a target board?
+if ![isnative] then {
+    return 0
+}
+
+set testfile "unload"
+set libfile "unloadshr"
+set libsrcfile ${libfile}.c
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+set shlibdir ${objdir}/${subdir}
+
+if [get_compiler_info ${binfile}] {
+    return -1
+}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "additional_flags=-DSHLIB_DIR\=\"${shlibdir}\"" "libs=-ldl"]] != "" } {
+     gdb_suppress_entire_file "Testcase compile failed, so all tests in this file will automatically fail."
+}
+
+# Build the shared libraries this test case needs.
+#
+
+if {$gcc_compiled == 0} {
+    if [istarget "hppa*-hp-hpux*"] then {
+	set additional_flags "additional_flags=+z"
+    } elseif { [istarget "mips-sgi-irix*"] } {
+	# Disable SGI compiler's implicit -Dsgi
+	set additional_flags "additional_flags=-Usgi"
+    } else {
+	# don't know what the compiler is...
+	set additional_flags ""
+    }
+} else {
+    if { ([istarget "powerpc*-*-aix*"]
+       || [istarget "rs6000*-*-aix*"]) } {
+	set additional_flags ""
+    } else {
+	set additional_flags "additional_flags=-fpic"
+    }
+}
+
+if {[gdb_compile "${srcdir}/${subdir}/${libsrcfile}" "${objdir}/${subdir}/${libfile}.o" object [list debug $additional_flags]] != ""} {
+     gdb_suppress_entire_file "Testcase compile failed, so all tests in this file will automatically fail."
+}
+
+if [istarget "hppa*-*-hpux*"] {
+    remote_exec build "ld -b ${objdir}/${subdir}/${libfile}.o -o ${objdir}/${subdir}/${libfile}.sl"
+} else {
+    set additional_flags "additional_flags=-shared"
+    if {[gdb_compile "${objdir}/${subdir}/${libfile}.o" "${objdir}/${subdir}/${libfile}.sl" executable [list debug $additional_flags]] != ""} {
+	gdb_suppress_entire_file "Testcase compile failed, so all tests in this file will automatically fail."
+    }
+}
+
+if { ($gcc_compiled 
+      &&  ([istarget "powerpc*-*-aix*"]
+	|| [istarget "rs6000*-*-aix*"] )) } {
+    set additional_flags "additional_flags=-L${objdir}/${subdir}"
+} elseif { [istarget "mips-sgi-irix*"] } {
+    set additional_flags "additional_flags=-rpath ${objdir}/${subdir}"
+} else {
+    set additional_flags ""
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if [target_info exists gdb_stub] {
+    gdb_step_for_stub;
+}
+
+#
+# Test setting a breakpoint in a dynamically loaded library which is
+# manually loaded and unloaded
+#
+
+gdb_test_multiple "break shrfunc1" "set pending breakpoint" {
+     -re ".*Make breakpoint pending.*y or \\\[n\\\]. $" {
+	    gdb_test "y" "Breakpoint.*shrfunc1.*pending." "set pending breakpoint"
+     }
+}
+
+gdb_test "info break" \
+    "Num Type\[ \]+Disp Enb Address\[ \]+What.*
+\[0-9\]+\[\t \]+breakpoint     keep y.*PENDING.*shrfunc1.*" \
+"single pending breakpoint info"
+
+set unloadshr_line [gdb_get_line_number "unloadshr break" ${srcdir}/${subdir}/${libsrcfile}]
+
+gdb_test "run" \
+"Starting program.*unload.*
+Breakpoint.*at.*
+Pending breakpoint \"shrfunc1\" resolved.*
+Breakpoint.*, shrfunc1 \\\(x=3\\\).*unloadshr.c:$unloadshr_line.*" \
+"running program"
+
+gdb_test "continue" \
+"Continuing.*y is 7.*warning: Temporarily disabling unloaded shared library breakpoints.*warning: breakpoint #.*Program exited normally." \
+"continuing to end of program"
+
+#
+# Try to rerun program and verify that shared breakpoint is reset properly
+#
+
+gdb_test "run" \
+".*Breakpoint.*shrfunc1.*at.*unloadshr.c:$unloadshr_line.*" \
+"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"
Index: gdb.base/unloadshr.c
===================================================================
RCS file: gdb.base/unloadshr.c
diff -N gdb.base/unloadshr.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb.base/unloadshr.c	10 Aug 2004 18:40:26 -0000
@@ -0,0 +1,27 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+ 
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+
+   Please email any bugs, comments, and/or additions to this file to:
+   bug-gdb@prep.ai.mit.edu  */
+
+#include <stdio.h>
+
+int shrfunc1 (int x)
+{
+  return x + 4; /* unloadshr break */
+}