From 4f578099f946b3e9f34a4e2de3ef62012a437fd1 Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Wed, 28 Aug 2024 17:00:37 +0100 Subject: [PATCH] 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 Approved-By: Tom Tromey --- gdb/breakpoint.c | 7 +---- gdb/testsuite/gdb.base/dlmopen.exp | 48 ++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 6c422e97cda..b2017ef8b81 100644 --- 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 14f9084dd6f..a8e3b08c016 100644 --- 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 -- 2.39.5