public inbox for frysk-cvs@sourceware.org
help / color / mirror / Atom feed
* [SCM]  master: Reset breakpoints on fork. Fixes bug #5331.
@ 2008-04-22 16:52 mark
  0 siblings, 0 replies; only message in thread
From: mark @ 2008-04-22 16:52 UTC (permalink / raw)
  To: frysk-cvs

The branch, master has been updated
       via  8a89c5014642554b0d94e146e6aa4fc7147e0015 (commit)
      from  227a64d1e88875e810e9e54f9fed3ff3778fc06f (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email.

- Log -----------------------------------------------------------------
commit 8a89c5014642554b0d94e146e6aa4fc7147e0015
Author: Mark Wielaard <mwielaard@redhat.com>
Date:   Tue Apr 22 18:34:11 2008 +0200

    Reset breakpoints on fork. Fixes bug #5331.
    
    frysk-core/frysk/proc/live/ChangeLog
    2008-04-22  Mark Wielaard  <mwielaard@redhat.com>
    
           * Breakpoint.java (reset): Make package private.
           (cloneForProc): New method.
           * BreakpointAddresses.java (cloneForProc): New method.
           (clearAllBreakpoints): New method.
           * LinuxPtraceProc.java (LinuxPtraceProc(Task,ProcessIdentifier)):
           Set breakpoints through forkingProc.breakpoints.cloneForProc().
           (breakpoints): Make package private.
           * LinuxPtraceTask.java (LinuxPtraceTask(LinuxPtraceTask,
           LinuxPtraceProc,TaskAttachedObserverXXX)): Set currentISA.
           * LinuxPtraceTaskState.java
           (StartMainTask.wantToDetach.handleAttach): Clear breakpoints.
           (StartMainTask.wantToDetach.StoppedEvent): Likewise.
           * TestTaskObserverCode.java (testCodeOverFork): Resolve bug #5331.

-----------------------------------------------------------------------

Summary of changes:
 frysk-core/frysk/proc/live/Breakpoint.java         |   23 ++++++++++++-
 .../frysk/proc/live/BreakpointAddresses.java       |   37 ++++++++++++++++++++
 frysk-core/frysk/proc/live/ChangeLog               |   18 +++++++++-
 frysk-core/frysk/proc/live/LinuxPtraceProc.java    |   11 ++++--
 frysk-core/frysk/proc/live/LinuxPtraceTask.java    |   13 +++++++
 .../frysk/proc/live/LinuxPtraceTaskState.java      |   12 +++++-
 .../frysk/proc/live/TestTaskObserverCode.java      |    3 --
 7 files changed, 107 insertions(+), 10 deletions(-)

First 500 lines of diff:
diff --git a/frysk-core/frysk/proc/live/Breakpoint.java b/frysk-core/frysk/proc/live/Breakpoint.java
index 0372b6d..54ea21f 100644
--- a/frysk-core/frysk/proc/live/Breakpoint.java
+++ b/frysk-core/frysk/proc/live/Breakpoint.java
@@ -175,8 +175,11 @@ public class Breakpoint implements Comparable
 
   /**
    * Actually removes the breakpoint.
+   * Package private for BreakpointAddresses.clearBreakpoints(),
+   * all other callers should use <code>remove()</code>.
+   *
    */
-  private void reset(Task task)
+  void reset(Task task)
   {
       ByteBuffer buffer = ((LinuxPtraceTask)task).getRawMemory();
     buffer.position(address);
@@ -379,4 +382,22 @@ public class Breakpoint implements Comparable
     return this.getClass().getName() + "[proc=" + proc
       + ", address=0x" + Long.toHexString(address) + "]";
   }
+
+  /**
+   * Makes a clone of this Breakpoint, but for a particular Proc. This
+   * is used by the BreakPointAddresses class to make a clone of all
+   * breakpoints set on a froked proc. First makes sure to have the
+   * canonical (installed) breakpoint. Then makes a copy and adds the
+   * original instruction. See BreakPointAddresses cloneForProc() and
+   * clearAllAddresses for more explanation.
+   */
+  Breakpoint cloneForProc(Proc proc)
+  {
+    // Make sure to have the canonical (installed) breakpoint.
+    // Then make a copy and add the original instruction.
+    Breakpoint breakpoint = create(this.getAddress(), this.getProc());
+    Breakpoint result = new Breakpoint(breakpoint.address, proc);
+    result.origInstruction = breakpoint.origInstruction;
+    return result;
+  }
 }
