From c4500be0662e5737c3f703c4e5d6e61e9f8bea61 Mon Sep 17 00:00:00 2001 From: aarzilli Date: Wed, 12 Jul 2023 16:06:33 +0200 Subject: [PATCH] proc: refactorings to implement follow-exec mode on Windows Miscellaneous non-functional changes to prepare for adding support for follow-exec mode on Windows: - removed (*nativeProcess).wait function from Windows backend (unused). - move close of ptraceDoneChan from release to handlePtraceFuncs, this makes postExit callable by a function executed by execPtraceFunc. - change addTarget to detach before creating the target object if we don't actually want to attach to the child process, also moved the detach call to (*processGroup).add instead of having one in addTarget and one in the code that calls (*processGroup).add. - changed Detach to be a method of TargetGroup/ProcessGroup, the Windows backend will need access to the process group to call WaitForDebugEvent. - moved resume method to processGroup. First all threads stopped at a breakpoint need to be stepped, then all other threads can be resumed. This is true also for linux even though it didn't cause the current tests to fail. --- pkg/proc/core/core.go | 2 +- pkg/proc/gdbserial/gdbserver.go | 6 ++-- pkg/proc/interface.go | 2 +- pkg/proc/native/nonative_darwin.go | 4 +-- pkg/proc/native/proc.go | 48 ++++++++++++++++++++++-------- pkg/proc/native/proc_darwin.go | 10 +++++-- pkg/proc/native/proc_freebsd.go | 9 +++--- pkg/proc/native/proc_linux.go | 33 ++++++++++++-------- pkg/proc/native/proc_windows.go | 13 ++++---- pkg/proc/target.go | 22 -------------- pkg/proc/target_group.go | 41 ++++++++++++++++++++----- 11 files changed, 114 insertions(+), 76 deletions(-) diff --git a/pkg/proc/core/core.go b/pkg/proc/core/core.go index 358c0b085f..134b1ae66f 100644 --- a/pkg/proc/core/core.go +++ b/pkg/proc/core/core.go @@ -454,7 +454,7 @@ func (p *process) Memory() proc.MemoryReadWriter { // Detach will always return nil and have no // effect as you cannot detach from a core file // and have it continue execution or exit. -func (p *process) Detach(bool) error { +func (p *process) Detach(int, bool) error { return nil } diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go index bb57f3d587..1394051878 100644 --- a/pkg/proc/gdbserial/gdbserver.go +++ b/pkg/proc/gdbserial/gdbserver.go @@ -748,7 +748,7 @@ func (p *gdbProcess) initialize(path, cmdline string, debugInfoDirs []string, st }) _, err = addTarget(p, p.conn.pid, p.currentThread, path, stopReason, cmdline) if err != nil { - p.Detach(true) + p.Detach(p.conn.pid, true) return nil, err } return grp, nil @@ -1058,7 +1058,9 @@ func (p *gdbProcess) getCtrlC(cctx *proc.ContinueOnceContext) bool { // Detach will detach from the target process, // if 'kill' is true it will also kill the process. -func (p *gdbProcess) Detach(kill bool) error { +// The _pid argument is unused as follow exec +// mode is not implemented with this backend. +func (p *gdbProcess) Detach(_pid int, kill bool) error { if kill && !p.exited { err := p.conn.kill() if err != nil { diff --git a/pkg/proc/interface.go b/pkg/proc/interface.go index 84bd80ab1c..2114f1aac7 100644 --- a/pkg/proc/interface.go +++ b/pkg/proc/interface.go @@ -11,6 +11,7 @@ import ( // ProcessGroup is a group of processes that are resumed at the same time. type ProcessGroup interface { ContinueOnce(*ContinueOnceContext) (Thread, StopReason, error) + Detach(int, bool) error } // Process represents the target of the debugger. This @@ -43,7 +44,6 @@ type ProcessInternal interface { // also returns an error describing why the Process is invalid (either // ErrProcessExited or ErrProcessDetached). Valid() (bool, error) - Detach(bool) error // RequestManualStop attempts to stop all the process' threads. RequestManualStop(cctx *ContinueOnceContext) error diff --git a/pkg/proc/native/nonative_darwin.go b/pkg/proc/native/nonative_darwin.go index 0fa7be5275..3a20b9289e 100644 --- a/pkg/proc/native/nonative_darwin.go +++ b/pkg/proc/native/nonative_darwin.go @@ -57,7 +57,7 @@ func (dbp *nativeProcess) requestManualStop() (err error) { panic(ErrNativeBackendDisabled) } -func (dbp *nativeProcess) resume() error { +func (*processGroup) resume() error { panic(ErrNativeBackendDisabled) } @@ -73,7 +73,7 @@ func (dbp *nativeProcess) updateThreadList() error { panic(ErrNativeBackendDisabled) } -func (dbp *nativeProcess) kill() (err error) { +func (*processGroup) kill(dbp *nativeProcess) (err error) { panic(ErrNativeBackendDisabled) } diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index aeb4b23f85..a73ef590f5 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -96,17 +96,24 @@ func (dbp *nativeProcess) BinInfo() *proc.BinaryInfo { // StartCallInjection notifies the backend that we are about to inject a function call. func (dbp *nativeProcess) StartCallInjection() (func(), error) { return func() {}, nil } +// detachWithoutGroup is a helper function to detach from a process which we +// haven't added to a process group yet. +func detachWithoutGroup(dbp *nativeProcess, kill bool) error { + grp := &processGroup{procs: []*nativeProcess{dbp}} + return grp.Detach(dbp.pid, kill) +} + // Detach from the process being debugged, optionally killing it. -func (dbp *nativeProcess) Detach(kill bool) (err error) { +func (procgrp *processGroup) Detach(pid int, kill bool) (err error) { + dbp := procgrp.procForPid(pid) if dbp.exited { return nil } if kill && dbp.childProcess { - err := dbp.kill() + err := procgrp.kill(dbp) if err != nil { return err } - dbp.bi.Close() return nil } dbp.execPtraceFunc(func() { @@ -226,8 +233,25 @@ func (procgrp *processGroup) procForThread(tid int) *nativeProcess { return nil } +func (procgrp *processGroup) procForPid(pid int) *nativeProcess { + for _, p := range procgrp.procs { + if p.pid == pid { + return p + } + } + return nil +} + func (procgrp *processGroup) add(p *nativeProcess, pid int, currentThread proc.Thread, path string, stopReason proc.StopReason, cmdline string) (*proc.Target, error) { tgt, err := procgrp.addTarget(p, pid, currentThread, path, stopReason, cmdline) + if tgt == nil { + i := len(procgrp.procs) + procgrp.procs = append(procgrp.procs, p) + procgrp.Detach(p.pid, false) + if i == len(procgrp.procs)-1 { + procgrp.procs = procgrp.procs[:i] + } + } if err != nil { return nil, err } @@ -246,15 +270,15 @@ func (procgrp *processGroup) ContinueOnce(cctx *proc.ContinueOnceContext) (proc. } for { + err := procgrp.resume() + if err != nil { + return nil, proc.StopUnknown, err + } for _, dbp := range procgrp.procs { - if dbp.exited { - continue - } - if err := dbp.resume(); err != nil { - return nil, proc.StopUnknown, err - } - for _, th := range dbp.threads { - th.CurrentBreakpoint.Clear() + if valid, _ := dbp.Valid(); valid { + for _, th := range dbp.threads { + th.CurrentBreakpoint.Clear() + } } } @@ -377,6 +401,7 @@ func (pt *ptraceThread) handlePtraceFuncs() { fn() pt.ptraceDoneChan <- nil } + close(pt.ptraceDoneChan) } func (dbp *nativeProcess) execPtraceFunc(fn func()) { @@ -473,6 +498,5 @@ func (pt *ptraceThread) release() { pt.ptraceRefCnt-- if pt.ptraceRefCnt == 0 { close(pt.ptraceChan) - close(pt.ptraceDoneChan) } } diff --git a/pkg/proc/native/proc_darwin.go b/pkg/proc/native/proc_darwin.go index 6332734487..1e2fe3d726 100644 --- a/pkg/proc/native/proc_darwin.go +++ b/pkg/proc/native/proc_darwin.go @@ -71,7 +71,7 @@ func Launch(cmd []string, wd string, flags proc.LaunchFlags, _ []string, _ strin dbp := newProcess(0) defer func() { if err != nil && dbp.pid != 0 { - _ = dbp.Detach(true) + _ = detachWithoutGroup(dbp, true) } }() var pid int @@ -172,14 +172,14 @@ func Attach(pid int, waitFor *proc.WaitFor, _ []string) (*proc.TargetGroup, erro tgt, err := dbp.initialize("", []string{}) if err != nil { - dbp.Detach(false) + detachWithoutGroup(dbp, false) return nil, err } return tgt, nil } // Kill kills the process. -func (dbp *nativeProcess) kill() (err error) { +func (procgrp *processGroup) kill(dbp *nativeProcess) (err error) { if dbp.exited { return nil } @@ -420,6 +420,10 @@ func (dbp *nativeProcess) exitGuard(err error) error { return err } +func (procgrp *processGroup) resume() error { + return procgrp.procs[0].resume() +} + func (dbp *nativeProcess) resume() error { // all threads stopped over a breakpoint are made to step over it for _, thread := range dbp.threads { diff --git a/pkg/proc/native/proc_freebsd.go b/pkg/proc/native/proc_freebsd.go index 2d04b92f04..f1b740b87f 100644 --- a/pkg/proc/native/proc_freebsd.go +++ b/pkg/proc/native/proc_freebsd.go @@ -79,7 +79,7 @@ func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []str dbp := newProcess(0) defer func() { if err != nil && dbp.pid != 0 { - _ = dbp.Detach(true) + _ = detachWithoutGroup(dbp, true) } }() dbp.execPtraceFunc(func() { @@ -146,7 +146,7 @@ func Attach(pid int, waitFor *proc.WaitFor, debugInfoDirs []string) (*proc.Targe tgt, err := dbp.initialize(findExecutable("", dbp.pid), debugInfoDirs) if err != nil { - dbp.Detach(false) + detachWithoutGroup(dbp, false) return nil, err } return tgt, nil @@ -186,7 +186,7 @@ func initialize(dbp *nativeProcess) (string, error) { } // kill kills the target process. -func (dbp *nativeProcess) kill() (err error) { +func (procgrp *processGroup) kill(dbp *nativeProcess) (err error) { if dbp.exited { return nil } @@ -425,7 +425,8 @@ func (dbp *nativeProcess) exitGuard(err error) error { } // Used by ContinueOnce -func (dbp *nativeProcess) resume() error { +func (procgrp *processGroup) resume() error { + dbp := procgrp.procs[0] // all threads stopped over a breakpoint are made to step over it for _, thread := range dbp.threads { if thread.CurrentBreakpoint.Breakpoint != nil { diff --git a/pkg/proc/native/proc_linux.go b/pkg/proc/native/proc_linux.go index 30a90d2682..1f9201551c 100644 --- a/pkg/proc/native/proc_linux.go +++ b/pkg/proc/native/proc_linux.go @@ -85,7 +85,7 @@ func Launch(cmd []string, wd string, flags proc.LaunchFlags, debugInfoDirs []str dbp := newProcess(0) defer func() { if err != nil && dbp.pid != 0 { - _ = dbp.Detach(true) + _ = detachWithoutGroup(dbp, true) } }() dbp.execPtraceFunc(func() { @@ -165,7 +165,7 @@ func Attach(pid int, waitFor *proc.WaitFor, debugInfoDirs []string) (*proc.Targe tgt, err := dbp.initialize(findExecutable("", dbp.pid), debugInfoDirs) if err != nil { - _ = dbp.Detach(false) + _ = detachWithoutGroup(dbp, false) return nil, err } @@ -261,7 +261,7 @@ func (dbp *nativeProcess) GetBufferedTracepoints() []ebpf.RawUProbeParams { } // kill kills the target process. -func (dbp *nativeProcess) kill() error { +func (procgrp *processGroup) kill(dbp *nativeProcess) error { if dbp.exited { return nil } @@ -519,7 +519,6 @@ func trapWaitInternal(procgrp *processGroup, pid int, options trapWaitOptions) ( cmdline, _ := dbp.initializeBasic() tgt, err := procgrp.add(dbp, dbp.pid, dbp.memthread, findExecutable("", dbp.pid), proc.StopLaunched, cmdline) if err != nil { - _ = dbp.Detach(false) return nil, err } if halt { @@ -648,20 +647,28 @@ func exitGuard(dbp *nativeProcess, procgrp *processGroup, err error) error { return err } -func (dbp *nativeProcess) resume() error { +func (procgrp *processGroup) resume() error { // all threads stopped over a breakpoint are made to step over it - for _, thread := range dbp.threads { - if thread.CurrentBreakpoint.Breakpoint != nil { - if err := thread.StepInstruction(); err != nil { - return err + for _, dbp := range procgrp.procs { + if valid, _ := dbp.Valid(); valid { + for _, thread := range dbp.threads { + if thread.CurrentBreakpoint.Breakpoint != nil { + if err := thread.StepInstruction(); err != nil { + return err + } + thread.CurrentBreakpoint.Clear() + } } - thread.CurrentBreakpoint.Clear() } } // everything is resumed - for _, thread := range dbp.threads { - if err := thread.resume(); err != nil && err != sys.ESRCH { - return err + for _, dbp := range procgrp.procs { + if valid, _ := dbp.Valid(); valid { + for _, thread := range dbp.threads { + if err := thread.resume(); err != nil && err != sys.ESRCH { + return err + } + } } } return nil diff --git a/pkg/proc/native/proc_windows.go b/pkg/proc/native/proc_windows.go index 73e4a7a4f9..923390b29a 100644 --- a/pkg/proc/native/proc_windows.go +++ b/pkg/proc/native/proc_windows.go @@ -65,7 +65,7 @@ func Launch(cmd []string, wd string, flags proc.LaunchFlags, _ []string, _ strin tgt, err := dbp.initialize(argv0Go, []string{}) if err != nil { - dbp.Detach(true) + detachWithoutGroup(dbp, true) return nil, err } return tgt, nil @@ -183,7 +183,7 @@ func Attach(pid int, waitFor *proc.WaitFor, _ []string) (*proc.TargetGroup, erro } tgt, err := dbp.initialize(exepath, []string{}) if err != nil { - dbp.Detach(true) + detachWithoutGroup(dbp, true) return nil, err } return tgt, nil @@ -261,7 +261,7 @@ func waitForSearchProcess(pfx string, seen map[int]struct{}) (int, error) { } // kill kills the process. -func (dbp *nativeProcess) kill() error { +func (procgrp *processGroup) kill(dbp *nativeProcess) error { if dbp.exited { return nil } @@ -496,15 +496,12 @@ func trapWait(procgrp *processGroup, pid int) (*nativeThread, error) { return th, nil } -func (dbp *nativeProcess) wait(pid, options int) (int, *sys.WaitStatus, error) { - return 0, nil, fmt.Errorf("not implemented: wait") -} - func (dbp *nativeProcess) exitGuard(err error) error { return err } -func (dbp *nativeProcess) resume() error { +func (procgrp *processGroup) resume() error { + dbp := procgrp.procs[0] for _, thread := range dbp.threads { if thread.CurrentBreakpoint.Breakpoint != nil { if err := thread.StepInstruction(); err != nil { diff --git a/pkg/proc/target.go b/pkg/proc/target.go index a612f9addb..f720a9f972 100644 --- a/pkg/proc/target.go +++ b/pkg/proc/target.go @@ -323,28 +323,6 @@ func (p *Target) SwitchThread(tid int) error { return fmt.Errorf("thread %d does not exist", tid) } -// detach will detach the target from the underlying process. -// This means the debugger will no longer receive events from the process -// we were previously debugging. -// If kill is true then the process will be killed when we detach. -func (t *Target) detach(kill bool) error { - if !kill { - if t.asyncPreemptChanged { - setAsyncPreemptOff(t, t.asyncPreemptOff) - } - for _, bp := range t.Breakpoints().M { - if bp != nil { - err := t.ClearBreakpoint(bp.Addr) - if err != nil { - return err - } - } - } - } - t.StopReason = StopUnknown - return t.proc.Detach(kill) -} - // setAsyncPreemptOff enables or disables async goroutine preemption by // writing the value 'v' to runtime.debug.asyncpreemptoff. // A value of '1' means off, a value of '0' means on. diff --git a/pkg/proc/target_group.go b/pkg/proc/target_group.go index e388310c3c..b391986d12 100644 --- a/pkg/proc/target_group.go +++ b/pkg/proc/target_group.go @@ -97,18 +97,21 @@ func Restart(grp, oldgrp *TargetGroup, discard func(*LogicalBreakpoint, error)) func (grp *TargetGroup) addTarget(p ProcessInternal, pid int, currentThread Thread, path string, stopReason StopReason, cmdline string) (*Target, error) { logger := logflags.DebuggerLogger() + if len(grp.targets) > 0 { + if !grp.followExecEnabled { + logger.Debugf("Detaching from child target (follow-exec disabled) %d %q", pid, cmdline) + return nil, nil + } + if grp.followExecRegex != nil && !grp.followExecRegex.MatchString(cmdline) { + logger.Debugf("Detaching from child target (follow-exec regex not matched) %d %q", pid, cmdline) + return nil, nil + } + } t, err := grp.newTarget(p, pid, currentThread, path, cmdline) if err != nil { return nil, err } t.StopReason = stopReason - if grp.followExecRegex != nil && len(grp.targets) > 0 { - if !grp.followExecRegex.MatchString(cmdline) { - logger.Debugf("Detaching from child target %d %q", t.Pid(), t.CmdLine) - t.detach(false) - return nil, nil - } - } logger.Debugf("Adding target %d %q", t.Pid(), t.CmdLine) if t.partOfGroup { panic("internal error: target is already part of group") @@ -178,7 +181,7 @@ func (grp *TargetGroup) Detach(kill bool) error { if !isvalid { continue } - err := t.detach(kill) + err := grp.detachTarget(t, kill) if err != nil { errs = append(errs, fmt.Sprintf("could not detach process %d: %v", t.Pid(), err)) } @@ -189,6 +192,28 @@ func (grp *TargetGroup) Detach(kill bool) error { return nil } +// detachTarget will detach the target from the underlying process. +// This means the debugger will no longer receive events from the process +// we were previously debugging. +// If kill is true then the process will be killed when we detach. +func (grp *TargetGroup) detachTarget(t *Target, kill bool) error { + if !kill { + if t.asyncPreemptChanged { + setAsyncPreemptOff(t, t.asyncPreemptOff) + } + for _, bp := range t.Breakpoints().M { + if bp != nil { + err := t.ClearBreakpoint(bp.Addr) + if err != nil { + return err + } + } + } + } + t.StopReason = StopUnknown + return grp.procgrp.Detach(t.Pid(), kill) +} + // HasSteppingBreakpoints returns true if any of the targets has stepping breakpoints set. func (grp *TargetGroup) HasSteppingBreakpoints() bool { for _, t := range grp.targets {