public inbox for frysk@sourceware.org
 help / color / mirror / Atom feed
* Mixing single-stepping and breakpointing
@ 2007-02-07 18:09 Mark Wielaard
  2007-02-12 15:24 ` Mark Wielaard
  0 siblings, 1 reply; 2+ messages in thread
From: Mark Wielaard @ 2007-02-07 18:09 UTC (permalink / raw)
  To: frysk

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

Hi,

Now that bug #3997 has been identified as the thing that caused strange
results on some machines (sig trap handler gets reset in the inferior
when ptrace single stepping the handler) here is finally the last
missing piece to make stepping work together with breakpointing and user
traps. The testcase (funit-breakpoints) has a workaround for now, but if
your machine fails the frysk-imports/tests/frysk3997/ptrace_step_sig
test then you have a buggy kernel installed and stepping through sigtrap
handlers will result in trouble.

The other thing that makes stepping slightly inconvenient is that on
some architectures you will get spurious trap events (for example for
system calls). So the Isa now defines a method hasExecutedSpuriousTrap()
that checks whether the last instruction was a trapping instruction that
the kernel will handle (if not, then we do need to signal a SIGTRAP to
the inferior). This only happens on x86, but not on x86_64 as far as I
can see. PowerPC is a stub for now (please check if you have access to
such a machine).

We also need to keep track of whether a signal was just send to the
inferior. In that case we do get a SIGTRAP, but not a step (the kernel
adjusts the pc to point to the start of the signal handle). We need to
distinguish this case from a real trap signal that also doesn't generate
a step, but which indicates that we need to signal the SIGTRAP to the
inferior.

All tests in TestBreakpoints has been duplicated to be run with or
without stepping the full test (checking whether breakpoints are still
hit). The number of iterations has been brought down from 42 till 7
because stepping a whole process is such a slow thing and we do want the
tests to run reasonable fast.

2007-02-07  Mark Wielaard  <mark@klomp.org>

        * funit-breakpoints.c (send_hup, receive_hup): New static ints.
        (trap_handler): Renamed to ...
        (signal_handler): Now also handles SIGHUP.
        (dummy): Send SIGHUP. Add SIGTRAP signal install workaround.
        (main): Check both hup and trap counts.

2007-02-07  Mark Wielaard  <mark@klomp.org>

        * Isa.java (hasExecutedSpuriousTrap): New method.
        * IsaIA32.java (hasExecutedSpuriousTrap): Likewise.
        * IsaPowerPC.java (hasExecutedSpuriousTrap): Likewise.
        * IsaX8664.java (hasExecutedSpuriousTrap): Likewise.
        * LinuxPtraceTask.java (sendContinue): Setup step_send and sig_send.
        (sendSyscallContinue): Likewise.
        (sendStepInstruction): Likewise.
        * LinuxPtraceTaskState.java (Running.handleTrappedEvent):
        Add sanity checks and check for spurios traps and signal handler
        entry. Chain to handleSignaledEvent() otherwise.
        * Task.java (step_send, sig_send): New fields.

        * TestBreakpoints.java (installInstructionObserver): New field.
        (setUp): Initialize installInstructionObserver to false.
        (testHitAndRun): Check whether an InstructionObserver should be
        installed.
        (testInsertRemove): Likewise.
        (testAddLots): Likewise.
        (testSteppingtestHitAndRun): New method.
        (testSteppingtestInsertRemove): New method.
        (testSteppingAddLots): New method.
        (InstructionObserver): New static class.

Committed,

Mark

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

Index: frysk/pkglibdir/funit-breakpoints.c
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/pkglibdir/funit-breakpoints.c,v
retrieving revision 1.1
diff -u -r1.1 funit-breakpoints.c
--- frysk/pkglibdir/funit-breakpoints.c	11 Dec 2006 04:18:05 -0000	1.1
+++ frysk/pkglibdir/funit-breakpoints.c	7 Feb 2007 18:03:02 -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
@@ -58,33 +58,40 @@
 static int send_trap;
 static int received_trap;
 
+static int send_hup;
+static int received_hup;
+
 static sigjmp_buf env;
 
 static void
-trap_handler(int sig)
+signal_handler(int sig)
 {
-  if (sig != SIGTRAP)
+  if (sig == SIGHUP)
+    {
+      received_hup++;
+    }
+  else if (sig == SIGTRAP)
+    {
+      received_trap++;
+      // Trap handler is triggered by inline invalide instruction in dummy()
+      // longjmp around it.
+      siglongjmp(env, SIGTRAP);
+    }
+  else
     {
       fprintf (stderr, "Wrong signal recieved %d\n", sig);
       exit (-1);
     }
-
-  received_trap++;
-
-  // Trap handler is triggered by inline invalide instruction in
-  // dummy().
-  if (received_trap % 2 == 0)
-    siglongjmp(env, SIGTRAP);
 }
 
-// Tries to trick frysk by sending trap signals and by having its
+// Tries to trick frysk by sending sighup signals and by having its
 // own trap instruction.
 static void
 dummy()
 {
-  // Sending ourselves a trap signal.
-  kill (pid, SIGTRAP);
-  send_trap++;
+  // Sending ourselves an HUP signal.
+  kill (pid, SIGHUP);
+  send_hup++;
 
   // Generating a trap event ourselves, simulating "bad code".  Setup
   // a sigsetjump so we can handle it and return from this function
@@ -93,6 +100,7 @@
   // instructions. So this makes sure we skip it when we return.
   if (sigsetjmp (env, 1) == 0)
     {
+      send_trap++;
 #if defined(__i386__) || defined(__x86_64__)
       asm("int3");
 #elif defined(__powerpc64__) || defined(__powerpc__)
@@ -100,10 +108,15 @@
 #else
       #error unsuported architecture
 #endif
+      abort(); // Should never be reached.
     }
   else
     {
       // Returned from signal handler through longjmp.
+      // XXX - Note that bug #3997 causes the trap sig handler to be reset
+      // on some kernels when single stepping.
+      // So for now we explicitly set it just to make this testcase work.
+      signal (SIGTRAP, &signal_handler);
       return;
     }
 }
@@ -128,7 +141,11 @@
   pid = getpid();
   received_trap = 0;
   send_trap = 0;
-  signal (SIGTRAP, &trap_handler);
+  received_hup = 0;
+  send_hup = 0;
+
+  signal (SIGTRAP, &signal_handler);
+  signal (SIGHUP, &signal_handler);
 
   // The number of runs the tester wants us to do.
   // Zero when we should terminate.
@@ -179,9 +196,14 @@
     }
 
   // Have our own trap signals and trap instructions triggered?
-  if (2 * send_trap != received_trap)
+  if (send_trap != received_trap)
+    {
+      fprintf (stderr, "TRAP send: %d, recv: %d\n", send_trap, received_trap);
+      exit (-1);
+    }
+  if (send_hup != received_hup)
     {
-      fprintf (stderr, "send: %d, recv: %d\n", send_trap, received_trap);
+      fprintf (stderr, "HUP send: %d, recv: %d\n", send_hup, received_hup);
       exit (-1);
     }
 
Index: frysk/proc/Isa.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/Isa.java,v
retrieving revision 1.16
diff -u -r1.16 Isa.java
--- frysk/proc/Isa.java	22 Jan 2007 13:35:40 -0000	1.16
+++ frysk/proc/Isa.java	7 Feb 2007 18:03:03 -0000
@@ -116,6 +116,17 @@
   boolean isTaskStepped(Task task);
 
   /**
+   * Returns true if the last instruction executed by the given Task was
+   * a trapping instruction. This method should distinquish instructions
+   * that are handled by the kernel (like syscall enter instructions) and
+   * those that generate a trap signal. True is returned only when the
+   * instruction should generate a signal. Called from the state machine
+   * when a trap event has been detected that cannot be attributed to
+   * entering a signal handler or a normal step instruction notification.
+   */
+  boolean hasExecutedSpuriousTrap(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/proc/IsaIA32.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/IsaIA32.java,v
retrieving revision 1.15
diff -u -r1.15 IsaIA32.java
--- frysk/proc/IsaIA32.java	22 Jan 2007 13:35:40 -0000	1.15
+++ frysk/proc/IsaIA32.java	7 Feb 2007 18:03:03 -0000
@@ -304,6 +304,27 @@
     return stepped;
   }
 
+  /**
+   * Returns true if the last instruction executed by the given Task
+   * was a trapping instruction that will be handled by the
+   * kernel. This method should distinquish instructions that are
+   * handled by the kernel (like syscall enter instructions) and those
+   * that generate a trap signal. True is returned only when the
+   * instruction shouldn't generate a signal. Called from the state
+   * machine when a trap event has been detected that cannot be
+   * attributed to entering a signal handler or a normal step
+   * instruction notification.
+   * <p>
+   * ia32 generate spurious trap events on "int 0x80" instructions
+   * that should trap into a kernel syscall.
+   */
+  public boolean hasExecutedSpuriousTrap(Task task)
+  {
+    long address = pc(task);
+    return (task.getMemory().getByte(address - 1) == (byte) 0x80
+	    && task.getMemory().getByte(address - 2) == (byte) 0xcd);
+  }
+
   public Syscall[] getSyscallList ()
   {
     return LinuxIa32Syscall.syscallList;
Index: frysk/proc/IsaPowerPC.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/IsaPowerPC.java,v
retrieving revision 1.2
diff -u -r1.2 IsaPowerPC.java
--- frysk/proc/IsaPowerPC.java	12 Jan 2007 21:55:22 -0000	1.2
+++ frysk/proc/IsaPowerPC.java	7 Feb 2007 18:03:03 -0000
@@ -124,6 +124,15 @@
     return false;
   }
 
+  /**
+   * FIXME. Not checked whether or not spurious trap events are
+   * generated on PowerPC.
+   */
+  public boolean hasExecutedSpuriousTrap(Task task)
+  {
+    return false;
+  }
+
   public Syscall[] getSyscallList ()
   {
     return LinuxPowerPCSyscall.syscallList;
Index: frysk/proc/IsaX8664.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/IsaX8664.java,v
retrieving revision 1.7
diff -u -r1.7 IsaX8664.java
--- frysk/proc/IsaX8664.java	22 Jan 2007 13:35:40 -0000	1.7
+++ frysk/proc/IsaX8664.java	7 Feb 2007 18:03:03 -0000
@@ -295,6 +295,25 @@
     return stepped;
   }
 
+  /**
+   * Returns true if the last instruction executed by the given Task
+   * was a trapping instruction that will be handled by the
+   * kernel. This method should distinquish instructions that are
+   * handled by the kernel (like syscall enter instructions) and those
+   * that generate a trap signal. True is returned only when the
+   * instruction shouldn't generate a signal. Called from the state
+   * machine when a trap event has been detected that cannot be
+   * attributed to entering a signal handler or a normal step
+   * instruction notification.
+   * 
+   * x86_64 doesn't generate spurious trap events and this method
+   * always returns false on this architecture.
+   */
+  public boolean hasExecutedSpuriousTrap(Task task)
+  {
+    return false;
+  }
+
   public Syscall[] getSyscallList ()
   {
     return LinuxX8664Syscall.syscallList;
Index: frysk/proc/LinuxPtraceTask.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/LinuxPtraceTask.java,v
retrieving revision 1.6
diff -u -r1.6 LinuxPtraceTask.java
--- frysk/proc/LinuxPtraceTask.java	23 Jan 2007 16:59:33 -0000	1.6
+++ frysk/proc/LinuxPtraceTask.java	7 Feb 2007 18:03:03 -0000
@@ -131,6 +131,8 @@
   protected void sendContinue (int sig)
   {
     logger.log(Level.FINE, "{0} sendContinue\n", this);
+    step_send = false;
+    sig_send = sig;
     try
       {
         Ptrace.cont(getTid(), sig);
@@ -144,6 +146,8 @@
   protected void sendSyscallContinue (int sig)
   {
     logger.log(Level.FINE, "{0} sendSyscallContinue\n", this);
+    step_send = false;
+    sig_send = sig;
     try
       {
         Ptrace.sysCall(getTid(), sig);
@@ -157,6 +161,8 @@
   protected void sendStepInstruction (int sig)
   {
     logger.log(Level.FINE, "{0} sendStepInstruction\n", this);
+    step_send = true;
+    sig_send = sig;
     try
       {
         Ptrace.singleStep(getTid(), sig);
Index: frysk/proc/LinuxPtraceTaskState.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/LinuxPtraceTaskState.java,v
retrieving revision 1.6
diff -u -r1.6 LinuxPtraceTaskState.java
--- frysk/proc/LinuxPtraceTaskState.java	23 Jan 2007 16:24:50 -0000	1.6
+++ frysk/proc/LinuxPtraceTaskState.java	7 Feb 2007 18:03:03 -0000
@@ -668,7 +668,9 @@
 		task.steppingBreakpoint = null;
 		bp = null;
 	      }
-	  
+	 
+	  // Step when there is a breakpoint at the current location
+	  // or there are Instruction observers installed.
 	  if (bp != null
 	      || task.instructionObservers.numberOfObservers() > 0)
 	    task.sendStepInstruction(sig);
@@ -872,6 +874,11 @@
 	  // signal.
 	  if (isa.isTaskStepped(task))
 	    {
+	      // Sanity check
+	      if (! task.step_send)
+		throw new IllegalStateException
+		  ("isTaskStepped() unexpectedly returned true");
+
 	      // Are we stepping a breakpoint? Reset/Reinstall it.
 	      // To be fully correct we should also check that the
 	      // 'current' instruction is right 'after' the
@@ -920,7 +927,29 @@
 	      else
 		{
 		  // This is not a trap event generated by us.
-		  return handleSignaledEvent(task, Sig.TRAP_);
+
+		  // When we just send a step to the task there are
+		  // two reasons we might get a Trap event that
+		  // doesn't relate to an actual instruction
+		  // step. Either we requested a step into a signal
+		  // (which depending on architecture does or doesn't
+		  // set the step debug flag).  Or a instruction that
+		  // looks like it is generating a trap is encountered
+		  // but that the kernel will handle (like a syscall
+		  // trapping instruction) that we should ignore (this
+		  // would be nice to use to support syscall tracking
+		  // during stepping, but it doesn't happen on all
+		  // architectures).
+		  if (task.step_send
+		      && (task.sig_send != 0
+			  || isa.hasExecutedSpuriousTrap(task)))
+		    {
+		      sendContinue(task, 0);
+		      return this;
+                    }
+
+		  // Deliver the real Trap event to the Task.
+                  return handleSignaledEvent(task, Sig.TRAP_);
 		}
 	    }
 	}
Index: frysk/proc/Task.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/Task.java,v
retrieving revision 1.120
diff -u -r1.120 Task.java
--- frysk/proc/Task.java	23 Jan 2007 16:24:50 -0000	1.120
+++ frysk/proc/Task.java	7 Feb 2007 18:03:03 -0000
@@ -198,7 +198,10 @@
       }
   }
 
-  // Send operation to corresponding underlying [kernel] task.
+  // Send operation to corresponding underlying [kernel] task.  The
+  // continue, syscall and step methods should sig_send and set/reset
+  // step_send to indicate that last reques tot the Task was a single
+  // step.
   protected abstract void sendContinue (int sig);
 
   protected abstract void sendSyscallContinue (int sig);
@@ -898,6 +901,12 @@
   // a step has been issued. Null when no step is being performed.
   Breakpoint steppingBreakpoint;
 
+  // Whether the last request to the process was a step request.
+  boolean step_send;
+
+  // The signal, or zero, send last to the task.
+  int sig_send;
+
   /**
    * Notify all Code observers of the breakpoint. Return the number of
    * blocking observers or -1 if no Code observer were installed on this
Index: frysk/proc/TestBreakpoints.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/TestBreakpoints.java,v
retrieving revision 1.16
diff -u -r1.16 TestBreakpoints.java
--- frysk/proc/TestBreakpoints.java	6 Feb 2007 21:01:50 -0000	1.16
+++ frysk/proc/TestBreakpoints.java	7 Feb 2007 18:03:03 -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
@@ -74,6 +74,9 @@
   // Monitor to notify and wait on for state of event changes..
   static Object monitor = new Object();
 
+  // Whether or not to install an Instruction observer
+  boolean installInstructionObserver;
+
   /**
    * Launch our test program and setup clean environment with a runner
    * eventLoop.
@@ -84,6 +87,9 @@
     // and destroyed in tearDown().
     super.setUp();
 
+    // Some tests run with an InstructionObserver. Default is not.
+    installInstructionObserver = false;
+
     // Create a process that we will communicate with through stdin/out.
     String command = getExecPath ("funit-breakpoints");
     ForkTestLib.ForkedProcess process;
@@ -185,6 +191,15 @@
 	  }
       }
 
+    // Test can run with or without stepping.
+    InstructionObserver io1 = new InstructionObserver(task, breakpoint1);
+    InstructionObserver io2 = new InstructionObserver(task, breakpoint2);
+    if (installInstructionObserver)
+      {
+        task.requestAddInstructionObserver(io1);
+        task.requestAddInstructionObserver(io2);
+      }
+
     // Put in breakpoint observers
     CodeObserver code1 = new CodeObserver(breakpoint1);
     task.requestAddCodeObserver(code1, breakpoint1);
@@ -210,7 +225,7 @@
     // Unblock and tell the process to go some rounds!
     task.requestUnblock(ao);
 
-    out.writeByte(42);
+    out.writeByte(7);
     out.flush();
     
     // Sanity check that the functions have actually been run.
@@ -219,11 +234,27 @@
     line = in.readLine();
     int bp2 = Integer.decode(line).intValue();
 
-    assertEquals(42, bp1);
-    assertEquals(42, bp2);
+    assertEquals(7, bp1);
+    assertEquals(7, bp2);
+
+    assertEquals(7, code1.getTriggered());
+    assertEquals(7, code2.getTriggered());
+
+    // Remove the stepper
+    if (installInstructionObserver)
+      {
+	assertEquals(7, io1.getAddrHit());
+	assertEquals(7, io2.getAddrHit());
 
-    assertEquals(42, code1.getTriggered());
-    assertEquals(42, code2.getTriggered());
+        task.requestDeleteInstructionObserver(io1);
+        task.requestDeleteInstructionObserver(io2);
+      }
+    else
+      {
+	// Shouldn't be triggered.
+	assertEquals(0, io1.getAddrHit());
+	assertEquals(0, io2.getAddrHit());
+      }
 
     // We are all done, you can go now.
     out.writeByte(0);
@@ -270,6 +301,12 @@
     assertFalse(exitSignal);
   }
 
+  public void testSteppingtestHitAndRun() throws IOException
+  {
+    installInstructionObserver = true;
+    testHitAndRun();
+  }
+
   public void testInsertRemove() throws IOException
   {
     // Start an EventLoop so there's no need to poll for events all
@@ -309,6 +346,15 @@
 	  }
       }
 
+    // Test can run with or without stepping.
+    InstructionObserver io1 = new InstructionObserver(task, breakpoint1);
+    InstructionObserver io2 = new InstructionObserver(task, breakpoint2);
+    if (installInstructionObserver)
+      {
+        task.requestAddInstructionObserver(io1);
+        task.requestAddInstructionObserver(io2);
+      }
+
     // Put in breakpoint observers
     CodeObserver code1 = new CodeObserver(breakpoint1);
     CodeObserver code2 = new CodeObserver(breakpoint2);
@@ -430,6 +476,22 @@
 	  }
       }
 
+    // Remove the stepper
+    if (installInstructionObserver)
+      {
+	assertEquals(13, io1.getAddrHit());
+	assertEquals(13, io2.getAddrHit());
+
+        task.requestDeleteInstructionObserver(io1);
+        task.requestDeleteInstructionObserver(io2);
+      }
+    else
+      {
+	// Shouldn't be triggered.
+	assertEquals(0, io1.getAddrHit());
+	assertEquals(0, io2.getAddrHit());
+      }
+
     // And we are done.
     out.writeByte(0);
     out.flush();
@@ -480,6 +542,12 @@
     assertEquals(3, code2.getTriggered());
   }
 
+  public void testSteppingtestInsertRemove() throws IOException
+  {
+    installInstructionObserver = true;
+    testInsertRemove();
+  }
+
   public void testAddLots() throws IOException
   {
     // Start an EventLoop so there's no need to poll for events all
@@ -519,6 +587,15 @@
 	  }
       }
 
+    // Test can run with or without stepping.
+    InstructionObserver io1 = new InstructionObserver(task, breakpoint1);
+    InstructionObserver io2 = new InstructionObserver(task, breakpoint2);
+    if (installInstructionObserver)
+      {
+        task.requestAddInstructionObserver(io1);
+        task.requestAddInstructionObserver(io2);
+      }
+
     // Put in breakpoint observers
     CodeObserver[] codes1 = new CodeObserver[1512];
     for (int i = 0; i < 1512; i++)
@@ -562,7 +639,7 @@
     task.requestUnblock(ao);
 
     // Run a couple of times.
-    out.writeByte(42);
+    out.writeByte(7);
     out.flush();
 
     // Sanity check that the functions have actually been run.
@@ -571,8 +648,24 @@
     line = in.readLine();
     int bp2 = Integer.decode(line).intValue();
 
-    assertEquals(42, bp1);
-    assertEquals(42, bp2);
+    assertEquals(7, bp1);
+    assertEquals(7, bp2);
+
+    // Remove the stepper
+    if (installInstructionObserver)
+      {
+	assertEquals(7, io1.getAddrHit());
+	assertEquals(7, io2.getAddrHit());
+
+        task.requestDeleteInstructionObserver(io1);
+        task.requestDeleteInstructionObserver(io2);
+      }
+    else
+      {
+	// Shouldn't be triggered.
+	assertEquals(0, io1.getAddrHit());
+	assertEquals(0, io2.getAddrHit());
+      }
 
     // And we are done.
     out.writeByte(0);
@@ -627,11 +720,16 @@
 
     for (int i = 0; i < 1512; i++)
       {
-	assertEquals(42, codes1[i].getTriggered());
-	assertEquals(42, codes2[i].getTriggered());
+	assertEquals(7, codes1[i].getTriggered());
+	assertEquals(7, codes2[i].getTriggered());
       }
   }
   
+  public void testSteppingAddLots() throws IOException
+  {
+    installInstructionObserver = true;
+    testAddLots();
+  }
 
   class AttachedObserver implements TaskObserver.Attached
   {
@@ -782,6 +880,56 @@
     }
   }
 
+  static class InstructionObserver implements TaskObserver.Instruction
+  { 
+    boolean added;
+    boolean deleted;
+
+    private final Task task; 
+    private final long addr;
+    private int addr_hit;
+
+    public void addedTo(Object o)
+    {
+      added = true;
+    }
+
+    public void deletedFrom(Object o)
+    {
+      deleted = true;
+    }
+
+    public void addFailed (Object o, Throwable w)
+    {
+      fail("add to " + o + " failed, because " + w);
+    }
+
+    InstructionObserver(Task task, long addr)
+    {
+      this.task = task;
+      this.addr = addr;
+    }
+
+    // Always continue, just counts steps on specified address.
+    public Action updateExecuted(Task task)
+    {
+      if (! task.equals(this.task))
+        throw new IllegalStateException("Wrong Task, given " + task
+                                        + " not equals expected "
+                                        + this.task);
+
+      if (task.getIsa().pc(task) == addr)
+	addr_hit++;
+      return Action.CONTINUE;
+    }
+
+    public int getAddrHit()
+    {
+      return addr_hit;
+    }
+  }
+
+
   static class EventLoopRunner extends Thread
   {
     private boolean stopped;

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

* Re: Mixing single-stepping and breakpointing
  2007-02-07 18:09 Mixing single-stepping and breakpointing Mark Wielaard
@ 2007-02-12 15:24 ` Mark Wielaard
  0 siblings, 0 replies; 2+ messages in thread
