From 852f0c95b3837b58a2334770f09f0dc9e8b17856 Mon Sep 17 00:00:00 2001
From: aarzilli <alessandro.arzilli@gmail.com>
Date: Fri, 12 Feb 2016 08:43:22 +0100
Subject: [PATCH] proc: Step should skip function prologue

Step disassembles the current instruction, if it is a CALL sets a
temp breakpoint inside the called function, after the prologue and
calls Continue.

Fixes #332
---
 proc/disasm_amd64.go            |  2 ++
 proc/proc.go                    | 18 ++++++++++++++++++
 proc/proc_test.go               | 19 ++++++++++++-------
 proc/registers_windows_amd64.go |  2 +-
 4 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/proc/disasm_amd64.go b/proc/disasm_amd64.go
index 4586570b..3339f27f 100644
--- a/proc/disasm_amd64.go
+++ b/proc/disasm_amd64.go
@@ -6,6 +6,8 @@ import (
 	"rsc.io/x86/x86asm"
 )
 
+var maxInstructionLength uint64 = 15
+
 type ArchInst x86asm.Inst
 
 func asmDecode(mem []byte, pc uint64) (*ArchInst, error) {
diff --git a/proc/proc.go b/proc/proc.go
index 87b8a764..67349f24 100644
--- a/proc/proc.go
+++ b/proc/proc.go
@@ -452,6 +452,15 @@ func (dbp *Process) Step() (err error) {
 			return err
 		}
 		for {
+			pc, err := dbp.CurrentThread.PC()
+			if err != nil {
+				return err
+			}
+			text, err := dbp.CurrentThread.Disassemble(pc, pc+maxInstructionLength, true)
+			if err == nil && len(text) > 0 && text[0].IsCall() && text[0].DestLoc != nil && text[0].DestLoc.Fn != nil {
+				return dbp.StepInto(text[0].DestLoc.Fn)
+			}
+
 			err = dbp.CurrentThread.StepInstruction()
 			if err != nil {
 				return err
@@ -471,6 +480,15 @@ func (dbp *Process) Step() (err error) {
 	return dbp.run(fn)
 }
 
+// StepInto sets a temp breakpoint after the prologue of fn and calls Continue
+func (dbp *Process) StepInto(fn *gosym.Func) error {
+	pc, _ := dbp.FirstPCAfterPrologue(fn, false)
+	if _, err := dbp.SetTempBreakpoint(pc); err != nil {
+		return err
+	}
+	return dbp.Continue()
+}
+
 // StepInstruction will continue the current thread for exactly
 // one instruction. This method affects only the thread
 // asssociated with the selected goroutine. All other
diff --git a/proc/proc_test.go b/proc/proc_test.go
index ded60b30..615cd2c8 100644
--- a/proc/proc_test.go
+++ b/proc/proc_test.go
@@ -19,8 +19,6 @@ import (
 	protest "github.com/derekparker/delve/proc/test"
 )
 
-const disableFailingTests = true
-
 func init() {
 	runtime.GOMAXPROCS(4)
 	os.Setenv("GOMAXPROCS", "4")
@@ -1463,7 +1461,6 @@ func TestStepIntoFunction(t *testing.T) {
 		if !strings.Contains(loc.File, "teststep") {
 			t.Fatalf("debugger stopped at incorrect location: %s:%d", loc.File, loc.Line)
 		}
-		// TODO(derekparker) consider skipping function prologue when stepping into func.
 		if loc.Line != 8 {
 			t.Fatalf("debugger stopped at incorrect line: %d", loc.Line)
 		}
@@ -1511,10 +1508,6 @@ func TestIssue332_Part2(t *testing.T) {
 	// In some parts of the prologue, for some functions, the FDE data is incorrect
 	// which leads to 'next' and 'stack' failing with error "could not find FDE for PC: <garbage>"
 	// because the incorrect FDE data leads to reading the wrong stack address as the return address
-	// TODO: the incorrect FDE data problem is fixed in go 1.6, reenable this test when 1.6 is released.
-	if disableFailingTests {
-		return
-	}
 	withTestProcess("issue332", t, func(p *Process, fixture protest.Fixture) {
 		start, _, err := p.goSymTable.LineToPC(fixture.Source, 8)
 		assertNoError(err, t, "LineToPC()")
@@ -1535,6 +1528,18 @@ func TestIssue332_Part2(t *testing.T) {
 			}
 		}
 
+		pc, err := p.CurrentThread.PC()
+		assertNoError(err, t, "PC()")
+		pcAfterPrologue, err := p.FindFunctionLocation("main.changeMe", true, -1)
+		assertNoError(err, t, "FindFunctionLocation()")
+		pcEntry, err := p.FindFunctionLocation("main.changeMe", false, 0)
+		if pcAfterPrologue == pcEntry {
+			t.Fatalf("main.changeMe and main.changeMe:0 are the same (%x)", pcAfterPrologue)
+		}
+		if pc != pcAfterPrologue {
+			t.Fatalf("Step did not skip the prologue: current pc: %x, first instruction after prologue: %x", pc, pcAfterPrologue)
+		}
+
 		assertNoError(p.Next(), t, "first Next()")
 		assertNoError(p.Next(), t, "second Next()")
 		assertNoError(p.Next(), t, "third Next()")
diff --git a/proc/registers_windows_amd64.go b/proc/registers_windows_amd64.go
index edfb6967..282b795b 100644
--- a/proc/registers_windows_amd64.go
+++ b/proc/registers_windows_amd64.go
@@ -5,9 +5,9 @@ import "C"
 import (
 	"bytes"
 	"fmt"
+	"rsc.io/x86/x86asm"
 	"syscall"
 	"unsafe"
-	"rsc.io/x86/x86asm"
 )
 
 // Regs represents CPU registers on an AMD64 processor.