* Watchpoint task observer refactor was Re: [SCM] master: Merge branch 'master' of ssh://sources.redhat.com/git/frysk
[not found] <20080402224121.8504.qmail@sourceware.org>
@ 2008-04-02 22:49 ` Phil Muldoon
0 siblings, 0 replies; only message in thread
From: Phil Muldoon @ 2008-04-02 22:49 UTC (permalink / raw)
To: frysk
pmuldoon@sourceware.org wrote:
I merged in this patch (sorry for the title). The original watchpoint
interface in LinuxPtraceTask was based on Code Breakpoints which were
software, single step based. I've decided that was not the best approach
for a hardware based approach and have moved the management code out of
Task, and will push it to a lower level. These changes mainly deal with
removing the code from LinuxPtraceTask. The code also introduces the
writeOnly flag which instructs the watchpoint to only trigger if it has
been written too.
Regards
Phil
> The branch, master has been updated
> via 21953327b97883d2adb9423c3dad56fc319f2f12 (commit)
> via c64ee523562cd444fc03bfd3e63e5f365f947c4b (commit)
> from 3a267df6b1a76f01ac0b8c796794a41ddf111cb9 (commit)
>
> Those revisions listed above that are new to this repository have
> not appeared on any other notification email.
>
> - Log -----------------------------------------------------------------
> commit 21953327b97883d2adb9423c3dad56fc319f2f12
> Merge: c64ee523562cd444fc03bfd3e63e5f365f947c4b 3a267df6b1a76f01ac0b8c796794a41ddf111cb9
> Author: Phil Muldoon <pmuldoon@redhat.com>
> Date: Wed Apr 2 23:40:22 2008 +0100
>
> Merge branch 'master' of ssh://sources.redhat.com/git/frysk
>
> commit c64ee523562cd444fc03bfd3e63e5f365f947c4b
> Author: Phil Muldoon <pmuldoon@redhat.com>
> Date: Wed Apr 2 23:37:20 2008 +0100
>
> Add writeOnly flag to interface. Remove watchpoint management code from task and subclasses.
>
> 2008-04-02 Phil Muldoon <pmuldoon@redhat.com>
> * Task.java (requestAddWatchObserver): Add writeOnly flag.
> (requestDeleteWatchObserver): Ditto.
> * TaskObserver.java (Watch.updateHit): Add length parameter.
>
> 2008-04-02 Phil Muldoon <pmuldoon@redhat.com>
>
> * DeadTask.java (requestAddWatchObserver): Add
> writeOnly flag.
> (requestDeleteWatchObserver): Ditto.
>
> 2008-04-02 Phil Muldoon <pmuldoon@redhat.com>
>
> * DummyTask.java (requestAddWatchObserver): Add writeOnly flag.
> (requestDeleteWatchObserver): Ditto.
>
> 2008-04-02 Phil Muldoon <pmuldoon@redhat.com>
>
> * LinuxPtraceProc.java (requestAddWatchObserver): Add writeOnly flag.
> (requestDeleteWatchObserver): Ditto.
> * LinuxPtraceTask.java (requestAddWatchObserver): Call LinuxPtraceProc
> implementation.
> (requestDeleteWatchObserver): Ditto.
> (notifyWatchpoint): New.
> (WatchpointAction): Delete.
> (requestAddWatchObserver): Delete private function.
> (requestDeleteWatchObserver): Delete private function.
>
> -----------------------------------------------------------------------
>
> Summary of changes:
> frysk-core/frysk/proc/ChangeLog | 5 +
> frysk-core/frysk/proc/Task.java | 6 +-
> frysk-core/frysk/proc/TaskObserver.java | 2 +-
> frysk-core/frysk/proc/dead/ChangeLog | 6 +
> frysk-core/frysk/proc/dead/DeadTask.java | 8 +-
> frysk-core/frysk/proc/dummy/ChangeLog | 5 +
> frysk-core/frysk/proc/dummy/DummyTask.java | 9 +-
> frysk-core/frysk/proc/live/ChangeLog | 14 ++
> frysk-core/frysk/proc/live/LinuxPtraceProc.java | 6 +-
> frysk-core/frysk/proc/live/LinuxPtraceTask.java | 163 ++++++-----------------
> 10 files changed, 91 insertions(+), 133 deletions(-)
>
> First 500 lines of diff:
> diff --git a/frysk-core/frysk/proc/ChangeLog b/frysk-core/frysk/proc/ChangeLog
> index 6a11424..f7e059d 100644
> --- a/frysk-core/frysk/proc/ChangeLog
> +++ b/frysk-core/frysk/proc/ChangeLog
> @@ -1,3 +1,8 @@
> +2008-04-02 Phil Muldoon <pmuldoon@redhat.com>
> + * Task.java (requestAddWatchObserver): Add writeOnly flag.
> + (requestDeleteWatchObserver): Ditto.
> + * TaskObserver.java (Watch.updateHit): Add length parameter.
> +
> 2008-04-01 Phil Muldoon <pmuldoon@redhat.com>
>
> * Task.java (requestAddWatchObserver): Publish.
> diff --git a/frysk-core/frysk/proc/Task.java b/frysk-core/frysk/proc/Task.java
> index dfebb7f..7dccaf6 100644
> --- a/frysk-core/frysk/proc/Task.java
> +++ b/frysk-core/frysk/proc/Task.java
> @@ -263,12 +263,14 @@ public abstract class Task {
> /**
> * Add TaskObserver.Watch to the TaskObserver pool.
> */
> - public abstract void requestAddWatchObserver(TaskObserver.Watch o, long address, int length);
> + public abstract void requestAddWatchObserver(TaskObserver.Watch o, long address,
> + int length, boolean writeOnly);
>
> /**
> * Delete TaskObserver.Watchfor the TaskObserver pool.
> */
> - public abstract void requestDeleteWatchObserver(TaskObserver.Watch o, long address, int length);
> + public abstract void requestDeleteWatchObserver(TaskObserver.Watch o, long address,
> + int length, boolean writeOnly);
>
>
>
> diff --git a/frysk-core/frysk/proc/TaskObserver.java b/frysk-core/frysk/proc/TaskObserver.java
> index 7ee7189..ecc005e 100644
> --- a/frysk-core/frysk/proc/TaskObserver.java
> +++ b/frysk-core/frysk/proc/TaskObserver.java
> @@ -221,6 +221,6 @@ public interface TaskObserver
> * a Proc share their breakpoints, so this method needs to
> * check the actual Task that got hit.
> */
> - Action updateHit (Task task, long address);
> + Action updateHit (Task task, long address, int length);
> }
> }
> diff --git a/frysk-core/frysk/proc/dead/ChangeLog b/frysk-core/frysk/proc/dead/ChangeLog
> index c1be2fe..0699be9 100644
> --- a/frysk-core/frysk/proc/dead/ChangeLog
> +++ b/frysk-core/frysk/proc/dead/ChangeLog
> @@ -1,3 +1,9 @@
> +2008-04-02 Phil Muldoon <pmuldoon@redhat.com>
> +
> + * DeadTask.java (requestAddWatchObserver): Add
> + writeOnly flag.
> + (requestDeleteWatchObserver): Ditto.
> +
> 2008-04-01 Andrew Cagney <cagney@redhat.com>
>
> * TestLinuxCore.java: Update to match PrintStackOptions.
> diff --git a/frysk-core/frysk/proc/dead/DeadTask.java b/frysk-core/frysk/proc/dead/DeadTask.java
> index 346b1bc..595e430 100644
> --- a/frysk-core/frysk/proc/dead/DeadTask.java
> +++ b/frysk-core/frysk/proc/dead/DeadTask.java
> @@ -202,15 +202,17 @@ abstract class DeadTask extends Task {
> /**
> * Add TaskObserver.Watch to the TaskObserver pool.
> */
> - public void requestAddWatchObserver(TaskObserver.Watch o, long address, int length) {
> + public void requestAddWatchObserver(TaskObserver.Watch o, long address,
> + int length, boolean writeOnly) {
> throw new RuntimeException("requestAddWatchObserver");
> }
>
> /**
> * Delete TaskObserver.Watch from the TaskObserver pool.
> */
> - public void requestDeleteWatchObserver(TaskObserver.Watch o, long address, int length) {
> - throw new RuntimeException("requestDeleteCodeObserver");
> + public void requestDeleteWatchObserver(TaskObserver.Watch o, long address,
> + int length, boolean writeOnly) {
> + throw new RuntimeException("requestDeleteWatchObserver");
> }
>
>
> diff --git a/frysk-core/frysk/proc/dummy/ChangeLog b/frysk-core/frysk/proc/dummy/ChangeLog
> index e487c75..89c6d46 100644
> --- a/frysk-core/frysk/proc/dummy/ChangeLog
> +++ b/frysk-core/frysk/proc/dummy/ChangeLog
> @@ -1,3 +1,8 @@
> +2008-04-02 Phil Muldoon <pmuldoon@redhat.com>
> +
> + * DummyTask.java (requestAddWatchObserver): Add writeOnly flag.
> + (requestDeleteWatchObserver): Ditto.
> +
> 2008-04-01 Phil Muldoon <pmuldoon@redhat.com>
>
> * DummyTask.java (requestAddWatchObserver): Implement.
> diff --git a/frysk-core/frysk/proc/dummy/DummyTask.java b/frysk-core/frysk/proc/dummy/DummyTask.java
> index 025343b..c3fb350 100644
> --- a/frysk-core/frysk/proc/dummy/DummyTask.java
> +++ b/frysk-core/frysk/proc/dummy/DummyTask.java
> @@ -130,12 +130,15 @@ public class DummyTask extends Task {
> throw new RuntimeException("oops!");
> }
>
> - public void requestAddWatchObserver(TaskObserver.Watch o, long address, int length) {
> + public void requestAddWatchObserver(TaskObserver.Watch o, long address,
> + int length, boolean writeOnly) {
> throw new RuntimeException("requestAddWatchObserver");
> }
>
> - public void requestDeleteWatchObserver(TaskObserver.Watch o, long address, int length) {
> - throw new RuntimeException("requestDeleteCodeObserver");
> + public void requestDeleteWatchObserver(TaskObserver.Watch o, long address,
> + int length, boolean writeOnly) {
> +
> + throw new RuntimeException("requestDeleteWatchObserver");
> }
>
> public void requestAddInstructionObserver(TaskObserver.Instruction o) {
> diff --git a/frysk-core/frysk/proc/live/ChangeLog b/frysk-core/frysk/proc/live/ChangeLog
> index 916c297..fc1f503 100644
> --- a/frysk-core/frysk/proc/live/ChangeLog
> +++ b/frysk-core/frysk/proc/live/ChangeLog
> @@ -1,3 +1,17 @@
> +2008-04-02 Phil Muldoon <pmuldoon@redhat.com>
> +
> + * LinuxPtraceProc.java (requestAddWatchObserver): Add writeOnly flag.
> + (requestDeleteWatchObserver): Ditto.
> + * LinuxPtraceTask.java (requestAddWatchObserver): Call LinuxPtraceProc
> + implementation.
> + (requestDeleteWatchObserver): Ditto.
> + (notifyWatchpoint): New.
> + (WatchpointAction): Delete.
> + (requestAddWatchObserver): Delete private function.
> + (requestDeleteWatchObserver): Delete private function.
> +
> +
> +
> 2008-03-31 Stan Cox <scox@redhat.com>
>
> * LinuxPtraceProc.java (getExe): Replace with getExeFile.
> diff --git a/frysk-core/frysk/proc/live/LinuxPtraceProc.java b/frysk-core/frysk/proc/live/LinuxPtraceProc.java
> index 659a22b..9f10c22 100644
> --- a/frysk-core/frysk/proc/live/LinuxPtraceProc.java
> +++ b/frysk-core/frysk/proc/live/LinuxPtraceProc.java
> @@ -625,7 +625,8 @@ public class LinuxPtraceProc extends LiveProc {
> void requestAddWatchObserver(Task task, TaskObservable observable,
> TaskObserver.Watch observer,
> final long address,
> - final int length) {
> + final int length,
> + boolean writeOnly) {
> }
>
> /**
> @@ -635,7 +636,8 @@ public class LinuxPtraceProc extends LiveProc {
> void requestDeleteWatchObserver(Task task, TaskObservable observable,
> TaskObserver.Watch observer,
> final long address,
> - final int length) {
> + final int length,
> + boolean writeOnly) {
> }
>
> /**
> diff --git a/frysk-core/frysk/proc/live/LinuxPtraceTask.java b/frysk-core/frysk/proc/live/LinuxPtraceTask.java
> index 7df22be..f1a559b 100644
> --- a/frysk-core/frysk/proc/live/LinuxPtraceTask.java
> +++ b/frysk-core/frysk/proc/live/LinuxPtraceTask.java
> @@ -86,7 +86,6 @@ public class LinuxPtraceTask extends LiveTask {
> ((LinuxPtraceHost)proc.getHost()).putTask(tid, this);
> ((LinuxPtraceProc)proc).addTask(this);
> newState = LinuxPtraceTaskState.detachedState();
> - this.watchpoints = new WatchpointAddresses(this);
> }
> /**
> * Create a new attached clone of Task.
> @@ -99,7 +98,6 @@ public class LinuxPtraceTask extends LiveTask {
> ((LinuxPtraceProc)cloningTask.getProc()).addTask(this);
> // XXX: shouldn't need to grub around in the old task's state.
> newState = LinuxPtraceTaskState.clonedState(cloningTask.getState());
> - this.watchpoints = new WatchpointAddresses(this);
> }
> /**
> * Create a new attached main Task of Proc.
> @@ -121,8 +119,6 @@ public class LinuxPtraceTask extends LiveTask {
> };
> proc.handleAddObservation(ob);
> }
> - this.watchpoints = new WatchpointAddresses(this);
> -
> }
>
> /**
> @@ -544,6 +540,46 @@ public class LinuxPtraceTask extends LiveTask {
> return blockers;
> }
>
> +
> + /**
> + * Set of Code observers.
> + *
> + * FIXME: Should be private only LinuxPtraceTaskState grubs around
> + * with this variable.
> + */
> + final TaskObservable watchObservers = new TaskObservable(this);
> + /**
> + * Notify all Code observers of the breakpoint. Return the number
> + * of blocking observers or -1 if no Code observer were installed
> + * on this address.
> + */
> + int notifyWatchpoint(long address, int length) {
> + for (Iterator i = watchObservers.iterator(); i.hasNext();) {
> + TaskObserver.Watch observer = (TaskObserver.Watch) i.next();
> + if (observer.updateHit(this, address, length) == Action.BLOCK) {
> + blockers.add(observer);
> + }
> + }
> + return blockers.size();
> + }
> +
> + /**
> + * Add a TaskObserver.Watch observer
> + * (hardware only)
> + */
> + public void requestAddWatchObserver(TaskObserver.Watch o, long address, int length, boolean writeOnly) {
> + fine.log(this,"requestAddWatchObserver");
> + ((LinuxPtraceProc)getProc()).requestAddWatchObserver(this, watchObservers, o, address, length, writeOnly);
> + }
> +
> + /**
> + * Delete TaskObserver.Code for the TaskObserver pool.
> + */
> + public void requestDeleteWatchObserver(TaskObserver.Watch o, long address, int length, boolean writeOnly) {
> + fine.log(this, "requestDeleteWatcheObserver");
> + ((LinuxPtraceProc)getProc()).requestDeleteWatchObserver(this, watchObservers, o, address, length, writeOnly);
> + }
> +
> /**
> * Set of Cloned observers.
> */
> @@ -576,23 +612,7 @@ 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.
> */
> @@ -933,107 +953,6 @@ public class LinuxPtraceTask extends LiveTask {
> }
>
> /**
> - * 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 watchAddress;
> -
> - private final int watchLength;
> -
> - private final boolean addition;
> -
> - WatchpointAction(TaskObserver.Watch watch, Task task, long address, int length,
> - boolean addition) {
> - this.watch = watch;
> - this.task = task;
> - this.watchAddress = address;
> - this.watchLength = length;
> - this.addition = addition;
> - }
> -
> - public void run() {
> - if (addition) {
> - boolean shouldInstall = watchpoints.addWatchpoint(watch, watchAddress, watchLength);
> - if (shouldInstall) {
> - Watchpoint watchpoint;
> - watchpoint = Watchpoint.create(watchAddress, watchLength, LinuxPtraceTask.this);
> - watchpoint.install(task);
> - }
> - } else {
> - boolean mustRemove = watchpoints.removeWatchpoint(watch, watchAddress, watchLength);
> - if (mustRemove) {
> - Watchpoint watchpoint;
> - watchpoint = Watchpoint.create(watchAddress, watchLength, 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.getWatchObservers(address, length) == null;
> - }
> - };
> - Manager.eventLoop.add(to);
> - }
> -
> - /**
> - * (Internal) Tell the process to delete the specified Watchpoint
> - * observation, detaching 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 observation;
> -
> - observation = new TaskObservation((LinuxPtraceTask)task, observable, observer,
> - watchAction, false) {
> - public void execute() {
> - try {
> - newState = oldState().handleDeleteObservation(LinuxPtraceTask.this, this);
> - } catch (Errno.Esrch e) {
> - newState = handleDisappearedEvent(e);
> - }
> - }
> -
> - public boolean needsSuspendedAction() {
> - return watchpoints.getWatchObservers(address, length).size() == 1;
> - }
> - };
> -
> - Manager.eventLoop.add(observation);
> - }
> -
> -
> - /**
> * Add TaskObserver.Code to the TaskObserver pool.
> */
> public void requestAddCodeObserver(TaskObserver.Code o, long a) {
>
>
> hooks/post-receive
> --
> frysk system monitor/debugger
>
^ permalink raw reply [flat|nested] only message in thread