2007-09-16 Daniel Jacobowitz <dan@codesourcery.com>

	    Jeff Johnston  <jjohnstn@redhat.com>

	* breakpoint.c (watchpoints_triggered): New.
	(bpstat_stop_status): Remove STOPPED_BY_WATCHPOINT argument.
	Check watchpoint_triggered instead.  Combine handling for software
	and hardware watchpoints.  Do not use target_stopped_data_address
	here.  Always check a watchpoint if its scope breakpoint triggers.
	Do not stop for thread or overlay events.  Improve check for
	triggered watchpoints without a value change.
	(watch_command_1): Insert the scope breakpoint first.  Link the
	scope breakpoint to the watchpoint.
	* breakpoint.h (enum watchpoint_triggered): New.
	(struct breakpoint): Add watchpoint_triggered.
	(bpstat_stop_status): Update prototype.
	(watchpoints_triggered): Declare.
	* infrun.c (enum infwait_status): Add infwait_step_watch_state.
	(stepped_after_stopped_by_watchpoint): Delete.
	(handle_inferior_event): Make stepped_after_stopped_by_watchpoint
	local.  Handle infwait_step_watch_state.  Update calls to
	bpstat_stop_status.  Use watchpoints_triggered to check
	watchpoints.
	* remote.c (stepped_after_stopped_by_watchpoint): Remove extern.
	(remote_stopped_data_address): Do not check it.

	* gdb.texinfo (Setting Watchpoints): Adjust warning text about
	multi-threaded watchpoints.
	* gdbint.texinfo (Watchpoints): Describe how watchpoints are
	checked.  Describe sticky notification.  Expand description
	of steppable and continuable watchpoints.
	(Watchpoints and Threads): New subsection.

	* gdb.threads/watchthreads.c (thread_function): Sleep between
	iterations.
	* gdb.threads/watchthreads.exp: Allow two watchpoints to trigger
	at once for S/390.  Generate matching fails and passes.

1 files changed, 196 insertions, 138 deletions

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 0d1cb9e..e4fdb33 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2537,6 +2537,83 @@ bpstat_alloc (struct bp_location *bl, bpstat cbs /* Current "bs" value */ )
   return bs;
 }
 
+/* The target has stopped with waitstatus WS.  Check if any hardware
+   watchpoints have triggered, according to the target.  */
+
+int
+watchpoints_triggered (struct target_waitstatus *ws)
+{
+  int stopped_by_watchpoint = STOPPED_BY_WATCHPOINT (*ws);
+  CORE_ADDR addr;
+  struct breakpoint *b;
+
+  if (!stopped_by_watchpoint)
+    {
+      /* We were not stopped by a watchpoint.  Mark all watchpoints
+	 as not triggered.  */
+      ALL_BREAKPOINTS (b)
+	if (b->type == bp_hardware_watchpoint
+	    || b->type == bp_read_watchpoint
+	    || b->type == bp_access_watchpoint)
+	  b->watchpoint_triggered = watch_triggered_no;
+
+      return 0;
+    }
+
+  if (!target_stopped_data_address (&current_target, &addr))
+    {
+      /* We were stopped by a watchpoint, but we don't know where.
+	 Mark all watchpoints as unknown.  */
+      ALL_BREAKPOINTS (b)
+	if (b->type == bp_hardware_watchpoint
+	    || b->type == bp_read_watchpoint
+	    || b->type == bp_access_watchpoint)
+	  b->watchpoint_triggered = watch_triggered_unknown;
+
+      return stopped_by_watchpoint;
+    }
+
+  /* The target could report the data address.  Mark watchpoints
+     affected by this data address as triggered, and all others as not
+     triggered.  */
+
+  ALL_BREAKPOINTS (b)
+    if (b->type == bp_hardware_watchpoint
+	|| b->type == bp_read_watchpoint
+	|| b->type == bp_access_watchpoint)
+      {
+	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;
+		      }
+		  }
+	      }
+	  }
+      }
+
+  return 1;
+}
+
 /* Possible return values for watchpoint_check (this can't be an enum
    because of check_errors).  */
 /* The watchpoint has been deleted.  */
@@ -2655,11 +2732,9 @@ which its expression is valid.\n");
 }
 
 /* Get a bpstat associated with having just stopped at address
-   BP_ADDR in thread PTID.  STOPPED_BY_WATCHPOINT is 1 if the
-   target thinks we stopped due to a hardware watchpoint, 0 if we
-   know we did not trigger a hardware watchpoint, and -1 if we do not know.  */
+   BP_ADDR in thread PTID.
 
