[patch] Fix breakpoints in unloaded shared libraries

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

breakpoints after:
	warning: Temporarily disabling breakpoints for ...

are currently not handled correctly and GDB does even crash (with -lmcheck).

-  else if (!b->loc)
+  else if (loc)
(effectively "(b->loc)" changed to "(loc)")
is only a cleanup of print_breakpoint_location() it IMO should deal only with
the one specified LOC and not to look at the list of B->LOC unrelated to its
core functionality.

gdb.base/unload.exp possibly would not have to load a different library and it
could just check the behavior of breakpoints after the first/single library
gets unloaded.  Still this way the second library gets loaded to the same
addresses making the possible GDB bugs more visible.

+    if (bl->shlib_disabled)
+      continue;
bpstat_stop_status should not print a breakpoint hit for breakpoint in an
unloaded library (Breakpoint 1) just because it has the same address as the
Breakpoint 2 which got hit for real.

+  if (loc != NULL && loc->shlib_disabled)
+    loc = NULL;
-  if (b->source_file)
+  if (b->source_file && loc)
The one who sets SHLIB_DISABLED could possibly automatically also clear
SOURCE_FILE but they are present in a different entity (BREAKPOINT vs.
BP_LOCATION) and it is a more general question if a single multi-location
breakpoint can have different source file names for different locations.
One could possibly move SHLIB_DISABLED into BREAKPOINT or SOURCE_FILE into
BP_LOCATION first; so just fixed it in this patch without changing anything.

No regressons on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.


Thanks,
Jan


2009-10-25  Jan Kratochvil  <jan.kratochvil@redhat.com>

gdb/
	* breakpoint.c (bpstat_stop_status): Check BL->SHLIB_DISABLED.
	(print_breakpoint_location): New comment.  Check LOC->SHLIB_DISABLED.
	Check LOC validity before printing it.  Use LOC instead of B->LOC.
	Swap the if conditionals order.

2009-10-25  Jan Kratochvil  <jan.kratochvil@redhat.com>

