Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] Keep breakpoints always inserted.
@ 2008-02-28 14:18 Vladimir Prus
  2008-03-10 21:09 ` Daniel Jacobowitz
  0 siblings, 1 reply; 19+ messages in thread
From: Vladimir Prus @ 2008-02-28 14:18 UTC (permalink / raw)
  To: gdb-patches


This is hopefully final revision of my patch to keep
breakpoints always inserted, which is needed to support
non-stop mode. A new variable 'breakpoint always-inserted'
is introduced that enables this mode.  When enabled,
breakpoints are inserted into target immediately when created,
and they are not removed when we stop.

Compared to the previous version of this patch:
- The always-inserted mode is actually configurable
- In always-inserted mode, breakpoints are removed
from target on detach/disconnect.

OK?

- Volodya

	* 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/Makefile.in  |    5 +-
 gdb/breakpoint.c |  419 +++++++++++++++++++++++++++++++-----------------------
 gdb/breakpoint.h |    5 +
 gdb/infrun.c     |   27 ++--
 gdb/linux-fork.c |    7 +
 gdb/target.c     |   14 ++-
 6 files changed, 281 insertions(+), 196 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index ba64982..114129f 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)
@@ -2871,7 +2872,7 @@ symtab.o: symtab.c $(defs_h) $(symtab_h) $(gdbtypes_h) $(gdbcore_h) \
 target.o: target.c $(defs_h) $(gdb_string_h) $(target_h) $(gdbcmd_h) \
 	$(symtab_h) $(inferior_h) $(bfd_h) $(symfile_h) $(objfiles_h) \
 	$(gdb_wait_h) $(dcache_h) $(regcache_h) $(gdb_assert_h) $(gdbcore_h) \
-	$(exceptions_h) $(target_descriptions_h)
+	$(exceptions_h) $(target_descriptions_h) $(gdb_stdint_h)
 target-descriptions.o: target-descriptions.c $(defs_h) $(arch_utils_h) \
 	$(target_h) $(target_descriptions_h) $(vec_h) $(xml_tdesc_h) \
 	$(gdbcmd_h) $(gdb_assert_h) $(gdbtypes_h) $(reggroups_h) \
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index d43a608..fd7364a 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -59,6 +59,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);
@@ -204,11 +206,14 @@ 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. */
 
@@ -256,6 +261,20 @@ 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
+   breakoints.
+   If 0, gdb will insert breakpoints into inferior only
+   when rusuming 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);
 
@@ -831,14 +850,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)
@@ -983,6 +998,20 @@ 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->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.
@@ -997,10 +1026,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.  */
@@ -1201,13 +1227,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;
@@ -1219,14 +1267,10 @@ 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)
     {
@@ -1365,17 +1409,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(). */
@@ -1457,6 +1515,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
@@ -1496,9 +1555,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)
@@ -2892,7 +2951,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;
@@ -4120,18 +4182,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;
 }
 
@@ -4139,6 +4189,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);
 }
 
@@ -4243,7 +4297,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;
@@ -4307,6 +4360,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
@@ -4322,7 +4376,7 @@ enable_longjmp_breakpoint (void)
     if (b->type == bp_longjmp)
     {
       b->enable_state = bp_enabled;
-      check_duplicates (b);
+      update_global_location_list ();
     }
 }
 
@@ -4336,7 +4390,7 @@ disable_longjmp_breakpoint (void)
 	|| b->type == bp_longjmp_resume)
     {
       b->enable_state = bp_disabled;
-      check_duplicates (b);
+      update_global_location_list ();
     }
 }
 
@@ -4363,6 +4417,7 @@ create_overlay_event_breakpoint (char *func_name)
       b->enable_state = bp_disabled;
       overlay_events_enabled = 0;
     }
+  update_global_location_list ();
 }
 
 void
@@ -4374,7 +4429,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;
     }
 }
@@ -4388,7 +4443,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;
     }
 }
@@ -4404,6 +4459,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;
 }
 
@@ -4448,6 +4505,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;
 }
 
@@ -4545,6 +4603,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);
 }
@@ -4582,6 +4642,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);
 }
@@ -4642,7 +4703,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;
     }
 }
@@ -4661,7 +4722,7 @@ disable_watchpoints_before_interactive_call_start (void)
 	&& breakpoint_enabled (b))
       {
 	b->enable_state = bp_call_disabled;
-	check_duplicates (b);
+	update_global_location_list ();
       }
   }
 }
@@ -4680,7 +4741,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 ();
       }
   }
 }
@@ -4706,6 +4767,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;
 }
 \f
@@ -5105,6 +5168,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
@@ -5422,6 +5487,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);
     }
   
@@ -5824,6 +5891,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
@@ -6382,6 +6450,7 @@ handle_gnu_v3_exceptions (int tempflag, char *cond_string,
 
   xfree (sals.sals);
   mention (b);
+  update_global_location_list ();
   return 1;
 }
 
@@ -6454,6 +6523,7 @@ create_ada_exception_breakpoint (struct symtab_and_line sal,
   b->ops = ops;
 
   mention (b);
+  update_global_location_list ();
 }
 
 /* Implement the "catch exception" command.  */
@@ -6775,33 +6845,105 @@ 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;
+  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);
 
-  if (here == NULL)
-    return;
+  bp_location_chain = NULL;
+  ALL_BREAKPOINTS (b)
+    {
+      for (loc = b->loc; loc; loc = loc->next)
+	{
+	  *next = loc;
+	  next = &(loc->global_next);
+	  *next = NULL;
+	}
+    }
 
-  for (; *tmp && here;)
+  /* Identify bp_location instances that are not
+     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)
     {
-      if (*tmp == here)
+      int found_object = 0;
+      int found_address = 0;
+      for (loc2 = bp_location_chain; loc2; loc2 = loc2->global_next)
+	if (loc2 == loc)
+	  {
+	    found_object = 1;
+	    break;
+	  }
+
+      /* If this location is not longer present, and
+	 inserted, look if there's maybe 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 window
+	 where a breakpoint at certian location is not
+	 inserted.  */
+      if (!found_object && loc->inserted)
 	{
-	  *tmp = here->global_next;
-	  here = here->next;
+	  /* 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 the 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;
+		  }
+	      }
 	}
-      else
+
+      if (loc->inserted && !found_object && !found_address)
 	{
-	  tmp = &((*tmp)->global_next);
+	  /* FIXME? Handle error? */
+	  remove_breakpoint (loc, mark_uninserted);
 	}
+
+      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
@@ -6812,7 +6954,7 @@ delete_breakpoint (struct breakpoint *bpt)
 {
   struct breakpoint *b;
   bpstat bs;
-  struct bp_location *loc;
+  struct bp_location *loc, *next;
 
   gdb_assert (bpt != NULL);
 
@@ -6836,18 +6978,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;
 
@@ -6858,85 +6988,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);
@@ -6973,16 +7024,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);
 }
 
@@ -7114,7 +7171,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)
@@ -7193,12 +7249,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 ();
 }
 
 
@@ -7294,10 +7345,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;
 
@@ -7597,7 +7644,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);
@@ -7636,7 +7683,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);
@@ -7721,7 +7768,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)
@@ -7772,7 +7819,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);
@@ -7940,6 +7987,11 @@ single_step_breakpoint_inserted_here_p (CORE_ADDR pc)
   return 0;
 }
 
+int breakpoints_always_inserted_mode ()
+{
+  return always_inserted_mode;
+}
+
 \f
 /* This help string is used for the break, hbreak, tbreak and thbreak commands.
    It is defined as a macro to prevent duplication.
@@ -8341,6 +8393,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 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 159c406..9e719f8 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
@@ -858,4 +861,6 @@ extern void breakpoint_restore_shadows (gdb_byte *buf,
 					ULONGEST memaddr, 
 					LONGEST len);
 
+extern int breakpoints_always_inserted_mode ();
+
 #endif /* !defined (BREAKPOINT_H) */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index c863736..6791cb4 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -789,9 +789,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 ();
 
@@ -1350,10 +1358,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
@@ -1393,9 +1397,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
@@ -2235,10 +2236,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
@@ -3139,7 +3136,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 32271eb..0dae58c 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -39,6 +39,7 @@
 #include "gdbcore.h"
 #include "exceptions.h"
 #include "target-descriptions.h"
+#include "gdb_stdint.h"
 
 static void target_info (char *, int);
 
@@ -1120,7 +1121,7 @@ memory_xfer_partial (struct target_ops *ops, void *readbuf, const void *writebuf
 static void
 restore_show_memory_breakpoints (void *arg)
 {
-  show_memory_breakpoints = (int)arg;
+  show_memory_breakpoints = (uintptr_t) arg;
 }
 
 struct cleanup *
@@ -1129,7 +1130,8 @@ make_show_memory_breakpoints_cleanup (int show)
   int current = show_memory_breakpoints;
   show_memory_breakpoints = show;
 
-  return make_cleanup (restore_show_memory_breakpoints, (void *)current);
+  return make_cleanup (restore_show_memory_breakpoints, 
+		       (void *) (uintptr_t) current);
 }
 
 static LONGEST
@@ -1682,6 +1684,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);
 }
 
@@ -1690,6 +1696,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)
 	{
-- 
1.5.3.5


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-02-28 14:18 [RFA] Keep breakpoints always inserted Vladimir Prus
@ 2008-03-10 21:09 ` Daniel Jacobowitz
  2008-03-14 20:22   ` Vladimir Prus
  2008-03-15 10:13   ` Vladimir Prus
  0 siblings, 2 replies; 19+ messages in thread
