This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA]: Change to_stopped_data_address ABI
- From: Andrew Cagney <cagney at gnu dot org>
- To: Jeff Johnston <jjohnstn at redhat dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Fri, 03 Sep 2004 17:19:01 -0400
- Subject: Re: [RFA]: Change to_stopped_data_address ABI
- References: <4134C991.7050507@redhat.com>
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