[RFA]: Change to_stopped_data_address ABI

[RFA]: Change to_stopped_data_address ABI

[Jeff Johnston]

[-- 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})

Re: [RFA]: Change to_stopped_data_address ABI

[Eli Zaretskii]

> Date: Tue, 31 Aug 2004 14:55:13 -0400
> From: Jeff Johnston <jjohnstn@redhat.com>
> 
> 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.

Thanks.

The idea is okay with me, but the code tells a bit different story
(unless I missed something, in which case I apologize).  From your
description, I initially understood that you want to allow to return a
zero address when the watchpoint triggers at that address.  For that,
if no watchpoint triggered, to_stopped_data_address will return zero
as its value, not put a NULL pointer into a place pointed to by its
argument.  That would be okay with me, but your code does something
different:

> @@ -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;

This seems to say that target_stopped_data_address indeed returns a
zero value for the case where no watchpoint triggered...

> +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;

...but this returns 1 as the function's value and puts zero where the
argument points.  Isn't that a contradiction?  And doesn't this code
in i386_stopped_data_address still disallow support for a watchpoint
at address zero by retaining the previous magic meaning of a zero
address?  Or did I miss something?

> +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.

I think "watchpoint is determined successfully" is not clear enough.
Please rewrite the text to say exactly what does the zero value mean.
The intent is to tell a GDB hacker how to handle the case of zero
return value from target_stopped_data_address.

Re: [RFA]: Change to_stopped_data_address ABI

[Jeff Johnston]

Eli Zaretskii wrote:
>>Date: Tue, 31 Aug 2004 14:55:13 -0400
>>From: Jeff Johnston <jjohnstn@redhat.com>
>>
>>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.
> 
> 
> Thanks.
> 
> The idea is okay with me, but the code tells a bit different story
> (unless I missed something, in which case I apologize).  From your
> description, I initially understood that you want to allow to return a
> zero address when the watchpoint triggers at that address.  For that,
> if no watchpoint triggered, to_stopped_data_address will return zero
> as its value, not put a NULL pointer into a place pointed to by its
> argument.  That would be okay with me, but your code does something
> different:
>

Perhaps my description isn't clear enough.  The function returns non-zero if 
successful (i.e. true).  It returns 0 (false) to indicate failure or no stopped 
data address.  It does not return an address at all; it is a true return code. 
The address is returned via the CORE_ADDR pointer argument.  One can tell if the 
function works on a platform by passing a NULL pointer.  The default function in 
target.c always returns 0 to indicate failure.

> 
>>@@ -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;
> 
> 
> This seems to say that target_stopped_data_address indeed returns a
> zero value for the case where no watchpoint triggered...
> 
> 
>>+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;
> 
> 
> ...but this returns 1 as the function's value and puts zero where the
> argument points.  Isn't that a contradiction?  And doesn't this code
> in i386_stopped_data_address still disallow support for a watchpoint
> at address zero by retaining the previous magic meaning of a zero
> address?  Or did I miss something?
> 
> 
>>+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.
> 
> 
> I think "watchpoint is determined successfully" is not clear enough.
> Please rewrite the text to say exactly what does the zero value mean.
> The intent is to tell a GDB hacker how to handle the case of zero
> return value from target_stopped_data_address.
> 

Re: [RFA]: Change to_stopped_data_address ABI

[Eli Zaretskii]

> Date: Tue, 31 Aug 2004 16:18:29 -0400
> From: Jeff Johnston <jjohnstn@redhat.com>
> Cc: gdb-patches@sources.redhat.com
> 
> Perhaps my description isn't clear enough.  The function returns non-zero if 
> successful (i.e. true).  It returns 0 (false) to indicate failure or no stopped 
> data address.

Well, I understood that, I think; but the code you posted seems to
work differently: if no watchpoint triggered, it returns 1, not zero:

> >>@@ -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;

