From c9cbaea291c9de515ab815012911b49359a0b1e6 Mon Sep 17 00:00:00 2001
From: Derek Parker <parkerderek86@gmail.com>
Date: Tue, 7 Oct 2014 12:25:33 -0500
Subject: [PATCH] Optimize Next implementation

Now that I'm using the step strategy, I put in an optimization where if
stepping into another function, simply find the return address, put a
breakpoint there, and then continue.
---
 dwarf/frame/frame_table.go   |  2 +-
 proctl/proctl_linux_amd64.go | 79 +++++++++++++++++++++++++-----------
 proctl/proctl_test.go        | 15 ++++---
 3 files changed, 64 insertions(+), 32 deletions(-)

diff --git a/dwarf/frame/frame_table.go b/dwarf/frame/frame_table.go
index 01b62ac3..231bc2b0 100644
--- a/dwarf/frame/frame_table.go
+++ b/dwarf/frame/frame_table.go
@@ -157,7 +157,7 @@ func (frame *FrameContext) ExecuteUntilPC(instructions []byte) {
 	// We only need to execute the instructions until
 	// ctx.loc > ctx.addess (which is the address we
 	// are currently at in the traced process).
-	for frame.loc <= frame.address && frame.buf.Len() > 0 {
+	for frame.address >= frame.loc && frame.buf.Len() > 0 {
 		executeDwarfInstruction(frame)
 	}
 	// make sure we get the update cfa offset
diff --git a/proctl/proctl_linux_amd64.go b/proctl/proctl_linux_amd64.go
index 9a080565..e27950a5 100644
--- a/proctl/proctl_linux_amd64.go
+++ b/proctl/proctl_linux_amd64.go
@@ -289,20 +289,22 @@ func (dbp *DebuggedProcess) Next() error {
 
 	loc := dbp.DebugLine.NextLocation(pc, l)
 	if !fde.AddressRange.Cover(loc.Address) {
-		addr := dbp.ReturnAddressFromOffset(fde.ReturnAddressOffset(pc))
-		bp, err := dbp.Break(uintptr(addr))
-		if err != nil {
-			if _, ok := err.(BreakPointExistsError); !ok {
-				return err
+		// Unconditionally step out of current function
+		// Don't bother looking up ret addr, next line is
+		// outside of current fn, should only be a few
+		// instructions left to RET
+		for fde.AddressRange.Cover(pc) {
+			err = dbp.Step()
+			if err != nil {
+				return fmt.Errorf("next stepping failed: ", err.Error())
 			}
-		}
 
-		err = dbp.Continue()
-		if err != nil {
-			return err
+			pc, err = dbp.CurrentPC()
+			if err != nil {
+				return err
+			}
 		}
-
-		return dbp.clearTempBreakpoint(bp.Addr)
+		return nil
 	}
 
 	for {
@@ -316,8 +318,10 @@ func (dbp *DebuggedProcess) Next() error {
 			return err
 		}
 
-		// TODO: if we have stepped into another function,
-		// find return address and continue there.
+		if !fde.AddressRange.Cover(pc) {
+			return dbp.continueToReturnAddress(pc, fde)
+		}
+
 		_, nl, nfn := dbp.GoSymTable.PCToLine(pc)
 		if nfn == fn && nl != l {
 			break
@@ -327,6 +331,35 @@ func (dbp *DebuggedProcess) Next() error {
 	return nil
 }
 
+func (dbp *DebuggedProcess) continueToReturnAddress(pc uint64, fde *frame.FrameDescriptionEntry) error {
+	for !fde.AddressRange.Cover(pc) {
+		addr := dbp.ReturnAddressFromOffset(fde.ReturnAddressOffset(pc))
+		bp, err := dbp.Break(uintptr(addr))
+		if err != nil {
+			if _, ok := err.(BreakPointExistsError); !ok {
+				return err
+			}
+		}
+
+		err = dbp.Continue()
+		if err != nil {
+			return err
+		}
+
+		err = dbp.clearTempBreakpoint(bp.Addr)
+		if err != nil {
+			return err
+		}
+
+		pc, err = dbp.CurrentPC()
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // Continue process until next breakpoint.
 func (dbp *DebuggedProcess) Continue() error {
 	// Stepping first will ensure we are able to continue
@@ -349,18 +382,18 @@ func (dbp *DebuggedProcess) CurrentPC() (uint64, error) {
 }
 
 func (dbp *DebuggedProcess) clearTempBreakpoint(pc uint64) error {
-	regs, err := dbp.Registers()
-	if err != nil {
-		return err
-	}
-
-	_, err = dbp.Clear(pc)
-	if err != nil {
-		return err
-	}
+	if bp, ok := dbp.PCtoBP(pc); ok {
+		regs, err := dbp.Registers()
+		if err != nil {
+			return err
+		}
 
-	if bp, ok := dbp.PCtoBP(regs.PC() - 1); ok {
 		// Reset program counter to our restored instruction.
+		bp, err = dbp.Clear(bp.Addr)
+		if err != nil {
+			return err
+		}
+
 		regs.SetPC(bp.Addr)
 		return syscall.PtraceSetRegs(dbp.Pid, regs)
 	}
diff --git a/proctl/proctl_test.go b/proctl/proctl_test.go
index 0d8df071..07954a0d 100644
--- a/proctl/proctl_test.go
+++ b/proctl/proctl_test.go
@@ -38,11 +38,11 @@ func currentPC(p *proctl.DebuggedProcess, t *testing.T) uint64 {
 	return pc
 }
 
-func currentLineNumber(p *proctl.DebuggedProcess, t *testing.T) int {
+func currentLineNumber(p *proctl.DebuggedProcess, t *testing.T) (string, int) {
 	pc := currentPC(p, t)
-	_, l, _ := p.GoSymTable.PCToLine(pc)
+	f, l, _ := p.GoSymTable.PCToLine(pc)
 
-	return l
+	return f, l
 }
 
 func TestAttachProcess(t *testing.T) {
@@ -158,7 +158,6 @@ func TestClearBreakPoint(t *testing.T) {
 
 func TestNext(t *testing.T) {
 	var (
-		ln             int
 		err            error
 		executablePath = "../_fixtures/testnextprog"
 	)
@@ -192,16 +191,16 @@ func TestNext(t *testing.T) {
 		assertNoError(p.Continue(), t, "Continue()")
 
 		for _, tc := range testcases {
-			ln = currentLineNumber(p, t)
+			f, ln := currentLineNumber(p, t)
 			if ln != tc.begin {
-				t.Fatalf("Program not stopped at correct spot expected %d was %d", tc.begin, ln)
+				t.Fatalf("Program not stopped at correct spot expected %d was %s:%d", tc.begin, f, ln)
 			}
 
 			assertNoError(p.Next(), t, "Next() returned an error")
 
-			ln = currentLineNumber(p, t)
+			f, ln = currentLineNumber(p, t)
 			if ln != tc.end {
-				t.Fatalf("Program did not continue to correct next location expected %d was %d", tc.end, ln)
+				t.Fatalf("Program did not continue to correct next location expected %d was %s:%d", tc.end, f, ln)
 			}
 		}
 
-- 
GitLab