This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Keep breakpoints always inserted.
- From: Vladimir Prus <ghost at cs dot msu dot su>
- To: gdb-patches at sources dot redhat dot com
- Date: Thu, 10 Apr 2008 02:27:00 +0400
- Subject: Re: [RFA] Keep breakpoints always inserted.
- References: <200802281717.14766.vladimir@codesourcery.com> <200803151312.15051.vladimir@codesourcery.com> <20080317222041.GC1309@caradoc.them.org> <200804031732.24478.vladimir@codesourcery.com> <20080407120806.GB5918@caradoc.them.org>
Daniel Jacobowitz wrote:
> On Thu, Apr 03, 2008 at 05:32:23PM +0400, Vladimir Prus wrote:
>> + Copyright 1992, 1993, 1994, 1995, 1999, 2002, 2003, 2007, 2008
>> + Free Software Foundation, Inc.
>
> Just 2008.
>
> Otherwise OK, thanks.
Unfortunately, while testing before commit I found an issue. The
watchpoint.exp test fails when running in always-inserted mode. I'm
not sure why I did not see that failure before, but at least I've
fixed it.
The problem was that when watchpoint is disabled, we failed to remove
the watchpoint from the target. We had the same bug for regular breakpoints,
but since for regular breakpoints we still do remove-insert dance when
stepping over breakpoints, the issue does not manifest.
I attach the revised patch, together with the delta relative to the
previous one. This has no regressions, neither in always-inserted nor
default mode. OK?
- Volodya
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 73a30df..3087f8f 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1033,6 +1033,9 @@ should_insert_location (struct bp_location *bpt)
if (!breakpoint_enabled (bpt->owner))
return 0;
+ if (bpt->owner->disposition == disp_del_at_next_stop)
+ return 0;
+
if (!bpt->enabled || bpt->shlib_disabled || bpt->inserted || bpt->duplicate)
return 0;
@@ -1301,7 +1304,7 @@ insert_breakpoint_locations (void)
ALL_BP_LOCATIONS_SAFE (b, temp)
{
- if (!breakpoint_enabled (b->owner))
+ if (!should_insert_location (b))
continue;
/* There is no point inserting thread-specific breakpoints if the
@@ -1329,6 +1332,9 @@ insert_breakpoint_locations (void)
if (bpt->enable_state != bp_enabled)
continue;
+
+ if (bpt->disposition == disp_del_at_next_stop)
+ continue;
for (loc = bpt->loc; loc; loc = loc->next)
if (!loc->inserted)
@@ -6961,8 +6967,9 @@ update_global_location_list (void)
we don't need to remove/insert the location. */
for (ix = 0; VEC_iterate(bp_location_p, old_locations, ix, loc); ++ix)
{
+ /* Tells if 'loc' is found amoung the new locations. If not, we
+ have to free it. */
int found_object = 0;
- int found_address = 0;
for (loc2 = bp_location_chain; loc2; loc2 = loc2->global_next)
if (loc2 == loc)
{
@@ -6975,42 +6982,56 @@ update_global_location_list (void)
inserted, and don't remove this one. This is needed so that we
don't have a time window where a breakpoint at certain location is not
inserted. */
- if (!found_object && loc->inserted)
+
+ if (loc->inserted)
{
- /* See if there's another location at the same address, in which
- case we don't need to remove this one. */
- if (breakpoint_address_is_meaningful (loc->owner))
- for (loc2 = bp_location_chain; loc2; loc2 = loc2->global_next)
- {
- /* For the sake of should_insert_location. The
- call to check_duplicates will fix up this later. */
- loc2->duplicate = 0;
- if (should_insert_location (loc2)
- && loc2 != loc && loc2->address == loc->address)
- {
- loc2->inserted = 1;
- loc2->target_info = loc->target_info;
- found_address = 1;
- break;
+ /* If the location is inserted now, we might have to remove it. */
+ int keep = 0;
+
+ if (found_object && should_insert_location (loc))
+ {
+ /* The location is still present in the location list, and still
+ should be inserted. Don't do anything. */
+ keep = 1;
+ }
+ else
+ {
+ /* The location is either no longer present, or got disabled.
+ See if there's another location at the same address, in which
+ case we don't need to remove this one from the target. */
+ if (breakpoint_address_is_meaningful (loc->owner))
+ for (loc2 = bp_location_chain; loc2; loc2 = loc2->global_next)
+ {
+ /* For the sake of should_insert_location. The
+ call to check_duplicates will fix up this later. */
+ loc2->duplicate = 0;
+ if (should_insert_location (loc2)
+ && loc2 != loc && loc2->address == loc->address)
+ {
+ loc2->inserted = 1;
+ loc2->target_info = loc->target_info;
+ keep = 1;
+ break;
+ }
}
+ }
+
+ if (!keep)
+ if (remove_breakpoint (loc, mark_uninserted))
+ {
+ /* This is just about all we can do. We could keep this
+ location on the global list, and try to remove it next
+ time, but there's no particular reason why we will
+ succeed next time.
+
+ Note that at this point, loc->owner is still valid,
+ as delete_breakpoint frees the breakpoint only
+ after calling us. */
+ printf_filtered (_("warning: Error removing breakpoint %d\n"),
+ loc->owner->number);
}
}
- if (loc->inserted && !found_object && !found_address)
- if (remove_breakpoint (loc, mark_uninserted))
- {
- /* This is just about all we can do. We could keep this
- location on the global list, and try to remove it next
- time, but there's no particular reason why we will
- succeed next time.
-
- Note that at this point, loc->owner is still valid,
- as delete_breakpoint frees the breakpoint only
- after calling us. */
- printf_filtered (_("warning: Error removing breakpoint %d\n"),
- loc->owner->number);
- }
-
if (!found_object)
free_bp_location (loc);
}
commit 02360e71972af63722ac8b98e093bfec82820b29
Author: Vladimir Prus <vladimir@codesourcery.com>
Date: Fri Nov 30 21:35:52 2007 +0300
Keep breakpoints always inserted.
[gdb]
* breakpoint.h (bp_location_p): New typedef.
Register a vector of bp_location_p.
* breakpoint.c (always_inserted_mode)
(show_always_inserted_mode): New.
(unlink_locations_from_global_list): Remove.
(update_global_location_list)
(update_global_location_list_nothrow): New.
(update_watchpoint): Don't free locations.
(should_insert_location): New.
(insert_bp_location): Use should_insert_location.
(insert_breakpoint_locations): Copied from
insert_breakpoints.
(insert_breakpoint): Use insert_breakpoint_locations.
(bpstat_stop_status): Call update_global_location_list
when disabling breakpoint.
(allocate_bp_location): Don't add to bp_location_chain.
(set_raw_breakpoint)
(create_longjmp_breakpoint, enable_longjmp_breakpoint)
(disable_longjmp_breakpoint, create_overlay_event_breakpoint)
(enable_overlay_breakpoints, disable_overlay_breakpoints)
(set_longjmp_resume_breakpoint)
(enable_watchpoints_after_interactive_call_stop)
(disable_watchpoints_before_interactive_call_start)
(create_internal_breakpoint)
(create_fork_vfork_event_catchpoint)
(create_exec_event_catchpoint, set_momentary_breakpoint)
(create_breakpoints, break_command_1, watch_command_1)
(create_exception_catchpoint)
(handle_gnu_v3_exceptions)
(disable_breakpoint, breakpoint_re_set_one)
(create_thread_event_breakpoint, create_solib_event_breakpoint)
(create_ada_exception_breakpoint): : Don't call check_duplicates.
Call update_global_location_list.
(delete_breakpoint): Don't remove locations and don't
try to reinsert them. Call update_global_location_list.
(update_breakpoint_locations): Likewise.
(restore_always_inserted_mode): New.
(update_breakpoints_after_exec): Temporary disable
always inserted mode.
* Makefile.in: Update dependencies.
* infrun.c (proceed): Remove breakpoints while stepping
over breakpoint.
(handle_inferior_event): Don't remove or insert
breakpoints.
* linux-fork.c (checkpoint_command): Remove breakpoints
before fork and insert after.
(linux_fork_context): Remove breakpoints before switch
and insert after.
* target.c (target_disconnect, target_detach): Remove
breakpoints from target.
[gdb/testsuite]
* lib/gdb.exp (gdb_continue_to_breakpoint): Allow the caller
to specify regexp for the location to stop at.
* gdb.base/break-always.c: New.
* gdb.base/break-always.exp: New.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index a2a9d4b..eef03b7 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1951,7 +1951,8 @@ breakpoint.o: breakpoint.c $(defs_h) $(symtab_h) $(frame_h) $(breakpoint_h) \
$(objfiles_h) $(source_h) $(linespec_h) $(completer_h) $(gdb_h) \
$(ui_out_h) $(cli_script_h) $(gdb_assert_h) $(block_h) $(solib_h) \
$(solist_h) $(observer_h) $(exceptions_h) $(gdb_events_h) \
- $(mi_common_h) $(memattr_h) $(ada_lang_h) $(top_h) $(hashtab_h)
+ $(mi_common_h) $(memattr_h) $(ada_lang_h) $(top_h) $(hashtab_h) \
+ $(gdb_stdint_h)
bsd-kvm.o: bsd-kvm.c $(defs_h) $(cli_cmds_h) $(command_h) $(frame_h) \
$(regcache_h) $(target_h) $(value_h) $(gdbcore_h) $(gdb_assert_h) \
$(readline_h) $(bsd_kvm_h)
diff --git a/gdb/NEWS b/gdb/NEWS
index 7d10ce5..6a7a736 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -31,6 +31,12 @@ show multiple-symbols
when an expression or a breakpoint location contains an ambiguous symbol
name (an overloaded function name, for instance).
+set breakpoint always-inserted
+show breakpoint always-inserted
+ Keep breakpoints always inserted in the target, as opposed to inserting
+ them when resuming the target, and removing them when the target stops.
+ This option can improve debugger performance on slow remote targets.
+
*** Changes in GDB 6.8
* New native configurations
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 6830efe..3087f8f 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -60,6 +60,8 @@
#include "gdb-events.h"
#include "mi/mi-common.h"
+#include "gdb_stdint.h"
+
/* Prototypes for local functions. */
static void until_break_command_continuation (struct continuation_arg *arg);
@@ -205,11 +207,13 @@ static void mark_breakpoints_out (void);
static struct bp_location *
allocate_bp_location (struct breakpoint *bpt, enum bptype bp_type);
-static void
-unlink_locations_from_global_list (struct breakpoint *bpt);
+static void update_global_location_list (void);
-static int
-is_hardware_watchpoint (struct breakpoint *bpt);
+static void update_global_location_list_nothrow (void);
+
+static int is_hardware_watchpoint (struct breakpoint *bpt);
+
+static void insert_breakpoint_locations (void);
/* Prototypes for exported functions. */
@@ -257,6 +261,18 @@ Automatic usage of hardware breakpoints is %s.\n"),
value);
}
+/* If 1, gdb will keep breakpoints inserted even as inferior is stopped,
+ and immediately insert any new breakpoints. If 0, gdb will insert
+ breakpoints into inferior only when resuming it, and will remove
+ breakpoints upon stop. */
+static int always_inserted_mode = 0;
+static void
+show_always_inserted_mode (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c, const char *value)
+{
+ fprintf_filtered (file, _("Always inserted breakpoint mode is %s.\n"), value);
+}
+
void _initialize_breakpoint (void);
@@ -867,14 +883,10 @@ update_watchpoint (struct breakpoint *b, int reparse)
struct bp_location *loc;
bpstat bs;
- unlink_locations_from_global_list (b);
- for (loc = b->loc; loc;)
- {
- struct bp_location *loc_next = loc->next;
- remove_breakpoint (loc, mark_uninserted);
- xfree (loc);
- loc = loc_next;
- }
+ /* We don't free locations. They are stored in
+ bp_location_chain and update_global_locations will
+ eventually delete them and remove breakpoints if
+ needed. */
b->loc = NULL;
if (b->disposition == disp_del_at_next_stop)
@@ -1013,6 +1025,23 @@ in which its expression is valid.\n"),
}
+/* Returns 1 iff breakpoint location should be
+ inserted in the inferior. */
+static int
+should_insert_location (struct bp_location *bpt)
+{
+ if (!breakpoint_enabled (bpt->owner))
+ return 0;
+
+ if (bpt->owner->disposition == disp_del_at_next_stop)
+ return 0;
+
+ if (!bpt->enabled || bpt->shlib_disabled || bpt->inserted || bpt->duplicate)
+ return 0;
+
+ return 1;
+}
+
/* Insert a low-level "breakpoint" of some type. BPT is the breakpoint.
Any error messages are printed to TMP_ERROR_STREAM; and DISABLED_BREAKS,
PROCESS_WARNING, and HW_BREAKPOINT_ERROR are used to report problems.
@@ -1027,10 +1056,7 @@ insert_bp_location (struct bp_location *bpt,
{
int val = 0;
- if (!breakpoint_enabled (bpt->owner))
- return 0;
-
- if (!bpt->enabled || bpt->shlib_disabled || bpt->inserted || bpt->duplicate)
+ if (!should_insert_location (bpt))
return 0;
/* Initialize the target-specific information. */
@@ -1231,13 +1257,35 @@ Note: automatically using hardware breakpoints for read-only addresses.\n"));
return 0;
}
+/* Make sure all breakpoints are inserted in inferior.
+ Throws exception on any error.
+ A breakpoint that is already inserted won't be inserted
+ again, so calling this function twice is safe. */
+void
+insert_breakpoints (void)
+{
+ struct breakpoint *bpt;
+
+ ALL_BREAKPOINTS (bpt)
+ if (is_hardware_watchpoint (bpt))
+ update_watchpoint (bpt, 0 /* don't reparse. */);
+
+ update_global_location_list ();
+
+ if (!always_inserted_mode && target_has_execution)
+ /* update_global_location_list does not insert breakpoints
+ when always_inserted_mode is not enabled. Explicitly
+ insert them now. */
+ insert_breakpoint_locations ();
+}
+
/* insert_breakpoints is used when starting or continuing the program.
remove_breakpoints is used when the program stops.
Both return zero if successful,
or an `errno' value if could not write the inferior. */
-void
-insert_breakpoints (void)
+static void
+insert_breakpoint_locations (void)
{
struct breakpoint *bpt;
struct bp_location *b, *temp;
@@ -1249,18 +1297,14 @@ insert_breakpoints (void)
struct ui_file *tmp_error_stream = mem_fileopen ();
make_cleanup_ui_file_delete (tmp_error_stream);
-
+
/* Explicitly mark the warning -- this will only be printed if
there was an error. */
fprintf_unfiltered (tmp_error_stream, "Warning:\n");
-
- ALL_BREAKPOINTS (bpt)
- if (is_hardware_watchpoint (bpt))
- update_watchpoint (bpt, 0 /* don't reparse */);
ALL_BP_LOCATIONS_SAFE (b, temp)
{
- if (!breakpoint_enabled (b->owner))
+ if (!should_insert_location (b))
continue;
/* There is no point inserting thread-specific breakpoints if the
@@ -1288,6 +1332,9 @@ insert_breakpoints (void)
if (bpt->enable_state != bp_enabled)
continue;
+
+ if (bpt->disposition == disp_del_at_next_stop)
+ continue;
for (loc = bpt->loc; loc; loc = loc->next)
if (!loc->inserted)
@@ -1395,17 +1442,31 @@ reattach_breakpoints (int pid)
return 0;
}
+static void
+restore_always_inserted_mode (void *p)
+{
+ always_inserted_mode = (uintptr_t) p;
+}
+
void
update_breakpoints_after_exec (void)
{
struct breakpoint *b;
struct breakpoint *temp;
+ struct cleanup *cleanup;
/* Doing this first prevents the badness of having delete_breakpoint()
write a breakpoint's current "shadow contents" to lift the bp. That
shadow is NOT valid after an exec()! */
mark_breakpoints_out ();
+ /* The binary we used to debug is now gone, and we're updating
+ breakpoints for the new binary. Until we're done, we should not
+ try to insert breakpoints. */
+ cleanup = make_cleanup (restore_always_inserted_mode,
+ (void *) (uintptr_t) always_inserted_mode);
+ always_inserted_mode = 0;
+
ALL_BREAKPOINTS_SAFE (b, temp)
{
/* Solib breakpoints must be explicitly reset after an exec(). */
@@ -1487,6 +1548,7 @@ update_breakpoints_after_exec (void)
}
/* FIXME what about longjmp breakpoints? Re-create them here? */
create_overlay_event_breakpoint ("_ovly_debug_event");
+ do_cleanups (cleanup);
}
int
@@ -1526,9 +1588,9 @@ remove_breakpoint (struct bp_location *b, insertion_state_t is)
/* Permanent breakpoints cannot be inserted or removed. */
return 0;
- if (b->owner->type == bp_none)
- warning (_("attempted to remove apparently deleted breakpoint #%d?"),
- b->owner->number);
+ /* The type of none suggests that owner is actually deleted.
+ This should not ever happen. */
+ gdb_assert (b->owner->type != bp_none);
if (b->loc_type == bp_loc_software_breakpoint
|| b->loc_type == bp_loc_hardware_breakpoint)
@@ -2953,7 +3015,10 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid)
{
/* We will stop here */
if (b->disposition == disp_disable)
- b->enable_state = bp_disabled;
+ {
+ b->enable_state = bp_disabled;
+ update_global_location_list ();
+ }
if (b->silent)
bs->print = 0;
bs->commands = b->commands;
@@ -4181,18 +4246,6 @@ allocate_bp_location (struct breakpoint *bpt, enum bptype bp_type)
internal_error (__FILE__, __LINE__, _("unknown breakpoint type"));
}
- /* Add this breakpoint to the end of the chain. */
-
- loc_p = bp_location_chain;
- if (loc_p == 0)
- bp_location_chain = loc;
- else
- {
- while (loc_p->global_next)
- loc_p = loc_p->global_next;
- loc_p->global_next = loc;
- }
-
return loc;
}
@@ -4200,6 +4253,10 @@ static void free_bp_location (struct bp_location *loc)
{
if (loc->cond)
xfree (loc->cond);
+
+ if (loc->function_name)
+ xfree (loc->function_name);
+
xfree (loc);
}
@@ -4304,7 +4361,6 @@ set_raw_breakpoint (struct symtab_and_line sal, enum bptype bptype)
set_breakpoint_location_function (b->loc);
- check_duplicates (b);
breakpoints_changed ();
return b;
@@ -4368,6 +4424,7 @@ create_longjmp_breakpoint (char *func_name)
b->silent = 1;
if (func_name)
b->addr_string = xstrdup (func_name);
+ update_global_location_list ();
}
/* Call this routine when stepping and nexting to enable a breakpoint
@@ -4383,7 +4440,7 @@ enable_longjmp_breakpoint (void)
if (b->type == bp_longjmp)
{
b->enable_state = bp_enabled;
- check_duplicates (b);
+ update_global_location_list ();
}
}
@@ -4397,7 +4454,7 @@ disable_longjmp_breakpoint (void)
|| b->type == bp_longjmp_resume)
{
b->enable_state = bp_disabled;
- check_duplicates (b);
+ update_global_location_list ();
}
}
@@ -4424,6 +4481,7 @@ create_overlay_event_breakpoint (char *func_name)
b->enable_state = bp_disabled;
overlay_events_enabled = 0;
}
+ update_global_location_list ();
}
void
@@ -4435,7 +4493,7 @@ enable_overlay_breakpoints (void)
if (b->type == bp_overlay_event)
{
b->enable_state = bp_enabled;
- check_duplicates (b);
+ update_global_location_list ();
overlay_events_enabled = 1;
}
}
@@ -4449,7 +4507,7 @@ disable_overlay_breakpoints (void)
if (b->type == bp_overlay_event)
{
b->enable_state = bp_disabled;
- check_duplicates (b);
+ update_global_location_list ();
overlay_events_enabled = 0;
}
}
@@ -4465,6 +4523,8 @@ create_thread_event_breakpoint (CORE_ADDR address)
/* addr_string has to be used or breakpoint_re_set will delete me. */
b->addr_string = xstrprintf ("*0x%s", paddr (b->loc->address));
+ update_global_location_list_nothrow ();
+
return b;
}
@@ -4509,6 +4569,7 @@ create_solib_event_breakpoint (CORE_ADDR address)
struct breakpoint *b;
b = create_internal_breakpoint (address, bp_shlib_event);
+ update_global_location_list_nothrow ();
return b;
}
@@ -4606,6 +4667,8 @@ create_fork_vfork_event_catchpoint (int tempflag, char *cond_string,
b->enable_state = bp_enabled;
b->disposition = tempflag ? disp_del : disp_donttouch;
b->forked_inferior_pid = 0;
+ update_global_location_list ();
+
mention (b);
}
@@ -4643,6 +4706,7 @@ create_exec_event_catchpoint (int tempflag, char *cond_string)
b->addr_string = NULL;
b->enable_state = bp_enabled;
b->disposition = tempflag ? disp_del : disp_donttouch;
+ update_global_location_list ();
mention (b);
}
@@ -4703,7 +4767,7 @@ set_longjmp_resume_breakpoint (CORE_ADDR pc, struct frame_id frame_id)
b->type);
b->enable_state = bp_enabled;
b->frame_id = frame_id;
- check_duplicates (b);
+ update_global_location_list ();
return;
}
}
@@ -4722,7 +4786,7 @@ disable_watchpoints_before_interactive_call_start (void)
&& breakpoint_enabled (b))
{
b->enable_state = bp_call_disabled;
- check_duplicates (b);
+ update_global_location_list ();
}
}
}
@@ -4741,7 +4805,7 @@ enable_watchpoints_after_interactive_call_stop (void)
&& (b->enable_state == bp_call_disabled))
{
b->enable_state = bp_enabled;
- check_duplicates (b);
+ update_global_location_list ();
}
}
}
@@ -4767,6 +4831,8 @@ set_momentary_breakpoint (struct symtab_and_line sal, struct frame_id frame_id,
if (in_thread_list (inferior_ptid))
b->thread = pid_to_thread_id (inferior_ptid);
+ update_global_location_list_nothrow ();
+
return b;
}
@@ -5166,6 +5232,8 @@ create_breakpoints (struct symtabs_and_lines sals, char **addr_string,
cond_string, type, disposition,
thread, ignore_count, from_tty);
}
+
+ update_global_location_list ();
}
/* Parse ARG which is assumed to be a SAL specification possibly
@@ -5483,6 +5551,8 @@ break_command_really (char *arg, char *cond_string, int thread,
b->ignore_count = ignore_count;
b->disposition = tempflag ? disp_del : disp_donttouch;
b->condition_not_parsed = 1;
+
+ update_global_location_list ();
mention (b);
}
@@ -5909,6 +5979,7 @@ watch_command_1 (char *arg, int accessflag, int from_tty)
value_free_to_mark (mark);
mention (b);
+ update_global_location_list ();
}
/* Return count of locations need to be watched and can be handled
@@ -6467,6 +6538,7 @@ handle_gnu_v3_exceptions (int tempflag, char *cond_string,
xfree (sals.sals);
mention (b);
+ update_global_location_list ();
return 1;
}
@@ -6539,6 +6611,7 @@ create_ada_exception_breakpoint (struct symtab_and_line sal,
b->ops = ops;
mention (b);
+ update_global_location_list ();
}
/* Implement the "catch exception" command. */
@@ -6860,33 +6933,124 @@ breakpoint_auto_delete (bpstat bs)
}
}
-/* Remove locations of breakpoint BPT from
- the global list of breakpoint locations. */
-
static void
-unlink_locations_from_global_list (struct breakpoint *bpt)
+update_global_location_list (void)
{
- /* This code assumes that the locations
- of a breakpoint are found in the global list
- in the same order, but not necessary adjacent. */
- struct bp_location **tmp = &bp_location_chain;
- struct bp_location *here = bpt->loc;
-
- if (here == NULL)
- return;
+ struct breakpoint *b;
+ struct bp_location **next = &bp_location_chain;
+ struct bp_location *loc;
+ struct bp_location *loc2;
+ struct gdb_exception e;
+ VEC(bp_location_p) *old_locations = NULL;
+ int ret;
+ int ix;
+
+ /* Store old locations for future reference. */
+ for (loc = bp_location_chain; loc; loc = loc->global_next)
+ VEC_safe_push (bp_location_p, old_locations, loc);
- for (; *tmp && here;)
+ bp_location_chain = NULL;
+ ALL_BREAKPOINTS (b)
{
- if (*tmp == here)
+ for (loc = b->loc; loc; loc = loc->next)
{
- *tmp = here->global_next;
- here = here->next;
+ *next = loc;
+ next = &(loc->global_next);
+ *next = NULL;
}
- else
+ }
+
+ /* Identify bp_location instances that are no longer present in the new
+ list, and therefore should be freed. Note that it's not necessary that
+ those locations should be removed from inferior -- if there's another
+ location at the same address (previously marked as duplicate),
+ we don't need to remove/insert the location. */
+ for (ix = 0; VEC_iterate(bp_location_p, old_locations, ix, loc); ++ix)
+ {
+ /* Tells if 'loc' is found amoung the new locations. If not, we
+ have to free it. */
+ int found_object = 0;
+ for (loc2 = bp_location_chain; loc2; loc2 = loc2->global_next)
+ if (loc2 == loc)
+ {
+ found_object = 1;
+ break;
+ }
+
+ /* If this location is no longer present, and inserted, look if there's
+ maybe a new location at the same address. If so, mark that one
+ inserted, and don't remove this one. This is needed so that we
+ don't have a time window where a breakpoint at certain location is not
+ inserted. */
+
+ if (loc->inserted)
{
- tmp = &((*tmp)->global_next);
+ /* If the location is inserted now, we might have to remove it. */
+ int keep = 0;
+
+ if (found_object && should_insert_location (loc))
+ {
+ /* The location is still present in the location list, and still
+ should be inserted. Don't do anything. */
+ keep = 1;
+ }
+ else
+ {
+ /* The location is either no longer present, or got disabled.
+ See if there's another location at the same address, in which
+ case we don't need to remove this one from the target. */
+ if (breakpoint_address_is_meaningful (loc->owner))
+ for (loc2 = bp_location_chain; loc2; loc2 = loc2->global_next)
+ {
+ /* For the sake of should_insert_location. The
+ call to check_duplicates will fix up this later. */
+ loc2->duplicate = 0;
+ if (should_insert_location (loc2)
+ && loc2 != loc && loc2->address == loc->address)
+ {
+ loc2->inserted = 1;
+ loc2->target_info = loc->target_info;
+ keep = 1;
+ break;
+ }
+ }
+ }
+
+ if (!keep)
+ if (remove_breakpoint (loc, mark_uninserted))
+ {
+ /* This is just about all we can do. We could keep this
+ location on the global list, and try to remove it next
+ time, but there's no particular reason why we will
+ succeed next time.
+
+ Note that at this point, loc->owner is still valid,
+ as delete_breakpoint frees the breakpoint only
+ after calling us. */
+ printf_filtered (_("warning: Error removing breakpoint %d\n"),
+ loc->owner->number);
+ }
}
+
+ if (!found_object)
+ free_bp_location (loc);
+ }
+
+ ALL_BREAKPOINTS (b)
+ {
+ check_duplicates (b);
}
+
+ if (always_inserted_mode && target_has_execution)
+ insert_breakpoint_locations ();
+}
+
+static void
+update_global_location_list_nothrow (void)
+{
+ struct gdb_exception e;
+ TRY_CATCH (e, RETURN_MASK_ERROR)
+ update_global_location_list ();
}
/* Delete a breakpoint and clean up all traces of it in the data
@@ -6897,7 +7061,7 @@ delete_breakpoint (struct breakpoint *bpt)
{
struct breakpoint *b;
bpstat bs;
- struct bp_location *loc;
+ struct bp_location *loc, *next;
gdb_assert (bpt != NULL);
@@ -6921,18 +7085,6 @@ delete_breakpoint (struct breakpoint *bpt)
deprecated_delete_breakpoint_hook (bpt);
breakpoint_delete_event (bpt->number);
- for (loc = bpt->loc; loc; loc = loc->next)
- {
- if (loc->inserted)
- remove_breakpoint (loc, mark_inserted);
-
- if (loc->cond)
- xfree (loc->cond);
-
- if (loc->function_name)
- xfree (loc->function_name);
- }
-
if (breakpoint_chain == bpt)
breakpoint_chain = bpt->next;
@@ -6943,85 +7095,6 @@ delete_breakpoint (struct breakpoint *bpt)
break;
}
- unlink_locations_from_global_list (bpt);
-
- check_duplicates (bpt);
-
- if (bpt->type != bp_hardware_watchpoint
- && bpt->type != bp_read_watchpoint
- && bpt->type != bp_access_watchpoint
- && bpt->type != bp_catch_fork
- && bpt->type != bp_catch_vfork
- && bpt->type != bp_catch_exec)
- for (loc = bpt->loc; loc; loc = loc->next)
- {
- /* If this breakpoint location was inserted, and there is
- another breakpoint at the same address, we need to
- insert the other breakpoint. */
- if (loc->inserted)
- {
- struct bp_location *loc2;
- ALL_BP_LOCATIONS (loc2)
- if (loc2->address == loc->address
- && loc2->section == loc->section
- && !loc->duplicate
- && loc2->owner->enable_state != bp_disabled
- && loc2->enabled
- && !loc2->shlib_disabled
- && loc2->owner->enable_state != bp_call_disabled)
- {
- int val;
-
- /* We should never reach this point if there is a permanent
- breakpoint at the same address as the one being deleted.
- If there is a permanent breakpoint somewhere, it should
- always be the only one inserted. */
- if (loc2->owner->enable_state == bp_permanent)
- internal_error (__FILE__, __LINE__,
- _("another breakpoint was inserted on top of "
- "a permanent breakpoint"));
-
- memset (&loc2->target_info, 0, sizeof (loc2->target_info));
- loc2->target_info.placed_address = loc2->address;
- if (b->type == bp_hardware_breakpoint)
- val = target_insert_hw_breakpoint (&loc2->target_info);
- else
- val = target_insert_breakpoint (&loc2->target_info);
-
- /* If there was an error in the insert, print a message, then stop execution. */
- if (val != 0)
- {
- struct ui_file *tmp_error_stream = mem_fileopen ();
- make_cleanup_ui_file_delete (tmp_error_stream);
-
-
- if (b->type == bp_hardware_breakpoint)
- {
- fprintf_unfiltered (tmp_error_stream,
- "Cannot insert hardware breakpoint %d.\n"
- "You may have requested too many hardware breakpoints.\n",
- b->number);
- }
- else
- {
- fprintf_unfiltered (tmp_error_stream, "Cannot insert breakpoint %d.\n", b->number);
- fprintf_filtered (tmp_error_stream, "Error accessing memory address ");
- fputs_filtered (paddress (loc2->address),
- tmp_error_stream);
- fprintf_filtered (tmp_error_stream, ": %s.\n",
- safe_strerror (val));
- }
-
- fprintf_unfiltered (tmp_error_stream,"The same program may be running in another process.");
- target_terminal_ours_for_output ();
- error_stream(tmp_error_stream);
- }
- else
- loc2->inserted = 1;
- }
- }
- }
-
free_command_lines (&bpt->commands);
if (bpt->cond_string != NULL)
xfree (bpt->cond_string);
@@ -7058,16 +7131,22 @@ delete_breakpoint (struct breakpoint *bpt)
bs->old_val = NULL;
/* bs->commands will be freed later. */
}
+
+ /* Now that breakpoint is removed from breakpoint
+ list, update the global location list. This
+ will remove locations that used to belong to
+ this breakpoint. Do this before freeing
+ the breakpoint itself, since remove_breakpoint
+ looks at location's owner. It might be better
+ design to have location completely self-contained,
+ but it's not the case now. */
+ update_global_location_list ();
+
+
/* On the chance that someone will soon try again to delete this same
bp, we mark it as deleted before freeing its storage. */
bpt->type = bp_none;
- for (loc = bpt->loc; loc;)
- {
- struct bp_location *loc_next = loc->next;
- xfree (loc);
- loc = loc_next;
- }
xfree (bpt);
}
@@ -7199,7 +7278,6 @@ update_breakpoint_locations (struct breakpoint *b,
if (all_locations_are_pending (existing_locations) && sals.nelts == 0)
return;
- unlink_locations_from_global_list (b);
b->loc = NULL;
for (i = 0; i < sals.nelts; ++i)
@@ -7278,12 +7356,7 @@ update_breakpoint_locations (struct breakpoint *b,
}
}
- while (existing_locations)
- {
- struct bp_location *next = existing_locations->next;
- free_bp_location (existing_locations);
- existing_locations = next;
- }
+ update_global_location_list ();
}
@@ -7379,10 +7452,6 @@ breakpoint_re_set_one (void *bint)
expanded = expand_line_sal_maybe (sals.sals[0]);
update_breakpoint_locations (b, expanded);
- /* Now that this is re-enabled, check_duplicates
- can be used. */
- check_duplicates (b);
-
xfree (sals.sals);
break;
@@ -7682,7 +7751,7 @@ disable_breakpoint (struct breakpoint *bpt)
bpt->enable_state = bp_disabled;
- check_duplicates (bpt);
+ update_global_location_list ();
if (deprecated_modify_breakpoint_hook)
deprecated_modify_breakpoint_hook (bpt);
@@ -7721,7 +7790,7 @@ disable_command (char *args, int from_tty)
struct bp_location *loc = find_location_by_number (args);
if (loc)
loc->enabled = 0;
- check_duplicates (loc->owner);
+ update_global_location_list ();
}
else
map_breakpoint_numbers (args, disable_breakpoint);
@@ -7806,7 +7875,7 @@ have been allocated for other watchpoints.\n"), bpt->number);
if (bpt->enable_state != bp_permanent)
bpt->enable_state = bp_enabled;
bpt->disposition = disposition;
- check_duplicates (bpt);
+ update_global_location_list ();
breakpoints_changed ();
if (deprecated_modify_breakpoint_hook)
@@ -7857,7 +7926,7 @@ enable_command (char *args, int from_tty)
struct bp_location *loc = find_location_by_number (args);
if (loc)
loc->enabled = 1;
- check_duplicates (loc->owner);
+ update_global_location_list ();
}
else
map_breakpoint_numbers (args, enable_breakpoint);
@@ -8025,6 +8094,11 @@ single_step_breakpoint_inserted_here_p (CORE_ADDR pc)
return 0;
}
+int breakpoints_always_inserted_mode (void)
+{
+ return always_inserted_mode;
+}
+
/* This help string is used for the break, hbreak, tbreak and thbreak commands.
It is defined as a macro to prevent duplication.
@@ -8426,6 +8500,19 @@ a warning will be emitted for such breakpoints."),
show_automatic_hardware_breakpoints,
&breakpoint_set_cmdlist,
&breakpoint_show_cmdlist);
+
+ add_setshow_boolean_cmd ("always-inserted", class_support,
+ &always_inserted_mode, _("\
+Set mode for inserting breakpoints."), _("\
+Show mode for inserting breakpoints."), _("\
+When this mode is off (which is the default), breakpoints are inserted in\n\
+inferior when it is resumed, and removed when execution stops. When this\n\
+mode is on, breakpoints are inserted immediately and removed only when\n\
+the user deletes the breakpoint."),
+ NULL,
+ &show_always_inserted_mode,
+ &breakpoint_set_cmdlist,
+ &breakpoint_show_cmdlist);
automatic_hardware_breakpoints = 1;
}
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 5376455..ed76f30 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -331,6 +331,9 @@ enum watchpoint_triggered
watch_triggered_yes
};
+typedef struct bp_location *bp_location_p;
+DEF_VEC_P(bp_location_p);
+
/* Note that the ->silent field is not currently used by any commands
(though the code is in there if it was to be, and set_raw_breakpoint
does set it to 0). I implemented it because I thought it would be
@@ -864,4 +867,6 @@ int watchpoints_triggered (struct target_waitstatus *);
void breakpoint_restore_shadows (gdb_byte *buf, ULONGEST memaddr,
LONGEST len);
+extern int breakpoints_always_inserted_mode (void);
+
#endif /* !defined (BREAKPOINT_H) */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9355d47..294557c 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3205,6 +3205,30 @@ type. If the target provides a memory map, @value{GDBN} will warn when
trying to set software breakpoint at a read-only address.
@end table
+@value{GDBN} normally implements breakpoints by replacing the program code
+at the breakpoint address with a special instruction, which, when
+executed, given control to the debugger. By default, the program
+code is so modified only when the program is resumed. As soon as
+the program stops, @value{GDBN} restores the original instructions. This
+behaviour guards against leaving breakpoints inserted in the
+target should gdb abrubptly disconnect. However, with slow remote
+targets, inserting and removing breakpoint can reduce the performance.
+This behavior can be controlled with the following commands::
+
+@kindex set breakpoint always-inserted
+@kindex show breakpoint always-inserted
+@table @code
+@item set breakpoint always-inserted off
+This is the default behaviour. All breakpoints, including newly added
+by the user, are inserted in the target only when the target is
+resumed. All breakpoints are removed from the target when it stops.
+
+@item set breakpoint always-inserted on
+Causes all breakpoints to be inserted in the target at all times. If
+the user adds a new breakpoint, or changes an existing breakpoint, the
+breakpoints in the target are updated immediately. A breakpoint is
+removed from the target only when breakpoint itself is removed.
+@end table
@cindex negative breakpoint numbers
@cindex internal @value{GDBN} breakpoints
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 6388d93..8ef91aa 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -614,19 +614,18 @@ a command like `return' or `jump' to continue execution."));
}
if ((step || singlestep_breakpoints_inserted_p)
- && breakpoint_here_p (read_pc ())
- && !breakpoint_inserted_here_p (read_pc ()))
+ && stepping_over_breakpoint)
{
- /* We're stepping, have breakpoint at PC, and it's
- not inserted. Most likely, proceed has noticed that
- we have breakpoint and tries to single-step over it,
- so that it's not hit. In which case, we need to
- single-step only this thread, and keep others stopped,
- as they can miss this breakpoint if allowed to run.
-
- The current code either has all breakpoints inserted,
- or all removed, so if we let other threads run,
- we can actually miss any breakpoint, not the one at PC. */
+ /* We're allowing a thread to run past a breakpoint it has
+ hit, by single-stepping the thread with the breakpoint
+ removed. In which case, we need to single-step only this
+ thread, and keep others stopped, as they can miss this
+ breakpoint if allowed to run.
+
+ The current code actually removes all breakpoints when
+ doing this, not just the one being stepped over, so if we
+ let other threads run, we can actually miss any
+ breakpoint, not just the one at PC. */
resume_ptid = inferior_ptid;
}
@@ -787,9 +786,17 @@ proceed (CORE_ADDR addr, enum target_signal siggnal, int step)
oneproc = 1;
if (oneproc)
- /* We will get a trace trap after one instruction.
- Continue it automatically and insert breakpoints then. */
- stepping_over_breakpoint = 1;
+ {
+ /* We will get a trace trap after one instruction.
+ Continue it automatically and insert breakpoints then. */
+ stepping_over_breakpoint = 1;
+ /* FIXME: if breakpoints are always inserted, we'll trap
+ if trying to single-step over breakpoint. Disable
+ all breakpoints. In future, we'd need to invent some
+ smart way of stepping over breakpoint instruction without
+ hitting breakpoint. */
+ remove_breakpoints ();
+ }
else
insert_breakpoints ();
@@ -1348,10 +1355,6 @@ handle_inferior_event (struct execution_control_state *ecs)
established. */
if (stop_soon == NO_STOP_QUIETLY)
{
- /* Remove breakpoints, SOLIB_ADD might adjust
- breakpoint addresses via breakpoint_re_set. */
- remove_breakpoints ();
-
/* Check for any newly added shared libraries if we're
supposed to be adding them automatically. Switch
terminal for any messages produced by
@@ -1391,9 +1394,6 @@ handle_inferior_event (struct execution_control_state *ecs)
/* NOTE drow/2007-05-11: This might be a good place to check
for "catch load". */
-
- /* Reinsert breakpoints and continue. */
- insert_breakpoints ();
}
/* If we are skipping through a shell, or through shared library
@@ -1402,6 +1402,10 @@ handle_inferior_event (struct execution_control_state *ecs)
we're attaching or setting up a remote connection. */
if (stop_soon == STOP_QUIETLY || stop_soon == NO_STOP_QUIETLY)
{
+ /* Loading of shared libraries might have changed breakpoint
+ addresses. Make sure new breakpoints are inserted. */
+ if (!breakpoints_always_inserted_mode ())
+ insert_breakpoints ();
resume (0, TARGET_SIGNAL_0);
prepare_to_wait (ecs);
return;
@@ -2039,8 +2043,7 @@ process_event_stop_test:
stop_signal = TARGET_SIGNAL_0;
if (prev_pc == read_pc ()
- && breakpoint_here_p (read_pc ())
- && !breakpoint_inserted_here_p (read_pc ())
+ && stepping_over_breakpoint
&& step_resume_breakpoint == NULL)
{
/* We were just starting a new sequence, attempting to
@@ -2216,10 +2219,6 @@ process_event_stop_test:
{
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_CHECK_SHLIBS\n");
- /* Remove breakpoints, we eventually want to step over the
- shlib event breakpoint, and SOLIB_ADD might adjust
- breakpoint addresses via breakpoint_re_set. */
- remove_breakpoints ();
/* Check for any newly added shared libraries if we're
supposed to be adding them automatically. Switch
@@ -3120,7 +3119,7 @@ normal_stop (void)
gdbarch_decr_pc_after_break needs to just go away. */
deprecated_update_frame_pc_hack (get_current_frame (), read_pc ());
- if (target_has_execution)
+ if (!breakpoints_always_inserted_mode () && target_has_execution)
{
if (remove_breakpoints ())
{
diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index 8c29827..a9f5fae 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -536,6 +536,10 @@ checkpoint_command (char *args, int from_tty)
/* Make this temp var static, 'cause it's used in the error context. */
static int temp_detach_fork;
+ /* Remove breakpoints, so that they are not inserted
+ in the forked process. */
+ remove_breakpoints ();
+
/* Make the inferior fork, record its (and gdb's) state. */
if (lookup_minimal_symbol ("fork", NULL, NULL) != NULL)
@@ -576,6 +580,7 @@ checkpoint_command (char *args, int from_tty)
if (!fp)
error (_("Failed to find new fork"));
fork_save_infrun_state (fp, 1);
+ insert_breakpoints ();
}
static void
@@ -593,7 +598,9 @@ linux_fork_context (struct fork_info *newfp, int from_tty)
oldfp = add_fork (ptid_get_pid (inferior_ptid));
fork_save_infrun_state (oldfp, 1);
+ remove_breakpoints ();
fork_load_infrun_state (newfp);
+ insert_breakpoints ();
printf_filtered (_("Switching to %s\n"),
target_pid_to_str (inferior_ptid));
diff --git a/gdb/target.c b/gdb/target.c
index 7f53944..944d601 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1676,6 +1676,10 @@ target_preopen (int from_tty)
void
target_detach (char *args, int from_tty)
{
+ /* If we're in breakpoints-always-inserted mode, have to
+ remove them before detaching. */
+ remove_breakpoints ();
+
(current_target.to_detach) (args, from_tty);
}
@@ -1684,6 +1688,10 @@ target_disconnect (char *args, int from_tty)
{
struct target_ops *t;
+ /* If we're in breakpoints-always-inserted mode, have to
+ remove them before disconnecting. */
+ remove_breakpoints ();
+
for (t = current_target.beneath; t != NULL; t = t->beneath)
if (t->to_disconnect != NULL)
{
diff --git a/gdb/testsuite/gdb.base/break-always.c b/gdb/testsuite/gdb.base/break-always.c
new file mode 100644
index 0000000..6c5515d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-always.c
@@ -0,0 +1,32 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2008 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+int bar ()
+{
+ return 1; /* break in bar */
+}
+
+int foo ()
+{
+ return bar ();
+}
+
+int main ()
+{
+ foo ();
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/break-always.exp b/gdb/testsuite/gdb.base/break-always.exp
new file mode 100644
index 0000000..6161943
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-always.exp
@@ -0,0 +1,32 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test that 'set breakpoint always-inserted 1' is not a brick
+
+if { [prepare_for_testing break-always.exp break-always break-always.c] } {
+ return -1
+}
+
+set bar_location [gdb_get_line_number "break in bar" break-always.c]
+
+gdb_test "set breakpoint always-inserted 1" ""
+
+runto foo
+
+gdb_test "break bar" "Breakpoint 2.*" "set breakpoint on bar"
+gdb_continue_to_breakpoint "bar" ".*/break-always.c:$bar_location.*"
+
+
+
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 9260e9a..1f8d842 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -430,13 +430,13 @@ proc runto_main { } {
### worked. Use NAME as part of the test name; each call to
### continue_to_breakpoint should use a NAME which is unique within
### that test file.
-proc gdb_continue_to_breakpoint {name} {
+proc gdb_continue_to_breakpoint {name {location_pattern .*}} {
global gdb_prompt
set full_name "continue to breakpoint: $name"
send_gdb "continue\n"
gdb_expect {
- -re "Breakpoint .* at .*\r\n$gdb_prompt $" {
+ -re "Breakpoint .* at $location_pattern\r\n$gdb_prompt $" {
pass $full_name
}
-re ".*$gdb_prompt $" {