2007-09-16 Daniel Jacobowitz <dan@codesourcery.com>
authorDaniel Jacobowitz <drow@false.org>
Mon, 1 Oct 2007 00:17:58 +0000 (00:17 +0000)
committerDaniel Jacobowitz <drow@false.org>
Mon, 1 Oct 2007 00:17:58 +0000 (00:17 +0000)
    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.

gdb/ChangeLog
gdb/breakpoint.c
gdb/breakpoint.h
gdb/doc/ChangeLog
gdb/doc/gdb.texinfo
gdb/doc/gdbint.texinfo
gdb/infrun.c
gdb/remote.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.threads/watchthreads.c
gdb/testsuite/gdb.threads/watchthreads.exp

index 8ae983d..1ecf2e4 100644 (file)
@@ -1,3 +1,28 @@
+2007-09-30  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.
+
 2007-09-29  Daniel Jacobowitz  <dan@codesourcery.com>
 
        * configure.ac: Add $LIBINTL when testing libbfd.
index 0d1cb9e..e4fdb33 100644 (file)
@@ -2537,6 +2537,83 @@ bpstat_alloc (struct bp_location *bl, bpstat cbs /* Current "bs" value */ )
   return bs;
 }
 \f
+/* 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;
 }
 \f
 /* 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);
 }
index 6dfb27c..1e68a2a 100644 (file)
@@ -318,6 +318,19 @@ struct breakpoint_ops
   void (*print_mention) (struct breakpoint *);
 };
 
+enum watchpoint_triggered
+{
+  /* This watchpoint definitely did not trigger.  */
+  watch_triggered_no = 0,
+
+  /* Some hardware watchpoint triggered, and it might have been this
+     one, but we do not know which it was.  */
+  watch_triggered_unknown,
+
+  /* This hardware watchpoint definitely did trigger.  */
+  watch_triggered_yes  
+};
+
 /* 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
@@ -395,6 +408,10 @@ struct breakpoint
        should be evaluated on the outermost frame.  */
     struct frame_id watchpoint_frame;
 
+    /* For hardware watchpoints, the triggered status according to the
+       hardware.  */
+    enum watchpoint_triggered watchpoint_triggered;
+
     /* Thread number for thread-specific breakpoint, or -1 if don't care */
     int thread;
 
@@ -459,8 +476,7 @@ extern void bpstat_clear (bpstat *);
    is part of the bpstat is copied as well.  */
 extern bpstat bpstat_copy (bpstat);
 
-extern bpstat bpstat_stop_status (CORE_ADDR pc, ptid_t ptid, 
-                                 int stopped_by_watchpoint);
+extern bpstat bpstat_stop_status (CORE_ADDR pc, ptid_t ptid);
 \f
 /* This bpstat_what stuff tells wait_for_inferior what to do with a
    breakpoint (a challenging task).  */
@@ -853,4 +869,8 @@ extern void remove_single_step_breakpoints (void);
 extern void *deprecated_insert_raw_breakpoint (CORE_ADDR);
 extern int deprecated_remove_raw_breakpoint (void *);
 
+/* Check if any hardware watchpoints have triggered, according to the
+   target.  */
+int watchpoints_triggered (struct target_waitstatus *);
+
 #endif /* !defined (BREAKPOINT_H) */
index 4c8e7cd..e7f03fd 100644 (file)
@@ -1,3 +1,12 @@
+2007-09-30  Daniel Jacobowitz  <dan@codesourcery.com>
+
+       * 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.
+
 2007-09-28  Vladimir Prus  <vladimir@codesourcery.com>
 
        * gdb.texinfo (Setting Breakpoints): Revise
index 3c4d99d..fac3f67 100644 (file)
@@ -3346,20 +3346,13 @@ rerun the program, you will need to set all such watchpoints again.  One
 way of doing that would be to set a code breakpoint at the entry to the
 @code{main} function and when it breaks, set all the watchpoints.
 
-@quotation
 @cindex watchpoints and threads
 @cindex threads and watchpoints
