Refactor: Use thread-locked goroutine for ptrace ops

Previously either the terminal client or the debugger service would
either lock main goroutine to a thread or provide a locked goroutine to
run _all_ DebuggedProcess functions in. This is unnecessary because only
ptrace functions need to be run from the same thread that originated the
PT_ATTACH request.

Here we use a specific thread-locked goroutine to service any ptrace
request. That goroutine is also responsible for the initial spawning /
attaching of the process, since it must be responsible for the PT_ATTACH
request.

proc/breakpoints.go

@@ -30,7 +30,7 @@ func (bp *Breakpoint) String() string {
 // hardware or software breakpoint.
 func (bp *Breakpoint) Clear(thread *Thread) (*Breakpoint, error) {
 	if bp.hardware {
-		if err := clearHardwareBreakpoint(bp.reg, thread.Id); err != nil {
+		if err := thread.dbp.clearHardwareBreakpoint(bp.reg, thread.Id); err != nil {
 			return nil, err
 		}
 		return bp, nil
@@ -121,7 +121,7 @@ func (dbp *DebuggedProcess) setBreakpoint(tid int, addr uint64, temp bool) (*Bre
 		}
 		if v == nil {
 			for t, _ := range dbp.Threads {
-				if err := setHardwareBreakpoint(i, t, addr); err != nil {
+				if err := dbp.setHardwareBreakpoint(i, t, addr); err != nil {
 					return nil, fmt.Errorf("could not set hardware breakpoint on thread %d: %s", t, err)
 				}
 			}

proc/breakpoints_darwin_amd64.go

@@ -3,11 +3,11 @@ package proc
 import "fmt"
 
 // TODO(darwin)
-func setHardwareBreakpoint(reg, tid int, addr uint64) error {
+func (dbp *DebuggedProcess) setHardwareBreakpoint(reg, tid int, addr uint64) error {
 	return fmt.Errorf("not implemented on darwin")
 }
 
 // TODO(darwin)
-func clearHardwareBreakpoint(reg, tid int) error {
+func (dbp *DebuggedProcess) clearHardwareBreakpoint(reg, tid int) error {
 	return fmt.Errorf("not implemented on darwin")
 }

proc/breakpoints_linux_amd64.go

@@ -21,7 +21,7 @@ import "fmt"
 // debug register `reg` with the address of the instruction
 // that we want to break at. There are only 4 debug registers
 // DR0-DR3. Debug register 7 is the control register.
-func setHardwareBreakpoint(reg, tid int, addr uint64) error {
+func (dbp *DebuggedProcess) setHardwareBreakpoint(reg, tid int, addr uint64) error {
 	if reg < 0 || reg > 3 {
 		return fmt.Errorf("invalid debug register value")
 	}
@@ -32,10 +32,12 @@ func setHardwareBreakpoint(reg, tid int, addr uint64) error {
 		drxmask   = uintptr((((1 << C.DR_CONTROL_SIZE) - 1) << uintptr(reg*C.DR_CONTROL_SIZE)) | (((1 << C.DR_ENABLE_SIZE) - 1) << uintptr(reg*C.DR_ENABLE_SIZE)))
 		drxenable = uintptr(0x1) << uintptr(reg*C.DR_ENABLE_SIZE)
 		drxctl    = uintptr(C.DR_RW_EXECUTE|C.DR_LEN_1) << uintptr(reg*C.DR_CONTROL_SIZE)
+		dr7       uintptr
 	)
 
 	// Get current state
-	dr7, err := PtracePeekUser(tid, dr7off)
+	var err error
+	dbp.execPtraceFunc(func() { dr7, err = PtracePeekUser(tid, dr7off) })
 	if err != nil {
 		return err
 	}
@@ -43,12 +45,14 @@ func setHardwareBreakpoint(reg, tid int, addr uint64) error {
 	// If addr == 0 we are expected to disable the breakpoint
 	if addr == 0 {
 		dr7 &= ^drxmask
-		return PtracePokeUser(tid, dr7off, dr7)
+		dbp.execPtraceFunc(func() { err = PtracePokeUser(tid, dr7off, dr7) })
+		return err
 	}
 
 	// Set the debug register `reg` with the address of the
 	// instruction we want to trigger a debug exception.
-	if err := PtracePokeUser(tid, drxoff, uintptr(addr)); err != nil {
+	dbp.execPtraceFunc(func() { err = PtracePokeUser(tid, drxoff, uintptr(addr)) })
+	if err != nil {
 		return err
 	}
 
@@ -61,12 +65,13 @@ func setHardwareBreakpoint(reg, tid int, addr uint64) error {
 	// instructs the cpu to raise a debug
 	// exception when hitting the address of
 	// an instruction stored in dr0-dr3.
-	return PtracePokeUser(tid, dr7off, dr7)
+	dbp.execPtraceFunc(func() { err = PtracePokeUser(tid, dr7off, dr7) })
+	return err
 }
 
 // Clears a hardware breakpoint. Essentially sets
 // the debug reg to 0 and clears the control register
 // flags for that reg.
-func clearHardwareBreakpoint(reg, tid int) error {
-	return setHardwareBreakpoint(reg, tid, 0)
+func (dbp *DebuggedProcess) clearHardwareBreakpoint(reg, tid int) error {
+	return dbp.setHardwareBreakpoint(reg, tid, 0)
 }

proc/proc.go

@@ -49,6 +49,23 @@ type DebuggedProcess struct {
 	running                 bool
 	halt                    bool
 	exited                  bool
+	ptraceChan              chan func()
+	ptraceDoneChan          chan interface{}
+}
+
+func New(pid int) *DebuggedProcess {
+	dbp := &DebuggedProcess{
+		Pid:            pid,
+		Threads:        make(map[int]*Thread),
+		Breakpoints:    make(map[uint64]*Breakpoint),
+		firstStart:     true,
+		os:             new(OSProcessDetails),
+		ast:            source.New(),
+		ptraceChan:     make(chan func()),
+		ptraceDoneChan: make(chan interface{}),
+	}
+	go dbp.handlePtraceFuncs()
+	return dbp
 }
 
 // A ManualStopError happens when the user triggers a
@@ -72,22 +89,33 @@ func (pe ProcessExitedError) Error() string {
 
 // Attach to an existing process with the given PID.
 func Attach(pid int) (*DebuggedProcess, error) {
-	dbp := &DebuggedProcess{
-		Pid:         pid,
-		Threads:     make(map[int]*Thread),
-		Breakpoints: make(map[uint64]*Breakpoint),
-		os:          new(OSProcessDetails),
-		ast:         source.New(),
-	}
-	dbp, err := initializeDebugProcess(dbp, "", true)
+	dbp, err := initializeDebugProcess(New(pid), "", true)
 	if err != nil {
 		return nil, err
 	}
 	return dbp, nil
 }
 
-func (dbp *DebuggedProcess) Detach() error {
-	return PtraceDetach(dbp.Pid, int(sys.SIGINT))
+func (dbp *DebuggedProcess) Detach(kill bool) (err error) {
+	// Clean up any breakpoints we've set.
+	for _, bp := range dbp.arch.HardwareBreakpoints() {
+		if bp != nil {
+			dbp.Clear(bp.Addr)
+		}
+	}
+	for _, bp := range dbp.Breakpoints {
+		if bp != nil {
+			dbp.Clear(bp.Addr)
+		}
+	}
+	dbp.execPtraceFunc(func() {
+		var sig int
+		if kill {
+			sig = int(sys.SIGINT)
+		}
+		err = PtraceDetach(dbp.Pid, sig)
+	})
+	return
 }
 
 // Returns whether or not Delve thinks the debugged
@@ -497,6 +525,21 @@ func (dbp *DebuggedProcess) PCToLine(pc uint64) (string, int, *gosym.Func) {
 	return dbp.goSymTable.PCToLine(pc)
 }
 
+// Finds the breakpoint for the given ID.
+func (dbp *DebuggedProcess) FindBreakpointByID(id int) (*Breakpoint, bool) {
+	for _, bp := range dbp.arch.HardwareBreakpoints() {
+		if bp != nil && bp.ID == id {
+			return bp, true
+		}
+	}
+	for _, bp := range dbp.Breakpoints {
+		if bp.ID == id {
+			return bp, true
+		}
+	}
+	return nil, false
+}
+
 // Finds the breakpoint for the given pc.
 func (dbp *DebuggedProcess) FindBreakpoint(pc uint64) (*Breakpoint, bool) {
 	for _, bp := range dbp.arch.HardwareBreakpoints() {
@@ -513,7 +556,8 @@ func (dbp *DebuggedProcess) FindBreakpoint(pc uint64) (*Breakpoint, bool) {
 // Returns a new DebuggedProcess struct.
 func initializeDebugProcess(dbp *DebuggedProcess, path string, attach bool) (*DebuggedProcess, error) {
 	if attach {
-		err := sys.PtraceAttach(dbp.Pid)
+		var err error
+		dbp.execPtraceFunc(func() { err = sys.PtraceAttach(dbp.Pid) })
 		if err != nil {
 			return nil, err
 		}
@@ -623,3 +667,20 @@ func (dbp *DebuggedProcess) run(fn func() error) error {
 	}
 	return nil
 }
+
+func (dbp *DebuggedProcess) handlePtraceFuncs() {
+	// We must ensure here that we are running on the same thread during
+	// the execution of dbg. This is due to the fact that ptrace(2) expects
+	// all commands after PTRACE_ATTACH to come from the same thread.
+	runtime.LockOSThread()
+
+	for fn := range dbp.ptraceChan {
+		fn()
+		dbp.ptraceDoneChan <- nil
+	}
+}
+
+func (dbp *DebuggedProcess) execPtraceFunc(fn func()) {
+	dbp.ptraceChan <- fn
+	<-dbp.ptraceDoneChan
+}

proc/proc_darwin.go

@@ -15,7 +15,6 @@ import (
 
 	"github.com/derekparker/delve/dwarf/frame"
 	"github.com/derekparker/delve/dwarf/line"
-	"github.com/derekparker/delve/source"
 	sys "golang.org/x/sys/unix"
 )
 
@@ -44,16 +43,12 @@ func Launch(cmd []string) (*DebuggedProcess, error) {
 	var argv **C.char
 	argv = &argvSlice[0]
 
-	dbp := &DebuggedProcess{
-		Threads:     make(map[int]*Thread),
-		Breakpoints: make(map[uint64]*Breakpoint),
-		firstStart:  true,
-		os:          new(OSProcessDetails),
-		ast:         source.New(),
-	}
-
-	ret := C.fork_exec(argv0, argv, &dbp.os.task, &dbp.os.portSet, &dbp.os.exceptionPort, &dbp.os.notificationPort)
-	pid := int(ret)
+	dbp := New(0)
+	var pid int
+	dbp.execPtraceFunc(func() {
+		ret := C.fork_exec(argv0, argv, &dbp.os.task, &dbp.os.portSet, &dbp.os.exceptionPort, &dbp.os.notificationPort)
+		pid = int(ret)
+	})
 	if pid <= 0 {
 		return nil, fmt.Errorf("could not fork/exec")
 	}

proc/proc_linux.go

@@ -15,7 +15,6 @@ import (
 
 	"github.com/derekparker/delve/dwarf/frame"
 	"github.com/derekparker/delve/dwarf/line"
-	"github.com/derekparker/delve/source"
 )
 
 const (
@@ -31,27 +30,27 @@ type OSProcessDetails interface{}
 // `cmd` is the program to run, and then rest are the arguments
 // to be supplied to that process.
 func Launch(cmd []string) (*DebuggedProcess, error) {
-	proc := exec.Command(cmd[0])
-	proc.Args = cmd
-	proc.Stdout = os.Stdout
-	proc.Stderr = os.Stderr
-	proc.SysProcAttr = &syscall.SysProcAttr{Ptrace: true}
-
-	if err := proc.Start(); err != nil {
+	var (
+		proc *exec.Cmd
+		err  error
+	)
+	dbp := New(0)
+	dbp.execPtraceFunc(func() {
+		proc = exec.Command(cmd[0])
+		proc.Args = cmd
+		proc.Stdout = os.Stdout
+		proc.Stderr = os.Stderr
+		proc.SysProcAttr = &syscall.SysProcAttr{Ptrace: true}
+		err = proc.Start()
+	})
+	if err != nil {
 		return nil, err
 	}
-	_, _, err := wait(proc.Process.Pid, 0)
+	dbp.Pid = proc.Process.Pid
+	_, _, err = wait(proc.Process.Pid, 0)
 	if err != nil {
 		return nil, fmt.Errorf("waiting for target execve failed: %s", err)
 	}
-	dbp := &DebuggedProcess{
-		Pid:         proc.Process.Pid,
-		Threads:     make(map[int]*Thread),
-		Breakpoints: make(map[uint64]*Breakpoint),
-		os:          new(OSProcessDetails),
-		ast:         source.New(),
-	}
-
 	return initializeDebugProcess(dbp, proc.Path, false)
 }
 
@@ -66,8 +65,9 @@ func (dbp *DebuggedProcess) addThread(tid int, attach bool) (*Thread, error) {
 		return thread, nil
 	}
 
+	var err error
 	if attach {
-		err := sys.PtraceAttach(tid)
+		dbp.execPtraceFunc(func() { err = sys.PtraceAttach(tid) })
 		if err != nil && err != sys.EPERM {
 			// Do not return err if err == EPERM,
 			// we may already be tracing this thread due to
@@ -86,14 +86,14 @@ func (dbp *DebuggedProcess) addThread(tid int, attach bool) (*Thread, error) {
 		}
 	}
 
-	err := syscall.PtraceSetOptions(tid, syscall.PTRACE_O_TRACECLONE)
+	dbp.execPtraceFunc(func() { err = syscall.PtraceSetOptions(tid, syscall.PTRACE_O_TRACECLONE) })
 	if err == syscall.ESRCH {
 		_, _, err = wait(tid, 0)
 		if err != nil {
 			return nil, fmt.Errorf("error while waiting after adding thread: %d %s", tid, err)
 		}
 
-		err := syscall.PtraceSetOptions(tid, syscall.PTRACE_O_TRACECLONE)
+		dbp.execPtraceFunc(func() { err = syscall.PtraceSetOptions(tid, syscall.PTRACE_O_TRACECLONE) })
 		if err != nil {
 			return nil, fmt.Errorf("could not set options for new traced thread %d %s", tid, err)
 		}
@@ -244,7 +244,8 @@ func (dbp *DebuggedProcess) trapWait(pid int) (*Thread, error) {
 		if status.StopSignal() == sys.SIGTRAP && status.TrapCause() == sys.PTRACE_EVENT_CLONE {
 			// A traced thread has cloned a new thread, grab the pid and
 			// add it to our list of traced threads.
-			cloned, err := sys.PtraceGetEventMsg(wpid)
+			var cloned uint
+			dbp.execPtraceFunc(func() { cloned, err = sys.PtraceGetEventMsg(wpid) })
 			if err != nil {
 				return nil, fmt.Errorf("could not get event message: %s", err)
 			}
@@ -256,7 +257,7 @@ func (dbp *DebuggedProcess) trapWait(pid int) (*Thread, error) {
 				if bp == nil {
 					continue
 				}
-				if err = setHardwareBreakpoint(reg, th.Id, bp.Addr); err != nil {
+				if err = dbp.setHardwareBreakpoint(reg, th.Id, bp.Addr); err != nil {
 					return nil, err
 				}
 			}

proc/proc_test.go

@@ -10,20 +10,22 @@ import (
 	protest "github.com/derekparker/delve/proc/test"
 )
 
+func init() {
+	runtime.GOMAXPROCS(2)
+}
+
 func TestMain(m *testing.M) {
 	protest.RunTestsWithFixtures(m)
 }
 
 func withTestProcess(name string, t *testing.T, fn func(p *DebuggedProcess, fixture protest.Fixture)) {
-	runtime.LockOSThread()
-
 	fixture := protest.BuildFixture(name)
 	p, err := Launch([]string{fixture.Path})
 	if err != nil {
 		t.Fatal("Launch():", err)
 	}
 
-	defer p.Detach()
+	defer p.Detach(true)
 
 	fn(p, fixture)
 }
@@ -88,6 +90,7 @@ func TestExit(t *testing.T) {
 }
 
 func TestHalt(t *testing.T) {
+	runtime.GOMAXPROCS(2)
 	withTestProcess("testprog", t, func(p *DebuggedProcess, fixture protest.Fixture) {
 		go func() {
 			for {

proc/registers_linux_amd64.go

@@ -18,14 +18,18 @@ func (r *Regs) CX() uint64 {
 	return r.regs.Rcx
 }
 
-func (r *Regs) SetPC(thread *Thread, pc uint64) error {
+func (r *Regs) SetPC(thread *Thread, pc uint64) (err error) {
 	r.regs.SetPC(pc)
-	return sys.PtraceSetRegs(thread.Id, r.regs)
+	thread.dbp.execPtraceFunc(func() { err = sys.PtraceSetRegs(thread.Id, r.regs) })
+	return
 }
 
 func registers(thread *Thread) (Registers, error) {
-	var regs sys.PtraceRegs
-	err := sys.PtraceGetRegs(thread.Id, &regs)
+	var (
+		regs sys.PtraceRegs
+		err  error
+	)
+	thread.dbp.execPtraceFunc(func() { err = sys.PtraceGetRegs(thread.Id, &regs) })
 	if err != nil {
 		return nil, err
 	}

proc/test/support.go

@@ -28,7 +28,7 @@ func BuildFixture(name string) Fixture {
 		return f
 	}
 	parent := ".."
-	fixturesDir := filepath.Join(parent, "_fixtures")
+	fixturesDir := "_fixtures"
 	for depth := 0; depth < 10; depth++ {
 		if _, err := os.Stat(fixturesDir); err == nil {
 			break

proc/threads_darwin.go

@@ -36,7 +36,9 @@ func (t *Thread) singleStep() error {
 
 func (t *Thread) resume() error {
 	// TODO(dp) set flag for ptrace stops
-	if PtraceCont(t.dbp.Pid, 0) == nil {
+	var err error
+	t.dbp.execPtraceFunc(func() { err = PtraceCont(t.dbp.Pid, 0) })
+	if err == nil {
 		return nil
 	}
 	kret := C.resume_thread(t.os.thread_act)

proc/threads_linux.go

@@ -27,12 +27,13 @@ func (t *Thread) Halt() error {
 	return nil
 }
 
-func (t *Thread) resume() error {
-	return PtraceCont(t.Id, 0)
+func (t *Thread) resume() (err error) {
+	t.dbp.execPtraceFunc(func() { err = PtraceCont(t.Id, 0) })
+	return
 }
 
-func (t *Thread) singleStep() error {
-	err := sys.PtraceSingleStep(t.Id)
+func (t *Thread) singleStep() (err error) {
+	t.dbp.execPtraceFunc(func() { err = sys.PtraceSingleStep(t.Id) })
 	if err != nil {
 		return err
 	}
@@ -51,26 +52,31 @@ func (t *Thread) blocked() bool {
 }
 
 func (thread *Thread) saveRegisters() (Registers, error) {
-	if err := sys.PtraceGetRegs(thread.Id, &thread.os.registers); err != nil {
+	var err error
+	thread.dbp.execPtraceFunc(func() { err = sys.PtraceGetRegs(thread.Id, &thread.os.registers) })
+	if err != nil {
 		return nil, fmt.Errorf("could not save register contents")
 	}
 	return &Regs{&thread.os.registers}, nil
 }
 
-func (thread *Thread) restoreRegisters() error {
-	return sys.PtraceSetRegs(thread.Id, &thread.os.registers)
+func (thread *Thread) restoreRegisters() (err error) {
+	thread.dbp.execPtraceFunc(func() { err = sys.PtraceSetRegs(thread.Id, &thread.os.registers) })
+	return
 }
 
-func writeMemory(thread *Thread, addr uintptr, data []byte) (int, error) {
+func writeMemory(thread *Thread, addr uintptr, data []byte) (written int, err error) {
 	if len(data) == 0 {
-		return 0, nil
+		return
 	}
-	return sys.PtracePokeData(thread.Id, addr, data)
+	thread.dbp.execPtraceFunc(func() { written, err = sys.PtracePokeData(thread.Id, addr, data) })
+	return
 }
 
-func readMemory(thread *Thread, addr uintptr, data []byte) (int, error) {
+func readMemory(thread *Thread, addr uintptr, data []byte) (read int, err error) {
 	if len(data) == 0 {
-		return 0, nil
+		return
 	}
-	return sys.PtracePeekData(thread.Id, addr, data)
+	thread.dbp.execPtraceFunc(func() { read, err = sys.PtracePeekData(thread.Id, addr, data) })
+	return
 }