-/* Determine whether we stopped at a breakpoint, etc, or whether we
+   Determine whether we stopped at a breakpoint, etc, or whether we
    don't understand this stop.  Result is a chain of bpstat's such that:
 
    if we don't understand the stop, the result is a null pointer.
@@ -2674,7 +2749,7 @@ which its expression is valid.\n");
    commands, FIXME??? fields.  */
 
 bpstat
-bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid, int stopped_by_watchpoint)
+bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid)
 {
   struct breakpoint *b = NULL;
   struct bp_location *bl;
@@ -2712,16 +2787,17 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid, int stopped_by_watchpoint)
 	  continue;
       }
 
-    /* Continuable hardware watchpoints are treated as non-existent if the 
-       reason we stopped wasn't a hardware watchpoint (we didn't stop on 
-       some data address).  Otherwise gdb won't stop on a break instruction 
-       in the code (not from a breakpoint) when a hardware watchpoint has 
-       been defined.  */
+    /* Continuable hardware watchpoints are treated as non-existent if the
+       reason we stopped wasn't a hardware watchpoint (we didn't stop on
+       some data address).  Otherwise gdb won't stop on a break instruction
+       in the code (not from a breakpoint) when a hardware watchpoint has
+       been defined.  Also skip watchpoints which we know did not trigger
+       (did not match the data address).  */
 
     if ((b->type == bp_hardware_watchpoint
 	 || b->type == bp_read_watchpoint
 	 || b->type == bp_access_watchpoint)
-	&& !stopped_by_watchpoint)
+	&& b->watchpoint_triggered == watch_triggered_no)
       continue;
 
     if (b->type == bp_hardware_breakpoint)
@@ -2787,82 +2863,33 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid, int stopped_by_watchpoint)
     bs->stop = 1;
     bs->print = 1;
 
-    if (b->type == bp_watchpoint ||
-	b->type == bp_hardware_watchpoint)
-      {
-	char *message = xstrprintf ("Error evaluating expression for watchpoint %d\n",
-				    b->number);
-	struct cleanup *cleanups = make_cleanup (xfree, message);
-	int e = catch_errors (watchpoint_check, bs, message, 
-			      RETURN_MASK_ALL);
-	do_cleanups (cleanups);
-	switch (e)
-	  {
-	  case WP_DELETED:
-	    /* We've already printed what needs to be printed.  */
-	    /* Actually this is superfluous, because by the time we
-               call print_it_typical() the wp will be already deleted,
-               and the function will return immediately. */
-	    bs->print_it = print_it_done;
-	    /* Stop.  */
-	    break;
-	  case WP_VALUE_CHANGED:
-	    /* Stop.  */
-	    ++(b->hit_count);
-	    break;
-	  case WP_VALUE_NOT_CHANGED:
-	    /* Don't stop.  */
-	    bs->print_it = print_it_noop;
-	    bs->stop = 0;
-	    continue;
-	  default:
-	    /* Can't happen.  */
-	    /* FALLTHROUGH */
-	  case 0:
-	    /* Error from catch_errors.  */
-	    printf_filtered (_("Watchpoint %d deleted.\n"), b->number);
-	    if (b->related_breakpoint)
-	      b->related_breakpoint->disposition = disp_del_at_next_stop;
-	    b->disposition = disp_del_at_next_stop;
-	    /* We've already printed what needs to be printed.  */
-	    bs->print_it = print_it_done;
-
-	    /* Stop.  */
-	    break;
-	  }
-      }
-    else if (b->type == bp_read_watchpoint || 
-	     b->type == bp_access_watchpoint)
+    if (b->type == bp_watchpoint
+	|| b->type == bp_read_watchpoint
+	|| b->type == bp_access_watchpoint
+	|| b->type == bp_hardware_watchpoint)
       {
 	CORE_ADDR addr;
 	struct value *v;
-	int found = 0;
-
-	if (!target_stopped_data_address (&current_target, &addr))
-	  continue;
-	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)))
-		      found = 1;
-		  }
-	      }
-	  }
-	if (found)
+	int must_check_value = 0;
+
+ 	if (b->type == bp_watchpoint)
+	  /* For a software watchpoint, we must always check the
+	     watched value.  */
+	  must_check_value = 1;
+	else if (b->watchpoint_triggered == watch_triggered_yes)
+	  /* We have a hardware watchpoint (read, write, or access)
+	     and the target earlier reported an address watched by
+	     this watchpoint.  */
+	  must_check_value = 1;
+	else if (b->watchpoint_triggered == watch_triggered_unknown
+		 && b->type == bp_hardware_watchpoint)
+	  /* We were stopped by a hardware watchpoint, but the target could
+	     not report the data address.  We must check the watchpoint's
+	     value.  Access and read watchpoints are out of luck; without
+	     a data address, we can't figure it out.  */
+	  must_check_value = 1;
+
+ 	if (must_check_value)
 	  {
 	    char *message = xstrprintf ("Error evaluating expression for watchpoint %d\n",
 					b->number);
@@ -2890,6 +2917,15 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid, int stopped_by_watchpoint)
 		++(b->hit_count);
 		break;
 	      case WP_VALUE_NOT_CHANGED:
