Re: [SCM] master: Use libunwind cursor to get at CFA.

Re: [SCM] master: Use libunwind cursor to get at CFA.

[Andrew Cagney]

mark@sourceware.org wrote:
>     frysk-core/frysk/stack/ChangeLog
>     2007-12-03  Mark Wielaard  <mwielaard@redhat.com>
>     
>            * LibunwindFrame.java (getFrameIdentifier): Get CFA from cursor.
>   

This isn't right.

The confusion comes from what libunwind considers to be the cursor's 
CFA.  It returns what you're more likely to recognize as the  
inner-to-cursor's CFA and not the cursor''s CFA.  Effectively the 
current frame's SP, in fact:

    case UNW_X86_64_CFA:
    case UNW_X86_64_RSP:
      if (write)
        return -UNW_EREADONLYREG;
      *valp = c->dwarf.cfa;
      return 0;

Andrew

Re: [SCM] master: Use libunwind cursor to get at CFA.

[Mark Wielaard]

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

Hi Andrew,

On Mon, 2007-12-03 at 11:19 -0500, Andrew Cagney wrote:
> The confusion comes from what libunwind considers to be the cursor's 
> CFA.  It returns what you're more likely to recognize as the  
> inner-to-cursor's CFA and not the cursor''s CFA.  Effectively the 
> current frame's SP, in fact:
>
>     case UNW_X86_64_CFA:
>     case UNW_X86_64_RSP:
>       if (write)
>         return -UNW_EREADONLYREG;
>       *valp = c->dwarf.cfa;
>       return 0;

Nice catch. Well that is somewhat of a bummer. Then we must fall back on
the unwind one frame trick and get the SP again.

What is somewhat worrying is that none of the tests caught this. So I
added one that does fail with my change, and passes with that commit
reverted.

frysk-core/frysk/pkglibdir/ChangeLog
2007-12-03  Mark Wielaard  <mwielaard@redhat.com>

    * funit-stepping-asm.S: Add fifth function and _stepOverPrologue_
    marker.

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

    * TestStepping.java (testASMFunctionStepOverPrologue): New test.

Cheers,

Mark

[-- Attachment #2: Type: text/x-patch, Size: 2508 bytes --]

diff --git a/frysk-core/frysk/pkglibdir/funit-stepping-asm.S b/frysk-core/frysk/pkglibdir/funit-stepping-asm.S
index 4af1506..cde2b22 100644
--- a/frysk-core/frysk/pkglibdir/funit-stepping-asm.S
+++ b/frysk-core/frysk/pkglibdir/funit-stepping-asm.S
@@ -40,10 +40,18 @@
 
 #include "frysk-asm.h"
 
+	FUNCTION_BEGIN(fifth,1)
+	FUNCTION_PROLOGUE(fifth,1) ; NO_OP	// _stepOverPrologue_
+	FUNCTION_EPILOGUE(fifth,1)
+	FUNCTION_RETURN(fifth,1)
+	FUNCTION_END(fifth,1)
+	
+	
 	FUNCTION_BEGIN(fourth,0)
 	FUNCTION_PROLOGUE(fourth,0)
 	NO_OP					// _stepAdvanceStart_
 	NO_OP
+	FUNCTION_CALL(fifth)
 	FUNCTION_EPILOGUE(fourth,0)
 	FUNCTION_RETURN(fourth,0)
 	FUNCTION_END(fourth,0)
diff --git a/frysk-core/frysk/stepping/TestStepping.java b/frysk-core/frysk/stepping/TestStepping.java
index 7f14ea1..cab4480 100644
--- a/frysk-core/frysk/stepping/TestStepping.java
+++ b/frysk-core/frysk/stepping/TestStepping.java
@@ -952,6 +952,52 @@ public class TestStepping extends TestLib {
 	cleanup();
     }
 
+  /**
+   * Tests that the line stepper steps OK even when stack pointer
+   * changes.
+   */                                                                          
+    public void testASMFunctionStepOverPrologue() {
+
+	/** Variable setup */
+	String source = Config.getRootSrcDir()
+	    + "frysk-core/frysk/pkglibdir/funit-stepping-asm.S";
+
+	this.scanner = new TestfileTokenScanner(new File(source));
+
+	/* The line number where the test begins, prologue of fifth
+	   function. */
+	int startLine = this.scanner.findTokenLine("_stepOverPrologue_");
+
+	/* The line number the test should end up at, still the same
+	   line (only prologue is stepped over, not the NO_OP) */
+	int endLine = this.scanner.findTokenLine("_stepOverPrologue_");
+
+	/* The test process */
+	dbae = new DaemonBlockedAtEntry(Config
+					.getPkgLibFile("funit-stepping-asm"));
+
+	Task theTask = dbae.getMainTask();
+
+	this.testStarted = false;
+
+	initTaskWithTask(theTask, source, startLine, endLine);
+
+	this.currentTest = new AssertLine(endLine, theTask);
+
+	DebugInfoFrame frame = DebugInfoStackFactory
+	    .createDebugInfoStackTrace(theTask);
+	assertTrue("Line information present", frame.getLines().length > 0);
+
+	/** The stepping operation */
+	this.se.stepOver(theTask, frame);
+
+	this.testStarted = true;
+	/** Run to completion */
+	assertRunUntilStop("Running test");
+	cleanup();
+    }
+
+
     boolean genericUpdate = false;
 
     public Task initTask(Offspring process, String source, int startLine,

Re: [SCM] master: Use libunwind cursor to get at CFA.

[Andrew Cagney]

Mark Wielaard wrote:
> Nice catch. Well that is somewhat of a bummer. Then we must fall back on
> the unwind one frame trick and get the SP again.
>   
That isn't as bad as it seems; computing the cfa requires running the 
unwind-program anyway.

I think having a method (perhaps called .getInnerCFA() though?) to 
differentiate between the sp and cfa is definitly a good idea.  Also, if 
possible, can the start code address of the CFI block be made 
available?  There's the theoretical possibility of further tuning the 
stepping code to use that to construct a quick-and-dirty 
frame-identifier when single stepping (letting us delay the request for 
the code's function).

(Your elimination of the uses of libunwind's lookup is wicked)
> What is somewhat worrying is that none of the tests caught this. So I
> added one that does fail with my change, and passes with that commit
> reverted.
>
>   
Yes.  Scary.  There's always one that gets away :-)  Nice simple 
portable test.

Andrew