-@emph{Warning:} In multi-thread programs, watchpoints have only limited
-usefulness.  With the current watchpoint implementation, @value{GDBN}
-can only watch the value of an expression @emph{in a single thread}.  If
-you are confident that the expression can only change due to the current
-thread's activity (and if you are also confident that no other thread
-can become current), then you can use watchpoints as usual.  However,
-@value{GDBN} may not notice when a non-current thread's activity changes
-the expression.
-
-@c FIXME: this is almost identical to the previous paragraph.
-@emph{HP-UX Warning:} In multi-thread programs, software watchpoints
+In multi-threaded programs, watchpoints will detect changes to the
+watched expression from every thread.
+
+@quotation
+@emph{Warning:} In multi-threaded programs, software watchpoints
 have only limited usefulness.  If @value{GDBN} creates a software
 watchpoint, it can only watch the value of an expression @emph{in a
 single thread}.  If you are confident that the expression can only
index e6d9e87..facea70 100644 (file)
@@ -660,15 +660,26 @@ section is mostly irrelevant for software watchpoints.
 
 When the inferior stops, @value{GDBN} tries to establish, among other
 possible reasons, whether it stopped due to a watchpoint being hit.
-For a data-write watchpoint, it does so by evaluating, for each
-watchpoint, the expression whose value is being watched, and testing
-whether the watched value has changed.  For data-read and data-access
-watchpoints, @value{GDBN} needs the target to supply a primitive that
-returns the address of the data that was accessed or read (see the
-description of @code{target_stopped_data_address} below): if this
-primitive returns a valid address, @value{GDBN} infers that a
-watchpoint triggered if it watches an expression whose evaluation uses
-that address.
+It first uses @code{STOPPED_BY_WATCHPOINT} to see if any watchpoint
+was hit.  If not, all watchpoint checking is skipped.
+
+Then @value{GDBN} calls @code{target_stopped_data_address} exactly
+once.  This method returns the address of the watchpoint which
+triggered, if the target can determine it.  If the triggered address
+is available, @value{GDBN} compares the address returned by this
+method with each watched memory address in each active watchpoint.
+For data-read and data-access watchpoints, @value{GDBN} announces
+every watchpoint that watches the triggered address as being hit.
+For this reason, data-read and data-access watchpoints
+@emph{require} that the triggered address be available; if not, read
+and access watchpoints will never be considered hit.  For data-write
+watchpoints, if the triggered address is available, @value{GDBN}
+considers only those watchpoints which match that address;
+otherwise, @value{GDBN} considers all data-write watchpoints.  For
+each data-write watchpoint that @value{GDBN} considers, it evaluates
+the expression whose value is being watched, and tests whether the
+watched value has changed.  Watchpoints whose watched values have
+changed are announced as hit.
 
 @value{GDBN} uses several macros and primitives to support hardware
 watchpoints:
@@ -721,26 +732,40 @@ These two macros should return 0 for success, non-zero for failure.
 @item target_stopped_data_address (@var{addr_p})
 If the inferior has some watchpoint that triggered, place the address
 associated with the watchpoint at the location pointed to by
-@var{addr_p} and return non-zero.  Otherwise, return zero.  Note that
-this primitive is used by @value{GDBN} only on targets that support
-data-read or data-access type watchpoints, so targets that have
-support only for data-write watchpoints need not implement these
-primitives.
+@var{addr_p} and return non-zero.  Otherwise, return zero.  This
+is required for data-read and data-access watchpoints.  It is
+not required for data-write watchpoints, but @value{GDBN} uses
+it to improve handling of those also.
+
+@value{GDBN} will only call this method once per watchpoint stop,
+immediately after calling @code{STOPPED_BY_WATCHPOINT}.  If the
+target's watchpoint indication is sticky, i.e., stays set after
+resuming, this method should clear it.  For instance, the x86 debug
+control register has sticky triggered flags.
 
 @findex HAVE_STEPPABLE_WATCHPOINT
 @item HAVE_STEPPABLE_WATCHPOINT
 If defined to a non-zero value, it is not necessary to disable a
