This is the mail archive of the frysk@sources.redhat.com mailing list for the frysk project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Isa support for isTaskStepped() and debug registers for x86/x86_64


Hi,

On Fri, 2007-01-12 at 22:56 +0100, Mark Wielaard wrote:
> So I decided to add a new method to Isa isTaskStepped()
> that examines (depending on architecture) the appropriate debug status
> registers to get at this info. The debug register support should be
> helpful in the future, there are no explicit tests for those yet though.
> I did add tests for the Isa.isTaskStepped() method.

I have now used this new infrastructure to make our TaskState much more
robust supporting stepping vs breakpoint traps. Running.handleTrap() has
been rewritten and all continue/step logic is now gated through
Running.sendContinue(), which checks whether or not a breakpoint is
being stepped. This finally fixes bug #3676 (test has been enabled now),
and hopefully some other mixed stepping/breakpointing issues (if you had
trouble with that please do retest). Tested on x86 and x86_64
(2.6.17-1.2174_FC5 kernel on both).

2007-01-15  Mark Wielaard  <mark@klomp.org>

    Fixes bug #3676
    * Breakpoint.java (stepDone): Only set if still installed.
    (isInstalled): new method.
    (toString): Prettify.
    * IsaIA32.java (isTaskStepped): Reset flag.
    * IsaX8664.java (isTaskStepped): Likewise.
    * LinuxIa32On64.java (LinuxIa32On64): Install IndirectRegisters for
    d0 till d7.
    * LinuxPtraceTaskState.java (Running.sendContinue): Rewritten to
    take breakpoints into account.
    (Running.handleStoppedEvent): Fix log message. Call sendContinue()
    on new state.
    (Running.handleTrappedEvent): Rewritten.
    (running, syscallRunning, inSyscallRunning, inSyscallRunningTraced):
    Now have type Running.
    (BlockedSignal.handleUnblock): Call sendContinue() on new state.
    * TestTaskObserverInstruction.java: Don't test Isa.isTaskStepped().
    * TestTaskObserverInstructionAndCode.java: Enable.

There is one thing that changed in the semantics of Isa.isTaskStepped()
for x86 and x86_64 (and Ia32On64 has been added). That is that the
stepping flag in the d6 register is being reset because "[the d6]
register is never cleared by the processor and must be cleared by
software after the contents have been read". This means that we are now
doing a inferior visible change, but I don't see any way to get around
this. If the inferior would be using instruction stepping itself there
would be all kinds of interesting issues anyway.

This also means the original tests have been removed from
TestTaskInstructionObserver since the stepping flag isn't visible from
that code path anymore, it is heavily tested now anyway since it is of
course checked through TaskState now. And the newly enabled
TestTaskObserverInstructionAndCode.

Cheers,

Mark
Index: frysk-core/frysk/proc/Breakpoint.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/Breakpoint.java,v
retrieving revision 1.5
diff -u -r1.5 Breakpoint.java
--- frysk-core/frysk/proc/Breakpoint.java	4 Dec 2006 20:53:55 -0000	1.5
+++ frysk-core/frysk/proc/Breakpoint.java	15 Jan 2007 13:23:26 -0000
@@ -234,7 +234,8 @@
           
     try
       {
-        set(task);
+	if (isInstalled())
+	  set(task);
       }
     catch (TaskException e)
       {
@@ -251,6 +252,17 @@
     stepping = false;
   }
 
+  /**
+   * Returns true if break point is installed and not yet removed.
+   */
+  public boolean isInstalled()
+  {
+    synchronized(installed)
+      {
+	return this.equals(installed.get(this));
+      }
+  }
+
   // Utility methods for keeping the map of breakpoints.
 
   public int hashCode()
@@ -270,6 +282,6 @@
   public String toString()
   {
     return this.getClass().getName() + "[proc=" + proc
-      + ", address=" + Long.toHexString(address) + "]";
+      + ", address=0x" + Long.toHexString(address) + "]";
   }
 }