From: Mark Wielaard @ 2007-02-12 15:24 UTC (permalink / raw)
  To: frysk

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

Hi,

On Wed, 2007-02-07 at 19:08 +0100, Mark Wielaard wrote:
> The other thing that makes stepping slightly inconvenient is that on
> some architectures you will get spurious trap events (for example for
> system calls). So the Isa now defines a method hasExecutedSpuriousTrap()
> that checks whether the last instruction was a trapping instruction that
> the kernel will handle (if not, then we do need to signal a SIGTRAP to
> the inferior). This only happens on x86, but not on x86_64 as far as I
> can see. PowerPC is a stub for now (please check if you have access to
> such a machine).
> 
> We also need to keep track of whether a signal was just send to the
> inferior. In that case we do get a SIGTRAP, but not a step (the kernel
> adjusts the pc to point to the start of the signal handle). We need to
> distinguish this case from a real trap signal that also doesn't generate
> a step, but which indicates that we need to signal the SIGTRAP to the
> inferior.

And there is a third reason the pc can be "suddenly" adjusted while
stepping which is when the inferior returns from a signal handler
through the sigreturn system call. The following patch adds support for
detecting that case, plus a new test. Again only implemented and tested
on x86 and x86_64.

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

        * funit-alarm.c: New test program.

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

        * Isa.java (isAtSyscallSigReturn): New method.
        * IsaIA32.java (isAtSyscallSigReturn): Likewise.
        * IsaPowerPC.java (isAtSyscallSigReturn): Likewise.
        * IsaX8664.java (isAtSyscallSigReturn): Likewise.
        * LinuxPtraceTask.java (sendStepInstruction): Set syscall_sigret.
        * LinuxPtraceTaskState.java (Running.handleTrapEvent): Check
        syscall_sigret.
        (BlockedSignal.unblock): SendContinue with pending signal.
        * Task.java (syscall_sigret): New field.
        * TestTaskObserverInstructionSigReturn.java: New test.

