This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[RFA] Fix hw watchpoints in process record.
- From: Michael Snyder <msnyder at vmware dot com>
- To: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>, Hui Zhu <teawater at gmail dot com>
- Date: Sat, 31 Oct 2009 18:15:27 -0700
- Subject: [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, ¬used);
+}
+
+/* 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;
}