From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13121 invoked by alias); 2 Apr 2008 07:52:53 -0000 Received: (qmail 13109 invoked by uid 22791); 2 Apr 2008 07:52:52 -0000 X-Spam-Status: No, hits=-2.6 required=5.0 tests=BAYES_00 X-Spam-Check-By: sourceware.org Received: from wildebeest.demon.nl (HELO gnu.wildebeest.org) (83.160.170.119) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 02 Apr 2008 07:52:27 +0000 Received: from cc1341701-a.deven1.ov.home.nl ([82.72.53.253]) by gnu.wildebeest.org with esmtp (Exim 4.63) (envelope-from ) id 1Jgxhu-0008Ax-BK; Wed, 02 Apr 2008 09:52:24 +0200 Subject: Re: [SCM] master: Implement hasWatchpointTriggered in abstract, and sub-classes. From: Mark Wielaard To: Phil Muldoon Cc: frysk@sourceware.org In-Reply-To: <47F2A1E1.1070404@redhat.com> References: <20080401204912.27374.qmail@sourceware.org> <47F2A1E1.1070404@redhat.com> Content-Type: text/plain Date: Wed, 02 Apr 2008 07:52:00 -0000 Message-Id: <1207122509.4953.27.camel@cc1341701-a.deven1.ov.home.nl> Mime-Version: 1.0 X-Mailer: Evolution 2.12.3 (2.12.3-3.fc8) Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact frysk-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: frysk-owner@sourceware.org X-SW-Source: 2008-q2/txt/msg00004.txt.bz2 Hi Phil, On Tue, 2008-04-01 at 21:58 +0100, Phil Muldoon wrote: > pmuldoon@sourceware.org wrote: > > This implements a function that checks the debug status register on > whether a hardware debug data breakpoint (watchpoint) has been > triggered. Normally this function is called after a SIGTRAP is issued > from ptrace. On x8664 and IA32 the first four bits represent the four > hardware breakpoint registers. If bit 0 = 1 then the debug hardware > data breakpoint (D0) has triggered, bit 1 for D1 and so on. I believe > the software is responsible for clearing the bit if set, and I have not > done this here. I'm torn whether to reset the bit if: > > debugStatus & (1L << index)) != 0; > > where debug status is the debug status register, and index is the > watchpoint to check. I think this is a little too architecture specific and a little error prone in usage. Code using this now has to iterate over all the possible watchpoint indexes for a particular architecture. Maybe this is small and trivial on all architectures, but it would feel more natural imho if there was just a method to query which watchpoint index was triggered if any. Something like: /** * Reads the Debug Status Register and checks whether * any watchpoint has fired. * * @param task - task to read the debug control * register from. * @returns the index of the watchpoint triggered, * or -1 if no watchpoint has triggered. */ public abstract int watchpointTriggered(Task task); With as possible architecture implementation something like: public int watchpointTriggered(Task task) { Register register = IA32Registers.DEBUG_STATUS; long debugStatus = task.getRegister(register); for (int index = 0; i < noOfWatchpoints; index++) if ((debugStatus & (1L << index)) != 0) return index; return -1; } (Not tested or compiled) You might actually want to make this a little bit more abstract and maybe just return the address, range and mode (read/write) that triggered instead of the index. That would need a little container class that holds these values. The benefit is that it would abstract away any architecture specific implementation details (like the index and whether or not this is done with specific cpu support or not). > Should the bit be reset in hasWatchpointTriggered? > Or in a separate function function called as a separate API? A separate method is cleaner. The problem I had with that (in the stepping flag) is that it then becomes somewhat harder to know when to clear the status flags. Since the state machine is guaranteed to check the status before any state transitions it is somewhat easier to make it a check-and-clear-status method: http://sourceware.org/ml/frysk/2007-q1/msg00024.html If you have a separate method you can either call it directly after checking the status, or in the Running.sendContinue() method. I introduced this to have one point in the ptrace state machine where a Task is set running again: http://sourceware.org/ml/frysk/2007-q2/msg00329.html We also already discussed basing the stepping flag on this new infrastructure: http://sourceware.org/ml/frysk/2007-q3/msg00324.html (And I agreed it would be somewhat cleaner than what is done now.) Cheers, Mark