Thus, in the case of "no stopped data address",
i386_stopped_data_address will return 1 with 0 in the place pointed to
by its argument, is that right?

Re: [RFA]: Change to_stopped_data_address ABI

[Andrew Cagney]

> 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.

Some notes:

- having an explicit success/fail is definitly a good idea.  We need 
more of that, ya!

- for GDB, rather than a magic calling convention, a separate predicate 
function is used when probing for a method vis:

	if (target_stopped_data_address_p (&current_target))
	  ... target_stopped_data_address (...);

- we're eliminating target macros replacing them with functions
Unfortunatly here we're also fighting existing tm-*.h / nm-*.h macros so 
some wiggling is required.

- we're making the ``target_ops'' an explicit parameter

so with the tweaks listed below (and the problem Eli identified 
addressed) it's ok ....

> Index: target
> ===================================================================
> 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;
>  }

Add something like (I'm guessing):

int
target_stopped_data_address_p (&current_target)
{
   if zero_func
	return 0;
   else if debug func wrapping zero func
	return 0;
   else
	return 1;
}

> 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_stopped_data_address) (struct target_ops *, CORE_ADDR *);
And update callers.
>      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) ()
Add explicit current target parameter:
> +#define target_stopped_data_address(x) \
> +    (*current_target.to_stopped_data_address) (x)
extern int target_stopped_data_address_p (&current_target);
#else
/* Horrible hack to get around existing macros :-(.  */
#define target_stopped_data-address_p(CURRENT_TARGET) (1)
>  #endif
>  
>  /* This will only be defined by a target that supports catching vfork events,

Andrew

Re: [RFA]: Change to_stopped_data_address ABI

[Jeff Johnston]

Ping.  I hope I clarified Eli's questions already.

Jeff Johnston wrote:
> 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.
> 
> 

Re: [RFA]: Change to_stopped_data_address ABI

[Eli Zaretskii]

> Date: Mon, 20 Sep 2004 17:35:17 -0400
> From: Jeff Johnston <jjohnstn@redhat.com>
> Cc: gdb-patches@sources.redhat.com
> 
> Ping.  I hope I clarified Eli's questions already.

I'm not sure you did; there was a contradiction between the intended
change of the interface and the actual code that was never resolved.

Andrew expressed his concerns in this message:

  http://sources.redhat.com/ml/gdb-patches/2004-09/msg00064.html

Also, in the future, please try to preserve the References header
in your mail, so that the thread history could be traced easier.

Re: [RFA]: Change to_stopped_data_address ABI

[Jeff Johnston]

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

Eli Zaretskii wrote:
>>Date: Mon, 20 Sep 2004 17:35:17 -0400
>>From: Jeff Johnston <jjohnstn@redhat.com>
>>Cc: gdb-patches@sources.redhat.com
>>
>>Ping.  I hope I clarified Eli's questions already.
> 
> 
> I'm not sure you did; there was a contradiction between the intended
> change of the interface and the actual code that was never resolved.
> 
> Andrew expressed his concerns in this message:
> 
>   http://sources.redhat.com/ml/gdb-patches/2004-09/msg00064.html
> 
> Also, in the future, please try to preserve the References header
> in your mail, so that the thread history could be traced easier.
>

Thanks Eli.  I didn't see Andrew's posting.  Some screw-up with my mail reader.
Anyway, I have reworked the code per Andrew's suggestions.  It makes the doc 
stuff much simpler and with the changes I have made, watching address zero is 
now supported.

Andrew/Eli, ok to commit?  Did I miss anything?

Built on x86-linux, x86-64-linux, and ia64-linux.

-- Jeff J.

2004-09-24  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-09-24  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: new.patch --]
[-- Type: text/plain, Size: 13317 bytes --]

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.182
diff -u -p -r1.182 breakpoint.c
--- breakpoint.c	13 Sep 2004 18:26:28 -0000	1.182
+++ breakpoint.c	24 Sep 2004 22:34:26 -0000
@@ -2741,8 +2741,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 (&current_target, &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	24 Sep 2004 22:34:27 -0000
@@ -1293,8 +1293,8 @@ 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;
 
@@ -1305,15 +1305,27 @@ 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 0;
+    }
+
+  return 1;
+}
+
+int
+frv_have_stopped_data_address (void)
+{
+  CORE_ADDR addr = 0;
+  return frv_stopped_data_address (&addr);
 }
 
 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	24 Sep 2004 22:34:27 -0000
@@ -564,14 +564,17 @@ 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 and return non-zero.  
+   Otherwise, set the address pointed to by ADDR_P to zero and return
+   zero.  */
 
