public inbox for frysk@sourceware.org
 help / color / mirror / Atom feed
* Isa support for isTaskStepped() and debug registers for x86/x86_64
@ 2007-01-12 21:56 Mark Wielaard
  2007-01-15 13:51 ` Mark Wielaard
  0 siblings, 1 reply; 2+ messages in thread
From: Mark Wielaard @ 2007-01-12 21:56 UTC (permalink / raw)
  To: Frysk List

[-- Attachment #1: Type: text/plain, Size: 2009 bytes --]

Hi,

I tried to add support for tracking both stepping and breakpointing to
recording all state ourselves. We do really need to query whether or not
we got a trap event either because we are hitting a breakpoint or
because of a step event. Unfortunately we are using waitpid() which
means we cannot get the siginfo_t which could have given us that
information. 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. This now works on
x86 and x86_64, but not yet on powerpc (the test will fail there). Since
I don't have access to powerpc I won't add the support myself. If
someone that has access to a powerpc could take a look that would be
appreciated. Note that in principle isTaskStepped() is independent from
debug register support, that was just the way to make it work for
x86/x86_64. As said above it might also come from waitid() or some other
mechanism on some other architecture if applicable.

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

    * Isa.java (isTaskStepped): New method.
    * IsaIA32.java (I387_OFFSET): Removed unused field.
    (DBG_OFFSET): Document how to get value.
    (getRegisterBankBuffers): Add "USR" bank.
    (DBGRegister): New static class.
    (dbg): Removed unused field.
    (IsaIA32): Create debug register and add to registerMap.
    (isTaskStepped): New method.
    * IsaPowerPC.java (isTaskStepped): Implemented (as returning false).
    * IsaX8664.java (FPREGS_OFFSET): removed unused field.
    (DBG_OFFSET): Document and correct value.
    (DBGRegister): New static class.
    (IsaX8664): Create debug register and add to registerMap.
    (isTaskStepped): New method.
    (getRegisterBankBuffers): Add "USR" bank.
    * TestTaskObserverInstruction.java: Add isTaskStepped() tests.

Cheers,

Mark

[-- Attachment #2: isStepped-debugreg.patch --]
[-- Type: text/x-patch, Size: 9977 bytes --]

Index: frysk-core/frysk/proc/Isa.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/Isa.java,v
retrieving revision 1.14
diff -u -r1.14 Isa.java
--- frysk-core/frysk/proc/Isa.java	3 Oct 2006 14:42:02 -0000	1.14
+++ frysk-core/frysk/proc/Isa.java	12 Jan 2007 21:03:42 -0000
@@ -110,6 +110,12 @@
   long getBreakpointAddress(Task task);
 
   /**
+   * Reports whether or not the given Task just did a step of an
+   * instruction.
+   */
+  boolean isTaskStepped(Task task);
+
+  /**
    * Return an array of ByteBuffers for accessing the register
    * banks. It's possible for different elements of the array to be
    * shared.
Index: frysk-core/frysk/proc/IsaIA32.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/IsaIA32.java,v
retrieving revision 1.11
diff -u -r1.11 IsaIA32.java
--- frysk-core/frysk/proc/IsaIA32.java	21 Nov 2006 14:05:15 -0000	1.11
+++ frysk-core/frysk/proc/IsaIA32.java	12 Jan 2007 21:03:42 -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
@@ -46,13 +46,28 @@
 import lib.unwind.RegisterX86;
 import inua.eio.ByteBuffer;
 import frysk.sys.Ptrace;
+import frysk.sys.PtraceByteBuffer;
 
 import frysk.sys.RegisterSetBuffer;
 
 class IsaIA32 implements Isa
 {
-  static final int I387_OFFSET = 18*4;
-  static final int DBG_OFFSET = 63 * 4;
+  /**
+   * Offset into user struct from user.h. Determined with:
+   *
+   * #include <sys/types.h>
+   * #include <sys/user.h>
+   * #include <stdio.h>
+   *
+   * #define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
+   * 
+   * int
+   * main (int argc, char **argv)
+   * {
+   *   printf("DBG_OFFSET = %d\n", offsetof(struct user, u_debugreg[0]));
+   * }
+   */
+  private static final int DBG_OFFSET = 252;
 
   private static final byte[] BREAKPOINT_INSTRUCTION = { (byte)0xcc };
   
@@ -60,13 +75,18 @@
   
   public ByteBuffer[] getRegisterBankBuffers(int pid) 
   {
-    bankBuffers = new ByteBuffer[3];
+    bankBuffers = new ByteBuffer[4];
     int[] bankNames =  { Ptrace.REGS, Ptrace.FPREGS, Ptrace.FPXREGS };
     for (int i = 0; i < 3; i++) 
       {
 	bankBuffers[i] = new RegisterSetBuffer(bankNames[i], pid);
 	bankBuffers[i].order(getByteOrder());
       }
+
+    // Debug registers come from the USR area.
+    bankBuffers[3] = new PtraceByteBuffer(pid, PtraceByteBuffer.Area.USR);
+    bankBuffers[3].order(getByteOrder());
+
     return bankBuffers;
   }
   
@@ -142,7 +162,20 @@
       super(2, 160 + regNum * 16, 16, name, xmmViews);
     }
   }
-  
+
+  /**
+   * Debug registers come from the debug bank (USR area) starting at
+   * DBG_OFFSET, are 4 bytes long and are named d0 till d7.
+   */
+  static class DBGRegister
+    extends Register
+  {
+    DBGRegister(int d)
+    {
+      super(3, DBG_OFFSET + d * 4, 4, "d" + d);
+    }
+  }
+
   private static final IA32Register[] 
   regDefs = { new IA32Register("eax", 6),
 	      new IA32Register("ebx", 0),
@@ -164,11 +197,6 @@
 
   private LinkedHashMap registerMap = new LinkedHashMap();
 
-  // No one is using the FP or debug registers yet, but here are
-  // some definitions for them.
-  // Register[] st = new Register[10];
-  Register[] dbg = new Register[8];
-
   IsaIA32()
   {
     for (int i = 0; i < regDefs.length; i++) 
@@ -189,13 +217,10 @@
 	String name = "xmm" + i;
 	registerMap.put(name, new XMMRegister(name, i));
       }
-    // don't print out debug registers for now
-    if (false) 
+    for (int i = 0; i < 8; i++) 
       {
-	for (int i = 0; i < dbg.length; i++) 
-	  {
-	    dbg[i] = new Register(0, DBG_OFFSET + i*4, 4, "d" + i);
-	  }
+	Register reg = new DBGRegister(i);
+	registerMap.put(reg.getName(), reg);
       }
   }
     
@@ -263,6 +288,16 @@
     return pcValue;
   }
 
+  /**
+   * 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.
+   */
+  public boolean isTaskStepped(Task task)
+  {
+    return (getRegisterByName("d6").get(task) & 0x4000) != 0;
+  }
+
   public Syscall[] getSyscallList ()
   {
     return LinuxIa32Syscall.syscallList;
Index: frysk-core/frysk/proc/IsaPowerPC.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/IsaPowerPC.java,v
retrieving revision 1.1
diff -u -r1.1 IsaPowerPC.java
--- frysk-core/frysk/proc/IsaPowerPC.java	9 Oct 2006 10:00:23 -0000	1.1
+++ frysk-core/frysk/proc/IsaPowerPC.java	12 Jan 2007 21:03:42 -0000
@@ -116,6 +116,14 @@
     return pcValue;
   }
 
+  /**
+   * FIXME. Not yet implemented for PowerPC platform.
+   */
+  public boolean isTaskStepped(Task task)
+  {
+    return false;
+  }
+
   public Syscall[] getSyscallList ()
   {
     return LinuxPowerPCSyscall.syscallList;
Index: frysk-core/frysk/proc/IsaX8664.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/IsaX8664.java,v
retrieving revision 1.3
diff -u -r1.3 IsaX8664.java
--- frysk-core/frysk/proc/IsaX8664.java	28 Nov 2006 21:28:12 -0000	1.3
+++ frysk-core/frysk/proc/IsaX8664.java	12 Jan 2007 21:03:42 -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
@@ -45,14 +45,29 @@
 import inua.eio.ByteOrder;
 import inua.eio.ByteBuffer;
 import frysk.sys.Ptrace;
+import frysk.sys.PtraceByteBuffer;
 import frysk.sys.RegisterSetBuffer;
 
 import lib.unwind.RegisterAMD64;
 
 class IsaX8664 implements Isa
 {
-  static final int FPREGS_OFFSET = 28 * 8;
-  static final int DBG_OFFSET = 32 * 8;
+  /**
+   * Offset into user struct from user.h. Determined with:
+   *
+   * #include <sys/types.h>
+   * #include <sys/user.h>
+   * #include <stdio.h>
+   *
+   * #define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
+   * 
+   * int
+   * main (int argc, char **argv)
+   * {
+   *   printf("DBG_OFFSET = %d\n", offsetof(struct user, u_debugreg[0]));
+   * }
+   */
+  private static final int DBG_OFFSET = 848;
 
   private static final byte[] BREAKPOINT_INSTRUCTION = { (byte)0xcc };
   
@@ -125,6 +140,19 @@
     }
   }
 
+  /**
+   * Debug registers come from the debug bank (USR area) starting at
+   * DBG_OFFSET, are 8 bytes long and are named d0 till d7.
+   */
+  static class DBGRegister
+    extends Register
+  {
+    DBGRegister(int d)
+    {
+      super(2, DBG_OFFSET + d * 8, 8, "d" + d);
+    }
+  }
+
   private static final X8664Register[] regDefs
   = { new X8664Register("rax", 10),
       new X8664Register("rbx", 5),
@@ -180,7 +208,11 @@
 	String name = "xmm" + i;
         registerMap.put(name, new XMMRegister(name, i));
       }
-    
+    for (int i = 0; i < 8; i++)
+      {
+	Register reg = new DBGRegister(i);
+	registerMap.put(reg.getName(), reg);
+      }
   }
 
   public Iterator RegisterIterator()
@@ -247,6 +279,16 @@
     return pcValue;
   }
 
+  /**
+   * 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.
+   */
+  public boolean isTaskStepped(Task task)
+  {
+    return (getRegisterByName("d6").get(task) & 0x4000) != 0;
+  }
+
   public Syscall[] getSyscallList ()
   {
     return LinuxX8664Syscall.syscallList;
@@ -264,13 +306,18 @@
 
   public ByteBuffer[] getRegisterBankBuffers(int pid) 
   {
-    ByteBuffer[] bankBuffers = new ByteBuffer[2];
+    ByteBuffer[] bankBuffers = new ByteBuffer[3];
     int[] bankNames =  { Ptrace.REGS, Ptrace.FPREGS };
     for (int i = 0; i < 2; i++) 
       {
 	bankBuffers[i] = new RegisterSetBuffer(bankNames[i], pid);
 	bankBuffers[i].order(getByteOrder());
       }
+
+    // Debug registers come from the USR area.
+    bankBuffers[2] = new PtraceByteBuffer(pid, PtraceByteBuffer.Area.USR);
+    bankBuffers[2].order(getByteOrder());
+    
     return bankBuffers;
   }
 }
Index: frysk-core/frysk/proc/TestTaskObserverInstruction.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/TestTaskObserverInstruction.java,v
retrieving revision 1.2
diff -u -r1.2 TestTaskObserverInstruction.java
--- frysk-core/frysk/proc/TestTaskObserverInstruction.java	6 Dec 2006 14:11:57 -0000	1.2
+++ frysk-core/frysk/proc/TestTaskObserverInstruction.java	12 Jan 2007 21:03:42 -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
@@ -41,7 +41,7 @@
 
 public class TestTaskObserverInstruction extends TestLib
 {
-  public void testInstruction()
+  public void testInstruction() throws TaskException
   {
     // We want a busy child, because we are going to follow its steps.
     Child child = new AckDaemonProcess(true);
@@ -56,6 +56,8 @@
     assertTrue("added", instr1.added);
     assertEquals("hit", 1, instr1.hit);
 
+    assertFalse("!isa.isTaskStepped()", task.getIsa().isTaskStepped(task));
+
     task.requestUnblock(instr1);
     assertRunUntilStop("unblock self and hit");
     
@@ -63,6 +65,8 @@
     assertTrue("added", instr1.added);
     assertEquals("hit", 2, instr1.hit);
 
+    assertTrue("isa.isTaskStepped()", task.getIsa().isTaskStepped(task));
+
     InstructionObserver instr2 = new InstructionObserver();
 
     task.requestAddInstructionObserver(instr2);

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Isa support for isTaskStepped() and debug registers for  x86/x86_64
  2007-01-12 21:56 Isa support for isTaskStepped() and debug registers for x86/x86_64 Mark Wielaard
@ 2007-01-15 13:51 ` Mark Wielaard
  0 siblings, 0 replies; 2+ messages in thread
From: Mark Wielaard @ 2007-01-15 13:51 UTC (permalink / raw)
  To: Frysk List

[-- Attachment #1: Type: text/plain, Size: 2663 bytes --]

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

[-- Attachment #2: stepbreakpoint.patch --]
[-- Type: text/x-patch, Size: 15329 bytes --]

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.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2007-01-15 13:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-01-12 21:56 Isa support for isTaskStepped() and debug registers for x86/x86_64 Mark Wielaard
2007-01-15 13:51 ` Mark Wielaard

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).