This is the mail archive of the frysk@sourceware.org mailing list for the frysk project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[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;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]