This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA] Fix hw watchpoints in process record.


Here is a patch to make hardware watchpoints work in precord.

Biggest change is in breakpoint.c, but it's mainly mechanical.
I abstracted most of the function "watchpoint_check" into a
separate function so it could be called from two places.

Then in record_wait, if not replay mode we call
target_stopped_by_watchpoint to see if a watchpoint
was triggered.  In replay mode we call the new function
hw_watchpoint_check.

2009-10-31  Michael Snyder  <msnyder@vmware.com>
	Make hardware watchpoints work for process record.
	* breakpoint.c (watchpoint_check_1): Abstracted from watchpoint_check.
	(watchpoint_check_2): Check_error entry point for above.
	(watchpoint_check): Call watchpoint_check_1.
	(hw_watchpoint_check): New function.  Return true if 
	a hardware watchpoint expression has changed.
	* breakpoint.h (hw_watchpoint_check): Export.

	* record.c (record_beneath_to_stopped_by_watchpoint): New pointer.
	(record_open): Initialize above pointer.
	(record_stopped_by_watchpoint): New target method.
	(record_wait): Check to see if hitting hardware watchpoint.

Index: gdb/breakpoint.c
===================================================================
--- gdb.orig/breakpoint.c	2009-10-31 17:31:15.000000000 -0700
+++ gdb/breakpoint.c	2009-10-31 17:50:07.000000000 -0700
@@ -3075,18 +3075,13 @@
 #define BP_TEMPFLAG 1
 #define BP_HARDWAREFLAG 2
 
-/* Check watchpoint condition.  */
-
 static int
-watchpoint_check (void *p)
+watchpoint_check_1 (void *p, struct value **new_valp)
 {
-  bpstat bs = (bpstat) p;
-  struct breakpoint *b;
+  struct breakpoint *b = p;
   struct frame_info *fr;
   int within_current_scope;
 
-  b = bs->breakpoint_at->owner;
-
   if (b->exp_valid_block == NULL)
     within_current_scope = 1;
   else
@@ -3137,20 +3132,16 @@
          we might be in the middle of evaluating a function call.  */
 
       struct value *mark = value_mark ();
-      struct value *new_val;
 
-      fetch_watchpoint_value (b->exp, &new_val, NULL, NULL);
-      if ((b->val != NULL) != (new_val != NULL)
-	  || (b->val != NULL && !value_equal (b->val, new_val)))
+      fetch_watchpoint_value (b->exp, new_valp, NULL, NULL);
+      if ((b->val != NULL) != (*new_valp != NULL)
+	  || (b->val != NULL && !value_equal (b->val, *new_valp)))
 	{
-	  if (new_val != NULL)
+	  if (*new_valp != NULL)
 	    {
-	      release_value (new_val);
+	      release_value (*new_valp);
 	      value_free_to_mark (mark);
 	    }
-	  bs->old_val = b->val;
-	  b->val = new_val;
-	  b->val_valid = 1;
 	  /* We will stop here */
 	  return WP_VALUE_CHANGED;
 	}
@@ -3181,8 +3172,9 @@
 	  (uiout, "reason", async_reason_lookup (EXEC_ASYNC_WATCHPOINT_SCOPE));
       ui_out_text (uiout, "\nWatchpoint ");
       ui_out_field_int (uiout, "wpnum", b->number);
-      ui_out_text (uiout, " deleted because the program has left the block in\n\
-which its expression is valid.\n");     
+      ui_out_text (uiout,
+		   " deleted because the program has left the block in\n\
+which its expression is valid.\n");
 
       if (b->related_breakpoint)
 	b->related_breakpoint->disposition = disp_del_at_next_stop;
@@ -3192,6 +3184,36 @@
     }
 }
 
+static int
+watchpoint_check_2 (void *p)
+{
+  struct value *notused;
+
+  return watchpoint_check_1 (p, &notused);
+}
+
+/* Check watchpoint condition.  */
+
+static int
+watchpoint_check (void *p)
+{
+  bpstat bs = (bpstat) p;
+  struct value *new_val;
+  struct breakpoint *b;
+  int ret;
+
+  b = bs->breakpoint_at->owner;
+  ret = watchpoint_check_1 (b, &new_val);
+
+  if (ret == WP_VALUE_CHANGED)
+    {
+      bs->old_val = b->val;
+      b->val = new_val;
+      b->val_valid = 1;
+    }
+  return ret;
+}
+
 /* Return true if it looks like target has stopped due to hitting
    breakpoint location BL.  This function does not check if we
    should stop, only if BL explains the stop.   */
