Use multiple locations for hardware watchpoints.

	This eliminates the need to traverse value chain, doing
	various checks, in three different places.

        * breakpoint.h (struct bp_location): New fields
        lengths and watchpoint_type.
        (struct breakpoint): Remove the val_chain field.
        * breakpoint.c (is_hardware_watchpoint): New.
        (free_valchain): Remove.
        (update_watchpoint): New.
        (insert_bp_location): For hardware watchpoint, just
        directly insert it.
        (insert_breakpoints): Call update_watchpoint_locations
        on all watchpoints.  If we have failed to insert
        any location of a hardware watchpoint, remove all inserted
        locations.
        (remove_breakpoint): For hardware watchpoints, directly
        remove location.
        (watchpoints_triggered): Iterate over locations.
        (bpstat_stop_status): Use only first location of
        a resource watchpoint.
        (delete_breakpoint): Don't call free_valchain.
        (print_one_breakpoint): Don't print all
        locations for watchpoints.
        (breakpoint_re_set_one): Use update_watchpoint for
        watchpoints.

7 files changed, 478 insertions, 252 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index dda1c03..120144d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,34 @@
 2008-01-29  Vladimir Prus  <vladimir@codesourcery.com>
 
+	Use multiple locations for hardware watchpoints.
+	This eliminates the need to traverse value chain, doing
+	various checks, in three different places.
+
+        * breakpoint.h (struct bp_location): New fields
+        lengths and watchpoint_type.
+        (struct breakpoint): Remove the val_chain field.
+        * breakpoint.c (is_hardware_watchpoint): New.
+        (free_valchain): Remove.
+        (update_watchpoint): New.
+        (insert_bp_location): For hardware watchpoint, just
+        directly insert it.
+        (insert_breakpoints): Call update_watchpoint_locations
+        on all watchpoints.  If we have failed to insert
+        any location of a hardware watchpoint, remove all inserted
+        locations.
+        (remove_breakpoint): For hardware watchpoints, directly
+        remove location.
+        (watchpoints_triggered): Iterate over locations.
+        (bpstat_stop_status): Use only first location of
+        a resource watchpoint.
+        (delete_breakpoint): Don't call free_valchain.
+        (print_one_breakpoint): Don't print all
+        locations for watchpoints.
+        (breakpoint_re_set_one): Use update_watchpoint for
+        watchpoints.
+
+2008-01-29  Vladimir Prus  <vladimir@codesourcery.com>
+
 	Don't reset watchpoint block on solib load.
 
         * breakpoint.c (insert_bp_location): For watchpoints,
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index ecc2478..0bed4ef 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -200,6 +200,15 @@ static void free_bp_location (struct bp_location *loc);
 
 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 int
+is_hardware_watchpoint (struct breakpoint *bpt);
+
 /* Prototypes for exported functions. */
 
 /* If FALSE, gdb will not use hardware support for watchpoints, even
@@ -808,24 +817,182 @@ insert_catchpoint (struct ui_out *uo, void *args)
     }
 }
 
-/* Helper routine: free the value chain for a breakpoint (watchpoint).  */
+static int
+is_hardware_watchpoint (struct breakpoint *bpt)
+{
+  return (bpt->type == bp_hardware_watchpoint
+	  || bpt->type == bp_read_watchpoint
+	  || bpt->type == bp_access_watchpoint);
+}
 
+/* Assuming that B is a hardware breakpoint:
+   - Reparse watchpoint expression, is REPARSE is non-zero
+   - Evaluate expression and store the result in B->val
+   - Update the list of values that must be watched in B->loc.
+
+   If the watchpoint is disabled, do nothing.  If this is
+   local watchpoint that is out of scope, delete it.  */
 static void
-free_valchain (struct bp_location *b)
+update_watchpoint (struct breakpoint *b, int reparse)
 {
-  struct value *v;
-  struct value *n;
+  int within_current_scope;
+  struct value *mark = value_mark ();
+  struct frame_id saved_frame_id;
+  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;
+    }
+  b->loc = NULL;
 