Index: frysk-core/frysk/proc/IsaIA32.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/IsaIA32.java,v
retrieving revision 1.12
diff -u -r1.12 IsaIA32.java
--- frysk-core/frysk/proc/IsaIA32.java	12 Jan 2007 21:55:22 -0000	1.12
+++ frysk-core/frysk/proc/IsaIA32.java	15 Jan 2007 13:23:26 -0000
@@ -292,10 +292,15 @@
    * Reports whether or not the given Task just did a step of an
    * instruction.  This can be deduced by examining the single step
    * flag (BS bit 14) in the debug status register (DR6) on x86.
+   * This resets the stepping flag.
    */
   public boolean isTaskStepped(Task task)
   {
-    return (getRegisterByName("d6").get(task) & 0x4000) != 0;
+    Register d6 = getRegisterByName("d6");
+    long value = d6.get(task);
+    boolean stepped = (value & 0x4000) != 0;
+    d6.put(task, value & ~0x4000);
+    return stepped;
   }
 
   public Syscall[] getSyscallList ()
Index: frysk-core/frysk/proc/IsaX8664.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/IsaX8664.java,v
retrieving revision 1.4
diff -u -r1.4 IsaX8664.java
--- frysk-core/frysk/proc/IsaX8664.java	12 Jan 2007 21:55:22 -0000	1.4
+++ frysk-core/frysk/proc/IsaX8664.java	15 Jan 2007 13:23:26 -0000
@@ -283,10 +283,15 @@
    * Reports whether or not the given Task just did a step of an
    * instruction.  This can be deduced by examining the single step
    * flag (BS bit 14) in the debug status register (DR6) on x86_64.
+   * This resets the stepping flag.
    */
   public boolean isTaskStepped(Task task)
   {
-    return (getRegisterByName("d6").get(task) & 0x4000) != 0;
+    Register d6 = getRegisterByName("d6");
+    long value = d6.get(task);
+    boolean stepped = (value & 0x4000) != 0;
+    d6.put(task, value & ~0x4000);
+    return stepped;
   }
 
   public Syscall[] getSyscallList ()
Index: frysk-core/frysk/proc/LinuxIa32On64.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/LinuxIa32On64.java,v
retrieving revision 1.3
diff -u -r1.3 LinuxIa32On64.java
--- frysk-core/frysk/proc/LinuxIa32On64.java	12 Dec 2006 17:06:57 -0000	1.3
+++ frysk-core/frysk/proc/LinuxIa32On64.java	15 Jan 2007 13:23:26 -0000
@@ -1,6 +1,6 @@
 // This file is part of the program FRYSK.
 //
-// Copyright 2006 Red Hat Inc.
+// Copyright 2006, 2007 Red Hat Inc.
 //
 // FRYSK is free software; you can redistribute it and/or modify it
 // under the terms of the GNU General Public License as published by
@@ -234,7 +234,7 @@
   public LinuxIa32On64() 
   {
     super();
-    // TODO: floating point and debug registers
+    // TODO: floating point
     final Register[] regDefs = new Register[] 
       { new IndirectRegister("eax", "rax"),
 	new IndirectRegister("ebx", "rbx"),
@@ -273,6 +273,11 @@
 	String fpName = "xmm" + i;
 	registerMap.put(fpName, new IndirectRegister(fpName, fpName));
       }
+    for (int i = 0; i < 8; i++)
+      {
+	String dbName = "d" + i;
+	registerMap.put(dbName, new IndirectRegister(dbName, dbName));
+      }
   }
 
   public Iterator RegisterIterator()