From: Daniel Jacobowitz @ 2008-03-10 21:09 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

On Thu, Feb 28, 2008 at 05:17:13PM +0300, Vladimir Prus wrote:
> 
> This is hopefully final revision of my patch to keep
> breakpoints always inserted, which is needed to support
> non-stop mode. A new variable 'breakpoint always-inserted'
> is introduced that enables this mode.  When enabled,
> breakpoints are inserted into target immediately when created,
> and they are not removed when we stop.
> 
> Compared to the previous version of this patch:
> - The always-inserted mode is actually configurable
> - In always-inserted mode, breakpoints are removed
> from target on detach/disconnect.
> 
> OK?

This patch is OK, if you will add the missing pieces: a NEWS entry and
documentation for the new command, and a testcase that enables
always-inserted and verifies that it isn't a brick.

> +static void
> +insert_breakpoint_locations (void);

No newline before function name in prototypes.

> +/* If 1, gdb will keep breakpoints inserted even as 
> +   inferior is stopped, and immediately insert any new
> +   breakoints.
> +   If 0, gdb will insert breakpoints into inferior only
> +   when rusuming it, and will remove breakpoints
> +   upon stop.  */

Typos: breakoints, rusuming.  I've noticed your comments always
wrap early, around column 60 instead of 72 or 79; is there any
particular reason for that?  It looks awkward when other comments in
the same file wrap further over.

> +  /* Identify bp_location instances that are not
> +     longer present in the new list, and therefore should

no longer present

> +      /* If this location is not longer present, and

ditto

> +	 inserted, look if there's maybe new location

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 window
> +	 where a breakpoint at certian location is not

certain

> +		/* For the sake of should_insert_location.  The
> +		   call the check_duplicates will fix up this later.  */

the call to

> +      if (loc->inserted && !found_object && !found_address)
>  	{
> -	  tmp = &((*tmp)->global_next);
> +	  /* FIXME? Handle error? */
> +	  remove_breakpoint (loc, mark_uninserted);
>  	}

Can you take care of this FIXME?

> +int breakpoints_always_inserted_mode ()
> +{
> +  return always_inserted_mode;
> +}

(void)


> +
>  \f
>  /* This help string is used for the break, hbreak, tbreak and thbreak commands.
>     It is defined as a macro to prevent duplication.
> @@ -8341,6 +8393,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 default), breakpoints are inserted in\n\

which is the default

> +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."),
> +extern int breakpoints_always_inserted_mode ();

(void)

> @@ -1350,10 +1358,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
> @@ -1393,9 +1397,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

This does the right thing even with breakpoints not always inserted,
right?

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-03-10 21:09 ` Daniel Jacobowitz
@ 2008-03-14 20:22   ` Vladimir Prus
  2008-03-15 10:13   ` Vladimir Prus
  1 sibling, 0 replies; 19+ messages in thread
From: Vladimir Prus @ 2008-03-14 20:22 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

On Tuesday 11 March 2008 00:08:44 Daniel Jacobowitz wrote:
> On Thu, Feb 28, 2008 at 05:17:13PM +0300, Vladimir Prus wrote:
> > 
> > This is hopefully final revision of my patch to keep
> > breakpoints always inserted, which is needed to support
> > non-stop mode. A new variable 'breakpoint always-inserted'
> > is introduced that enables this mode.  When enabled,
> > breakpoints are inserted into target immediately when created,
> > and they are not removed when we stop.
> > 
> > Compared to the previous version of this patch:
> > - The always-inserted mode is actually configurable
> > - In always-inserted mode, breakpoints are removed
> > from target on detach/disconnect.
> > 
> > OK?
> 
> This patch is OK, if you will add the missing pieces: a NEWS entry and
> documentation for the new command, and a testcase that enables
> always-inserted and verifies that it isn't a brick.

Hmm. I think the ideally, we'd run entire testsuite with this
mode enabled -- there are various corner cases. But then, we might
regress the not-always-inserted-breakpoints mode. Although the
potential for breakage is higher for always-inserted mode. I really
don't know how to ensure good coverage for both.

- Volodya


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-03-10 21:09 ` Daniel Jacobowitz
  2008-03-14 20:22   ` Vladimir Prus
@ 2008-03-15 10:13   ` Vladimir Prus
  2008-03-15 15:10     ` Eli Zaretskii
  2008-03-17 22:21     ` Daniel Jacobowitz
  1 sibling, 2 replies; 19+ messages in thread
From: Vladimir Prus @ 2008-03-15 10:13 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 2081 bytes --]

On Tuesday 11 March 2008 00:08:44 Daniel Jacobowitz wrote:
> On Thu, Feb 28, 2008 at 05:17:13PM +0300, Vladimir Prus wrote:
> > 
> > This is hopefully final revision of my patch to keep
> > breakpoints always inserted, which is needed to support
> > non-stop mode. A new variable 'breakpoint always-inserted'
> > is introduced that enables this mode.  When enabled,
> > breakpoints are inserted into target immediately when created,
> > and they are not removed when we stop.
> > 
> > Compared to the previous version of this patch:
> > - The always-inserted mode is actually configurable
> > - In always-inserted mode, breakpoints are removed
> > from target on detach/disconnect.
> > 
> > OK?
> 
> This patch is OK, if you will add the missing pieces: a NEWS entry and
> documentation for the new command, and a testcase that enables
> always-inserted and verifies that it isn't a brick.

I've added NEW and documentation.

I'm still not sure how to test this adequately.

> > @@ -1350,10 +1358,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
> > @@ -1393,9 +1397,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
> 
> This does the right thing even with breakpoints not always inserted,
> right?

I'm afraid no. On this code path, we call 'resume' immediately, which does not
insert breakpoints. I've fixed this. Does does this one look?

- Volodya

 



[-- Attachment #2: 0001-Keep-breakpoints-always-inserted.patch --]
[-- Type: text/x-diff, Size: 37850 bytes --]

From 79766b2a4096b2c27847c3737232184581ead9ed Mon Sep 17 00:00:00 2001
From: Vladimir Prus <vladimir@codesourcery.com>
Date: Fri, 30 Nov 2007 21:35:52 +0300
Subject: [RFA] Keep breakpoints always inserted.
To: gdb-patches@sources.redhat.com
X-KMail-Transport: CodeSourcery
X-KMail-Identity: 901867920

	* 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/Makefile.in     |    3 +-
 gdb/NEWS            |    4 +
 gdb/breakpoint.c    |  422 +++++++++++++++++++++++++++++----------------------
 gdb/breakpoint.h    |    5 +
 gdb/doc/gdb.texinfo |   21 +++
 gdb/infrun.c        |   57 ++++----
 gdb/linux-fork.c    |    7 +
 gdb/target.c        |    8 +
 8 files changed, 319 insertions(+), 208 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index a3e73b9..eb4c6a5 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 4aba328..173e3c1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -22,6 +22,10 @@ show exec-wrapper
 unset exec-wrapper
   Use a wrapper program to launch programs for debugging.
 
+set breakpoint always-inserted
+show breakpoint always-inserted
+  Keep breakpoints always inserted in the target.
+
 *** Changes in GDB 6.8
 
 * New native configurations
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 6830efe..73a30df 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,20 @@ 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->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 +1053,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 +1254,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,14 +1294,10 @@ 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)
     {
@@ -1395,17 +1436,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 +1542,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 +1582,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 +3009,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 +4240,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 +4247,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 +4355,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 +4418,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 +4434,7 @@ enable_longjmp_breakpoint (void)
     if (b->type == bp_longjmp)
     {
       b->enable_state = bp_enabled;
-      check_duplicates (b);
+      update_global_location_list ();
     }
 }
 
@@ -4397,7 +4448,7 @@ disable_longjmp_breakpoint (void)
 	|| b->type == bp_longjmp_resume)
     {
       b->enable_state = bp_disabled;
-      check_duplicates (b);
+      update_global_location_list ();
     }
 }
 
@@ -4424,6 +4475,7 @@ create_overlay_event_breakpoint (char *func_name)
       b->enable_state = bp_disabled;
       overlay_events_enabled = 0;
     }
+  update_global_location_list ();
 }
 
 void
@@ -4435,7 +4487,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 +4501,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 +4517,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 +4563,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 +4661,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 +4700,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 +4761,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 +4780,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 +4799,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 +4825,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;
 }
 \f
@@ -5166,6 +5226,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 +5545,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 +5973,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 +6532,7 @@ handle_gnu_v3_exceptions (int tempflag, char *cond_string,
 
   xfree (sals.sals);
   mention (b);
+  update_global_location_list ();
   return 1;
 }
 
@@ -6539,6 +6605,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 +6927,109 @@ 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)
+    {
+      int found_object = 0;
+      int found_address = 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 (!found_object && loc->inserted)
 	{
-	  tmp = &((*tmp)->global_next);
+	  /* 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 (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);
+    }
+    
+  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 +7040,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 +7064,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 +7074,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 +7110,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 +7257,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 +7335,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 +7431,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 +7730,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 +7769,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 +7854,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 +7905,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 +8073,11 @@ single_step_breakpoint_inserted_here_p (CORE_ADDR pc)
   return 0;
 }
 
+int breakpoints_always_inserted_mode (void)
+{
+  return always_inserted_mode;
+}
+
 \f
 /* This help string is used for the break, hbreak, tbreak and thbreak commands.
    It is defined as a macro to prevent duplication.
@@ -8426,6 +8479,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 dbc9efc..78f8363 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3205,6 +3205,27 @@ 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
 
+By default, @value{GDBN} inserts breakpoints in the target only when
+resuming the target, and removes breakpoints whenever the target stop.
+This behaviour guards against leaving breakpoints inserted in the
+target should gdb abrubptly disconnect, which is possible with remote
+targets.  This bevaious 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 c863736..43e59a7 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -616,19 +616,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;
 	}
 
@@ -789,9 +788,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 ();
 
@@ -1350,10 +1357,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
@@ -1393,9 +1396,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
@@ -1404,6 +1404,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;
@@ -2058,8 +2062,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
@@ -2235,10 +2238,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
@@ -3139,7 +3138,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 9d2f1fd..b5c3cd3 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1677,6 +1677,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);
 }
 
@@ -1685,6 +1689,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)
 	{
-- 
1.5.3.5


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-03-15 10:13   ` Vladimir Prus
@ 2008-03-15 15:10     ` Eli Zaretskii
  2008-04-02 13:02       ` Vladimir Prus
  2008-03-17 22:21     ` Daniel Jacobowitz
  1 sibling, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2008-03-15 15:10 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: drow, gdb-patches

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Sat, 15 Mar 2008 13:12:14 +0300
> Cc: gdb-patches@sources.redhat.com
> 
> I've added NEW and documentation.

Thanks.

> +set breakpoint always-inserted
> +show breakpoint always-inserted
> +  Keep breakpoints always inserted in the target.

This entry is not detailed enough to be useful.  Please keep in mind
that GDB users are not necessarily aware of the fact that GDB removes
the breakpoints whenever the inferior stops and reinserts them before
resuming it.

> +By default, @value{GDBN} inserts breakpoints in the target only when
> +resuming the target, and removes breakpoints whenever the target stop.
                                                                    ^^^^
"stops".

> +This behaviour guards against leaving breakpoints inserted in the
> +target should gdb abrubptly disconnect, which is possible with remote
> +targets.

Without explaining that a breakpoint replaces a portion of code with a
breakpoint instruction, the reader will not understand what is
dangerous about this.

>                      This bevaious can be controlled with the following
                            ^^^^^^^^
You mean "behavior", right?

> +This is the default behaviour.  All breakpoints, including newly added
                       ^^^^^^^^^
Please use the US spelling ("behavior"), for consistency.

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

It would be good to give the reader a hint when this behavior is
useful.

Otherwise, okay with me.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-03-15 10:13   ` Vladimir Prus
  2008-03-15 15:10     ` Eli Zaretskii
@ 2008-03-17 22:21     ` Daniel Jacobowitz
  2008-03-18  4:15       ` Eli Zaretskii
  2008-04-03 18:10       ` Vladimir Prus
  1 sibling, 2 replies; 19+ messages in thread
From: Daniel Jacobowitz @ 2008-03-17 22:21 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

On Sat, Mar 15, 2008 at 01:12:14PM +0300, Vladimir Prus wrote:
> > This patch is OK, if you will add the missing pieces: a NEWS entry and
> > documentation for the new command, and a testcase that enables
> > always-inserted and verifies that it isn't a brick.
> 
> I've added NEW and documentation.
> 
> I'm still not sure how to test this adequately.

This version is OK when Eli is happy with the documentation.

For a testcase, I agree that we need to run the whole testsuite in
this mode to get full coverage.  But it's useful to have a single
test that runs in this mode even if the rest of the testsuite
doesn't, in case someone completely breaks it.  That's why the
gdb.server tests run for native targets that build gdbserver,
even when the rest of the testsuite uses "run".

Could you write a test that uses "set break always-inserted" and does
something simple like running to a function and then continuing past
it to another breakpoint?

> +	  /* 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.  */