diff --git a/frysk-core/frysk/proc/live/BreakpointAddresses.java b/frysk-core/frysk/proc/live/BreakpointAddresses.java
index 1fb127a..4564d40 100644
--- a/frysk-core/frysk/proc/live/BreakpointAddresses.java
+++ b/frysk-core/frysk/proc/live/BreakpointAddresses.java
@@ -236,4 +236,41 @@ class BreakpointAddresses
     map.clear();
     breakpoints.clear();
   }
+
+  /**
+   * Sets up a BreakpointAddresses map for a forked proc.  This sets
+   * the list of breakpoint addresses (plus original instructions) to
+   * be reset later with clearAllBreakpoints(). See comments in
+   * LinuxPtraceProc, LinuxPtraceTask and
+   * LinuxPtraceTaskState.StartMainTask.wantToDetach for the sequence
+   * of events.
+   */
+  BreakpointAddresses cloneForProc(Proc proc)
+  {
+    BreakpointAddresses addresses = new BreakpointAddresses(proc);
+    Iterator it = breakpoints.iterator();
+    while (it.hasNext())
+      {
+	Breakpoint breakpoint = (Breakpoint) it.next();
+	breakpoint = breakpoint.cloneForProc(proc);
+	addresses.breakpoints.add(breakpoint);
+      }
+    return addresses;
+  }
+
+  /**
+   * Actually resets the breakpoints in the breakpoint addresses left
+   * by cloneForProc(). Called when the main task of the forked proc
+   * is ready in LinuxPtraceTaskState.StartMainTask.wantToDetach.
+   */
+  void clearAllBreakpoints()
+  {
+    Iterator it = breakpoints.iterator();
+    while (it.hasNext())
+      {
+        Breakpoint breakpoint = (Breakpoint) it.next();
+        breakpoint.reset(proc.getMainTask());
+      }
+    removeAllCodeObservers();
+  }
 }
diff --git a/frysk-core/frysk/proc/live/ChangeLog b/frysk-core/frysk/proc/live/ChangeLog
index 6a549ac..8231f51 100644
--- a/frysk-core/frysk/proc/live/ChangeLog
+++ b/frysk-core/frysk/proc/live/ChangeLog
@@ -1,3 +1,19 @@
+2008-04-22  Mark Wielaard  <mwielaard@redhat.com>
+
+	* Breakpoint.java (reset): Make package private.
+	(cloneForProc): New method.
+	* BreakpointAddresses.java (cloneForProc): New method.
+	(clearAllBreakpoints): New method.
+	* LinuxPtraceProc.java (LinuxPtraceProc(Task,ProcessIdentifier)):
+	Set breakpoints through forkingProc.breakpoints.cloneForProc().
+	(breakpoints): Make package private.
+	* LinuxPtraceTask.java (LinuxPtraceTask(LinuxPtraceTask,
+	LinuxPtraceProc,TaskAttachedObserverXXX)): Set currentISA.
+	* LinuxPtraceTaskState.java
+	(StartMainTask.wantToDetach.handleAttach): Clear breakpoints.
+	(StartMainTask.wantToDetach.StoppedEvent): Likewise.
+	* TestTaskObserverCode.java (testCodeOverFork): Resolve bug #5331.
+	
 2008-04-22  Phil Muldoon  <pmuldoon@redhat.com>
 
 	* LinuxPtraceTaskState.java (Running.checkWatchpoint): Check
@@ -6,7 +22,7 @@
 	watchpoints that have been triggered, then notify.
 	* LinuxPtraceProc.java (LinuxPtraceProc): Wide watchpoint scope
 	to protected.
-	
+
 2008-04-21  Mark Wielaard  <mwielaard@redhat.com>
 
 	* BreakpointAddresses.java: Mark class and methods package private.