-CORE_ADDR
-i386_stopped_data_address (void)
+int
+i386_stopped_data_address (CORE_ADDR *addr_p)
 {
   CORE_ADDR addr = 0;
   int i;
+  int rc = 0;
 
   dr_status_mirror = I386_DR_LOW_GET_STATUS ();
 
@@ -586,6 +589,7 @@ i386_stopped_data_address (void)
 	  && I386_DR_GET_RW_LEN (i) != 0)
 	{
 	  addr = dr_mirror[i];
+	  rc = 1;
 	  if (maint_show_dr)
 	    i386_show_dr ("watchpoint_hit", addr, -1, hw_write);
 	}
@@ -593,7 +597,15 @@ 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 rc;
+}
+
+int
+i386_stopped_by_watchpoint (void)
+{
+  CORE_ADDR addr = 0;
+  return i386_stopped_data_address (&addr);
 }
 
 /* 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	24 Sep 2004 22:34:27 -0000
@@ -636,12 +636,13 @@ 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;
+  ptid_t ptid = inferior_ptid;
 
   tid = TIDGET(ptid);
   if (tid == 0)
@@ -652,14 +653,25 @@ 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 0;
+    }
 
   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;
+  return ia64_linux_stopped_data_address (&addr);
 }
 
 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	24 Sep 2004 22:34:27 -0000
@@ -1450,16 +1450,23 @@ m32r_remove_watchpoint (CORE_ADDR addr, 
   return 0;
 }
 
-CORE_ADDR
-m32r_stopped_data_address (void)
+int
+m32r_stopped_data_address (struct target_ops *target, CORE_ADDR *addr_p)
 {
-  return hit_watchpoint_addr;
+  int rc = 0;
+  if (hit_watchpoint_addr != 0x00000000)
+    {
+      *addr_p = hit_watchpoint_addr;
+      rc = 1;
+    }
+  return rc;
 }
 
 int
 m32r_stopped_by_watchpoint (void)
 {
-  return (hit_watchpoint_addr != 0x00000000);
+  CORE_ADDR addr;
+  return m32r_stopped_data_address (&current_target, &addr);
 }
 
 
Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.147
diff -u -p -r1.147 remote.c
--- remote.c	13 Sep 2004 18:26:30 -0000	1.147
+++ remote.c	24 Sep 2004 22:34:27 -0000
@@ -4551,13 +4551,20 @@ 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 (struct target_ops *target, CORE_ADDR *addr_p)
 {
+  int rc = 0;
   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;
+      rc = 1;
+    }
+  else 
+    *addr_p = (CORE_ADDR)0;
+
+  return rc;
 }
 
 
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.80
diff -u -p -r1.80 target.c
--- target.c	12 Sep 2004 15:20:47 -0000	1.80
+++ target.c	24 Sep 2004 22:34:27 -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 (struct target_ops *, 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 (*) (struct target_ops *, CORE_ADDR *))
 	    return_zero);
   de_fault (to_region_size_ok_for_hw_watchpoint,
 	    default_region_size_ok_for_hw_watchpoint);
@@ -860,6 +860,19 @@ target_write_memory (CORE_ADDR memaddr, 
   return target_xfer_memory (memaddr, myaddr, len, 1);
 }
 
+#ifndef target_stopped_data_address_p
+int
+target_stopped_data_address_p (struct target_ops *target)
+{
+  if (target->to_stopped_data_address == return_zero
+      || (target->to_stopped_data_address == debug_to_stopped_data_address
+	  && debug_target.to_stopped_data_address == return_zero))
+    return 0;
+  else
+    return 1;
+}
+#endif
+
 static int trust_readonly = 0;
 
 /* Move memory to or from the targets.  The top target gets priority;
@@ -1921,16 +1934,17 @@ debug_to_stopped_by_watchpoint (void)
   return retval;
 }
 
-static CORE_ADDR
-debug_to_stopped_data_address (void)
+static int
+debug_to_stopped_data_address (struct target_ops *target, CORE_ADDR *addr)
 {
-  CORE_ADDR retval;
+  int retval;
 
-  retval = debug_target.to_stopped_data_address ();
+  retval = debug_target.to_stopped_data_address (target, addr);
 
   fprintf_unfiltered (gdb_stdlog,
-		      "target_stopped_data_address () = 0x%lx\n",
-		      (unsigned long) retval);
+		      "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	24 Sep 2004 22:34:27 -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) (struct target_ops *, CORE_ADDR *);
     int (*to_region_size_ok_for_hw_watchpoint) (int);
     void (*to_terminal_init) (void);
     void (*to_terminal_inferior) (void);
@@ -1083,9 +1083,14 @@ extern void (*deprecated_target_new_objf
      (*current_target.to_remove_hw_breakpoint) (addr, save)
 #endif
 
+extern int target_stopped_data_address_p (struct target_ops *);
+
 #ifndef target_stopped_data_address
-#define target_stopped_data_address() \
-    (*current_target.to_stopped_data_address) ()
+#define target_stopped_data_address(target, x) \
+    (*target.to_stopped_data_address) (target, x)
+#else
+/* Horrible hack to get around existing macros :-(.  */
+#define target_stopped_data_address_p(CURRENT_TARGET) (1)
 #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.6
