From 4bc72a457122c6ab16f11e37edf1bea1e1279a35 Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Fri, 6 Sep 2024 16:18:09 +0100 Subject: gdb: remove disable_breakpoints_in_shlibs function I think there is a problem with the disable_breakpoints_in_shlibs function: it can disable breakpoint locations without calling notify_breakpoint_modified. This means that the Python API's breakpoint_modified event will not trigger, nor will the MI send a breakpoint modified event. I started looking at disable_breakpoints_in_shlibs because of an earlier commit: commit 8c48ec7a6160aed0d1126c623443935e4435cd41 Date: Thu Aug 29 12:34:15 2024 +0100 gdb: handle dprintf breakpoints when unloading a shared library Currently disable_breakpoints_in_shlibs is only called from one location, clear_solib in solib.c. clear_solib also calls notify_solib_unloaded for every solib in the program_space of interest, and notify_solib_unloaded will call disable_breakpoints_in_unloaded_shlib via the solib_unloaded observer. These two function, disable_breakpoints_in_shlibs and disable_breakpoints_in_unloaded_shlib are very similar in what they do. I think that we can remove the disable_breakpoints_in_shlibs call, and instead, tweak how we call disable_breakpoints_in_unloaded_shlib in order to get the same end result, except that, after this change, we will call notify_breakpoint_modified, which means the Python API event will trigger, and the MI events will be emitted. All that disable_breakpoints_in_shlibs does is disable some breakpoints. Meanwhile, disable_breakpoints_in_unloaded_shlib, will disable the same set of breakpoints, call notify_breakpoint_modified, and then (for some breakpoint types) print a message telling the user that the breakpoint has been disabled. However, this function will ignore any breakpoints that are already disabled. As disable_breakpoints_in_shlibs disables the same set of breakpoints, the result of the current code is that disable_breakpoints_in_shlibs serves only to prevent the notify_breakpoint_modified call, which I think is wrong, and to prevent the user message being printed, which I think is reasonable. If we remove the disable_breakpoints_in_shlibs call without making any additional changes, then we start to see some message printed in cases like this: (gdb) start The program being debugged has been started already. Start it from the beginning? (y or n) y warning: Temporarily disabling breakpoints for unloaded shared library "/tmp/shared-lib-test/libfoo.so" Temporary breakpoint 3 at 0x40113e: file test.c, line 9. Starting program: /tmp/shared-lib-test/test.x Notice the 'warning:' line, which is new. I think this is confusing because, in most cases the breakpoint will be enabled again by the time the inferior reaches `main` and stops. In the future I'm interested in exploring if GDB could be smarter about when to print these 'Temporarily disabling breakpoints ...' messages so that if the 'start' command does mean a breakpoint is left disabled, then the user would be informed. However, I don't propose doing that work immediately, and certainly not in this commit. For now, my intention is to leave things as they are right now, GDB doesn't warn about disabling breakpoints during an inferior re-start. To achieve this I think we need to pass a new argument to disable_breakpoints_in_unloaded_shlib which controls whether we should print a message about the breakpoint being disabled or not. With this added we can now silence the warning when the inferior is restarted (i.e. when disable_breakpoints_in_unloaded_shlib is called from clear_solib), but keep the warning for cases like stepping over a dlclose() call in the inferior. After this commit, GDB now emits breakpoint modified events (in Python and/or MI) when a breakpoint is disabled as a result of all shared libraries being unloaded. This will be visible in two places that I can thing of, the 'nosharedlibrary' command, and when an inferior is restarted. --- gdb/breakpoint.c | 38 ++++++-------------------------------- 1 file changed, 6 insertions(+), 32 deletions(-) (limited to 'gdb/breakpoint.c') diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 189d7b8..0fb6fd9 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -3104,7 +3104,6 @@ insert_bp_location (struct bp_location *bl, || shared_objfile_contains_address_p (bl->pspace, bl->address))) { - /* See also: disable_breakpoints_in_shlibs. */ bl->shlib_disabled = 1; notify_breakpoint_modified (bl->owner); if (!*disabled_breaks) @@ -8084,44 +8083,19 @@ create_and_insert_solib_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR add return b; } -/* See breakpoint.h. */ - -void -disable_breakpoints_in_shlibs (program_space *pspace) -{ - for (bp_location *loc : all_bp_locations ()) - { - /* ALL_BP_LOCATIONS bp_location has LOC->OWNER always non-NULL. */ - struct breakpoint *b = loc->owner; - - /* We apply the check to all breakpoints, including disabled for - those with loc->duplicate set. This is so that when breakpoint - 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_jit_event) - || is_breakpoint (b) - || is_tracepoint (b)) - && loc->pspace == pspace - && !loc->shlib_disabled - && solib_name_from_address (loc->pspace, loc->address) - ) - { - loc->shlib_disabled = 1; - } - } -} - /* Disable any breakpoints and tracepoints that are in SOLIB upon notification of unloaded_shlib. Only apply to enabled breakpoints, disabled ones can just stay disabled. When STILL_IN_USE is true, SOLIB hasn't really been unmapped from - the inferior. In this case, don't disable anything. */ + the inferior. In this case, don't disable anything. + + When SILENT is false notify the user if any breakpoints are disabled, + otherwise, still disable the breakpoints, but don't tell the user. */ static void disable_breakpoints_in_unloaded_shlib (program_space *pspace, const solib &solib, - bool still_in_use) + bool still_in_use, bool silent) { if (still_in_use) return; @@ -8165,7 +8139,7 @@ disable_breakpoints_in_unloaded_shlib (program_space *pspace, const solib &solib bp_modified = true; - if (!disabled_shlib_breaks && user_breakpoint_p (&b)) + if (!disabled_shlib_breaks && !silent && user_breakpoint_p (&b)) { target_terminal::ours_for_output (); warning (_("Temporarily disabling breakpoints " -- cgit v1.1