Extra spacing here?

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-03-17 22:21     ` Daniel Jacobowitz
@ 2008-03-18  4:15       ` Eli Zaretskii
  2008-03-18  6:06         ` Vladimir Prus
  2008-04-03 18:10       ` Vladimir Prus
  1 sibling, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2008-03-18  4:15 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: vladimir, gdb-patches

> Date: Mon, 17 Mar 2008 18:20:41 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: gdb-patches@sources.redhat.com
> 
> On Sat, Mar 15, 2008 at 01:12:14PM +0300, Vladimir Prus wrote:
> > > This patch is OK, if you will add the missing pieces: a NEWS entry and
> > > documentation for the new command, and a testcase that enables
> > > always-inserted and verifies that it isn't a brick.
> > 
> > I've added NEW and documentation.
> > 
> > I'm still not sure how to test this adequately.
> 
> This version is OK when Eli is happy with the documentation.

I already reviewed it, didn't I?  If I missed it, please point me to
the archives.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-03-18  4:15       ` Eli Zaretskii
@ 2008-03-18  6:06         ` Vladimir Prus
  0 siblings, 0 replies; 19+ messages in thread
From: Vladimir Prus @ 2008-03-18  6:06 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Daniel Jacobowitz, gdb-patches

On Tuesday 18 March 2008 07:14:58 Eli Zaretskii wrote:
> > Date: Mon, 17 Mar 2008 18:20:41 -0400
> > From: Daniel Jacobowitz <drow@false.org>
> > Cc: gdb-patches@sources.redhat.com
> > 
> > On Sat, Mar 15, 2008 at 01:12:14PM +0300, Vladimir Prus wrote:
> > > > This patch is OK, if you will add the missing pieces: a NEWS entry and
> > > > documentation for the new command, and a testcase that enables
> > > > always-inserted and verifies that it isn't a brick.
> > > 
> > > I've added NEW and documentation.
> > > 
> > > I'm still not sure how to test this adequately.
> > 
> > This version is OK when Eli is happy with the documentation.
> 
> I already reviewed it, didn't I?  If I missed it, please point me to
> the archives.

You did -- I think Dan might have missed your reply. I shall adjust per
your comments today and repost.

Thanks,
Volodya

 



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-03-15 15:10     ` Eli Zaretskii
@ 2008-04-02 13:02       ` Vladimir Prus
  2008-04-02 18:18         ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Vladimir Prus @ 2008-04-02 13:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: drow, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 2091 bytes --]

On Saturday 15 March 2008 18:09:38 Eli Zaretskii wrote:
> > From: Vladimir Prus <vladimir@codesourcery.com>
> > Date: Sat, 15 Mar 2008 13:12:14 +0300
> > Cc: gdb-patches@sources.redhat.com
> > 
> > I've added NEW and documentation.
> 
> Thanks.
> 
> > +set breakpoint always-inserted
> > +show breakpoint always-inserted
> > +  Keep breakpoints always inserted in the target.
> 
> This entry is not detailed enough to be useful.  Please keep in mind
> that GDB users are not necessarily aware of the fact that GDB removes
> the breakpoints whenever the inferior stops and reinserts them before
> resuming it.
> 
> > +By default, @value{GDBN} inserts breakpoints in the target only when
> > +resuming the target, and removes breakpoints whenever the target stop.
>                                                                     ^^^^
> "stops".
> 
> > +This behaviour guards against leaving breakpoints inserted in the
> > +target should gdb abrubptly disconnect, which is possible with remote
> > +targets.
> 
> Without explaining that a breakpoint replaces a portion of code with a
> breakpoint instruction, the reader will not understand what is
> dangerous about this.
> 
> >                      This bevaious can be controlled with the following
>                             ^^^^^^^^
> You mean "behavior", right?
> 
> > +This is the default behaviour.  All breakpoints, including newly added
>                        ^^^^^^^^^
> Please use the US spelling ("behavior"), for consistency.
> 
> > +@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.
> 
> It would be good to give the reader a hint when this behavior is
> useful.
> 
> Otherwise, okay with me.

I have clarified this a bit. Does the attached looks OK for you (I have omitted
the code bits since they were not changed).

- Volodya


 



