diff --git a/_fixtures/callme.go b/_fixtures/callme.go index 4a40a627..9f7c58e9 100644 --- a/_fixtures/callme.go +++ b/_fixtures/callme.go @@ -13,6 +13,7 @@ func callme2() { for i := 0; i < nBytes; i++ { zeroarr[i] = '0' } + fmt.Println("callme2") } func callme3() { diff --git a/_fixtures/issue664.go b/_fixtures/issue664.go new file mode 100644 index 00000000..86006022 --- /dev/null +++ b/_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() +} diff --git a/dwarf/line/line_parser_test.go b/dwarf/line/line_parser_test.go index e367e888..b1989f19 100644 --- a/dwarf/line/line_parser_test.go +++ b/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") { - t.Fatal("First file entry not parsed correctly") + ok := false + 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") } } diff --git a/dwarf/line/state_machine.go b/dwarf/line/state_machine.go index 1e2051a6..25a2ff6b 100644 --- a/dwarf/line/state_machine.go +++ b/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 - pcs = append(pcs, sm.address) + 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) { diff --git a/proc/go_version.go b/proc/go_version.go index 21afab8d..87d5dec9 100644 --- a/proc/go_version.go +++ b/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 diff --git a/proc/proc.go b/proc/proc.go index c059adea..a58896dc 100644 --- a/proc/proc.go +++ b/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() diff --git a/proc/proc_test.go b/proc/proc_test.go index ec418edf..4a49e769 100644 --- a/proc/proc_test.go +++ b/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") - assertNoError(err, t, "setFunctionBreakpoint()") + setFileBreakpoint(p, t, fixture, 10) 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) } }) } diff --git a/service/test/integration1_test.go b/service/test/integration1_test.go index e4b66893..6c821d1e 100644 --- a/service/test/integration1_test.go +++ b/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 - findLocationHelper(t, c, "callme.go:18", false, 1, callme3Z) + ver, _ := proc.ParseVersionString(runtime.Version()) + 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") } diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go index 737076d1..885333d4 100644 --- a/service/test/integration2_test.go +++ b/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 - findLocationHelper(t, c, "callme.go:18", false, 1, callme3Z) + ver, _ := proc.ParseVersionString(runtime.Version()) + 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") }