From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24320 invoked by alias); 3 Sep 2004 21:20:31 -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 24310 invoked from network); 3 Sep 2004 21:20:30 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 3 Sep 2004 21:20:30 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.10/8.12.10) with ESMTP id i83LKOS2022340 for ; Fri, 3 Sep 2004 17:20:29 -0400 Received: from localhost.redhat.com (porkchop.devel.redhat.com [172.16.58.2]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i83LKD325163; Fri, 3 Sep 2004 17:20:13 -0400 Received: from gnu.org (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 97A6F28D2; Fri, 3 Sep 2004 17:19:01 -0400 (EDT) Message-ID: <4138DFC5.1040302@gnu.org> Date: Fri, 03 Sep 2004 21:20:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-GB; rv:1.4.1) Gecko/20040831 MIME-Version: 1.0 To: Jeff Johnston Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA]: Change to_stopped_data_address ABI References: <4134C991.7050507@redhat.com> In-Reply-To: <4134C991.7050507@redhat.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2004-09/txt/msg00064.txt.bz2 > 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 (¤t_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 (¤t_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 (¤t_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