From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28935 invoked by alias); 8 Apr 2008 14:56:19 -0000 Received: (qmail 28824 invoked by uid 22791); 8 Apr 2008 14:56:17 -0000 X-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL,BAYES_05,J_CHICKENPOX_64,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 08 Apr 2008 14:55:59 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id m38Etvpm008923 for ; Tue, 8 Apr 2008 10:55:57 -0400 Received: from pobox-2.corp.redhat.com (pobox-2.corp.redhat.com [10.11.255.15]) by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m38EtveU002070 for ; Tue, 8 Apr 2008 10:55:57 -0400 Received: from localhost.localdomain (vpn-14-114.rdu.redhat.com [10.11.14.114]) by pobox-2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m38Ettx9029204 for ; Tue, 8 Apr 2008 10:55:56 -0400 Message-ID: <47FB877A.4040003@redhat.com> Date: Wed, 09 Apr 2008 14:15:00 -0000 From: Phil Muldoon User-Agent: Thunderbird 2.0.0.12 (X11/20080226) MIME-Version: 1.0 To: frysk@sourceware.org Subject: Re: [SCM] master: Resequence Running and Stepping handleTrappedEvents to better flow with multiple events happening on one sigtrap. References: <20080408144626.23884.qmail@sourceware.org> In-Reply-To: <20080408144626.23884.qmail@sourceware.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.58 on 172.16.52.254 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/msg00038.txt.bz2 pmuldoon@sourceware.org wrote: Mark and I reworked the Stepping and Running handleTrappedEvent states to now properly accommodate watchpoints. This required rewriting the flow to accumulate blockers through stepping, code software breakpoints, hardware data watchpoints, and unhandled sigtraps. This is code is pretty rich in edge cases. So there is a case building to re-architect it. However doing it now, while blind the inevitable edge-cases watchpoints will continue to bring would be premature. This code primarily alters the program flow so that a code breakpoint, a hardware watchpoint, nor a step will consume a sigtrap and starve any other dependent events. It's passed on down to each interested event, and the number of blockers accumulated returned. If none of the events are interested in the sigtrap, the sigtrap just gets passed onto the task. Regards Phil > The branch, master has been updated > via cbb50856fe829fa9491ad30a89cd6ce846969693 (commit) > from 5f4e88713844da0810ba5d1eeaea572f7b5d9c2a (commit) > > Those revisions listed above that are new to this repository have > not appeared on any other notification email. > > - Log ----------------------------------------------------------------- > commit cbb50856fe829fa9491ad30a89cd6ce846969693 > Author: Phil Muldoon > Date: Tue Apr 8 15:45:04 2008 +0100 > > Resequence Running and Stepping handleTrappedEvents to better flow with multiple events happening on one sigtrap. > > 2008-04-08 Phil Muldoon > Mark Wielaard > > * LinuxPtraceTaskState.java (Running.checkWatchpoint): New. > (Running.handleTrappedEvent): Rewrite sequence to accumulate > blockers for all sigtrap events, then return. > (Stepping.handleTrappedEvent): Ditto. > > ----------------------------------------------------------------------- > > Summary of changes: > frysk-core/frysk/proc/live/ChangeLog | 8 + > .../frysk/proc/live/LinuxPtraceTaskState.java | 172 +++++++++++--------- > 2 files changed, 104 insertions(+), 76 deletions(-) > > First 500 lines of diff: > diff --git a/frysk-core/frysk/proc/live/ChangeLog b/frysk-core/frysk/proc/live/ChangeLog > index edbc1ad..99e9c3a 100644 > --- a/frysk-core/frysk/proc/live/ChangeLog > +++ b/frysk-core/frysk/proc/live/ChangeLog > @@ -1,3 +1,11 @@ > +2008-04-08 Phil Muldoon > + Mark Wielaard > + > + * LinuxPtraceTaskState.java (Running.checkWatchpoint): New. > + (Running.handleTrappedEvent): Rewrite sequence to accumulate > + blockers for all sigtrap events, then return. > + (Stepping.handleTrappedEvent): Ditto. > + > 2008-04-07 Stan Cox > > * LinuxPtraceHost.java (requestCreateAttachedProc): Add libs parameter. > diff --git a/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java b/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java > index 8f35740..3e590c4 100644 > --- a/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java > +++ b/frysk-core/frysk/proc/live/LinuxPtraceTaskState.java > @@ -929,8 +929,32 @@ abstract class LinuxPtraceTaskState extends State { > return blockedContinue(); > return sendContinue(task, Signal.NONE); > } > + > > /** > + * Returns the number of blockers if a watchpoint was hit, or -1 when no watchpoint > + * was hit for this Task. > + */ > + int checkWatchpoint(LinuxPtraceTask task) { > + int blockers = -1; > + // First test if this is a watchpoint event. > + WatchpointFunctions watchpointFunction = WatchpointFunctionFactory. > + getWatchpointFunctions(task.getISA()); > + for (int i=0; i + // Test if a watchpoint has fired > + if (watchpointFunction.hasWatchpointTriggered(task, i)) { > + if (blockers == -1) > + blockers = 0; > + frysk.isa.watchpoints.Watchpoint trigger = watchpointFunction.readWatchpoint(task, i); > + blockers += task.notifyWatchpoint(trigger.getAddress(), trigger.getRange()); > + watchpointFunction.resetWatchpoint(task, i); > + } > + } > + > + return blockers; > + } > + > + /** > * Handles traps caused by breakpoints or instruction > * stepping. If there are any Code observers at the address of > * the trap they get notified. If none of the Code observers > @@ -943,24 +967,7 @@ abstract class LinuxPtraceTaskState extends State { > */ > LinuxPtraceTaskState handleTrappedEvent(LinuxPtraceTask task) { > fine.log("handleTrappedEvent", task); > - > - // First test if this is a watchpoint event. > - WatchpointFunctions watchpointFunction = WatchpointFunctionFactory.getWatchpointFunctions(task.getISA()); > - for (int i=0; i - // Test if a watchpoint has fired > - if (watchpointFunction.hasWatchpointTriggered(task, i)) { > - frysk.isa.watchpoints.Watchpoint trigger = watchpointFunction.readWatchpoint(task, i); > - watchpointFunction.resetWatchpoint(task, i); > - int blockers = task.notifyWatchpoint(trigger.getAddress(), trigger.getRange()); > - if (blockers == 0) > - return sendContinue(task, Signal.NONE); > - else > - return blockedContinue; > - } > - } > > - Isa isa; > - isa = task.getIsaFIXME(); > // 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 > @@ -969,33 +976,49 @@ abstract class LinuxPtraceTaskState extends State { > // first step onto the first instruction of a just started > // task sometimes doesn't set the right task stepped flag. > // So we check and immediately clear here. > + Isa isa; > + isa = task.getIsaFIXME(); > if (isa != null && (isa.isTaskStepped(task) || task.justStartedXXX)) { > if (task.justStartedXXX) { > return stepping.handleTrappedEvent(task); > } else { > - System.err.println("Whoa! Wrong state for stepping: " > - + this); > - return sendContinue(task, Signal.NONE); > - } > - } else { > - // Do we have a breakpoint installed here? > - long address = isa.getBreakpointAddress(task); > - int blockers = task.notifyCodeBreakpoint(address); > - if (blockers >= 0) { > - // Prepare for stepping the breakpoint > - setupSteppingBreakpoint(task, address); > - > - if (blockers == 0) > - return sendContinue(task, Signal.NONE); > - else > - return blockedContinue(); > - } else { > - // This is not a trap event generated by us. And we > - // also didn't send a step request. Deliver the > - // real Trap event to the Task. > - return handleSignaledEvent(task, Signal.TRAP); > + throw new RuntimeException("Whoa! Wrong state for stepping: " > + + this); > } > } > + > + // Check if a watchpoint triggered. > + int blockers = checkWatchpoint(task); > + boolean isTrapHandled; > + if (blockers != -1) > + isTrapHandled = true; > + else { > + isTrapHandled = false; > + blockers = 0; > + } > + > + // Do we have a breakpoint installed here? > + long address = isa.getBreakpointAddress(task); > + int stepBlockers = task.notifyCodeBreakpoint(address); > + if (stepBlockers >= 0) { > + // Prepare for stepping the breakpoint > + setupSteppingBreakpoint(task, address); > + blockers += stepBlockers; > + isTrapHandled = true; > + } > + > + if (!isTrapHandled) > + // This is not a trap event generated by us. And we > + // also didn't send a step request. Deliver the > + // real Trap event to the Task. > + return handleSignaledEvent(task, Signal.TRAP); > + > + // Otherwise return a blocked or continue depending > + // on whether something blocked. > + if (blockers == 0) > + return sendContinue(task, Signal.NONE); > + else > + return blockedContinue; > } > > LinuxPtraceTaskState handleAddObservation(LinuxPtraceTask task, TaskObservation observation) { > @@ -1080,25 +1103,19 @@ abstract class LinuxPtraceTaskState extends State { > fine.log("handleTrappedEvent", task); > > // First test if this is a watchpoint event. > - WatchpointFunctions watchpointFunction = WatchpointFunctionFactory.getWatchpointFunctions(task.getISA()); > - for (int i=0; i - // Test if a watchpoint has fired > - if (watchpointFunction.hasWatchpointTriggered(task, i)) { > - frysk.isa.watchpoints.Watchpoint trigger = watchpointFunction.readWatchpoint(task, i); > - watchpointFunction.resetWatchpoint(task, i); > - int blockers = task.notifyWatchpoint(trigger.getAddress(), trigger.getRange()); > - if (blockers == 0) > - return sendContinue(task, Signal.NONE); > - else > - return blockedContinue; > - } > + int blockers = checkWatchpoint(task); > + boolean isTrapHandled; > + if (blockers != -1) > + isTrapHandled = true; > + else { > + isTrapHandled = false; > + blockers = 0; > } > - > - > + > Isa isa; > isa = task.getIsaFIXME(); > > - // First see if this was just an indication the we stepped. > + // Second, 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 > @@ -1118,15 +1135,13 @@ abstract class LinuxPtraceTaskState extends State { > task.steppingBreakpoint = null; > } > > - if (task.notifyInstruction() > 0) > - return blockedContinue(); > - else > - return sendContinue(task, Signal.NONE); > + blockers += task.notifyInstruction(); > + isTrapHandled = true; > } else { > // Do we have a breakpoint installed here? > long address = isa.getBreakpointAddress(task); > - int blockers = task.notifyCodeBreakpoint(address); > - if (blockers >= 0) { > + int breakpointBlockers = task.notifyCodeBreakpoint(address); > + if (breakpointBlockers >= 0) { > // Sanity check > if (task.steppingBreakpoint != null) > throw new RuntimeException("Already breakpoint stepping: " > @@ -1135,11 +1150,8 @@ abstract class LinuxPtraceTaskState extends State { > // Prepare for stepping the breakpoint > setupSteppingBreakpoint(task, address); > > - if (blockers == 0) > - return sendContinue(task, Signal.NONE); > - else > - return blockedContinue(); > - > + blockers += breakpointBlockers; > + isTrapHandled = true; > } else { > // This is not a trap event generated by us. > > @@ -1158,20 +1170,28 @@ abstract class LinuxPtraceTaskState extends State { > // made so we should notify any instruction observers. > if ((task.sigSendXXX != Signal.NONE > || task.syscallSigretXXX > - || isa.hasExecutedSpuriousTrap(task))) > - if (task.notifyInstruction() > 0) > - return blockedContinue(); > - else > - return sendContinue(task, Signal.NONE); > - > - // Deliver the real Trap event to the Task. This is > - // somewhat weird, we are either stepping a trapping > - // instruction (breakpoint) that we didn't install, or > - // something is sending this process an explicit trap > - // signal. > - return handleSignaledEvent(task, Signal.TRAP); > + || isa.hasExecutedSpuriousTrap(task))) { > + blockers += task.notifyInstruction(); > + isTrapHandled = true; > + } > } > } > + > + if (!isTrapHandled) > + // Deliver the real Trap event to the Task. This is > + // somewhat weird, we are either stepping a trapping > + // instruction (breakpoint) that we didn't install, or > + // something is sending this process an explicit trap > + // signal. > + return handleSignaledEvent(task, Signal.TRAP); > + > + // Otherwise return a blocked or continue depending > + // on whether something blocked. > + if (blockers == 0) > + return sendContinue(task, Signal.NONE); > + else > + return blockedContinue; > + > } > > private void checkBreakpointStepping(LinuxPtraceTask task) { > > > hooks/post-receive > -- > frysk system monitor/debugger >