Fix regression in always-inserted breakpoints mode.

[Pedro Alves <pedro at codesourcery dot com>]

I forward ported to mainline a patch that makes
always-inserted mode work with watchpoint locations too,
and surprisingly, GDB was still removing/inserting watchpoints
at every step/event.  The original patch worked correctly, but
it predated the update_global_location_list rework to qsort
the locations.

I traced it down to the use of bp_location_compare in
update_global_location_list.  All else being equal,
bp_location_compare compares the input pointers.
Often, after a remove/insert sequence, we reach the
bp_location_compare call line shown in the patch below,

       while (locp < bp_location + bp_location_count
	     && bp_location_compare (*locp, old_loc) < 0)
 	locp++;

with `*locp' and `old_loc' having the same address, but
bp_location_compare would still return < 0, due to the
fallback pointer comparision.  This made it so that
locp++ statement was done one time too much.  The
`found_object' check would fail, and GDB would uninsert
a location from the target, that shouldn't be removed.

There's for loop a bit below, that starts iterating on
`locp', and that would misses a few locations, due to the
spurious increment mentioned above.  Here's the loop
in question:

 		  for (loc2p = locp;
		       loc2p < bp_location + bp_location_count
		       && breakpoint_address_match ((*loc2p)->pspace->aspace,
						    (*loc2p)->address,
						    old_loc->pspace->aspace,
						    old_loc->address);
 		       loc2p++)

There's another problem with this for loop.  The
breakpoint_address_match is too strict when debugging
more than one inferior.  Even if breakpoint_address_match
fails due to address space mismatch, there could be other
locations with the same address following loc2p.

Here's a fix for all this.  Tested on x86-64-linux, native
and gdbserver, no regressions.

Jan, could you double check?

-- 
Pedro Alves

2009-11-20  Pedro Alves  <pedro@codesourcery.com>

	* breakpoint.c (update_global_location_list): Fix duplicate
	locations detection.

---
 gdb/breakpoint.c |   51 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 19 deletions(-)

Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2009-11-20 00:38:12.000000000 +0000
+++ src/gdb/breakpoint.c	2009-11-20 01:02:18.000000000 +0000
@@ -8249,10 +8249,11 @@ update_global_location_list (int should_
        old_locp++)
     {
       struct bp_location *old_loc = *old_locp;
+      struct bp_location **loc2p;
 
       /* Tells if 'old_loc' is found amoung the new locations.  If not, we
 	 have to free it.  */
-      int found_object;
+      int found_object = 0;
       /* Tells if the location should remain inserted in the target.  */
       int keep_in_target = 0;
       int removed = 0;
@@ -8260,9 +8261,20 @@ update_global_location_list (int should_
       /* Skip LOCP entries which will definitely never be needed.  Stop either
 	 at or being the one matching OLD_LOC.  */
       while (locp < bp_location + bp_location_count
-	     && bp_location_compare (*locp, old_loc) < 0)
+	     && (*locp)->address < old_loc->address)
 	locp++;
-      found_object = locp < bp_location + bp_location_count && *locp == old_loc;
+
+      for (loc2p = locp;
+	   (loc2p < bp_location + bp_location_count
+	    && (*loc2p)->address == old_loc->address);
+	   loc2p++)
+	{
+	  if (*loc2p == old_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 
@@ -8288,27 +8300,28 @@ update_global_location_list (int should_
 
 	      if (breakpoint_address_is_meaningful (old_loc->owner))
 		{
-		  struct bp_location **loc2p;
-
 		  for (loc2p = locp;
-		       loc2p < bp_location + bp_location_count
-		       && breakpoint_address_match ((*loc2p)->pspace->aspace,
-						    (*loc2p)->address,
-						    old_loc->pspace->aspace,
-						    old_loc->address);
+		       (loc2p < bp_location + bp_location_count
+			&& (*loc2p)->address == old_loc->address);
 		       loc2p++)
 		    {
 		      struct bp_location *loc2 = *loc2p;
 
-		      /* For the sake of should_be_inserted.
-			 Duplicates check below will fix up this later.  */
-		      loc2->duplicate = 0;
-		      if (loc2 != old_loc && should_be_inserted (loc2))
-			{		  
-			  loc2->inserted = 1;
-			  loc2->target_info = old_loc->target_info;
-			  keep_in_target = 1;
-			  break;
+		      if (breakpoint_address_match (loc2->pspace->aspace,
+						    loc2->address,
+						    old_loc->pspace->aspace,
+						    old_loc->address))
+			{
+			  /* For the sake of should_be_inserted.
+			     Duplicates check below will fix up this later.  */
+			  loc2->duplicate = 0;
+			  if (loc2 != old_loc && should_be_inserted (loc2))
+			    {
+			      loc2->inserted = 1;
+			      loc2->target_info = old_loc->target_info;
+			      keep_in_target = 1;
+			      break;
+			    }
 			}
 		    }
 		}