[RFA]: Change to_stopped_data_address ABI

Jeff Johnston
2004-08-31T18:55:16Z

[-- Attachment #1: Type: text/plain, Size: 3180 bytes --]

I am proposing a change to the to_stopped_data_address target vector function.
There are two reasons. The first reason is that there is no way to determine if
a platform actually supports to_stopped_data_address. For example, the S390
supports hardware watchpoints, but doesn't support figuring out which address
caused a watchpoint to trigger. This level of detail is necessary for proper
threaded watchpoint support to know whether to trust in the
to_stopped_data_address function or whether to check all watchpoints for value
changes.

The second reason for the proposed change is that there is no way to watch
address 0 since to_stopped_data_address currently returns the address 0 to
indicate failure.

The proposed change is to change the prototype to be:

int
to_stopped_data_address (CORE_ADDR *addr_p);

If the input pointer is NULL, the function returns non-zero if it works on the
given target, otherwise, it fails by returning 0. The function also returns 0
if unsuccessful. By separating out the success/fail code from the address, the
new prototype allows for succeeding and returning any address, including 0.

Ok to commit?

-- Jeff J.

2004-08-31 Jeff Johnston <jjohnstn@redhat.com>

* target.h (to_stopped_data_address): Change prototype to
take a CORE_ADDR pointer and return an int.
* target.c (update_current_target): Change to_stopped_data_address
to match new prototype.
(debug_to_stopped_data_address): Change appropriately.
* breakpoint.c (bpstat_stop_status): Change call to
target_stopped_data_address to use new prototype.
* frv-tdep.c (frv_have_stopped_data_address): New function.
(frv_stopped_data_address): Change to new prototype and
functionality.
* ia64-linux-nat.c (ia64_stopped_data_address): Change to new
prototype and functionality.
(ia64_stopped_by_watchpoint): New function.
* i386-nat.c (i386_stopped_data_address): Change to new
prototype and functionality.
(i386_stopped_by_watchpoint): New function.
* remote.c (remote_stopped_data_address): Change to new prototype
and functionality.
* remote-m32r-sdi.c (m32r_stopped_data_address): Ditto.
* config/frv/tm-frv.h (frv_stopped_data_address): Change prototype.
(STOPPED_BY_WATCHPOINT): Change to use frv_have_stopped_data_address.
* config/i386/nm-i386.h (STOPPED_BY_WATCHPOINT): Change to use
new i386_stopped_by_watchpoint function.
(i386_stopped_by_watchpoint): New prototype.
(i386_stoppped_data_address): Change to new prototype.
* config/ia64/nm-linux.h (STOPPED_BY_WATCHPOINT): Change to use
new ia64_stopped_by_watchpoint function.
(ia64_stopped_by_watchpoint): New prototype.
(ia64_stopped_data_address): Ditto.

doc/ChangeLog:

2004-08-31 Jeff Johnston <jjohnstn@redhat.com>

* gdbint.texinfo (target_stopped_data_address): Update to
new prototype.
(i386_stopped_data_address): Update prototype and description.
(i386_stopped_by_watchpoint): New function and description.

[-- Attachment #2: sda.patch --]
[-- Type: text/plain, Size: 12514 bytes --]

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.179
diff -u -p -r1.179 breakpoint.c
--- breakpoint.c 28 Jul 2004 17:26:26 -0000 1.179
+++ breakpoint.c 31 Aug 2004 18:49:50 -0000
@@ -2739,8 +2739,7 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
struct value *v;
int found = 0;

- addr = target_stopped_data_address ();
- if (addr == 0)
+ if (!target_stopped_data_address (&addr))
continue;
for (v = b->val_chain; v; v = v->next)
{
Index: frv-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/frv-tdep.c,v
retrieving revision 1.89
diff -u -p -r1.89 frv-tdep.c
--- frv-tdep.c 2 Aug 2004 19:44:39 -0000 1.89
+++ frv-tdep.c 31 Aug 2004 18:49:50 -0000
@@ -1293,11 +1293,14 @@ frv_check_watch_resources (int type, int
}

-CORE_ADDR
-frv_stopped_data_address (void)
+int
+frv_stopped_data_address (CORE_ADDR *addr_p)
{
CORE_ADDR brr, dbar0, dbar1, dbar2, dbar3;

+ if (addr_p == NULL)
+ return 1;
+
brr = read_register (brr_regnum);
dbar0 = read_register (dbar0_regnum);
dbar1 = read_register (dbar1_regnum);
@@ -1305,15 +1308,25 @@ frv_stopped_data_address (void)
dbar3 = read_register (dbar3_regnum);

if (brr & (1<<11))
- return dbar0;
+ *addr_p = dbar0;
else if (brr & (1<<10))
- return dbar1;
+ *addr_p = dbar1;
else if (brr & (1<<9))
- return dbar2;
+ *addr_p = dbar2;
else if (brr & (1<<8))
- return dbar3;
+ *addr_p = dbar3;
else
- return 0;
+ *addr_p = 0;
+
+ return 1;
+}
+
+int
+frv_have_stopped_data_address (void)
+{
+ CORE_ADDR addr = 0;
+ int retval = frv_stopped_data_address (&addr);
+ return addr != (CORE_ADDR)0;
}

static CORE_ADDR
Index: i386-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-nat.c,v
retrieving revision 1.8
diff -u -p -r1.8 i386-nat.c
--- i386-nat.c 5 Mar 2004 20:58:00 -0000 1.8
+++ i386-nat.c 31 Aug 2004 18:49:50 -0000
@@ -564,15 +564,20 @@ i386_region_ok_for_watchpoint (CORE_ADDR
return nregs <= DR_NADDR ? 1 : 0;
}

-/* If the inferior has some watchpoint that triggered, return the
- address associated with that watchpoint. Otherwise, return zero. */
+/* If the inferior has some watchpoint that triggered, set the
+ address associated with that watchpoint. Otherwise, set the address
+ pointed to by ADDR_P to zero. If ADDR_P is NULL, simply return
+ true to indicate the function works. */

-CORE_ADDR
-i386_stopped_data_address (void)
+int
+i386_stopped_data_address (CORE_ADDR *addr_p)
{
CORE_ADDR addr = 0;
int i;

+ if (addr_p == NULL)
+ return 1;
+
dr_status_mirror = I386_DR_LOW_GET_STATUS ();

ALL_DEBUG_REGISTERS(i)
@@ -593,7 +598,16 @@ i386_stopped_data_address (void)
if (maint_show_dr && addr == 0)
i386_show_dr ("stopped_data_addr", 0, 0, hw_write);

- return addr;
+ *addr_p = addr;
+ return 1;
+}
+
+int
+i386_stopped_by_watchpoint (void)
+{
+ CORE_ADDR addr = 0;
+ i386_stopped_data_address (&addr);
+ return addr != 0;
}

/* Return non-zero if the inferior has some break/watchpoint that
Index: ia64-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/ia64-linux-nat.c,v
retrieving revision 1.24
diff -u -p -r1.24 ia64-linux-nat.c
--- ia64-linux-nat.c 22 Aug 2004 16:32:35 -0000 1.24
+++ ia64-linux-nat.c 31 Aug 2004 18:49:51 -0000
@@ -636,13 +636,16 @@ ia64_linux_remove_watchpoint (ptid_t pti
return -1;
}

-CORE_ADDR
-ia64_linux_stopped_by_watchpoint (ptid_t ptid)
+int
+ia64_linux_stopped_data_address (CORE_ADDR *addr_p)
{
CORE_ADDR psr;
int tid;
struct siginfo siginfo;

+ if (addr_p == NULL)
+ return 1;
+
tid = TIDGET(ptid);
if (tid == 0)
tid = PIDGET (ptid);
@@ -652,14 +655,26 @@ ia64_linux_stopped_by_watchpoint (ptid_t

if (errno != 0 || siginfo.si_signo != SIGTRAP ||
(siginfo.si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */)
- return 0;
+ {
+ *addr_p = 0;
+ return 1;
+ }

psr = read_register_pid (IA64_PSR_REGNUM, ptid);
psr |= IA64_PSR_DD; /* Set the dd bit - this will disable the watchpoint
for the next instruction */
write_register_pid (IA64_PSR_REGNUM, psr, ptid);

- return (CORE_ADDR) siginfo.si_addr;
+ *addr_p = (CORE_ADDR)siginfo.si_addr;
+ return 1;
+}
+
+int
+ia64_linux_stopped_by_watchpoint (void)
+{
+ CORE_ADDR addr;
+ ia64_linux_stopped_data_address (&addr);
+ return addr != 0;
}

LONGEST
Index: remote-m32r-sdi.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-m32r-sdi.c,v
retrieving revision 1.7
diff -u -p -r1.7 remote-m32r-sdi.c
--- remote-m32r-sdi.c 27 Jul 2004 01:00:42 -0000 1.7
+++ remote-m32r-sdi.c 31 Aug 2004 18:49:51 -0000
@@ -1450,10 +1450,12 @@ m32r_remove_watchpoint (CORE_ADDR addr,
return 0;
}

-CORE_ADDR
-m32r_stopped_data_address (void)
+int
+m32r_stopped_data_address (CORE_ADDR *addr_p)
{
- return hit_watchpoint_addr;
+ if (addr_p != NULL)
+ *addr_p = hit_watchpoint_addr;
+ return 1;
}

int
Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.145
diff -u -p -r1.145 remote.c
--- remote.c 3 Aug 2004 02:02:23 -0000 1.145
+++ remote.c 31 Aug 2004 18:49:51 -0000
@@ -4554,13 +4554,18 @@ remote_stopped_by_watchpoint (void)

extern int stepped_after_stopped_by_watchpoint;

-static CORE_ADDR
-remote_stopped_data_address (void)
+static int
+remote_stopped_data_address (CORE_ADDR *addr_p)
{
+ if (addr_p == NULL)
+ return 1;
+
if (remote_stopped_by_watchpoint ()
|| stepped_after_stopped_by_watchpoint)
- return remote_watch_data_address;
- return (CORE_ADDR)0;
+ *addr_p = remote_watch_data_address;
+ else *addr_p = (CORE_ADDR)0;
+
+ return 1;
}

Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.78
diff -u -p -r1.78 target.c
--- target.c 3 Aug 2004 00:57:26 -0000 1.78
+++ target.c 31 Aug 2004 18:49:51 -0000
@@ -127,7 +127,7 @@ static int debug_to_remove_watchpoint (C

static int debug_to_stopped_by_watchpoint (void);

-static CORE_ADDR debug_to_stopped_data_address (void);
+static int debug_to_stopped_data_address (CORE_ADDR *);

static int debug_to_region_size_ok_for_hw_watchpoint (int);

@@ -522,7 +522,7 @@ update_current_target (void)
(int (*) (void))
return_zero);
de_fault (to_stopped_data_address,
- (CORE_ADDR (*) (void))
+ (int (*) (CORE_ADDR *))
return_zero);
de_fault (to_region_size_ok_for_hw_watchpoint,
default_region_size_ok_for_hw_watchpoint);
@@ -1919,16 +1919,22 @@ debug_to_stopped_by_watchpoint (void)
return retval;
}