@@ -3250,6 +3272,29 @@
   return 1;
 }
 
+int
+hw_watchpoint_check (void)
+{
+  struct breakpoint *b;
+  struct value *new_val;
+
+  ALL_BREAKPOINTS (b)
+    if (b->type == bp_hardware_watchpoint
+	|| b->type == bp_access_watchpoint)
+      {
+	char *msg
+	  = xstrprintf (_("Error evaluating expression for watchpoint %d\n"),
+			b->number);
+	struct cleanup *cleanups = make_cleanup (xfree, msg);
+	int e = catch_errors (watchpoint_check_2, b, msg, RETURN_MASK_ALL);
+	do_cleanups (cleanups);
+	if (e == WP_VALUE_CHANGED)
+	  return 1;	/* should stop */
+      }
+  return 0;		/* don't stop */
+}
+
+
 /* If BS refers to a watchpoint, determine if the watched values
    has actually changed, and we should stop.  If not, set BS->stop
    to 0.  */
@@ -3267,7 +3312,7 @@
       CORE_ADDR addr;
       struct value *v;
       int must_check_value = 0;
-      
+
       if (b->type == bp_watchpoint)
 	/* For a software watchpoint, we must always check the
 	   watched value.  */
@@ -3284,7 +3329,7 @@
 	   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",
Index: gdb/breakpoint.h
===================================================================
--- gdb.orig/breakpoint.h	2009-10-31 17:31:21.000000000 -0700
+++ gdb/breakpoint.h	2009-10-31 17:33:23.000000000 -0700
@@ -978,4 +978,6 @@
    is newly allocated; the caller should free when done with it.  */
 extern VEC(breakpoint_p) *all_tracepoints (void);
 
+extern int hw_watchpoint_check (void);
+
 #endif /* !defined (BREAKPOINT_H) */
Index: gdb/record.c
===================================================================
--- gdb.orig/record.c	2009-10-31 17:31:00.000000000 -0700
+++ gdb/record.c	2009-10-31 17:34:24.000000000 -0700
@@ -224,6 +224,7 @@
 						   struct bp_target_info *);
 static int (*record_beneath_to_remove_breakpoint) (struct gdbarch *,
 						   struct bp_target_info *);
+static int (*record_beneath_to_stopped_by_watchpoint) (void);
 
 /* Alloc and free functions for record_reg, record_mem, and record_end 
    entries.  */
@@ -770,6 +771,7 @@
 					struct bp_target_info *);
 static int (*tmp_to_remove_breakpoint) (struct gdbarch *,
 					struct bp_target_info *);
+static int (*tmp_to_stopped_by_watchpoint) (void);
 
 static void record_restore (void);
 
@@ -894,6 +896,8 @@
 	tmp_to_insert_breakpoint = t->to_insert_breakpoint;
       if (!tmp_to_remove_breakpoint)
 	tmp_to_remove_breakpoint = t->to_remove_breakpoint;
+      if (!tmp_to_stopped_by_watchpoint)
+	tmp_to_stopped_by_watchpoint = t->to_stopped_by_watchpoint;
     }
   if (!tmp_to_xfer_partial)
     error (_("Could not find 'to_xfer_partial' method on the target stack."));
@@ -915,6 +919,7 @@
   record_beneath_to_xfer_partial = tmp_to_xfer_partial;
   record_beneath_to_insert_breakpoint = tmp_to_insert_breakpoint;
   record_beneath_to_remove_breakpoint = tmp_to_remove_breakpoint;
+  record_beneath_to_stopped_by_watchpoint = tmp_to_stopped_by_watchpoint;
 
   if (current_target.to_stratum == core_stratum)
     record_core_open_1 (name, from_tty);
@@ -1010,6 +1015,9 @@
     record_list = record_list->prev;
 }
 
