From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5242 invoked by alias); 7 May 2010 15:25:45 -0000 Received: (qmail 5227 invoked by uid 22791); 7 May 2010 15:25:43 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 07 May 2010 15:25:38 +0000 Received: (qmail 10556 invoked from network); 7 May 2010 15:25:36 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 7 May 2010 15:25:36 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [RFC] Add watchpoint hit address function to procfs.c Date: Fri, 07 May 2010 15:25:00 -0000 User-Agent: KMail/1.12.2 (Linux/2.6.31-20-generic; KDE/4.3.2; x86_64; ; ) Cc: "Pierre Muller" References: <006601cae54b$368452f0$a38cf8d0$@muller@ics-cnrs.unistra.fr> In-Reply-To: <006601cae54b$368452f0$a38cf8d0$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201005071625.18583.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: 2010-05/txt/msg00174.txt.bz2 On Monday 26 April 2010 15:17:33, Pierre Muller wrote: > The following patch adds support for > to_stopped_data_address field of target_ops, > using > prstatus.pr_lwp.pr_info.__data.__fault.__addr field. > (or prstatus.pr_info.__data.__fault.__addr if NEW_PROC_API > is not defined) I found that in the OpenSolaris sources. > > It works for my OpenSolaris 2.11. > > The main problem is that I have no ideas if this works for other > targets using procfs.c source. > > How should this be modified to avoid problem if other targets > do not have/set these fields? > > Comments welcome. > > Pierre Muller > Pascal language support maintainer for GDB > > 2010-04-26 Pierre Muller > > * procfs.c (proc_watchpoint_address): New function. > (procfs_stopped_data_address): New function. > (procfs_use_watchpoints): Register new watchpoint related > function. > > Index: procfs.c > =================================================================== > RCS file: /cvs/src/src/gdb/procfs.c,v > retrieving revision 1.131 > diff -u -p -r1.131 procfs.c > --- procfs.c 20 Apr 2010 22:36:35 -0000 1.131 > +++ procfs.c 26 Apr 2010 14:05:29 -0000 > @@ -1312,6 +1312,30 @@ proc_what (procinfo *pi) > return pi->prstatus.pr_what; > #endif > } > +/* > + * Function: proc_watchpoint_address > + * > + * returns the prstatus.pr_lwp.pr_info.__data.__fault.__addr field. > + */ > + > +CORE_ADDR > +proc_watchpoint_address (procinfo *pi) > +{ > + if (!pi->status_valid) > + if (!proc_get_status (pi)) > + return 0; /* FIXME: not a good failure value (but what is?) */ Add a new CORE_ADDR pointer parameter, and use that as output. C hange the return type to int to indicate failure/success. That is, model the interface of this function from to_stopped_data_address's interface. > + > +#ifdef NEW_PROC_API > + return unsigned_pointer_to_address (target_gdbarch, > + builtin_type (target_gdbarch)->builtin_data_ptr, > + (gdb_byte *) &pi->prstatus.pr_lwp.pr_info.__data.__fault.__addr); > +#else > + return unsigned_pointer_to_address (target_gdbarch, > + builtin_type (target_gdbarch)->builtin_data_ptr, > + (gdb_byte *) &pi->prstatus.pr_info.__data.__fault.__addr); > +#endif Isn't there a macro to access __data.__fault.__addr ? I'd have guessed pi->prstatus.pr_info.sa_addr, similarly to how we use pi->prstatus.pr_lwp.pr_info.si_signo already. That is, pr_info is a siginfo_t. That would probably get rid of the NEW_PROC_API usage and worries. > +} > + > > #ifndef PIOCSSPCACT /* The following is not supported on OSF. */ > /* > @@ -5560,6 +5584,45 @@ procfs_stopped_by_watchpoint (void) > } > return 0; > } > +/* > + * Function procfs_stopped_data_address > + * Returns non-zero if we can find the position > + * of the triggring watchpoint. > + */ > + > +static int > +procfs_stopped_data_address (struct target_ops *targ, CORE_ADDR *addr) > +{ > + CORE_ADDR waddr; > + procinfo *pi; > + int watchpoint_hit = 0; > + > + pi = find_procinfo_or_die (PIDGET (inferior_ptid), 0); > + > + if (!pi) /* If no process, then not stopped by watchpoint! */ > + return 0; Find_procinfo_or_die throws if the procinfo isn't found (it's the die part), so this check is dead code. I wonder if this procinfo is the correct one. Shouldn't this look for the the procinfo for the current thread (the tid in inferior_ptid), not the one for the main thread? > + > + if (proc_flags (pi) & (PR_STOPPED | PR_ISTOP)) > + { > + if (proc_why (pi) == PR_FAULTED) > + { > +#ifdef FLTWATCH > + if (proc_what (pi) == FLTWATCH) > + watchpoint_hit = 1; > +#endif > +#ifdef FLTKWATCH > + if (proc_what (pi) == FLTKWATCH) > + watchpoint_hit = 1; > +#endif > + } > + } > + if (!watchpoint_hit) > + return 0; You can assume that this target_ops method is only ever called if a watchpoint is hit in the first place (if target_stopped_by_watchpoint returned true), so remove all this (duplicated) code. > + waddr = proc_watchpoint_address (pi); > + if (addr) > + *addr = waddr; > + return (waddr != 0); > +} > > static int > procfs_insert_watchpoint (CORE_ADDR addr, int len, int type) > @@ -5607,6 +5670,7 @@ procfs_use_watchpoints (struct target_op > t->to_remove_watchpoint = procfs_remove_watchpoint; > t->to_region_ok_for_hw_watchpoint = procfs_region_ok_for_hw_watchpoint; > t->to_can_use_hw_breakpoint = procfs_can_use_hw_breakpoint; > + t->to_stopped_data_address = procfs_stopped_data_address; > } > > /* > > -- Pedro Alves