-  /* Free the saved value chain.  We will construct a new one
-     the next time the watchpoint is inserted.  */
-  for (v = b->owner->val_chain; v; v = n)
+  if (b->disposition == disp_del_at_next_stop)
+    return;
+ 
+  /* Save the current frame's ID so we can restore it after
+     evaluating the watchpoint expression on its own frame.  */
+  /* FIXME drow/2003-09-09: It would be nice if evaluate_expression
+     took a frame parameter, so that we didn't have to change the
+     selected frame.  */
+  saved_frame_id = get_frame_id (get_selected_frame (NULL));
+
+  /* Determine if the watchpoint is within scope.  */
+  if (b->exp_valid_block == NULL)
+    within_current_scope = 1;
+  else
     {
-      n = value_next (v);
-      value_free (v);
+      struct frame_info *fi;
+      fi = frame_find_by_id (b->watchpoint_frame);
+      within_current_scope = (fi != NULL);
+      if (within_current_scope)
+	select_frame (fi);
+    }
+
+  if (within_current_scope && reparse)
+    {
+      char *s;
+      if (b->exp)
+	{
+	  xfree (b->exp);
+	  b->exp = NULL;
+	}
+      s = b->exp_string;
+      b->exp = parse_exp_1 (&s, b->exp_valid_block, 0);
+      /* If the meaning of expression itself changed, the old value is
+	 no longer relevant.  We don't want to report a watchpoint hit
+	 to the user when the old value and the new value may actually
+	 be completely different objects.  */
+      value_free (b->val);
+      b->val = NULL;      
     }
-  b->owner->val_chain = NULL;
+  
+
+  /* If we failed to parse the expression, for example because
+     it refers to a global variable in a not-yet-loaded shared library,
+     don't try to insert watchpoint.  We don't automatically delete
+     such watchpoint, though, since failure to parse expression
+     is different from out-of-scope watchpoint.  */
+  if (within_current_scope && b->exp)
+    {
+      struct value *v, *next;
+
+      /* Evaluate the expression and make sure it's not lazy, so that
+	 after target stops again, we have a non-lazy previous value
+	 to compare with. Also, making the value non-lazy will fetch
+	 intermediate values as needed, which we use to decide which
+	 addresses to watch.
+
+	 The value returned by evaluate_expression is stored in b->val.
+	 In addition, we look at all values which were created
+	 during evaluation, and set watchoints at addresses as needed.
+	 Those values are explicitly deleted here.  */
+      v = evaluate_expression (b->exp);
+      /* Avoid setting b->val if it's already set.  The meaning of
+	 b->val is 'the last value' user saw, and we should update
+	 it only if we reported that last value to user.  As it
+	 happens, the code that reports it updates b->val directly.  */
+      if (b->val == NULL)
+	b->val = v;
+      value_contents (v);
+      value_release_to_mark (mark);
+
+      /* Look at each value on the value chain.  */
+      for (; v; v = next)
+	{
+	  /* If it's a memory location, and GDB actually needed
+	     its contents to evaluate the expression, then we
+	     must watch it.  */
+	  if (VALUE_LVAL (v) == lval_memory
+	      && ! value_lazy (v))
+	    {
+	      struct type *vtype = check_typedef (value_type (v));
+
+	      /* We only watch structs and arrays if user asked
+		 for it explicitly, never if they just happen to
+		 appear in the middle of some value chain.  */
+	      if (v == b->val
+		  || (TYPE_CODE (vtype) != TYPE_CODE_STRUCT
+		      && TYPE_CODE (vtype) != TYPE_CODE_ARRAY))
+		{
+		  CORE_ADDR addr;
+		  int len, type;
+		  struct bp_location *loc, **tmp;
+
+		  addr = VALUE_ADDRESS (v) + value_offset (v);
+		  len = TYPE_LENGTH (value_type (v));
+		  type = hw_write;
+		  if (b->type == bp_read_watchpoint)
+		    type = hw_read;
+		  else if (b->type == bp_access_watchpoint)
+		    type = hw_access;
+		  
+		  loc = allocate_bp_location (b, bp_hardware_watchpoint);
+		  for (tmp = &(b->loc); *tmp != NULL; tmp = &((*tmp)->next))
+		    ;
+		  *tmp = loc;
+		  loc->address = addr;
+		  loc->length = len;
+		  loc->watchpoint_type = type;
+		}
+	    }
+
+	  next = value_next (v);
+	  if (v != b->val)
+	    value_free (v);
+	}
+
+      if (reparse && b->cond_string != NULL)
+	{
+	  char *s = b->cond_string;
+	  if (b->loc->cond)
+	    {
+	      xfree (b->loc->cond);
+	      b->loc->cond = NULL;
+	    }
+	  b->loc->cond = parse_exp_1 (&s, b->exp_valid_block, 0);
+	}
+    }
+  else if (!within_current_scope)
+    {
+      printf_filtered (_("\
+Hardware watchpoint %d deleted because the program has left the block \n\
+in which its expression is valid.\n"),
+		       b->number);
+      if (b->related_breakpoint)
+	b->related_breakpoint->disposition = disp_del_at_next_stop;
+      b->disposition = disp_del_at_next_stop;
+    }
+
+  /* Restore the selected frame.  */
+  select_frame (frame_find_by_id (saved_frame_id));
 }
 