-static CORE_ADDR
-debug_to_stopped_data_address (void)
+static int
+debug_to_stopped_data_address (CORE_ADDR *addr)
{
- CORE_ADDR retval;
+ int retval;

- retval = debug_target.to_stopped_data_address ();
+ retval = debug_target.to_stopped_data_address (addr);

- fprintf_unfiltered (gdb_stdlog,
- "target_stopped_data_address () = 0x%lx\n",
- (unsigned long) retval);
+ if (addr == NULL)
+ fprintf_unfiltered (gdb_stdlog,
+ "target_stopped_data_address (NULL) = %d\n",
+ retval);
+ else
+ fprintf_unfiltered (gdb_stdlog,
+ "target_stopped_data_address ([0x%lx]) = %ld\n",
+ (unsigned long)*addr,
+ (unsigned long)retval);
return retval;
}

Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.60
diff -u -p -r1.60 target.h
--- target.h 7 Jun 2004 17:58:32 -0000 1.60
+++ target.h 31 Aug 2004 18:49:51 -0000
@@ -342,7 +342,7 @@ struct target_ops
int (*to_insert_watchpoint) (CORE_ADDR, int, int);
int (*to_stopped_by_watchpoint) (void);
int to_have_continuable_watchpoint;
- CORE_ADDR (*to_stopped_data_address) (void);
+ int (*to_stopped_data_address) (CORE_ADDR *);
int (*to_region_size_ok_for_hw_watchpoint) (int);
void (*to_terminal_init) (void);
void (*to_terminal_inferior) (void);
@@ -1084,8 +1084,8 @@ extern void (*deprecated_target_new_objf
#endif

#ifndef target_stopped_data_address
-#define target_stopped_data_address() \
- (*current_target.to_stopped_data_address) ()
+#define target_stopped_data_address(x) \
+ (*current_target.to_stopped_data_address) (x)
#endif

/* This will only be defined by a target that supports catching vfork events,
Index: config/i386/nm-i386.h
===================================================================
RCS file: /cvs/src/src/gdb/config/i386/nm-i386.h,v
retrieving revision 1.5
diff -u -p -r1.5 nm-i386.h
--- config/i386/nm-i386.h 17 Jan 2004 21:56:12 -0000 1.5
+++ config/i386/nm-i386.h 31 Aug 2004 18:49:51 -0000
@@ -51,10 +51,14 @@ extern int i386_region_ok_for_watchpoint
triggered. */
extern int i386_stopped_by_hwbp (void);

-/* If the inferior has some break/watchpoint that triggered, return
+/* Return non-zero if the inferior has been stopped by a triggered
+ watchpoint. */
+extern int i386_stopped_by_watchpoint (void);
+
+/* If the inferior has some break/watchpoint that triggered, set
the address associated with that break/watchpoint. Otherwise,
- return zero. */
-extern CORE_ADDR i386_stopped_data_address (void);
+ set the watchpoint address to zero. Always return true. */
+extern int i386_stopped_data_address (CORE_ADDR *);

/* Insert a hardware-assisted breakpoint at address ADDR. SHADOW is
unused. Return 0 on success, EBUSY on failure. */
@@ -95,9 +99,9 @@ extern int i386_remove_hw_breakpoint (C

#define HAVE_CONTINUABLE_WATCHPOINT 1

-#define STOPPED_BY_WATCHPOINT(W) (i386_stopped_data_address () != 0)
+#define STOPPED_BY_WATCHPOINT(W) (i386_stopped_by_watchpoint () != 0)

-#define target_stopped_data_address() i386_stopped_data_address ()
+#define target_stopped_data_address(x) i386_stopped_data_address(x)

/* Use these macros for watchpoint insertion/removal. */

Index: config/frv/tm-frv.h
===================================================================
RCS file: /cvs/src/src/gdb/config/frv/tm-frv.h,v
retrieving revision 1.5
diff -u -p -r1.5 tm-frv.h
--- config/frv/tm-frv.h 1 Aug 2004 14:37:01 -0000 1.5
+++ config/frv/tm-frv.h 31 Aug 2004 18:49:51 -0000
@@ -1,5 +1,5 @@
/* Target definitions for the Fujitsu FR-V, for GDB, the GNU Debugger.
- Copyright 2000 Free Software Foundation, Inc.
+ Copyright 2000, 2004 Free Software Foundation, Inc.

This file is part of GDB.

@@ -35,10 +35,11 @@ extern int frv_check_watch_resources (in
#define STOPPED_BY_WATCHPOINT(W) \
((W).kind == TARGET_WAITKIND_STOPPED \
&& (W).value.sig == TARGET_SIGNAL_TRAP \
- && (frv_stopped_data_address() != ((CORE_ADDR)0)))
-extern CORE_ADDR frv_stopped_data_address(void);
+ && frv_have_stopped_data_address())
+extern int frv_have_stopped_data_address(void);

/* Use these macros for watchpoint insertion/deletion. */
-#define target_stopped_data_address() frv_stopped_data_address()
+#define target_stopped_data_address(x) frv_stopped_data_address(x)
+extern int frv_stopped_data_address(CORE_ADDR *addr_p);

#include "solib.h" /* Include support for shared libraries. */
Index: config/ia64/nm-linux.h
===================================================================
RCS file: /cvs/src/src/gdb/config/ia64/nm-linux.h,v
retrieving revision 1.14
diff -u -p -r1.14 nm-linux.h
--- config/ia64/nm-linux.h 22 Aug 2004 16:32:35 -0000 1.14
+++ config/ia64/nm-linux.h 31 Aug 2004 18:49:51 -0000
@@ -1,5 +1,4 @@
/* Native support for GNU/Linux, for GDB, the GNU debugger.
-
Copyright 1999, 2000, 2001, 2004
Free Software Foundation, Inc.

@@ -60,8 +59,11 @@ extern int ia64_cannot_store_register (i
#define HAVE_STEPPABLE_WATCHPOINT 1

#define STOPPED_BY_WATCHPOINT(W) \
- ia64_linux_stopped_by_watchpoint (inferior_ptid)
-extern CORE_ADDR ia64_linux_stopped_by_watchpoint (ptid_t ptid);
+ ia64_linux_stopped_by_watchpoint ()
+extern int ia64_linux_stopped_by_watchpoint ();
+
+#define target_stopped_data_address(x) ia64_linux_stopped_data_address(x)
+extern int ia64_linux_stopped_data_address (CORE_ADDR *addr_p);

#define target_insert_watchpoint(addr, len, type) \
ia64_linux_insert_watchpoint (inferior_ptid, addr, len, type)

[-- Attachment #3: sda-doc.patch --]
[-- Type: text/plain, Size: 2339 bytes --]

Index: doc/gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.221
diff -u -p -r1.221 gdbint.texinfo
--- doc/gdbint.texinfo 24 Aug 2004 19:58:24 -0000 1.221
+++ doc/gdbint.texinfo 31 Aug 2004 15:36:30 -0000
@@ -476,9 +476,14 @@ no other code touches these values, the
two macros can use them for their internal purposes.

@findex target_stopped_data_address
-@item target_stopped_data_address ()
-If the inferior has some watchpoint that triggered, return the address
-associated with that watchpoint. Otherwise, return zero.
+@item target_stopped_data_address (@var{addr_p})
+Determine the data address associated with the current triggered hardware
+watchpoint and store it at the location pointed to by @var{addr_p}.
+If @var{addr_p} is NULL, return non-zero if the target has the capability
+to determine a data
+address associated with a triggered hardware watchpoint, otherwise return
+zero. When @var{addr_p} is non-NULL, return non-zero if the data address
+for the triggered watchpoint is determined successfully, otherwise, return zero.

@findex HAVE_STEPPABLE_WATCHPOINT
@item HAVE_STEPPABLE_WATCHPOINT
@@ -598,15 +603,22 @@ less than 4, the number of debug registe
processors.

@findex i386_stopped_data_address
-@item i386_stopped_data_address (void)
-The macros @code{STOPPED_BY_WATCHPOINT} and
-@code{target_stopped_data_address} are set to call this function. The
-argument passed to @code{STOPPED_BY_WATCHPOINT} is ignored. This
+@item i386_stopped_data_address (@var{addr_p})
+The target function
+@code{target_stopped_data_address} is set to call this function.
+This
function examines the breakpoint condition bits in the DR6 Debug
Status register, as returned by the @code{I386_DR_LOW_GET_STATUS}
macro, and returns the address associated with the first bit that is
set in DR6.

+@findex i386_stopped_by_watchpoint
+@item i386_stopped_by_watchpoint (void)
+The macro @code{STOPPED_BY_WATCHPOINT}
+is set to call this function. The
+argument passed to @code{STOPPED_BY_WATCHPOINT} is ignored. This
+function uses the same logic as @code{i386_stopped_data_address}.
+
@findex i386_insert_watchpoint
@findex i386_remove_watchpoint
@item i386_insert_watchpoint (@var{addr}, @var{len}, @var{type})