This patch also fixes a problem where a blocked Task would not get the
pending signal delivered. The new test also triggers this issue.

In theory we could extend this new Isa isAtSyscallSigReturn() support
for more reliably reporting syscall enter and exit even during stepping.
But for now I haven't done this because syscall tracing still is not
really supported even in the non-stepping case. If that is fixed then
this new Isa method could return the actual system call being executed
during stepping and the TaskState machine could use that to supply
syscall enter/exit events to the observers.

Committed,

Mark

[-- Attachment #2: step-sigreturn.patch --]
[-- Type: text/x-patch, Size: 15482 bytes --]

Index: frysk-core/frysk/pkglibdir/funit-alarm.c
===================================================================
RCS file: frysk-core/frysk/pkglibdir/funit-alarm.c
diff -N frysk-core/frysk/pkglibdir/funit-alarm.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ frysk-core/frysk/pkglibdir/funit-alarm.c	12 Feb 2007 15:05:34 -0000
@@ -0,0 +1,98 @@
+// This file is part of the program FRYSK.
+//
+// Copyright 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
+// the Free Software Foundation; version 2 of the License.
+//
+// FRYSK is distributed in the hope that it will be useful, but
+// WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+// General Public License for more details.
+// 
+// You should have received a copy of the GNU General Public License
+// along with FRYSK; if not, write to the Free Software Foundation,
+// Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+// 
+// In addition, as a special exception, Red Hat, Inc. gives You the
+// additional right to link the code of FRYSK with code not covered
+// under the GNU General Public License ("Non-GPL Code") and to
+// distribute linked combinations including the two, subject to the
+// limitations in this paragraph. Non-GPL Code permitted under this
+// exception must only link to the code of FRYSK through those well
+// defined interfaces identified in the file named EXCEPTION found in
+// the source code files (the "Approved Interfaces"). The files of
+// Non-GPL Code may instantiate templates or use macros or inline
+// functions from the Approved Interfaces without causing the
+// resulting work to be covered by the GNU General Public
+// License. Only Red Hat, Inc. may make changes or additions to the
+// list of Approved Interfaces. You must obey the GNU General Public
+// License in all respects for all of the FRYSK code and other code
+// used in conjunction with FRYSK except the Non-GPL Code covered by
+// this exception. If you modify this file, you may extend this
+// exception to your version of the file, but you are not obligated to
+// do so. If you do not wish to provide this exception without
+// modification, you must delete this exception statement from your
+// version and license this file solely under the GPL without
+// exception.
+
+#include <unistd.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/time.h>
+
+// When counter is zero stop.
+static volatile int counter;
+
+// Dummy counter to increment so we look busy.
+static volatile int i;
+
+// struct to set itimer.
+static struct itimerval ival;
+
+static void
+signal_handler(int sig)
+{
+  if (sig == SIGPROF)
+    {
+      counter--;
+      if (counter == 0)
+	{
+	  // Shutdown timer.
+	  ival.it_value.tv_sec = 0;
+	  ival.it_value.tv_usec = 0;
+	  setitimer (ITIMER_PROF, &ival, NULL);
+	}
+    }
+  else
+    {
+      fprintf (stderr, "Wrong signal recieved %d\n", sig);
+      exit (-1);
+    }
+}
+
+int
+main (int argc, char *argv[])
+{
+  counter = 3;
+
+  signal (SIGPROF, &signal_handler);
+
+  // Setup a timer to fire after 0.005 seconds again and again.
+  ival.it_value.tv_sec = 0;
+  ival.it_value.tv_usec = 5000;
+  ival.it_interval.tv_sec = 0;
+  ival.it_interval.tv_usec = 5000;
+  if (setitimer (ITIMER_PROF, &ival, NULL) != 0)
+    {
+      perror ("setitimer failed");
+      exit (-1);
+    }
+
+  while (counter > 0)
+    i += counter; // Do something useful...
+
+  return 0;
+}
Index: frysk-core/frysk/proc/Isa.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/Isa.java,v
retrieving revision 1.17
diff -u -u -r1.17 Isa.java
--- frysk-core/frysk/proc/Isa.java	7 Feb 2007 18:08:25 -0000	1.17
+++ frysk-core/frysk/proc/Isa.java	12 Feb 2007 15:05:34 -0000
@@ -1,6 +1,6 @@
 // This file is part of the program FRYSK.
 //
-// Copyright 2005, 2006 Red Hat Inc.
+// Copyright 2005, 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
@@ -127,6 +127,12 @@
   boolean hasExecutedSpuriousTrap(Task task);
 
   /**
+   * Returns true if the given Task is at an instruction that will invoke
+   * the sig return system call.
+   */
+  boolean isAtSyscallSigReturn(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.16
diff -u -u -r1.16 IsaIA32.java
--- frysk-core/frysk/proc/IsaIA32.java	7 Feb 2007 18:08:25 -0000	1.16
+++ frysk-core/frysk/proc/IsaIA32.java	12 Feb 2007 15:05:34 -0000
@@ -325,6 +325,27 @@
 	    && task.getMemory().getByte(address - 2) == (byte) 0xcd);
   }
 