+
 /* 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.
@@ -1016,136 +1183,10 @@ Note: automatically using hardware breakpoints for read-only addresses.\n"));
 	      watchpoints.  It's not clear that it's necessary... */
 	   && bpt->owner->disposition != disp_del_at_next_stop)
     {
-      /* FIXME drow/2003-09-08: This code sets multiple hardware watchpoints
-	 based on the expression.  Ideally this should happen at a higher level,
-	 and there should be one bp_location for each computed address we
-	 must watch.  As soon as a many-to-one mapping is available I'll
-	 convert this.  */
-
-      int within_current_scope;
-      struct value *mark = value_mark ();
-      struct value *v;
-      struct frame_id saved_frame_id;
-
-      /* Save the current frame's ID so we can restore it after
-	 evaluating the watchpoint expression on its own frame.  */
-      /* FIXME drow/2003-09-09: It would be nice if evaluate_expression
-	 took a frame parameter, so that we didn't have to change the
-	 selected frame.  */
-      saved_frame_id = get_frame_id (get_selected_frame (NULL));
-
-      /* Determine if the watchpoint is within scope.  */
-      if (bpt->owner->exp_valid_block == NULL)
-	within_current_scope = 1;
-      else
-	{
-	  struct frame_info *fi;
-	  fi = frame_find_by_id (bpt->owner->watchpoint_frame);
-	  within_current_scope = (fi != NULL);
-	  if (within_current_scope)
-	    select_frame (fi);
-	}
-
-      if (within_current_scope)
-	{
-	  free_valchain (bpt);
-
-	  /* Evaluate the expression and cut the chain of values
-	     produced off from the value chain.
-
-	     Make sure the value returned isn't lazy; we use
-	     laziness to determine what memory GDB actually needed
-	     in order to compute the value of the expression.  */
-	  v = evaluate_expression (bpt->owner->exp);
-	  value_contents (v);
-	  value_release_to_mark (mark);
-
-	  bpt->owner->val_chain = v;
-	  bpt->inserted = 1;
-
-	  /* Look at each value on the value chain.  */
-	  for (; v; v = value_next (v))
-	    {
-	      /* If it's a memory location, and GDB actually needed
-		 its contents to evaluate the expression, then we
-		 must watch it.  */
-	      if (VALUE_LVAL (v) == lval_memory
-		  && ! value_lazy (v))
-		{
-		  struct type *vtype = check_typedef (value_type (v));
-
-		  /* We only watch structs and arrays if user asked
-		     for it explicitly, never if they just happen to
-		     appear in the middle of some value chain.  */
-		  if (v == bpt->owner->val_chain
-		      || (TYPE_CODE (vtype) != TYPE_CODE_STRUCT
-			  && TYPE_CODE (vtype) != TYPE_CODE_ARRAY))
-		    {
-		      CORE_ADDR addr;
-		      int len, type;
-
-		      addr = VALUE_ADDRESS (v) + value_offset (v);
-		      len = TYPE_LENGTH (value_type (v));
-		      type = hw_write;
-		      if (bpt->owner->type == bp_read_watchpoint)
-			type = hw_read;
-		      else if (bpt->owner->type == bp_access_watchpoint)
-			type = hw_access;
-
-		      val = target_insert_watchpoint (addr, len, type);
-		      if (val == -1)
-			{
-			  /* Don't exit the loop, try to insert
-			     every value on the value chain.  That's
-			     because we will be removing all the
-			     watches below, and removing a
-			     watchpoint we didn't insert could have
-			     adverse effects.  */
-			  bpt->inserted = 0;
-			}
-		      val = 0;
-		    }
-		}
-	    }
-
-	  if (bpt->owner->cond_string != NULL)
-	    {
-	      char *s = bpt->owner->cond_string;
-	      if (bpt->cond)
-		{
-		  xfree (bpt->cond);
-		  bpt->cond = NULL;
-		}
-	      bpt->cond = parse_exp_1 (&s, bpt->owner->exp_valid_block, 0);
-	    }
-	      
-	  /* Failure to insert a watchpoint on any memory value in the
-	     value chain brings us here.  */
-	  if (!bpt->inserted)
-	    {
-	      remove_breakpoint (bpt, mark_uninserted);
-	      *hw_breakpoint_error = 1;
-	      fprintf_unfiltered (tmp_error_stream,
-				  "Could not insert hardware watchpoint %d.\n", 
-				  bpt->owner->number);
-	      val = -1;
-	    }               
-	}
-      else
-	{
-	  printf_filtered (_("\
-Hardware watchpoint %d deleted because the program has left the block \n\
-in which its expression is valid.\n"),
-			   bpt->owner->number);
-	  if (bpt->owner->related_breakpoint)
-	    bpt->owner->related_breakpoint->disposition = disp_del_at_next_stop;
-	  bpt->owner->disposition = disp_del_at_next_stop;
-	}
-
-      /* Restore the selected frame.  */
-      select_frame (frame_find_by_id (saved_frame_id));
-
-      return val;
+      val = target_insert_watchpoint (bpt->address, 
+				      bpt->length,
+				      bpt->watchpoint_type);
+      bpt->inserted = (val != -1);
     }
 
   else if (bpt->owner->type == bp_catch_fork
@@ -1178,6 +1219,7 @@ in which its expression is valid.\n"),
 void
 insert_breakpoints (void)
 {
+  struct breakpoint *bpt;
   struct bp_location *b, *temp;
   int error = 0;
   int val = 0;
@@ -1192,6 +1234,10 @@ insert_breakpoints (void)
      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))
