gdb: disable internal b/p when a solib is unloaded

Bug PR gdb/32079 highlights an issue where GDB will try to remove a
breakpoint for a shared library that has been unloaded.  This will
trigger an error from GDB like:

  (gdb) next
  61            dlclose (handle[dl]);
  (gdb) next
  warning: error removing breakpoint 0 at 0x7ffff78169b9
  warning: error removing breakpoint 0 at 0x7ffff7730b57
  warning: error removing breakpoint 0 at 0x7ffff7730ad3
  54        for (dl = 0; dl < 4; ++dl)
  (gdb)

What happens is that as the inferior steps over the dlclose() call,
GDB notices that the library has been unloaded and calls
disable_breakpoints_in_unloaded_shlib.  However, this function only
operates on user breakpoints and tracepoints.

In the example above what is happening is that the test loads multiple
copies of libc into different linker namespsaces.  When we 'next' over
the dlclose call one of the copies of libc is unloaded.  As GDB placed
longjmp master breakpoints within the copy of libc that was just
unloaded, the warnings we see are GDB trying (and failing) to remove
these breakpoints.

I think the solution is for disable_breakpoints_in_unloaded_shlib to
handle all breakpoints, even internal ones like the longjmp master
breakpoints.

If we do this then the breakpoint will be marked as shlib_disabled and
also will be marked as not inserted.  Later when we call
breakpoint_re_set() and the longjmp breakpoints are deleted we will no
longer try to remove them.

This solution is inspired by a patch suggested in the bug report:

  https://sourceware.org/bugzilla/show_bug.cgi?id=32079#c3

There are some differences with my approach compared to the patch
suggested in the bug.  First I have no need to delete the breakpoint
inside disable_breakpoints_in_unloaded_shlib as an earlier patch in
this series arranged for breakpoint_re_set to be called when shared
libraries are removed.  Calling breakpoint_re_set will take care of
deleting the breakpoint for us.  For details see the earlier commit
titled:

  gdb: fixes for code_breakpoint::disabled_by_cond logic

Next, rather than only handling bp_longjmp and bp_longjmp_master, I
allow all breakpoints to be handled.  I also only give the warning
about disabling breakpoints for user breakpoints, I don't see the
point of warning the user about internal b/p changes.

With this done the issues in PR gdb/32079 are resolved.

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

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 6c422e97cda738a32b027528b91ebf2f18f99b11..b2017ef8b81cff90c7e8727be39c0bf59e72ceca 100644 (file)
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8107,11 +8107,6 @@ disable_breakpoints_in_unloaded_shlib (program_space *pspace, const solib &solib
     {
       bool bp_modified = false;
 
-      if (b.type != bp_jit_event
-         && !is_breakpoint (&b)
-         && !is_tracepoint (&b))
-       continue;
-
       for (bp_location &loc : b.locations ())
        {
          if (pspace != loc.pspace || loc.shlib_disabled)
@@ -8145,7 +8140,7 @@ disable_breakpoints_in_unloaded_shlib (program_space *pspace, const solib &solib
 
          bp_modified = true;
 
-         if (!disabled_shlib_breaks)
+         if (!disabled_shlib_breaks && user_breakpoint_p (&b))
            {
              target_terminal::ours_for_output ();
              warning (_("Temporarily disabling breakpoints "

diff --git a/gdb/testsuite/gdb.base/dlmopen.exp b/gdb/testsuite/gdb.base/dlmopen.exp
index 14f9084dd6f289d9ce9c954a034354ee728af5e3..a8e3b08c0168c166aeb8c6849742849d86b6483a 100644 (file)
--- a/gdb/testsuite/gdb.base/dlmopen.exp
+++ b/gdb/testsuite/gdb.base/dlmopen.exp
@@ -376,7 +376,55 @@ proc_with_prefix test_solib_unmap_events { } {
        "one dynamic linker found after dlclose calls"
 }
 
+# Check that we can 'next' over the dlclose calls without GDB giving any
+# warnings or errors.
+proc_with_prefix test_next_over_dlclose {} {
+    clean_restart $::binfile
+
+    if { ![runto_main] } {
+       return
+    }
+
+    set dlclose_lineno [gdb_get_line_number "dlclose" $::srcfile]
+    gdb_breakpoint $::srcfile:$dlclose_lineno
+    gdb_breakpoint $::srcfile:$::bp_main
+
+    # Remove the pause.  We no longer need it.
+    gdb_test "print wait_for_gdb = 0" "\\\$1 = 0"
+
+    set loc_re [multi_line \
+                   "\[^\r\n\]+/[string_to_regexp $::srcfile]:$dlclose_lineno" \
+                   "$dlclose_lineno\\s+dlclose \[^\r\n\]+"]
+
+    # This loop mirrors the loop in dlmopen.c where the inferior performs
+    # four calls to dlclose.  Here we continue to the dlclose, and then use
+    # 'next' to step over the call.  As part of the 'next' GDB will insert
+    # breakpoints to catch longjmps into the multiple copies of libc which
+    # have been loaded.  Each dlclose call will cause a copy of libc to be
+    # unloaded, which should disable the longjmp breakpoint that GDB
+    # previously inserted.
+    #
+    # At one point a bug in GDB meant that we failed to correctly disable
+    # the longjmp breakpoints and GDB would try to remove the breakpoint
+    # from a library after it had been unloaded, which would trigger a
+    # warning.
+    for { set i 0 } { $i < 4 } { incr i } {
+       gdb_continue_to_breakpoint "continue to dlclose $i" $loc_re
+       gdb_test "next" "^$::decimal\\s+for \\(dl = 0;\[^\r\n\]+\\)" \
+           "next over dlclose $i"
+    }
+
+    # Just to confirm that we are where we think we are, continue to the
+    # final 'return' line in main.  If this isn't hit then we likely went
+    # wrong earlier.
+    gdb_continue_to_breakpoint "continue to final return" \
+       [multi_line \
+            "\[^\r\n\]+/[string_to_regexp $::srcfile]:$::bp_main" \
+            "$::bp_main\\s+return 0;\[^\r\n\]+"]
+}
+
 # Run the actual tests.
 test_dlmopen_no_attach
 test_dlmopen_with_attach
 test_solib_unmap_events
+test_next_over_dlclose