Index: frysk-core/frysk/proc/LinuxPtraceTaskState.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/LinuxPtraceTaskState.java,v
retrieving revision 1.4
diff -u -r1.4 LinuxPtraceTaskState.java
--- frysk-core/frysk/proc/LinuxPtraceTaskState.java	20 Dec 2006 15:42:00 -0000	1.4
+++ frysk-core/frysk/proc/LinuxPtraceTaskState.java	15 Jan 2007 13:23:27 -0000
@@ -654,16 +654,28 @@
 	}
 	
         /**
-	 * Tells the Task to continue, with or without syscall tracing.
+	 * Tells the Task to continue, keeping in kind pending
+	 * breakpoints, with or without syscall tracing.
 	 */
         private void sendContinue(Task task, int sig)
         {
-	    if (task.instructionObservers.numberOfObservers() > 0)
-		task.sendStepInstruction(sig);
-	    else if (syscalltracing)
-		task.sendSyscallContinue(sig);
-	    else
-		task.sendContinue(sig);
+	  Breakpoint bp = task.steppingBreakpoint;
+	  if (bp != null)
+	    if (! bp.isInstalled())
+	      {
+		// Apparently the breakpoint was removed already.
+		bp.stepDone(task);
+		task.steppingBreakpoint = null;
+		bp = null;
+	      }
+	  
+	  if (bp != null
+	      || task.instructionObservers.numberOfObservers() > 0)
+	    task.sendStepInstruction(sig);
+	  else if (syscalltracing)
+	    task.sendSyscallContinue(sig);
+	  else
+	    task.sendContinue(sig);
         }
 
         /**
@@ -694,7 +706,7 @@
 	    // XXX Real stop event! - Do we want observers here?
 	    // What state should the task be after being stopped?
 	    if (pendingObservations.isEmpty())
-		logger.log(Level.WARNING, "{1} Unhandled real stop event", task);
+		logger.log(Level.WARNING, "{0} Unhandled real stop event", task);
 
 	    Iterator it = pendingObservations.iterator();
 	    while (it.hasNext())
@@ -712,21 +724,16 @@
 		return blockedContinue();
 
 	    // See how to continue depending on the kind of observers.
+	    Running newState;
 	    if (task.instructionObservers.numberOfObservers() > 0)
-		{
-		    task.sendStepInstruction(0);
-		    return insyscall ? inSyscallRunning : running;
-		}
+	      newState = insyscall ? inSyscallRunning : running;
 	    else if (task.syscallObservers.numberOfObservers() > 0)
-		{
-		    task.sendSyscallContinue(0);
-		    return insyscall ? inSyscallRunningTraced : syscallRunning;
-		}
+	      newState = insyscall ? inSyscallRunningTraced : syscallRunning;
 	    else
-		{
-		    task.sendContinue(0);
-		    return insyscall ? inSyscallRunning : running;
-		}
+	      newState = insyscall ? inSyscallRunning : running;
+
+	    newState.sendContinue(task, 0);
+	    return newState;
 	}
 
 	TaskState handleTerminatingEvent (Task task, boolean signal,
@@ -853,70 +860,86 @@
 	 */
 	TaskState handleTrappedEvent (Task task)
 	{
-	    logger.log (Level.FINE, "{0} handleTrappedEvent\n", task);
-
-	    // To be fully correct we should also check that the 'current'
-	    // instruction is right 'after' the breakpoint. Otherwise it could
-	    // actually be a trap generated by the instruction we are currently
-	    // stepping. FIXME.
-	    Breakpoint steppingBreakpoint = task.steppingBreakpoint;
-	    if (steppingBreakpoint != null)
-		{
-		    steppingBreakpoint.stepDone(task);
-		    task.steppingBreakpoint = null;
-		    sendContinue(task, 0);
-		    return this;
-		}
-
-	    long address;
-	    try
+	  logger.log (Level.FINE, "{0} handleTrappedEvent\n", task);
+	  
+	  Isa isa;
+	  try
+	    {
+	      isa = task.getIsa();
+	    }
+	  catch (TaskException tte)
+	    {
+	      // XXX - Now what - did the process die suddenly?
+	      throw new RuntimeException(tte);
+	    }
+
+	  // 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
+	  // real trap event and we should treat it like a trap
+	  // signal.
+	  if (isa.isTaskStepped(task))
+	    {
+	      // Are we stepping a breakpoint? Reset/Reinstall it.
+	      // To be fully correct we should also check that the
+	      // 'current' instruction is right 'after' the
+	      // breakpoint.
+	      Breakpoint steppingBreakpoint = task.steppingBreakpoint;
+	      if (steppingBreakpoint != null)
 		{
-		    address = task.getIsa().getBreakpointAddress(task);
+		  steppingBreakpoint.stepDone(task);
+		  task.steppingBreakpoint = null;
 		}
-	    catch (TaskException tte)
+	      
+	      if (task.notifyInstruction() > 0)
+		return blockedContinue();
+	      else
 		{
-		    // XXX - Now what - did the process die suddenly?
-		    throw new RuntimeException(tte);
+		  sendContinue(task, 0);
+		  return this;
 		}
-
-	    int blockers = task.notifyCodeBreakpoint(address);
-	    if (blockers == -1)
-		{
-		    // Maybe we were stepping this Task
-		    if (task.instructionObservers.numberOfObservers() > 0)
-			{
-			    if (task.notifyInstruction() > 0)
-				return blockedContinue();
-			    else
-				{
-				    sendContinue(task, 0);
-				    return this;
-				}
-			}
-		    else
-			{
-			    // This is not a trap event generated by us.
-			    return handleSignaledEvent (task, Sig.TRAP_);
-			}
+	    }
+	  else
+	    {
+	      // Do we have a breakpoint installed here?
+	      long address = isa.getBreakpointAddress(task);
+	      int blockers = task.notifyCodeBreakpoint(address);
+	      if (blockers >= 0)
+		{
+		  // Sanity check
+		  if (task.steppingBreakpoint != null)
+		    throw new RuntimeException("Already stepping: "
+					       + task.steppingBreakpoint);
+
+		  // Prepare for stepping the breakpoint
+		  try
+		    {
+		      Proc proc = task.getProc();
+		      Breakpoint bp = Breakpoint.create(address, proc);
+		      bp.prepareStep(task);
+		      task.steppingBreakpoint = bp;
+		    }
+		  catch (TaskException te)
+		    {
+		      // Argh, major trouble!
+		      // No way to recover from this one...
+		      throw new RuntimeException(te);
+		    }
+		  
+		  if (blockers == 0)
+		    {
+		      sendContinue(task, 0);
+		      return this;
+		    }
+		  else
+		    return blockedContinue();
 		}
-	    else if (blockers == 0)
+	      else
 		{
-		    try
-			{
-			    Breakpoint bp = Breakpoint.create(address, task.getProc());
-			    bp.prepareStep(task);
-			    task.sendStepInstruction(0);
-			    task.steppingBreakpoint = bp;
-			    return this;
-			}
-		    catch (TaskException te)
-			{
-			    // Argh, major trouble! No way to recover from this one...
-			    throw new RuntimeException(te);
-			}
+		  // This is not a trap event generated by us.
+		  return handleSignaledEvent(task, Sig.TRAP_);
 		}
-	    else
-		return blockedContinue();
+	    }
 	}
 
 	TaskState handleAddObservation(Task task, TaskObservation observation)
@@ -982,21 +1005,21 @@
     /**
      * Sharable instance of the running state.
      */
-    private static final TaskState running =
+    private static final Running running =
 	new Running("running", false, false);
 
     /**
      * Sharable instance of the syscallRunning state.
      */
-    private static final TaskState syscallRunning =
+    private static final Running syscallRunning =
 	new Running("syscallRunning", true, false);
     
     // Task is running inside a syscall.
-    private static final TaskState inSyscallRunning =
+    private static final Running inSyscallRunning =
 	new Running("inSyscallRunning", true, false);
 
     // Task is running inside a syscall.
-    private static final TaskState inSyscallRunningTraced =
+    private static final Running inSyscallRunningTraced =
 	new Running("inSyscallRunningTraced", true, true);
 
     private static final TaskState detaching = new TaskState ("detaching")
@@ -1121,25 +1144,19 @@
 	}
 	TaskState handleUnblock (Task task, TaskObserver observer)
 	{
-	    logger.log (Level.FINE, "{0} handleUnblock\n", task); 
-	    task.blockers.remove (observer);
-	    if (task.blockers.size () > 0)
-		return this; // Still blocked.
-	    if (task.instructionObservers.numberOfObservers() > 0)
-		{
-		    task.sendStepInstruction(sig);
-		    return running;
-		}
-	    if (task.syscallObservers.numberOfObservers() > 0)
-		{
-		    task.sendSyscallContinue(sig);
-		    return insyscall ? inSyscallRunningTraced : syscallRunning;
-		}
-	    else
-		{
-		    task.sendContinue (sig);
-		    return running;
-		}
+	  logger.log (Level.FINE, "{0} handleUnblock\n", task); 
+	  task.blockers.remove (observer);
+	  if (task.blockers.size () > 0)
+	    return this; // Still blocked.
+	  Running newState;
+	  if (task.instructionObservers.numberOfObservers() > 0)
+	    newState = insyscall ? inSyscallRunning : running;
+	  if (task.syscallObservers.numberOfObservers() > 0)
+	    newState = insyscall ? inSyscallRunningTraced : syscallRunning;
+	  else
+	    newState = running;
+	  newState.sendContinue(task, 0);
+	  return newState;
 	}
 	
 	TaskState handleDeleteObservation(Task task, TaskObservation observation)