@@ -1203,19 +1249,6 @@ insert_breakpoints (void)
 	  && !valid_thread_id (b->owner->thread))
 	continue;
 
-      /* FIXME drow/2003-10-07: This code should be pushed elsewhere when
-	 hardware watchpoints are split into multiple loc breakpoints.  */
-      if ((b->loc_type == bp_loc_hardware_watchpoint
-	   || b->owner->type == bp_watchpoint) && !b->owner->val)
-	{
-	  struct value *val;
-	  val = evaluate_expression (b->owner->exp);
-	  release_value (val);
-	  if (value_lazy (val))
-	    value_fetch_lazy (val);
-	  b->owner->val = val;
-	}
-
       val = insert_bp_location (b, tmp_error_stream,
 				    &disabled_breaks, &process_warning,
 				    &hw_breakpoint_error);
@@ -1223,6 +1256,39 @@ insert_breakpoints (void)
 	error = val;
     }
 
+  /* If we failed to insert all locations of a watchpoint,
+     remove them, as half-inserted watchpoint is of limited use.  */
+  ALL_BREAKPOINTS (bpt)  
+    {
+      int some_failed = 0;
+      struct bp_location *loc;
+
+      if (!is_hardware_watchpoint (bpt))
+	continue;
+
+      if (bpt->enable_state != bp_enabled)
+	continue;
+      
+      for (loc = bpt->loc; loc; loc = loc->next)
+	if (!loc->inserted)
+	  {
+	    some_failed = 1;
+	    break;
+	  }
+      if (some_failed)
+	{
+	  for (loc = bpt->loc; loc; loc = loc->next)
+	    if (loc->inserted)
+	      remove_breakpoint (loc, mark_uninserted);
+
+	  hw_breakpoint_error = 1;
+	  fprintf_unfiltered (tmp_error_stream,
+			      "Could not insert hardware watchpoint %d.\n", 
+			      bpt->number);
+	  error = -1;
+	}
+    }
+
   if (error)
     {
       /* If a hardware breakpoint or watchpoint was inserted, add a
@@ -1508,46 +1574,15 @@ remove_breakpoint (struct bp_location *b, insertion_state_t is)
 	return val;
       b->inserted = (is == mark_inserted);
     }
-  else if (b->loc_type == bp_loc_hardware_watchpoint
-	   && breakpoint_enabled (b->owner)
-	   && !b->duplicate)
+  else if (b->loc_type == bp_loc_hardware_watchpoint)
     {
       struct value *v;
       struct value *n;
 
       b->inserted = (is == mark_inserted);
-      /* Walk down the saved value chain.  */
-      for (v = b->owner->val_chain; v; v = value_next (v))
-	{
-	  /* For each memory reference remove the watchpoint
-	     at that address.  */
-	  if (VALUE_LVAL (v) == lval_memory
-	      && ! value_lazy (v))
-	    {
-	      struct type *vtype = check_typedef (value_type (v));
-
-	      if (v == b->owner->val_chain
-		  || (TYPE_CODE (vtype) != TYPE_CODE_STRUCT
-		      && TYPE_CODE (vtype) != TYPE_CODE_ARRAY))
-		{
-		  CORE_ADDR addr;
-		  int len, type;
+      val = target_remove_watchpoint (b->address, b->length, 
+				      b->watchpoint_type);
 
-		  addr = VALUE_ADDRESS (v) + value_offset (v);
-		  len = TYPE_LENGTH (value_type (v));
-		  type   = hw_write;
-		  if (b->owner->type == bp_read_watchpoint)
-		    type = hw_read;
-		  else if (b->owner->type == bp_access_watchpoint)
-		    type = hw_access;
-
-		  val = target_remove_watchpoint (addr, len, type);
-		  if (val == -1)
-		    b->inserted = 1;
-		  val = 0;
-		}
-	    }
-	}
       /* Failure to remove any of the hardware watchpoints comes here.  */
       if ((is == mark_uninserted) && (b->inserted))
 	warning (_("Could not remove hardware watchpoint %d."),
@@ -2451,33 +2486,19 @@ watchpoints_triggered (struct target_waitstatus *ws)
 	|| b->type == bp_read_watchpoint
 	|| b->type == bp_access_watchpoint)
       {
+	struct bp_location *loc;
 	struct value *v;
 
 	b->watchpoint_triggered = watch_triggered_no;
-	for (v = b->val_chain; v; v = value_next (v))
-	  {
-	    if (VALUE_LVAL (v) == lval_memory && ! value_lazy (v))
-	      {
-		struct type *vtype = check_typedef (value_type (v));
-
-		if (v == b->val_chain
-		    || (TYPE_CODE (vtype) != TYPE_CODE_STRUCT
-			&& TYPE_CODE (vtype) != TYPE_CODE_ARRAY))
-		  {
-		    CORE_ADDR vaddr;
-
-		    vaddr = VALUE_ADDRESS (v) + value_offset (v);
-		    /* Exact match not required.  Within range is
-		       sufficient.  */
-		    if (addr >= vaddr
-			&& addr < vaddr + TYPE_LENGTH (value_type (v)))
-		      {
-			b->watchpoint_triggered = watch_triggered_yes;
-			break;
-		      }
-		  }
-	      }
-	  }
+	for (loc = b->loc; loc; loc = loc->next)
+	  /* Exact match not required.  Within range is
+	     sufficient.  */
+	  if (addr >= loc->address
+	      && addr < loc->address + loc->length)
+	    {
+	      b->watchpoint_triggered = watch_triggered_yes;
+	      break;
+	    }
       }
 
   return 1;
@@ -2716,6 +2737,15 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid)
 	&& !inferior_has_execd (PIDGET (inferior_ptid), &b->exec_pathname))
       continue;
 
