From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16339 invoked by alias); 11 Apr 2008 17:30:15 -0000 Received: (qmail 16330 invoked by uid 22791); 11 Apr 2008 17:30:13 -0000 X-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00 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; Fri, 11 Apr 2008 17:29:53 +0000 Received: from cc1341701-a.deven1.ov.home.nl ([82.72.26.122] helo=[192.168.1.109]) by gnu.wildebeest.org with esmtp (Exim 4.63) (envelope-from ) id 1JkN4O-0005yc-Nt for frysk@sourceware.org; Fri, 11 Apr 2008 19:29:50 +0200 Subject: [patch] Bug #6029 - Set pc correctly before calling Code observers From: Mark Wielaard To: frysk@sourceware.org Content-Type: multipart/mixed; boundary="=-9oNck9il0zMZI/O2cFWq" Date: Fri, 11 Apr 2008 21:33:00 -0000 Message-Id: <1207934985.3449.43.camel@localhost.localdomain> Mime-Version: 1.0 X-Mailer: Evolution 2.12.3 (2.12.3-3.fc8) X-Spam-Score: -0.7 (/) 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: 2008-q2/txt/msg00046.txt.bz2 --=-9oNck9il0zMZI/O2cFWq Content-Type: text/plain Content-Transfer-Encoding: 7bit Content-length: 1272 Hi, Petr found a bug when generating a backtrace from inside the Code observer updateHit() method. This didn't show up in other places since we would set the pc up correctly immediately after we detected a breakpoint, but after calling the Code observers. It would also only show if you were unlucky enough to set a breakpoint on an address right before the dwarf cfi info would mark a cfa change. Of course Petr's test hit both cases. Fixed as follows: frysk-core/frysk/proc/live/ChangeLog 2008-04-11 Mark Wielaard * LinuxPtraceTaskState.java (Running.setupSteppingBreakpoint): Removed. (Running.handleTrappedEvent): Don't call setupSteppingBreakpoint(). (Stepping.handleTrappedEvent): Don't do stepping breakpoint sanity check. Don't call setupSteppingBreakpoint(). * LinuxPtraceTask.java (notifyCodeBreakpoint): Add stepping breakpoint sanity check. Set task pc when breakpoint found. Set steppingBreakpoint. frysk-core/frysk/stack/ChangeLog 2008-04-11 Mark Wielaard * TestFrame.java (testBogusAddressPrevFrame): Resolved. No regressions on x86 and x86_64 and the test now passes on both architectures. Committed and pushed, Mark --=-9oNck9il0zMZI/O2cFWq Content-Disposition: inline; filename=patch Content-Type: text/x-patch; name=patch; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-length: 4793 diff --git a/frysk-core/frysk/proc/live/LinuxPtraceTask.java b/frysk-core/frysk/proc/live/LinuxPtraceTask.java index 8f651f3..a0f75c7 100644 --- a/frysk-core/frysk/proc/live/LinuxPtraceTask.java +++ b/frysk-core/frysk/proc/live/LinuxPtraceTask.java @@ -937,9 +937,33 @@ public class LinuxPtraceTask extends LiveTask { */ int notifyCodeBreakpoint(long address) { fine.log(this, "notifyCodeBreakpoint address", address); - Collection observers = ((LinuxPtraceProc)getProc()).breakpoints.getCodeObservers(address); + LinuxPtraceProc proc = (LinuxPtraceProc) getProc(); + Collection observers = proc.breakpoints.getCodeObservers(address); if (observers == null) return -1; + + // Sanity check + if (steppingBreakpoint != null) + throw new RuntimeException("Already breakpoint stepping: " + + steppingBreakpoint); + + // Reset pc, some architectures might leave the pc right after + // the breakpoint, but since we haven't actually executed the + // real instruction yet we want it to be at the actual address + // of the original instruction. + setPC(address); + + // All logic for determining how and where to step the + // Breakpoint is determined by Proc and + // Breakpoint.prepareStep() (called in sendContinue). + Breakpoint bp = Breakpoint.create(address,proc); + + // TODO: This should really move us to a new TaskState. + // Currently we rely on the Task.steppingBreakpoint + // being set and the Breakpoint/Instruction having all + // the state necessary. + steppingBreakpoint = bp; + Iterator i = observers.iterator(); while (i.hasNext()) { TaskObserver.Code observer = (TaskObserver.Code) i.next(); diff --git a/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java b/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java index 9a71530..50ebabd 100644 --- a/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java +++ b/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java @@ -725,26 +725,6 @@ abstract class LinuxPtraceTaskState extends State { this.insyscall = insyscall; } - void setupSteppingBreakpoint(LinuxPtraceTask task, long address) { - // Reset pc, this should maybe be moved into the Breakpoint, - // but if the breakpoint gets removed before we step it, and - // the architecture puts the pc just behind the breakpoint - // address, then there is no good other place to get at the - // original pc location. - task.setPC(address); - - // All logic for determining how and where to step the - // Breakpoint is determined by Proc and - // Breakpoint.prepareStep() (called in sendContinue). - Breakpoint bp = Breakpoint.create(address,((LinuxPtraceProc)task.getProc())); - - // TODO: This should really move us to a new TaskState. - // Currently we rely on the Task.steppingBreakpoint - // being set and the Breakpoint/Instruction having all - // the state necessary. - task.steppingBreakpoint = bp; - } - /** * Tells the LinuxPtraceTask to continue, keeping in kind pending * breakpoints, with or without syscall tracing. @@ -1006,7 +986,6 @@ abstract class LinuxPtraceTaskState extends State { int stepBlockers = task.notifyCodeBreakpoint(address); if (stepBlockers >= 0) { // Prepare for stepping the breakpoint - setupSteppingBreakpoint(task, address); blockers += stepBlockers; isTrapHandled = true; } @@ -1146,14 +1125,6 @@ abstract class LinuxPtraceTaskState extends State { long address = isa.getBreakpointAddress(task); int breakpointBlockers = task.notifyCodeBreakpoint(address); if (breakpointBlockers >= 0) { - // Sanity check - if (task.steppingBreakpoint != null) - throw new RuntimeException("Already breakpoint stepping: " - + task.steppingBreakpoint); - - // Prepare for stepping the breakpoint - setupSteppingBreakpoint(task, address); - blockers += breakpointBlockers; isTrapHandled = true; } else { diff --git a/frysk-core/frysk/stack/TestFrame.java b/frysk-core/frysk/stack/TestFrame.java index 327e1a6..c647efa 100644 --- a/frysk-core/frysk/stack/TestFrame.java +++ b/frysk-core/frysk/stack/TestFrame.java @@ -42,7 +42,6 @@ package frysk.stack; import java.util.Iterator; import java.util.List; -//import frysk.proc.Proc; import frysk.config.Config; import frysk.proc.Action; import frysk.proc.Manager; @@ -174,8 +173,6 @@ public class TestFrame extends TestLib { } public void testBogusAddressPrevFrame() throws ElfException { - if (unresolved(6029)) - return; class CodeObserver1 implements TaskObserver.Code { public boolean hit = false; --=-9oNck9il0zMZI/O2cFWq--