[patch] Always allow instruction stepping through SteppingEngine.

[patch] Always allow instruction stepping through SteppingEngine.

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]

Hi,

This patch fixes bug #5907. The SteppingEngine didn't make a difference
between instruction stepping and line stepping and always tried to skip
plt entries and the dynamic loader. That is really annoying if you are
investigating issues with plt entries or the dynamic loader like I was
for the last couple of weeks :)

The InstructionStepState now keeps track of whether it is instruction or
line stepping. Only with line stepping does it try to do any magic. I
adjusted one test that assumed stepping would always end inside the plt
entry, this is now never true, except if you explicitly do step
--instruction of course.

frysk-core/frysk/stepping/ChangeLog
2008-03-19  Mark Wielaard  <mwielaard@redhat.com>

   * InstructionStepState.java (InstructionStepState): Take and set
   isLine argument.
   (handleUpdate): Only try skipping plt when line stepping.
   * LineStepState.java (handleUpdate): When no line info found act
   like an InstructionStepState.
   * SteppingEngine.java (stepInstruction): Create InstructionStepState
   for instruction stepping.
   (stepInstruction): Likewise.
   (stepLine): Create InstructionStepState for line stepping.
   (setUp): Create InstructionStepState using isLine.
   * TestStepping.java (testInstructionStepThroughSection): Just do one
   line step.

No regressions (fedora 8 - x86/x86_64).

Committed and pushed,

Mark