+    /* For hardware watchpoints, we look only at the first location.
+       The watchpoint_check function will work on entire expression,
+       not the individual locations.  For read watchopints, the
+       watchpoints_triggered function have checked all locations
+       alrea
+     */
+    if (b->type == bp_hardware_watchpoint && bl != b->loc)
+      continue;
+
     /* Come here if it's a watchpoint, or if the break address matches */
 
     bs = bpstat_alloc (bl, bs);	/* Alloc a bpstat to explain stop */
@@ -2909,6 +2939,10 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid)
 	      || bs->breakpoint_at->owner->type == bp_read_watchpoint
 	      || bs->breakpoint_at->owner->type == bp_access_watchpoint))
 	{
+	  /* remove/insert can invalidate bs->breakpoint_at, if this
+	     location is no longer used by the watchpoint.  Prevent
+	     further code from trying to use it.  */
+	  bs->breakpoint_at = NULL;
 	  remove_breakpoints ();
 	  insert_breakpoints ();
 	  break;
@@ -3629,10 +3663,14 @@ print_one_breakpoint (struct breakpoint *b,
 	 disabled, we print it as if it had
 	 several locations, since otherwise it's hard to
 	 represent "breakpoint enabled, location disabled"
-	 situation.  */	 
+	 situation.  
+	 Note that while hardware watchpoints have
+	 several locations internally, that's no a property
+	 exposed to user.  */
       if (b->loc 
+	  && !is_hardware_watchpoint (b)
 	  && (b->loc->next || !b->loc->enabled)
-	  && !ui_out_is_mi_like_p (uiout))
+	  && !ui_out_is_mi_like_p (uiout)) 
 	{
 	  struct bp_location *loc;
 	  int n = 1;
@@ -6829,9 +6867,7 @@ delete_breakpoint (struct breakpoint *bpt)
     {
       if (loc->inserted)
 	remove_breakpoint (loc, mark_inserted);
-
-      free_valchain (loc);
-
+      
       if (loc->cond)
 	xfree (loc->cond);
 
@@ -7265,39 +7301,7 @@ breakpoint_re_set_one (void *bint)
 	 
 	 Don't do anything about disabled watchpoints, since they will
 	 be reevaluated again when enabled.  */
-
-      if (!breakpoint_enabled (b))
-	break;
-
-      if (b->exp_valid_block == NULL
-	  || frame_find_by_id (b->watchpoint_frame) != NULL)
-	{
-	  if (b->exp)
-	    {
-	      xfree (b->exp);
-	      b->exp = NULL;
-	    }
-	  s = b->exp_string;
-	  b->exp = parse_exp_1 (&s, b->exp_valid_block, 0);
-
-	  /* Since we reparsed expression, we need to update the
-	     value.  I'm not aware of any way a single solib load or unload
-	     can change a valid value into different valid value, but:
-	     - even if the value is no longer valid, we have to record
-	     this fact, so that when it becomes valid we reports this
-	     as value change
-	     - unloaded followed by load can change the value for sure.
-
-   	     We set value to NULL, and insert_breakpoints will 
-	     update the value.  */
-	  if (b->val)
-	    value_free (b->val);
-	  b->val = NULL;
-
-	  /* Loading of new shared library change the meaning of
-	     watchpoint condition.  However, insert_bp_location will
-	     recompute watchpoint condition anyway, nothing to do here.  */
-	}
+      update_watchpoint (b, 1 /* reparse */);
       break;
       /* We needn't really do anything to reset these, since the mask
          that requests them is unaffected by e.g., new libraries being
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 91667ab..7b72e19 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -273,6 +273,12 @@ struct bp_location
      bp_loc_other.  */
   CORE_ADDR address;
 
+  /* For hardware watchpoints, the size of data ad ADDRESS being watches.  */
+  int length;
+
+  /* Type of hardware watchpoint. */
+  enum target_hw_bp_type watchpoint_type;
+
   /* For any breakpoint type with an address, this is the BFD section
      associated with the address.  Used primarily for overlay debugging.  */
   asection *section;
@@ -388,9 +394,6 @@ struct breakpoint
     /* Value of the watchpoint the last time we checked it.  */
     struct value *val;
 
-    /* Holds the value chain for a hardware watchpoint expression.  */
-    struct value *val_chain;
-
     /* Holds the address of the related watchpoint_scope breakpoint
        when using watchpoints on local variables (might the concept
        of a related breakpoint be useful elsewhere, if not just call
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index f3858ed..a5d5920 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2008-01-29  Vladimir Prus  <vladimir@codesourcery.com>
+
+	* gdb.base/watchpoint-solib.exp: New.
+        * gdb.base/watchpoint-solib.c: New.
+        * gdb.base/watchpoint-solib-shr.c: New.
+
 2008-01-29  Pierre Muller  <muller@ics.u-strasbg.fr>
 
 	* gdb.base/gdb1056.exp: Add unsigned integer test.
diff --git a/gdb/testsuite/gdb.base/watchpoint-solib-shr.c b/gdb/testsuite/gdb.base/watchpoint-solib-shr.c
new file mode 100644
index 0000000..699f559
--- /dev/null
+++ b/gdb/testsuite/gdb.base/watchpoint-solib-shr.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004, 2007 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/>. */
+
+#include <stdio.h>
+
+int g = 0;
+
+void foo (int i)
+{
+  g = i;
+}
diff --git a/gdb/testsuite/gdb.base/watchpoint-solib.c b/gdb/testsuite/gdb.base/watchpoint-solib.c
new file mode 100644
index 0000000..b316024
--- /dev/null
+++ b/gdb/testsuite/gdb.base/watchpoint-solib.c
@@ -0,0 +1,68 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004, 2007 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/>. */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#ifdef __WIN32__
+#include <windows.h>
+#define dlopen(name, mode) LoadLibrary (TEXT (name))
+#ifdef _WIN32_WCE
+# define dlsym(handle, func) GetProcAddress (handle, TEXT (func))
+#else
+# define dlsym(handle, func) GetProcAddress (handle, func)
+#endif
+#define dlclose(handle) FreeLibrary (handle)
+#define dlerror() "error %d occurred", GetLastError ()
+#else
+#include <dlfcn.h>
+#endif
+
+
+void open_shlib ()
+{
+  void *handle;
+  void (*foo) (int);
+
+  handle = dlopen (SHLIB_NAME, RTLD_LAZY);
+  
+  if (!handle)
+    {
+      fprintf (stderr, dlerror ());
+      exit (1);
+    }
+
+  foo = (void (*)(int))dlsym (handle, "foo");
+
+  if (!foo)
+    {
+      fprintf (stderr, dlerror ());
+      exit (1);
+    }
+
+  foo (1); // call to foo
+  foo (2);
+
+  dlclose (handle);
+}
+
+
+int main()
+{
+  open_shlib ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/watchpoint-solib.exp b/gdb/testsuite/gdb.base/watchpoint-solib.exp
new file mode 100644
index 0000000..f883f91
--- /dev/null
+++ b/gdb/testsuite/gdb.base/watchpoint-solib.exp
@@ -0,0 +1,91 @@
+#   Copyright 2007 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/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+#
+# test running programs
+#
+set prms_id 0
+set bug_id 0
+
+if {[skip_shlib_tests]} {
+    return 0
+}
+
+# TODO: Use LoadLibrary on this target instead of dlopen.
+if {[istarget arm*-*-symbianelf*]} {
+    return 0
+}
+
+set testfile "watchpoint-solib"
+set libfile "watchpoint-solib-shr"
+set libname "${libfile}.sl"
+set libsrcfile ${libfile}.c
+set srcfile $srcdir/$subdir/$testfile.c
+set binfile $objdir/$subdir/$testfile
+set shlibdir ${objdir}/${subdir}
+set libsrc  $srcdir/$subdir/$libfile.c
+set lib_sl  $objdir/$subdir/$libname
+
+if [get_compiler_info ${binfile}] {
+    return -1
+}
+
+set lib_opts debug
+set exec_opts [list debug shlib_load additional_flags=-DSHLIB_NAME\=\"${libname}\"]
+
+if { [gdb_compile_shlib $libsrc $lib_sl $lib_opts] != ""
+     || [gdb_compile $srcfile $binfile executable $exec_opts] != ""} {
+    untested "Couldn't compile $libsrc or $srcfile."
+    return -1
+}
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+gdb_load_shlibs $lib_sl
+
+if [target_info exists gdb_stub] {
+    gdb_step_for_stub;
+}
+
+runto_main
+
+# Test that if we set a watchpoint on a global variable
+# in a explicitly loaded shared library, and then
+# re-run the application, gdb does not crash.
+gdb_test_multiple "break foo" "set pending breakpoint" {
+     -re ".*Make breakpoint pending.*y or \\\[n\\\]. $" {
+	    gdb_test "y" "Breakpoint.*foo.*pending." "set pending breakpoint"
+     }
+}
+
+gdb_test "continue" ".*Breakpoint 2.*foo.*" "continue to foo"
+gdb_test "watch g" "Hardware watchpoint 3: g" "set watchpoint on g"
+gdb_test "continue" ".*New value = 1.*" "continue to watchpoint hit"
+rerun_to_main
+gdb_test "continue" ".*Breakpoint 2.*foo.*" "continue to foo again"
+gdb_test "continue" ".*New value = 1.*" "continue to watchpoint hit again"
+
+
+
+
+