From 01b01423aecbcc00110d680dd6ee20ff905d90e7 Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Fri, 26 Nov 2021 17:06:23 +0100 Subject: [PATCH] proc/*: minor miscellaneous code cleanups (#2790) * made Pid a method of Target instead of a method of Process * changed argument of NewTarget to ProcessInternal, since that's the interface that backends have to implement * removed warnings about ProcessInternal since there is no way for users of pkg/proc to access those methods anyway * made RecordingManipulation an optional interface for backends, Target supplies its own dummy implementation when the backend doesn't * inlined small interfaces that only existed to be inlined in proc.Process anyway * removed unused function findExecutable in the Windows and no-native darwin backends * removed (*EvalScope).EvalVariable, an old synonym for EvalExpression --- pkg/proc/core/core.go | 9 +--- pkg/proc/core/core_test.go | 6 +-- pkg/proc/dump.go | 2 +- pkg/proc/eval.go | 5 -- pkg/proc/gdbserial/gdbserver.go | 4 +- pkg/proc/interface.go | 77 +++++++++++++----------------- pkg/proc/native/dump_linux.go | 4 +- pkg/proc/native/nonative_darwin.go | 4 -- pkg/proc/native/proc.go | 49 ++----------------- pkg/proc/native/proc_darwin.go | 2 +- pkg/proc/native/proc_freebsd.go | 2 +- pkg/proc/native/proc_linux.go | 6 +-- pkg/proc/native/proc_windows.go | 6 +-- pkg/proc/proc_test.go | 16 +++---- pkg/proc/scope_test.go | 2 +- pkg/proc/target.go | 63 ++++++++++++++++++++++-- service/debugger/debugger.go | 6 +-- service/test/variables_test.go | 2 +- 18 files changed, 124 insertions(+), 141 deletions(-) diff --git a/pkg/proc/core/core.go b/pkg/proc/core/core.go index 392d9f93..10ff0974 100644 --- a/pkg/proc/core/core.go +++ b/pkg/proc/core/core.go @@ -164,8 +164,6 @@ type process struct { breakpoints proc.BreakpointMap } -var _ proc.ProcessInternal = &process{} - // thread represents a thread in the core file being debugged. type thread struct { th osThread @@ -222,7 +220,7 @@ func OpenCore(corePath, exePath string, debugInfoDirs []string) (*proc.Target, e return nil, ErrNoThreads } - return proc.NewTarget(p, currentThread, proc.NewTargetConfig{ + return proc.NewTarget(p, p.pid, currentThread, proc.NewTargetConfig{ Path: exePath, DebugInfoDirs: debugInfoDirs, DisableAsyncPreempt: false, @@ -459,11 +457,6 @@ func (p *process) Valid() (bool, error) { return true, nil } -// Pid returns the process ID of this process. -func (p *process) Pid() int { - return p.pid -} - // ResumeNotify is a no-op on core files as we cannot // control execution. func (p *process) ResumeNotify(chan<- struct{}) { diff --git a/pkg/proc/core/core_test.go b/pkg/proc/core/core_test.go index 4c39dab5..25c08826 100644 --- a/pkg/proc/core/core_test.go +++ b/pkg/proc/core/core_test.go @@ -297,7 +297,7 @@ func TestCore(t *testing.T) { if mainFrame == nil { t.Fatalf("Couldn't find main in stack %v", panickingStack) } - msg, err := proc.FrameToScope(p, p.Memory(), nil, *mainFrame).EvalVariable("msg", proc.LoadConfig{MaxStringLen: 64}) + msg, err := proc.FrameToScope(p, p.Memory(), nil, *mainFrame).EvalExpression("msg", proc.LoadConfig{MaxStringLen: 64}) if err != nil { t.Fatalf("Couldn't EvalVariable(msg, ...): %v", err) } @@ -429,11 +429,11 @@ mainSearch: scope := proc.FrameToScope(p, p.Memory(), nil, *mainFrame) loadConfig := proc.LoadConfig{FollowPointers: true, MaxVariableRecurse: 1, MaxStringLen: 64, MaxArrayValues: 64, MaxStructFields: -1} - v1, err := scope.EvalVariable("t", loadConfig) + v1, err := scope.EvalExpression("t", loadConfig) assertNoError(err, t, "EvalVariable(t)") assertNoError(v1.Unreadable, t, "unreadable variable 't'") t.Logf("t = %#v\n", v1) - v2, err := scope.EvalVariable("s", loadConfig) + v2, err := scope.EvalExpression("s", loadConfig) assertNoError(err, t, "EvalVariable(s)") assertNoError(v2.Unreadable, t, "unreadable variable 's'") t.Logf("s = %#v\n", v2) diff --git a/pkg/proc/dump.go b/pkg/proc/dump.go index 0cd5dd39..3989dc00 100644 --- a/pkg/proc/dump.go +++ b/pkg/proc/dump.go @@ -155,7 +155,7 @@ func (t *Target) Dump(out elfwriter.WriteCloserSeeker, flags DumpFlags, state *D notes = append(notes, elfwriter.Note{ Type: elfwriter.DelveHeaderNoteType, Name: "Delve Header", - Data: []byte(fmt.Sprintf("%s/%s\n%s\n%s%d\n%s%#x\n", bi.GOOS, bi.Arch.Name, version.DelveVersion.String(), elfwriter.DelveHeaderTargetPidPrefix, t.Pid(), elfwriter.DelveHeaderEntryPointPrefix, entryPoint)), + Data: []byte(fmt.Sprintf("%s/%s\n%s\n%s%d\n%s%#x\n", bi.GOOS, bi.Arch.Name, version.DelveVersion.String(), elfwriter.DelveHeaderTargetPidPrefix, t.pid, elfwriter.DelveHeaderEntryPointPrefix, entryPoint)), }) threads := t.ThreadList() diff --git a/pkg/proc/eval.go b/pkg/proc/eval.go index c5dad939..29218c8e 100644 --- a/pkg/proc/eval.go +++ b/pkg/proc/eval.go @@ -423,11 +423,6 @@ func (scope *EvalScope) setValue(dstv, srcv *Variable, srcExpr string) error { return fmt.Errorf("can not set variables of type %s (not implemented)", dstv.Kind.String()) } -// EvalVariable returns the value of the given expression (backwards compatibility). -func (scope *EvalScope) EvalVariable(name string, cfg LoadConfig) (*Variable, error) { - return scope.EvalExpression(name, cfg) -} - // SetVariable sets the value of the named variable func (scope *EvalScope) SetVariable(name, value string) error { t, err := parser.ParseExpr(name) diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go index b4b09a74..a9445256 100644 --- a/pkg/proc/gdbserial/gdbserver.go +++ b/pkg/proc/gdbserial/gdbserver.go @@ -170,7 +170,7 @@ type gdbProcess struct { onDetach func() // called after a successful detach } -var _ proc.ProcessInternal = &gdbProcess{} +var _ proc.RecordingManipulationInternal = &gdbProcess{} // gdbThread represents an operating system thread. type gdbThread struct { @@ -704,7 +704,7 @@ func (p *gdbProcess) initialize(path string, debugInfoDirs []string, stopReason return nil, err } } - tgt, err := proc.NewTarget(p, p.currentThread, proc.NewTargetConfig{ + tgt, err := proc.NewTarget(p, p.conn.pid, p.currentThread, proc.NewTargetConfig{ Path: path, DebugInfoDirs: debugInfoDirs, DisableAsyncPreempt: runtime.GOOS == "darwin", diff --git a/pkg/proc/interface.go b/pkg/proc/interface.go index e5797e26..e403e1c6 100644 --- a/pkg/proc/interface.go +++ b/pkg/proc/interface.go @@ -13,9 +13,20 @@ import ( // There is one exception to this rule: it is safe to call RequestManualStop // concurrently with ContinueOnce. type Process interface { - Info - ProcessManipulation - RecordingManipulation + // ResumeNotify specifies a channel that will be closed the next time + // ContinueOnce finishes resuming the target. + ResumeNotify(chan<- struct{}) + BinInfo() *BinaryInfo + EntryPoint() (uint64, error) + + // RequestManualStop attempts to stop all the process' threads. + RequestManualStop() error + // CheckAndClearManualStopRequest returns true the first time it's called + // after a call to RequestManualStop. + CheckAndClearManualStopRequest() bool + + FindThread(threadID int) (Thread, bool) + ThreadList() []Thread Breakpoints() *BreakpointMap @@ -23,22 +34,16 @@ type Process interface { Memory() MemoryReadWriter } -// ProcessInternal holds a set of methods that are not meant to be called by -// anyone except for an instance of `proc.Target`. These methods are not -// safe to use by themselves and should never be called directly outside of -// the `proc` package. -// This is temporary and in support of an ongoing refactor. +// ProcessInternal holds a set of methods that need to be implemented by a +// Delve backend. Methods in the Process interface are safe to be called by +// clients of the 'proc' library, while all other methods are only called +// directly within 'proc'. type ProcessInternal interface { + Process // Valid returns true if this Process can be used. When it returns false it // also returns an error describing why the Process is invalid (either // ErrProcessExited or ErrProcessDetached). Valid() (bool, error) - // Restart restarts the recording from the specified position, or from the - // last checkpoint if pos == "". - // If pos starts with 'c' it's a checkpoint ID, otherwise it's an event - // number. - // Returns the new current thread after the restart has completed. - Restart(pos string) (Thread, error) Detach(bool) error ContinueOnce() (trapthread Thread, stopReason StopReason, err error) @@ -47,6 +52,7 @@ type ProcessInternal interface { SupportsBPF() bool SetUProbe(string, int64, []ebpf.UProbeArgMap) error + GetBufferedTracepoints() []ebpf.RawUProbeParams // DumpProcessNotes returns ELF core notes describing the process and its threads. // Implementing this method is optional. @@ -54,8 +60,6 @@ type ProcessInternal interface { // MemoryMap returns the memory map of the target process. This method must be implemented if CanDump is true. MemoryMap() ([]MemoryMapEntry, error) - GetBufferedTracepoints() []ebpf.RawUProbeParams - // StartCallInjection notifies the backend that we are about to inject a function call. StartCallInjection() (func(), error) } @@ -79,6 +83,19 @@ type RecordingManipulation interface { ClearCheckpoint(id int) error } +// RecordingManipulationInternal is an interface that a Delve backend can +// implement if it is a recording. +type RecordingManipulationInternal interface { + RecordingManipulation + + // Restart restarts the recording from the specified position, or from the + // last checkpoint if pos == "". + // If pos starts with 'c' it's a checkpoint ID, otherwise it's an event + // number. + // Returns the new current thread after the restart has completed. + Restart(pos string) (Thread, error) +} + // Direction is the direction of execution for the target process. type Direction int8 @@ -95,31 +112,3 @@ type Checkpoint struct { When string Where string } - -// Info is an interface that provides general information on the target. -type Info interface { - Pid() int - // ResumeNotify specifies a channel that will be closed the next time - // ContinueOnce finishes resuming the target. - ResumeNotify(chan<- struct{}) - BinInfo() *BinaryInfo - EntryPoint() (uint64, error) - - ThreadInfo -} - -// ThreadInfo is an interface for getting information on active threads -// in the process. -type ThreadInfo interface { - FindThread(threadID int) (Thread, bool) - ThreadList() []Thread -} - -// ProcessManipulation is an interface for changing the execution state of a process. -type ProcessManipulation interface { - // RequestManualStop attempts to stop all the process' threads. - RequestManualStop() error - // CheckAndClearManualStopRequest returns true the first time it's called - // after a call to RequestManualStop. - CheckAndClearManualStopRequest() bool -} diff --git a/pkg/proc/native/dump_linux.go b/pkg/proc/native/dump_linux.go index dd38d68a..fc393c45 100644 --- a/pkg/proc/native/dump_linux.go +++ b/pkg/proc/native/dump_linux.go @@ -12,10 +12,10 @@ import ( func (p *nativeProcess) MemoryMap() ([]proc.MemoryMapEntry, error) { const VmFlagsPrefix = "VmFlags:" - smapsbuf, err := ioutil.ReadFile(fmt.Sprintf("/proc/%d/smaps", p.Pid())) + smapsbuf, err := ioutil.ReadFile(fmt.Sprintf("/proc/%d/smaps", p.pid)) if err != nil { // Older versions of Linux don't have smaps but have maps which is in a similar format. - smapsbuf, err = ioutil.ReadFile(fmt.Sprintf("/proc/%d/maps", p.Pid())) + smapsbuf, err = ioutil.ReadFile(fmt.Sprintf("/proc/%d/maps", p.pid)) if err != nil { return nil, err } diff --git a/pkg/proc/native/nonative_darwin.go b/pkg/proc/native/nonative_darwin.go index 73308b0d..0ef1e43a 100644 --- a/pkg/proc/native/nonative_darwin.go +++ b/pkg/proc/native/nonative_darwin.go @@ -37,10 +37,6 @@ type osProcessDetails struct{} func (os *osProcessDetails) Close() {} -func findExecutable(path string, pid int) string { - panic(ErrNativeBackendDisabled) -} - func killProcess(pid int) error { panic(ErrNativeBackendDisabled) } diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index bae9c86a..f5b25cb4 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -46,8 +46,6 @@ type nativeProcess struct { exited, detached bool } -var _ proc.ProcessInternal = &nativeProcess{} - // newProcess returns an initialized Process struct. Before returning, // it will also launch a goroutine in order to handle ptrace(2) // functions. For more information, see the documentation on @@ -72,40 +70,6 @@ func (dbp *nativeProcess) BinInfo() *proc.BinaryInfo { return dbp.bi } -// Recorded always returns false for the native proc backend. -func (dbp *nativeProcess) Recorded() (bool, string) { return false, "" } - -// Restart will always return an error in the native proc backend, only for -// recorded traces. -func (dbp *nativeProcess) Restart(string) (proc.Thread, error) { return nil, proc.ErrNotRecorded } - -// ChangeDirection will always return an error in the native proc backend, only for -// recorded traces. -func (dbp *nativeProcess) ChangeDirection(dir proc.Direction) error { - if dir != proc.Forward { - return proc.ErrNotRecorded - } - return nil -} - -// GetDirection will always return Forward. -func (p *nativeProcess) GetDirection() proc.Direction { return proc.Forward } - -// When will always return an empty string and nil, not supported on native proc backend. -func (dbp *nativeProcess) When() (string, error) { return "", nil } - -// Checkpoint will always return an error on the native proc backend, -// only supported for recorded traces. -func (dbp *nativeProcess) Checkpoint(string) (int, error) { return -1, proc.ErrNotRecorded } - -// Checkpoints will always return an error on the native proc backend, -// only supported for recorded traces. -func (dbp *nativeProcess) Checkpoints() ([]proc.Checkpoint, error) { return nil, proc.ErrNotRecorded } - -// ClearCheckpoint will always return an error on the native proc backend, -// only supported in recorded traces. -func (dbp *nativeProcess) ClearCheckpoint(int) error { return proc.ErrNotRecorded } - // StartCallInjection notifies the backend that we are about to inject a function call. func (dbp *nativeProcess) StartCallInjection() (func(), error) { return func() {}, nil } @@ -143,7 +107,7 @@ func (dbp *nativeProcess) Valid() (bool, error) { return false, proc.ErrProcessDetached } if dbp.exited { - return false, proc.ErrProcessExited{Pid: dbp.Pid()} + return false, proc.ErrProcessExited{Pid: dbp.pid} } return true, nil } @@ -154,11 +118,6 @@ func (dbp *nativeProcess) ResumeNotify(ch chan<- struct{}) { dbp.resumeChan = ch } -// Pid returns the process ID. -func (dbp *nativeProcess) Pid() int { - return dbp.pid -} - // ThreadList returns a list of threads in the process. func (dbp *nativeProcess) ThreadList() []proc.Thread { r := make([]proc.Thread, 0, len(dbp.threads)) @@ -188,7 +147,7 @@ func (dbp *nativeProcess) Breakpoints() *proc.BreakpointMap { // sends SIGSTOP to all threads. func (dbp *nativeProcess) RequestManualStop() error { if dbp.exited { - return proc.ErrProcessExited{Pid: dbp.Pid()} + return proc.ErrProcessExited{Pid: dbp.pid} } dbp.stopMu.Lock() defer dbp.stopMu.Unlock() @@ -245,7 +204,7 @@ func (dbp *nativeProcess) EraseBreakpoint(bp *proc.Breakpoint) error { // This could be the result of a breakpoint or signal. func (dbp *nativeProcess) ContinueOnce() (proc.Thread, proc.StopReason, error) { if dbp.exited { - return nil, proc.StopExited, proc.ErrProcessExited{Pid: dbp.Pid()} + return nil, proc.StopExited, proc.ErrProcessExited{Pid: dbp.pid} } for { @@ -306,7 +265,7 @@ func (dbp *nativeProcess) initialize(path string, debugInfoDirs []string) (*proc if !dbp.childProcess { stopReason = proc.StopAttached } - tgt, err := proc.NewTarget(dbp, dbp.memthread, proc.NewTargetConfig{ + tgt, err := proc.NewTarget(dbp, dbp.pid, dbp.memthread, proc.NewTargetConfig{ Path: path, DebugInfoDirs: debugInfoDirs, DisableAsyncPreempt: runtime.GOOS == "windows" || runtime.GOOS == "freebsd", diff --git a/pkg/proc/native/proc_darwin.go b/pkg/proc/native/proc_darwin.go index 4be4462e..74e176be 100644 --- a/pkg/proc/native/proc_darwin.go +++ b/pkg/proc/native/proc_darwin.go @@ -435,7 +435,7 @@ func (dbp *nativeProcess) resume() error { // stop stops all running threads and sets breakpoints func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) { if dbp.exited { - return nil, proc.ErrProcessExited{Pid: dbp.Pid()} + return nil, proc.ErrProcessExited{Pid: dbp.pid} } for _, th := range dbp.threads { if !th.Stopped() { diff --git a/pkg/proc/native/proc_freebsd.go b/pkg/proc/native/proc_freebsd.go index d7fb5d5b..c2adc73e 100644 --- a/pkg/proc/native/proc_freebsd.go +++ b/pkg/proc/native/proc_freebsd.go @@ -352,7 +352,7 @@ func (dbp *nativeProcess) resume() error { // stop stops all running threads and sets breakpoints func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) { if dbp.exited { - return nil, proc.ErrProcessExited{Pid: dbp.Pid()} + return nil, proc.ErrProcessExited{Pid: dbp.pid} } // set breakpoints on all threads for _, th := range dbp.threads { diff --git a/pkg/proc/native/proc_linux.go b/pkg/proc/native/proc_linux.go index 848d4fbf..371735fe 100644 --- a/pkg/proc/native/proc_linux.go +++ b/pkg/proc/native/proc_linux.go @@ -540,7 +540,7 @@ func (dbp *nativeProcess) resume() error { // stop stops all running threads and sets breakpoints func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) { if dbp.exited { - return nil, proc.ErrProcessExited{Pid: dbp.Pid()} + return nil, proc.ErrProcessExited{Pid: dbp.pid} } for _, th := range dbp.threads { @@ -772,7 +772,7 @@ func (dbp *nativeProcess) SetUProbe(fnName string, goidOffset int64, args []ebpf if err != nil { return err } - err = dbp.os.ebpf.AttachUprobe(dbp.Pid(), debugname, off) + err = dbp.os.ebpf.AttachUprobe(dbp.pid, debugname, off) if err != nil { return err } @@ -783,7 +783,7 @@ func (dbp *nativeProcess) SetUProbe(fnName string, goidOffset int64, args []ebpf return err } - return dbp.os.ebpf.AttachUprobe(dbp.Pid(), debugname, off) + return dbp.os.ebpf.AttachUprobe(dbp.pid, debugname, off) } func killProcess(pid int) error { diff --git a/pkg/proc/native/proc_windows.go b/pkg/proc/native/proc_windows.go index a53b918f..5b8c3d35 100644 --- a/pkg/proc/native/proc_windows.go +++ b/pkg/proc/native/proc_windows.go @@ -235,10 +235,6 @@ func (dbp *nativeProcess) addThread(hThread syscall.Handle, threadID int, attach return thread, nil } -func findExecutable(path string, pid int) string { - return path -} - type waitForDebugEventFlags int const ( @@ -428,7 +424,7 @@ func (dbp *nativeProcess) resume() error { // stop stops all running threads threads and sets breakpoints func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) { if dbp.exited { - return nil, proc.ErrProcessExited{Pid: dbp.Pid()} + return nil, proc.ErrProcessExited{Pid: dbp.pid} } dbp.os.running = false diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 30251769..a9fad724 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -1164,7 +1164,7 @@ func evalVariableOrError(p *proc.Target, symbol string) (*proc.Variable, error) if err != nil { return nil, err } - return scope.EvalVariable(symbol, normalLoadConfig) + return scope.EvalExpression(symbol, normalLoadConfig) } func evalVariable(p *proc.Target, t testing.TB, symbol string) *proc.Variable { @@ -1310,7 +1310,7 @@ func TestFrameEvaluation(t *testing.T) { scope, err := proc.ConvertEvalScope(p, g.ID, frame, 0) assertNoError(err, t, "ConvertEvalScope()") t.Logf("scope = %v", scope) - v, err := scope.EvalVariable("i", normalLoadConfig) + v, err := scope.EvalExpression("i", normalLoadConfig) t.Logf("v = %v", v) if err != nil { t.Logf("Goroutine %d: %v\n", g.ID, err) @@ -1338,7 +1338,7 @@ func TestFrameEvaluation(t *testing.T) { for i := 0; i <= 3; i++ { scope, err := proc.ConvertEvalScope(p, g.ID, i+1, 0) assertNoError(err, t, fmt.Sprintf("ConvertEvalScope() on frame %d", i+1)) - v, err := scope.EvalVariable("n", normalLoadConfig) + v, err := scope.EvalExpression("n", normalLoadConfig) assertNoError(err, t, fmt.Sprintf("EvalVariable() on frame %d", i+1)) n, _ := constant.Int64Val(v.Value) t.Logf("frame %d n %d\n", i+1, n) @@ -1367,7 +1367,7 @@ func TestThreadFrameEvaluation(t *testing.T) { // be a thread scope. scope, err := proc.ConvertEvalScope(p, 0, 0, 0) assertNoError(err, t, "ConvertEvalScope() on frame 0") - _, err = scope.EvalVariable("s", normalLoadConfig) + _, err = scope.EvalExpression("s", normalLoadConfig) assertNoError(err, t, "EvalVariable(\"s\") on frame 0") }) } @@ -1520,10 +1520,10 @@ func TestBreakpointCountsWithDetection(t *testing.T) { } scope, err := proc.GoroutineScope(p, th) assertNoError(err, t, "Scope()") - v, err := scope.EvalVariable("i", normalLoadConfig) + v, err := scope.EvalExpression("i", normalLoadConfig) assertNoError(err, t, "evalVariable") i, _ := constant.Int64Val(v.Value) - v, err = scope.EvalVariable("id", normalLoadConfig) + v, err = scope.EvalExpression("id", normalLoadConfig) assertNoError(err, t, "evalVariable") id, _ := constant.Int64Val(v.Value) m[id] = i @@ -4253,11 +4253,11 @@ func TestReadDeferArgs(t *testing.T) { } } - avar, err := scope.EvalVariable("a", normalLoadConfig) + avar, err := scope.EvalExpression("a", normalLoadConfig) if err != nil { t.Fatal(err) } - bvar, err := scope.EvalVariable("b", normalLoadConfig) + bvar, err := scope.EvalExpression("b", normalLoadConfig) if err != nil { t.Fatal(err) } diff --git a/pkg/proc/scope_test.go b/pkg/proc/scope_test.go index 03a4cf0e..f95a3711 100644 --- a/pkg/proc/scope_test.go +++ b/pkg/proc/scope_test.go @@ -285,7 +285,7 @@ func (check *scopeCheck) checkVar(v *proc.Variable, t *testing.T) { } func (varCheck *varCheck) checkInScope(line int, scope *proc.EvalScope, t *testing.T) { - v, err := scope.EvalVariable(varCheck.name, normalLoadConfig) + v, err := scope.EvalExpression(varCheck.name, normalLoadConfig) assertNoError(err, t, fmt.Sprintf("EvalVariable(%s)", varCheck.name)) varCheck.check(line, v, t, "EvalExpression") diff --git a/pkg/proc/target.go b/pkg/proc/target.go index dc503e19..66e61d71 100644 --- a/pkg/proc/target.go +++ b/pkg/proc/target.go @@ -36,8 +36,12 @@ const ( // Target represents the process being debugged. type Target struct { Process + RecordingManipulation - proc ProcessInternal + proc ProcessInternal + recman RecordingManipulationInternal + + pid int // StopReason describes the reason why the target process is stopped. // A process could be stopped for multiple simultaneous reasons, in which @@ -169,7 +173,8 @@ func DisableAsyncPreemptEnv() []string { } // NewTarget returns an initialized Target object. -func NewTarget(p Process, currentThread Thread, cfg NewTargetConfig) (*Target, error) { +// The p argument can optionally implement the RecordingManipulation interface. +func NewTarget(p ProcessInternal, pid int, currentThread Thread, cfg NewTargetConfig) (*Target, error) { entryPoint, err := p.EntryPoint() if err != nil { return nil, err @@ -187,13 +192,21 @@ func NewTarget(p Process, currentThread Thread, cfg NewTargetConfig) (*Target, e t := &Target{ Process: p, - proc: p.(ProcessInternal), + proc: p, fncallForG: make(map[int]*callInjection), StopReason: cfg.StopReason, currentThread: currentThread, CanDump: cfg.CanDump, + pid: pid, } + if recman, ok := p.(RecordingManipulationInternal); ok { + t.recman = recman + } else { + t.recman = &dummyRecordingManipulation{} + } + t.RecordingManipulation = t.recman + g, _ := GetG(currentThread) t.selectedGoroutine = g @@ -210,6 +223,11 @@ func NewTarget(p Process, currentThread Thread, cfg NewTargetConfig) (*Target, e return t, nil } +// Pid returns the pid of the target process. +func (t *Target) Pid() int { + return t.pid +} + // IsCgo returns the value of runtime.iscgo func (t *Target) IsCgo() bool { if t.iscgo != nil { @@ -265,7 +283,7 @@ func (t *Target) ClearCaches() { // Restarting of a normal process happens at a higher level (debugger.Restart). func (t *Target) Restart(from string) error { t.ClearCaches() - currentThread, err := t.proc.Restart(from) + currentThread, err := t.recman.Restart(from) if err != nil { return err } @@ -544,3 +562,40 @@ func (t *Target) dwrapUnwrap(fn *Function) *Function { } return fn } + +type dummyRecordingManipulation struct { +} + +// Recorded always returns false for the native proc backend. +func (*dummyRecordingManipulation) Recorded() (bool, string) { return false, "" } + +// ChangeDirection will always return an error in the native proc backend, only for +// recorded traces. +func (*dummyRecordingManipulation) ChangeDirection(dir Direction) error { + if dir != Forward { + return ErrNotRecorded + } + return nil +} + +// GetDirection will always return Forward. +func (*dummyRecordingManipulation) GetDirection() Direction { return Forward } + +// When will always return an empty string and nil, not supported on native proc backend. +func (*dummyRecordingManipulation) When() (string, error) { return "", nil } + +// Checkpoint will always return an error on the native proc backend, +// only supported for recorded traces. +func (*dummyRecordingManipulation) Checkpoint(string) (int, error) { return -1, ErrNotRecorded } + +// Checkpoints will always return an error on the native proc backend, +// only supported for recorded traces. +func (*dummyRecordingManipulation) Checkpoints() ([]Checkpoint, error) { return nil, ErrNotRecorded } + +// ClearCheckpoint will always return an error on the native proc backend, +// only supported in recorded traces. +func (*dummyRecordingManipulation) ClearCheckpoint(int) error { return ErrNotRecorded } + +// Restart will always return an error in the native proc backend, only for +// recorded traces. +func (*dummyRecordingManipulation) Restart(string) (Thread, error) { return nil, ErrNotRecorded } diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index 055eaf17..24fbae0a 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -1332,7 +1332,7 @@ func (d *Debugger) collectBreakpointInformation(state *api.DebuggerState) error bpi.Variables = make([]api.Variable, len(bp.Variables)) } for i := range bp.Variables { - v, err := s.EvalVariable(bp.Variables[i], proc.LoadConfig{FollowPointers: true, MaxVariableRecurse: 1, MaxStringLen: 64, MaxArrayValues: 64, MaxStructFields: -1}) + v, err := s.EvalExpression(bp.Variables[i], proc.LoadConfig{FollowPointers: true, MaxVariableRecurse: 1, MaxStringLen: 64, MaxArrayValues: 64, MaxStructFields: -1}) if err != nil { bpi.Variables[i] = api.Variable{Name: bp.Variables[i], Unreadable: fmt.Sprintf("eval error: %v", err)} } else { @@ -1516,7 +1516,7 @@ func (d *Debugger) Function(goid, frame, deferredCall int, cfg proc.LoadConfig) // EvalVariableInScope will attempt to evaluate the variable represented by 'symbol' // in the scope provided. -func (d *Debugger) EvalVariableInScope(goid, frame, deferredCall int, symbol string, cfg proc.LoadConfig) (*proc.Variable, error) { +func (d *Debugger) EvalVariableInScope(goid, frame, deferredCall int, expr string, cfg proc.LoadConfig) (*proc.Variable, error) { d.targetMutex.Lock() defer d.targetMutex.Unlock() @@ -1524,7 +1524,7 @@ func (d *Debugger) EvalVariableInScope(goid, frame, deferredCall int, symbol str if err != nil { return nil, err } - return s.EvalVariable(symbol, cfg) + return s.EvalExpression(expr, cfg) } // LoadResliced will attempt to 'reslice' a map, array or slice so that the values diff --git a/service/test/variables_test.go b/service/test/variables_test.go index 0b67aa6c..caffbf6e 100644 --- a/service/test/variables_test.go +++ b/service/test/variables_test.go @@ -101,7 +101,7 @@ func evalVariable(p *proc.Target, symbol string, cfg proc.LoadConfig) (*proc.Var return nil, err } - return scope.EvalVariable(symbol, cfg) + return scope.EvalExpression(symbol, cfg) } func (tc *varTest) alternateVarTest() varTest {