proc: bugs setting next breakpoints

1. A running goroutine is by definition not parked waiting for a
chan recv
2. The FDE end address is intended to be exclusive, the code
interpreted as inclusive and sometimes ended up setting a breakpoint
on a function other than the current one.
This commit is contained in:
aarzilli 2016-01-31 13:03:53 +01:00
parent 51e87386da
commit b370e20cd5
5 changed files with 88 additions and 7 deletions

15
_fixtures/issue332.go Normal file

@ -0,0 +1,15 @@
package main
import "fmt"
func main() {
m := make([]string, 1, 25)
fmt.Println(m) // [ ]
changeMe(m)
fmt.Println(m) // [Todd]
}
func changeMe(z []string) {
z[0] = "Todd"
fmt.Println(z) // [Todd]
}

@ -19,6 +19,8 @@ import (
protest "github.com/derekparker/delve/proc/test" protest "github.com/derekparker/delve/proc/test"
) )
const disableFailingTests = true
func init() { func init() {
runtime.GOMAXPROCS(4) runtime.GOMAXPROCS(4)
os.Setenv("GOMAXPROCS", "4") os.Setenv("GOMAXPROCS", "4")
@ -1475,3 +1477,65 @@ func TestIssue384(t *testing.T) {
assertNoError(err, t, "EvalVariable()") assertNoError(err, t, "EvalVariable()")
}) })
} }
func TestIssue332_Part1(t *testing.T) {
// Next shouldn't step inside a function call
withTestProcess("issue332", t, func(p *Process, fixture protest.Fixture) {
start, _, err := p.goSymTable.LineToPC(fixture.Source, 8)
assertNoError(err, t, "LineToPC()")
_, err = p.SetBreakpoint(start)
assertNoError(err, t, "SetBreakpoint()")
assertNoError(p.Continue(), t, "Continue()")
assertNoError(p.Next(), t, "first Next()")
locations, err := p.CurrentThread.Stacktrace(2)
assertNoError(err, t, "Stacktrace()")
if locations[0].Call.Fn == nil {
t.Fatalf("Not on a function")
}
if locations[0].Call.Fn.Name != "main.main" {
t.Fatalf("Not on main.main after Next: %s (%s:%d)", locations[0].Call.Fn.Name, locations[0].Call.File, locations[0].Call.Line)
}
if locations[0].Call.Line != 9 {
t.Fatalf("Not on line 9 after Next: %s (%s:%d)", locations[0].Call.Fn.Name, locations[0].Call.File, locations[0].Call.Line)
}
})
}
func TestIssue332_Part2(t *testing.T) {
// Step should skip a function's prologue
// 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()")
_, err = p.SetBreakpoint(start)
assertNoError(err, t, "SetBreakpoint()")
assertNoError(p.Continue(), t, "Continue()")
// step until we enter changeMe
for {
assertNoError(p.Step(), t, "Step()")
locations, err := p.CurrentThread.Stacktrace(2)
assertNoError(err, t, "Stacktrace()")
if locations[0].Call.Fn == nil {
t.Fatalf("Not on a function")
}
if locations[0].Call.Fn.Name == "main.changeMe" {
break
}
}
assertNoError(p.Next(), t, "first Next()")
assertNoError(p.Next(), t, "second Next()")
assertNoError(p.Next(), t, "third Next()")
err = p.Continue()
if _, exited := err.(ProcessExitedError); !exited {
assertNoError(err, t, "final Continue()")
}
})
}

@ -164,7 +164,7 @@ func (ge GoroutineExitingError) Error() string {
// Set breakpoints at every line, and the return address. Also look for // Set breakpoints at every line, and the return address. Also look for
// a deferred function and set a breakpoint there too. // a deferred function and set a breakpoint there too.
func (thread *Thread) next(curpc uint64, fde *frame.FrameDescriptionEntry, file string, line int) error { func (thread *Thread) next(curpc uint64, fde *frame.FrameDescriptionEntry, file string, line int) error {
pcs := thread.dbp.lineInfo.AllPCsBetween(fde.Begin(), fde.End(), file) pcs := thread.dbp.lineInfo.AllPCsBetween(fde.Begin(), fde.End()-1, file)
g, err := thread.GetG() g, err := thread.GetG()
if err != nil { if err != nil {

@ -311,7 +311,7 @@ func (scope *EvalScope) PtrSize() int {
// ChanRecvBlocked returns whether the goroutine is blocked on // ChanRecvBlocked returns whether the goroutine is blocked on
// a channel read operation. // a channel read operation.
func (g *G) ChanRecvBlocked() bool { func (g *G) ChanRecvBlocked() bool {
return g.WaitReason == chanRecv return (g.thread == nil) && (g.WaitReason == chanRecv)
} }
// chanRecvReturnAddr returns the address of the return from a channel read. // chanRecvReturnAddr returns the address of the return from a channel read.

@ -882,7 +882,7 @@ func printcontextThread(t *Term, th *api.Thread) {
fn := th.Function fn := th.Function
if th.Breakpoint == nil { if th.Breakpoint == nil {
fmt.Printf("> %s() %s:%d\n", fn.Name, ShortenFilePath(th.File), th.Line) fmt.Printf("> %s() %s:%d (PC: %#v)\n", fn.Name, ShortenFilePath(th.File), th.Line, th.PC)
return return
} }
@ -896,21 +896,23 @@ func printcontextThread(t *Term, th *api.Thread) {
} }
if hitCount, ok := th.Breakpoint.HitCount[strconv.Itoa(th.GoroutineID)]; ok { if hitCount, ok := th.Breakpoint.HitCount[strconv.Itoa(th.GoroutineID)]; ok {
fmt.Printf("> %s(%s) %s:%d (hits goroutine(%d):%d total:%d)\n", fmt.Printf("> %s(%s) %s:%d (hits goroutine(%d):%d total:%d) (PC: %#v)\n",
fn.Name, fn.Name,
args, args,
ShortenFilePath(th.File), ShortenFilePath(th.File),
th.Line, th.Line,
th.GoroutineID, th.GoroutineID,
hitCount, hitCount,
th.Breakpoint.TotalHitCount) th.Breakpoint.TotalHitCount,
th.PC)
} else { } else {
fmt.Printf("> %s(%s) %s:%d (hits total:%d)\n", fmt.Printf("> %s(%s) %s:%d (hits total:%d) (PC: %#v)\n",
fn.Name, fn.Name,
args, args,
ShortenFilePath(th.File), ShortenFilePath(th.File),
th.Line, th.Line,
th.Breakpoint.TotalHitCount) th.Breakpoint.TotalHitCount,
th.PC)
} }
if th.BreakpointInfo != nil { if th.BreakpointInfo != nil {