diff -u -p -r1.6 nm-i386.h
--- config/i386/nm-i386.h	13 Sep 2004 14:06:03 -0000	1.6
+++ config/i386/nm-i386.h	24 Sep 2004 22:34:27 -0000
@@ -47,10 +47,10 @@ 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
+/* 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.  */
@@ -91,9 +91,11 @@ extern int  i386_remove_hw_breakpoint (C
 
 #define HAVE_CONTINUABLE_WATCHPOINT 1
 
-#define STOPPED_BY_WATCHPOINT(W)       (i386_stopped_data_address () != 0)
+extern int i386_stopped_by_watchpoint (void);
 
-#define target_stopped_data_address()  i386_stopped_data_address ()
+#define STOPPED_BY_WATCHPOINT(W)       (i386_stopped_by_watchpoint () != 0)
+
+#define target_stopped_data_address(target, x)  i386_stopped_data_address(x)
 
 /* Use these macros for watchpoint insertion/removal.  */
 
Index: config/ia64/nm-linux.h
===================================================================
RCS file: /cvs/src/src/gdb/config/ia64/nm-linux.h,v
retrieving revision 1.15
diff -u -p -r1.15 nm-linux.h
--- config/ia64/nm-linux.h	13 Sep 2004 14:06:03 -0000	1.15
+++ config/ia64/nm-linux.h	24 Sep 2004 22:34:27 -0000
@@ -58,8 +58,12 @@ 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(target, 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)
Index: config/frv/tm-frv.h
===================================================================
RCS file: /cvs/src/src/gdb/config/frv/tm-frv.h,v
retrieving revision 1.6
diff -u -p -r1.6 tm-frv.h
--- config/frv/tm-frv.h	13 Sep 2004 14:06:02 -0000	1.6
+++ config/frv/tm-frv.h	24 Sep 2004 22:34:27 -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.
 
@@ -33,10 +33,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(target, x) frv_stopped_data_address(x)
+extern int frv_stopped_data_address(CORE_ADDR *addr_p);
 
 #include "solib.h"		/* Include support for shared libraries.  */

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

Index: gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.224
diff -u -p -r1.224 gdbint.texinfo
--- gdbint.texinfo	12 Sep 2004 15:20:49 -0000	1.224
+++ gdbint.texinfo	24 Sep 2004 22:18:27 -0000
@@ -476,9 +476,10 @@ 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})
+If the inferior has some watchpoint that triggered, place the address
+associated with the watchpoint at the location pointed to by
+@var{addr_p} and return non-zero.  Otherwise, return zero.
 
 @findex HAVE_STEPPABLE_WATCHPOINT
 @item HAVE_STEPPABLE_WATCHPOINT
