gdb: handle dprintf breakpoints when unloading a shared library

While working on the previous commit I realised that GDB would not
handle dprintf breakpoints correctly when a shared library was
unloaded.

Consider this example using the test binary from shlib-unload.exp.  In
the function 'foo' we create a dprintf is in a shared library:

  (gdb) b 59
  Breakpoint 1 at 0x401215: file /tmp/projects/binutils-gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/shlib-unload.c, line 59.
  (gdb) r
  Starting program: /tmp/projects/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/shlib-unload/shlib-unload

  Breakpoint 1, main () at /tmp/projects/binutils-gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/shlib-unload.c:59
  59	  res = dlclose (handle);	/* Break here.  */
  (gdb) dprintf foo,"In foo"
  Dprintf 2 at 0x7ffff7fc50fd: file /tmp/projects/binutils-gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/shlib-unload-lib.c, line 23.
  (gdb) n
  Error in re-setting breakpoint 2: Function "foo" not defined.
  warning: error removing breakpoint 2 at 0x7ffff7fc50fd
  warning: error removing breakpoint 2 at 0x7ffff7fc50fd
  warning: error removing breakpoint 2 at 0x7ffff7fc50fd
  warning: error removing breakpoint 2 at 0x7ffff7fc50fd
  warning: error removing breakpoint 2 at 0x7ffff7fc50fd
  warning: error removing breakpoint 2 at 0x7ffff7fc50fd
  warning: error removing breakpoint 2 at 0x7ffff7fc50fd
  warning: error removing breakpoint 2 at 0x7ffff7fc50fd
  Cannot remove breakpoints because program is no longer writable.
  Further execution is probably impossible.
  60	  assert (res == 0);
  (gdb)

What happens here is that as the inferior steps over the dlclose call
the shared library containing 'foo' is unloaded and
disable_breakpoints_in_unloaded_shlib is called.  However in
disable_breakpoints_in_unloaded_shlib we have this check:

  if (b.type != bp_breakpoint
     && b.type != bp_jit_event
     && b.type != bp_hardware_breakpoint
     && !is_tracepoint (&b))
   continue;

As the dprintf has type bp_dprintf then this check triggers and we
ignore the dprintf, meaning the dprintf is not disabled.  When the
inferior stops after the 'next' GDB tries to remove all breakpoints
but the dprintf can no longer be removed, the memory in which it was
placed has been unmapped from the inferior.

The fix is to start using is_breakpoint() in
disable_breakpoints_in_unloaded_shlib instead of the bp_breakpoint and
bp_hardware_breakpoint checks.  The is_breakpoint() function also
checks for bp_dprintf.

With this fix in place GDB now correctly disables the breakpoint and
we no longer see the warning about removing the breakpoint.

During review it was pointed out that PR gdb/23149 and PR gdb/20208
both describe something similar, though for these bugs, the inferior
is restarted (which unloads all currently loaded shlib) rather than
passing over the dlclose.  But the consequences are pretty similar.
I've included a test which covers this case.

One additional thing that these two bugs did show though is that
disable_breakpoints_in_shlibs also needs to start using is_breakpoint
for the same reason.  Without this change, when an inferior is
restarted we get a warning like this for dprintf breakpoints:

  warning: Temporarily disabling breakpoints for unloaded shared library "..."

but we don't get a similar warning for "normal" breakpoints.  This is
because disable_breakpoints_in_shlibs is called from clear_solib,
which is called when an inferior is restarted.

It is best not to think too hard about disable_breakpoints_in_shlibs,
as this function is pretty broken, e.g. it doesn't call
notify_breakpoint_modified, despite modifying the breakpoints.  But
for now I'm ignoring that, but fixing this is definitely on my list
for my next set of breakpoint related fixes, it's just that a lot of
these breakpoint fixes end up being depending on one another, but I
want to avoid making this series too long.  So for now, I'm ignoring
the existing bug (missing breakpoint modified events), and fixing
disable_breakpoints_in_shlibs to cover dprintf.

With these fixes in place, the two bugs mentioned above should be
fixed.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23149
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=20208

Tested-By: Hannes Domani <ssbssa@yahoo.de>
Approved-By: Tom Tromey <tom@tromey.com>

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index bb5985e37834a78b751bd53758acfbcfb0e02b86..af0d1da441fc6f9559e97d94eb6d0aaaff3ebc80 100644 (file)
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8074,10 +8074,9 @@ disable_breakpoints_in_shlibs (program_space *pspace)
         becomes enabled, or the duplicate is removed, gdb will try to
         insert all breakpoints.  If we don't set shlib_disabled here,
         we'll try to insert those breakpoints and fail.  */
