From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17531 invoked by alias); 4 May 2009 19:32:46 -0000 Received: (qmail 17521 invoked by uid 22791); 4 May 2009 19:32:45 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 04 May 2009 19:32:39 +0000 Received: (qmail 9266 invoked from network); 4 May 2009 19:32:36 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 4 May 2009 19:32:36 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [RFC] Remove last occurences of target_{insert/remove}_watchpoint Date: Mon, 04 May 2009 19:32:00 -0000 User-Agent: KMail/1.9.10 Cc: "Pierre Muller" , Joel Brobecker References: <001a01c9c39f$e3f217c0$abd64740$@u-strasbg.fr> <200904300000.26661.pedro@codesourcery.com> In-Reply-To: <200904300000.26661.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200905042033.09398.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-05/txt/msg00069.txt.bz2 On Thursday 30 April 2009 00:00:26, Pedro Alves wrote: > On Thursday 23 April 2009 00:13:03, Pierre Muller wrote: > > The two macros target_insert_watchpoint and target_remove_watchpoint > > and only still defined inn three config files. > > > > These macros are really bad, because that > > forbid remote target to work properly on the targets > > that define them. > > Right on. > > > The patch below tries to get rid of them, > > by defining appropriate target functions. > > > > I needed to insert another new macros that I called > > STOP_AFTER_WATCHPOINT because out of three, two of these targets > > call procfs_set_watchpoint with after_trap set to one, > > and the third (mips/nm-irix5.h) set to zero. > > i386-solaris and sparc-solaris can select between continuable > and non-continuable watchpoints through the WA_TRAPAFTER flag, > which is what the after_trap flag controls. In the GDB user's > perpective, watchpoints are always reported after the memory access > having happened. If behind the scenes, they don't trap after > the memory access, then GDB single steps once to finish the memory > access. This latter case is the that of the mips architecture, > mips-irix included. See: > > #else /* Irix method for watchpoints */ > enum { READ_WATCHFLAG = MA_READ, > WRITE_WATCHFLAG = MA_WRITE, > EXEC_WATCHFLAG = MA_EXEC, > AFTER_WATCHFLAG = 0 /* trapafter not implemented */ > }; > #endif > > Unfortunatelly, infrun.c's way of checking for > continuable/non-continuable watchpoints is full of historic twists. > There's a to_have_continuable_watchpoint target_ops flag, but nothing > uses it anymore. Instead, continuable == (!steppable && !non-steppable), > but, `steppable' is a target property (to_have_steppable_watchpoint), and > non_steppable is a gdbarch property (gdbarch_have_nonsteppable_watchpoint). > > Knowing that, we can avoid introducing that STOP_AFTER_WATCHPOINT > macro. Instead do the same checks infrun.c wants to do. > > As I was describing this, I went ahead and cleaned up your > patch, as below. Completely untested. Care to take a look? I've just ran this version through sparc solaris 8, and it survived without regressions (although it looks like solaris test results got much worse somewhere along these last weeks -- around 1300 fails), and watchpoints still work. Joel, would you like to take a look at this and/or ran it on mips-irix? > And since you got me started, ..., notice that > TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT was at some point converted > to TARGET_REGION_OK_FOR_HW_WATCHPOINT, but, these procfs uses were > missed in the convertion. procfs allows setting watchpoints watching > large memory regions, but, it that support has been broken > since. While I'm here, I'm converting TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT > to the proper target method. TARGET_CAN_USE_HARDWARE_WATCHPOINT is > not used anymore, so we can remove those, and same > for HAVE_CONTINUABLE_WATCHPOINT. > > Note that this isn't ideal yet, but, cleaning this up better > involves better splitting of procfs.c into further *-nat.c files > that inherit and extend the return of procfs_target. It is now > getting easier to do it though. > -- Pedro Alves 2009-05-04 Pierre Muller Pedro Alves * procfs.c (procfs_insert_watchpoint, procfs_remove_watchpoint) (procfs_region_ok_for_hw_watchpoint, procfs_use_watchpoints): New functions. (procfs_stopped_by_watchpoint): Made static, ptid argument removed. (_initialize_procfs): Register new watchpoint related target functions. * config/i386/nm-i386sol2.h (TARGET_CAN_USE_HARDWARE_WATCHPOINT) (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT, STOPPED_BY_WATCHPOINT) (HAVE_CONTINUABLE_WATCHPOINT): Delete. (target_insert_watchpoint, target_remove_watchpoint): Delete. (procfs_stopped_by_watchpoint, procfs_set_watchpoint): Delete declarations. * config/mips/nm-irix5.h (STOPPED_BY_WATCHPOINT) (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT): Delete. (target_insert_watchpoint, target_remove_watchpoint): Delete. (procfs_stopped_by_watchpoint, procfs_set_watchpoint): Delete declarations. * config/sparc/nm-sol2.h (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT) (HAVE_CONTINUABLE_WATCHPOINT, STOPPED_BY_WATCHPOINT): Delete. (target_insert_watchpoint, target_remove_watchpoint): Delete. (procfs_stopped_by_watchpoint, procfs_set_watchpoint): Delete declarations. --- gdb/config/i386/nm-i386sol2.h | 24 ----------------- gdb/config/mips/nm-irix5.h | 23 ---------------- gdb/config/sparc/nm-sol2.h | 25 ------------------ gdb/procfs.c | 58 +++++++++++++++++++++++++++++++++++++++--- 4 files changed, 54 insertions(+), 76 deletions(-) Index: src/gdb/procfs.c =================================================================== --- src.orig/gdb/procfs.c 2009-04-30 11:39:18.000000000 +0100 +++ src/gdb/procfs.c 2009-05-04 12:42:31.000000000 +0100 @@ -5321,13 +5321,12 @@ procfs_can_use_hw_breakpoint (int type, * else returns zero. */ -int -procfs_stopped_by_watchpoint (ptid_t ptid) +static int +procfs_stopped_by_watchpoint (void) { procinfo *pi; - pi = find_procinfo_or_die (PIDGET (ptid) == -1 ? - PIDGET (inferior_ptid) : PIDGET (ptid), 0); + pi = find_procinfo_or_die (PIDGET (inferior_ptid), 0); if (!pi) /* If no process, then not stopped by watchpoint! */ return 0; @@ -5349,6 +5348,53 @@ procfs_stopped_by_watchpoint (ptid_t pti return 0; } +static int +procfs_insert_watchpoint (CORE_ADDR addr, int len, int type) +{ + if (!HAVE_STEPPABLE_WATCHPOINT + && !gdbarch_have_nonsteppable_watchpoint (current_gdbarch)) + { + /* When a hardware watchpoint fires off the PC will be left at + the instruction following the one which caused the + watchpoint. It will *NOT* be necessary for GDB to step over + the watchpoint. */ + return procfs_set_watchpoint (inferior_ptid, addr, len, type, 1); + } + else + { + /* When a hardware watchpoint fires off the PC will be left at + the instruction which caused the watchpoint. It will be + necessary for GDB to step over the watchpoint. */ + return procfs_set_watchpoint (inferior_ptid, addr, len, type, 0); + } +} + +static int +procfs_remove_watchpoint (CORE_ADDR addr, int len, int type) +{ + return procfs_set_watchpoint (inferior_ptid, addr, 0, 0, 0); +} + +static int +procfs_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len) +{ + /* The man page for proc(4) on Solaris 2.6 and up says that the + system can support "thousands" of hardware watchpoints, but gives + no 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'. */ + return 1; +} + +void +procfs_use_watchpoints (struct target_ops *t) +{ + t->to_stopped_by_watchpoint = procfs_stopped_by_watchpoint; + t->to_insert_watchpoint = procfs_insert_watchpoint; + t->to_remove_watchpoint = procfs_remove_watchpoint; + t->to_region_ok_for_hw_watchpoint = procfs_region_ok_for_hw_watchpoint; +} + /* * Memory Mappings Functions: */ @@ -5970,6 +6016,10 @@ _initialize_procfs (void) t = procfs_target (); +#ifdef TARGET_HAS_HARDWARE_WATCHPOINTS + procfs_use_watchpoints (t); +#endif + add_target (t); add_info ("proc", info_proc_cmd, _("\ Index: src/gdb/config/i386/nm-i386sol2.h =================================================================== --- src.orig/gdb/config/i386/nm-i386sol2.h 2009-04-30 11:39:17.000000000 +0100 +++ src/gdb/config/i386/nm-i386sol2.h 2009-05-04 11:33:44.000000000 +0100 @@ -20,17 +20,6 @@ #define TARGET_HAS_HARDWARE_WATCHPOINTS -/* The man page for proc4 on solaris 6 and 7 says that the system - 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. - It will *NOT* be necessary for GDB to step over the watchpoint. */ -#define HAVE_CONTINUABLE_WATCHPOINT 1 - /* Solaris x86 2.6 and 2.7 targets have a kernel bug when stepping over an instruction that causes a page fault without triggering a hardware watchpoint. The kernel properly notices that it shouldn't @@ -41,17 +30,4 @@ step anyway. */ #define CANNOT_STEP_HW_WATCHPOINTS -extern int procfs_stopped_by_watchpoint (ptid_t); -#define STOPPED_BY_WATCHPOINT(W) \ - procfs_stopped_by_watchpoint(inferior_ptid) - -/* Use these macros for watchpoint insertion/deletion. */ -/* type can be 0: write watch, 1: read watch, 2: access watch (read/write) */ - -extern int procfs_set_watchpoint (ptid_t, CORE_ADDR, int, int, int); -#define target_insert_watchpoint(ADDR, LEN, TYPE) \ - procfs_set_watchpoint (inferior_ptid, ADDR, LEN, TYPE, 1) -#define target_remove_watchpoint(ADDR, LEN, TYPE) \ - procfs_set_watchpoint (inferior_ptid, ADDR, 0, 0, 0) - #endif /* NEW_PROC_API */ Index: src/gdb/config/mips/nm-irix5.h =================================================================== --- src.orig/gdb/config/mips/nm-irix5.h 2009-04-30 11:39:18.000000000 +0100 +++ src/gdb/config/mips/nm-irix5.h 2009-05-04 11:33:44.000000000 +0100 @@ -19,26 +19,3 @@ along with this program. If not, see . */ #define TARGET_HAS_HARDWARE_WATCHPOINTS - -/* TARGET_CAN_USE_HARDWARE_WATCHPOINT is now defined to go through - the target vector. For Irix5, procfs_can_use_hw_watchpoint() - should be invoked. */ - -/* When a hardware watchpoint fires off the PC will be left at the - instruction which caused the watchpoint. It will be necessary for - GDB to step over the watchpoint. */ - -#define STOPPED_BY_WATCHPOINT(W) \ - procfs_stopped_by_watchpoint(inferior_ptid) -extern int procfs_stopped_by_watchpoint (ptid_t); - -/* Use these macros for watchpoint insertion/deletion. */ -/* type can be 0: write watch, 1: read watch, 2: access watch (read/write) */ -#define target_insert_watchpoint(ADDR, LEN, TYPE) \ - procfs_set_watchpoint (inferior_ptid, ADDR, LEN, TYPE, 0) -#define target_remove_watchpoint(ADDR, LEN, TYPE) \ - 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 - Index: src/gdb/config/sparc/nm-sol2.h =================================================================== --- src.orig/gdb/config/sparc/nm-sol2.h 2009-04-30 11:39:17.000000000 +0100 +++ src/gdb/config/sparc/nm-sol2.h 2009-05-04 11:33:45.000000000 +0100 @@ -30,31 +30,6 @@ #define TARGET_HAS_HARDWARE_WATCHPOINTS -/* The man page for proc(4) on Solaris 2.6 and up says that the system - can support "thousands" of hardware watchpoints, but gives no - 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 - *NOT* be necessary for GDB to step over the watchpoint. */ -#define HAVE_CONTINUABLE_WATCHPOINT 1 - -extern int procfs_stopped_by_watchpoint (ptid_t); -#define STOPPED_BY_WATCHPOINT(W) \ - procfs_stopped_by_watchpoint(inferior_ptid) - -/* Use these macros for watchpoint insertion/deletion. TYPE can be 0 - (write watch), 1 (read watch), 2 (access watch (read/write). */ - -extern int procfs_set_watchpoint (ptid_t, CORE_ADDR, int, int, int); -#define target_insert_watchpoint(ADDR, LEN, TYPE) \ - procfs_set_watchpoint (inferior_ptid, ADDR, LEN, TYPE, 1) -#define target_remove_watchpoint(ADDR, LEN, TYPE) \ - procfs_set_watchpoint (inferior_ptid, ADDR, 0, 0, 0) - #endif /* NEW_PROC_API */ #endif /* nm-sol2.h */