@@ -598,15 +599,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})
@@ -654,7 +662,7 @@ register.
 @item i386_stopped_by_hwbp (void)
 This function returns non-zero if the inferior has some watchpoint or
 hardware breakpoint that triggered.  It works like
-@code{i386_stopped_data_address}, except that it doesn't return the
+@code{i386_stopped_data_address}, except that it doesn't record the
 address whose watchpoint triggered.
 
 @findex i386_cleanup_dregs

Re: [RFA]: Change to_stopped_data_address ABI

[Eli Zaretskii]

> Date: Fri, 24 Sep 2004 18:41:53 -0400
> From: Jeff Johnston <jjohnstn@redhat.com>
> Cc: gdb-patches@sources.redhat.com

Sorry for a late response.

> Andrew/Eli, ok to commit?  Did I miss anything?

Thanks, most of the concerns I had with your original patch are now
gone.  Still, a few minor ones remain, see below.

> +int
> +frv_stopped_data_address (CORE_ADDR *addr_p)
>  {
>    CORE_ADDR brr, dbar0, dbar1, dbar2, dbar3;
>  
> @@ -1305,15 +1305,27 @@ 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 0;
> +    }
> +
> +  return 1;
> +}

I don't understand why do you put a zero into the address pointed by
addr_p in the case that no watchpoint has fired.  It is customary to
leave the arguments unaltered in such cases.  isn't it enough that you
return a zero as the function's value?

Similar code is in the other functions that return the stopped data
address; I have similar issue with them.

> RCS file: /cvs/src/src/gdb/config/i386/nm-i386.h,v
> retrieving revision 1.6
> diff -u -p -r1.6 nm-i386.h
> --- config/i386/nm-i386.h	13 Sep 2004 14:06:03 -0000	1.6
> +++ config/i386/nm-i386.h	24 Sep 2004 22:34:27 -0000
> @@ -47,10 +47,10 @@ 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
> +/* 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.  */

The last sentence is not true: we return zero (false) sometimes.

> +@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}.

I'd prefer if we describe the operation of i386_stopped_by_watchpoint
explicitly, not by a reference to the logic of i386_stopped_data_address.

Re: [RFA]: Change to_stopped_data_address ABI

[Jeff Johnston]

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

The attached patches address your comments below.  Ok now?

-- Jeff J.

