From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16258 invoked by alias); 25 Jan 2006 08:34:39 -0000 Received: (qmail 16249 invoked by uid 22791); 25 Jan 2006 08:34:37 -0000 X-Spam-Check-By: sourceware.org Received: from ausmtp03.au.ibm.com (HELO ausmtp03.au.ibm.com) (202.81.18.151) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 25 Jan 2006 08:34:34 +0000 Received: from sd0112e0.au.ibm.com (d23rh903.au.ibm.com [202.81.18.201]) by ausmtp03.au.ibm.com (8.12.10/8.12.10) with ESMTP id k0P8bdU0369770 for ; Wed, 25 Jan 2006 19:37:43 +1100 Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.250.244]) by sd0112e0.au.ibm.com (8.12.10/NCO/VERS6.8) with ESMTP id k0P8bYMX252308 for ; Wed, 25 Jan 2006 19:37:35 +1100 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.12.11/8.13.3) with ESMTP id k0P8YNii031219 for ; Wed, 25 Jan 2006 19:34:23 +1100 Received: from wks190294wss.cn.ibm.com (wks190294wss.cn.ibm.com [9.181.133.242]) by d23av03.au.ibm.com (8.12.11/8.12.11) with ESMTP id k0P8Y8fc030583; Wed, 25 Jan 2006 19:34:12 +1100 Date: Wed, 25 Jan 2006 08:34:00 -0000 From: Wu Zhou To: Daniel Jacobowitz cc: Eli Zaretskii , gdb-patches@sources.redhat.com, bje@au1.ibm.com, anton@au1.ibm.com, pgilliam@us.ibm.com Subject: Replace to_region_size_ok_for_hw_watchpoint references with to_region_ok_for_hw_watchpoint ones In-Reply-To: <20060124212046.GC26974@nevyn.them.org> Message-ID: References: <20060122205641.GF27224@nevyn.them.org> <20060124034304.GA4719@nevyn.them.org> <20060124212046.GC26974@nevyn.them.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-01/txt/msg00401.txt.bz2 Hi Daniel, This is the patch we talked about to replace to_region_size_ok_for_hw_watchpoint references with references to to_region_ok_for_hw_watchpoint. I am also thinking of replace the macro TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT (SIZE) with TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR, SIZE). Thus the code will seem to be more clean: we will only have one macro to see if the target region is ok for watchpoint monitoring. Following this way, function default_region_ok_for_hw_watchpoint will also return back to its original implementation. What is your thought on this? Here goes the patch. Please review. Thanks. P.S: I had tested this on ppc64 and x86. Didn't see any regression. 2006-01-25 Wu Zhou * breakpoint.c (TARGET_REGION_OK_FOR_HW_WATCHPOINT): Delete. * config/i386/nm-i386sol2.h (TARGET_REGION_OK_FOR_HW_WATCHPOINT): New. (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT): Delete. * config/mips/nm-irix5.h (TARGET_REGION_OK_FOR_HW_WATCHPOINT): New. (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT): Delete. * config/sparc/nm-sol2.h (TARGET_REGION_OK_FOR_HW_WATCHPOINT): New. (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT): Delete. * inf-ttrace.c (inf_ttrace_region_ok_for_hw_watchpoint): New. (inf_ttrace_region_size_ok_for_hw_watchpoint): Delete. (inf_ttrace_target): Delete to_region_size_ok_for_hw_watchpoint and add to_region_ok_for_hw_watchpoint. * s390-nat.c (s390_region_size_ok_for_hw_watchpoint): Delete. (s390_region_ok_for_hw_watchpoint): New. (_initialize_s390_nat): Delete to_region_size_ok_for_hw_watchpoint and add to_region_ok_for_hw_watchpoint. * target.c (default_region_size_ok_for_hw_watchpoint, debug_to_region_size_ok_for_hw_watchpoint): Delete prototype. (update_current_target): Delete to_region_size_ok_for_hw_watchpoint inheritance and default_region_size_ok_for_hw_watchpoint. (default_region_ok_for_hw_watchpoint): If len is less than or equal the length of void pointer, return ok. (default_region_size_ok_for_hw_watchpoint): Delete. (debug_to_region_size_ok_for_hw_watchpoint): Delete. (setup_target_debug): Delete to_region_size_ok_for_hw_watchpoint. * target.h (struct target_ops): Delete to_region_size_ok_for_hw_watchpoint. (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT): Delete. diff -rcp src/gdb/breakpoint.c src.new/gdb/breakpoint.c *** src/gdb/breakpoint.c 2005-05-28 23:13:17.000000000 -0400 --- src.new/gdb/breakpoint.c 2006-01-24 22:38:40.000000000 -0500 *************** watch_command_1 (char *arg, int accessfl *** 5791,5801 **** in hardware. If the watchpoint can not be handled in hardware return zero. */ - #if !defined(TARGET_REGION_OK_FOR_HW_WATCHPOINT) - #define TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR,LEN) \ - (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(LEN)) - #endif - static int can_use_hardware_watchpoint (struct value *v) { --- 5791,5796 ---- diff -rcp src/gdb/config/i386/nm-i386sol2.h src.new/gdb/config/i386/nm-i386sol2.h *** src/gdb/config/i386/nm-i386sol2.h 2004-11-30 10:05:19.000000000 -0500 --- src.new/gdb/config/i386/nm-i386sol2.h 2006-01-24 22:43:40.000000000 -0500 *************** *** 34,40 **** can support "thousands" of hardware watchpoints, but gives no method for finding out how many. So just tell GDB 'yes'. */ #define TARGET_CAN_USE_HARDWARE_WATCHPOINT(TYPE, CNT, OT) 1 ! #define TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(SIZE) 1 /* When a hardware watchpoint fires off the PC will be left at the instruction following the one which caused the watchpoint. --- 34,40 ---- can support "thousands" of hardware watchpoints, but gives no method for finding out how many. So just tell GDB 'yes'. */ #define TARGET_CAN_USE_HARDWARE_WATCHPOINT(TYPE, CNT, OT) 1 ! #define TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR, SIZE) 1 /* When a hardware watchpoint fires off the PC will be left at the instruction following the one which caused the watchpoint. diff -rcp src/gdb/config/mips/nm-irix5.h src.new/gdb/config/mips/nm-irix5.h *** src/gdb/config/mips/nm-irix5.h 2004-11-30 10:05:20.000000000 -0500 --- src.new/gdb/config/mips/nm-irix5.h 2006-01-24 22:44:33.000000000 -0500 *************** extern int procfs_stopped_by_watchpoint *** 51,57 **** procfs_set_watchpoint (inferior_ptid, ADDR, 0, 0, 0) extern int procfs_set_watchpoint (ptid_t, CORE_ADDR, int, int, int); ! #define TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(SIZE) 1 /* Override register locations in upage for SGI machines */ #define REGISTER_U_ADDR(addr, blockend, regno) \ --- 51,57 ---- procfs_set_watchpoint (inferior_ptid, ADDR, 0, 0, 0) extern int procfs_set_watchpoint (ptid_t, CORE_ADDR, int, int, int); ! #define TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR, SIZE) 1 /* Override register locations in upage for SGI machines */ #define REGISTER_U_ADDR(addr, blockend, regno) \ diff -rcp src/gdb/config/sparc/nm-sol2.h src.new/gdb/config/sparc/nm-sol2.h *** src/gdb/config/sparc/nm-sol2.h 2004-01-03 05:08:45.000000000 -0500 --- src.new/gdb/config/sparc/nm-sol2.h 2006-01-24 22:42:56.000000000 -0500 *************** *** 40,46 **** method for finding out how many; It doesn't say anything about the allowed size for the watched area either. So we just tell GDB 'yes'. */ ! #define TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(SIZE) 1 /* When a hardware watchpoint fires off the PC will be left at the instruction following the one which caused the watchpoint. It will --- 40,46 ---- method for finding out how many; It doesn't say anything about the allowed size for the watched area either. So we just tell GDB 'yes'. */ ! #define TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR, SIZE) 1 /* When a hardware watchpoint fires off the PC will be left at the instruction following the one which caused the watchpoint. It will diff -rcp src/gdb/inf-ttrace.c src.new/gdb/inf-ttrace.c *** src/gdb/inf-ttrace.c 2005-10-29 17:22:39.000000000 -0400 --- src.new/gdb/inf-ttrace.c 2006-01-24 22:40:56.000000000 -0500 *************** inf_ttrace_can_use_hw_breakpoint (int ty *** 360,366 **** } static int ! inf_ttrace_region_size_ok_for_hw_watchpoint (int len) { return 1; } --- 360,366 ---- } static int ! inf_ttrace_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len) { return 1; } *************** inf_ttrace_target (void) *** 1132,1139 **** t->to_insert_watchpoint = inf_ttrace_insert_watchpoint; t->to_remove_watchpoint = inf_ttrace_remove_watchpoint; t->to_stopped_by_watchpoint = inf_ttrace_stopped_by_watchpoint; ! t->to_region_size_ok_for_hw_watchpoint = ! inf_ttrace_region_size_ok_for_hw_watchpoint; t->to_kill = inf_ttrace_kill; t->to_create_inferior = inf_ttrace_create_inferior; t->to_follow_fork = inf_ttrace_follow_fork; --- 1132,1139 ---- t->to_insert_watchpoint = inf_ttrace_insert_watchpoint; t->to_remove_watchpoint = inf_ttrace_remove_watchpoint; t->to_stopped_by_watchpoint = inf_ttrace_stopped_by_watchpoint; ! t->to_region_ok_for_hw_watchpoint = ! inf_ttrace_region_ok_for_hw_watchpoint; t->to_kill = inf_ttrace_kill; t->to_create_inferior = inf_ttrace_create_inferior; t->to_follow_fork = inf_ttrace_follow_fork; diff -rcp src/gdb/s390-nat.c src.new/gdb/s390-nat.c *** src/gdb/s390-nat.c 2005-09-11 17:54:58.000000000 -0400 --- src.new/gdb/s390-nat.c 2006-01-24 22:51:26.000000000 -0500 *************** s390_can_use_hw_breakpoint (int type, in *** 358,364 **** } static int ! s390_region_size_ok_for_hw_watchpoint (int cnt) { return 1; } --- 358,364 ---- } static int ! s390_region_ok_for_hw_watchpoint (CORE_ADDR addr, int cnt) { return 1; } *************** _initialize_s390_nat (void) *** 380,386 **** /* Add our watchpoint methods. */ t->to_can_use_hw_breakpoint = s390_can_use_hw_breakpoint; ! t->to_region_size_ok_for_hw_watchpoint = s390_region_size_ok_for_hw_watchpoint; t->to_have_continuable_watchpoint = 1; t->to_stopped_by_watchpoint = s390_stopped_by_watchpoint; t->to_insert_watchpoint = s390_insert_watchpoint; --- 380,386 ---- /* Add our watchpoint methods. */ t->to_can_use_hw_breakpoint = s390_can_use_hw_breakpoint; ! t->to_region_ok_for_hw_watchpoint = s390_region_ok_for_hw_watchpoint; t->to_have_continuable_watchpoint = 1; t->to_stopped_by_watchpoint = s390_stopped_by_watchpoint; t->to_insert_watchpoint = s390_insert_watchpoint; diff -rcp src/gdb/target.c src.new/gdb/target.c *** src/gdb/target.c 2006-01-25 10:16:16.000000000 -0500 --- src.new/gdb/target.c 2006-01-24 22:34:45.000000000 -0500 *************** static void default_terminal_info (char *** 49,56 **** static int default_region_ok_for_hw_watchpoint (CORE_ADDR, int); - static int default_region_size_ok_for_hw_watchpoint (int); - static int nosymbol (char *, CORE_ADDR *); static void tcomplain (void); --- 49,54 ---- *************** static int debug_to_stopped_data_address *** 132,139 **** static int debug_to_region_ok_for_hw_watchpoint (CORE_ADDR, int); - static int debug_to_region_size_ok_for_hw_watchpoint (int); - static void debug_to_terminal_init (void); static void debug_to_terminal_inferior (void); --- 130,135 ---- *************** update_current_target (void) *** 410,416 **** INHERIT (to_stopped_by_watchpoint, t); INHERIT (to_have_continuable_watchpoint, t); INHERIT (to_region_ok_for_hw_watchpoint, t); - INHERIT (to_region_size_ok_for_hw_watchpoint, t); INHERIT (to_terminal_init, t); INHERIT (to_terminal_inferior, t); INHERIT (to_terminal_ours_for_output, t); --- 406,411 ---- *************** update_current_target (void) *** 538,545 **** return_zero); de_fault (to_region_ok_for_hw_watchpoint, default_region_ok_for_hw_watchpoint); - de_fault (to_region_size_ok_for_hw_watchpoint, - default_region_size_ok_for_hw_watchpoint); de_fault (to_terminal_init, (void (*) (void)) target_ignore); --- 533,538 ---- *************** find_default_create_inferior (char *exec *** 1584,1596 **** static int default_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len) { ! return TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT (len); ! } ! ! static int ! default_region_size_ok_for_hw_watchpoint (int byte_count) ! { ! return (byte_count <= TYPE_LENGTH (builtin_type_void_data_ptr)); } static int --- 1577,1583 ---- static int default_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len) { ! return (len <= TYPE_LENGTH (builtin_type_void_data_ptr)); } static int *************** debug_to_region_ok_for_hw_watchpoint (CO *** 2143,2162 **** } static int - debug_to_region_size_ok_for_hw_watchpoint (int byte_count) - { - CORE_ADDR retval; - - retval = debug_target.to_region_size_ok_for_hw_watchpoint (byte_count); - - fprintf_unfiltered (gdb_stdlog, - "TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT (%ld) = 0x%lx\n", - (unsigned long) byte_count, - (unsigned long) retval); - return retval; - } - - static int debug_to_stopped_by_watchpoint (void) { int retval; --- 2130,2135 ---- *************** setup_target_debug (void) *** 2562,2568 **** current_target.to_stopped_by_watchpoint = debug_to_stopped_by_watchpoint; current_target.to_stopped_data_address = debug_to_stopped_data_address; current_target.to_region_ok_for_hw_watchpoint = debug_to_region_ok_for_hw_watchpoint; - current_target.to_region_size_ok_for_hw_watchpoint = debug_to_region_size_ok_for_hw_watchpoint; current_target.to_terminal_init = debug_to_terminal_init; current_target.to_terminal_inferior = debug_to_terminal_inferior; current_target.to_terminal_ours_for_output = debug_to_terminal_ours_for_output; --- 2535,2540 ---- diff -rcp src/gdb/target.h src.new/gdb/target.h *** src/gdb/target.h 2006-01-24 16:59:38.000000000 -0500 --- src.new/gdb/target.h 2006-01-24 22:30:50.000000000 -0500 *************** struct target_ops *** 346,352 **** int to_have_continuable_watchpoint; int (*to_stopped_data_address) (struct target_ops *, CORE_ADDR *); int (*to_region_ok_for_hw_watchpoint) (CORE_ADDR, int); - int (*to_region_size_ok_for_hw_watchpoint) (int); void (*to_terminal_init) (void); void (*to_terminal_inferior) (void); void (*to_terminal_ours_for_output) (void); --- 346,351 ---- *************** extern void (*deprecated_target_new_objf *** 1036,1046 **** (*current_target.to_region_ok_for_hw_watchpoint) (addr, len) #endif - #if !defined(TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT) - #define TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(byte_count) \ - (*current_target.to_region_size_ok_for_hw_watchpoint) (byte_count) - #endif - /* Set/clear a hardware watchpoint starting at ADDR, for LEN bytes. TYPE is 0 for write, 1 for read, and 2 for read/write accesses. Returns 0 for --- 1035,1040 ---- Regards - Wu Zhou On Tue, 24 Jan 2006, Daniel Jacobowitz wrote: > On Tue, Jan 24, 2006 at 06:59:53PM +0800, Wu Zhou wrote: > > Hi Daniel, > > > > Because there are quite a few places in different arch/target of GDB which > > use region_size_ok_for_hw_watchpoint, so I am prefering to get this done > > in two steps: first add to_region_ok_for_hw_watchpoint into struct > > target_ops, then replace these to_region_size_ok_for_hw_watchpoint > > reference with to_region_ok_for_hw_watchpoint ones. IMHO, it is easier to > > not confuse the original intention of this patch with this replacement. > > What is your thought on this? > > Sure. Couple small things but we're almost done. > > > static int > > +default_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len) > > +{ > > + return (len <= TYPE_LENGTH (builtin_type_void_data_ptr)); > > +} > > TARGET_REGION_OK_FOR_HW_WATCHPOINT will now always be defined, because > of the #ifdef in target.h. Therefore this won't trigger (from > breakpoint.c): > > #if !defined(TARGET_REGION_OK_FOR_HW_WATCHPOINT) > #define TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR,LEN) \ > (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(LEN)) > #endif > > So you need to call TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT (len) here, > in case the target has overridden that. > > > -- > Daniel Jacobowitz > CodeSourcery > >