proc/native: refactor Halt/setCurrentBreakpoint call pair

pkg/proc/native/proc.go

@@ -246,10 +246,7 @@ func (dbp *Process) ContinueOnce() (proc.Thread, error) {
 	if err != nil {
 		return nil, err
 	}
-	if err := dbp.Halt(); err != nil {
-		return nil, dbp.exitGuard(err)
-	}
-	if err := dbp.setCurrentBreakpoints(trapthread); err != nil {
+	if err := dbp.stop(trapthread); err != nil {
 		return nil, err
 	}
 	return trapthread, err
@@ -324,19 +321,6 @@ func (dbp *Process) SwitchGoroutine(gid int) error {
 	return nil
 }
 
-// Halt stops all threads.
-func (dbp *Process) Halt() (err error) {
-	if dbp.exited {
-		return &proc.ProcessExitedError{Pid: dbp.Pid()}
-	}
-	for _, th := range dbp.threads {
-		if err := th.Halt(); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 // FindBreakpoint finds the breakpoint for the given pc.
 func (dbp *Process) FindBreakpoint(pc uint64) (*proc.Breakpoint, bool) {
 	// Check to see if address is past the breakpoint, (i.e. breakpoint was hit).

pkg/proc/native/proc_darwin.go

@@ -111,12 +111,7 @@ func Launch(cmd []string, wd string) (*Process, error) {
 	if err != nil {
 		return nil, err
 	}
-	if err := dbp.Halt(); err != nil {
-		return nil, dbp.exitGuard(err)
-	}
-
-	_, err = dbp.waitForStop()
-	if err != nil {
+	if err := dbp.stop(nil); err != nil {
 		return nil, err
 	}
 
@@ -384,23 +379,6 @@ func (dbp *Process) waitForStop() ([]int, error) {
 	}
 }
 
-func (dbp *Process) setCurrentBreakpoints(trapthread *Thread) error {
-	ports, err := dbp.waitForStop()
-	if err != nil {
-		return err
-	}
-	trapthread.SetCurrentBreakpoint()
-	for _, port := range ports {
-		if th, ok := dbp.threads[port]; ok {
-			err := th.SetCurrentBreakpoint()
-			if err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}
-
 func (dbp *Process) loadProcessInformation(wg *sync.WaitGroup) {
 	wg.Done()
 }
@@ -446,6 +424,39 @@ func (dbp *Process) resume() error {
 	return nil
 }
 
+// stop stops all running threads threads and sets breakpoints
+func (dbp *Process) stop(trapthread *Thread) (err error) {
+	if dbp.exited {
+		return &proc.ProcessExitedError{Pid: dbp.Pid()}
+	}
+	for _, th := range dbp.threads {
+		if !th.Stopped() {
+			if err := th.halt(); err != nil {
+				return dbp.exitGuard(err)
+			}
+		}
+		th.running = false
+	}
+
+	ports, err := dbp.waitForStop()
+	if err != nil {
+		return err
+	}
+	if !dbp.os.initialized {
+		return nil
+	}
+	trapthread.SetCurrentBreakpoint()
+	for _, port := range ports {
+		if th, ok := dbp.threads[port]; ok {
+			err := th.SetCurrentBreakpoint()
+			if err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
 func (dbp *Process) detach(kill bool) error {
 	return PtraceDetach(dbp.pid, 0)
 }

pkg/proc/native/proc_linux.go

@@ -377,35 +377,6 @@ func (dbp *Process) wait(pid, options int) (int, *sys.WaitStatus, error) {
 	}
 }
 
-func (dbp *Process) setCurrentBreakpoints(trapthread *Thread) error {
-	// wait for all threads to stop
-	for {
-		allstopped := true
-		for _, th := range dbp.threads {
-			if th.running {
-				allstopped = false
-				break
-			}
-		}
-		if allstopped {
-			break
-		}
-		dbp.halt = true
-		_, err := dbp.trapWait(-1)
-		if err != nil {
-			return err
-		}
-	}
-	for _, th := range dbp.threads {
-		if th.CurrentBreakpoint.Breakpoint == nil {
-			if err := th.SetCurrentBreakpoint(); err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}
-
 func (dbp *Process) exitGuard(err error) error {
 	if err != sys.ESRCH {
 		return err
@@ -437,6 +408,49 @@ func (dbp *Process) resume() error {
 	return nil
 }
 
+// stop stops all running threads threads and sets breakpoints
+func (dbp *Process) stop(trapthread *Thread) (err error) {
+	if dbp.exited {
+		return &proc.ProcessExitedError{Pid: dbp.Pid()}
+	}
+	for _, th := range dbp.threads {
+		if !th.Stopped() {
+			if err := th.halt(); err != nil {
+				return dbp.exitGuard(err)
+			}
+		}
+	}
+
+	// wait for all threads to stop
+	for {
+		allstopped := true
+		for _, th := range dbp.threads {
+			if th.running {
+				allstopped = false
+				break
+			}
+		}
+		if allstopped {
+			break
+		}
+		dbp.halt = true
+		_, err := dbp.trapWait(-1)
+		if err != nil {
+			return err
+		}
+	}
+
+	// set breakpoints on all threads
+	for _, th := range dbp.threads {
+		if th.CurrentBreakpoint.Breakpoint == nil {
+			if err := th.SetCurrentBreakpoint(); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
 func (dbp *Process) detach(kill bool) error {
 	for threadID := range dbp.threads {
 		err := PtraceDetach(threadID, 0)

pkg/proc/native/proc_windows.go

@@ -2,7 +2,6 @@ package native
 
 import (
 	"debug/pe"
-	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -175,9 +174,6 @@ func (dbp *Process) kill() error {
 	if dbp.exited {
 		return nil
 	}
-	if !dbp.threads[dbp.pid].Stopped() {
-		return errors.New("process must be stopped in order to kill it")
-	}
 
 	p, err := os.FindProcess(dbp.pid)
 	if err != nil {
@@ -393,7 +389,40 @@ func (dbp *Process) wait(pid, options int) (int, *sys.WaitStatus, error) {
 	return 0, nil, fmt.Errorf("not implemented: wait")
 }
 
-func (dbp *Process) setCurrentBreakpoints(trapthread *Thread) error {
+func (dbp *Process) exitGuard(err error) error {
+	return err
+}
+
+func (dbp *Process) resume() error {
+	for _, thread := range dbp.threads {
+		if thread.CurrentBreakpoint.Breakpoint != nil {
+			if err := thread.StepInstruction(); err != nil {
+				return err
+			}
+			thread.CurrentBreakpoint.Clear()
+		}
+	}
+
+	for _, thread := range dbp.threads {
+		thread.running = true
+		_, err := _ResumeThread(thread.os.hThread)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// stop stops all running threads threads and sets breakpoints
+func (dbp *Process) stop(trapthread *Thread) (err error) {
+	if dbp.exited {
+		return &proc.ProcessExitedError{Pid: dbp.Pid()}
+	}
+	for _, th := range dbp.threads {
+		th.running = false
+	}
+
 	// While the debug event that stopped the target was being propagated
 	// other target threads could generate other debug events.
 	// After this function we need to know about all the threads
@@ -404,7 +433,7 @@ func (dbp *Process) setCurrentBreakpoints(trapthread *Thread) error {
 	// call to _ContinueDebugEvent will resume execution of some of the
 	// target threads.
 
-	err := trapthread.SetCurrentBreakpoint()
+	err = trapthread.SetCurrentBreakpoint()
 	if err != nil {
 		return err
 	}
@@ -441,31 +470,6 @@ func (dbp *Process) setCurrentBreakpoints(trapthread *Thread) error {
 	return nil
 }
 
-func (dbp *Process) exitGuard(err error) error {
-	return err
-}
-
-func (dbp *Process) resume() error {
-	for _, thread := range dbp.threads {
-		if thread.CurrentBreakpoint.Breakpoint != nil {
-			if err := thread.StepInstruction(); err != nil {
-				return err
-			}
-			thread.CurrentBreakpoint.Clear()
-		}
-	}
-
-	for _, thread := range dbp.threads {
-		thread.running = true
-		_, err := _ResumeThread(thread.os.hThread)
-		if err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
 func (dbp *Process) detach(kill bool) error {
 	if !kill {
 		for _, thread := range dbp.threads {

pkg/proc/native/threads.go

@@ -113,13 +113,6 @@ func (thread *Thread) SetPC(pc uint64) error {
 	return regs.SetPC(thread, pc)
 }
 
-// Stopped returns whether the thread is stopped at
-// the operating system level. Actual implementation
-// is OS dependant, look in OS thread file.
-func (thread *Thread) Stopped() bool {
-	return thread.stopped()
-}
-
 // SetCurrentBreakpoint sets the current breakpoint that this
 // thread is stopped at as CurrentBreakpoint on the thread struct.
 func (thread *Thread) SetCurrentBreakpoint() error {

pkg/proc/native/threads_darwin.go

@@ -27,20 +27,6 @@ type OSSpecificDetails struct {
 // be continued.
 var ErrContinueThread = fmt.Errorf("could not continue thread")
 
-// Halt stops this thread from executing.
-func (thread *Thread) Halt() (err error) {
-	defer func() {
-		if err == nil {
-			thread.running = false
-		}
-	}()
-	if thread.Stopped() {
-		return
-	}
-	err = thread.halt()
-	return
-}
-
 func (t *Thread) halt() (err error) {
 	kret := C.thread_suspend(t.os.threadAct)
 	if kret != C.KERN_SUCCESS {
@@ -116,7 +102,9 @@ func (t *Thread) Blocked() bool {
 	}
 }
 
-func (t *Thread) stopped() bool {
+// Stopped returns whether the thread is stopped at
+// the operating system level.
+func (t *Thread) Stopped() bool {
 	return C.thread_blocked(t.os.threadAct) > C.int(0)
 }
 

pkg/proc/native/threads_linux.go

@@ -16,15 +16,6 @@ type OSSpecificDetails struct {
 	registers sys.PtraceRegs
 }
 
-// Halt stops this thread from executing.
-func (thread *Thread) Halt() (err error) {
-	if thread.Stopped() {
-		return
-	}
-	err = thread.halt()
-	return
-}
-
 func (t *Thread) halt() (err error) {
 	err = sys.Tgkill(t.dbp.pid, t.ID, sys.SIGSTOP)
 	if err != nil {
@@ -34,7 +25,9 @@ func (t *Thread) halt() (err error) {
 	return
 }
 
-func (t *Thread) stopped() bool {
+// Stopped returns whether the thread is stopped at
+// the operating system level.
+func (t *Thread) Stopped() bool {
 	state := status(t.ID, t.dbp.os.comm)
 	return state == StatusTraceStop || state == StatusTraceStopT
 }

pkg/proc/native/threads_windows.go

@@ -18,30 +18,6 @@ type OSSpecificDetails struct {
 	hThread syscall.Handle
 }
 
-// Halt stops this thread from executing.
-func (thread *Thread) Halt() (err error) {
-	defer func() {
-		if err == nil {
-			thread.running = false
-		}
-	}()
-	if thread.Stopped() {
-		return
-	}
-	err = thread.halt()
-	return
-}
-
-func (t *Thread) halt() (err error) {
-	// Ignore the request to halt. On Windows, all threads are halted
-	// on return from WaitForDebugEvent.
-	return nil
-
-	// TODO - This may not be correct in all usages of dbp.Halt.  There
-	// are some callers who use dbp.Halt() to stop the process when it is not
-	// already broken on a debug event.
-}
-
 func (t *Thread) singleStep() error {
 	context := newCONTEXT()
 	context.ContextFlags = _CONTEXT_ALL
@@ -140,9 +116,9 @@ func (t *Thread) Blocked() bool {
 	}
 }
 
-func (t *Thread) stopped() bool {
-	// TODO: We are assuming that threads are always stopped
-	// during command execution.
+// Stopped returns whether the thread is stopped at the operating system
+// level. On windows this always returns true.
+func (t *Thread) Stopped() bool {
 	return true
 }