Eli Zaretskii wrote:
>>Date: Fri, 24 Sep 2004 18:41:53 -0400
>>From: Jeff Johnston <jjohnstn@redhat.com>
>>Cc: gdb-patches@sources.redhat.com
> 
> 
> Sorry for a late response.
> 
> 
>>Andrew/Eli, ok to commit?  Did I miss anything?
> 
> 
> Thanks, most of the concerns I had with your original patch are now
> gone.  Still, a few minor ones remain, see below.
> 
> 
>>+int
>>+frv_stopped_data_address (CORE_ADDR *addr_p)
>> {
>>   CORE_ADDR brr, dbar0, dbar1, dbar2, dbar3;
>> 
>>@@ -1305,15 +1305,27 @@ 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 0;
>>+    }
>>+
>>+  return 1;
>>+}
> 
> 
> I don't understand why do you put a zero into the address pointed by
> addr_p in the case that no watchpoint has fired.  It is customary to
> leave the arguments unaltered in such cases.  isn't it enough that you
> return a zero as the function's value?
> 
> Similar code is in the other functions that return the stopped data
> address; I have similar issue with them.
> 
> 
>>RCS file: /cvs/src/src/gdb/config/i386/nm-i386.h,v
>>retrieving revision 1.6
>>diff -u -p -r1.6 nm-i386.h
>>--- config/i386/nm-i386.h	13 Sep 2004 14:06:03 -0000	1.6
>>+++ config/i386/nm-i386.h	24 Sep 2004 22:34:27 -0000
>>@@ -47,10 +47,10 @@ 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
>>+/* 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.  */
> 
> 
> The last sentence is not true: we return zero (false) sometimes.
> 
> 
>>+@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}.
> 
> 
> I'd prefer if we describe the operation of i386_stopped_by_watchpoint
> explicitly, not by a reference to the logic of i386_stopped_data_address.
> 

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

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.182
diff -u -p -r1.182 breakpoint.c
--- breakpoint.c	13 Sep 2004 18:26:28 -0000	1.182
+++ breakpoint.c	6 Oct 2004 20:46:19 -0000
@@ -2741,8 +2741,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 (&current_target, &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	6 Oct 2004 20:46:19 -0000
@@ -1293,8 +1293,8 @@ 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;
 
@@ -1305,15 +1305,24 @@ 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;
+
+  return 1;
+}
+
+int
+frv_have_stopped_data_address (void)
+{
+  CORE_ADDR addr = 0;
+  return frv_stopped_data_address (&addr);
 }
 
 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	6 Oct 2004 20:46:19 -0000
@@ -564,14 +564,16 @@ 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 and return non-zero.  
+   Otherwise, return zero.  */
 
-CORE_ADDR
-i386_stopped_data_address (void)
+int
+i386_stopped_data_address (CORE_ADDR *addr_p)
 {
   CORE_ADDR addr = 0;
   int i;
+  int rc = 0;
 
   dr_status_mirror = I386_DR_LOW_GET_STATUS ();
 
@@ -586,6 +588,7 @@ i386_stopped_data_address (void)
 	  && I386_DR_GET_RW_LEN (i) != 0)
 	{
 	  addr = dr_mirror[i];
+	  rc = 1;
 	  if (maint_show_dr)
 	    i386_show_dr ("watchpoint_hit", addr, -1, hw_write);
 	}
@@ -593,7 +596,16 @@ i386_stopped_data_address (void)
   if (maint_show_dr && addr == 0)
     i386_show_dr ("stopped_data_addr", 0, 0, hw_write);
 