Index: frysk-core/frysk/proc/TestTaskObserverInstruction.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/TestTaskObserverInstruction.java,v
retrieving revision 1.3
diff -u -r1.3 TestTaskObserverInstruction.java
--- frysk-core/frysk/proc/TestTaskObserverInstruction.java	12 Jan 2007 21:55:22 -0000	1.3
+++ frysk-core/frysk/proc/TestTaskObserverInstruction.java	15 Jan 2007 13:23:27 -0000
@@ -56,8 +56,6 @@
     assertTrue("added", instr1.added);
     assertEquals("hit", 1, instr1.hit);
 
-    assertFalse("!isa.isTaskStepped()", task.getIsa().isTaskStepped(task));
-
     task.requestUnblock(instr1);
     assertRunUntilStop("unblock self and hit");
     
@@ -65,8 +63,6 @@
     assertTrue("added", instr1.added);
     assertEquals("hit", 2, instr1.hit);
 
-    assertTrue("isa.isTaskStepped()", task.getIsa().isTaskStepped(task));
-
     InstructionObserver instr2 = new InstructionObserver();
 
     task.requestAddInstructionObserver(instr2);
Index: frysk-core/frysk/proc/TestTaskObserverInstructionAndCode.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/TestTaskObserverInstructionAndCode.java,v
retrieving revision 1.2
diff -u -r1.2 TestTaskObserverInstructionAndCode.java
--- frysk-core/frysk/proc/TestTaskObserverInstructionAndCode.java	7 Dec 2006 20:27:06 -0000	1.2
+++ frysk-core/frysk/proc/TestTaskObserverInstructionAndCode.java	15 Jan 2007 13:23:27 -0000
@@ -43,9 +43,6 @@
 {
   public void testInstructionAndCode()
   {
-    if (brokenXXX(3676))
-      return;
-
     // Create busy child.
     Child child = new AckDaemonProcess(true);
     Task task = child.findTaskUsingRefresh (true);
@@ -91,7 +88,8 @@
     task.requestAddCodeObserver(code, address);
     instr.setContinue(true);
     task.requestAddInstructionObserver(instr);
-    // XXX - and here the inferior crashes - bug #3500 ?
+
+    // And here the inferior used to crash - bug #3676
     assertRunUntilStop("add both then wait for block");
 
     // Verify the code observer got hit.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]