+/* Flag set to TRUE for target_stopped_by_watchpoint.  */
+static int record_hw_watchpoint = 0;
+
 /* "to_wait" target method for process record target.
 
    In record mode, the target is always run in singlestep mode
@@ -1069,21 +1077,27 @@
 		{
 		  struct regcache *regcache;
 
-		  /* Yes -- check if there is a breakpoint.  */
+		  /* Yes -- check if there is a breakpoint or watchpoint.  */
 		  registers_changed ();
 		  regcache = get_current_regcache ();
 		  tmp_pc = regcache_read_pc (regcache);
 		  if (breakpoint_inserted_here_p (get_regcache_aspace (regcache),
-						  tmp_pc))
+						  tmp_pc)
+		      || target_stopped_by_watchpoint ())
 		    {
-		      /* There is a breakpoint.  GDB will want to stop.  */
-		      struct gdbarch *gdbarch = get_regcache_arch (regcache);
+		      /* There is a breakpoint, or watchpoint.
+			 GDB will want to stop.  */
+		      if (!target_stopped_by_watchpoint ())
+			{
+			  struct gdbarch *gdbarch
+			    = get_regcache_arch (regcache);
 		      CORE_ADDR decr_pc_after_break
 			= gdbarch_decr_pc_after_break (gdbarch);
 		      if (decr_pc_after_break)
 			regcache_write_pc (regcache,
 					   tmp_pc + decr_pc_after_break);
 		    }
+		    }
 		  else
 		    {
 		      /* There is not a breakpoint, and gdb is not
@@ -1116,9 +1130,10 @@
       struct cleanup *old_cleanups = make_cleanup (record_wait_cleanups, 0);
       CORE_ADDR tmp_pc;
 
+      record_hw_watchpoint = 0;
       status->kind = TARGET_WAITKIND_STOPPED;
 
-      /* Check breakpoint when forward execute.  */
+      /* Check for breakpoint or watchpoint when forward execute.  */
       if (execution_direction == EXEC_FORWARD)
 	{
 	  tmp_pc = regcache_read_pc (regcache);
@@ -1136,6 +1151,14 @@
 				   gdbarch_decr_pc_after_break (gdbarch));
 	      goto replay_out;
 	    }
+	  if (hw_watchpoint_check ())
+	    {
+	      if (record_debug)
+		fprintf_unfiltered (gdb_stdlog,
+				    "Process record: hit hw watchpoint.\n");
+	      record_hw_watchpoint = 1;
+	    }
+
 	}
 
       record_get_sig = 0;
@@ -1219,6 +1242,15 @@
 					   gdbarch_decr_pc_after_break (gdbarch));
 		      continue_flag = 0;
 		    }
+		  /* check watchpoint */
+		  if (hw_watchpoint_check ())
+		    {
+		      if (record_debug)
+			fprintf_unfiltered (gdb_stdlog,
+					    "Process record: hit hw watchpoint.\n");
+		      record_hw_watchpoint = 1;
+		      continue_flag = 0;
+		    }
 		  /* Check target signal */
 		  if (record_list->u.end.sigval != TARGET_SIGNAL_0)
 		    /* FIXME: better way to check */
@@ -1260,6 +1292,16 @@
   return inferior_ptid;
 }
 
+/* to_stopped_by_watchpoint method */
+static int
+record_stopped_by_watchpoint (void)
+{
+  if (RECORD_IS_REPLAY)
+    return record_hw_watchpoint;
+  else
+    return record_beneath_to_stopped_by_watchpoint ();
+}
+
 /* "to_disconnect" method for process record target.  */
 
 static void
@@ -1587,6 +1629,7 @@
   /* Add bookmark target methods.  */
   record_ops.to_get_bookmark = record_get_bookmark;
   record_ops.to_goto_bookmark = record_goto_bookmark;
+  record_ops.to_stopped_by_watchpoint = record_stopped_by_watchpoint;
   record_ops.to_magic = OPS_MAGIC;
 }
 
@@ -1795,6 +1838,7 @@
   /* Add bookmark target methods.  */
   record_core_ops.to_get_bookmark = record_get_bookmark;
   record_core_ops.to_goto_bookmark = record_goto_bookmark;
+  record_core_ops.to_stopped_by_watchpoint = record_stopped_by_watchpoint;
   record_core_ops.to_magic = OPS_MAGIC;
 }
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]