-watchpoint to step over it.
+watchpoint to step over it.    Like @code{gdbarch_have_nonsteppable_watchpoint},
+this is usually set when watchpoints trigger at the instruction
+which will perform an interesting read or write.  It should be
+set if there is a temporary disable bit which allows the processor
+to step over the interesting instruction without raising the
+watchpoint exception again.
 
 @findex gdbarch_have_nonsteppable_watchpoint 
 @item int gdbarch_have_nonsteppable_watchpoint (@var{gdbarch})
 If it returns a non-zero value, @value{GDBN} should disable a
-watchpoint to step the inferior over it.
+watchpoint to step the inferior over it.  This is usually set when
+watchpoints trigger at the instruction which will perform an
+interesting read or write.
 
 @findex HAVE_CONTINUABLE_WATCHPOINT
 @item HAVE_CONTINUABLE_WATCHPOINT
 If defined to a non-zero value, it is possible to continue the
-inferior after a watchpoint has been hit.
+inferior after a watchpoint has been hit.  This is usually set
+when watchpoints trigger at the instruction following an interesting
+read or write.
 
 @findex CANNOT_STEP_HW_WATCHPOINTS
 @item CANNOT_STEP_HW_WATCHPOINTS
@@ -763,6 +788,32 @@ determine for sure whether the inferior stopped due to a watchpoint,
 it could return non-zero ``just in case''.
 @end table
 
+@subsection Watchpoints and Threads
+@cindex watchpoints, with threads
+
+@value{GDBN} only supports process-wide watchpoints, which trigger
+in all threads.  @value{GDBN} uses the thread ID to make watchpoints
+act as if they were thread-specific, but it cannot set hardware
+watchpoints that only trigger in a specific thread.  Therefore, even
+if the target supports threads, per-thread debug registers, and
+watchpoints which only affect a single thread, it should set the
+per-thread debug registers for all threads to the same value.  On
+@sc{gnu}/Linux native targets, this is accomplished by using
+@code{ALL_LWPS} in @code{target_insert_watchpoint} and
+@code{target_remove_watchpoint} and by using
+@code{linux_set_new_thread} to register a handler for newly created
+threads.
+
+@value{GDBN}'s @sc{gnu}/Linux support only reports a single event
+at a time, although multiple events can trigger simultaneously for
+multi-threaded programs.  When multiple events occur, @file{linux-nat.c}
+queues subsequent events and returns them the next time the program
+is resumed.  This means that @code{STOPPED_BY_WATCHPOINT} and
+@code{target_stopped_data_address} only need to consult the current
+thread's state---the thread indicated by @code{inferior_ptid}.  If
+two threads have hit watchpoints simultaneously, those routines
+will be called a second time for the second thread.
+
 @subsection x86 Watchpoints
 @cindex x86 debug registers
 @cindex watchpoints, on x86
index f1a96a3..f03a5f2 100644 (file)
@@ -881,6 +881,7 @@ enum infwait_states
 {
   infwait_normal_state,
   infwait_thread_hop_state,
+  infwait_step_watch_state,
   infwait_nonstep_watch_state
 };
 
@@ -1220,17 +1221,12 @@ adjust_pc_after_break (struct execution_control_state *ecs)
    by an event from the inferior, figure out what it means and take
    appropriate action.  */
 
