From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10639 invoked by alias); 21 Apr 2008 10:39:02 -0000 Received: (qmail 10626 invoked by uid 22791); 21 Apr 2008 10:39:01 -0000 X-Spam-Status: No, hits=-0.5 required=5.0 tests=AWL,BAYES_20,J_CHICKENPOX_44,J_CHICKENPOX_48,J_CHICKENPOX_66 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; Mon, 21 Apr 2008 10:38:41 +0000 Received: from cc1341701-a.deven1.ov.home.nl ([82.72.26.122] helo=[192.168.1.113]) by gnu.wildebeest.org with esmtp (Exim 4.63) (envelope-from ) id 1JntPw-0002ix-7M for frysk@sourceware.org; Mon, 21 Apr 2008 12:38:38 +0200 Subject: [patch] Record Task registering TaskObserver.Code with BreakpointAddresses From: Mark Wielaard To: frysk@sourceware.org Content-Type: multipart/mixed; boundary="=-sHSvQcq4U8mrwn/2mzuu" Date: Tue, 22 Apr 2008 16:56:00 -0000 Message-Id: <1208774312.3615.11.camel@dijkstra.wildebeest.org> Mime-Version: 1.0 X-Mailer: Evolution 2.12.3 (2.12.3-4.fc8) X-Spam-Score: -0.9 (/) 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/msg00062.txt.bz2 --=-sHSvQcq4U8mrwn/2mzuu Content-Type: text/plain Content-Transfer-Encoding: 7bit Content-length: 2022 Hi, This solves a long standing bug (#5234) where a TaskObserver.Code was triggered by a breakpoint being hit on a Task that it didn't originally register with. The problem was that the BreakpointAddresses didn't have the original Task against which the TaskObserver.Code was registered. And originally it was thought to not be such a big deal if a TaskObserver.Code interface saw all breakpoint events in a Proc. It could check that it was given the right Task in its updateHit() anyway. But this required some ugly workarounds in some cases which are now not necessary anymore. We now have a little static final helper class CodeObserver that records the original Task and TaskObserver.Code so that notifyCodeObserver() can check whether the Task (and observer) were registered for this address. Note that it still must report zero, and not -1 if there are any breakpoints installed, even if it wasn't for this particular Task because the ptrace state machine depends on that to know whether or not to go past an installed breakpoint on the current address. The patch also marks the BreakpointAddresses class and (helper) methods as package private. frysk-core/frysk/proc/live/ChangeLog 2008-04-21 Mark Wielaard * BreakpointAddresses.java: Mark class and methods package private. (CodeObserver): New final static helper class. (addBreakpoint): Take CodeObserver as argument, not TaskObserver.Code. (removeBreakpoint): Likewise. (getCodeObservers): Return collection of CodeObservers, not TaskObserver.Code. * LinuxPtraceProc.java (BreakpointAction.run): Install or remove BreakpointAddresses.CodeObserver instead of bare TaskObserver.Code. * LinuxPtraceTask.java (notifyCodeBreakpoint): Check Task against BreakpointAddresses.CodeObserver. * TestTaskObserverCode.java (testMultiTaskUpdateCalledSeveralTimes): Resolved bug #5234. The testcase testMultiTaskUpdateCalledSeveralTimes now PASSes. No regressions. Committed and pushed, Mark --=-sHSvQcq4U8mrwn/2mzuu Content-Disposition: inline; filename=patch Content-Type: text/x-patch; name=patch; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-length: 7097 diff --git a/frysk-core/frysk/proc/live/BreakpointAddresses.java b/frysk-core/frysk/proc/live/BreakpointAddresses.java index dc59c34..1fb127a 100644 --- a/frysk-core/frysk/proc/live/BreakpointAddresses.java +++ b/frysk-core/frysk/proc/live/BreakpointAddresses.java @@ -42,6 +42,7 @@ package frysk.proc.live; import frysk.proc.TaskObserver; import frysk.proc.Proc; +import frysk.proc.Task; import java.util.Collection; import java.util.Iterator; import java.util.HashMap; @@ -62,8 +63,45 @@ import java.util.ArrayList; * This datastructure isn't multithread safe, it should only be called * from the eventloop in response to requests pending for the Proc. */ -public class BreakpointAddresses +class BreakpointAddresses { + + /** + * Small package private class holding Task and TaskObserver.Code + * interested in a particular breakpoint. Used by + * addBreakpoint(), removeBreakpoint and + * getBreakpoints(). + */ + final static class CodeObserver + { + final Task task; + final TaskObserver.Code observer; + + CodeObserver(Task task, TaskObserver.Code observer) + { + if (task == null || observer == null) + throw new NullPointerException(); + + this.task = task; + this.observer = observer; + } + + public boolean equals(Object o) + { + if (! (o instanceof CodeObserver)) + return false; + + CodeObserver other = (CodeObserver) o; + return (this.task.equals(other.task) + && this.observer.equals(other.observer)); + } + + public int hashCode() + { + return task.hashCode() ^ observer.hashCode(); + } + } + /** * Proc used to set breakpoints and which sents us notifications * when breakpoints are hit. @@ -85,7 +123,7 @@ public class BreakpointAddresses /** * Package private constructor used by the Proc when created. */ - public BreakpointAddresses(Proc proc) + BreakpointAddresses(Proc proc) { this.proc = proc; map = new HashMap(); @@ -99,7 +137,7 @@ public class BreakpointAddresses * added to the list of objects to notify when the breakpoint is * hit (and the method returns false). */ - public boolean addBreakpoint(TaskObserver.Code observer, long address) + boolean addBreakpoint(CodeObserver observer, long address) { Breakpoint breakpoint = Breakpoint.create(address, proc); @@ -129,7 +167,7 @@ public class BreakpointAddresses * * @throws IllegalArgumentException if the observer was never added. */ - public boolean removeBreakpoint(TaskObserver.Code observer, long address) + boolean removeBreakpoint(CodeObserver observer, long address) { Breakpoint breakpoint = Breakpoint.create(address, proc); ArrayList list = (ArrayList) map.get(breakpoint); @@ -149,10 +187,11 @@ public class BreakpointAddresses /** * Called by the Proc when it has trapped a breakpoint. Returns a - * Collection of TaskObserver.Code observers interested in the given + * Collection of CodeObservers interested in the given * address or null when no Code observer was installed on this address. + * Package private (as is CodeObserver). */ - public Collection getCodeObservers(long address) { + Collection getCodeObservers(long address) { ArrayList observers; Breakpoint breakpoint = Breakpoint.create(address, proc); ArrayList list = (ArrayList) map.get(breakpoint); @@ -165,7 +204,7 @@ public class BreakpointAddresses return observers; } - public Breakpoint getBreakpoint(long address) + Breakpoint getBreakpoint(long address) { Breakpoint breakpoint = Breakpoint.create(address, proc); Object observer = map.get(breakpoint); @@ -180,7 +219,7 @@ public class BreakpointAddresses * (possibly) installed starting at the from address up to (but not * including) the till address. */ - public Iterator getBreakpoints(long from, long till) + Iterator getBreakpoints(long from, long till) { Breakpoint fromBreakpoint = Breakpoint.create(from, proc); Breakpoint tillBreakpoint = Breakpoint.create(till, proc); @@ -191,9 +230,8 @@ public class BreakpointAddresses * Called from TaskState when the Task gets an execed event which * clears the whole address space. * - * XXX: Should not be public. */ - public void removeAllCodeObservers() + void removeAllCodeObservers() { map.clear(); breakpoints.clear(); diff --git a/frysk-core/frysk/proc/live/LinuxPtraceProc.java b/frysk-core/frysk/proc/live/LinuxPtraceProc.java index d376fc2..14e1e00 100644 --- a/frysk-core/frysk/proc/live/LinuxPtraceProc.java +++ b/frysk-core/frysk/proc/live/LinuxPtraceProc.java @@ -552,14 +552,20 @@ public class LinuxPtraceProc extends LiveProc { public void run() { if (addition) { - boolean mustInstall = breakpoints.addBreakpoint(code, address); + BreakpointAddresses.CodeObserver observer; + boolean mustInstall; + observer = new BreakpointAddresses.CodeObserver(task, code); + mustInstall = breakpoints.addBreakpoint(observer, address); if (mustInstall) { Breakpoint breakpoint; breakpoint = Breakpoint.create(address, LinuxPtraceProc.this); breakpoint.install(task); } } else { - boolean mustRemove = breakpoints.removeBreakpoint(code, address); + BreakpointAddresses.CodeObserver observer; + boolean mustRemove; + observer = new BreakpointAddresses.CodeObserver(task, code); + mustRemove = breakpoints.removeBreakpoint(observer, address); if (mustRemove) { Breakpoint breakpoint; breakpoint = Breakpoint.create(address, LinuxPtraceProc.this); diff --git a/frysk-core/frysk/proc/live/LinuxPtraceTask.java b/frysk-core/frysk/proc/live/LinuxPtraceTask.java index 2121018..397f69b 100644 --- a/frysk-core/frysk/proc/live/LinuxPtraceTask.java +++ b/frysk-core/frysk/proc/live/LinuxPtraceTask.java @@ -966,10 +966,11 @@ public class LinuxPtraceTask extends LiveTask { Iterator i = observers.iterator(); while (i.hasNext()) { - TaskObserver.Code observer = (TaskObserver.Code) i.next(); - if (codeObservers.contains(observer)) - if (observer.updateHit(this, address) == Action.BLOCK) - blockers.add(observer); + BreakpointAddresses.CodeObserver co; + co = (BreakpointAddresses.CodeObserver) i.next(); + if (co.task.equals(this)) + if (co.observer.updateHit(this, address) == Action.BLOCK) + blockers.add(co.observer); } return blockers.size(); } diff --git a/frysk-core/frysk/proc/live/TestTaskObserverCode.java b/frysk-core/frysk/proc/live/TestTaskObserverCode.java index 62108f2..db209b8 100644 --- a/frysk-core/frysk/proc/live/TestTaskObserverCode.java +++ b/frysk-core/frysk/proc/live/TestTaskObserverCode.java @@ -763,9 +763,6 @@ public class TestTaskObserverCode extends TestLib // get separate update events. bug #5234 public void testMultiTaskUpdateCalledSeveralTimes() throws Exception { - if (unresolved(5234)) - return; - // Create a child. LegacyOffspring child = LegacyOffspring.createDaemon(); --=-sHSvQcq4U8mrwn/2mzuu--