gdb/testsuite/
	* gdb.base/unload.c (main): Change the UNLOADSHR parameter to 1.
	Replace the printf call of Y by provided "y-set-1" label.  New block
	for the second shared library.
	* gdb.base/unload.exp: Compile also the second library, call
	gdb_load_shlibs also for it.  Use now gdb_breakpoint.
	(single pending breakpoint info): Rename to ...
	(pending breakpoint info before run): ... this extended test.
	(libfile2, libname2, libsrcfile2, libsrc2)
	(lib_sl2): New variables.
	(exec_opts): Set also SHLIB_NAME2.
	(pending breakpoint info on first run at shrfunc1)
	(pending breakpoint info on second run at shrfunc1)
	(pending breakpoint info on second run at shrfunc2)
	(print y from libfile, print y from libfile2): New tests.
	* gdb.base/unloadshr.c (shrfunc1): Change the returned value.
	* gdb.base/unloadshr2.c: New.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3373,6 +3373,8 @@ bpstat_stop_status (struct address_space *aspace,
     gdb_assert (b);
     if (!breakpoint_enabled (b) && b->enable_state != bp_permanent)
       continue;
+    if (bl->shlib_disabled)
+      continue;
 
     /* For hardware watchpoints, we look only at the first location.
        The watchpoint_check function will work on entire expression,
@@ -3737,6 +3739,8 @@ bpstat_should_step (void)
 
 
 
+/* Print the LOC location out of the list of B->LOC locations.  */
+
 static void print_breakpoint_location (struct breakpoint *b,
 				       struct bp_location *loc,
 				       char *wrap_indent,
@@ -3744,10 +3748,13 @@ static void print_breakpoint_location (struct breakpoint *b,
 {
   struct cleanup *old_chain = save_current_program_space ();
 
+  if (loc != NULL && loc->shlib_disabled)
+    loc = NULL;
+
   if (loc != NULL)
     set_current_program_space (loc->pspace);
 
-  if (b->source_file)
+  if (b->source_file && loc)
     {
       struct symbol *sym 
 	= find_pc_sect_function (loc->address, loc->section);
@@ -3773,15 +3780,13 @@ static void print_breakpoint_location (struct breakpoint *b,
       
       ui_out_field_int (uiout, "line", b->line_number);
     }
-  else if (!b->loc)
-    {
-      ui_out_field_string (uiout, "pending", b->addr_string);
-    }
-  else
+  else if (loc)
     {
       print_address_symbolic (loc->address, stb->stream, demangle, "");
       ui_out_field_stream (uiout, "at", stb);
     }
+  else
+    ui_out_field_string (uiout, "pending", b->addr_string);
 
   do_cleanups (old_chain);
 }
--- a/gdb/testsuite/gdb.base/unload.c
+++ b/gdb/testsuite/gdb.base/unload.c
@@ -48,7 +48,7 @@ int main()
       exit (1);
     }
 
-  unloadshr = (int (*)(int))dlsym (handle, "shrfunc1");
+  unloadshr = (int (*) (int)) dlsym (handle, "shrfunc1");
 
   if (!unloadshr)
     {
@@ -60,11 +60,37 @@ int main()
       exit (1);
     }
 
-  y = (*unloadshr)(3);
+  y = (*unloadshr) (1);
 
-  printf ("y is %d\n", y);
+  dlclose (handle);
+  handle = NULL;	/* y-set-1 */
+
+  /* The second library should share the same memory address.  */
+
+  handle = dlopen (SHLIB_NAME2, RTLD_LAZY);
+  
+  if (!handle)
+    {
+      fprintf (stderr, dlerror ());
+      exit (1);
+    }
+
+  unloadshr = (int (*)(int)) dlsym (handle, "shrfunc2");
+
+  if (!unloadshr)
+    {
+#ifdef __WIN32__
+      fprintf (stderr, "error %d occurred", GetLastError ());
+#else
+      fprintf (stderr, "%s", dlerror ());
+#endif
+      exit (1);
+    }
+
+  y = (*unloadshr) (2);
 
   dlclose (handle);
+  handle = NULL;	/* y-set-2 */
 
   return 0;
 }
--- a/gdb/testsuite/gdb.base/unload.exp
+++ b/gdb/testsuite/gdb.base/unload.exp
@@ -37,24 +37,30 @@ if {[istarget arm*-*-symbianelf*]} {
 
 set testfile "unload"
 set libfile "unloadshr"
+set libfile2 "unloadshr2"
 set libname "${libfile}.sl"
+set libname2 "${libfile2}.sl"
 set libsrcfile ${libfile}.c
+set libsrcfile2 ${libfile2}.c
 set srcfile $srcdir/$subdir/$testfile.c
 set binfile $objdir/$subdir/$testfile
 set shlibdir ${objdir}/${subdir}
 set libsrc  $srcdir/$subdir/$libfile.c
+set libsrc2  $srcdir/$subdir/$libfile2.c
 set lib_sl  $objdir/$subdir/$libname
+set lib_sl2  $objdir/$subdir/$libname2
 
 if [get_compiler_info ${binfile}] {
     return -1
 }
 
 set lib_opts debug
-set exec_opts [list debug shlib_load additional_flags=-DSHLIB_NAME\=\"${libname}\"]
+set exec_opts [list debug shlib_load additional_flags=-DSHLIB_NAME\=\"${libname}\" additional_flags=-DSHLIB_NAME2\=\"${libname2}\"]
 
 if { [gdb_compile_shlib $libsrc $lib_sl $lib_opts] != ""
+     || [gdb_compile_shlib $libsrc2 $lib_sl2 $lib_opts] != ""
      || [gdb_compile $srcfile $binfile executable $exec_opts] != ""} {
-    untested "Couldn't compile $libsrc or $srcfile."
+    untested "Couldn't compile $libsrc or $libsrc2 or $srcfile."
     return -1
 }
 
@@ -64,7 +70,7 @@ gdb_exit
 gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
-gdb_load_shlibs $lib_sl
+gdb_load_shlibs $lib_sl $lib_sl2
 
 if [target_info exists gdb_stub] {
     gdb_step_for_stub;
@@ -75,27 +81,35 @@ if [target_info exists gdb_stub] {
 # 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_breakpoint "shrfunc1" allow-pending
+gdb_breakpoint "shrfunc2" allow-pending
 
 gdb_test "info break" \
-    "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-\[0-9\]+\[\t \]+breakpoint     keep y.*PENDING.*shrfunc1.*" \
-"single pending breakpoint info"
+    "Num     Type\[ \]+Disp Enb Address\[ \]+What\r?
+1\[\t \]+breakpoint     keep y *<PENDING> *shrfunc1\r?
+2\[\t \]+breakpoint     keep y *<PENDING> *shrfunc2" \
+"pending breakpoint info before run"
 
 set unloadshr_line [gdb_get_line_number "unloadshr break" ${libsrcfile}]
 
 gdb_run_cmd
 gdb_test "" \
-".*Breakpoint.*, shrfunc1 \\\(x=3\\\).*unloadshr.c:$unloadshr_line.*" \
+"Breakpoint 1, shrfunc1 \\\(x=1\\\).*${libsrcfile}:$unloadshr_line.*" \
 "running program"
 
+gdb_test "info break" \
+    "Num     Type\[ \]+Disp Enb Address\[ \]+What\r?
+1\[\t \]+breakpoint     keep y *0x\[0-9a-f\]+ *in shrfunc1 at .*
+2\[\t \]+breakpoint     keep y *<PENDING> *shrfunc2.*" \
+"pending breakpoint info on first run at shrfunc1"
+
+gdb_breakpoint "$testfile.c:[gdb_get_line_number "y-set-1" $testfile.c]" temporary
+
 gdb_test "continue" \
-"Continuing.*warning: Temporarily disabling breakpoints for.*unloadshr.sl.*Program exited normally." \
-"continuing to end of program"
+"Continuing.*warning: Temporarily disabling breakpoints for.*${libname}.*y-set-1.*" \
+"continuing to unloaded libfile"
+
+gdb_test "print y" " = 10" "print y from libfile"
 
 #
 # Try to rerun program and verify that shared breakpoint is reset properly
@@ -103,10 +117,38 @@ gdb_test "continue" \
 
 gdb_run_cmd
 gdb_test "" \
-".*Breakpoint.*shrfunc1.*at.*unloadshr.c:$unloadshr_line.*" \
+"Breakpoint 1, shrfunc1 \\\(x=1\\\).*${libsrcfile}:$unloadshr_line.*" \
 "rerun to shared library breakpoint"
 
+gdb_test "info break" \
+    "Num     Type\[ \]+Disp Enb Address\[ \]+What\r?
+1\[\t \]+breakpoint     keep y *0x\[0-9a-f\]+ *in shrfunc1 at .*
+2\[\t \]+breakpoint     keep y *<PENDING> *shrfunc2.*" \
+"pending breakpoint info on second run at shrfunc1"
+
+gdb_breakpoint "$testfile.c:[gdb_get_line_number "y-set-1" $testfile.c]" temporary
+
+gdb_test "continue" \
+"Continuing.*warning: Temporarily disabling breakpoints for.*${libname}.*y-set-1.*" \
+"continuing to unloaded libfile"
+
+set unloadshr_line2 [gdb_get_line_number "unloadshr2 break" ${libsrcfile2}]
+
+gdb_test "continue" \
+"Breakpoint 2, shrfunc2 \\\(x=2\\\).*${libsrcfile2}:$unloadshr_line2\r\n.*" \
+"continue to shrfunc2"
+
+gdb_test "info break" \
+    "Num     Type\[ \]+Disp Enb Address\[ \]+What\r?
+1\[\t \]+breakpoint     keep y *<PENDING> *shrfunc1\r?
+\[\t \]+breakpoint already hit \[0-9\]+ (time|times)\r?
+2\[\t \]+breakpoint     keep y *0x\[0-9a-f\]+ *in shrfunc2 at .*" \
+"pending breakpoint info on second run at shrfunc2"
+
+gdb_breakpoint "$testfile.c:[gdb_get_line_number "y-set-2" $testfile.c]"
+
 gdb_test "continue" \
-"Continuing.*warning: Temporarily disabling breakpoints for.*unloadshr.sl.*Program exited normally." \
-"continuing to end of program second time"
+"Continuing.*warning: Temporarily disabling breakpoints for.*${libname2}.*y-set-2.*" \
+"continuing to unloaded libfile2"
 
+gdb_test "print y" " = 200" "print y from libfile2"
--- a/gdb/testsuite/gdb.base/unloadshr.c
+++ b/gdb/testsuite/gdb.base/unloadshr.c
@@ -17,7 +17,8 @@
 
 #include <stdio.h>
 
-int shrfunc1 (int x)
+int
+shrfunc1 (int x)
 {
-  return x + 4; /* unloadshr break */
+  return x * 10;	/* unloadshr break */
 }
--- /dev/null
+++ b/gdb/testsuite/gdb.base/unloadshr2.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2009 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 3 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, see <http://www.gnu.org/licenses/>. */
+
+#include <stdio.h>
+
+int
+shrfunc2 (int x)
+{
+  return x * 100;	/* unloadshr2 break */
+}