-  return addr;
+  if (rc)
+    *addr_p = addr;
+  return rc;
+}
+
+int
+i386_stopped_by_watchpoint (void)
+{
+  CORE_ADDR addr = 0;
+  return i386_stopped_data_address (&addr);
 }
 
 /* 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	6 Oct 2004 20:46:19 -0000
@@ -636,12 +636,13 @@ 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;
+  ptid_t ptid = inferior_ptid;
 
   tid = TIDGET(ptid);
   if (tid == 0)
@@ -659,7 +660,15 @@ ia64_linux_stopped_by_watchpoint (ptid_t
                            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;
+  return ia64_linux_stopped_data_address (&addr);
 }
 
 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	6 Oct 2004 20:46:19 -0000
@@ -1450,16 +1450,23 @@ m32r_remove_watchpoint (CORE_ADDR addr, 
   return 0;
 }
 
-CORE_ADDR
-m32r_stopped_data_address (void)
+int
+m32r_stopped_data_address (struct target_ops *target, CORE_ADDR *addr_p)
 {
-  return hit_watchpoint_addr;
+  int rc = 0;
+  if (hit_watchpoint_addr != 0x00000000)
+    {
+      *addr_p = hit_watchpoint_addr;
+      rc = 1;
+    }
+  return rc;
 }
 
 int
 m32r_stopped_by_watchpoint (void)
 {
-  return (hit_watchpoint_addr != 0x00000000);
+  CORE_ADDR addr;
+  return m32r_stopped_data_address (&current_target, &addr);
 }
 
 
Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.147
diff -u -p -r1.147 remote.c
--- remote.c	13 Sep 2004 18:26:30 -0000	1.147
+++ remote.c	6 Oct 2004 20:46:19 -0000
@@ -4551,13 +4551,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 (struct target_ops *target, CORE_ADDR *addr_p)
 {
+  int rc = 0;
   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;
+      rc = 1;
+    }
+
+  return rc;
 }
 
 
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.80
diff -u -p -r1.80 target.c
--- target.c	12 Sep 2004 15:20:47 -0000	1.80
+++ target.c	6 Oct 2004 20:46:19 -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 (struct target_ops *, 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 (*) (struct target_ops *, CORE_ADDR *))
 	    return_zero);
   de_fault (to_region_size_ok_for_hw_watchpoint,
 	    default_region_size_ok_for_hw_watchpoint);
@@ -860,6 +860,19 @@ target_write_memory (CORE_ADDR memaddr, 
   return target_xfer_memory (memaddr, myaddr, len, 1);
 }
 
+#ifndef target_stopped_data_address_p
+int
+target_stopped_data_address_p (struct target_ops *target)
+{
+  if (target->to_stopped_data_address == return_zero
+      || (target->to_stopped_data_address == debug_to_stopped_data_address
+	  && debug_target.to_stopped_data_address == return_zero))
+    return 0;
+  else
+    return 1;
+}
+#endif
+
 static int trust_readonly = 0;
 
 /* Move memory to or from the targets.  The top target gets priority;
@@ -1921,16 +1934,17 @@ debug_to_stopped_by_watchpoint (void)
   return retval;
 }
 
-static CORE_ADDR
-debug_to_stopped_data_address (void)
+static int
+debug_to_stopped_data_address (struct target_ops *target, CORE_ADDR *addr)
 {
-  CORE_ADDR retval;
+  int retval;
 
-  retval = debug_target.to_stopped_data_address ();
+  retval = debug_target.to_stopped_data_address (target, addr);
 
   fprintf_unfiltered (gdb_stdlog,
-		      "target_stopped_data_address () = 0x%lx\n",
-		      (unsigned long) retval);
+		      "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	6 Oct 2004 20:46:19 -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) (struct target_ops *, CORE_ADDR *);
     int (*to_region_size_ok_for_hw_watchpoint) (int);
     void (*to_terminal_init) (void);
     void (*to_terminal_inferior) (void);
@@ -1083,9 +1083,14 @@ extern void (*deprecated_target_new_objf
      (*current_target.to_remove_hw_breakpoint) (addr, save)
 #endif
 
+extern int target_stopped_data_address_p (struct target_ops *);
+
 #ifndef target_stopped_data_address
-#define target_stopped_data_address() \
-    (*current_target.to_stopped_data_address) ()
+#define target_stopped_data_address(target, x) \
+    (*target.to_stopped_data_address) (target, x)
+#else
+/* Horrible hack to get around existing macros :-(.  */
+#define target_stopped_data_address_p(CURRENT_TARGET) (1)
 #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.6
