* Re: [SCM] master: Implement top level interfaces. Test primary test case in TestWatchpoint. Add two modifications two stepping.handleTrappedEvent and running.handleTrappedEvent.
[not found] <20080404143840.17059.qmail@sourceware.org>
@ 2008-04-04 20:33 ` Phil Muldoon
0 siblings, 0 replies; only message in thread
From: Phil Muldoon @ 2008-04-04 20:33 UTC (permalink / raw)
To: frysk
pmuldoon@sourceware.org wrote:
This commit principally adds the top level watchpoint logic, to the
bottom. As show in the test cases in
TestWatchpoint.testWatchpointTrigger() you can now add a watchpoint
observer to a task. There is lots to be done. Immediately in the next
few weeks.
- Many more test cases. edge cases.
- Watchpoint optimization
- Better integration in the Stepping.handTrappedEvent and
Running.handleTrappedEvent. As a watchpoint (at least according to the
AMD documentation) only fires after the instruction that has
written/read to/from that address, it should be ok to solve the
watchpoint tests before any other test in the two states shown above.
However, this requires testcases, and discussion.
Regards
Phil
> The branch, master has been updated
> via dfa543f7189c681764596fc958a6d640a9558d46 (commit)
> from 1e6a205bbcbe119523df8e4a569a9aa3c9aa29b3 (commit)
>
> Those revisions listed above that are new to this repository have
> not appeared on any other notification email.
>
> - Log -----------------------------------------------------------------
> commit dfa543f7189c681764596fc958a6d640a9558d46
> Author: Phil Muldoon <pmuldoon@redhat.com>
> Date: Fri Apr 4 15:37:31 2008 +0100
>
> Implement top level interfaces. Test primary test case in TestWatchpoint. Add two modifications two stepping.handleTrappedEvent and running.handleTrappedEvent.
>
> 2008-04-04 Phil Muldoon <pmuldoon@redhat.com>
>
> * TestWatchpoint.java (testWatchpointTrigger): New.
> (WatchpointObserver): New.
> (TerminatedObserver): New.
> * WatchpointFunctionFactory.java (getWatchpointFunctions): Renamed from
> getWatchpoint.
> * TestWatchpoint: Update getWatchpoint -> getWatchpointFunctions.
>
> 2008-04-04 Phil Muldoon <pmuldoon@redhat.com>
>
> * LinuxPtraceProc.java (LinuxPtraceProc): Instatiate watchpoint
> address manager.
> (WatchpointAction): New. Install watchpoints.
> (requestAddWatchObserver): Implement.
> (requestDeleteWatchObserver): Ditto.
> * WatchpointAddresses.java (proc): Store a proc, not a task.
> (addWatchpoint): Take a length, and a write only flag.
> (removeWatchpoint): Ditto.
> (getWatchObservers): Ditto.
> (getWatchpoint): Ditto.
> * Watchpoint.java (Watchpoint): Take a task and a writeOnly flag.
> (create): Ditto
> (getLength): New.
> (isWriteOnly): New.
> (set): Actually set the watchpoint.
> (reset): Delete the watchpoint.
> * LinuxPtraceTaskState.java (Running.handleTrappedEvent): Add watchpoint
> catch/test.
> (Stepping.handleTrappedEvent): Ditto.
>
> -----------------------------------------------------------------------
>
> Summary of changes:
> frysk-core/frysk/isa/watchpoints/ChangeLog | 9 ++
> .../frysk/isa/watchpoints/TestWatchpoint.java | 87 +++++++++++++++++-
> .../isa/watchpoints/WatchpointFunctionFactory.java | 2 +-
> frysk-core/frysk/proc/live/ChangeLog | 24 +++++-
> frysk-core/frysk/proc/live/LinuxPtraceProc.java | 93 ++++++++++++++++++--
> .../frysk/proc/live/LinuxPtraceTaskState.java | 34 +++++++-
> frysk-core/frysk/proc/live/Watchpoint.java | 91 +++++++++++++------
> .../frysk/proc/live/WatchpointAddresses.java | 40 ++++-----
> 8 files changed, 315 insertions(+), 65 deletions(-)
>
> First 500 lines of diff:
> diff --git a/frysk-core/frysk/isa/watchpoints/ChangeLog b/frysk-core/frysk/isa/watchpoints/ChangeLog
> index 33613b2..4d0e585 100644
> --- a/frysk-core/frysk/isa/watchpoints/ChangeLog
> +++ b/frysk-core/frysk/isa/watchpoints/ChangeLog
> @@ -1,3 +1,12 @@
> +2008-04-04 Phil Muldoon <pmuldoon@redhat.com>
> +
> + * TestWatchpoint.java (testWatchpointTrigger): New.
> + (WatchpointObserver): New.
> + (TerminatedObserver): New.
> + * WatchpointFunctionFactory.java (getWatchpointFunctions): Renamed from
> + getWatchpoint.
> + * TestWatchpoint: Update getWatchpoint -> getWatchpointFunctions.
> +
> 2008-04-03 Phil Muldoon <pmuldoon@redhat.com>
>
> * WatchpointFunctions.java (readStatusRegister): Declare.
> diff --git a/frysk-core/frysk/isa/watchpoints/TestWatchpoint.java b/frysk-core/frysk/isa/watchpoints/TestWatchpoint.java
> index db52a10..b6f516b 100644
> --- a/frysk-core/frysk/isa/watchpoints/TestWatchpoint.java
> +++ b/frysk-core/frysk/isa/watchpoints/TestWatchpoint.java
> @@ -51,13 +51,92 @@ import lib.dwfl.ElfSymbolVisibility;
> import lib.dwfl.SymbolBuilder;
> import frysk.config.Config;
> import frysk.dwfl.DwflCache;
> +import frysk.isa.signals.Signal;
> +import frysk.proc.Action;
> +import frysk.proc.Manager;
> import frysk.proc.Proc;
> import frysk.proc.Task;
> +import frysk.proc.TaskObserver;
> import frysk.testbed.DaemonBlockedAtEntry;
> import frysk.testbed.TestLib;
> +
> public class TestWatchpoint extends TestLib {
>
>
> + public void testWatchpointTrigger () {
> + if (unresolvedOnPPC(5991))
> + return;
> +
> + DaemonBlockedAtEntry ackProc = new DaemonBlockedAtEntry(
> + Config.getPkgLibFile("funit-watchpoint"));
> + assertNotNull(ackProc);
> +
> + Proc proc = ackProc.getMainTask().getProc();
> + Task task = proc.getMainTask();
> + long address = getGlobalSymbolAddress(task,"source");
> +
> + WatchpointObserver wpo = new WatchpointObserver(address,4);
> + task.requestAddWatchObserver(wpo, address, 4, true);
> + assertRunUntilStop("Add Observer");
> + TerminatedObserver terminated = new TerminatedObserver();
> + task.requestAddTerminatedObserver(terminated);
> + assertRunUntilStop("Add Observer");
> + ackProc.requestUnblock();
> + assertRunUntilStop("Test");
> + }
> +
> + static class WatchpointObserver
> + implements TaskObserver.Watch
> + {
> +
> + long address;
> + int length;
> +
> + WatchpointObserver(long address, int length) {
> + this.address = address;
> + this.length = length;
> +
> + }
> + public Action updateHit(Task task, long address, int length) {
> + assertEquals("Triggered watchpoint address matches expected", this.address, address);
> + assertEquals("Triggered watchpoint length matches expected", this.length, length);
> + return Action.CONTINUE;
> + }
> +
> + public void addFailed(Object observable, Throwable w) {
> + fail("TaskOberer.Watch failed to be added to te task");
> + }
> +
> + public void addedTo(Object observable) {
> + Manager.eventLoop.requestStop();
> + }
> +
> + public void deletedFrom(Object observable) {
> + }
> +
> + }
> +
> + static class TerminatedObserver
> + implements TaskObserver.Terminated
> + {
> +
> + public void addFailed(Object observable, Throwable w) {
> + }
> +
> + public void addedTo(Object observable) {
> + Manager.eventLoop.requestStop();
> + }
> +
> + public void deletedFrom(Object observable) {
> + }
> +
> + public Action updateTerminated(Task task, Signal signal, int value) {
> + Manager.eventLoop.requestStop();
> + return Action.CONTINUE;
> + }
> +
> + }
> +
> public void testWatchFourBytesBitPattern() {
> // Test Four byte bit pattern in a cumulative fasion
> // across all WP registers. Assume global exact and
> @@ -69,7 +148,7 @@ public class TestWatchpoint extends TestLib {
> Task task = proc.getMainTask();
> long address = getGlobalSymbolAddress(task,"source");
> long debugControlRegister;
> - WatchpointFunctions wp = WatchpointFunctionFactory.getWatchpoint(task.getISA());
> + WatchpointFunctions wp = WatchpointFunctionFactory.getWatchpointFunctions(task.getISA());
> long savedDebugControlRegister = wp.readControlRegister(task);
> for (int i=0; i<4; i++) {
>
> @@ -122,7 +201,7 @@ public class TestWatchpoint extends TestLib {
> Task task = proc.getMainTask();
> long address = getGlobalSymbolAddress(task,"source");
> long debugControlRegister;
> - WatchpointFunctions wp = WatchpointFunctionFactory.getWatchpoint(task.getISA());
> + WatchpointFunctions wp = WatchpointFunctionFactory.getWatchpointFunctions(task.getISA());
> long savedDebugControlRegister = wp.readControlRegister(task);
> for (int i=0; i<4; i++) {
>
> @@ -177,7 +256,7 @@ public class TestWatchpoint extends TestLib {
> Task task = proc.getMainTask();
> long address = getGlobalSymbolAddress(task,"source");
> long debugControlRegister;
> - WatchpointFunctions wp = WatchpointFunctionFactory.getWatchpoint(task.getISA());
> + WatchpointFunctions wp = WatchpointFunctionFactory.getWatchpointFunctions(task.getISA());
>
> long savedDebugControlRegister = wp.readControlRegister(task);
>
> @@ -230,7 +309,7 @@ public class TestWatchpoint extends TestLib {
> Proc proc = giveMeABlockedProc();
> Task task = proc.getMainTask();
> long address = getGlobalSymbolAddress(task,"source");
> - WatchpointFunctions wp = WatchpointFunctionFactory.getWatchpoint(task.getISA());
> + WatchpointFunctions wp = WatchpointFunctionFactory.getWatchpointFunctions(task.getISA());
> for (int i=0; i<wp.getWatchpointCount(); i++)
> wp.setWatchpoint(task, i, address, lengthSet[i], true);
>
> diff --git a/frysk-core/frysk/isa/watchpoints/WatchpointFunctionFactory.java b/frysk-core/frysk/isa/watchpoints/WatchpointFunctionFactory.java
> index bf8211f..da434ce 100644
> --- a/frysk-core/frysk/isa/watchpoints/WatchpointFunctionFactory.java
> +++ b/frysk-core/frysk/isa/watchpoints/WatchpointFunctionFactory.java
> @@ -52,7 +52,7 @@ public class WatchpointFunctionFactory {
> .put(ISA.X8664, new X8664WatchpointFunctions())
> ;
>
> - public static WatchpointFunctions getWatchpoint(ISA isa) {
> + public static WatchpointFunctions getWatchpointFunctions(ISA isa) {
> return (WatchpointFunctions) watchpointTables.get(isa);
> }
> }
> diff --git a/frysk-core/frysk/proc/live/ChangeLog b/frysk-core/frysk/proc/live/ChangeLog
> index fc1f503..2c1cacf 100644
> --- a/frysk-core/frysk/proc/live/ChangeLog
> +++ b/frysk-core/frysk/proc/live/ChangeLog
> @@ -1,3 +1,25 @@
> +2008-04-04 Phil Muldoon <pmuldoon@redhat.com>
> +
> + * LinuxPtraceProc.java (LinuxPtraceProc): Instatiate watchpoint
> + address manager.
> + (WatchpointAction): New. Install watchpoints.
> + (requestAddWatchObserver): Implement.
> + (requestDeleteWatchObserver): Ditto.
> + * WatchpointAddresses.java (proc): Store a proc, not a task.
> + (addWatchpoint): Take a length, and a write only flag.
> + (removeWatchpoint): Ditto.
> + (getWatchObservers): Ditto.
> + (getWatchpoint): Ditto.
> + * Watchpoint.java (Watchpoint): Take a task and a writeOnly flag.
> + (create): Ditto
> + (getLength): New.
> + (isWriteOnly): New.
> + (set): Actually set the watchpoint.
> + (reset): Delete the watchpoint.
> + * LinuxPtraceTaskState.java (Running.handleTrappedEvent): Add watchpoint
> + catch/test.
> + (Stepping.handleTrappedEvent): Ditto.
> +
> 2008-04-02 Phil Muldoon <pmuldoon@redhat.com>
>
> * LinuxPtraceProc.java (requestAddWatchObserver): Add writeOnly flag.
> @@ -9,8 +31,6 @@
> (WatchpointAction): Delete.
> (requestAddWatchObserver): Delete private function.
> (requestDeleteWatchObserver): Delete private function.
> -
> -
>
> 2008-03-31 Stan Cox <scox@redhat.com>
>
> diff --git a/frysk-core/frysk/proc/live/LinuxPtraceProc.java b/frysk-core/frysk/proc/live/LinuxPtraceProc.java
> index 9f10c22..c4a6853 100644
> --- a/frysk-core/frysk/proc/live/LinuxPtraceProc.java
> +++ b/frysk-core/frysk/proc/live/LinuxPtraceProc.java
> @@ -86,6 +86,7 @@ public class LinuxPtraceProc extends LiveProc {
> this.newState = LinuxPtraceProcState.initial(false);
> this.stat = stat;
> this.breakpoints = new BreakpointAddresses(this);
> + this.watchpoints = new WatchpointAddresses(this);
> ((LinuxPtraceHost)host).addProc(this);
> }
>
> @@ -97,6 +98,7 @@ public class LinuxPtraceProc extends LiveProc {
> super(task, fork);
> this.newState = LinuxPtraceProcState.initial(true);
> this.breakpoints = new BreakpointAddresses(this);
> + this.watchpoints = new WatchpointAddresses(this);
> ((LinuxPtraceHost)getHost()).addProc(this);
> }
>
> @@ -617,27 +619,105 @@ public class LinuxPtraceProc extends LiveProc {
>
>
> /**
> - * (Internal) Tell the process to add the specified Code
> + * Class describing the action to take on the suspended Task
> + * before adding or deleting a Code 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 writeOnly;
> +
> + private final boolean addition;
> +
> + WatchpointAction(TaskObserver.Watch watch, Task task, long address,
> + int length, boolean writeOnly, boolean addition) {
> + this.watch = watch;
> + this.task = task;
> + this.address = address;
> + this.addition = addition;
> + this.length = length;
> + this.writeOnly = writeOnly;
> +
> + }
> +
> + public void run() {
> + if (addition) {
> + boolean mustInstall = watchpoints.addWatchpoint(watch, task, address, length, writeOnly);
> + if (mustInstall) {
> + Watchpoint watchpoint;
> + watchpoint = Watchpoint.create(address, length, writeOnly, task);
> + watchpoint.install(task);
> + }
> + } else {
> + boolean mustRemove = watchpoints.removeWatchpoint(watch, task, address, length, writeOnly);
> + if (mustRemove) {
> + Watchpoint watchpoint;
> + watchpoint = Watchpoint.create(address, length, writeOnly, task);
> + watchpoint.remove(task);
> + }
> + }
> + }
> + }
> +
> +
> + /**
> + * (Internal) Tell the process to add the specified Watch
> * Observation, attaching to the process if necessary. Adds a
> * TaskCodeObservation to the eventloop which instructs the task
> - * to install the breakpoint if necessary.
> + * to install the watchpoint if necessary.
> */
> - void requestAddWatchObserver(Task task, TaskObservable observable,
> + void requestAddWatchObserver(final Task task, TaskObservable observable,
> TaskObserver.Watch observer,
> final long address,
> final int length,
> - boolean writeOnly) {
> + final boolean writeOnly) {
> + fine.log(this, "requestAddWatchObserver");
> + WatchpointAction wpa = new WatchpointAction(observer, task, address, length, writeOnly, true);
> + TaskObservation to;
> + to = new TaskObservation((LinuxPtraceTask) task, observable, observer, wpa, true) {
> + public void execute() {
> + handleAddObservation(this);
> + }
> + public boolean needsSuspendedAction() {
> + return watchpoints.getWatchObservers(task, address, length, writeOnly) == null;
> + }
> + };
> + Manager.eventLoop.add(to);
> +
> +
> }
>
> /**
> * (Internal) Tell the process to delete the specified Code
> * Observation, detaching from the process if necessary.
> */
> - void requestDeleteWatchObserver(Task task, TaskObservable observable,
> + void requestDeleteWatchObserver(final Task task, TaskObservable observable,
> TaskObserver.Watch observer,
> final long address,
> final int length,
> - boolean writeOnly) {
> + final boolean writeOnly) {
> +
> + fine.log(this, "requestDeleteWatchObserver");
> + WatchpointAction wpa = new WatchpointAction(observer, task, address, length, writeOnly, false);
> + TaskObservation to;
> + to = new TaskObservation((LinuxPtraceTask)task, observable, observer, wpa, false) {
> + public void execute() {
> + newState = oldState().handleDeleteObservation(LinuxPtraceProc.this, this);
> + }
> +
> + public boolean needsSuspendedAction() {
> + return watchpoints.getWatchObservers(task, address, length, writeOnly).size() == 1;
> + }
> + };
> +
> + Manager.eventLoop.add(to);
> +
> }
>
> /**
> @@ -756,6 +836,7 @@ public class LinuxPtraceProc extends LiveProc {
> * XXX: Should not be public.
> */
> public final BreakpointAddresses breakpoints;
> + public final WatchpointAddresses watchpoints;
>
> // List of available addresses for out of line stepping.
> // Used a lock in getOutOfLineAddress() and doneOutOfLine().
> diff --git a/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java b/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java
> index 07c9bd5..c08ff72 100644
> --- a/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java
> +++ b/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java
> @@ -41,6 +41,8 @@ package frysk.proc.live;
>
> import frysk.sys.SignalSet;
> import frysk.sys.proc.Status;
> +import frysk.isa.watchpoints.WatchpointFunctionFactory;
> +import frysk.isa.watchpoints.WatchpointFunctions;
> import frysk.proc.TaskObserver;
> import frysk.proc.Observer;
> import frysk.proc.Observable;
> @@ -942,9 +944,22 @@ abstract class LinuxPtraceTaskState extends State {
> LinuxPtraceTaskState handleTrappedEvent(LinuxPtraceTask task) {
> fine.log("handleTrappedEvent", task);
>
> + // First test if this is a watchpoint event.
> + WatchpointFunctions watchpointFunction = WatchpointFunctionFactory.getWatchpointFunctions(task.getISA());
> + for (int i=0; i<watchpointFunction.getWatchpointCount(); i++) {
> + // Test if a watchpoint has fired
> + if (watchpointFunction.hasWatchpointTriggered(task, i)) {
> + frysk.isa.watchpoints.Watchpoint trigger = watchpointFunction.readWatchpoint(task, i);
> + int blockers = task.notifyWatchpoint(trigger.getAddress(), trigger.getRange());
> + if (blockers == 0)
> + return sendContinue(task, Signal.NONE);
> + else
> + return blockedContinue;
> + }
> + }
> +
> Isa isa;
> isa = task.getIsaFIXME();
> -
> // First see if this was just an indication the we stepped.
> // And see if we were stepping a breakpoint. Or whether we
> // installed a breakpoint at the address. Otherwise it is a
> @@ -1062,7 +1077,22 @@ abstract class LinuxPtraceTaskState extends State {
> */
> LinuxPtraceTaskState handleTrappedEvent(LinuxPtraceTask task) {
> fine.log("handleTrappedEvent", task);
> -
> +
> + // First test if this is a watchpoint event.
> + WatchpointFunctions watchpointFunction = WatchpointFunctionFactory.getWatchpointFunctions(task.getISA());
> + for (int i=0; i<watchpointFunction.getWatchpointCount(); i++) {
> + // Test if a watchpoint has fired
> + if (watchpointFunction.hasWatchpointTriggered(task, i)) {
> + frysk.isa.watchpoints.Watchpoint trigger = watchpointFunction.readWatchpoint(task, i);
> + int blockers = task.notifyWatchpoint(trigger.getAddress(), trigger.getRange());
> + if (blockers == 0)
> + return sendContinue(task, Signal.NONE);
> + else
> + return blockedContinue;
> + }
> + }
> +
> +
> Isa isa;
> isa = task.getIsaFIXME();
>
> diff --git a/frysk-core/frysk/proc/live/Watchpoint.java b/frysk-core/frysk/proc/live/Watchpoint.java
> index 7915d84..86d1e00 100644
> --- a/frysk-core/frysk/proc/live/Watchpoint.java
> +++ b/frysk-core/frysk/proc/live/Watchpoint.java
> @@ -40,7 +40,9 @@
>
> package frysk.proc.live;
>
> +import java.util.ArrayList;
> import java.util.HashMap;
> +import java.util.Iterator;
>
> import frysk.proc.Task;
>
> @@ -55,6 +57,7 @@ public class Watchpoint implements Comparable
> // These two fields define a Breakpoint
> private final long address;
> private final int length;
> + private final boolean writeOnly;
> private final Task task;
>
>
> @@ -67,7 +70,7 @@ public class Watchpoint implements Comparable
> * Private constructor called by create to record address and
> * proc.
> */
> - private Watchpoint(long address, int length, Task task)
> + private Watchpoint(long address, int length, boolean writeOnly, Task task)
> {
> if (task == null)
> throw new NullPointerException("Cannot place a watchpoint when task == null.");
> @@ -75,7 +78,7 @@ public class Watchpoint implements Comparable
> this.address = address;
> this.task = task;
> this.length = length;
> -
> + this.writeOnly = writeOnly;
> if (this.length <= 0)
> throw new RuntimeException("Watchpoint length has to be > 0");
> }
> @@ -86,9 +89,9 @@ public class Watchpoint implements Comparable
> * 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)
> + public static Watchpoint create(long address, int length, boolean writeOnly, Task task)
> {
> - Watchpoint watchpoint = new Watchpoint(address, length, task);
> + Watchpoint watchpoint = new Watchpoint(address, length, writeOnly, task);
>
> // If possible return an existing installed breakpoint.
> synchronized (installedWatchpoints)
> @@ -105,6 +108,15 @@ public class Watchpoint implements Comparable
> return address;
> }
>
> + public int getLength()
> + {
> + return length;
> + }
> +
> + public boolean isWriteOnly()
> + {
> + return writeOnly;
> + }
>
> /**
> * Installs breakpoint. Caller must make sure there is no breakpoint set
> @@ -130,16 +142,28 @@ public class Watchpoint implements Comparable
> */
> 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);
> + // XXX: Need a get empty watchpoint locator. It's a bit
> + // much for the set() to find the empty watchpoint. Also
> + // question the value of optimizing at this point.
> + int watchpointIndex = -1;
> + frysk.isa.watchpoints.WatchpointFunctions wpf =
> + frysk.isa.watchpoints.WatchpointFunctionFactory.
> + getWatchpointFunctions(task.getISA());
> + ArrayList watchpointList = (ArrayList) wpf.getAllWatchpoints(task);
> + Iterator i = watchpointList.iterator();
> + while (i.hasNext()) {
> + frysk.isa.watchpoints.Watchpoint emptyTest =
> + ((frysk.isa.watchpoints.Watchpoint)i.next());
> + if (emptyTest.getAddress() == 0) {
> + watchpointIndex = emptyTest.getRegister();
> + break;
>
>
> hooks/post-receive
> --
> frysk system monitor/debugger
>
^ permalink raw reply [flat|nested] only message in thread