From 74960c600257a48df8589c16b527ab6f56d1a664 Mon Sep 17 00:00:00 2001 From: Vladimir Prus Date: Thu, 24 Apr 2008 10:21:45 +0000 Subject: [PATCH] * 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/ChangeLog | 55 ++++ gdb/Makefile.in | 3 +- gdb/NEWS | 6 + gdb/breakpoint.c | 445 +++++++++++++++++++------------- gdb/breakpoint.h | 5 + gdb/doc/gdb.texinfo | 24 ++ gdb/infrun.c | 57 ++-- gdb/linux-fork.c | 7 + gdb/target.c | 8 + gdb/testsuite/ChangeLog | 7 + gdb/testsuite/gdb.base/break-always.c | 32 +++ gdb/testsuite/gdb.base/break-always.exp | 32 +++ gdb/testsuite/lib/gdb.exp | 4 +- 13 files changed, 474 insertions(+), 211 deletions(-) create mode 100644 gdb/testsuite/gdb.base/break-always.c create mode 100644 gdb/testsuite/gdb.base/break-always.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index f9742dc..806173c 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,60 @@ 2008-04-24 Vladimir Prus + * 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. + + +2008-04-24 Vladimir Prus + * breakpoint.c (print_one_breakpoint_location): In MI mode, report the location string the breakpoint was originally created with. diff --git a/gdb/Makefile.in b/gdb/Makefile.in index ba5f6ac..9304bc2 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -1974,7 +1974,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 2af9883..fcda6d1 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -43,6 +43,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 1ba9570..84802ef 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); static const char * bpdisp_text (enum bpdisp disp) @@ -265,6 +269,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); @@ -875,14 +891,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) @@ -1020,6 +1032,23 @@ in which its expression is valid.\n"), } +/* Returns 1 iff breakpoint location should be + inserted in the inferior. */ +static int +should_be_inserted (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->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. @@ -1034,10 +1063,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_be_inserted (bpt) || bpt->inserted) return 0; /* Initialize the target-specific information. */ @@ -1238,13 +1264,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; @@ -1256,18 +1304,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_be_inserted (b) || b->inserted) continue; /* There is no point inserting thread-specific breakpoints if the @@ -1295,6 +1339,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) @@ -1402,17 +1449,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(). */ @@ -1494,6 +1555,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 @@ -1533,9 +1595,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) @@ -2968,7 +3030,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; @@ -4203,18 +4268,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; } @@ -4222,6 +4275,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); } @@ -4326,7 +4383,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; @@ -4390,6 +4446,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 @@ -4405,7 +4462,7 @@ enable_longjmp_breakpoint (void) if (b->type == bp_longjmp) { b->enable_state = bp_enabled; - check_duplicates (b); + update_global_location_list (); } } @@ -4419,7 +4476,7 @@ disable_longjmp_breakpoint (void) || b->type == bp_longjmp_resume) { b->enable_state = bp_disabled; - check_duplicates (b); + update_global_location_list (); } } @@ -4446,6 +4503,7 @@ create_overlay_event_breakpoint (char *func_name) b->enable_state = bp_disabled; overlay_events_enabled = 0; } + update_global_location_list (); } void @@ -4457,7 +4515,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; } } @@ -4471,7 +4529,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; } } @@ -4487,6 +4545,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; } @@ -4531,6 +4591,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; } @@ -4628,6 +4689,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); } @@ -4665,6 +4728,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); } @@ -4725,7 +4789,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; } } @@ -4744,7 +4808,7 @@ disable_watchpoints_before_interactive_call_start (void) && breakpoint_enabled (b)) { b->enable_state = bp_call_disabled; - check_duplicates (b); + update_global_location_list (); } } } @@ -4763,7 +4827,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 (); } } } @@ -4789,6 +4853,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; } @@ -5192,6 +5258,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 @@ -5509,6 +5577,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); } @@ -5935,6 +6005,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 @@ -6493,6 +6564,7 @@ handle_gnu_v3_exceptions (int tempflag, char *cond_string, xfree (sals.sals); mention (b); + update_global_location_list (); return 1; } @@ -6565,6 +6637,7 @@ create_ada_exception_breakpoint (struct symtab_and_line sal, b->ops = ops; mention (b); + update_global_location_list (); } /* Implement the "catch exception" command. */ @@ -6886,33 +6959,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_be_inserted (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_be_inserted (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 @@ -6923,7 +7087,7 @@ delete_breakpoint (struct breakpoint *bpt) { struct breakpoint *b; bpstat bs; - struct bp_location *loc; + struct bp_location *loc, *next; gdb_assert (bpt != NULL); @@ -6947,18 +7111,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; @@ -6969,85 +7121,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); @@ -7084,16 +7157,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); } @@ -7225,7 +7304,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) @@ -7304,12 +7382,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 (); } @@ -7405,10 +7478,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; @@ -7708,7 +7777,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); @@ -7747,7 +7816,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); @@ -7832,7 +7901,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) @@ -7883,7 +7952,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); @@ -8051,6 +8120,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. @@ -8452,6 +8526,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 6337790..e422d44 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -3209,6 +3209,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 df042a1..da8c3e0 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/ChangeLog b/gdb/testsuite/ChangeLog index 37855d5..eb50238 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,5 +1,12 @@ 2008-04-24 Vladimir Prus + * 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. + +2008-04-24 Vladimir Prus + * lib/mi-support.exp (mi_runto_helper): Adjust for the original-location field. (mi_create_breakpoint, mi_list_breakpoints): New. 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 . */ + +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 . + +# 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 8ad8792..bb9bd24 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -432,13 +432,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 $" { -- 2.7.4