-int stepped_after_stopped_by_watchpoint;
-
 void
 handle_inferior_event (struct execution_control_state *ecs)
 {
-  /* NOTE: bje/2005-05-02: If you're looking at this code and thinking
-     that the variable stepped_after_stopped_by_watchpoint isn't used,
-     then you're wrong!  See remote.c:remote_stopped_data_address.  */
-
   int sw_single_step_trap_p = 0;
-  int stopped_by_watchpoint = -1;      /* Mark as unknown.  */
+  int stopped_by_watchpoint;
+  int stepped_after_stopped_by_watchpoint = 0;
 
   /* Cache the last pid/waitstatus. */
   target_last_wait_ptid = ecs->ptid;
@@ -1250,7 +1246,14 @@ handle_inferior_event (struct execution_control_state *ecs)
     case infwait_normal_state:
       if (debug_infrun)
         fprintf_unfiltered (gdb_stdlog, "infrun: infwait_normal_state\n");
-      stepped_after_stopped_by_watchpoint = 0;
+      break;
+
+    case infwait_step_watch_state:
+      if (debug_infrun)
+        fprintf_unfiltered (gdb_stdlog,
+                           "infrun: infwait_step_watch_state\n");
+
+      stepped_after_stopped_by_watchpoint = 1;
       break;
 
     case infwait_nonstep_watch_state:
@@ -1435,7 +1438,7 @@ handle_inferior_event (struct execution_control_state *ecs)
 
       stop_pc = read_pc ();
 
-      stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid, 0);
+      stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid);
 
       ecs->random_signal = !bpstat_explains_signal (stop_bpstat);
 
@@ -1483,7 +1486,7 @@ handle_inferior_event (struct execution_control_state *ecs)
       ecs->saved_inferior_ptid = inferior_ptid;
       inferior_ptid = ecs->ptid;
 
-      stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid, 0);
+      stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid);
 
       ecs->random_signal = !bpstat_explains_signal (stop_bpstat);
       inferior_ptid = ecs->saved_inferior_ptid;
@@ -1796,24 +1799,20 @@ handle_inferior_event (struct execution_control_state *ecs)
       singlestep_breakpoints_inserted_p = 0;
     }
 
-  /* It may not be necessary to disable the watchpoint to stop over
-     it.  For example, the PA can (with some kernel cooperation)
-     single step over a watchpoint without disabling the watchpoint.  */
-  if (HAVE_STEPPABLE_WATCHPOINT && STOPPED_BY_WATCHPOINT (ecs->ws))
+  if (stepped_after_stopped_by_watchpoint)
+    stopped_by_watchpoint = 0;
+  else
+    stopped_by_watchpoint = watchpoints_triggered (&ecs->ws);
+
+  /* If necessary, step over this watchpoint.  We'll be back to display
+     it in a moment.  */
+  if (stopped_by_watchpoint
+      && (HAVE_STEPPABLE_WATCHPOINT
+         || gdbarch_have_nonsteppable_watchpoint (current_gdbarch)))
     {
       if (debug_infrun)
        fprintf_unfiltered (gdb_stdlog, "infrun: STOPPED_BY_WATCHPOINT\n");
-      resume (1, 0);
-      prepare_to_wait (ecs);
-      return;
-    }
 
-  /* It is far more common to need to disable a watchpoint to step
-     the inferior over it.  FIXME.  What else might a debug
-     register or page protection watchpoint scheme need here?  */
-  if (gdbarch_have_nonsteppable_watchpoint (current_gdbarch)
-      && STOPPED_BY_WATCHPOINT (ecs->ws))
-    {
       /* At this point, we are stopped at an instruction which has
          attempted to write to a piece of memory under control of
          a watchpoint.  The instruction hasn't actually executed
@@ -1823,31 +1822,31 @@ handle_inferior_event (struct execution_control_state *ecs)
 
          In order to make watchpoints work `right', we really need
          to complete the memory write, and then evaluate the
-         watchpoint expression.  The following code does that by
-         removing the watchpoint (actually, all watchpoints and
-         breakpoints), single-stepping the target, re-inserting
-         watchpoints, and then falling through to let normal
-         single-step processing handle proceed.  Since this
-         includes evaluating watchpoints, things will come to a
-         stop in the correct manner.  */
-
-      if (debug_infrun)
-       fprintf_unfiltered (gdb_stdlog, "infrun: STOPPED_BY_WATCHPOINT\n");
-      remove_breakpoints ();
+         watchpoint expression.  We do this by single-stepping the
+        target.
+
+        It may not be necessary to disable the watchpoint to stop over
+        it.  For example, the PA can (with some kernel cooperation)
+        single step over a watchpoint without disabling the watchpoint.
+
+        It is far more common to need to disable a watchpoint to step
+        the inferior over it.  If we have non-steppable watchpoints,
+        we must disable the current watchpoint; it's simplest to
+        disable all watchpoints and breakpoints.  */
+        
+      if (!HAVE_STEPPABLE_WATCHPOINT)
+       remove_breakpoints ();
       registers_changed ();
       target_resume (ecs->ptid, 1, TARGET_SIGNAL_0);   /* Single step */
-
       ecs->waiton_ptid = ecs->ptid;
-      ecs->wp = &(ecs->ws);
-      ecs->infwait_state = infwait_nonstep_watch_state;
+      if (HAVE_STEPPABLE_WATCHPOINT)
+       ecs->infwait_state = infwait_step_watch_state;
+      else
+       ecs->infwait_state = infwait_nonstep_watch_state;
       prepare_to_wait (ecs);
       return;
     }
 