+		if (b->type == bp_hardware_watchpoint
+		    || b->type == bp_watchpoint)
+		  {
+		    /* Don't stop: write watchpoints shouldn't fire if
+		       the value hasn't changed.  */
+		    bs->print_it = print_it_noop;
+		    bs->stop = 0;
+		    continue;
+		  }
 		/* Stop.  */
 		++(b->hit_count);
 		break;
@@ -2906,12 +2942,12 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid, int stopped_by_watchpoint)
 		break;
 	      }
 	  }
-	else	/* found == 0 */
+	else	/* must_check_value == 0 */
 	  {
-	    /* This is a case where some watchpoint(s) triggered,
-	       but not at the address of this watchpoint (FOUND
-	       was left zero).  So don't print anything for this
-	       watchpoint.  */
+	    /* This is a case where some watchpoint(s) triggered, but
+	       not at the address of this watchpoint, or else no
+	       watchpoint triggered after all.  So don't print
+	       anything for this watchpoint.  */
 	    bs->print_it = print_it_noop;
 	    bs->stop = 0;
             continue;
@@ -2933,6 +2969,13 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid, int stopped_by_watchpoint)
       {
 	int value_is_zero = 0;
 
+	/* If this is a scope breakpoint, mark the associated
+	   watchpoint as triggered so that we will handle the
+	   out-of-scope event.  We'll get to the watchpoint next
+	   iteration.  */
+	if (b->type == bp_watchpoint_scope)
+	  b->related_breakpoint->watchpoint_triggered = watch_triggered_yes;
+
 	if (bl->cond)
 	  {
 	    /* Need to select the frame, with all that implies
@@ -2963,6 +3006,9 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid, int stopped_by_watchpoint)
 	    annotate_ignore_count_change ();
 	    bs->stop = 0;
 	  }
+	else if (b->type == bp_thread_event || b->type == bp_overlay_event)
+	  /* We do not stop for these.  */
+	  bs->stop = 0;
 	else
 	  {
 	    /* We will stop here */
@@ -2989,17 +3035,27 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid, int stopped_by_watchpoint)
   bs->next = NULL;		/* Terminate the chain */
   bs = root_bs->next;		/* Re-grab the head of the chain */
 
-  /* The value of a hardware watchpoint hasn't changed, but the
-     intermediate memory locations we are watching may have.  */
-  if (bs && !bs->stop &&
-      (b->type == bp_hardware_watchpoint ||
-       b->type == bp_read_watchpoint ||
-       b->type == bp_access_watchpoint))
-    {
-      remove_breakpoints ();
-      insert_breakpoints ();
-    }
-  return bs;
+  /* If we aren't stopping, the value of some hardware watchpoint may
+     not have changed, but the intermediate memory locations we are
+     watching may have.  Don't bother if we're stopping; this will get
+     done later.  */
+  for (bs = root_bs->next; bs != NULL; bs = bs->next)
+    if (bs->stop)
+      break;
+
+  if (bs == NULL)
+    for (bs = root_bs->next; bs != NULL; bs = bs->next)
+      if (!bs->stop
+	  && (bs->breakpoint_at->owner->type == bp_hardware_watchpoint
+	      || bs->breakpoint_at->owner->type == bp_read_watchpoint
+	      || bs->breakpoint_at->owner->type == bp_access_watchpoint))
+	{
+	  remove_breakpoints ();
+	  insert_breakpoints ();
+	  break;
+	}
+
+  return root_bs->next;
 }
 
 /* Tell what to do about this bpstat.  */
@@ -5965,7 +6021,7 @@ stopat_command (char *arg, int from_tty)
 static void
 watch_command_1 (char *arg, int accessflag, int from_tty)
 {
-  struct breakpoint *b;
+  struct breakpoint *b, *scope_breakpoint = NULL;
   struct symtab_and_line sal;
   struct expression *exp;
   struct block *exp_valid_block;
@@ -6043,6 +6099,37 @@ watch_command_1 (char *arg, int accessflag, int from_tty)
   if (!mem_cnt || target_resources_ok <= 0)
     bp_type = bp_watchpoint;
 
+  frame = block_innermost_frame (exp_valid_block);
+  if (frame)
+    prev_frame = get_prev_frame (frame);
+  else
+    prev_frame = NULL;
+
+  /* If the expression is "local", then set up a "watchpoint scope"
+     breakpoint at the point where we've left the scope of the watchpoint
+     expression.  Create the scope breakpoint before the watchpoint, so
+     that we will encounter it first in bpstat_stop_status.  */
+  if (innermost_block && prev_frame)
+    {
+      scope_breakpoint = create_internal_breakpoint (get_frame_pc (prev_frame),
+						     bp_watchpoint_scope);
+
+      scope_breakpoint->enable_state = bp_enabled;
+
+      /* Automatically delete the breakpoint when it hits.  */
+      scope_breakpoint->disposition = disp_del;
+
+      /* Only break in the proper frame (help with recursion).  */
+      scope_breakpoint->frame_id = get_frame_id (prev_frame);
+
+      /* Set the address at which we will stop.  */
+      scope_breakpoint->loc->requested_address
+	= get_frame_pc (prev_frame);
+      scope_breakpoint->loc->address
+	= adjust_breakpoint_address (scope_breakpoint->loc->requested_address,
+				     scope_breakpoint->type);
+    }
+
   /* Now set up the breakpoint.  */
   b = set_raw_breakpoint (sal, bp_type);
   set_breakpoint_count (breakpoint_count + 1);
@@ -6058,48 +6145,19 @@ watch_command_1 (char *arg, int accessflag, int from_tty)
   else
     b->cond_string = 0;
 
-  frame = block_innermost_frame (exp_valid_block);
   if (frame)
-    {
-      prev_frame = get_prev_frame (frame);
-      b->watchpoint_frame = get_frame_id (frame);
-    }
+    b->watchpoint_frame = get_frame_id (frame);
   else
-    {
-      memset (&b->watchpoint_frame, 0, sizeof (b->watchpoint_frame));
-    }
+    memset (&b->watchpoint_frame, 0, sizeof (b->watchpoint_frame));
 
-  /* If the expression is "local", then set up a "watchpoint scope"
-     breakpoint at the point where we've left the scope of the watchpoint
-     expression.  */
-  if (innermost_block)
+  if (scope_breakpoint != NULL)
     {
-      if (prev_frame)
-	{
-	  struct breakpoint *scope_breakpoint;
-	  scope_breakpoint = create_internal_breakpoint (get_frame_pc (prev_frame),
-							 bp_watchpoint_scope);
-
-	  scope_breakpoint->enable_state = bp_enabled;
-
-	  /* Automatically delete the breakpoint when it hits.  */
-	  scope_breakpoint->disposition = disp_del;
-
-	  /* Only break in the proper frame (help with recursion).  */
-	  scope_breakpoint->frame_id = get_frame_id (prev_frame);
-
-	  /* Set the address at which we will stop.  */
-	  scope_breakpoint->loc->requested_address
-	    = get_frame_pc (prev_frame);
-	  scope_breakpoint->loc->address
-	    = adjust_breakpoint_address (scope_breakpoint->loc->requested_address,
-	                                 scope_breakpoint->type);
-
-	  /* The scope breakpoint is related to the watchpoint.  We
-	     will need to act on them together.  */
-	  b->related_breakpoint = scope_breakpoint;
-	}
+      /* The scope breakpoint is related to the watchpoint.  We will
+	 need to act on them together.  */
+      b->related_breakpoint = scope_breakpoint;
+      scope_breakpoint->related_breakpoint = b;
     }
+
   value_free_to_mark (mark);
   mention (b);
 }