-      if (((b->type == bp_breakpoint)
-          || (b->type == bp_jit_event)
-          || (b->type == bp_hardware_breakpoint)
-          || (is_tracepoint (b)))
+      if (((b->type == bp_jit_event)
+          || is_breakpoint (b)
+          || is_tracepoint (b))
          && loc->pspace == pspace
          && !loc->shlib_disabled
          && solib_name_from_address (loc->pspace, loc->address)
@@ -8108,9 +8107,8 @@ disable_breakpoints_in_unloaded_shlib (program_space *pspace, const solib &solib
     {
       bool bp_modified = false;
 
-      if (b.type != bp_breakpoint
-         && b.type != bp_jit_event
-         && b.type != bp_hardware_breakpoint
+      if (b.type != bp_jit_event
+         && !is_breakpoint (&b)
          && !is_tracepoint (&b))
        continue;
 

diff --git a/gdb/testsuite/gdb.base/shlib-unload.exp b/gdb/testsuite/gdb.base/shlib-unload.exp
index f6de2ed86abf90aecdda6b687a5c01163ce967fb..f3e8cce5aba768bb5bc6e13a3634fd31d222947d 100644 (file)
--- a/gdb/testsuite/gdb.base/shlib-unload.exp
+++ b/gdb/testsuite/gdb.base/shlib-unload.exp
@@ -111,4 +111,120 @@ proc_with_prefix test_bp_modified_events {} {
     }
 }
 
+# Check that GDB disables dprintf breakpoints within a shared library
+# when the shared library is unloaded.
+proc_with_prefix test_dprintf_after_unload {} {
+    clean_restart $::binfile
+
+    if {![runto_main]} {
+       return
+    }
+
+    gdb_breakpoint $::srcfile:$::bp_line
+    gdb_continue_to_breakpoint "stop before dlclose"
+
+    # Setup a dprintf within the shared library.
+    gdb_test "dprintf foo,\"In foo\""
+    set bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \
+                   "get b/p number"]
+
+    # Unload the shared library, GDB should disable our b/p.
+    gdb_test "next" $::stop_after_bp_re
+
+    # Check that our b/p is now showing as disabled.
+    gdb_test "info breakpoints $bp_num" \
+       [multi_line \
+            "^Num\\s+Type\\s+Disp\\s+Enb\\s+Address\\s+What" \
+            "$bp_num\\s+dprintf\\s+keep\\s+y\\s+<PENDING>\\s+foo" \
+            "\\s+printf \"In foo\""]
+}
+
+# Create a dprintf breakpoint in a shared library.  Restart the
+# inferior.  We should not get an error about re-setting the dprintf
+# breakpoint.
+proc_with_prefix test_dprintf_with_rerun {} {
+    clean_restart $::binfile
+
+    if {![runto_main]} {
+       return
+    }
+
+    gdb_breakpoint $::srcfile:$::bp_line
+    gdb_continue_to_breakpoint "stop before dlclose"
+
+    # Setup a dprintf within the shared library.
+    gdb_test "dprintf foo,\"In foo\""
+    set bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \
+                   "get b/p number"]
+
+    # Check that the dprintf b/p is initially not pending.
+    gdb_test "info breakpoints $bp_num" \
+       [multi_line \
+            "^Num\\s+Type\\s+Disp\\s+Enb\\s+Address\\s+What" \
+            "$bp_num\\s+dprintf\\s+keep\\s+y\\s+$::hex\\s+in foo at \[^\r\n\]+" \
+            "\\s+printf \"In foo\""] \
+       "dprintf is non-pending before restart"
+
+    # Restart the inferior.
+    gdb_run_cmd
+
+    # The inferior will stop at the initial 'main' breakpoint.  This
+    # is at a location before any of the shlibs have been loaded.
+    set saw_bp_reset_error false
+    set saw_bp_disable_warning false
+    gdb_test_multiple "" "stop before shlib are reloaded" {
+       -re "warning: Temporarily disabling breakpoints for unloaded shared library \"\[^\r\n\]+/$::libname\"\r\n" {
+           set saw_bp_disable_warning true
+           exp_continue
+       }
+
+       -re "Error in re-setting breakpoint $bp_num: \[^\r\n\]+" {
+           set saw_bp_reset_error true
+           exp_continue
+       }
+
+       -re "Breakpoint $::decimal, main \\(\\) at \[^\r\n\]+\r\n$::decimal\\s+\[^\r\n\]+\r\n$::gdb_prompt $" {
+           gdb_assert { !$saw_bp_reset_error && !$saw_bp_disable_warning } $gdb_test_name
+       }
+    }
+
+    # Check that the dprintf b/p is still enabled, but marked pending
+    # before the shlib are loaded.
+    gdb_test "info breakpoints $bp_num" \
+       [multi_line \
+            "^Num\\s+Type\\s+Disp\\s+Enb\\s+Address\\s+What" \
+            "$bp_num\\s+dprintf\\s+keep\\s+y\\s+<PENDING>\\s+foo" \
+            "\\s+printf \"In foo\""] \
+       "dprintf is pending before shlib reload"
+
+    set saw_in_foo_output false
+    gdb_test_multiple "continue" "stop after libraries are reloaded" {
+       -re "^continue\r\n" {
+           exp_continue
+       }
+       -re "^Continuing\\.\r\n" {
+           exp_continue
+       }
+       -re "^In foo\r\n" {
+           set saw_in_foo_output true
+           exp_continue
+       }
+       -re "^Breakpoint $::decimal, main \\(\\) at .* Break here\\.  \\*/\r\n$::gdb_prompt $" {
+           gdb_assert { $saw_in_foo_output } $gdb_test_name
+       }
+    }
+
+    # Check that the dprintf b/p is still enabled, but is now, no
+    # longer pending.
+    gdb_test "info breakpoints $bp_num" \
+       [multi_line \
+            "^Num\\s+Type\\s+Disp\\s+Enb\\s+Address\\s+What" \
+            "$bp_num\\s+dprintf\\s+keep\\s+y\\s+$::hex\\s+in foo at \[^\r\n\]+" \
+            "\\s+breakpoint already hit 1 time" \
+            "\\s+printf \"In foo\""] \
+       "dprintf is non-pending after restart"
+}
+
 test_bp_modified_events
+test_dprintf_after_unload
+test_dprintf_with_rerun