More cleanup

proctl/proctl.go

@@ -51,11 +51,6 @@ func Attach(pid int) (*DebuggedProcess, error) {
 	if err != nil {
 		return nil, err
 	}
-	// Attach to all currently active threads.
-	// TODO(dp) doing this in newDebugProcess already for mach
-	if err := dbp.updateThreadList(); err != nil {
-		return nil, err
-	}
 	return dbp, nil
 }
 

proctl/proctl_darwin.go

@@ -66,7 +66,6 @@ func (dbp *DebuggedProcess) updateThreadList() error {
 	var (
 		err   error
 		kret  C.kern_return_t
-		th    *ThreadContext
 		count = C.thread_count(C.task_t(dbp.os.task))
 	)
 	if count == -1 {
@@ -74,7 +73,7 @@ func (dbp *DebuggedProcess) updateThreadList() error {
 	}
 	list := make([]uint32, count)
 
-	// TODO(dp) might be better to malloc mem in C and them free it here
+	// TODO(dp) might be better to malloc mem in C and then free it here
 	// instead of getting count above and passing in a slice
 	kret = C.get_threads(C.task_t(dbp.os.task), unsafe.Pointer(&list[0]))
 	if kret != C.KERN_SUCCESS {
@@ -85,17 +84,15 @@ func (dbp *DebuggedProcess) updateThreadList() error {
 	}
 
 	for _, port := range list {
-		th, err = dbp.addThread(int(port), false)
-		if err != nil {
-			return err
+		if _, ok := dbp.Threads[int(port)]; !ok {
+			fmt.Println("new thread spawned", port)
+			_, err = dbp.addThread(int(port), false)
+			if err != nil {
+				return err
+			}
 		}
 	}
 
-	// TODO(dp) account for GOMAXPROCS=1 or attaching to pid
-	if count == 1 {
-		dbp.CurrentThread = th
-	}
-
 	return nil
 }
 
@@ -115,6 +112,9 @@ func (dbp *DebuggedProcess) addThread(port int, attach bool) (*ThreadContext, er
 	}
 	dbp.Threads[port] = thread
 	thread.os.thread_act = C.thread_act_t(port)
+	if dbp.CurrentThread == nil {
+		dbp.CurrentThread = thread
+	}
 	return thread, nil
 }
 
@@ -169,11 +169,6 @@ func (dbp *DebuggedProcess) obtainGoSymbols(exe *macho.File, wg *sync.WaitGroup)
 	dbp.GoSymTable = tab
 }
 
-// TODO(darwin) IMPLEMENT ME
-func stopped(pid int) bool {
-	return false
-}
-
 func (dbp *DebuggedProcess) findExecutable() (*macho.File, error) {
 	pathptr, err := C.find_executable(C.int(dbp.Pid))
 	if err != nil {

proctl/proctl_linux.go

@@ -203,23 +203,6 @@ func (dbp *DebuggedProcess) obtainGoSymbols(exe *elf.File, wg *sync.WaitGroup) {
 	dbp.GoSymTable = tab
 }
 
-// TODO(dp) seems like it could be unneccessary
-func addNewThread(dbp *DebuggedProcess, cloner, cloned int) error {
-	fmt.Println("new thread spawned", cloned)
-
-	th, err := dbp.addThread(cloned, false)
-	if err != nil {
-		return err
-	}
-
-	err = th.Continue()
-	if err != nil {
-		return fmt.Errorf("could not continue new thread %d %s", cloned, err)
-	}
-
-	return dbp.Threads[cloner].Continue()
-}
-
 func stopped(pid int) bool {
 	f, err := os.Open(fmt.Sprintf("/proc/%d/stat", pid))
 	if err != nil {
@@ -257,14 +240,27 @@ func trapWait(dbp *DebuggedProcess, pid int) (int, *sys.WaitStatus, 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.
-			tid, err := sys.PtraceGetEventMsg(wpid)
+			cloned, err := sys.PtraceGetEventMsg(wpid)
 			if err != nil {
 				return -1, nil, fmt.Errorf("could not get event message: %s", err)
 			}
-			err = addNewThread(dbp, wpid, int(tid))
+
+			fmt.Println("new thread spawned", cloned)
+
+			th, err := dbp.addThread(cloned, false)
 			if err != nil {
 				return -1, nil, err
 			}
+
+			err = th.Continue()
+			if err != nil {
+				return -1, nil, fmt.Errorf("could not continue new thread %d %s", cloned, err)
+			}
+
+			err = dbp.Threads[cloner].Continue()
+			if err != nil {
+				return -1, nil, fmt.Errorf("could not continue new thread %d %s", cloned, err)
+			}
 			continue
 		}
 		if status.StopSignal() == sys.SIGTRAP {

proctl/threads_darwin.go

@@ -12,7 +12,6 @@ type OSSpecificDetails struct {
 }
 
 func (t *ThreadContext) Halt() error {
-	// TODO(dp) may be able to just task_suspend instead of suspending individual threads
 	var kret C.kern_return_t
 	kret = C.thread_suspend(t.os.thread_act)
 	if kret != C.KERN_SUCCESS {
@@ -49,7 +48,6 @@ func (t *ThreadContext) resume() error {
 
 func (t *ThreadContext) blocked() bool {
 	// TODO(dp) cache the func pc to remove this lookup
-	// TODO(dp) check err
 	pc, _ := t.CurrentPC()
 	fn := t.Process.GoSymTable.PCToFunc(pc)
 	if fn != nil && ((fn.Name == "runtime.mach_semaphore_wait") || (fn.Name == "runtime.usleep")) {

proctl/threads_linux.go

@@ -40,7 +40,6 @@ func (t *ThreadContext) singleStep() error {
 
 func (t *ThreadContext) blocked() bool {
 	// TODO(dp) cache the func pc to remove this lookup
-	// TODO(dp) check err
 	pc, _ := t.CurrentPC()
 	fn := t.Process.GoSymTable.PCToFunc(pc)
 	if fn != nil && ((fn.Name == "runtime.futex") || (fn.Name == "runtime.usleep") || (fn.Name == "runtime.clone")) {