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

@@ -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()
+}

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 thread.CurrentBreakpoint != nil {
-		if err != nil {
+			if err := thread.Step(); err != nil {
-			return fmt.Errorf("could not continue thread %d %s", thread.Id, err)
+				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 (thread.CurrentBreakpoint != nil) || (dbp.halt) {
-	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++
 		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" {

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
+}

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
+}

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)
+			}
+		}
+	})
+}

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
+}