Re: [RFC] GDB patches for hw watchpoints - revised

Wu Zhou
2005-12-22T04:46:45Z

On Thu, 22 Dec 2005, Eli Zaretskii wrote:

> > Date: Thu, 22 Dec 2005 12:05:47 +0800 (CST)
> > From: Wu Zhou <woodzltc@cn.ibm.com>
> > cc: drow@false.org, mark@xs4all.nl, bje@au1.ibm.com, anton@au1.ibm.com
> >
> > Here is the reworked patch for latest 6.4 tree. Any comments? Thanks a
> > lot!
>
> I don't see in your patch where last_stopped_data_address is reset to
> zero. It seems like it always stores the last address, so GDB might
> think a watchpoint was hit where in fact it wasn't.
>
> Am I missing something?

Sorry, it is me who missed something. :-)

After we get stopped data address, we can set it back to 0, because it is
not used any more if not hit another time by a h/w watchpoint. Did it
make sense?

Here is the modified patch:

2005-12-22 Ben Elliston <bje@au1.ibm.com>
Wu Zhou <woodzltc@cn.ibm.com>

* rs6000-tdep.c (rs6000_gdbarch_init): If the macn is p630, set
gdbarch to have nonsteppable watchpoint.
* ppc-linux-nat.c: Define three macro: PTRACE_GET_DEBUGREG,
PTRACE_SET_DEBUGREG and PTRACE_GETSIGINFO. Define one static
variable last_stopped_data_address.
(ppc_linux_check_watch_resources): New function to check whether
the target have available hardware watchpoint resource.
(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.
(_initialize_ppc_linux_nat): Set the above hardware watchpoint
related target vectors.

Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.248
diff -c -3 -p -r1.248 rs6000-tdep.c
*** rs6000-tdep.c 1 Nov 2005 19:32:36 -0000 1.248
--- rs6000-tdep.c 22 Dec 2005 04:39:40 -0000
*************** rs6000_gdbarch_init (struct gdbarch_info
*** 3278,3283 ****
--- 3278,3291 ----
_("rs6000_gdbarch_init: "
"received unexpected BFD 'arch' value"));

+ /* If the MACH is p630, set have_nonsteppable_watchpoint to 1.
+
+ FIXME: not quite sure if all ppc64 mach support nonsteppable watchpoint.
+ But p630 do support nonsteppable h/w watchpoint. */
+
+ if (bfd_mach_ppc_630)
+ set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 1);
+
/* Sanity check on registers. */
gdb_assert (strcmp (tdep->regs[tdep->ppc_gp0_regnum].name, "r0") == 0);

Index: ppc-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-nat.c,v
retrieving revision 1.55
diff -c -3 -p -r1.55 ppc-linux-nat.c
*** ppc-linux-nat.c 10 Sep 2005 18:11:04 -0000 1.55
--- ppc-linux-nat.c 22 Dec 2005 04:39:40 -0000
***************
*** 81,86 ****
--- 81,96 ----
#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.
*************** struct gdb_evrregset_t
*** 146,151 ****
--- 156,162 ----
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
*************** int have_ptrace_getvrregs = 1;
*** 153,159 ****
error. */
int have_ptrace_getsetevrregs = 1;

-
int
kernel_u_size (void)
{
--- 164,169 ----
*************** store_ppc_registers (int tid)
*** 777,782 ****
--- 787,932 ----
store_spe_register (tid, -1);
}

+ 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 variations.
+ Some variation 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 syscall support PTRACE_SET_DEBUGREG and
+ whether the ppc arch have DABR. If either answer is no, the ptrace call
+ will return -1. Return 0 for that. */
+ 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_size_ok_for_hw_watchpoint (int cnt)
+ {
+ return 1;
+ }
+
+ /* Set a watchpoint of type TYPE at address ADDR. */
+ long
+ ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+ {
+ int tid;
+ long dabr_value;
+ ptid_t ptid = inferior_ptid;
+
+ /* Handle sub-8-byte quantities. */
+ if (len <= 0)
+ return -1;
+
+ /* addr+len must fall in the 8 byte watchable region. */
+ if ((addr + len) > (addr & ~7) + 8)
+ return -1;
+
+ 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);
+ }
+
+ 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);
+ }
+
+ 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;
+ }
+
+ /*
+ int
+ ppc_linux_stopped_data_address (struct target_ops *target, CORE_ADDR *addr_p)
+ {
+ int tid;
+ struct siginfo siginfo;
+ ptid_t ptid = inferior_ptid;
+
+ tid = TIDGET (ptid);
+ if (tid == 0)
+ tid = PIDGET (ptid);
+
+ errno = 0;
+ ptrace (PTRACE_GET_DEBUGREG, tid, (PTRACE_TYPE_ARG3) 0, addr_p);
+ if (errno)
+ return 0;
+
+ *addr_p = *addr_p & ~7;
+ return 1;
+ }
+ */
+
+ 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)
{
*************** _initialize_ppc_linux_nat (void)
*** 900,905 ****
--- 1050,1063 ----
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_size_ok_for_hw_watchpoint = ppc_linux_region_size_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);
}

Regards
- Wu Zhou