From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26543 invoked by alias); 20 Feb 2008 11:39:11 -0000 Received: (qmail 26532 invoked by uid 22791); 20 Feb 2008 11:39:09 -0000 X-Spam-Status: No, hits=2.5 required=5.0 tests=AWL,BAYES_50,J_CHICKENPOX_34,J_CHICKENPOX_44,J_CHICKENPOX_45,J_CHICKENPOX_46,J_CHICKENPOX_47,J_CHICKENPOX_48,J_CHICKENPOX_54,J_CHICKENPOX_57,J_CHICKENPOX_64,J_CHICKENPOX_65,J_CHICKENPOX_66,J_CHICKENPOX_74,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 20 Feb 2008 11:38:51 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id m1KBcnHL004864 for ; Wed, 20 Feb 2008 06:38:49 -0500 Received: from pobox-2.corp.redhat.com (pobox-2.corp.redhat.com [10.11.255.15]) by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m1KBcnnY019909 for ; Wed, 20 Feb 2008 06:38:49 -0500 Received: from localhost.localdomain (vpn-6-40.fab.redhat.com [10.33.6.40]) by pobox-2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m1KBcljA012917 for ; Wed, 20 Feb 2008 06:38:48 -0500 Message-ID: <47BC1147.6060005@redhat.com> Date: Wed, 20 Feb 2008 11:39:00 -0000 From: Phil Muldoon User-Agent: Thunderbird 2.0.0.9 (X11/20071115) MIME-Version: 1.0 To: frysk@sourceware.org Subject: Re: [SCM] master: Add watchpoint interfaces. References: <20080220104622.3044.qmail@sourceware.org> In-Reply-To: <20080220104622.3044.qmail@sourceware.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.58 on 172.16.52.254 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-q1/txt/msg00071.txt.bz2 Hi all These interfaces constitute the form of the api calls for watchpoints. Keen observers will note they look just like the Code (breakpoint) observers, and are in fact a copy of them. I'd like to keep the interfaces the same. Maybe later we can look at see if there is need for two separate interfaces. Anyway, a quick note that these are committed without tests. Given that the the proc namespace is under constant development/refactoring, and I spend a lot of time just merging; and that I want to get as much code-to-eyes as early on as possible, I thought it prudent to commit the interface code. Regards Phil pmuldoon@sourceware.org wrote: > The branch, master has been updated > via a698552443da8465f68ad185f7760ef1c9dcdf2a (commit) > from 312f766777c680ddc21a09ef364283882e1cfdac (commit) > > Those revisions listed above that are new to this repository have > not appeared on any other notification email. > > - Log ----------------------------------------------------------------- > commit a698552443da8465f68ad185f7760ef1c9dcdf2a > Author: Phil Muldoon > Date: Wed Feb 20 10:46:12 2008 +0000 > > Add watchpoint interfaces. > > 2008-02-20 Phil Muldoon > > * TaskObserver.java: Add watch interface. > > 2008-02-20 Phil Muldoon > > * LinuxPtraceProc.java (requestAddWatchObservers): New. > (requestDeleteWatchObserver): New. > * LinuxPtraceTask.java (requestAddWatchObservers): New. > (requestDeleteWatchObserver): New. > * Watchpoint.java: New. > * WatchpointAddresses.java: New. > > ----------------------------------------------------------------------- > > Summary of changes: > frysk-core/frysk/proc/ChangeLog | 5 + > frysk-core/frysk/proc/TaskObserver.java | 12 + > frysk-core/frysk/proc/live/ChangeLog | 9 + > frysk-core/frysk/proc/live/LinuxPtraceProc.java | 26 +++- > frysk-core/frysk/proc/live/LinuxPtraceTask.java | 116 ++++++++++ > frysk-core/frysk/proc/live/Watchpoint.java | 229 ++++++++++++++++++++ > ...ointAddresses.java => WatchpointAddresses.java} | 62 +++---- > 7 files changed, 421 insertions(+), 38 deletions(-) > create mode 100644 frysk-core/frysk/proc/live/Watchpoint.java > copy frysk-core/frysk/proc/live/{BreakpointAddresses.java => WatchpointAddresses.java} (80%) > > First 500 lines of diff: > diff --git a/frysk-core/frysk/proc/ChangeLog b/frysk-core/frysk/proc/ChangeLog > index 260a1b0..d0e9ac2 100644 > --- a/frysk-core/frysk/proc/ChangeLog > +++ b/frysk-core/frysk/proc/ChangeLog > @@ -1,3 +1,8 @@ > +2008-02-20 Phil Muldoon > + > + * TaskObserver.java: Add watch interface. > + > + > 2008-02-14 Andrew Cagney > > * Proc.java (pid): New; Pass to constructors. > diff --git a/frysk-core/frysk/proc/TaskObserver.java b/frysk-core/frysk/proc/TaskObserver.java > index 2371e8a..14f47d6 100644 > --- a/frysk-core/frysk/proc/TaskObserver.java > +++ b/frysk-core/frysk/proc/TaskObserver.java > @@ -212,6 +212,18 @@ public interface TaskObserver > Action updateHit (Task task, long address); > } > > + public interface Watch > + extends TaskObserver > + { > + /** > + * The task has hit the breakpoint. Return Action.BLOCK to > + * block the task's further execution. Note that all Tasks of > + * a Proc share their breakpoints, so this method needs to > + * check the actual Task that got hit. > + */ > + Action updateHit (Task task, long address); > + } > + > /** > * Interface used to notify of a Task that has has been attached, > * and is about to resume execution in that state. Only after a > diff --git a/frysk-core/frysk/proc/live/ChangeLog b/frysk-core/frysk/proc/live/ChangeLog > index 7647a09..f38b01c 100644 > --- a/frysk-core/frysk/proc/live/ChangeLog > +++ b/frysk-core/frysk/proc/live/ChangeLog > @@ -1,3 +1,12 @@ > +2008-02-20 Phil Muldoon > + > + * LinuxPtraceProc.java (requestAddWatchObservers): New. > + (requestDeleteWatchObserver): New. > + * LinuxPtraceTask.java (requestAddWatchObservers): New. > + (requestDeleteWatchObserver): New. > + * Watchpoint.java: New. > + * WatchpointAddresses.java: New. > + > 2008-02-19 Andrew Cagney > > * IsaFactory.java: Update to match lib.dwfl. > diff --git a/frysk-core/frysk/proc/live/LinuxPtraceProc.java b/frysk-core/frysk/proc/live/LinuxPtraceProc.java > index 6f59dd0..87c366a 100644 > --- a/frysk-core/frysk/proc/live/LinuxPtraceProc.java > +++ b/frysk-core/frysk/proc/live/LinuxPtraceProc.java > @@ -597,6 +597,30 @@ public class LinuxPtraceProc extends LiveProc { > Manager.eventLoop.add(to); > } > > + > + > + /** > + * (Internal) Tell the process to add the specified Code > + * Observation, attaching to the process if necessary. Adds a > + * TaskCodeObservation to the eventloop which instructs the task > + * to install the breakpoint if necessary. > + */ > + void requestAddWatchObserver(Task task, TaskObservable observable, > + TaskObserver.Watch observer, > + final long address, > + final int length) { > + } > + > + /** > + * (Internal) Tell the process to delete the specified Code > + * Observation, detaching from the process if necessary. > + */ > + void requestDeleteWatchObserver(Task task, TaskObservable observable, > + TaskObserver.Watch observer, > + final long address, > + final int length) { > + } > + > /** > * Class describing the action to take on the suspended Task > * before adding or deleting an Instruction observer. No > @@ -680,7 +704,7 @@ public class LinuxPtraceProc extends LiveProc { > * XXX: Should not be public. > */ > public final BreakpointAddresses breakpoints; > - > + > // List of available addresses for out of line stepping. > // Used a lock in getOutOfLineAddress() and doneOutOfLine(). > private final ArrayList outOfLineAddresses = new ArrayList(); > diff --git a/frysk-core/frysk/proc/live/LinuxPtraceTask.java b/frysk-core/frysk/proc/live/LinuxPtraceTask.java > index 5f86dd6..40e14b5 100644 > --- a/frysk-core/frysk/proc/live/LinuxPtraceTask.java > +++ b/frysk-core/frysk/proc/live/LinuxPtraceTask.java > @@ -82,6 +82,7 @@ public class LinuxPtraceTask extends LiveTask { > super(proc, pid); > ((LinuxPtraceHost)proc.getHost()).putTask(tid, this); > newState = LinuxPtraceTaskState.detachedState(); > + this.watchpoints = new WatchpointAddresses(this); > } > /** > * Create a new attached clone of Task. > @@ -91,6 +92,7 @@ public class LinuxPtraceTask extends LiveTask { > super(task, clone); > ((LinuxPtraceHost)getProc().getHost()).putTask(tid, this); > newState = LinuxPtraceTaskState.clonedState(((LinuxPtraceTask)task).getState ()); > + this.watchpoints = new WatchpointAddresses(this); > } > /** > * Create a new attached main Task of Proc. > @@ -109,6 +111,8 @@ public class LinuxPtraceTask extends LiveTask { > }; > proc.handleAddObservation(ob); > } > + this.watchpoints = new WatchpointAddresses(this); > + > } > > /** > @@ -519,6 +523,23 @@ public class LinuxPtraceTask extends LiveTask { > return blockers.size(); > } > /** > + * Add a TaskObserver.Watch observer > + * (hardware only) > + */ > + public void requestAddWatchObserver(TaskObserver.Watch o, long address, int length) { > + fine.log(this,"requestAddWatchObserver"); > + requestAddWatchObserver(this, codeObservers, o, address, length); > + } > + > + /** > + * Delete TaskObserver.Code for the TaskObserver pool. > + */ > + public void requestDeleteWatchObserver(TaskObserver.Watch o, long address, int length) { > + fine.log(this, "requestDeleteWatcheObserver"); > + requestDeleteWatchObserver(this, codeObservers, o, address, length); > + } > + > + /** > * Add a TaskObserver.Cloned observer. > */ > public void requestAddClonedObserver(TaskObserver.Cloned o) { > @@ -856,6 +877,101 @@ public class LinuxPtraceTask extends LiveTask { > } > return blockers.size(); > } > + > + /** > + * Class describing the action to take on the suspended Task > + * before adding or deleting a Watchpoint observer. > + */ > + final class WatchpointAction implements Runnable { > + private final TaskObserver.Watch watch; > + > + private final Task task; > + > + private final long address; > + > + private final int length; > + > + private final boolean addition; > + > + WatchpointAction(TaskObserver.Watch watch, Task task, long address, int length, > + boolean addition) { > + this.watch = watch; > + this.task = task; > + this.address = address; > + this.addition = addition; > + this.length = length; > + } > + > + public void run() { > + if (addition) { > + boolean mustInstall = watchpoints.addBreakpoint(watch, address, length); > + if (mustInstall) { > + Watchpoint watchpoint; > + watchpoint = Watchpoint.create(address, length, LinuxPtraceTask.this); > + watchpoint.install(task); > + } > + } else { > + boolean mustRemove = watchpoints.removeBreakpoint(watch, address, length); > + if (mustRemove) { > + Watchpoint watchpoint; > + watchpoint = Watchpoint.create(address, length, LinuxPtraceTask.this); > + watchpoint.remove(task); > + } > + } > + } > + } > + > + public final WatchpointAddresses watchpoints; > + > + /** > + * (Internal) Tell the task to add the specified Watchpoint > + * observation, attaching to the task if necessary. Adds a > + * TaskCodeObservation to the eventloop which instructs the task > + * to install the breakpoint if necessary. > + */ > + void requestAddWatchObserver(Task task, TaskObservable observable, > + TaskObserver.Watch observer, > + final long address, > + final int length) { > + WatchpointAction watchAction = new WatchpointAction(observer, task, address, length, true); > + TaskObservation to; > + to = new TaskObservation((LinuxPtraceTask) task, observable, observer, > + watchAction, true) { > + public void execute() { > + handleAddObservation(this); > + } > + public boolean needsSuspendedAction() { > + return watchpoints.getCodeObservers(address, length) == null; > + } > + }; > + Manager.eventLoop.add(to); > + } > + > + /** > + * (Internal) Tell the process to delete the specified Watchpoint > + * observation, detaching from the process if necessary. > + */ > + void requestDeleteWatchObserver(Task task, TaskObservable observable, > + TaskObserver.Watch observer, > + final long address, > + final int length) { > + WatchpointAction watchAction = new WatchpointAction(observer, task, address, length, false); > + TaskObservation to; > + to = new TaskObservation((LinuxPtraceTask)task, observable, observer, > + watchAction, false) { > + public void execute() { > + newState = oldState().handleDeleteObservation(LinuxPtraceTask.this, this); > + } > + > + public boolean needsSuspendedAction() { > + return watchpoints.getCodeObservers(address, length).size() == 1; > + } > + }; > + > + Manager.eventLoop.add(to); > + } > + > + > /** > * Add TaskObserver.Code to the TaskObserver pool. > */ > diff --git a/frysk-core/frysk/proc/live/Watchpoint.java b/frysk-core/frysk/proc/live/Watchpoint.java > new file mode 100644 > index 0000000..59c25a4 > --- /dev/null > +++ b/frysk-core/frysk/proc/live/Watchpoint.java > @@ -0,0 +1,229 @@ > +// This file is part of the program FRYSK. > +// > +// Copyright 2006, 2007, 2008 Red Hat Inc. > +// > +// FRYSK is free software; you can redistribute it and/or modify it > +// under the terms of the GNU General Public License as published by > +// the Free Software Foundation; version 2 of the License. > +// > +// FRYSK is distributed in the hope that it will be useful, but > +// WITHOUT ANY WARRANTY; without even the implied warranty of > +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > +// General Public License for more details. > +// > +// You should have received a copy of the GNU General Public License > +// along with FRYSK; if not, write to the Free Software Foundation, > +// Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA. > +// > +// In addition, as a special exception, Red Hat, Inc. gives You the > +// additional right to link the code of FRYSK with code not covered > +// under the GNU General Public License ("Non-GPL Code") and to > +// distribute linked combinations including the two, subject to the > +// limitations in this paragraph. Non-GPL Code permitted under this > +// exception must only link to the code of FRYSK through those well > +// defined interfaces identified in the file named EXCEPTION found in > +// the source code files (the "Approved Interfaces"). The files of > +// Non-GPL Code may instantiate templates or use macros or inline > +// functions from the Approved Interfaces without causing the > +// resulting work to be covered by the GNU General Public > +// License. Only Red Hat, Inc. may make changes or additions to the > +// list of Approved Interfaces. You must obey the GNU General Public > +// License in all respects for all of the FRYSK code and other code > +// used in conjunction with FRYSK except the Non-GPL Code covered by > +// this exception. If you modify this file, you may extend this > +// exception to your version of the file, but you are not obligated to > +// do so. If you do not wish to provide this exception without > +// modification, you must delete this exception statement from your > +// version and license this file solely under the GPL without > +// exception. > + > + > +package frysk.proc.live; > + > +import java.util.HashMap; > + > +import frysk.proc.Task; > + > +/** > + * Internal proc class that represents a Breakpoint at a certain > + * address in a Proc. Some attempts are made to have synchronize > + * different Breakpoint instances at the same address in the same > + * Proc, but currently this isn't a full singleton. > + */ > +public class Watchpoint implements Comparable > +{ > + // These two fields define a Breakpoint > + private final long address; > + private final int length; > + private final Task task; > + > + > + // Static cache of installed break points. > + private static HashMap installed = new HashMap(); > + > + > + > + /** > + * Private constructor called by create to record address and > + * proc. > + */ > + private Watchpoint(long address, int length, Task task) > + { > + if (task == null) > + throw new NullPointerException("Cannot place a watchpoint when task == null."); > + > + this.address = address; > + this.task = task; > + this.length = length; > + > + if (this.length <= 0) > + throw new RuntimeException("Watchpoint length has to be > 0"); > + } > + > + /** > + * Creates a Breakpoint for the Proc at the given Address but does > + * not set it yet. Returns the appropriate Breakpoint depending on > + * host type. If a Breakpoint for this address and proc is already > + * installed that Breakpoint will be returned. > + */ > + public static Watchpoint create(long address, int length, Task task) > + { > + Watchpoint breakpoint = new Watchpoint(address, length, task); > + > + // If possible return an existing installed breakpoint. > + synchronized (installed) > + { > + Watchpoint existing = (Watchpoint) installed.get(breakpoint); > + if (existing != null) > + return existing; > + } > + return breakpoint; > + } > + > + public long getAddress() > + { > + return address; > + } > + > + > + /** > + * Installs breakpoint. Caller must make sure there is no breakpoint set > + * at that address yet and that install() is not called again till remove() > + * is called on it. > + */ > + public void install(Task task) > + { > + synchronized (installed) > + { > + Watchpoint existing = (Watchpoint) installed.get(this); > + if (existing != null) > + throw new IllegalStateException("Already installed: " + this); > + > + installed.put(this, this); > + > + set(task); > + } > + } > + > + /** > + * Actually sets the breakpoint. > + */ > + private void set(Task task) > + { > +// ByteBuffer buffer = ((LinuxPtraceTask)task).getRawMemory(); > +// Isa isa = ((LinuxPtraceTask)task).getIsaFIXME(); > +// Instruction bpInstruction = isa.getBreakpointInstruction(); > +// > +// origInstruction = isa.getInstruction(buffer, address); > +// > +// // Put in the breakpoint. > +// byte[] bs = bpInstruction.getBytes(); > +// buffer.position(address); > +// buffer.put(bs); > + } > + > + /** > + * Removes the breakpoint. Caller must make sure it is called only > + * when it is installed and not in the middle of a step. > + */ > + public void remove(Task task) > + { > + synchronized (installed) > + { > + if (! this.equals(installed.remove(this))) > + throw new IllegalStateException("Not installed: " + this); > + > + reset(task); > + } > + } > + > + /** > + * Actually removes the breakpoint. > + */ > + private void reset(Task task) > + { > +// ByteBuffer buffer = ((LinuxPtraceTask)task).getRawMemory(); > +// buffer.position(address); > +// > +// Isa isa = ((LinuxPtraceTask)task).getIsaFIXME(); > +// Instruction bpInstruction = isa.getBreakpointInstruction(); > +// byte[] bp = bpInstruction.getBytes(); > +// > +// byte[] bs = origInstruction.getBytes(); > +// > +// // Only need to put back the part of the original instruction > +// // covered by the breakpoint instruction bytes. > +// buffer.put(bs, 0, bp.length); > + } > + > + > + /** > + * Returns the Proc to which this breakpoint belongs. > + */ > + public Task getTask() > + { > + return this.task; > + } > + > + /** > + * Returns true if break point is installed and not yet removed. > + */ > + public boolean isInstalled() > + { > + synchronized(installed) > + { > + return this.equals(installed.get(this)); > + } > + } > + > + // Utility methods for keeping the map of breakpoints. > + > + public int hashCode() > + { > + return (int) (address ^ (address >>> 32)); > + } > + > + public boolean equals(Object o) > + { > + if (o == null || o.getClass() != this.getClass()) > + return false; > + > + Watchpoint other = (Watchpoint) o; > + return other.task.equals(task) && other.address == address; > + } > + > + /** > + * Uses natural ordering on address. > + */ > + public int compareTo(Object o) > + { > + Watchpoint other = (Watchpoint) o; > + return (int) (this.address - other.address); > + } > + > + public String toString() > + { > + return this.getClass().getName() + "[task=" + task > + + ", address=0x" + Long.toHexString(address) + "]"; > + } > +} > diff --git a/frysk-core/frysk/proc/live/BreakpointAddresses.java b/frysk-core/frysk/proc/live/WatchpointAddresses.java > similarity index 80% > copy from frysk-core/frysk/proc/live/BreakpointAddresses.java > copy to frysk-core/frysk/proc/live/WatchpointAddresses.java > index dc59c34..aba7d16 100644 > --- a/frysk-core/frysk/proc/live/BreakpointAddresses.java > +++ b/frysk-core/frysk/proc/live/WatchpointAddresses.java > @@ -40,13 +40,13 @@ > > package frysk.proc.live; > > -import frysk.proc.TaskObserver; > -import frysk.proc.Proc; > > > hooks/post-receive > -- > frysk system monitor/debugger >