From: Andrew Burgess Date: Thu, 29 Aug 2024 11:34:15 +0000 (+0100) Subject: gdb: handle dprintf breakpoints when unloading a shared library X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=8c48ec7a6160aed0d1126c623443935e4435cd41;p=thirdparty%2Fbinutils-gdb.git 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 Approved-By: Tom Tromey --- diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index bb5985e3783..af0d1da441f 100644 --- 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 f6de2ed86ab..f3e8cce5aba 100644 --- 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+\\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+\\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