[-- Attachment #2: stepping-instruction.patch --]
[-- Type: text/x-patch, Size: 6320 bytes --]

diff --git a/frysk-core/frysk/stepping/InstructionStepState.java b/frysk-core/frysk/stepping/InstructionStepState.java
index 4baea23..6e20c2f 100644
--- a/frysk-core/frysk/stepping/InstructionStepState.java
+++ b/frysk-core/frysk/stepping/InstructionStepState.java
@@ -1,6 +1,6 @@
 // This file is part of the program FRYSK.
 //
-// Copyright 2007, Red Hat Inc.
+// Copyright 2007, 2008, 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
@@ -49,8 +49,15 @@ public class InstructionStepState extends State {
     private ElfSectionCache elfCache;
     private final String PLT_DL_FIXUP = "_dl_fixup";
 
-    public InstructionStepState(Task task) {
+    /**
+     * Indicates that this is a step (into) line, if true then plt
+     * entries are skipped.
+     */
+    private final boolean isLine;
+
+    public InstructionStepState(Task task, boolean isLine) {
 	this.task = task;
+	this.isLine = isLine;
     }
 
     /**
@@ -64,14 +71,20 @@ public class InstructionStepState extends State {
      */
     public State handleUpdate(TaskStepEngine tse) {
 
+      if (isLine) {
 	long addr = this.task.getPC();
 	this.elfCache = new ElfSectionCache(this.task);
 	ElfSectionHeader header = this.elfCache.getSectionHeader(".plt", addr);
 
-	/* If the user steps into a function call, the following catches the .plt section of the program, and
-	 * ensures that Frysk steps past it, so that the user is landed back into their own code after the call. */
-	if ((header != null && header.addr <= addr && (header.addr + header.offset) >= addr)
-		&& (header.type == ElfSectionHeader.ELF_SHT_PROGBITS || header.type == ElfSectionHeader.ELF_SHT_NOBITS)) {
+	/* If the user steps into a function call, the following
+	 * catches the .plt section of the program, and ensures that
+	 * Frysk steps past it, so that the user is landed back into
+	 * their own code after the call. */
+	if (header != null
+	    && header.addr <= addr
+	    && (header.addr + header.offset) >= addr
+	    && (header.type == ElfSectionHeader.ELF_SHT_PROGBITS
+		|| header.type == ElfSectionHeader.ELF_SHT_NOBITS)) {
 
 	    DwflLine line = tse.getDwflLine();
 
@@ -80,8 +93,8 @@ public class InstructionStepState extends State {
 		return new InstructionStepThroughState(task, PLT_DL_FIXUP);
 	    }
 	}
-
-	return new StoppedState(this.task);
+      }
+      return new StoppedState(this.task);
     }
 
     public boolean isStopped() {
diff --git a/frysk-core/frysk/stepping/LineStepState.java b/frysk-core/frysk/stepping/LineStepState.java
index 59d9539..f95a996 100644
--- a/frysk-core/frysk/stepping/LineStepState.java
+++ b/frysk-core/frysk/stepping/LineStepState.java
@@ -1,6 +1,6 @@
 // This file is part of the program FRYSK.
 //
-// Copyright 2007, Red Hat Inc.
+// Copyright 2007, 2008, 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
@@ -62,9 +62,13 @@ public class LineStepState extends State {
 	if (line == null) /* We're in no-debuginfo land */
 	{
 	    tse.setLine(0);
-	    /* Returned a StoppedState because line-stepping has no meaning
-	     * when there is no debug information to relate the 'lines' to. */
-	    return new StoppedState(this.task);
+	    // Pretend we are an InstructionStepState because
+	    // line-stepping has no meaning when there is no debug
+	    // information to relate the 'lines' to.  And
+	    // InstructionStepState with isLine set to true will
+	    // return StoppedState, but not before it went through a
+	    // plt section first.
+	    return new InstructionStepState(task, true).handleUpdate(tse);
 	} else
 	    lineNum = line.getLineNum();
 
diff --git a/frysk-core/frysk/stepping/SteppingEngine.java b/frysk-core/frysk/stepping/SteppingEngine.java
index 2b5d91d..6fa9b3e 100644
--- a/frysk-core/frysk/stepping/SteppingEngine.java
+++ b/frysk-core/frysk/stepping/SteppingEngine.java
@@ -198,7 +198,7 @@ public class SteppingEngine {
 	if (!tse.isStopped())
 	    return false;
 
-	tse.setState(new InstructionStepState(task));
+	tse.setState(new InstructionStepState(task, false));
 	this.steppingObserver.notifyNotBlocked(tse);
 	this.contextMap.put(task.getProc(), new Integer(1));
 
@@ -228,7 +228,7 @@ public class SteppingEngine {
 	while (iter.hasNext()) {
 	    t = (Task) iter.next();
 	    TaskStepEngine tse = (TaskStepEngine) this.taskStateMap.get(t);
-	    tse.setState(new InstructionStepState(t));
+	    tse.setState(new InstructionStepState(t, false));
 	    this.steppingObserver.notifyNotBlocked(tse);
 	    continueForStepping(t, true);
 	}
@@ -258,7 +258,7 @@ public class SteppingEngine {
 	    DwflLine line = tse.getDwflLine();
 
 	    if (line == null) {
-		tse.setState(new InstructionStepState(task));
+	        tse.setState(new InstructionStepState(task, true));
 		if (continueForStepping(task, true)) {
 		    this.steppingObserver.notifyNotBlocked(tse);
 		}
@@ -321,7 +321,7 @@ public class SteppingEngine {
 		 * from the Task at this point. If not, there's no point in doing
 		 * 'line stepping' since there are no 'lines' to step. */
 		if (line == null) {
-		    tse.setState(new InstructionStepState(t));
+		    tse.setState(new InstructionStepState(t, isLine));
 		} else {
 		    tse.setLine(line.getLineNum());
 		    tse.setState(new LineStepState(t));
diff --git a/frysk-core/frysk/stepping/TestStepping.java b/frysk-core/frysk/stepping/TestStepping.java
index fd0431f..b14c4f3 100644
--- a/frysk-core/frysk/stepping/TestStepping.java
+++ b/frysk-core/frysk/stepping/TestStepping.java
@@ -1017,7 +1017,6 @@ public class TestStepping extends TestLib {
 
 	    Task testTask = null;
 	    String callingFrame = "foo";
-	    boolean first = true;
 	    int endLine = 0;
 	    
 	    public InstructionStepThroughSectionTest(Task task, int lineNum) {
@@ -1032,12 +1031,6 @@ public class TestStepping extends TestLib {
 	
 		String s = frame.getSymbol().getDemangledName();
 		
-		if (first) {
-		    se.stepLine(this.testTask);
-		    this.first = false;
-		    return;
-		}
-		
 		assertEquals("calling frame", callingFrame, s);
 		assertEquals("line number", endLine, frame.getLine().getLine());
 		Manager.eventLoop.requestStop();