Re: Display of read/access watchpoints when HAVE_NONSTEPPABLE_WATCHPOINT

[Orjan Friberg <orjan.friberg@axis.com>]

Orjan Friberg wrote:
> 
> Agreed.  I am quite happy to live with your suggested solution, at least 
> for now.  I certainly don't have the audacity to suggest such changes 
> should be made to accomodate a target that isn't even submitted yet ;) .

... which brings me back to the reason for re-opening this thread: getting Paul 
Koning's patch to make read/access watchpoints work when 
HAVE_NONSTEPPABLE_WATCHPOINT is defined accepted.

I did change one thing in Paul's patch, which should be highlighted: the change 
in bpstat_stop_status previously applied to bp_watchpoint types also (in 
addition to bp_hardware_watchpoint, bp_read_watchpoint, and 
bp_access_watchpoint), but as far as I can tell target_stopped_data_address 
applies only to hardware-assisted watchpoints, not software watchpoints.  Maybe 
that needs to be made more clear in the comment.

Comments?


2004-04-22  Orjan Friberg <orjanf@axis.com>

	From Paul Koning <pkoning@equallogic.com>:
	* breakpoint.c (free_valchain): New function.
	(insert_bp_location, delete_breakpoint): Use free_valchain.
	(remove_breakpoint): Do not remove the valchain.
	(bpstat_stop_status): If not stopped by watchpoint, skip
	watchpoints when generating stop status list.
	* infrun.c (handle_inferior_event): Make
	stepped_after_stopped_by_watchpoint a global variable.
	* remote.c (remote_stopped_data_address): Return watch data
	address rather than zero if stepped_after_stopped_by_watchpoint is
	set.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.167
diff -u -p -r1.167 breakpoint.c
--- breakpoint.c        21 Apr 2004 23:52:19 -0000      1.167
+++ breakpoint.c        22 Apr 2004 14:56:02 -0000
@@ -746,6 +746,23 @@ insert_catchpoint (struct ui_out *uo, vo
    return 0;
  }

+/* Helper routine: free the value chain for a breakpoint (watchpoint).  */
+
+static void free_valchain (struct bp_location *b)
+{
+  struct value *v;
+  struct value *n;
+
+  /* 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)
+    {
+      n = v->next;
+      value_free (v);
+    }
+  b->owner->val_chain = NULL;
+}
+
  /* 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.
@@ -920,6 +937,8 @@ insert_bp_location (struct bp_location *

        if (within_current_scope)
         {
+         free_valchain (bpt);
+
           /* Evaluate the expression and cut the chain of values
              produced off from the value chain.

@@ -1505,15 +1524,6 @@ remove_breakpoint (struct bp_location *b
        if ((is == mark_uninserted) && (b->inserted))
         warning ("Could not remove hardware watchpoint %d.",
                  b->owner->number);
-
-      /* 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)
-       {
-         n = v->next;
-         value_free (v);
-       }
-      b->owner->val_chain = NULL;
      }
    else if ((b->owner->type == bp_catch_fork ||
             b->owner->type == bp_catch_vfork ||
@@ -2616,10 +2626,15 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
      if (!breakpoint_enabled (b) && b->enable_state != bp_permanent)
        continue;

+    /* 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 watchpoint has been defined.  */
      if (b->type != bp_watchpoint
-       && b->type != bp_hardware_watchpoint
-       && b->type != bp_read_watchpoint
-       && b->type != bp_access_watchpoint
+       && !((b->type == bp_hardware_watchpoint
+             || b->type == bp_read_watchpoint
+             || b->type == bp_access_watchpoint)
+          && target_stopped_data_address () != 0)
         && b->type != bp_hardware_breakpoint
         && b->type != bp_catch_fork
         && b->type != bp_catch_vfork
@@ -6880,6 +6895,8 @@ delete_breakpoint (struct breakpoint *bp

    if (bpt->loc->inserted)
      remove_breakpoint (bpt->loc, mark_inserted);
+
+  free_valchain (bpt->loc);

    if (breakpoint_chain == bpt)
      breakpoint_chain = bpt->next;
Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.133
diff -u -p -r1.133 remote.c
--- remote.c    21 Apr 2004 23:52:20 -0000      1.133
+++ remote.c    22 Apr 2004 14:56:05 -0000
@@ -4640,10 +4640,13 @@ remote_stopped_by_watchpoint (void)
      return remote_stopped_by_watchpoint_p;
  }

+extern int stepped_after_stopped_by_watchpoint;
+
  static CORE_ADDR
  remote_stopped_data_address (void)
  {
-  if (remote_stopped_by_watchpoint ())
+  if (remote_stopped_by_watchpoint ()
+      || stepped_after_stopped_by_watchpoint)
      return remote_watch_data_address;
    return (CORE_ADDR)0;
  }
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.147
diff -u -p -r1.147 infrun.c
--- infrun.c    21 Apr 2004 23:52:20 -0000      1.147
+++ infrun.c    22 Apr 2004 14:56:07 -0000
@@ -1356,6 +1356,8 @@ adjust_pc_after_break (struct execution_
     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)
  {
@@ -1364,7 +1366,6 @@ handle_inferior_event (struct execution_
       isn't used, then you're wrong!  The macro STOPPED_BY_WATCHPOINT,
       defined in the file "config/pa/nm-hppah.h", accesses the variable
       indirectly.  Mutter something rude about the HP merge.  */
-  int stepped_after_stopped_by_watchpoint;
    int sw_single_step_trap_p = 0;

    /* Cache the last pid/waitstatus. */

-- 
Orjan Friberg
Axis Communications