[-- Attachment #2: always_inserted_docs.diff --]
[-- Type: text/x-diff, Size: 4766 bytes --]

commit 24adf67c1cec1bca65dc4bae7611e59733c9986c
Author: Vladimir Prus <vladimir@codesourcery.com>
Date:   Fri Nov 30 21:35:52 2007 +0300

    Keep breakpoints always inserted.
    
    	* 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.

diff --git a/gdb/NEWS b/gdb/NEWS
index 232e201..ff468d0 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -25,6 +25,12 @@ show exec-wrapper
 unset exec-wrapper
   Use a wrapper program to launch programs for debugging.
 
+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/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index db5cdd4..c73a6d9 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-04-02 13:02       ` Vladimir Prus
@ 2008-04-02 18:18         ` Eli Zaretskii
  0 siblings, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2008-04-02 18:18 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: drow, gdb-patches

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Wed, 2 Apr 2008 16:26:06 +0400
> Cc: drow@false.org,
>  gdb-patches@sources.redhat.com
> 
> I have clarified this a bit. Does the attached looks OK for you (I have omitted
> the code bits since they were not changed).

Yes, I'm happy now.  Thanks.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-03-17 22:21     ` Daniel Jacobowitz
  2008-03-18  4:15       ` Eli Zaretskii
@ 2008-04-03 18:10       ` Vladimir Prus
  2008-04-07 15:32         ` Daniel Jacobowitz
  1 sibling, 1 reply; 19+ messages in thread
From: Vladimir Prus @ 2008-04-03 18:10 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1802 bytes --]

On Tuesday 18 March 2008 01:20:41 Daniel Jacobowitz wrote:
> On Sat, Mar 15, 2008 at 01:12:14PM +0300, Vladimir Prus wrote:
> > > This patch is OK, if you will add the missing pieces: a NEWS entry and
> > > documentation for the new command, and a testcase that enables
> > > always-inserted and verifies that it isn't a brick.
> > 
> > I've added NEW and documentation.
> > 
> > I'm still not sure how to test this adequately.
> 
> This version is OK when Eli is happy with the documentation.
> 
> For a testcase, I agree that we need to run the whole testsuite in
> this mode to get full coverage.  But it's useful to have a single
> test that runs in this mode even if the rest of the testsuite
> doesn't, in case someone completely breaks it.  That's why the
> gdb.server tests run for native targets that build gdbserver,
> even when the rest of the testsuite uses "run".
> 
> Could you write a test that uses "set break always-inserted" and does
> something simple like running to a function and then continuing past
> it to another breakpoint?

How about the attached? It depends on my previous patch to introduce
some testsuite helper functions.

> > +	  /* 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.  */
> 
> Extra spacing here?

Yes, I've fixed this.

- Volodya

[-- Attachment #2: always_inserted_test.diff --]
[-- Type: text/x-diff, Size: 6033 bytes --]

commit 87e70184c2c39b198f857ae6367696c7d10ec3e6
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/testsuite/gdb.base/break-always.c b/gdb/testsuite/gdb.base/break-always.c
new file mode 100644
index 0000000..ad79c34
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-always.c
@@ -0,0 +1,33 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 1992, 1993, 1994, 1995, 1999, 2002, 2003, 2007, 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 8814e24..31e9feb 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 $" {

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-04-03 18:10       ` Vladimir Prus
@ 2008-04-07 15:32         ` Daniel Jacobowitz
  2008-04-09 23:45           ` Vladimir Prus
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel Jacobowitz @ 2008-04-07 15:32 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

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.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-04-07 15:32         ` Daniel Jacobowitz
@ 2008-04-09 23:45           ` Vladimir Prus
  2008-04-10  8:16             ` Luis Machado
  2008-04-17 16:50             ` Daniel Jacobowitz
  0 siblings, 2 replies; 19+ messages in thread
From: Vladimir Prus @ 2008-04-09 23:45 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 900 bytes --]

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


 


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: delta.diff --]
[-- Type: text/x-diff; name="delta.diff", Size: 4507 bytes --]

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);
     }


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch.diff --]
[-- Type: text/x-diff; name="patch.diff", Size: 41239 bytes --]

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;
 }
 \f