diff --git a/frysk-core/frysk/proc/live/LinuxPtraceProc.java b/frysk-core/frysk/proc/live/LinuxPtraceProc.java
index 621d343..08db321 100644
--- a/frysk-core/frysk/proc/live/LinuxPtraceProc.java
+++ b/frysk-core/frysk/proc/live/LinuxPtraceProc.java
@@ -97,7 +97,8 @@ public class LinuxPtraceProc extends LiveProc {
     public LinuxPtraceProc(Task task, ProcessIdentifier fork) {
 	super(task, fork);
 	this.newState = LinuxPtraceProcState.initial(true);
-	this.breakpoints = new BreakpointAddresses(this);
+	LinuxPtraceProc forkingProc = (LinuxPtraceProc) task.getProc();
+	this.breakpoints = forkingProc.breakpoints.cloneForProc(this);
 	this.watchpoints = new WatchpointAddresses(this);
 	((LinuxPtraceHost)getHost()).addProc(this);
     }
@@ -833,9 +834,13 @@ public class LinuxPtraceProc extends LiveProc {
     }
 
     /**
-     * XXX: Should not be public.
+     * The addresses on which we have installed breakpoints (with
+     * associated Tasks and TaskObserver.Code listeners). Package
+     * private for access in LinuxPtraceTask constructor that creates
+     * a forked task and the LinuxPtraceTaskState.MainTask.wantToDetach
+     * class to clearthem. See the comments there.
      */
-    public final BreakpointAddresses breakpoints;
+    final BreakpointAddresses breakpoints;
     
     // List of available addresses for out of line stepping.
     // Used a lock in getOutOfLineAddress() and doneOutOfLine().
diff --git a/frysk-core/frysk/proc/live/LinuxPtraceTask.java b/frysk-core/frysk/proc/live/LinuxPtraceTask.java
index 06ba6a2..0ecadb3 100644
--- a/frysk-core/frysk/proc/live/LinuxPtraceTask.java
+++ b/frysk-core/frysk/proc/live/LinuxPtraceTask.java
@@ -110,6 +110,19 @@ public class LinuxPtraceTask extends LiveTask {
 	((LinuxPtraceHost)proc.getHost()).putTask(tid, this);
 	((LinuxPtraceProc)proc).addTask(this);
 	newState = LinuxPtraceTaskState.mainState();
+	// See the various FIXMEs below around the isa, ISA,
+	// currentISA, getISA(), getIsaFIXME() and clearIsa(). The
+	// current design is such that the isa isn't a constant of a
+	// proc (actually task), which means we cannot remove
+	// breakpoint instructions from the fork (the breakpoint
+	// instruction is an intrinsic of the isa).  Luckily we know
+	// that the fork will have the same isa as the task it forked
+	// from, so we explicitly set it now. We cannot do the
+	// breakpoint resetting here (the LinuxPtraceTask is created,
+	// but not fully setup yet), we do that in the
+	// LinuxPtraceTaskState.StartMainTask.wantToDetach class
+	// handlers.
+	currentISA = forkingTask.currentISA;
 	if (attached != null) {
 	    TaskObservation ob = new TaskObservation(this, attachedObservers,
 						     attached, true) {
diff --git a/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java b/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java
index 07323db..0556c3c 100644
--- a/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java
+++ b/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java
@@ -459,13 +459,21 @@ abstract class LinuxPtraceTaskState extends State {
 	    new StartMainTask("wantToDetach") {
 		LinuxPtraceTaskState handleAttach(LinuxPtraceTask task) {
 		    fine.log("handleAttach", task); 
-		    ((LinuxPtraceProc)task.getProc())
-			.performTaskAttachCompleted(task);
+		    LinuxPtraceProc proc = (LinuxPtraceProc) task.getProc();
+		    // Clear left over breakpoints here. They were
+		    // captured when the Proc was created.
+		    proc.breakpoints.clearAllBreakpoints();
+		    proc.performTaskAttachCompleted(task);
 		    return StartMainTask.wantToAttach;
 		}
 		LinuxPtraceTaskState handleStoppedEvent(LinuxPtraceTask task,
 							Signal signal) {
 		    if (signal == Signal.STOP || signal == Signal.TRAP) {
+		        LinuxPtraceProc proc;
+		        proc = (LinuxPtraceProc) task.getProc();
+		        // Clear left over breakpoints here. They were
+		        // captured when the Proc was created.
+		        proc.breakpoints.clearAllBreakpoints();
 			task.initializeAttachedState();
 			if (task.notifyForkedOffspring() > 0) {
 			    return StartMainTask.detachBlocked;
diff --git a/frysk-core/frysk/proc/live/TestTaskObserverCode.java b/frysk-core/frysk/proc/live/TestTaskObserverCode.java
index db209b8..f7e6f6a 100644
--- a/frysk-core/frysk/proc/live/TestTaskObserverCode.java
+++ b/frysk-core/frysk/proc/live/TestTaskObserverCode.java
@@ -886,9 +886,6 @@ public class TestTaskObserverCode extends TestLib
   // after the fork.
   public void testCodeOverFork() throws Exception
   {
-    if (unresolved(5331))
-      return;
-
     String[] argv = {getExecPath ("funit-fib-fork"), "2"};
     child = null;
     DaemonBlockedAtEntry child = new DaemonBlockedAtEntry(argv);


hooks/post-receive
--
frysk system monitor/debugger


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2008-04-22 16:52 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-04-22 16:52 [SCM] master: Reset breakpoints on fork. Fixes bug #5331 mark

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).