[SCM] master: Reset breakpoints on fork. Fixes bug #5331.

[mark@sourceware.org]

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