@@ -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;
+}
+
 \f
 /* 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 $" {


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-04-09 23:45           ` Vladimir Prus
@ 2008-04-10  8:16             ` Luis Machado
  2008-04-17 16:50             ` Daniel Jacobowitz
  1 sibling, 0 replies; 19+ messages in thread
From: Luis Machado @ 2008-04-10  8:16 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

On Thu, 2008-04-10 at 02:27 +0400, Vladimir Prus wrote:

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

No regressions for PPC as well.

-- 
Luis Machado
Software Engineer 
IBM Linux Technology Center


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-04-09 23:45           ` Vladimir Prus
  2008-04-10  8:16             ` Luis Machado
@ 2008-04-17 16:50             ` Daniel Jacobowitz
  2008-04-23 21:18               ` Pedro Alves
  1 sibling, 1 reply; 19+ messages in thread
From: Daniel Jacobowitz @ 2008-04-17 16:50 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

On Thu, Apr 10, 2008 at 02:27:00AM +0400, Vladimir Prus wrote:
> 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?

OK.  Luis, thanks for the extra testing.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-04-17 16:50             ` Daniel Jacobowitz
@ 2008-04-23 21:18               ` Pedro Alves
  2008-04-23 21:24                 ` Daniel Jacobowitz
  0 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2008-04-23 21:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Daniel Jacobowitz, Vladimir Prus

[-- Attachment #1: Type: text/plain, Size: 3253 bytes --]

> On Thu, Apr 10, 2008 at 02:27:00AM +0400, Vladimir Prus wrote:
> > 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?
>

Testing with this patch applied and with displaced stepping on top
revealed one problem.

One should be able to call insert_breakpoints several times
in succession and the effect should be as if only one call was made.

There is a bug in the patch where if there is more than
one breakpoint at the same address, the second insert_breakpoints
call will remove the breakpoint location from the target.

This happens because should_insert_location returns false if the
location is already inserted.  

 /* 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;
 }

On the case were we have two locations at the same address, when we
get to the loop the patch is touching looking for another location
at the same address, we'll fail to mark this location as to be kept,
and go on to remove the breakpoint from the target,

	  if (!keep)
	    if (remove_breakpoint (loc, mark_uninserted))
	      {

The attached patch inlines the tests we're really interested in
at this point (I'm not sure about the disposition test), and removes
the need for the (incomplete) hack of setting loc2->duplicate = 0
before calling should_insert_location.

This failed with the displaced stepping patch on top,
because with that patch there are now several code paths
that call insert_breakpoints without a remove_breakpoints
call in between.  You can also reproduce this easilly
by applying something like this:


---
 gdb/infrun.c |    2 ++
 1 file changed, 2 insertions(+)

Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c       2008-04-23 18:38:33.000000000 +0100
+++ src/gdb/infrun.c    2008-04-23 18:38:47.000000000 +0100
@@ -2921,6 +2921,8 @@ keep_going (struct execution_control_sta
          TRY_CATCH (e, RETURN_MASK_ERROR)
            {
              insert_breakpoints ();
+             insert_breakpoints ();
+             insert_breakpoints ();
            }
          if (e.reason < 0)
            {

The test case that made this visible was thread-specific.exp,
which inserts more than one breakpoint at the same location
(although marked for different threads).

The symptom is that the inferior will not stop at the
breakpoint.
Easilly tested in any test, by "b main; b main; run".

I've tested this patch on top of Vladimir's and with displaced
stepping on, on x86-pc-linux-gnu.  Luis tested it on PPC, where
it also fixed things.

P.S.:
Anyone object to adding debug output support to the
breakpoint module?  I've writen something similar twice
already, and I'd like to not have to a third time.  Something
like "set debug breakpoint 1", where it prints
"breakpoints inserted", "breakpoints removed", etc.

-- 
Pedro Alves

[-- Attachment #2: fix_displaced.diff --]
[-- Type: text/x-diff, Size: 1056 bytes --]

---
 gdb/breakpoint.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2008-04-23 18:26:53.000000000 +0100
+++ src/gdb/breakpoint.c	2008-04-23 18:28:43.000000000 +0100
@@ -7019,12 +7019,12 @@ update_global_location_list (void)
 	      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)
-		      {		  
+		    if (loc2 != loc
+			&& loc2->address == loc->address
+			&& breakpoint_enabled (loc2->owner)
+			&& loc2->enabled && !loc2->shlib_disabled
+			&& loc2->owner->disposition != disp_del_at_next_stop)
+		      {
 			loc2->inserted = 1;
 			loc2->target_info = loc->target_info;
 			keep = 1;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-04-23 21:18               ` Pedro Alves
@ 2008-04-23 21:24                 ` Daniel Jacobowitz
  2008-04-24 11:22                   ` Vladimir Prus
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel Jacobowitz @ 2008-04-23 21:24 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Vladimir Prus

On Wed, Apr 23, 2008 at 08:45:54PM +0100, Pedro Alves wrote:
> I've tested this patch on top of Vladimir's and with displaced
> stepping on, on x86-pc-linux-gnu.  Luis tested it on PPC, where
> it also fixed things.

OK when the original patch is committed.

> P.S.:
> Anyone object to adding debug output support to the
> breakpoint module?  I've writen something similar twice
> already, and I'd like to not have to a third time.  Something
> like "set debug breakpoint 1", where it prints
> "breakpoints inserted", "breakpoints removed", etc.

Sounds like a good idea to me.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-04-23 21:24                 ` Daniel Jacobowitz
@ 2008-04-24 11:22                   ` Vladimir Prus
  2008-04-24 12:10                     ` Vladimir Prus
  0 siblings, 1 reply; 19+ messages in thread
From: Vladimir Prus @ 2008-04-24 11:22 UTC (permalink / raw)
  To: gdb-patches

Daniel Jacobowitz wrote:

> On Wed, Apr 23, 2008 at 08:45:54PM +0100, Pedro Alves wrote:
>> I've tested this patch on top of Vladimir's and with displaced
>> stepping on, on x86-pc-linux-gnu.  Luis tested it on PPC, where
>> it also fixed things.
> 
> OK when the original patch is committed.

Well, I have an obvious adjustment to the original patch to fix this
issue in a more clear way -- my making should_insert_location not
return false for an already inserted location.

>> Anyone object to adding debug output support to the
>> breakpoint module?  I've writen something similar twice
>> already, and I'd like to not have to a third time.  Something
>> like "set debug breakpoint 1", where it prints
>> "breakpoints inserted", "breakpoints removed", etc.
> 
> Sounds like a good idea to me.

Likewise.

- Volodya



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA] Keep breakpoints always inserted.
  2008-04-24 11:22                   ` Vladimir Prus
@ 2008-04-24 12:10                     ` Vladimir Prus
  0 siblings, 0 replies; 19+ messages in thread
From: Vladimir Prus @ 2008-04-24 12:10 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 840 bytes --]

Vladimir Prus wrote:

> Daniel Jacobowitz wrote:
> 
>> On Wed, Apr 23, 2008 at 08:45:54PM +0100, Pedro Alves wrote:
>>> I've tested this patch on top of Vladimir's and with displaced
>>> stepping on, on x86-pc-linux-gnu.  Luis tested it on PPC, where
>>> it also fixed things.
>> 
>> OK when the original patch is committed.
> 
> Well, I have an obvious adjustment to the original patch to fix this
> issue in a more clear way -- my making should_insert_location not
> return false for an already inserted location.

I've checked in the below. I've tested this with default mode and
in always-inserted mode, but not in async mode or in async mode +
displaced patch. I'll do that now and should there be issues,
post them separately -- it seems more productive than having folks
send patches against not-yet-checked in code :-)

- Volodya



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: commit.diff --]
[-- Type: text/x-diff; name="commit.diff", Size: 29808 bytes --]

Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.9307
diff -u -p -r1.9307 ChangeLog
--- gdb/ChangeLog	23 Apr 2008 21:17:04 -0000	1.9307
+++ gdb/ChangeLog	24 Apr 2008 08:45:16 -0000
@@ -1,3 +1,9 @@
+2008-04-24  Vladimir Prus  <vladimir@codesourcery.com>
+
+	* breakpoint.c (print_one_breakpoint_location): In MI
+	mode, report the location string the breakpoint was
+	originally created with.
+
 2008-04-23  Maxim Grigoriev  <maxim2405@gmail.com>
 
 	* Makefile.in (xtensa-tdep.o): Update dependencies.
Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.310
diff -u -p -r1.310 breakpoint.c
--- gdb/breakpoint.c	18 Apr 2008 00:41:28 -0000	1.310
+++ gdb/breakpoint.c	24 Apr 2008 08:45:16 -0000
@@ -3696,6 +3696,15 @@ print_one_breakpoint_location (struct br
       print_command_lines (uiout, l, 4);
       do_cleanups (script_chain);
     }
+
+  if (ui_out_is_mi_like_p (uiout) && !part_of_multiple)
+    {
+      if (b->addr_string)
+	ui_out_field_string (uiout, "original-location", b->addr_string);
+      else if (b->exp_string)
+	ui_out_field_string (uiout, "original-location", b->exp_string);
+    }
+	
   do_cleanups (bkpt_chain);
   do_cleanups (old_chain);
 }
Index: gdb/testsuite/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
retrieving revision 1.1615
diff -u -p -r1.1615 ChangeLog
--- gdb/testsuite/ChangeLog	23 Apr 2008 12:21:50 -0000	1.1615
+++ gdb/testsuite/ChangeLog	24 Apr 2008 08:45:17 -0000
@@ -1,5 +1,24 @@
+2008-04-24  Vladimir Prus  <vladimir@codesourcery.com>
+
+	* lib/mi-support.exp (mi_runto_helper): Adjust
+	for the original-location field.
+	(mi_create_breakpoint, mi_list_breakpoints): New.
+	* gdb.mi/mi-break.exp: Adjust.
+	* gdb.mi/mi2-break.exp: Adjust.
+	* gdb.mi/mi-pending.exp: Adjust.
+	* gdb.mi/mi-simplerun.exp: Adjust.
+	* gdb.mi/mi2-simplerun.exp: Adjust.
+	* gdb.mi/mi-syn-frame.exp: Adjust.
+	* gdb.mi/mi2-syn-frame.exp: Adjust.
+	* gdb.mi/mi-until.exp: Adjust.
+	* gdb.mi/mi2-until.exp: Adjust.
+	* gdb.mi/mi-var-display.exp: Adjust.
+	* gdb.mi/mi2-var-display.exp: Adjust.
+	* gdb.mi/mi-watch.exp: Adjust.
+	* gdb.mi/mi2-watch.exp: Adjust.
+
 2008-04-23  Paolo Bonzini  <bonzini@gnu.org>
-		
+
         * aclocal.m4: Add override.m4.
         * configure: Regenerate.
 
@@ -13,7 +32,7 @@
 
 	* gdb.dwarf2/dw2-compressed.S, gdb.dwarf2/dw2-compressed.exp: New
 	files.
-	
+
 2008-04-18  Joel Brobecker  <brobecker@adacore.com>
 
 	* gdb.ada/atomic_enum: New test program.
Index: gdb/testsuite/gdb.mi/mi-break.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-break.exp,v
retrieving revision 1.16
diff -u -p -r1.16 mi-break.exp
--- gdb/testsuite/gdb.mi/mi-break.exp	27 Feb 2008 20:29:31 -0000	1.16
+++ gdb/testsuite/gdb.mi/mi-break.exp	24 Apr 2008 08:45:17 -0000
@@ -78,25 +78,21 @@ proc test_tbreak_creation_and_listing {}
     # -break-insert -t srcfile:$line_callee4_head
     # -break-list
 
-    mi_gdb_test "222-break-insert -t main" \
-             "222\\^done,bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"main\",file=\".*basics.c\",${fullname},line=\"$line_main_body\",times=\"0\"\}" \
+    mi_create_breakpoint "-t main" 1 del main ".*basics.c" $line_main_body $hex \
              "break-insert -t operation"
 
-    mi_gdb_test "333-break-insert -t basics.c:callee2" \
-            "333\\^done,bkpt=\{number=\"2\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"callee2\",file=\".*basics.c\",${fullname},line=\"$line_callee2_body\",times=\"0\"\}" \
+    mi_create_breakpoint "-t basics.c:callee2" 2 del callee2 ".*basics.c" $line_callee2_body $hex \
              "insert temp breakpoint at basics.c:callee2"
 
-    mi_gdb_test "444-break-insert -t basics.c:$line_callee3_head" \
-            "444\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"callee3\",file=\".*basics.c\",${fullname},line=\"$line_callee3_body\",times=\"0\"\}" \
+    mi_create_breakpoint "-t basics.c:$line_callee3_head" 3 del callee3 ".*basics.c" $line_callee3_body $hex \
              "insert temp breakpoint at basics.c:\$line_callee3_head"
 
     # Getting the quoting right is tricky.  That is "\"<file>\":$line_callee4_head"
-    mi_gdb_test "555-break-insert -t \"\\\"${srcfile}\\\":$line_callee4_head\"" \
-            "555\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",${fullname},line=\"$line_callee4_body\",times=\"0\"\}" \
+    mi_create_breakpoint "-t \"\\\"${srcfile}\\\":$line_callee4_head\"" 4 del callee4 ".*basics.c" $line_callee4_body $hex \
              "insert temp breakpoint at \"<fullfilename>\":\$line_callee4_head"
 
     mi_gdb_test "666-break-list" \
-     	    "666\\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\}.*colhdr=\"Type\".*colhdr=\"Disp\".*colhdr=\"Enb\".*colhdr=\"Address\".*colhdr=\"What\".*\\\],body=\\\[bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"main\",file=\".*basics.c\",${fullname},line=\"$line_main_body\",times=\"0\"\}.*\\\]\}" \
+     	    "666\\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\}.*colhdr=\"Type\".*colhdr=\"Disp\".*colhdr=\"Enb\".*colhdr=\"Address\".*colhdr=\"What\".*\\\],body=\\\[bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"main\",file=\".*basics.c\",${fullname},line=\"$line_main_body\",times=\"0\",original-location=\".*\"\}.*\\\]\}" \
                 "list of breakpoints"
 
     mi_gdb_test "777-break-delete" \
Index: gdb/testsuite/gdb.mi/mi-pending.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-pending.exp,v
retrieving revision 1.5
diff -u -p -r1.5 mi-pending.exp
--- gdb/testsuite/gdb.mi/mi-pending.exp	15 Apr 2008 14:33:54 -0000	1.5
+++ gdb/testsuite/gdb.mi/mi-pending.exp	24 Apr 2008 08:45:17 -0000
@@ -64,7 +64,7 @@ if [target_info exists gdb_stub] {
 
 # Set pending breakpoint via MI
 mi_gdb_test "-break-insert -f pendfunc1" \
-    ".*\\^done,bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"<PENDING>\",pending=\"pendfunc1\",times=\"0\"\}"\
+    ".*\\^done,bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"<PENDING>\",pending=\"pendfunc1\",times=\"0\",original-location=\"pendfunc1\"\}"\
     "MI pending breakpoint on pendfunc1"
 
 mi_run_cmd
Index: gdb/testsuite/gdb.mi/mi-simplerun.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-simplerun.exp,v
retrieving revision 1.20
diff -u -p -r1.20 mi-simplerun.exp
--- gdb/testsuite/gdb.mi/mi-simplerun.exp	15 Apr 2008 14:33:54 -0000	1.20
+++ gdb/testsuite/gdb.mi/mi-simplerun.exp	24 Apr 2008 08:45:17 -0000
@@ -68,24 +68,20 @@ proc test_breakpoints_creation_and_listi
     # -break-disable
     # -break-info
 
-    mi_gdb_test "200-break-insert main" \
-             "200\\^done,bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"main\",file=\".*basics.c\",line=\"$line_main_body\",times=\"0\"\}" \
+    mi_create_breakpoint "main" 1 keep main ".*basics.c" $line_main_body $hex \
              "break-insert operation"
 
-    mi_gdb_test "201-break-insert basics.c:callee2" \
-             "201\\^done,bkpt=\{number=\"2\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"callee2\",file=\".*basics.c\",line=\"$line_callee2_body\",times=\"0\"\}" \
+    mi_create_breakpoint "basics.c:callee2" 2 keep callee2 ".*basics.c" $line_callee2_body $hex \
              "insert breakpoint at basics.c:callee2"
 
-    mi_gdb_test "202-break-insert basics.c:$line_callee3_head" \
-             "202\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"callee3\",file=\".*basics.c\",line=\"$line_callee3_body\",times=\"0\"\}" \
+    mi_create_breakpoint "basics.c:$line_callee3_head" 3 keep callee3 ".*basics.c" $line_callee3_body $hex \
              "insert breakpoint at basics.c:\$line_callee3_head"
 
-    mi_gdb_test "203-break-insert \"\\\"${srcfile}\\\":$line_callee4_head\"" \
-             "203\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"$line_callee4_body\",times=\"0\"\}" \
+    mi_create_breakpoint "\"\\\"${srcfile}\\\":$line_callee4_head\"" 4 keep callee4 ".*basics.c" $line_callee4_body $hex \
              "insert breakpoint at \"<fullfilename>\":\$line_callee4_head"
 
     mi_gdb_test "204-break-list" \
-	    "204\\^done,BreakpointTable=\{.*,hdr=\\\[.*\\\],body=\\\[bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"main\",file=\".*basics.c\",line=\"$line_main_body\",times=\"0\"\},.*\}\\\]\}" \
+	    "204\\^done,BreakpointTable=\{.*,hdr=\\\[.*\\\],body=\\\[bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"main\",file=\".*basics.c\",line=\"$line_main_body\",times=\"0\",original-location=\".*\"\},.*\}\\\]\}" \
                 "list of breakpoints"
 
     mi_gdb_test "205-break-disable 2 3 4" \
Index: gdb/testsuite/gdb.mi/mi-syn-frame.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-syn-frame.exp,v
retrieving revision 1.15
diff -u -p -r1.15 mi-syn-frame.exp
--- gdb/testsuite/gdb.mi/mi-syn-frame.exp	5 Apr 2008 17:12:46 -0000	1.15
+++ gdb/testsuite/gdb.mi/mi-syn-frame.exp	24 Apr 2008 08:45:17 -0000
@@ -39,10 +39,8 @@ mi_gdb_exit
 mi_gdb_start
 mi_run_to_main
 
-mi_gdb_test "400-break-insert foo" \
-  "400\\^done,bkpt=\{number=\"2\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"foo\",file=\".*mi-syn-frame.c\",line=\"$decimal\",times=\"0\"\}" \
-  "insert breakpoint foo"
-
+mi_create_breakpoint "foo" 2 keep foo ".*mi-syn-frame.c" $decimal $hex \
+    "insert breakpoint foo"
 
 #
 # Call foo() by hand, where we'll hit a breakpoint.
@@ -73,9 +71,8 @@ mi_gdb_test "404-stack-list-frames 0 0" 
 # Call have_a_very_merry_interrupt() which will eventually raise a signal
 # that's caught by handler() which calls subroutine().
 
-mi_gdb_test "405-break-insert subroutine" \
-  "405\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"subroutine\",file=\".*mi-syn-frame.c\",line=\"$decimal\",times=\"0\"\}" \
-  "insert breakpoint subroutine"
+mi_create_breakpoint "subroutine" 3 keep subroutine ".*mi-syn-frame.c" $decimal $hex \
+    "insert breakpoint subroutine"
 
 mi_gdb_test "406-data-evaluate-expression have_a_very_merry_interrupt()" \
   "406\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" \
Index: gdb/testsuite/gdb.mi/mi-until.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-until.exp,v
retrieving revision 1.17
diff -u -p -r1.17 mi-until.exp
--- gdb/testsuite/gdb.mi/mi-until.exp	15 Apr 2008 14:33:54 -0000	1.17
+++ gdb/testsuite/gdb.mi/mi-until.exp	24 Apr 2008 08:45:17 -0000
@@ -50,8 +50,7 @@ proc test_running_to_foo {} {
     global mi_gdb_prompt
     global hex
 
-    mi_gdb_test "200-break-insert 10" \
-             "200\\^done,bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"foo\",file=\".*until.c\",line=\"10\",times=\"0\"\}" \
+    mi_create_breakpoint "10" 1 "keep" foo ".*until.c" 10 ".*" \
              "break-insert operation"
 
     mi_run_cmd
Index: gdb/testsuite/gdb.mi/mi-var-display.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-var-display.exp,v
retrieving revision 1.28
diff -u -p -r1.28 mi-var-display.exp
--- gdb/testsuite/gdb.mi/mi-var-display.exp	15 Apr 2008 14:33:55 -0000	1.28
+++ gdb/testsuite/gdb.mi/mi-var-display.exp	24 Apr 2008 08:45:17 -0000
@@ -42,9 +42,8 @@ mi_gdb_load ${binfile}
 
 set line_dct_end [gdb_get_line_number "{int a = 0;}"]
 
-mi_gdb_test "200-break-insert $srcfile:$line_dct_end" \
-	"200\\^done,bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"do_children_tests\",file=\".*var-cmd.c\",line=\"$line_dct_end\",times=\"0\"\}" \
-	"break-insert operation"
+mi_create_breakpoint "$srcfile:$line_dct_end" 1 keep do_children_tests ".*var-cmd.c" $line_dct_end $hex \
+    "break-insert operation"
 
 mi_run_cmd
 mi_expect_stop "breakpoint-hit" "do_children_tests" "" ".*var-cmd.c" \
@@ -371,9 +370,8 @@ mi_gdb_test "-var-delete weird" \
 
 set line_dst_incr_a_2 [gdb_get_line_number "incr_a(2);"]
 
-mi_gdb_test "200-break-insert $line_dst_incr_a_2" \
-	"200\\^done,bkpt=\{number=\"2\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"do_special_tests\",file=\".*var-cmd.c\",line=\"$line_dst_incr_a_2\",times=\"0\"\}" \
-	"break-insert operation"
+mi_create_breakpoint "$line_dst_incr_a_2" 2 keep do_special_tests ".*var-cmd.c" $line_dst_incr_a_2 $hex \
+	"break-insert operation 2"
 
 mi_execute_to "exec-continue" "breakpoint-hit" "do_special_tests" "" \
     ".*var-cmd.c" $line_dst_incr_a_2 { "" "disp=\"keep\"" } \
Index: gdb/testsuite/gdb.mi/mi-watch.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-watch.exp,v
retrieving revision 1.21
diff -u -p -r1.21 mi-watch.exp
--- gdb/testsuite/gdb.mi/mi-watch.exp	5 Apr 2008 17:12:46 -0000	1.21
+++ gdb/testsuite/gdb.mi/mi-watch.exp	24 Apr 2008 08:45:17 -0000
@@ -59,7 +59,7 @@ proc test_watchpoint_creation_and_listin
              "break-watch operation"
 
     mi_gdb_test "222-break-list" \
-	    "222\\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\}.*colhdr=\"Type\".*colhdr=\"Disp\".*colhdr=\"Enb\".*colhdr=\"Address\".*colhdr=\"What\".*\\\],body=\\\[bkpt=\{number=\"2\",type=\".*watchpoint\",disp=\"keep\",enabled=\"y\",addr=\"\",what=\"C\",times=\"0\"\}\\\]\}" \
+	    "222\\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\}.*colhdr=\"Type\".*colhdr=\"Disp\".*colhdr=\"Enb\".*colhdr=\"Address\".*colhdr=\"What\".*\\\],body=\\\[bkpt=\{number=\"2\",type=\".*watchpoint\",disp=\"keep\",enabled=\"y\",addr=\"\",what=\"C\",times=\"0\",original-location=\"C\"\}\\\]\}" \
                 "list of watchpoints"
 
 }
Index: gdb/testsuite/gdb.mi/mi2-break.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi2-break.exp,v
retrieving revision 1.9
diff -u -p -r1.9 mi2-break.exp
--- gdb/testsuite/gdb.mi/mi2-break.exp	27 Feb 2008 20:29:31 -0000	1.9
+++ gdb/testsuite/gdb.mi/mi2-break.exp	24 Apr 2008 08:45:17 -0000
@@ -79,25 +79,21 @@ proc test_tbreak_creation_and_listing {}
     # -break-insert -t srcfile:$line_callee4_head
     # -break-list
 
-    mi_gdb_test "222-break-insert -t main" \
-             "222\\^done,bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"main\",file=\".*basics.c\",${fullname},line=\"$line_main_body\",times=\"0\"\}" \
+    mi_create_breakpoint "-t main" 1 del main ".*basics.c" $line_main_body $hex \
              "break-insert -t operation"
 
-    mi_gdb_test "333-break-insert -t basics.c:callee2" \
-             "333\\^done,bkpt=\{number=\"2\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"callee2\",file=\".*basics.c\",${fullname},line=\"$line_callee2_body\",times=\"0\"\}" \
+    mi_create_breakpoint "-t basics.c:callee2" 2 del callee2 ".*basics.c" $line_callee2_body $hex \
              "insert temp breakpoint at basics.c:callee2"
 
-    mi_gdb_test "444-break-insert -t basics.c:$line_callee3_head" \
-             "444\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"callee3\",file=\".*basics.c\",${fullname},line=\"$line_callee3_body\",times=\"0\"\}" \
+    mi_create_breakpoint "-t basics.c:$line_callee3_head" 3 del callee3 ".*basics.c" $line_callee3_body $hex \
              "insert temp breakpoint at basics.c:\$line_callee3_head"
 
     # Getting the quoting right is tricky.  That is "\"<file>\":$line_callee4_head"
-    mi_gdb_test "555-break-insert -t \"\\\"${srcfile}\\\":$line_callee4_head\"" \
-             "555\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",${fullname},line=\"$line_callee4_body\",times=\"0\"\}" \
+    mi_create_breakpoint "-t \"\\\"${srcfile}\\\":$line_callee4_head\"" 4 del callee4 ".*basics.c" $line_callee4_body $hex \
              "insert temp breakpoint at \"<fullfilename>\":\$line_callee4_head"
 
     mi_gdb_test "666-break-list" \
-	    "666\\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\}.*colhdr=\"Type\".*colhdr=\"Disp\".*colhdr=\"Enb\".*colhdr=\"Address\".*colhdr=\"What\".*\\\],body=\\\[bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"main\",file=\".*basics.c\",${fullname},line=\"$line_main_body\",times=\"0\"\}.*\\\]\}" \
+	    "666\\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\}.*colhdr=\"Type\".*colhdr=\"Disp\".*colhdr=\"Enb\".*colhdr=\"Address\".*colhdr=\"What\".*\\\],body=\\\[bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"main\",file=\".*basics.c\",${fullname},line=\"$line_main_body\",times=\"0\",original-location=\".*\"\}.*\\\]\}" \
                 "list of breakpoints"
 
     mi_gdb_test "777-break-delete" \
Index: gdb/testsuite/gdb.mi/mi2-simplerun.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi2-simplerun.exp,v
retrieving revision 1.11
diff -u -p -r1.11 mi2-simplerun.exp
--- gdb/testsuite/gdb.mi/mi2-simplerun.exp	15 Apr 2008 14:33:55 -0000	1.11
+++ gdb/testsuite/gdb.mi/mi2-simplerun.exp	24 Apr 2008 08:45:17 -0000
@@ -68,24 +68,20 @@ proc test_breakpoints_creation_and_listi
     # -break-disable
     # -break-info
 
-    mi_gdb_test "200-break-insert main" \
-             "200\\^done,bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"main\",file=\".*basics.c\",line=\"$line_main_body\",times=\"0\"\}" \
+    mi_create_breakpoint "main" 1 keep main ".*basics.c" $line_main_body $hex \
              "break-insert operation"
 
-    mi_gdb_test "201-break-insert basics.c:callee2" \
-             "201\\^done,bkpt=\{number=\"2\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"callee2\",file=\".*basics.c\",line=\"$line_callee2_body\",times=\"0\"\}" \
+    mi_create_breakpoint "basics.c:callee2" 2 keep callee2 ".*basics.c" $line_callee2_body $hex \
              "insert breakpoint at basics.c:callee2"
 
-    mi_gdb_test "202-break-insert basics.c:$line_callee3_head" \
-             "202\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"callee3\",file=\".*basics.c\",line=\"$line_callee3_body\",times=\"0\"\}" \
+    mi_create_breakpoint "basics.c:$line_callee3_head" 3 keep callee3 ".*basics.c" $line_callee3_body $hex \
              "insert breakpoint at basics.c:\$line_callee3_head"
 
-    mi_gdb_test "203-break-insert \"\\\"${srcfile}\\\":$line_callee4_head\"" \
-             "203\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"$line_callee4_body\",times=\"0\"\}" \
+    mi_create_breakpoint "\"\\\"${srcfile}\\\":$line_callee4_head\"" 4 keep callee4 ".*basics.c" $line_callee4_body $hex \
              "insert breakpoint at \"<fullfilename>\":\$line_callee4_head"
 
     mi_gdb_test "204-break-list" \
-	    "204\\^done,BreakpointTable=\{.*,hdr=\\\[.*\\\],body=\\\[bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"main\",file=\".*basics.c\",line=\"$line_main_body\",times=\"0\"\},.*\}\\\]\}" \
+	    "204\\^done,BreakpointTable=\{.*,hdr=\\\[.*\\\],body=\\\[bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"main\",file=\".*basics.c\",line=\"$line_main_body\",times=\"0\",original-location=\".*\"\},.*\}\\\]\}" \
                 "list of breakpoints"
 
     mi_gdb_test "205-break-disable 2 3 4" \
Index: gdb/testsuite/gdb.mi/mi2-syn-frame.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi2-syn-frame.exp,v
retrieving revision 1.11
diff -u -p -r1.11 mi2-syn-frame.exp
--- gdb/testsuite/gdb.mi/mi2-syn-frame.exp	4 Apr 2008 21:59:25 -0000	1.11
+++ gdb/testsuite/gdb.mi/mi2-syn-frame.exp	24 Apr 2008 08:45:17 -0000
@@ -41,10 +41,8 @@ mi_gdb_exit
 mi_gdb_start
 mi_run_to_main
 
-mi_gdb_test "400-break-insert foo" \
-  "400\\^done,bkpt=\{number=\"2\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"foo\",file=\".*mi-syn-frame.c\",line=\"$decimal\",times=\"0\"\}" \
-  "insert breakpoint foo"
-
+mi_create_breakpoint "foo" 2 keep foo ".*mi-syn-frame.c" $decimal $hex \
+    "insert breakpoint foo"
 
 #
 # Call foo() by hand, where we'll hit a breakpoint.
@@ -77,9 +75,8 @@ mi_gdb_test "404-stack-list-frames 0 0" 
 # Call have_a_very_merry_interrupt() which will eventually raise a signal
 # that's caught by handler() which calls subroutine().
 
-mi_gdb_test "405-break-insert subroutine" \
-  "405\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"subroutine\",file=\".*mi-syn-frame.c\",line=\"$decimal\",times=\"0\"\}" \
-  "insert breakpoint subroutine"
+mi_create_breakpoint "subroutine" 3 keep subroutine ".*mi-syn-frame.c" $decimal $hex \
+    "insert breakpoint subroutine"
 
 mi_gdb_test "406-data-evaluate-expression have_a_very_merry_interrupt()" \
   "406\\^error,msg=\"The program being debugged stopped while in a function called from GDB.\\\\nWhen the function \\(have_a_very_merry_interrupt\\) is done executing, GDB will silently\\\\nstop \\(instead of continuing to evaluate the expression containing\\\\nthe function call\\).\"" \
Index: gdb/testsuite/gdb.mi/mi2-until.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi2-until.exp,v
retrieving revision 1.11
diff -u -p -r1.11 mi2-until.exp
--- gdb/testsuite/gdb.mi/mi2-until.exp	15 Apr 2008 14:33:55 -0000	1.11
+++ gdb/testsuite/gdb.mi/mi2-until.exp	24 Apr 2008 08:45:17 -0000
@@ -51,8 +51,7 @@ proc test_running_to_foo {} {
     global mi_gdb_prompt
     global hex
 
-    mi_gdb_test "200-break-insert 10" \
-             "200\\^done,bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"foo\",file=\".*until.c\",line=\"10\",times=\"0\"\}" \
+    mi_create_breakpoint "10" 1 "keep" foo ".*until.c" 10 ".*" \
              "break-insert operation"
 
     mi_run_cmd
Index: gdb/testsuite/gdb.mi/mi2-var-display.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi2-var-display.exp,v
retrieving revision 1.21
diff -u -p -r1.21 mi2-var-display.exp
--- gdb/testsuite/gdb.mi/mi2-var-display.exp	15 Apr 2008 14:33:55 -0000	1.21
+++ gdb/testsuite/gdb.mi/mi2-var-display.exp	24 Apr 2008 08:45:17 -0000
@@ -42,9 +42,8 @@ mi_gdb_load ${binfile}
 
 set line_dct_end [gdb_get_line_number "{int a = 0;}"]
 
-mi_gdb_test "200-break-insert $srcfile:$line_dct_end" \
-	"200\\^done,bkpt=\{number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"do_children_tests\",file=\".*var-cmd.c\",line=\"$line_dct_end\",times=\"0\"\}" \
-	"break-insert operation"
+mi_create_breakpoint "$srcfile:$line_dct_end" 1 keep do_children_tests ".*var-cmd.c" $line_dct_end $hex \
+    "break-insert operation"
 
 mi_run_cmd
 mi_expect_stop "breakpoint-hit" "do_children_tests" "" ".*var-cmd.c" \
@@ -370,9 +369,8 @@ mi_gdb_test "-var-delete weird" \
 
 set line_dst_incr_a_2 [gdb_get_line_number "incr_a(2);"]
 
-mi_gdb_test "200-break-insert $line_dst_incr_a_2" \
-	"200\\^done,bkpt=\{number=\"2\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"do_special_tests\",file=\".*var-cmd.c\",line=\"$line_dst_incr_a_2\",times=\"0\"\}" \
-	"break-insert operation"
+mi_create_breakpoint "$line_dst_incr_a_2" 2 keep do_special_tests ".*var-cmd.c" $line_dst_incr_a_2 $hex \
+	"break-insert operation 2"
 
 mi_execute_to "exec-continue" "breakpoint-hit" "do_special_tests" "" \
     ".*var-cmd.c" $line_dst_incr_a_2 { "" "disp=\"keep\"" } \
Index: gdb/testsuite/gdb.mi/mi2-watch.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi2-watch.exp,v
retrieving revision 1.12
diff -u -p -r1.12 mi2-watch.exp
--- gdb/testsuite/gdb.mi/mi2-watch.exp	5 Apr 2008 17:12:46 -0000	1.12
+++ gdb/testsuite/gdb.mi/mi2-watch.exp	24 Apr 2008 08:45:17 -0000
@@ -59,7 +59,7 @@ proc test_watchpoint_creation_and_listin
              "break-watch operation"
 
     mi_gdb_test "222-break-list" \
-	    "222\\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\}.*colhdr=\"Type\".*colhdr=\"Disp\".*colhdr=\"Enb\".*colhdr=\"Address\".*colhdr=\"What\".*\\\],body=\\\[bkpt=\{number=\"2\",type=\".*watchpoint\",disp=\"keep\",enabled=\"y\",addr=\"\",what=\"C\",times=\"0\"\}\\\]\}" \
+	    "222\\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\}.*colhdr=\"Type\".*colhdr=\"Disp\".*colhdr=\"Enb\".*colhdr=\"Address\".*colhdr=\"What\".*\\\],body=\\\[bkpt=\{number=\"2\",type=\".*watchpoint\",disp=\"keep\",enabled=\"y\",addr=\"\",what=\"C\",times=\"0\",original-location=\"C\"\}\\\]\}" \
                 "list of watchpoints"
 
 }
Index: gdb/testsuite/lib/mi-support.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/mi-support.exp,v
retrieving revision 1.56
diff -u -p -r1.56 mi-support.exp
--- gdb/testsuite/lib/mi-support.exp	15 Apr 2008 14:33:55 -0000	1.56
+++ gdb/testsuite/lib/mi-support.exp	24 Apr 2008 08:45:17 -0000
@@ -875,7 +875,7 @@ proc mi_runto_helper {func run_or_contin
 
   set test "mi runto $func"
   mi_gdb_test "200-break-insert -t $func" \
-    "200\\^done,bkpt=\{number=\"\[0-9\]+\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"$func\(\\\(.*\\\)\)?\",file=\".*\",line=\"\[0-9\]*\",times=\"0\"\}" \
+    "200\\^done,bkpt=\{number=\"\[0-9\]+\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"$func\(\\\(.*\\\)\)?\",file=\".*\",line=\"\[0-9\]*\",times=\"0\",original-location=\".*\"\}" \
     "breakpoint at $func"
 
   if {![regexp {number="[0-9]+"} $expect_out(buffer) str]
@@ -1059,6 +1059,40 @@ proc mi0_continue_to { bkptno func args 
 	"$func" "$args" "$file" "$line" "" "$test"
 }
 
+# Creates a breakpoint and checks the reported fields are as expected
+proc mi_create_breakpoint { location number disp func file line address test } {
+    verbose -log "Expecting: 222\\^done,bkpt=\{number=\"$number\",type=\"breakpoint\",disp=\"$disp\",enabled=\"y\",addr=\"$address\",func=\"$func\",file=\"$file\",fullname=\".*\",line=\"$line\",times=\"0\",original-location=\".*\"\}"
+    mi_gdb_test "222-break-insert $location" \
+        "222\\^done,bkpt=\{number=\"$number\",type=\"breakpoint\",disp=\"$disp\",enabled=\"y\",addr=\"$address\",func=\"$func\",file=\"$file\",fullname=\".*\",line=\"$line\",times=\"0\",original-location=\".*\"\}" \
+        $test
+}
+
+proc mi_list_breakpoints { expected test } {
+    set fullname ".*"
+
+    set body ""
+    set first 1
+
+    foreach item $children {
+        if {$first == 0} {
+            set body "$body,"
+        }
+    set number disp func file line address
+        set number [lindex $item 0]
+        set disp [lindex $item 1]
+        set func [lindex $item 2]
+        set line [lindex $item 3]
+        set address [lindex $item 4]
+        set body "$body,bkpt=\{number=\"$number\",type=\"breakpoint\",disp=\"$disp\",enabled=\"y\",addr=\"$address\",func=\"$func\",file=\"$file\",${fullname},line=\"$line\",times=\"0\",original-location=\".*\"\}"
+        set first 0
+    }
+
+    verbose -log "Expecint: 666\\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\}.*colhdr=\"Type\".*colhdr=\"Disp\".*colhdr=\"Enb\".*colhdr=\"Address\".*colhdr=\"What\".*\\\],body=\\\[$body\\\]\}" \
+    mi_gdb_test "666-break-list" \
+        "666\\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\}.*colhdr=\"Type\".*colhdr=\"Disp\".*colhdr=\"Enb\".*colhdr=\"Address\".*colhdr=\"What\".*\\\],body=\\\[$body\\\]\}" \
+        $test
+}
+
 # Creates varobj named NAME for EXPRESSION.
 # Name cannot be "-".
 proc mi_create_varobj { name expression testname } {


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2008-04-24 10:24 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-28 14:18 [RFA] Keep breakpoints always inserted Vladimir Prus
2008-03-10 21:09 ` Daniel Jacobowitz
2008-03-14 20:22   ` Vladimir Prus
2008-03-15 10:13   ` Vladimir Prus
2008-03-15 15:10     ` Eli Zaretskii
2008-04-02 13:02       ` Vladimir Prus
2008-04-02 18:18         ` Eli Zaretskii
2008-03-17 22:21     ` Daniel Jacobowitz
2008-03-18  4:15       ` Eli Zaretskii
2008-03-18  6:06         ` Vladimir Prus
2008-04-03 18:10       ` Vladimir Prus
2008-04-07 15:32         ` Daniel Jacobowitz
2008-04-09 23:45           ` Vladimir Prus
2008-04-10  8:16             ` Luis Machado
2008-04-17 16:50             ` Daniel Jacobowitz
2008-04-23 21:18               ` Pedro Alves
2008-04-23 21:24                 ` Daniel Jacobowitz
2008-04-24 11:22                   ` Vladimir Prus
2008-04-24 12:10                     ` Vladimir Prus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox