Go 1.8 compatibility (part 2) (#667)

* dwarf/line: bugfix: not all values of the state machine can be used

According to DWARF Version 3 Section 6.2 "Line Number Information" not
all the values transversed by the line numbers state machine are valid
instructions, only the ones after a "special opcode", after the
standard opcode DW_LNS_copy and the extended opcode
DW_LINE_end_sequence.

DWARF3 describes this by specifying that only the opcodes listed above
"append a row to the matrix".

Additionally the implementation of DW_LNS_const_add_pc was wrong.

Fixes #664

* dwarf/line: fixed test failing with go1.8

* service/test: fix prologue detection tests

The conditions about which function prologue is emitted by the compiler
changed in go1.8, changed the test program so that callme2 will still
have a prologue under go1.8.

* service/test: fix step test

compilation units are linked in a different order under go1.8 so the
code of 'fmt' is no longer located after 'main' in the executable,
changed the tests so that they don't rely on this assumption anymore.

* proc: change runtime.Breakpoint support for go1.8

Before 1.8 it was sufficient to step twice to exit a
runtime.Breakpoint(), but go 1.8 added frame pointer tracking to small
functions making runtime.Breakpoint longer.
This changes runtime.Breakpoint handling in Continue to single step as
many times as are needed to exit runtime.Breakpoint.

* proc/test: fix TestIssue561 for go1.8

_fixtures/callme.go

@@ -13,6 +13,7 @@ func callme2() {
 	for i := 0; i < nBytes; i++ {
 		zeroarr[i] = '0'
 	}
+	fmt.Println("callme2")
 }
 
 func callme3() {

_fixtures/issue664.go

@@ -0,0 +1,14 @@
+package main
+
+func asdfasdf() {
+	for i := 0; i < 5; i++ {
+		for i := 0; i < 5; i++ {
+			//...
+		}
+		//...
+	}
+}
+
+func main() {
+	asdfasdf()
+}

dwarf/line/line_parser_test.go

@@ -38,6 +38,13 @@ func grabDebugLineSection(p string, t *testing.T) []byte {
 	return data
 }
 
+const (
+	lineBaseGo14  int8  = -1
+	lineBaseGo18  int8  = -4
+	lineRangeGo14 uint8 = 4
+	lineRangeGo18 uint8 = 10
+)
+
 func TestDebugLinePrologueParser(t *testing.T) {
 	// Test against known good values, from readelf --debug-dump=rawline _fixtures/testnextprog
 	p, err := filepath.Abs("../../_fixtures/testnextprog")
@@ -67,11 +74,15 @@ func TestDebugLinePrologueParser(t *testing.T) {
 		t.Fatal("Initial value of 'is_stmt' not parsed correctly", prologue.InitialIsStmt)
 	}
 
-	if prologue.LineBase != int8(-1) {
+	if prologue.LineBase != lineBaseGo14 && prologue.LineBase != lineBaseGo18 {
+		// go < 1.8 uses -1
+		// go >= 1.8 uses -4
 		t.Fatal("Line base not parsed correctly", prologue.LineBase)
 	}
 
-	if prologue.LineRange != uint8(4) {
+	if prologue.LineRange != lineRangeGo14 && prologue.LineRange != lineRangeGo18 {
+		// go < 1.8 uses 4
+		// go >= 1.8 uses 10
 		t.Fatal("Line Range not parsed correctly", prologue.LineRange)
 	}
 
@@ -90,8 +101,15 @@ func TestDebugLinePrologueParser(t *testing.T) {
 		t.Fatal("Include dirs not parsed correctly")
 	}
 
-	if !strings.Contains(dbl.FileNames[0].Name, "/delve/_fixtures/testnextprog.go") {
+	ok := false
-		t.Fatal("First file entry not parsed correctly")
+	for _, n := range dbl.FileNames {
+		if strings.Contains(n.Name, "/delve/_fixtures/testnextprog.go") {
+			ok = true
+			break
+		}
+	}
+	if !ok {
+		t.Fatal("File names table not parsed correctly")
 	}
 }
 

dwarf/line/state_machine.go

@@ -27,6 +27,11 @@ type StateMachine struct {
 	endSeq          bool
 	lastWasStandard bool
 	lastDelta       int
+	// valid is true if the current value of the state machine is the address of
+	// an instruction (using the terminology used by DWARF spec the current
+	// value of the state machine should be appended to the matrix representing
+	// the compilation unit)
+	valid bool
 }
 
 type opcodefn func(*StateMachine, *bytes.Buffer)
@@ -91,7 +96,9 @@ func (dbl *DebugLines) AllPCsForFileLine(f string, l int) (pcs []uint64) {
 		}
 		if sm.line == l && sm.file == f && sm.address != lastAddr {
 			foundFile = true
+			if sm.valid {
 				pcs = append(pcs, sm.address)
+			}
 			line := sm.line
 			// Keep going until we're on a different line. We only care about
 			// when a line comes back around (i.e. for loop) so get to next line,
@@ -123,6 +130,9 @@ func (dbl *DebugLines) AllPCsBetween(begin, end uint64, filename string) ([]uint
 
 	for b, err := buf.ReadByte(); err == nil; b, err = buf.ReadByte() {
 		findAndExecOpcode(sm, buf, b)
+		if !sm.valid {
+			continue
+		}
 		if sm.address > end {
 			break
 		}
@@ -157,9 +167,10 @@ func execSpecialOpcode(sm *StateMachine, instr byte) {
 
 	sm.lastDelta = int(sm.dbl.Prologue.LineBase + int8(decoded%sm.dbl.Prologue.LineRange))
 	sm.line += sm.lastDelta
-	sm.address += uint64(decoded / sm.dbl.Prologue.LineRange)
+	sm.address += uint64(decoded/sm.dbl.Prologue.LineRange) * uint64(sm.dbl.Prologue.MinInstrLength)
 	sm.basicBlock = false
 	sm.lastWasStandard = false
+	sm.valid = true
 }
 
 func execExtendedOpcode(sm *StateMachine, instr byte, buf *bytes.Buffer) {
@@ -170,6 +181,7 @@ func execExtendedOpcode(sm *StateMachine, instr byte, buf *bytes.Buffer) {
 		panic(fmt.Sprintf("Encountered unknown extended opcode %#v\n", b))
 	}
 	sm.lastWasStandard = false
+	sm.valid = false
 
 	fn(sm, buf)
 }
@@ -180,12 +192,14 @@ func execStandardOpcode(sm *StateMachine, instr byte, buf *bytes.Buffer) {
 		panic(fmt.Sprintf("Encountered unknown standard opcode %#v\n", instr))
 	}
 	sm.lastWasStandard = true
+	sm.valid = false
 
 	fn(sm, buf)
 }
 
 func copyfn(sm *StateMachine, buf *bytes.Buffer) {
 	sm.basicBlock = false
+	sm.valid = true
 }
 
 func advancepc(sm *StateMachine, buf *bytes.Buffer) {
@@ -218,7 +232,7 @@ func setbasicblock(sm *StateMachine, buf *bytes.Buffer) {
 }
 
 func constaddpc(sm *StateMachine, buf *bytes.Buffer) {
-	sm.address += (255 / uint64(sm.dbl.Prologue.LineRange))
+	sm.address += uint64((255-sm.dbl.Prologue.OpcodeBase)/sm.dbl.Prologue.LineRange) * uint64(sm.dbl.Prologue.MinInstrLength)
 }
 
 func fixedadvancepc(sm *StateMachine, buf *bytes.Buffer) {
@@ -230,6 +244,7 @@ func fixedadvancepc(sm *StateMachine, buf *bytes.Buffer) {
 
 func endsequence(sm *StateMachine, buf *bytes.Buffer) {
 	sm.endSeq = true
+	sm.valid = true
 }
 
 func setaddress(sm *StateMachine, buf *bytes.Buffer) {

proc/go_version.go

@@ -16,6 +16,10 @@ type GoVersion struct {
 	RC    int
 }
 
+var (
+	GoVer18Beta = GoVersion{1, 8, -1, 0, 0}
+)
+
 func ParseVersionString(ver string) (GoVersion, bool) {
 	var r GoVersion
 	var err1, err2, err3 error

proc/proc.go

@@ -359,10 +359,18 @@ func (dbp *Process) Continue() error {
 		case dbp.CurrentThread.CurrentBreakpoint == nil:
 			// runtime.Breakpoint or manual stop
 			if dbp.CurrentThread.onRuntimeBreakpoint() {
-				for i := 0; i < 2; i++ {
+				// Single-step current thread until we exit runtime.breakpoint and
+				// runtime.Breakpoint.
+				// On go < 1.8 it was sufficient to single-step twice on go1.8 a change
+				// to the compiler requires 4 steps.
+				for {
 					if err = dbp.CurrentThread.StepInstruction(); err != nil {
 						return err
 					}
+					loc, err := dbp.CurrentThread.Location()
+					if err != nil || loc.Fn == nil || (loc.Fn.Name != "runtime.breakpoint" && loc.Fn.Name != "runtime.Breakpoint") {
+						break
+					}
 				}
 			}
 			return dbp.conditionErrors()

proc/proc_test.go

@@ -2051,8 +2051,7 @@ func TestIssue561(t *testing.T) {
 	// Step fails to make progress when PC is at a CALL instruction
 	// where a breakpoint is also set.
 	withTestProcess("issue561", t, func(p *Process, fixture protest.Fixture) {
-		_, err := setFunctionBreakpoint(p, "main.main")
+		setFileBreakpoint(p, t, fixture, 10)
-		assertNoError(err, t, "setFunctionBreakpoint()")
 		assertNoError(p.Continue(), t, "Continue()")
 		assertNoError(p.Step(), t, "Step()")
 		_, ln := currentLineNumber(p, t)
@@ -2364,18 +2363,25 @@ func TestIssue683(t *testing.T) {
 		_, err := setFunctionBreakpoint(p, "main.main")
 		assertNoError(err, t, "setFunctionBreakpoint()")
 		assertNoError(p.Continue(), t, "First Continue()")
-		goterror := false
 		for i := 0; i < 20; i++ {
 			// eventually an error about the source file not being found will be
 			// returned, the important thing is that we shouldn't panic
 			err := p.Step()
 			if err != nil {
-				goterror = true
 				break
 			}
 		}
-		if !goterror {
+	})
-			t.Fatal("expeceted an error we didn't get")
+}
+
+func TestIssue664(t *testing.T) {
+	withTestProcess("issue664", t, func(p *Process, fixture protest.Fixture) {
+		setFileBreakpoint(p, t, fixture, 4)
+		assertNoError(p.Continue(), t, "Continue()")
+		assertNoError(p.Next(), t, "Next()")
+		f, ln := currentLineNumber(p, t)
+		if ln != 5 {
+			t.Fatalf("Did not continue to line 5: %s:%d", f, ln)
 		}
 	})
 }

service/test/integration1_test.go

@@ -169,7 +169,7 @@ func Test1ClientServer_exit(t *testing.T) {
 
 func Test1ClientServer_step(t *testing.T) {
 	withTestClient1("testprog", t, func(c *rpc1.RPCClient) {
-		_, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.helloworld", Line: 1})
+		_, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.helloworld", Line: -1})
 		if err != nil {
 			t.Fatalf("Unexpected error: %v", err)
 		}
@@ -1010,8 +1010,16 @@ func Test1SkipPrologue2(t *testing.T) {
 
 		callme3 := findLocationHelper(t, c, "main.callme3", false, 1, 0)[0]
 		callme3Z := findLocationHelper(t, c, "main.callme3:0", false, 1, 0)[0]
-		// callme3 does not have local variables therefore the first line of the function is immediately after the prologue
+		ver, _ := proc.ParseVersionString(runtime.Version())
-		findLocationHelper(t, c, "callme.go:18", false, 1, callme3Z)
+		if ver.Major < 0 || ver.AfterOrEqual(proc.GoVer18Beta) {
+			findLocationHelper(t, c, "callme.go:19", false, 1, callme3)
+		} else {
+			// callme3 does not have local variables therefore the first line of the
+			// function is immediately after the prologue
+			// This is only true before 1.8 where frame pointer chaining introduced a
+			// bit of prologue even for functions without local variables
+			findLocationHelper(t, c, "callme.go:19", false, 1, callme3Z)
+		}
 		if callme3 == callme3Z {
 			t.Fatal("Skip prologue failed")
 		}

service/test/integration2_test.go

@@ -178,7 +178,7 @@ func TestClientServer_exit(t *testing.T) {
 
 func TestClientServer_step(t *testing.T) {
 	withTestClient2("testprog", t, func(c service.Client) {
-		_, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.helloworld", Line: 1})
+		_, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.helloworld", Line: -1})
 		if err != nil {
 			t.Fatalf("Unexpected error: %v", err)
 		}
@@ -1048,8 +1048,16 @@ func TestSkipPrologue2(t *testing.T) {
 
 		callme3 := findLocationHelper(t, c, "main.callme3", false, 1, 0)[0]
 		callme3Z := findLocationHelper(t, c, "main.callme3:0", false, 1, 0)[0]
-		// callme3 does not have local variables therefore the first line of the function is immediately after the prologue
+		ver, _ := proc.ParseVersionString(runtime.Version())
-		findLocationHelper(t, c, "callme.go:18", false, 1, callme3Z)
+		if ver.Major < 0 || ver.AfterOrEqual(proc.GoVer18Beta) {
+			findLocationHelper(t, c, "callme.go:19", false, 1, callme3)
+		} else {
+			// callme3 does not have local variables therefore the first line of the
+			// function is immediately after the prologue
+			// This is only true before 1.8 where frame pointer chaining introduced a
+			// bit of prologue even for functions without local variables
+			findLocationHelper(t, c, "callme.go:19", false, 1, callme3Z)
+		}
 		if callme3 == callme3Z {
 			t.Fatal("Skip prologue failed")
 		}