This is the mail archive of the
frysk@sourceware.org
mailing list for the frysk project.
[patch] Bug #6029 - Set pc correctly before calling Code observers
- From: Mark Wielaard <mark at klomp dot org>
- To: frysk at sourceware dot org
- Date: Fri, 11 Apr 2008 19:29:45 +0200
- Subject: [patch] Bug #6029 - Set pc correctly before calling Code observers
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 <mwielaard@redhat.com>
* 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 <mwielaard@redhat.com>
* TestFrame.java (testBogusAddressPrevFrame): Resolved.
No regressions on x86 and x86_64 and the test now passes on both
architectures.
Committed and pushed,
Mark
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;