Toolchain/binutils-gdb
1
0
Fork 0
forked from Imagelibrary/binutils-gdb
Code Pull Requests Activity

gdb: handle dprintf breakpoints when unloading a shared library

Browse Source 
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>
This commit is contained in:
Andrew Burgess
2024-08-29 12:34:15 +01:00
parent e9709998ff
commit 8c48ec7a61
2 changed files with 121 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
gdb/breakpoint.c
View File

@@ -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;

116
gdb/testsuite/gdb.base/shlib-unload.exp
View File

@@ -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