+  /**
+   * Returns true if the given Task is at an instruction that will invoke
+   * the sig return system call.
+   *
+   * On x86 this is when the pc is at a int 0x80 instruction and the
+   * eax register contains 0x77.
+   */
+  public boolean isAtSyscallSigReturn(Task task)
+  {
+    long address = pc(task);
+    boolean result = (task.getMemory().getByte(address) == (byte) 0xcd
+		      && task.getMemory().getByte(address + 1) == (byte) 0x80);
+    if (result)
+      {
+	Register eax = getRegisterByName("eax");
+	long syscall_num = eax.get(task);
+	result &= syscall_num == 0x77;
+      }
+    return result;
+  }
+
   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.3
diff -u -u -r1.3 IsaPowerPC.java
--- frysk-core/frysk/proc/IsaPowerPC.java	7 Feb 2007 18:08:25 -0000	1.3
+++ frysk-core/frysk/proc/IsaPowerPC.java	12 Feb 2007 15:05:34 -0000
@@ -133,6 +133,18 @@
     return false;
   }
 
+  /**
+   * Returns true if the given Task is at an instruction that will invoke
+   * the sig return system call.
+   *
+   * FIXME On powerpc this method is not yet implemented and always
+   * return false.
+   */
+  public boolean isAtSyscallSigReturn(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.8
diff -u -u -r1.8 IsaX8664.java
--- frysk-core/frysk/proc/IsaX8664.java	7 Feb 2007 18:08:25 -0000	1.8
+++ frysk-core/frysk/proc/IsaX8664.java	12 Feb 2007 15:05:34 -0000
@@ -314,6 +314,27 @@
     return false;
   }
 
