From 8be3ffc774a36512870f40130febf08bdae897ea Mon Sep 17 00:00:00 2001 From: Derek Parker Date: Sat, 22 Nov 2014 18:57:26 -0600 Subject: [PATCH] Refactor: wrap syscall.Wait4 Wrap syscall.Wait4 and cleanup a few coordination issues. There are still some issues here where background threads are left sleeping. This could potentially cause weird issues. There are a few more things I have planned to cleanup thread coordination issues. --- proctl/proctl_linux_amd64.go | 36 ++++++++++++++----------- proctl/proctl_test.go | 4 ++- proctl/threads_linux_amd64.go | 50 +++-------------------------------- 3 files changed, 27 insertions(+), 63 deletions(-) diff --git a/proctl/proctl_linux_amd64.go b/proctl/proctl_linux_amd64.go index d061e096..3a0e3430 100644 --- a/proctl/proctl_linux_amd64.go +++ b/proctl/proctl_linux_amd64.go @@ -95,7 +95,7 @@ func Launch(cmd []string) (*DebuggedProcess, error) { return nil, err } - _, err := syscall.Wait4(proc.Process.Pid, nil, syscall.WALL, nil) + _, _, err := wait(proc.Process.Pid, 0) if err != nil { return nil, fmt.Errorf("waiting for target execve failed: %s", err) } @@ -161,9 +161,8 @@ func (dbp *DebuggedProcess) AttachThread(tid int) (*ThreadContext, error) { return nil, fmt.Errorf("could not attach to new thread %d %s", tid, err) } - var status syscall.WaitStatus - pid, e := syscall.Wait4(tid, &status, syscall.WALL, nil) - if e != nil { + pid, status, err := wait(tid, 0) + if err != nil { return nil, err } @@ -177,7 +176,7 @@ func (dbp *DebuggedProcess) AttachThread(tid int) (*ThreadContext, error) { func (dbp *DebuggedProcess) addThread(tid int) (*ThreadContext, error) { err := syscall.PtraceSetOptions(tid, syscall.PTRACE_O_TRACECLONE) if err == syscall.ESRCH { - _, err = syscall.Wait4(tid, nil, syscall.WALL, nil) + _, _, err = wait(tid, 0) if err != nil { return nil, fmt.Errorf("error while waiting after adding thread: %d %s", tid, err) } @@ -267,7 +266,9 @@ func (dbp *DebuggedProcess) Next() error { for _, thread := range dbp.Threads { err := thread.Next() if err != nil { - if _, ok := err.(ProcessExitedError); !ok { + // TODO(dp): There are some coordination issues + // here that need to be resolved. + if _, ok := err.(TimeoutError); !ok && err != syscall.ESRCH { return err } } @@ -391,10 +392,8 @@ func (pe ProcessExitedError) Error() string { } func trapWait(dbp *DebuggedProcess, pid int, options int) (int, *syscall.WaitStatus, error) { - var status syscall.WaitStatus - for { - wpid, err := syscall.Wait4(pid, &status, syscall.WALL|options, nil) + wpid, status, err := wait(pid, 0) if err != nil { return -1, nil, fmt.Errorf("wait err %s %d", err, pid) } @@ -402,10 +401,10 @@ func trapWait(dbp *DebuggedProcess, pid int, options int) (int, *syscall.WaitSta continue } if th, ok := dbp.Threads[wpid]; ok { - th.Status = &status + th.Status = status } if status.Exited() && wpid == dbp.Pid { - return -1, &status, ProcessExitedError{wpid} + return -1, status, ProcessExitedError{wpid} } if status.StopSignal() == syscall.SIGTRAP && status.TrapCause() == syscall.PTRACE_EVENT_CLONE { err = addNewThread(dbp, wpid) @@ -415,7 +414,7 @@ func trapWait(dbp *DebuggedProcess, pid int, options int) (int, *syscall.WaitSta continue } if status.StopSignal() == syscall.SIGTRAP { - return wpid, &status, nil + return wpid, status, nil } } } @@ -483,7 +482,7 @@ func stopTheWorld(dbp *DebuggedProcess, thread *ThreadContext, pid int) error { return err } - pid, err := syscall.Wait4(th.Id, nil, syscall.WALL, nil) + pid, _, err := wait(th.Id, 0) if err != nil { return fmt.Errorf("wait err %s %d", err, pid) } @@ -539,7 +538,6 @@ func (err TimeoutError) Error() string { // scheduler or sleeping due to a user calling sleep. func timeoutWait(thread *ThreadContext, options int) (int, *syscall.WaitStatus, error) { var ( - status syscall.WaitStatus statchan = make(chan *waitstats) errchan = make(chan error) ) @@ -554,12 +552,12 @@ func timeoutWait(thread *ThreadContext, options int) (int, *syscall.WaitStatus, } go func(pid int) { - wpid, err := syscall.Wait4(pid, &status, syscall.WALL|options, nil) + wpid, status, err := wait(pid, 0) if err != nil { errchan <- fmt.Errorf("wait err %s %d", err, pid) } - statchan <- &waitstats{pid: wpid, status: &status} + statchan <- &waitstats{pid: wpid, status: status} }(thread.Id) select { @@ -575,3 +573,9 @@ func timeoutWait(thread *ThreadContext, options int) (int, *syscall.WaitStatus, return -1, nil, err } } + +func wait(pid, options int) (int, *syscall.WaitStatus, error) { + var status syscall.WaitStatus + wpid, err := syscall.Wait4(pid, &status, syscall.WALL|options, nil) + return wpid, &status, err +} diff --git a/proctl/proctl_test.go b/proctl/proctl_test.go index 822e4b98..6cdfb1f5 100644 --- a/proctl/proctl_test.go +++ b/proctl/proctl_test.go @@ -95,7 +95,9 @@ func TestBreakPoint(t *testing.T) { } err = p.Step() - assertNoError(err, t, "Step()") + if _, ok := err.(proctl.TimeoutError); !ok { + assertNoError(err, t, "Step()") + } pc, err = p.CurrentPC() if err != nil { diff --git a/proctl/threads_linux_amd64.go b/proctl/threads_linux_amd64.go index 163c64b2..62aa735f 100644 --- a/proctl/threads_linux_amd64.go +++ b/proctl/threads_linux_amd64.go @@ -24,16 +24,7 @@ type ThreadContext struct { func (thread *ThreadContext) Registers() (*syscall.PtraceRegs, error) { err := syscall.PtraceGetRegs(thread.Id, thread.Regs) if err != nil { - syscall.Tgkill(thread.Process.Pid, thread.Id, syscall.SIGSTOP) - err = thread.wait() - if err != nil { - return nil, err - } - - err := syscall.PtraceGetRegs(thread.Id, thread.Regs) - if err != nil { - return nil, fmt.Errorf("Registers(): %s", err) - } + return nil, err } return thread.Regs, nil @@ -118,22 +109,7 @@ func (thread *ThreadContext) Clear(pc uint64) (*BreakPoint, error) { } if _, err := syscall.PtracePokeData(thread.Id, uintptr(bp.Addr), bp.OriginalData); err != nil { - ps, err := parseProcessStatus(thread.Id) - if err != nil { - return nil, err - } - - if ps.state != STATUS_TRACE_STOP { - if err := syscall.Tgkill(thread.Process.Pid, thread.Id, syscall.SIGSTOP); err != nil { - return nil, err - } - if _, err := syscall.Wait4(thread.Id, nil, syscall.WALL, nil); err != nil { - return nil, err - } - if _, err := syscall.PtracePokeData(thread.Id, uintptr(bp.Addr), bp.OriginalData); err != nil { - return nil, err - } - } + return nil, err } delete(thread.Process.BreakPoints, pc) @@ -194,10 +170,7 @@ func (thread *ThreadContext) Step() (err error) { _, _, err = timeoutWait(thread, 0) if err != nil { - if _, ok := err.(TimeoutError); ok { - return nil - } - return fmt.Errorf("step failed: %s", err.Error()) + return err } return nil @@ -223,7 +196,7 @@ func (thread *ThreadContext) Next() (err error) { step := func() (uint64, error) { err = thread.Step() if err != nil { - return 0, fmt.Errorf("next stepping failed: %s", err.Error()) + return 0, err } return thread.CurrentPC() @@ -308,7 +281,6 @@ func (thread *ThreadContext) clearTempBreakpoint(pc uint64) error { if bp, ok := thread.Process.BreakPoints[pc]; ok { _, err := thread.Clear(bp.Addr) if err != nil { - fmt.Println("ERR", err) return err } @@ -325,20 +297,6 @@ func (thread *ThreadContext) clearTempBreakpoint(pc uint64) error { return nil } -func (thread *ThreadContext) wait() error { - var status syscall.WaitStatus - _, err := syscall.Wait4(thread.Id, &status, 0, nil) - if err != nil { - if status.Exited() { - delete(thread.Process.Threads, thread.Id) - return ProcessExitedError{thread.Id} - } - return err - } - - return nil -} - func threadIds(pid int) []int { var threads []int dir, err := os.Open(fmt.Sprintf("/proc/%d/task", pid))