From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15111 invoked by alias); 6 Oct 2004 20:47:59 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 15077 invoked from network); 6 Oct 2004 20:47:53 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 6 Oct 2004 20:47:53 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.11/8.12.10) with ESMTP id i96KlmH9021338 for ; Wed, 6 Oct 2004 16:47:48 -0400 Received: from pobox.toronto.redhat.com (pobox.toronto.redhat.com [172.16.14.4]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i96Kllr25046; Wed, 6 Oct 2004 16:47:47 -0400 Received: from touchme.toronto.redhat.com (IDENT:postfix@touchme.toronto.redhat.com [172.16.14.9]) by pobox.toronto.redhat.com (8.12.8/8.12.8) with ESMTP id i96KllbU011454; Wed, 6 Oct 2004 16:47:47 -0400 Received: from [172.16.14.72] (toocool.toronto.redhat.com [172.16.14.72]) by touchme.toronto.redhat.com (Postfix) with ESMTP id 2EAFC800384; Wed, 6 Oct 2004 16:47:47 -0400 (EDT) Message-ID: <416459F3.3070306@redhat.com> Date: Wed, 06 Oct 2004 20:47:00 -0000 From: Jeff Johnston User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.2) Gecko/20040805 Netscape/7.2 MIME-Version: 1.0 To: Eli Zaretskii Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA]: Change to_stopped_data_address ABI References: <4134C991.7050507@redhat.com> <414F4D15.7010503@redhat.com> <01c49f8f$Blat.v2.2.2$a6da98a0@zahav.net.il> <4154A2B1.9010908@redhat.com> <01c4abbe$Blat.v2.2.2$89197760@zahav.net.il> In-Reply-To: <01c4abbe$Blat.v2.2.2$89197760@zahav.net.il> Content-Type: multipart/mixed; boundary="------------090609020100000003040906" X-SW-Source: 2004-10/txt/msg00122.txt.bz2 This is a multi-part message in MIME format. --------------090609020100000003040906 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2679 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 >>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. > --------------090609020100000003040906 Content-Type: text/plain; name="sda.patch2" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="sda.patch2" Content-length: 12902 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 (¤t_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 (¤t_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) --------------090609020100000003040906 Content-Type: text/plain; name="doc.patch2" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="doc.patch2" Content-length: 2487 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 --------------090609020100000003040906--