From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4504 invoked by alias); 7 Feb 2007 18:09:08 -0000 Received: (qmail 4492 invoked by uid 22791); 7 Feb 2007 18:09:04 -0000 X-Spam-Status: No, hits=-1.4 required=5.0 tests=AWL,BAYES_05,FORGED_RCVD_HELO,TW_GJ X-Spam-Check-By: sourceware.org Received: from wildebeest.demon.nl (HELO gnu.wildebeest.org) (83.160.170.119) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 07 Feb 2007 18:08:53 +0000 Received: from hermans.wildebeest.org ([192.168.1.28]) by gnu.wildebeest.org with esmtp (Exim 3.36 #1 (Debian)) id 1HErE1-00039v-00 for ; Wed, 07 Feb 2007 19:08:57 +0100 Subject: Mixing single-stepping and breakpointing From: Mark Wielaard To: frysk@sourceware.org Content-Type: multipart/mixed; boundary="=-XdBck3rvHSsEIPydzAAE" Date: Wed, 07 Feb 2007 18:09:00 -0000 Message-Id: <1170871725.3142.30.camel@hermans.wildebeest.org> Mime-Version: 1.0 X-Mailer: Evolution 2.8.2.1 (2.8.2.1-3.fc6) X-Virus-Checked: Checked by ClamAV on sourceware.org X-IsSubscribed: yes Mailing-List: contact frysk-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: frysk-owner@sourceware.org X-SW-Source: 2007-q1/txt/msg00093.txt.bz2 --=-XdBck3rvHSsEIPydzAAE Content-Type: text/plain Content-Transfer-Encoding: 7bit Content-length: 3258 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 * 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 * 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 --=-XdBck3rvHSsEIPydzAAE Content-Disposition: inline; filename=breakpointstepping.patch Content-Type: text/x-patch; name=breakpointstepping.patch; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-length: 20031 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. + *

+ * 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; --=-XdBck3rvHSsEIPydzAAE--