This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[patch not to be accepted] Keep watchpoints always-inserted [Re: [patch 1/4] Fix hw watchpoints: [x86*] repeated rwatch output]


On Fri, 02 Oct 2009 02:06:31 +0200, Joel Brobecker wrote:
> you clear this flag when the watchpoint gets inserted.  But does this work
> in breakpoint always-inserted on mode?

This patch implements watchpoints always-inserted when `set breakpoint
always-inserted'.  It has no regressions on
{x86_64,x86_64-m32,i686-fedora11-linux-gnu}.

This is not submitted for an approval but just as a playground.

It does break the patch (in `set breakpoint always-inserted' mode):
	[patch 1/4] Fix hw watchpoints: [x86*] repeated rwatch output]

`set breakpoint always-inserted' has a meaning mostly only for the non-stop
mode I think, filed for it now the following Bug also demonstrating why
`set breakpoint always-inserted' is unrelated to hardware watchpoints:
PR threads/10729: non-stop && hw-watchpoint: Couldn't write debug register: No such process.

More is in the new comment at update_watchpoint() in my other mail.


Thanks,
Jan


	Keep watchpoints always inserted for `set breakpoint always-inserted'.
	* breakpoint.c (update_watchpoint): Removed unused varibles loc and tmp.
	New variables b_loc_saved, loc_chain, loc_chain_next, loc_chain_next,
	loc1 and loc2.  New pass trying to keep old B_>LOC if it stays the same.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -909,12 +909,14 @@ update_watchpoint (struct breakpoint *b, int reparse)
 {
   int within_current_scope;
   struct frame_id saved_frame_id;
-  struct bp_location *loc;
+  struct bp_location *b_loc_saved;
   bpstat bs;
 
-  /* We don't free locations.  They are stored in bp_location_chain and
-     update_global_locations will eventually delete them and remove
+  /* We don't free locations.  They are now also stored in bp_location_chain
+     and if they will no longer stay linked from BREAKPOINT_CHAIN after this
+     function update_global_locations will eventually delete them and remove
      breakpoints if needed.  */
+  b_loc_saved = b->loc;
   b->loc = NULL;
 
   if (b->disposition == disp_del_at_next_stop)
@@ -966,6 +968,8 @@ update_watchpoint (struct breakpoint *b, int reparse)
   if (within_current_scope && b->exp)
     {
       struct value *val_chain, *v, *result, *next;
+      struct bp_location *loc_chain = NULL, **loc_chain_next = &loc_chain;
+      struct bp_location *loc1, *loc2;
 
       fetch_watchpoint_value (b->exp, &v, &result, &val_chain);
 
@@ -1026,7 +1030,7 @@ update_watchpoint (struct breakpoint *b, int reparse)
 		{
 		  CORE_ADDR addr;
 		  int len, type;
-		  struct bp_location *loc, **tmp;
+		  struct bp_location *loc;
 
 		  addr = value_address (v);
 		  len = TYPE_LENGTH (value_type (v));
@@ -1037,9 +1041,9 @@ update_watchpoint (struct breakpoint *b, int reparse)
 		    type = hw_access;
 		  
 		  loc = allocate_bp_location (b);
-		  for (tmp = &(b->loc); *tmp != NULL; tmp = &((*tmp)->next))
-		    ;
-		  *tmp = loc;
+		  *loc_chain_next = loc;
+		  loc_chain_next = &loc->next;
+
 		  loc->gdbarch = get_type_arch (value_type (v));
 		  loc->address = addr;
 		  loc->length = len;
@@ -1052,6 +1056,50 @@ update_watchpoint (struct breakpoint *b, int reparse)
 	    value_free (v);
 	}
 
+      /* Compare the old and new location list of B and keep the former items
+	 if they did not change to prevent needless temporary remove/insert by
+	 update_global_locations set in ALWAYS_INSERTED_MODE.  */
+
+      loc1 = b_loc_saved;
+      loc2 = loc_chain;
+      while (loc1 && loc2)
+	{
+	  if (!(loc1->watchpoint_type == loc2->watchpoint_type
+	        && loc1->gdbarch == loc2->gdbarch
+	        && loc1->address == loc2->address
+	        && loc1->length == loc2->length))
+	    break;
+
+	  loc1 = loc1->next;
+	  loc2 = loc2->next;
+	}
+
+      /* Does the new chain match the old one?  */
+
+      if (loc1 == NULL && loc2 == NULL)
+	{
+	  /* Free the newly created entries still not linked anywhere else.  */
+
+	  while (loc_chain)
+	    {
+	      loc2 = loc_chain->next;
+	      xfree (loc_chain);
+	      loc_chain = loc2;
+	    }
+
+	  /* Restore the old chain which stays still valid.  */
+
+	  b->loc = b_loc_saved;
+	}
+      else
+	{
+	  /* Use the new chain as it has some changes, the old chain will be
+	     freed by update_global_locations as being linked from
+	     BP_LOCATION_CHAIN.  */
+
+	  b->loc = loc_chain;
+	}
+
       /* We just regenerated the list of breakpoint locations.
          The new location does not have its condition field set to anything
          and therefore, we must always reparse the cond_string, independently


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]