From a9e2696f466217ba8f96c018a0a77b839eae30d9 Mon Sep 17 00:00:00 2001 From: aarzilli Date: Fri, 2 Oct 2015 18:17:07 +0200 Subject: [PATCH] proc: bugfix: proc.(*Process).Continue skips breakpoints Breakpoints are skipped either because: 1. when multiple breakpoints are hit simultaneously only one is processed 2. a thread hits a breakpoint while another thread is being singlestepped over the breakpoint. Additionally fixed a race condition between Continue and tracee termination. --- _fixtures/bpcountstest.go | 28 +++++++++++++++++++++ proc/proc.go | 45 ++++++++++++++++++++------------- proc/proc_darwin.go | 4 +++ proc/proc_linux.go | 12 +++++++++ proc/proc_test.go | 52 +++++++++++++++++++++++++++++++++++++++ proc/threads.go | 19 ++++++++++++++ 6 files changed, 143 insertions(+), 17 deletions(-) create mode 100644 _fixtures/bpcountstest.go diff --git a/_fixtures/bpcountstest.go b/_fixtures/bpcountstest.go new file mode 100644 index 00000000..9ffbcb8d --- /dev/null +++ b/_fixtures/bpcountstest.go @@ -0,0 +1,28 @@ +package main + +import ( + "fmt" + "math/rand" + "sync" + "time" +) + +func demo(id int, wait *sync.WaitGroup) { + for i := 0; i < 100; i++ { + sleep := rand.Intn(10) + 1 + fmt.Printf("id: %d step: %d sleeping %d\n", id, i, sleep) + time.Sleep(time.Duration(sleep) * time.Millisecond) + } + + wait.Done() +} + +func main() { + wait := new(sync.WaitGroup) + wait.Add(1) + wait.Add(1) + go demo(1, wait) + go demo(2, wait) + + wait.Wait() +} diff --git a/proc/proc.go b/proc/proc.go index 8551044e..ea178084 100644 --- a/proc/proc.go +++ b/proc/proc.go @@ -364,10 +364,19 @@ func (dbp *Process) setChanRecvBreakpoints() (int, error) { // Resume process. func (dbp *Process) Continue() error { + // all threads stopped over a breakpoint are made to step over it for _, thread := range dbp.Threads { - err := thread.Continue() - if err != nil { - return fmt.Errorf("could not continue thread %d %s", thread.Id, err) + if thread.CurrentBreakpoint != nil { + if err := thread.Step(); err != nil { + return err + } + thread.CurrentBreakpoint = nil + } + } + // everything is resumed + for _, thread := range dbp.Threads { + if err := thread.resume(); err != nil { + return dbp.exitGuard(err) } } return dbp.run(func() error { @@ -376,9 +385,17 @@ func (dbp *Process) Continue() error { return err } if err := dbp.Halt(); err != nil { - return err + return dbp.exitGuard(err) } dbp.SwitchThread(thread.Id) + for _, th := range dbp.Threads { + if th.CurrentBreakpoint == nil { + err := th.SetCurrentBreakpoint() + if err != nil { + return err + } + } + } loc, err := thread.Location() if err != nil { return err @@ -642,24 +659,18 @@ func (dbp *Process) handleBreakpointOnThread(id int) (*Thread, error) { if !ok { return nil, fmt.Errorf("could not find thread for %d", id) } - pc, err := thread.PC() + // Check to see if we have hit a breakpoint. + err := thread.SetCurrentBreakpoint() if err != nil { return nil, err } - // Check to see if we have hit a breakpoint. - if bp, ok := dbp.FindBreakpoint(pc); ok { - thread.CurrentBreakpoint = bp - if err = thread.SetPC(bp.Addr); err != nil { - return nil, err - } - if g, err := thread.GetG(); err == nil { - thread.CurrentBreakpoint.HitCount[g.Id]++ - } - thread.CurrentBreakpoint.TotalHitCount++ + if (thread.CurrentBreakpoint != nil) || (dbp.halt) { return thread, nil } - if dbp.halt { - return thread, nil + + pc, err := thread.PC() + if err != nil { + return nil, err } fn := dbp.goSymTable.PCToFunc(pc) if fn != nil && fn.Name == "runtime.breakpoint" { diff --git a/proc/proc_darwin.go b/proc/proc_darwin.go index a947814f..21c2949f 100644 --- a/proc/proc_darwin.go +++ b/proc/proc_darwin.go @@ -327,3 +327,7 @@ func (dbp *Process) wait(pid, options int) (int, *sys.WaitStatus, error) { wpid, err := sys.Wait4(pid, &status, options, nil) return wpid, &status, err } + +func (dbp *Process) exitGuard(err error) error { + return err +} diff --git a/proc/proc_linux.go b/proc/proc_linux.go index b2f99825..d35fb0f7 100644 --- a/proc/proc_linux.go +++ b/proc/proc_linux.go @@ -377,3 +377,15 @@ func (dbp *Process) wait(pid, options int) (int, *sys.WaitStatus, error) { } } } + +func (dbp *Process) exitGuard(err error) error { + if err != sys.ESRCH { + return err + } + if status(dbp.Pid, dbp.os.comm) == STATUS_ZOMBIE { + _, err := dbp.trapWait(-1) + return err + } + + return err +} diff --git a/proc/proc_test.go b/proc/proc_test.go index e695d0ed..a6bcb0bf 100644 --- a/proc/proc_test.go +++ b/proc/proc_test.go @@ -94,6 +94,25 @@ func TestExit(t *testing.T) { }) } +func TestExitAfterContinue(t *testing.T) { + withTestProcess("continuetestprog", t, func(p *Process, fixture protest.Fixture) { + _, err := setFunctionBreakpoint(p, "main.sayhi") + assertNoError(err, t, "setFunctionBreakpoint()") + assertNoError(p.Continue(), t, "First Continue()") + err = p.Continue() + pe, ok := err.(ProcessExitedError) + if !ok { + t.Fatalf("Continue() returned unexpected error type %s", err) + } + if pe.Status != 0 { + t.Errorf("Unexpected error status: %d", pe.Status) + } + if pe.Pid != p.Pid { + t.Errorf("Unexpected process id: %d", pe.Pid) + } + }) +} + func setFunctionBreakpoint(p *Process, fname string) (*Breakpoint, error) { addr, err := p.FindFunctionLocation(fname, true, 0) if err != nil { @@ -1077,3 +1096,36 @@ func TestIssue325(t *testing.T) { t.Logf("iface2fn2: %v\n", iface2fn2v) }) } + +func TestBreakpointCounts(t *testing.T) { + withTestProcess("bpcountstest", t, func(p *Process, fixture protest.Fixture) { + addr, _, err := p.goSymTable.LineToPC(fixture.Source, 12) + assertNoError(err, t, "LineToPC") + bp, err := p.SetBreakpoint(addr) + assertNoError(err, t, "SetBreakpoint()") + + for { + if err := p.Continue(); err != nil { + if _, exited := err.(ProcessExitedError); exited { + break + } + assertNoError(err, t, "Continue()") + } + } + + t.Logf("TotalHitCount: %d", bp.TotalHitCount) + if bp.TotalHitCount != 200 { + t.Fatalf("Wrong TotalHitCount for the breakpoint (%d)", bp.TotalHitCount) + } + + if len(bp.HitCount) != 2 { + t.Fatalf("Wrong number of goroutines for breakpoint (%d)", len(bp.HitCount)) + } + + for _, v := range bp.HitCount { + if v != 100 { + t.Fatalf("Wrong HitCount for breakpoint (%v)", bp.HitCount) + } + } + }) +} diff --git a/proc/threads.go b/proc/threads.go index 0b590689..aa782b79 100644 --- a/proc/threads.go +++ b/proc/threads.go @@ -313,3 +313,22 @@ func (thread *Thread) Scope() (*EvalScope, error) { } return locations[0].Scope(thread), nil } + +func (thread *Thread) SetCurrentBreakpoint() error { + thread.CurrentBreakpoint = nil + pc, err := thread.PC() + if err != nil { + return err + } + if bp, ok := thread.dbp.FindBreakpoint(pc); ok { + thread.CurrentBreakpoint = bp + if err = thread.SetPC(bp.Addr); err != nil { + return err + } + if g, err := thread.GetG(); err == nil { + thread.CurrentBreakpoint.HitCount[g.Id]++ + } + thread.CurrentBreakpoint.TotalHitCount++ + } + return nil +} -- GitLab