This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Fix hw watchpoints in process record.
I've applied this slightly updated patch to fix hardware watchpoints.
We were missing a target_stopped_data_address method in precord, plus,
address space matches in the new breakpoint.c method.
Michael, I left out the new NEWS entry, as I didn't see Eli's OK
on that. I also left out the new tests, as I wasn't sure you
had updated versions of those or not. I'll let you handle them.
I'll apply a patch to fix hardware breakpoints next.
--
Pedro Alves
2009-11-22 Pedro Alves <pedro@codesourcery.com>
Michael Snyder <msnyder@vmware.com>
Make hardware watchpoints work for process record.
* breakpoint.c (hardware_watchpoint_inserted_in_range): New.
* breakpoint.h (hardware_watchpoint_inserted_in_range): Declare.
* record.c (record_beneath_to_stopped_by_watchpoint)
(record_beneath_to_stopped_data_address, record_hw_watchpoint):
New globals.
(record_exec_insn): Check for watchpoint hits.
(tmp_to_stopped_by_watchpoint, tmp_to_stopped_data_address): New
globals.
(record_open): Set tmp_to_stopped_by_watchpoint,
tmp_to_stopped_data_address,
record_beneath_to_stopped_by_watchpoint and
record_beneath_to_stopped_data_address.
(record_wait): Report watchpoint hits to the core. Update and
extend comments.
(record_stopped_by_watchpoint): New.
(record_stopped_data_address): New.
(init_record_ops): Install them.
(init_record_core_ops): Ditto.
---
gdb/NEWS | 4 +
gdb/breakpoint.c | 32 ++++++++++
gdb/breakpoint.h | 6 +
gdb/record.c | 83 ++++++++++++++++++++++++---
gdb/testsuite/gdb.reverse/watch-precsave.exp | 80 +++++++++++++++++++++++++-
gdb/testsuite/gdb.reverse/watch-reverse.exp | 82 ++++++++++++++++++++++++++
6 files changed, 275 insertions(+), 12 deletions(-)
Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c 2009-11-22 14:48:48.000000000 +0000
+++ src/gdb/breakpoint.c 2009-11-22 15:29:10.000000000 +0000
@@ -2380,6 +2380,38 @@ software_breakpoint_inserted_here_p (str
return 0;
}
+int
+hardware_watchpoint_inserted_in_range (struct address_space *aspace,
+ CORE_ADDR addr, ULONGEST len)
+{
+ struct breakpoint *bpt;
+
+ ALL_BREAKPOINTS (bpt)
+ {
+ struct bp_location *loc;
+
+ if (bpt->type != bp_hardware_watchpoint
+ && bpt->type != bp_access_watchpoint)
+ continue;
+
+ if (!breakpoint_enabled (bpt))
+ continue;
+
+ for (loc = bpt->loc; loc; loc = loc->next)
+ if (loc->pspace->aspace == aspace && loc->inserted)
+ {
+ CORE_ADDR l, h;
+
+ /* Check for intersection. */
+ l = max (loc->address, addr);
+ h = min (loc->address + loc->length, addr + len);
+ if (l < h)
+ return 1;
+ }
+ }
+ return 0;
+}
+
/* breakpoint_thread_match (PC, PTID) returns true if the breakpoint at
PC is valid for process/thread PTID. */
Index: src/gdb/breakpoint.h
===================================================================
--- src.orig/gdb/breakpoint.h 2009-11-22 14:48:48.000000000 +0000
+++ src/gdb/breakpoint.h 2009-11-22 15:02:43.000000000 +0000
@@ -736,6 +736,12 @@ extern int regular_breakpoint_inserted_h
extern int software_breakpoint_inserted_here_p (struct address_space *, CORE_ADDR);
+/* Returns true if there's a hardware watchpoint or access watchpoint
+ inserted in the range defined by ADDR and LEN. */
+extern int hardware_watchpoint_inserted_in_range (struct address_space *,
+ CORE_ADDR addr,
+ ULONGEST len);
+
extern int breakpoint_thread_match (struct address_space *, CORE_ADDR, ptid_t);
extern void until_break_command (char *, int, int);
Index: src/gdb/record.c
===================================================================
--- src.orig/gdb/record.c 2009-11-22 14:49:45.000000000 +0000
+++ src/gdb/record.c 2009-11-22 15:26:45.000000000 +0000
@@ -224,6 +224,9 @@ static int (*record_beneath_to_insert_br
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);
+static int (*record_beneath_to_stopped_data_address) (struct target_ops *,
+ CORE_ADDR *);
/* Alloc and free functions for record_reg, record_mem, and record_end
entries. */
@@ -673,6 +676,9 @@ record_gdb_operation_disable_set (void)
return old_cleanups;
}
+/* Flag set to TRUE for target_stopped_by_watchpoint. */
+static int record_hw_watchpoint = 0;
+
/* Execute one instruction from the record log. Each instruction in
the log will be represented by an arbitrary sequence of register
entries and memory entries, followed by an 'end' entry. */
@@ -739,7 +745,22 @@ record_exec_insn (struct regcache *regca
entry->u.mem.len);
}
else
- memcpy (record_get_loc (entry), mem, entry->u.mem.len);
+ {
+ memcpy (record_get_loc (entry), mem, entry->u.mem.len);
+
+ /* We've changed memory --- check if a hardware
+ watchpoint should trap. Note that this
+ presently assumes the target beneath supports
+ continuable watchpoints. On non-continuable
+ watchpoints target, we'll want to check this
+ _before_ actually doing the memory change, and
+ not doing the change at all if the watchpoint
+ traps. */
+ if (hardware_watchpoint_inserted_in_range
+ (get_regcache_aspace (regcache),
+ entry->u.mem.addr, entry->u.mem.len))
+ record_hw_watchpoint = 1;
+ }
}
}
}
@@ -770,6 +791,8 @@ static int (*tmp_to_insert_breakpoint) (
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 int (*tmp_to_stopped_data_address) (struct target_ops *, CORE_ADDR *);
static void record_restore (void);
@@ -894,6 +917,10 @@ record_open (char *name, int from_tty)
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_stopped_data_address)
+ tmp_to_stopped_data_address = t->to_stopped_data_address;
}
if (!tmp_to_xfer_partial)
error (_("Could not find 'to_xfer_partial' method on the target stack."));
@@ -915,6 +942,8 @@ record_open (char *name, int from_tty)
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;
+ record_beneath_to_stopped_data_address = tmp_to_stopped_data_address;
if (current_target.to_stratum == core_stratum)
record_core_open_1 (name, from_tty);
@@ -1069,14 +1098,23 @@ record_wait (struct target_ops *ops,
{
struct regcache *regcache;
- /* Yes -- check if there is a breakpoint. */
+ /* Yes -- this is likely our single-step finishing,
+ but check if there's any reason the core would be
+ interested in the event. */
+
registers_changed ();
regcache = get_current_regcache ();
tmp_pc = regcache_read_pc (regcache);
- if (breakpoint_inserted_here_p (get_regcache_aspace (regcache),
- tmp_pc))
+
+ if (target_stopped_by_watchpoint ())
{
- /* There is a breakpoint. GDB will want to stop. */
+ /* Always interested in watchpoints. */
+ }
+ else if (breakpoint_inserted_here_p (get_regcache_aspace (regcache),
+ tmp_pc))
+ {
+ /* There is a breakpoint here. Let the core
+ handle it. */
struct gdbarch *gdbarch = get_regcache_arch (regcache);
CORE_ADDR decr_pc_after_break
= gdbarch_decr_pc_after_break (gdbarch);
@@ -1086,10 +1124,8 @@ record_wait (struct target_ops *ops,
}
else
{
- /* There is not a breakpoint, and gdb is not
- stepping, therefore gdb will not stop.
- Therefore we will not return to gdb.
- Record the insn and resume. */
+ /* This must be a single-step trap. Record the
+ insn and issue another step. */
if (!do_record_message (regcache, TARGET_SIGNAL_0))
break;
@@ -1116,6 +1152,7 @@ record_wait (struct target_ops *ops,
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. */
@@ -1219,6 +1256,14 @@ record_wait (struct target_ops *ops,
gdbarch_decr_pc_after_break (gdbarch));
continue_flag = 0;
}
+
+ if (record_hw_watchpoint)
+ {
+ if (record_debug)
+ fprintf_unfiltered (gdb_stdlog,
+ "Process record: hit hw watchpoint.\n");
+ continue_flag = 0;
+ }
/* Check target signal */
if (record_list->u.end.sigval != TARGET_SIGNAL_0)
/* FIXME: better way to check */
@@ -1260,6 +1305,24 @@ replay_out:
return inferior_ptid;
}
+static int
+record_stopped_by_watchpoint (void)
+{
+ if (RECORD_IS_REPLAY)
+ return record_hw_watchpoint;
+ else
+ return record_beneath_to_stopped_by_watchpoint ();
+}
+
+static int
+record_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
+{
+ if (RECORD_IS_REPLAY)
+ return 0;
+ else
+ return record_beneath_to_stopped_data_address (ops, addr_p);
+}
+
/* "to_disconnect" method for process record target. */
static void
@@ -1594,6 +1657,7 @@ init_record_ops (void)
record_ops.to_xfer_partial = record_xfer_partial;
record_ops.to_insert_breakpoint = record_insert_breakpoint;
record_ops.to_remove_breakpoint = record_remove_breakpoint;
+ record_ops.to_stopped_by_watchpoint = record_stopped_by_watchpoint;
record_ops.to_can_execute_reverse = record_can_execute_reverse;
record_ops.to_stratum = record_stratum;
/* Add bookmark target methods. */
@@ -1801,6 +1865,7 @@ init_record_core_ops (void)
record_core_ops.to_xfer_partial = record_core_xfer_partial;
record_core_ops.to_insert_breakpoint = record_core_insert_breakpoint;
record_core_ops.to_remove_breakpoint = record_core_remove_breakpoint;
+ record_core_ops.to_stopped_by_watchpoint = record_stopped_by_watchpoint;
record_core_ops.to_can_execute_reverse = record_can_execute_reverse;
record_core_ops.to_has_execution = record_core_has_execution;
record_core_ops.to_stratum = record_stratum;