-  /* It may be possible to simply continue after a watchpoint.  */
-  if (HAVE_CONTINUABLE_WATCHPOINT)
-    stopped_by_watchpoint = STOPPED_BY_WATCHPOINT (ecs->ws);
-
   ecs->stop_func_start = 0;
   ecs->stop_func_end = 0;
   ecs->stop_func_name = 0;
@@ -1969,8 +1968,7 @@ handle_inferior_event (struct execution_control_state *ecs)
       else
        {
          /* See if there is a breakpoint at the current PC.  */
-         stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid,
-                                           stopped_by_watchpoint);
+         stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid);
 
          /* Following in case break condition called a
             function.  */
index 309ee67..23fa18c 100644 (file)
@@ -5406,14 +5406,11 @@ remote_stopped_by_watchpoint (void)
     return remote_stopped_by_watchpoint_p;
 }
 
-extern int stepped_after_stopped_by_watchpoint;
-
 static int
 remote_stopped_data_address (struct target_ops *target, CORE_ADDR *addr_p)
 {
   int rc = 0;
-  if (remote_stopped_by_watchpoint ()
-      || stepped_after_stopped_by_watchpoint)
+  if (remote_stopped_by_watchpoint ())
     {
       *addr_p = remote_watch_data_address;
       rc = 1;
index e5212be..f4882ef 100644 (file)
@@ -1,3 +1,10 @@
+2007-09-30  Daniel Jacobowitz  <dan@codesourcery.com>
+
+       * 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.
+
 2007-09-27  Vladimir Prus  <vladimir@codesourcery.com>
 
        * gdb.mi/var-cmd.c (do_children_tests): Initialize
index c607429..ef391f0 100644 (file)
@@ -56,7 +56,7 @@ void *thread_function(void *arg) {
     /* Don't run forever.  Run just short of it :)  */
     while (*myp > 0)
       {
-       (*myp) ++;  /* Loop increment.  */
+       (*myp) ++; usleep (1);  /* Loop increment.  */
       }
 
     pthread_exit(NULL);
index cdd3edb..11fc2af 100644 (file)
@@ -30,6 +30,10 @@ if [target_info exists gdb,no_hardware_watchpoints] {
     return 0;
 }
 
+proc target_no_stopped_data { } {
+    return [istarget s390*-*-*]
+}
+
 set testfile "watchthreads"
 set srcfile ${testfile}.c
 set binfile ${objdir}/${subdir}/${testfile}
@@ -61,20 +65,58 @@ gdb_test "watch args\[1\]" "Hardware watchpoint 3: args\\\[1\\\]"
 
 set init_line [expr [gdb_get_line_number "Init value"]+1]
 set inc_line [gdb_get_line_number "Loop increment"]
+set main_loc "main \\\(\\\) at .*watchthreads.c:$init_line"
+set thread0_loc "thread_function \\\(arg=0x0\\\) at .*watchthreads.c:$inc_line"
+set thread1_loc "thread_function \\\(arg=0x1\\\) at .*watchthreads.c:$inc_line"
 
 # Loop and continue to allow both watchpoints to be triggered.
 for {set i 0} {$i < 30} {incr i} {
+  set test_flag_0 0
+  set test_flag_1 0
   set test_flag 0
   gdb_test_multiple "continue" "threaded watch loop" {
-    -re "Hardware watchpoint 2: args\\\[0\\\].*Old value = 0.*New value = 1.*main \\\(\\\) at .*watchthreads.c:$init_line.*$gdb_prompt $"
-       { set args_0 1; set test_flag 1 }
-    -re "Hardware watchpoint 3: args\\\[1\\\].*Old value = 0.*New value = 1.*main \\\(\\\) at .*watchthreads.c:$init_line.*$gdb_prompt $"
-       { set args_1 1; set test_flag 1 }
-    -re "Hardware watchpoint 2: args\\\[0\\\].*Old value = $args_0.*New value = [expr $args_0+1].*in thread_function \\\(arg=0x0\\\) at .*watchthreads.c:$inc_line.*$gdb_prompt $"
-       { set args_0 [expr $args_0+1]; set test_flag 1 }
-    -re "Hardware watchpoint 3: args\\\[1\\\].*Old value = $args_1.*New value = [expr $args_1+1].*in thread_function \\\(arg=0x1\\\) at .*watchthreads.c:$inc_line.*$gdb_prompt $"
-       { set args_1 [expr $args_1+1]; set test_flag 1 }
+    -re "(.*Hardware watchpoint.*)$gdb_prompt $" {
+       # At least one hardware watchpoint was hit.  Check if both were.
+       set string $expect_out(1,string)
+
+       if [regexp "Hardware watchpoint 2: args\\\[0\\\]\[^\r\]*\r\[^\r\]*\r\[^\r\]*Old value = $args_0\[^\r\]*\r\[^\r\]*New value = [expr $args_0+1]\r" $string] {
+           incr args_0
+           incr test_flag_0
+       }
+       if [regexp "Hardware watchpoint 3: args\\\[1\\\]\[^\r\]*\r\[^\r\]*\r\[^\r\]*Old value = $args_1\[^\r\]*\r\[^\r\]*New value = [expr $args_1+1]\r" $string] {
+           incr args_1
+           incr test_flag_1
+       }
+
+       set expected_loc "bogus location"
+       if { $test_flag_0 == 1 && $test_flag_1 == 0 && $args_0 == 1 } {
+           set expected_loc $main_loc
+       } elseif { $test_flag_0 == 0 && $test_flag_1 == 1 && $args_1 == 1 } {
+           set expected_loc $main_loc
+       } elseif { $test_flag_0 == 1 && $test_flag_1 == 0 } {
+           set expected_loc $thread0_loc
+       } elseif { $test_flag_0 == 0 && $test_flag_1 == 1 } {
+           set expected_loc $thread1_loc
+       } elseif { $test_flag_0 + $test_flag_1 == 2 } {
+           # On S/390, or any other system which can not report the
+           # stopped data address, it is OK to report two watchpoints
+           # at once in this test.  Make sure the reported location
+           # corresponds to at least one of the watchpoints (and not,
+           # e.g., __nptl_create_event).  On other systems, we should
+           # report the two watchpoints serially.
+           if { [target_no_stopped_data] } {
+               set expected_loc "($main_loc|$thread0_loc|$thread1_loc)"
+           }
+       }
+
+       if [ regexp "$expected_loc" $string ] {
+           set test_flag 1
+       } else {
+           fail "threaded watch loop"
+       }
+    }
   }
+
   # If we fail above, don't bother continuing loop
   if { $test_flag == 0 } {
     set i 30;
@@ -120,7 +162,14 @@ if { $args_1 > 1 } {
 
 # Verify that all watchpoint hits are accounted for.
 set message "combination of threaded watchpoints = 30"
-if { [expr $args_0+$args_1] == 30 } {
+if { [target_no_stopped_data] } {
+    # See above.  If we allow two watchpoints to be hit at once, we
+    # may have more than 30 hits total.
+    set result [expr $args_0 + $args_1 >= 30]
+} else {
+    set result [expr $args_0 + $args_1 == 30]
+}
+if { $result } {
   pass $message 
 } else {
   fail $message