diff -u -p -r1.6 nm-i386.h
--- config/i386/nm-i386.h	13 Sep 2004 14:06:03 -0000	1.6
+++ config/i386/nm-i386.h	6 Oct 2004 20:46:19 -0000
@@ -47,10 +47,10 @@ 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
-   the address associated with that break/watchpoint.  Otherwise,
-   return zero.  */
-extern CORE_ADDR i386_stopped_data_address (void);
+/* If the inferior has some break/watchpoint that triggered, set
+   the address associated with that break/watchpoint and return
+   true.  Otherwise, return false.  */
+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.  */
@@ -91,9 +91,11 @@ extern int  i386_remove_hw_breakpoint (C
 
 #define HAVE_CONTINUABLE_WATCHPOINT 1
 
-#define STOPPED_BY_WATCHPOINT(W)       (i386_stopped_data_address () != 0)
+extern int i386_stopped_by_watchpoint (void);
 
-#define target_stopped_data_address()  i386_stopped_data_address ()
+#define STOPPED_BY_WATCHPOINT(W)       (i386_stopped_by_watchpoint () != 0)
+
+#define target_stopped_data_address(target, 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.6
diff -u -p -r1.6 tm-frv.h
--- config/frv/tm-frv.h	13 Sep 2004 14:06:02 -0000	1.6
+++ config/frv/tm-frv.h	6 Oct 2004 20:46:19 -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.
 
@@ -33,10 +33,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(target, 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.15
diff -u -p -r1.15 nm-linux.h
--- config/ia64/nm-linux.h	13 Sep 2004 14:06:03 -0000	1.15
+++ config/ia64/nm-linux.h	6 Oct 2004 20:46:19 -0000
@@ -58,8 +58,12 @@ 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(target, 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: doc.patch2 --]
[-- Type: text/plain, Size: 2487 bytes --]

Index: gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.224
diff -u -r1.224 gdbint.texinfo
--- gdbint.texinfo	12 Sep 2004 15:20:49 -0000	1.224
+++ gdbint.texinfo	6 Oct 2004 20:38:14 -0000
@@ -476,9 +476,10 @@
 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})
+If the inferior has some watchpoint that triggered, place the address
+associated with the watchpoint at the location pointed to by
+@var{addr_p} and return non-zero.  Otherwise, return zero.
 
 @findex HAVE_STEPPABLE_WATCHPOINT
 @item HAVE_STEPPABLE_WATCHPOINT
@@ -598,15 +599,25 @@
 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 examines the breakpoint condition bits in the DR6 Debug
+Status register, as returned by the @code{I386_DR_LOW_GET_STATUS}
+macro, and returns true if any bit is set.  Otherwise, false is
+returned.
+
 @findex i386_insert_watchpoint
 @findex i386_remove_watchpoint
 @item i386_insert_watchpoint (@var{addr}, @var{len}, @var{type})
@@ -654,7 +665,7 @@
 @item i386_stopped_by_hwbp (void)
 This function returns non-zero if the inferior has some watchpoint or
 hardware breakpoint that triggered.  It works like
-@code{i386_stopped_data_address}, except that it doesn't return the
+@code{i386_stopped_data_address}, except that it doesn't record the
 address whose watchpoint triggered.
 
 @findex i386_cleanup_dregs

Re: [RFA]: Change to_stopped_data_address ABI

[Eli Zaretskii]

> Date: Wed, 06 Oct 2004 16:47:47 -0400
> From: Jeff Johnston <jjohnstn@redhat.com>
> Cc: gdb-patches@sources.redhat.com
> 
> The attached patches address your comments below.  Ok now?

Yes, thanks.

Re: [RFA]: Change to_stopped_data_address ABI

[jjohnstn]

On Fri, 8 Oct 2004, Eli Zaretskii wrote:

> > Date: Wed, 06 Oct 2004 16:47:47 -0400
> > From: Jeff Johnston <jjohnstn@redhat.com>
> > Cc: gdb-patches@sources.redhat.com
> > 
> > The attached patches address your comments below.  Ok now?
> 
> Yes, thanks.
>

Thanks Eli.  Patches checked in.

-- Jeff J.