This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] GDB patches for hw watchpoints - revised
On Tue, 24 Jan 2006, Daniel Jacobowitz wrote:
> On Tue, Jan 24, 2006 at 06:59:53PM +0800, Wu Zhou wrote:
> > Hi Daniel,
> >
> > Because there are quite a few places in different arch/target of GDB which
> > use region_size_ok_for_hw_watchpoint, so I am prefering to get this done
> > in two steps: first add to_region_ok_for_hw_watchpoint into struct
> > target_ops, then replace these to_region_size_ok_for_hw_watchpoint
> > reference with to_region_ok_for_hw_watchpoint ones. IMHO, it is easier to
> > not confuse the original intention of this patch with this replacement.
> > What is your thought on this?
>
> Sure. Couple small things but we're almost done.
Thanks! Here is the revised patch to address these small things.
OK to commit?
2006-01-22 Ben Elliston <bje@au1.ibm.com>
Wu Zhou <woodzltc@cn.ibm.com>
* ppc-linux-nat.c (PTRACE_GET_DEBUGREG, PTRACE_SET_DEBUGREG,
PTRACE_GETSIGINFO): Define.
(last_stopped_data_address): New.
(ppc_linux_check_watch_resources): New function.
(ppc_linux_region_ok_for_hw_watchpoint): New function.
(ppc_linux_insert_watchpoint): New function.
(ppc_linux_remove_watchpoint): New function.
(ppc_linux_stopped_data_address): New function.
(ppc_linux_stopped_by_watchpoint): New function.
(_initialize_ppc_linux_nat): Set the above hardware watchpoint
related target vectors.
* rs6000-tdep.c (rs6000_gdbarch_init): Set PPC architectures
to have nonsteppable watchpoint.
* target.c (default_region_ok_for_hw_watchpoint,
debug_to_region_ok_for_hw_watchpoint): New prototypes.
(update_current_target): Inherit to_region_ok_for_hw_watchpoint
and set default to_region_ok_for_hw_watchpoint.
(default_region_ok_for_hw_watchpoint): New function.
(debug_to_region_ok_for_hw_watchpoint): New function.
(setup_target_debug): Set to_region_ok_for_hw_watchpoint of
debug_target.
* target.h (struct target_ops): Add a new target vector
to_region_ok_for_hw_watchpoint.
(TARGET_REGION_OK_FOR_HW_WATCHPOINT): Define this if it is not
defined anyplace else.
Index: ppc-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-nat.c,v
retrieving revision 1.55
diff -u -p -r1.55 ppc-linux-nat.c
--- ppc-linux-nat.c 10 Sep 2005 18:11:04 -0000 1.55
+++ ppc-linux-nat.c 25 Jan 2006 02:20:49 -0000
@@ -81,6 +81,16 @@
#define PTRACE_SETEVRREGS 21
#endif
+/* Similarly for the hardware watchpoint support. */
+#ifndef PTRACE_GET_DEBUGREG
+#define PTRACE_GET_DEBUGREG 25
+#endif
+#ifndef PTRACE_SET_DEBUGREG
+#define PTRACE_SET_DEBUGREG 26
+#endif
+#ifndef PTRACE_GETSIGINFO
+#define PTRACE_GETSIGINFO 0x4202
+#endif
/* This oddity is because the Linux kernel defines elf_vrregset_t as
an array of 33 16 bytes long elements. I.e. it leaves out vrsave.
@@ -146,6 +156,7 @@ struct gdb_evrregset_t
error. */
int have_ptrace_getvrregs = 1;
+static CORE_ADDR last_stopped_data_address = 0;
/* Non-zero if our kernel may support the PTRACE_GETEVRREGS and
PTRACE_SETEVRREGS requests, for reading and writing the SPE
@@ -153,7 +164,6 @@ int have_ptrace_getvrregs = 1;
error. */
int have_ptrace_getsetevrregs = 1;
-
int
kernel_u_size (void)
{
@@ -777,6 +787,124 @@ store_ppc_registers (int tid)
store_spe_register (tid, -1);
}
+static int
+ppc_linux_check_watch_resources (int type, int cnt, int ot)
+{
+ int tid;
+ ptid_t ptid = inferior_ptid;
+
+ /* DABR (data address breakpoint register) is optional for PPC variants.
+ Some variants have one DABR, others have none. So CNT can't be larger
+ than 1. */
+ if (cnt > 1)
+ return 0;
+
+ /* We need to know whether ptrace supports PTRACE_SET_DEBUGREG and whether
+ the target has DABR. If either answer is no, the ptrace call will
+ return -1. Fail in that case. */
+ tid = TIDGET (ptid);
+ if (tid == 0)
+ tid = PIDGET (ptid);
+
+ if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
+ return 0;
+ return 1;
+}
+
+static int
+ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
+{
+ /* Handle sub-8-byte quantities. */
+ if (len <= 0)
+ return 0;
+
+ /* addr+len must fall in the 8 byte watchable region. */
+ if ((addr + len) > (addr & ~7) + 8)
+ return 0;
+
+ return 1;
+}
+
+/* Set a watchpoint of type TYPE at address ADDR. */
+static long
+ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+{
+ int tid;
+ long dabr_value;
+ ptid_t ptid = inferior_ptid;
+
+ dabr_value = addr & ~7;
+ switch (rw)
+ {
+ case hw_read:
+ /* Set read and translate bits. */
+ dabr_value |= 5;
+ break;
+ case hw_write:
+ /* Set write and translate bits. */
+ dabr_value |= 6;
+ break;
+ case hw_access:
+ /* Set read, write and translate bits. */
+ dabr_value |= 7;
+ break;
+ }
+
+ tid = TIDGET (ptid);
+ if (tid == 0)
+ tid = PIDGET (ptid);
+
+ return ptrace (PTRACE_SET_DEBUGREG, tid, 0, dabr_value);
+}
+
+static long
+ppc_linux_remove_watchpoint (CORE_ADDR addr, int len)
+{
+ int tid;
+ ptid_t ptid = inferior_ptid;
+
+ tid = TIDGET (ptid);
+ if (tid == 0)
+ tid = PIDGET (ptid);
+
+ return ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0);
+}
+
+static int
+ppc_linux_stopped_data_address (struct target_ops *target, CORE_ADDR *addr_p)
+{
+ if (last_stopped_data_address)
+ {
+ *addr_p = last_stopped_data_address;
+ last_stopped_data_address = 0;
+ return 1;
+ }
+ return 0;
+}
+
+static int
+ppc_linux_stopped_by_watchpoint (void)
+{
+ int tid;
+ struct siginfo siginfo;
+ ptid_t ptid = inferior_ptid;
+ CORE_ADDR *addr_p;
+
+ tid = TIDGET(ptid);
+ if (tid == 0)
+ tid = PIDGET (ptid);
+
+ errno = 0;
+ ptrace (PTRACE_GETSIGINFO, tid, (PTRACE_TYPE_ARG3) 0, &siginfo);
+
+ if (errno != 0 || siginfo.si_signo != SIGTRAP ||
+ (siginfo.si_code & 0xffff) != 0x0004)
+ return 0;
+
+ last_stopped_data_address = (CORE_ADDR) siginfo.si_addr;
+ return 1;
+}
+
static void
ppc_linux_store_inferior_registers (int regno)
{
@@ -900,6 +1028,14 @@ _initialize_ppc_linux_nat (void)
t->to_fetch_registers = ppc_linux_fetch_inferior_registers;
t->to_store_registers = ppc_linux_store_inferior_registers;
+ /* Add our watchpoint methods. */
+ t->to_can_use_hw_breakpoint = ppc_linux_check_watch_resources;
+ t->to_region_ok_for_hw_watchpoint = ppc_linux_region_ok_for_hw_watchpoint;
+ t->to_insert_watchpoint = ppc_linux_insert_watchpoint;
+ t->to_remove_watchpoint = ppc_linux_remove_watchpoint;
+ t->to_stopped_by_watchpoint = ppc_linux_stopped_by_watchpoint;
+ t->to_stopped_data_address = ppc_linux_stopped_data_address;
+
/* Register the target. */
add_target (t);
}
Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.248
diff -u -p -r1.248 rs6000-tdep.c
--- rs6000-tdep.c 1 Nov 2005 19:32:36 -0000 1.248
+++ rs6000-tdep.c 25 Jan 2006 02:20:51 -0000
@@ -3278,6 +3278,8 @@ rs6000_gdbarch_init (struct gdbarch_info
_("rs6000_gdbarch_init: "
"received unexpected BFD 'arch' value"));
+ set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 1);
+
/* Sanity check on registers. */
gdb_assert (strcmp (tdep->regs[tdep->ppc_gp0_regnum].name, "r0") == 0);
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.111
diff -u -p -r1.111 target.c
--- target.c 4 Sep 2005 16:18:20 -0000 1.111
+++ target.c 25 Jan 2006 02:20:51 -0000
@@ -47,6 +47,8 @@ static void kill_or_be_killed (int);
static void default_terminal_info (char *, int);
+static int default_region_ok_for_hw_watchpoint (CORE_ADDR, int);
+
static int default_region_size_ok_for_hw_watchpoint (int);
static int nosymbol (char *, CORE_ADDR *);
@@ -128,6 +130,8 @@ static int debug_to_stopped_by_watchpoin
static int debug_to_stopped_data_address (struct target_ops *, CORE_ADDR *);
+static int debug_to_region_ok_for_hw_watchpoint (CORE_ADDR, int);
+
static int debug_to_region_size_ok_for_hw_watchpoint (int);
static void debug_to_terminal_init (void);
@@ -405,6 +409,7 @@ update_current_target (void)
INHERIT (to_stopped_data_address, t);
INHERIT (to_stopped_by_watchpoint, t);
INHERIT (to_have_continuable_watchpoint, t);
+ INHERIT (to_region_ok_for_hw_watchpoint, t);
INHERIT (to_region_size_ok_for_hw_watchpoint, t);
INHERIT (to_terminal_init, t);
INHERIT (to_terminal_inferior, t);
@@ -531,6 +536,8 @@ update_current_target (void)
de_fault (to_stopped_data_address,
(int (*) (struct target_ops *, CORE_ADDR *))
return_zero);
+ de_fault (to_region_ok_for_hw_watchpoint,
+ default_region_ok_for_hw_watchpoint);
de_fault (to_region_size_ok_for_hw_watchpoint,
default_region_size_ok_for_hw_watchpoint);
de_fault (to_terminal_init,
@@ -1575,6 +1582,12 @@ find_default_create_inferior (char *exec
}
static int
+default_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
+{
+ return TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT (len);
+}
+
+static int
default_region_size_ok_for_hw_watchpoint (int byte_count)
{
return (byte_count <= TYPE_LENGTH (builtin_type_void_data_ptr));
@@ -2115,6 +2128,21 @@ debug_to_can_use_hw_breakpoint (int type
}
static int
+debug_to_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
+{
+ CORE_ADDR retval;
+
+ retval = debug_target.to_region_ok_for_hw_watchpoint (addr, len);
+
+ fprintf_unfiltered (gdb_stdlog,
+ "TARGET_REGION_OK_FOR_HW_WATCHPOINT (%ld, %ld) = 0x%lx\n",
+ (unsigned long) addr,
+ (unsigned long) len,
+ (unsigned long) retval);
+ return retval;
+}
+
+static int
debug_to_region_size_ok_for_hw_watchpoint (int byte_count)
{
CORE_ADDR retval;
@@ -2533,6 +2561,7 @@ setup_target_debug (void)
current_target.to_remove_watchpoint = debug_to_remove_watchpoint;
current_target.to_stopped_by_watchpoint = debug_to_stopped_by_watchpoint;
current_target.to_stopped_data_address = debug_to_stopped_data_address;
+ current_target.to_region_ok_for_hw_watchpoint = debug_to_region_ok_for_hw_watchpoint;
current_target.to_region_size_ok_for_hw_watchpoint = debug_to_region_size_ok_for_hw_watchpoint;
current_target.to_terminal_init = debug_to_terminal_init;
current_target.to_terminal_inferior = debug_to_terminal_inferior;
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.76
diff -u -p -r1.76 target.h
--- target.h 4 Sep 2005 16:18:20 -0000 1.76
+++ target.h 25 Jan 2006 02:20:51 -0000
@@ -345,6 +345,7 @@ struct target_ops
int (*to_stopped_by_watchpoint) (void);
int to_have_continuable_watchpoint;
int (*to_stopped_data_address) (struct target_ops *, CORE_ADDR *);
+ int (*to_region_ok_for_hw_watchpoint) (CORE_ADDR, int);
int (*to_region_size_ok_for_hw_watchpoint) (int);
void (*to_terminal_init) (void);
void (*to_terminal_inferior) (void);
@@ -1030,6 +1031,11 @@ extern void (*deprecated_target_new_objf
(*current_target.to_can_use_hw_breakpoint) (TYPE, CNT, OTHERTYPE);
#endif
+#ifndef TARGET_REGION_OK_FOR_HW_WATCHPOINT
+#define TARGET_REGION_OK_FOR_HW_WATCHPOINT(addr, len) \
+ (*current_target.to_region_ok_for_hw_watchpoint) (addr, len)
+#endif
+
#if !defined(TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT)
#define TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(byte_count) \
(*current_target.to_region_size_ok_for_hw_watchpoint) (byte_count)
Regards
- Wu Zhou
>
> > (ppc_linux_check_watch_resources): New function to check whether
> > the target has available hardware watchpoint resource.
> > (ppc_linux_region_ok_for_hw_watchpoint): New function to check if
> > the region is ok for hardware watchpoint.
> > (ppc_linux_insert_watchpoint): New function to insert a hardware
> > watchpoint.
> > (ppc_linux_remove_watchpoint): New function to remove a hardware
> > watchpoint.
> > (ppc_linux_stopped_data_address): New function to get the stopped
> > data address of the watchpoint.
> > (ppc_linux_stopped_by_watchpoint): New function to see if the
> > inferior is stopped by watchpoint.
>
> "New function" is sufficient; if you feel that the function needs an
> explanation, it should go in the code, not in the changelog.
>
> > target.h (struct target_ops): Add a new target vector
>
> Missed a "* " there.
>
> > + /* P630 has nonsteppable watchpoint. So we are assuming that all PowerPC
> > + targets have nonsteppable watchpoint. */
> > + set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 1);
>
> Please skip this comment.
>
> > @@ -1575,6 +1582,12 @@ find_default_create_inferior (char *exec
> > }
> >
> > static int
> > +default_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
> > +{
> > + return (len <= TYPE_LENGTH (builtin_type_void_data_ptr));
> > +}
>
> TARGET_REGION_OK_FOR_HW_WATCHPOINT will now always be defined, because
> of the #ifdef in target.h. Therefore this won't trigger (from
> breakpoint.c):
>
> #if !defined(TARGET_REGION_OK_FOR_HW_WATCHPOINT)
> #define TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR,LEN) \
> (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(LEN))
> #endif
>
> So you need to call TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT (len) here,
> in case the target has overridden that.
>
>
> --
> Daniel Jacobowitz
> CodeSourcery
>
>