+  /**
+   * Returns true if the given Task is at an instruction that will invoke
+   * the sig return system call.
+   *
+   * On x86_64 this is when the pc is at a 'syscall' instruction and the
+   * rax register contains 0x0f.
+   */
+  public boolean isAtSyscallSigReturn(Task task)
+  {
+    long address = pc(task);
+    boolean result = (task.getMemory().getByte(address) == (byte) 0x0f
+		      && task.getMemory().getByte(address + 1) == (byte) 0x05);
+    if (result)
+      {
+	Register rax = getRegisterByName("rax");
+	long syscall_num = rax.get(task);
+	result &= syscall_num == 0x0f;
+      }
+    return result;
+  }
+
   public Syscall[] getSyscallList ()
   {
     return LinuxX8664Syscall.syscallList;
Index: frysk-core/frysk/proc/LinuxPtraceTask.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/LinuxPtraceTask.java,v
retrieving revision 1.7
diff -u -u -r1.7 LinuxPtraceTask.java
--- frysk-core/frysk/proc/LinuxPtraceTask.java	7 Feb 2007 18:08:25 -0000	1.7
+++ frysk-core/frysk/proc/LinuxPtraceTask.java	12 Feb 2007 15:05:34 -0000
@@ -1,6 +1,6 @@
 // This file is part of the program FRYSK.
 //
-// Copyright 2005, 2006, Red Hat Inc.
+// Copyright 2005, 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
@@ -163,6 +163,7 @@
     logger.log(Level.FINE, "{0} sendStepInstruction\n", this);
     step_send = true;
     sig_send = sig;
+    syscall_sigret = getIsa().isAtSyscallSigReturn(this);
     try
       {
         Ptrace.singleStep(getTid(), sig);
Index: frysk-core/frysk/proc/LinuxPtraceTaskState.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/LinuxPtraceTaskState.java,v
retrieving revision 1.8
diff -u -u -r1.8 LinuxPtraceTaskState.java
--- frysk-core/frysk/proc/LinuxPtraceTaskState.java	8 Feb 2007 22:29:47 -0000	1.8
+++ frysk-core/frysk/proc/LinuxPtraceTaskState.java	12 Feb 2007 15:05:35 -0000
@@ -942,6 +942,7 @@
 		  // architectures).
 		  if (task.step_send
 		      && (task.sig_send != 0
+			  || task.syscall_sigret
 			  || isa.hasExecutedSpuriousTrap(task)))
 		    {
 		      sendContinue(task, 0);
@@ -1167,7 +1168,7 @@
 	    newState = insyscall ? inSyscallRunningTraced : syscallRunning;
 	  else
 	    newState = running;
-	  newState.sendContinue(task, 0);
+	  newState.sendContinue(task, sig);
 	  return newState;
 	}
 	
Index: frysk-core/frysk/proc/Task.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/Task.java,v
retrieving revision 1.121
diff -u -u -r1.121 Task.java
--- frysk-core/frysk/proc/Task.java	7 Feb 2007 18:08:25 -0000	1.121
+++ frysk-core/frysk/proc/Task.java	12 Feb 2007 15:05:35 -0000
@@ -907,6 +907,11 @@
   // The signal, or zero, send last to the task.
   int sig_send;
 
+  // When the last request to the process was a step request, whether
+  // it was a request to step a sigreturn syscall.
+  // Set by sendStepInstruction().
+  boolean syscall_sigret;
+
   /**
    * Notify all Code observers of the breakpoint. Return the number of
    * blocking observers or -1 if no Code observer were installed on this
Index: frysk-core/frysk/proc/TestTaskObserverInstructionSigReturn.java
===================================================================
RCS file: frysk-core/frysk/proc/TestTaskObserverInstructionSigReturn.java
diff -N frysk-core/frysk/proc/TestTaskObserverInstructionSigReturn.java
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ frysk-core/frysk/proc/TestTaskObserverInstructionSigReturn.java	12 Feb 2007 15:05:35 -0000
@@ -0,0 +1,150 @@
+// This file is part of the program FRYSK.
+//
+// Copyright 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
+// the Free Software Foundation; version 2 of the License.
+//
+// FRYSK is distributed in the hope that it will be useful, but
+// WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+// General Public License for more details.
+// 
+// You should have received a copy of the GNU General Public License
+// along with FRYSK; if not, write to the Free Software Foundation,
+// Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+// 
+// In addition, as a special exception, Red Hat, Inc. gives You the
+// additional right to link the code of FRYSK with code not covered
+// under the GNU General Public License ("Non-GPL Code") and to
+// distribute linked combinations including the two, subject to the
+// limitations in this paragraph. Non-GPL Code permitted under this
+// exception must only link to the code of FRYSK through those well
+// defined interfaces identified in the file named EXCEPTION found in
+// the source code files (the "Approved Interfaces"). The files of
+// Non-GPL Code may instantiate templates or use macros or inline
+// functions from the Approved Interfaces without causing the
+// resulting work to be covered by the GNU General Public
+// License. Only Red Hat, Inc. may make changes or additions to the
+// list of Approved Interfaces. You must obey the GNU General Public
+// License in all respects for all of the FRYSK code and other code
+// used in conjunction with FRYSK except the Non-GPL Code covered by
+// this exception. If you modify this file, you may extend this
+// exception to your version of the file, but you are not obligated to
+// do so. If you do not wish to provide this exception without
+// modification, you must delete this exception statement from your
+// version and license this file solely under the GPL without
+// exception.
+
+package frysk.proc;
+
+import java.io.File;
+
+import frysk.Config;
+import frysk.sys.Sig;
+
+public class TestTaskObserverInstructionSigReturn
+  extends TestLib
+  implements TaskObserver.Attached,
+  TaskObserver.Instruction,
+  TaskObserver.Terminating,
+  TaskObserver.Signaled
+{
+  // Counter for instruction observer hits.
+  long hit;
+
+  // How the process exited.
+  int exit;
+
+  // What process task are we talking about?
+  Task task;
+
+  // How many times have we been signaled?
+  int signaled;
+
+  public void testStepSigReturn()
+  {
+    // Init.
+    hit = 0;
+    signaled = 0;
+    exit = -1;
+
+    // Start and attach.
+    String command = new File(Config.getPkgLibDir(), "funit-alarm").getPath();
+    Manager.host.requestCreateAttachedProc(new String[] {command}, this);
+    assertRunUntilStop("Creating process");
+
+    // Add a terminated and signaled observer.
+    task.requestAddTerminatingObserver(this);
+    task.requestAddSignaledObserver(this);
+    task.requestUnblock(this);
+    assertRunUntilStop("Waiting for first PROF signal");
+
+    // Add a stepping observer.
+    task.requestAddInstructionObserver(this);
+    task.requestUnblock(this);
+    assertRunUntilStop("Stepping through till completion");
+
+    assertTrue("steps were made",
+	       hit > 5 * signaled /* random very low bound guess really */);
+    assertEquals("signaled", 3, signaled);
+    assertEquals("process exited nicely", 0, exit);
+  }
+
+  // Common interface methods
+  public void addedTo (Object observable)
+  {
+    // ignored
+  }
+
+  public void addFailed (Object observable, Throwable w)
+  {
+    w.printStackTrace();
+  }
+
+  public void deletedFrom (Object observable)
+  {
+    // ignored
+  }
+
+  // TaskObserver.Attached interface
+  public Action updateAttached(Task task)
+  {
+    this.task = task;
+    Manager.eventLoop.requestStop();
+    return Action.BLOCK;
+  }
+
+  // TaskObserver.Instruction interface
+  public Action updateExecuted(Task task)
+  {
+    hit++;
+    return Action.CONTINUE;
+  }
+
+  // TaskObserver.Terminated interface
+  public Action updateTerminating(Task task, boolean signal, int exit)
+  {
+    Manager.eventLoop.requestStop();
+
+    this.exit = exit;
+    return Action.CONTINUE;
+  }
+
+  // TaskObserver.Signaled interface
+  public Action updateSignaled (Task task, int signal)
+  {
+    if (signal != Sig.PROF_)
+      fail("Wrong signal received: " + signal);
+    
+    signaled++;
+    if (signaled == 1)
+      {
+	Manager.eventLoop.requestStop();
+	return Action.BLOCK;
+      }
+
+    return Action.CONTINUE;
+  }
+}

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

end of thread, other threads:[~2007-02-12 15:24 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-07 18:09 Mixing single-stepping and breakpointing Mark Wielaard
2007-02-12 15:24 ` 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).