diff --git a/pkg/proc/breakpoints.go b/pkg/proc/breakpoints.go index bcd7cb27..443f54b9 100644 --- a/pkg/proc/breakpoints.go +++ b/pkg/proc/breakpoints.go @@ -422,11 +422,11 @@ func (rbpi *returnBreakpointInfo) Collect(thread Thread) []*Variable { g, err := GetG(thread) if err != nil { - return returnInfoError("could not get g", err, thread) + return returnInfoError("could not get g", err, thread.ProcessMemory()) } scope, err := GoroutineScope(thread) if err != nil { - return returnInfoError("could not get scope", err, thread) + return returnInfoError("could not get scope", err, thread.ProcessMemory()) } v, err := scope.evalAST(rbpi.retFrameCond) if err != nil || v.Unreadable != nil || v.Kind != reflect.Bool { @@ -444,12 +444,12 @@ func (rbpi *returnBreakpointInfo) Collect(thread Thread) []*Variable { oldSP := uint64(rbpi.spOffset + int64(g.stack.hi)) err = fakeFunctionEntryScope(scope, rbpi.fn, oldFrameOffset, oldSP) if err != nil { - return returnInfoError("could not read function entry", err, thread) + return returnInfoError("could not read function entry", err, thread.ProcessMemory()) } vars, err := scope.Locals() if err != nil { - return returnInfoError("could not evaluate return variables", err, thread) + return returnInfoError("could not evaluate return variables", err, thread.ProcessMemory()) } vars = filterVariables(vars, func(v *Variable) bool { return (v.Flags & VariableReturnArgument) != 0 diff --git a/pkg/proc/core/core.go b/pkg/proc/core/core.go index abb05b78..93bb277d 100644 --- a/pkg/proc/core/core.go +++ b/pkg/proc/core/core.go @@ -154,9 +154,8 @@ type process struct { entryPoint uint64 - bi *proc.BinaryInfo - breakpoints proc.BreakpointMap - currentThread *thread + bi *proc.BinaryInfo + breakpoints proc.BreakpointMap } var _ proc.ProcessInternal = &process{} @@ -188,7 +187,7 @@ var ( ErrChangeRegisterCore = errors.New("can not change register values of core process") ) -type openFn func(string, string) (*process, error) +type openFn func(string, string) (*process, proc.Thread, error) var openFns = []openFn{readLinuxCore, readAMD64Minidump} @@ -201,9 +200,10 @@ var ErrUnrecognizedFormat = errors.New("unrecognized core format") // for external debug files in the directories passed in. func OpenCore(corePath, exePath string, debugInfoDirs []string) (*proc.Target, error) { var p *process + var currentThread proc.Thread var err error for _, openFn := range openFns { - p, err = openFn(corePath, exePath) + p, currentThread, err = openFn(corePath, exePath) if err != ErrUnrecognizedFormat { break } @@ -212,7 +212,7 @@ func OpenCore(corePath, exePath string, debugInfoDirs []string) (*proc.Target, e return nil, err } - return proc.NewTarget(p, proc.NewTargetConfig{ + return proc.NewTarget(p, currentThread, proc.NewTargetConfig{ Path: exePath, DebugInfoDirs: debugInfoDirs, DisableAsyncPreempt: false, @@ -239,7 +239,7 @@ func (p *process) WriteBreakpoint(addr uint64) (file string, line int, fn *proc. func (p *process) Recorded() (bool, string) { return true, "" } // Restart will only return an error for core files, as they are not executing. -func (p *process) Restart(string) error { return ErrContinueCore } +func (p *process) Restart(string) (proc.Thread, error) { return nil, ErrContinueCore } // ChangeDirection will only return an error as you cannot continue a core process. func (p *process) ChangeDirection(proc.Direction) error { return ErrContinueCore } @@ -262,8 +262,8 @@ func (p *process) ClearCheckpoint(int) error { return errors.New("checkpoint not // ReadMemory will return memory from the core file at the specified location and put the // read memory into `data`, returning the length read, and returning an error if // the length read is shorter than the length of the `data` buffer. -func (t *thread) ReadMemory(data []byte, addr uint64) (n int, err error) { - n, err = t.p.mem.ReadMemory(data, addr) +func (p *process) ReadMemory(data []byte, addr uint64) (n int, err error) { + n, err = p.mem.ReadMemory(data, addr) if err == nil && n != len(data) { err = ErrShortRead } @@ -272,10 +272,15 @@ func (t *thread) ReadMemory(data []byte, addr uint64) (n int, err error) { // WriteMemory will only return an error for core files, you cannot write // to the memory of a core process. -func (t *thread) WriteMemory(addr uint64, data []byte) (int, error) { +func (p *process) WriteMemory(addr uint64, data []byte) (int, error) { return 0, ErrWriteCore } +// ProcessMemory returns the memory of this thread's process. +func (t *thread) ProcessMemory() proc.MemoryReadWriter { + return t.p +} + // Location returns the location of this thread based on // the value of the instruction pointer register. func (t *thread) Location() (*proc.Location, error) { @@ -400,9 +405,9 @@ func (p *process) CheckAndClearManualStopRequest() bool { return false } -// CurrentThread returns the current active thread. -func (p *process) CurrentThread() proc.Thread { - return p.currentThread +// Memory returns the process memory. +func (p *process) Memory() proc.MemoryReadWriter { + return p } // Detach will always return nil and have no @@ -442,8 +447,3 @@ func (p *process) FindThread(threadID int) (proc.Thread, bool) { t, ok := p.Threads[threadID] return t, ok } - -// SetCurrentThread is used internally by proc.Target to change the current thread. -func (p *process) SetCurrentThread(th proc.Thread) { - p.currentThread = th.(*thread) -} diff --git a/pkg/proc/core/core_test.go b/pkg/proc/core/core_test.go index 43e1ebcd..5a0dca5c 100644 --- a/pkg/proc/core/core_test.go +++ b/pkg/proc/core/core_test.go @@ -244,7 +244,7 @@ func TestCore(t *testing.T) { if mainFrame == nil { t.Fatalf("Couldn't find main in stack %v", panickingStack) } - msg, err := proc.FrameToScope(p.BinInfo(), p.CurrentThread(), nil, *mainFrame).EvalVariable("msg", proc.LoadConfig{MaxStringLen: 64}) + msg, err := proc.FrameToScope(p.BinInfo(), p.Memory(), nil, *mainFrame).EvalVariable("msg", proc.LoadConfig{MaxStringLen: 64}) if err != nil { t.Fatalf("Couldn't EvalVariable(msg, ...): %v", err) } @@ -371,7 +371,7 @@ mainSearch: t.Fatal("could not find main.main frame") } - scope := proc.FrameToScope(p.BinInfo(), p.CurrentThread(), nil, *mainFrame) + scope := proc.FrameToScope(p.BinInfo(), p.Memory(), nil, *mainFrame) loadConfig := proc.LoadConfig{FollowPointers: true, MaxVariableRecurse: 1, MaxStringLen: 64, MaxArrayValues: 64, MaxStructFields: -1} v1, err := scope.EvalVariable("t", loadConfig) assertNoError(err, t, "EvalVariable(t)") diff --git a/pkg/proc/core/linux_core.go b/pkg/proc/core/linux_core.go index 77c5e108..6861451a 100644 --- a/pkg/proc/core/linux_core.go +++ b/pkg/proc/core/linux_core.go @@ -42,7 +42,8 @@ const ( const elfErrorBadMagicNumber = "bad magic number" -func linuxThreadsFromNotes(p *process, notes []*note, machineType elf.Machine) { +func linuxThreadsFromNotes(p *process, notes []*note, machineType elf.Machine) proc.Thread { + var currentThread proc.Thread var lastThreadAMD *linuxAMD64Thread var lastThreadARM *linuxARM64Thread for _, note := range notes { @@ -52,15 +53,15 @@ func linuxThreadsFromNotes(p *process, notes []*note, machineType elf.Machine) { t := note.Desc.(*linuxPrStatusAMD64) lastThreadAMD = &linuxAMD64Thread{linutil.AMD64Registers{Regs: &t.Reg}, t} p.Threads[int(t.Pid)] = &thread{lastThreadAMD, p, proc.CommonThread{}} - if p.currentThread == nil { - p.currentThread = p.Threads[int(t.Pid)] + if currentThread == nil { + currentThread = p.Threads[int(t.Pid)] } } else if machineType == _EM_AARCH64 { t := note.Desc.(*linuxPrStatusARM64) lastThreadARM = &linuxARM64Thread{linutil.ARM64Registers{Regs: &t.Reg}, t} p.Threads[int(t.Pid)] = &thread{lastThreadARM, p, proc.CommonThread{}} - if p.currentThread == nil { - p.currentThread = p.Threads[int(t.Pid)] + if currentThread == nil { + currentThread = p.Threads[int(t.Pid)] } } case _NT_FPREGSET: @@ -79,6 +80,7 @@ func linuxThreadsFromNotes(p *process, notes []*note, machineType elf.Machine) { p.pid = int(note.Desc.(*linuxPrPsInfo).Pid) } } + return currentThread } // readLinuxCore reads a core file from corePath corresponding to the executable at @@ -87,35 +89,35 @@ func linuxThreadsFromNotes(p *process, notes []*note, machineType elf.Machine) { // http://uhlo.blogspot.fr/2012/05/brief-look-into-core-dumps.html, // elf_core_dump in http://lxr.free-electrons.com/source/fs/binfmt_elf.c, // and, if absolutely desperate, readelf.c from the binutils source. -func readLinuxCore(corePath, exePath string) (*process, error) { +func readLinuxCore(corePath, exePath string) (*process, proc.Thread, error) { coreFile, err := elf.Open(corePath) if err != nil { if _, isfmterr := err.(*elf.FormatError); isfmterr && (strings.Contains(err.Error(), elfErrorBadMagicNumber) || strings.Contains(err.Error(), " at offset 0x0: too short")) { // Go >=1.11 and <1.11 produce different errors when reading a non-elf file. - return nil, ErrUnrecognizedFormat + return nil, nil, ErrUnrecognizedFormat } - return nil, err + return nil, nil, err } exe, err := os.Open(exePath) if err != nil { - return nil, err + return nil, nil, err } exeELF, err := elf.NewFile(exe) if err != nil { - return nil, err + return nil, nil, err } if coreFile.Type != elf.ET_CORE { - return nil, fmt.Errorf("%v is not a core file", coreFile) + return nil, nil, fmt.Errorf("%v is not a core file", coreFile) } if exeELF.Type != elf.ET_EXEC && exeELF.Type != elf.ET_DYN { - return nil, fmt.Errorf("%v is not an exe file", exeELF) + return nil, nil, fmt.Errorf("%v is not an exe file", exeELF) } machineType := exeELF.Machine notes, err := readNotes(coreFile, machineType) if err != nil { - return nil, err + return nil, nil, err } memory := buildMemory(coreFile, exeELF, exe, notes) @@ -127,7 +129,7 @@ func readLinuxCore(corePath, exePath string) (*process, error) { case _EM_AARCH64: bi = proc.NewBinaryInfo("linux", "arm64") default: - return nil, fmt.Errorf("unsupported machine type") + return nil, nil, fmt.Errorf("unsupported machine type") } entryPoint := findEntryPoint(notes, bi.Arch.PtrSize()) @@ -140,8 +142,8 @@ func readLinuxCore(corePath, exePath string) (*process, error) { breakpoints: proc.NewBreakpointMap(), } - linuxThreadsFromNotes(p, notes, machineType) - return p, nil + currentThread := linuxThreadsFromNotes(p, notes, machineType) + return p, currentThread, nil } type linuxAMD64Thread struct { diff --git a/pkg/proc/core/windows_amd64_minidump.go b/pkg/proc/core/windows_amd64_minidump.go index 554a66f4..aeadd11a 100644 --- a/pkg/proc/core/windows_amd64_minidump.go +++ b/pkg/proc/core/windows_amd64_minidump.go @@ -7,7 +7,7 @@ import ( "github.com/go-delve/delve/pkg/proc/winutil" ) -func readAMD64Minidump(minidumpPath, exePath string) (*process, error) { +func readAMD64Minidump(minidumpPath, exePath string) (*process, proc.Thread, error) { var logfn func(string, ...interface{}) if logflags.Minidump() { logfn = logflags.MinidumpLogger().Infof @@ -16,9 +16,9 @@ func readAMD64Minidump(minidumpPath, exePath string) (*process, error) { mdmp, err := minidump.Open(minidumpPath, logfn) if err != nil { if _, isNotAMinidump := err.(minidump.ErrNotAMinidump); isNotAMinidump { - return nil, ErrUnrecognizedFormat + return nil, nil, ErrUnrecognizedFormat } - return nil, err + return nil, nil, err } memory := &splicedMemory{} @@ -45,11 +45,12 @@ func readAMD64Minidump(minidumpPath, exePath string) (*process, error) { for i := range mdmp.Threads { th := &mdmp.Threads[i] p.Threads[int(th.ID)] = &thread{&windowsAMD64Thread{th}, p, proc.CommonThread{}} - if p.currentThread == nil { - p.currentThread = p.Threads[int(th.ID)] - } } - return p, nil + var currentThread proc.Thread + if len(mdmp.Threads) > 0 { + currentThread = p.Threads[int(mdmp.Threads[0].ID)] + } + return p, currentThread, nil } type windowsAMD64Thread struct { diff --git a/pkg/proc/disasm.go b/pkg/proc/disasm.go index 808a11e9..8d5d50b9 100644 --- a/pkg/proc/disasm.go +++ b/pkg/proc/disasm.go @@ -71,7 +71,7 @@ type opcodeSeq []uint64 // If sameline is set firstPCAfterPrologueDisassembly will always return an // address associated with the same line as fn.Entry func firstPCAfterPrologueDisassembly(p Process, fn *Function, sameline bool) (uint64, error) { - var mem MemoryReadWriter = p.CurrentThread() + mem := p.Memory() breakpoints := p.Breakpoints() bi := p.BinInfo() text, err := disassemble(mem, nil, breakpoints, bi, fn.Entry, fn.End, false) diff --git a/pkg/proc/eval.go b/pkg/proc/eval.go index 6563e275..07c1608d 100644 --- a/pkg/proc/eval.go +++ b/pkg/proc/eval.go @@ -74,13 +74,6 @@ func ConvertEvalScope(dbp *Target, gid, frame, deferCall int) (*EvalScope, error return ThreadScope(ct) } - var thread MemoryReadWriter - if g.Thread == nil { - thread = ct - } else { - thread = g.Thread - } - var opts StacktraceOptions if deferCall > 0 { opts = StacktraceReadDefers @@ -108,7 +101,7 @@ func ConvertEvalScope(dbp *Target, gid, frame, deferCall int) (*EvalScope, error return d.EvalScope(ct) } - return FrameToScope(dbp.BinInfo(), thread, g, locs[frame:]...), nil + return FrameToScope(dbp.BinInfo(), dbp.Memory(), g, locs[frame:]...), nil } // FrameToScope returns a new EvalScope for frames[0]. @@ -145,7 +138,7 @@ func ThreadScope(thread Thread) (*EvalScope, error) { if len(locations) < 1 { return nil, errors.New("could not decode first frame") } - return FrameToScope(thread.BinInfo(), thread, nil, locations...), nil + return FrameToScope(thread.BinInfo(), thread.ProcessMemory(), nil, locations...), nil } // GoroutineScope returns an EvalScope for the goroutine running on the given thread. @@ -161,7 +154,7 @@ func GoroutineScope(thread Thread) (*EvalScope, error) { if err != nil { return nil, err } - return FrameToScope(thread.BinInfo(), thread, g, locations...), nil + return FrameToScope(thread.BinInfo(), thread.ProcessMemory(), g, locations...), nil } // EvalExpression returns the value of the given expression. diff --git a/pkg/proc/fncall.go b/pkg/proc/fncall.go index da7157c2..7ce7ee29 100644 --- a/pkg/proc/fncall.go +++ b/pkg/proc/fncall.go @@ -309,7 +309,7 @@ func evalFunctionCall(scope *EvalScope, node *ast.CallExpr) (*Variable, error) { return nil, err } // write the desired argument frame size at SP-(2*pointer_size) (the extra pointer is the saved PC) - if err := writePointer(bi, thread, regs.SP()-3*uint64(bi.Arch.PtrSize()), uint64(fncall.argFrameSize)); err != nil { + if err := writePointer(bi, scope.Mem, regs.SP()-3*uint64(bi.Arch.PtrSize()), uint64(fncall.argFrameSize)); err != nil { return nil, err } @@ -422,7 +422,7 @@ func callOP(bi *BinaryInfo, thread Thread, regs Registers, callAddr uint64) erro if err := thread.SetSP(sp); err != nil { return err } - if err := writePointer(bi, thread, sp, regs.PC()); err != nil { + if err := writePointer(bi, thread.ProcessMemory(), sp, regs.PC()); err != nil { return err } return thread.SetPC(callAddr) diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go index b1937b86..843e0a30 100644 --- a/pkg/proc/gdbserial/gdbserver.go +++ b/pkg/proc/gdbserial/gdbserver.go @@ -576,7 +576,7 @@ func (p *gdbProcess) initialize(path string, debugInfoDirs []string, stopReason return nil, err } } - tgt, err := proc.NewTarget(p, proc.NewTargetConfig{ + tgt, err := proc.NewTarget(p, p.currentThread, proc.NewTargetConfig{ Path: path, DebugInfoDirs: debugInfoDirs, DisableAsyncPreempt: runtime.GOOS == "darwin", @@ -649,15 +649,9 @@ func (p *gdbProcess) ThreadList() []proc.Thread { return r } -// CurrentThread returns the current active -// selected thread. -func (p *gdbProcess) CurrentThread() proc.Thread { - return p.currentThread -} - -// SetCurrentThread is used internally by proc.Target to change the current thread. -func (p *gdbProcess) SetCurrentThread(th proc.Thread) { - p.currentThread = th.(*gdbThread) +// Memory returns the process memory. +func (p *gdbProcess) Memory() proc.MemoryReadWriter { + return p } const ( @@ -775,6 +769,7 @@ continueLoop: // the signals that are reported here can not be propagated back to the target process. trapthread.sig = 0 } + p.currentThread = trapthread return trapthread, stopReason, err } @@ -937,9 +932,9 @@ func (p *gdbProcess) Detach(kill bool) error { } // Restart will restart the process from the given position. -func (p *gdbProcess) Restart(pos string) error { +func (p *gdbProcess) Restart(pos string) (proc.Thread, error) { if p.tracedir == "" { - return proc.ErrNotRecorded + return nil, proc.ErrNotRecorded } p.exited = false @@ -952,19 +947,19 @@ func (p *gdbProcess) Restart(pos string) error { err := p.conn.restart(pos) if err != nil { - return err + return nil, err } // for some reason we have to send a vCont;c after a vRun to make rr behave // properly, because that's what gdb does. _, _, err = p.conn.resume(nil, nil) if err != nil { - return err + return nil, err } err = p.updateThreadList(&threadUpdater{p: p}) if err != nil { - return err + return nil, err } p.clearThreadSignals() p.clearThreadRegisters() @@ -973,7 +968,7 @@ func (p *gdbProcess) Restart(pos string) error { p.conn.setBreakpoint(addr) } - return p.setCurrentBreakpoints() + return p.currentThread, p.setCurrentBreakpoints() } // When executes the 'when' command for the Mozilla RR backend. @@ -1265,8 +1260,8 @@ func (p *gdbProcess) setCurrentBreakpoints() error { } // ReadMemory will read into 'data' memory at the address provided. -func (t *gdbThread) ReadMemory(data []byte, addr uint64) (n int, err error) { - err = t.p.conn.readMemory(data, addr) +func (p *gdbProcess) ReadMemory(data []byte, addr uint64) (n int, err error) { + err = p.conn.readMemory(data, addr) if err != nil { return 0, err } @@ -1274,8 +1269,12 @@ func (t *gdbThread) ReadMemory(data []byte, addr uint64) (n int, err error) { } // WriteMemory will write into the memory at 'addr' the data provided. -func (t *gdbThread) WriteMemory(addr uint64, data []byte) (written int, err error) { - return t.p.conn.writeMemory(addr, data) +func (p *gdbProcess) WriteMemory(addr uint64, data []byte) (written int, err error) { + return p.conn.writeMemory(addr, data) +} + +func (t *gdbThread) ProcessMemory() proc.MemoryReadWriter { + return t.p } // Location returns the current location of this thread. @@ -1522,18 +1521,18 @@ func (t *gdbThread) reloadGAtPC() error { } savedcode := make([]byte, len(movinstr)) - _, err := t.ReadMemory(savedcode, pc) + _, err := t.p.ReadMemory(savedcode, pc) if err != nil { return err } - _, err = t.WriteMemory(pc, movinstr) + _, err = t.p.WriteMemory(pc, movinstr) if err != nil { return err } defer func() { - _, err0 := t.WriteMemory(pc, savedcode) + _, err0 := t.p.WriteMemory(pc, savedcode) if err == nil { err = err0 } diff --git a/pkg/proc/interface.go b/pkg/proc/interface.go index cd1ad0c8..f785480f 100644 --- a/pkg/proc/interface.go +++ b/pkg/proc/interface.go @@ -13,6 +13,9 @@ type Process interface { RecordingManipulation Breakpoints() *BreakpointMap + + // Memory returns a memory read/writer for this process's memory. + Memory() MemoryReadWriter } // ProcessInternal holds a set of methods that are not meant to be called by @@ -21,12 +24,12 @@ type Process interface { // the `proc` package. // This is temporary and in support of an ongoing refactor. type ProcessInternal interface { - SetCurrentThread(Thread) // 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. - Restart(pos string) error + // 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) @@ -91,7 +94,6 @@ type Info interface { type ThreadInfo interface { FindThread(threadID int) (Thread, bool) ThreadList() []Thread - CurrentThread() Thread } // ProcessManipulation is an interface for changing the execution state of a process. diff --git a/pkg/proc/linutil/dynamic.go b/pkg/proc/linutil/dynamic.go index 46eab4db..2e02a774 100644 --- a/pkg/proc/linutil/dynamic.go +++ b/pkg/proc/linutil/dynamic.go @@ -44,7 +44,7 @@ func readUintRaw(reader io.Reader, order binary.ByteOrder, ptrSize int) (uint64, // dynamicSearchDebug searches for the DT_DEBUG entry in the .dynamic section func dynamicSearchDebug(p proc.Process) (uint64, error) { bi := p.BinInfo() - mem := p.CurrentThread() + mem := p.Memory() dynbuf := make([]byte, bi.ElfDynamicSection.Size) _, err := mem.ReadMemory(dynbuf, bi.ElfDynamicSection.Addr) @@ -73,7 +73,7 @@ func dynamicSearchDebug(p proc.Process) (uint64, error) { func readPtr(p proc.Process, addr uint64) (uint64, error) { ptrbuf := make([]byte, p.BinInfo().Arch.PtrSize()) - _, err := p.CurrentThread().ReadMemory(ptrbuf, addr) + _, err := p.Memory().ReadMemory(ptrbuf, addr) if err != nil { return 0, err } @@ -115,7 +115,7 @@ func readCString(p proc.Process, addr uint64) (string, error) { if addr == 0 { return "", nil } - mem := p.CurrentThread() + mem := p.Memory() buf := make([]byte, 1) r := []byte{} for { diff --git a/pkg/proc/native/nonative_darwin.go b/pkg/proc/native/nonative_darwin.go index 06009a27..9531239c 100644 --- a/pkg/proc/native/nonative_darwin.go +++ b/pkg/proc/native/nonative_darwin.go @@ -81,11 +81,6 @@ func (dbp *nativeProcess) EntryPoint() (uint64, error) { panic(ErrNativeBackendDisabled) } -// Blocked returns true if the thread is blocked -func (t *nativeThread) Blocked() bool { - panic(ErrNativeBackendDisabled) -} - // SetPC sets the value of the PC register. func (t *nativeThread) SetPC(pc uint64) error { panic(ErrNativeBackendDisabled) diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index fdd72061..6509bf54 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -22,8 +22,8 @@ type nativeProcess struct { // List of threads mapped as such: pid -> *Thread threads map[int]*nativeThread - // Active thread - currentThread *nativeThread + // Thread used to read and write memory + memthread *nativeThread os *osProcessDetails firstStart bool @@ -72,7 +72,7 @@ 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) error { return proc.ErrNotRecorded } +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. @@ -166,14 +166,9 @@ func (dbp *nativeProcess) FindThread(threadID int) (proc.Thread, bool) { return th, ok } -// CurrentThread returns the current selected, active thread. -func (dbp *nativeProcess) CurrentThread() proc.Thread { - return dbp.currentThread -} - -// SetCurrentThread is used internally by proc.Target to change the current thread. -func (p *nativeProcess) SetCurrentThread(th proc.Thread) { - p.currentThread = th.(*nativeThread) +// Memory returns the process memory. +func (dbp *nativeProcess) Memory() proc.MemoryReadWriter { + return dbp.memthread } // Breakpoints returns a list of breakpoints currently set. @@ -209,11 +204,11 @@ func (dbp *nativeProcess) WriteBreakpoint(addr uint64) (string, int, *proc.Funct f, l, fn := dbp.bi.PCToLine(uint64(addr)) originalData := make([]byte, dbp.bi.Arch.BreakpointSize()) - _, err := dbp.currentThread.ReadMemory(originalData, addr) + _, err := dbp.memthread.ReadMemory(originalData, addr) if err != nil { return "", 0, nil, nil, err } - if err := dbp.writeSoftwareBreakpoint(dbp.currentThread, addr); err != nil { + if err := dbp.writeSoftwareBreakpoint(dbp.memthread, addr); err != nil { return "", 0, nil, nil, err } @@ -221,7 +216,7 @@ func (dbp *nativeProcess) WriteBreakpoint(addr uint64) (string, int, *proc.Funct } func (dbp *nativeProcess) EraseBreakpoint(bp *proc.Breakpoint) error { - return dbp.currentThread.ClearBreakpoint(bp) + return dbp.memthread.ClearBreakpoint(bp) } // ContinueOnce will continue the target until it stops. @@ -255,6 +250,7 @@ func (dbp *nativeProcess) ContinueOnce() (proc.Thread, proc.StopReason, error) { return nil, proc.StopUnknown, err } if trapthread != nil { + dbp.memthread = trapthread return trapthread, proc.StopUnknown, nil } } @@ -288,7 +284,7 @@ func (dbp *nativeProcess) initialize(path string, debugInfoDirs []string) (*proc if !dbp.childProcess { stopReason = proc.StopAttached } - return proc.NewTarget(dbp, proc.NewTargetConfig{ + return proc.NewTarget(dbp, 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 3df1522e..f44aa312 100644 --- a/pkg/proc/native/proc_darwin.go +++ b/pkg/proc/native/proc_darwin.go @@ -118,7 +118,7 @@ func Launch(cmd []string, wd string, flags proc.LaunchFlags, _ []string, _ strin } dbp.os.initialized = true - dbp.currentThread = trapthread + dbp.memthread = trapthread tgt, err := dbp.initialize(argv0Go, []string{}) if err != nil { @@ -188,7 +188,7 @@ func (dbp *nativeProcess) kill() (err error) { func (dbp *nativeProcess) requestManualStop() (err error) { var ( task = C.mach_port_t(dbp.os.task) - thread = C.mach_port_t(dbp.currentThread.os.threadAct) + thread = C.mach_port_t(dbp.memthread.os.threadAct) exceptionPort = C.mach_port_t(dbp.os.exceptionPort) ) dbp.os.halt = true @@ -267,8 +267,8 @@ func (dbp *nativeProcess) addThread(port int, attach bool) (*nativeThread, error } dbp.threads[port] = thread thread.os.threadAct = C.thread_act_t(port) - if dbp.currentThread == nil { - dbp.currentThread = thread + if dbp.memthread == nil { + dbp.memthread = thread } return thread, nil } diff --git a/pkg/proc/native/proc_freebsd.go b/pkg/proc/native/proc_freebsd.go index 7f5a854f..a2a3ab16 100644 --- a/pkg/proc/native/proc_freebsd.go +++ b/pkg/proc/native/proc_freebsd.go @@ -180,8 +180,8 @@ func (dbp *nativeProcess) addThread(tid int, attach bool) (*nativeThread, error) os: new(osSpecificDetails), } - if dbp.currentThread == nil { - dbp.currentThread = dbp.threads[tid] + if dbp.memthread == nil { + dbp.memthread = dbp.threads[tid] } return dbp.threads[tid], nil diff --git a/pkg/proc/native/proc_linux.go b/pkg/proc/native/proc_linux.go index d85a992a..40255276 100644 --- a/pkg/proc/native/proc_linux.go +++ b/pkg/proc/native/proc_linux.go @@ -253,8 +253,8 @@ func (dbp *nativeProcess) addThread(tid int, attach bool) (*nativeThread, error) dbp: dbp, os: new(osSpecificDetails), } - if dbp.currentThread == nil { - dbp.currentThread = dbp.threads[tid] + if dbp.memthread == nil { + dbp.memthread = dbp.threads[tid] } return dbp.threads[tid], nil } diff --git a/pkg/proc/native/proc_windows.go b/pkg/proc/native/proc_windows.go index f3f86420..d9ee5999 100644 --- a/pkg/proc/native/proc_windows.go +++ b/pkg/proc/native/proc_windows.go @@ -206,8 +206,8 @@ func (dbp *nativeProcess) addThread(hThread syscall.Handle, threadID int, attach } thread.os.hThread = hThread dbp.threads[threadID] = thread - if dbp.currentThread == nil { - dbp.currentThread = dbp.threads[threadID] + if dbp.memthread == nil { + dbp.memthread = dbp.threads[threadID] } if suspendNewThreads { _, err := _SuspendThread(thread.os.hThread) diff --git a/pkg/proc/native/threads.go b/pkg/proc/native/threads.go index 65b5ab75..c15d63bd 100644 --- a/pkg/proc/native/threads.go +++ b/pkg/proc/native/threads.go @@ -175,3 +175,8 @@ func (t *nativeThread) PC() (uint64, error) { } return regs.PC(), nil } + +// ProcessMemory returns this thread's process memory. +func (t *nativeThread) ProcessMemory() proc.MemoryReadWriter { + return t.dbp.Memory() +} diff --git a/pkg/proc/native/threads_darwin.go b/pkg/proc/native/threads_darwin.go index 183ddff9..fe12bede 100644 --- a/pkg/proc/native/threads_darwin.go +++ b/pkg/proc/native/threads_darwin.go @@ -85,25 +85,6 @@ func (t *nativeThread) resume() error { return nil } -func (t *nativeThread) Blocked() bool { - // TODO(dp) cache the func pc to remove this lookup - regs, err := t.Registers() - if err != nil { - return false - } - pc := regs.PC() - fn := t.BinInfo().PCToFunc(pc) - if fn == nil { - return false - } - switch fn.Name { - case "runtime.kevent", "runtime.mach_semaphore_wait", "runtime.usleep", "runtime.mach_semaphore_timedwait": - return true - default: - return false - } -} - // Stopped returns whether the thread is stopped at // the operating system level. func (t *nativeThread) Stopped() bool { diff --git a/pkg/proc/native/threads_freebsd.go b/pkg/proc/native/threads_freebsd.go index 6e7cf427..4af26e1b 100644 --- a/pkg/proc/native/threads_freebsd.go +++ b/pkg/proc/native/threads_freebsd.go @@ -75,17 +75,6 @@ func (t *nativeThread) singleStep() (err error) { return nil } -func (t *nativeThread) Blocked() bool { - loc, err := t.Location() - if err != nil { - return false - } - if loc.Fn != nil && ((loc.Fn.Name == "runtime.futex") || (loc.Fn.Name == "runtime.usleep") || (loc.Fn.Name == "runtime.clone")) { - return true - } - return false -} - func (t *nativeThread) restoreRegisters(savedRegs proc.Registers) error { sr := savedRegs.(*fbsdutil.AMD64Registers) diff --git a/pkg/proc/native/threads_linux.go b/pkg/proc/native/threads_linux.go index 90af3240..d0e978c0 100644 --- a/pkg/proc/native/threads_linux.go +++ b/pkg/proc/native/threads_linux.go @@ -71,19 +71,6 @@ func (t *nativeThread) singleStep() (err error) { } } -func (t *nativeThread) Blocked() bool { - regs, err := t.Registers() - if err != nil { - return false - } - pc := regs.PC() - fn := t.BinInfo().PCToFunc(pc) - if fn != nil && ((fn.Name == "runtime.futex") || (fn.Name == "runtime.usleep") || (fn.Name == "runtime.clone")) { - return true - } - return false -} - func (t *nativeThread) WriteMemory(addr uint64, data []byte) (written int, err error) { if t.dbp.exited { return 0, proc.ErrProcessExited{Pid: t.dbp.pid} diff --git a/pkg/proc/native/threads_windows.go b/pkg/proc/native/threads_windows.go index 610b75c4..887a639d 100644 --- a/pkg/proc/native/threads_windows.go +++ b/pkg/proc/native/threads_windows.go @@ -111,26 +111,6 @@ func (t *nativeThread) resume() error { return err } -func (t *nativeThread) Blocked() bool { - // TODO: Probably incorrect - what are the runtime functions that - // indicate blocking on Windows? - regs, err := t.Registers() - if err != nil { - return false - } - pc := regs.PC() - fn := t.BinInfo().PCToFunc(pc) - if fn == nil { - return false - } - switch fn.Name { - case "runtime.kevent", "runtime.usleep": - return true - default: - return false - } -} - // Stopped returns whether the thread is stopped at the operating system // level. On windows this always returns true. func (t *nativeThread) Stopped() bool { diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index ad2720b9..f33ddac3 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -359,7 +359,7 @@ func TestClearBreakpointBreakpoint(t *testing.T) { _, err := p.ClearBreakpoint(bp.Addr) assertNoError(err, t, "ClearBreakpoint()") - data, err := dataAtAddr(p.CurrentThread(), bp.Addr) + data, err := dataAtAddr(p.Memory(), bp.Addr) assertNoError(err, t, "dataAtAddr") int3 := []byte{0xcc} @@ -949,19 +949,6 @@ func TestStacktraceGoroutine(t *testing.T) { {{10, "main.agoroutine"}}, } - tgtAgoroutineCount := 10 - - if goversion.VersionAfterOrEqual(runtime.Version(), 1, 14) { - // We try to make sure that all goroutines are stopped at a sensible place - // before reading their stacktrace, but due to the nature of the test - // program there is no guarantee that we always find them in a reasonable - // state. - // Asynchronous preemption in Go 1.14 exacerbates this problem, to avoid - // unnecessary flakiness reduce the target count to 9, allowing one - // goroutine to be in a bad state. - tgtAgoroutineCount = 9 - } - protest.AllowRecording(t) withTestProcess("goroutinestackprog", t, func(p *proc.Target, fixture protest.Fixture) { bp := setFunctionBreakpoint(p, t, "main.stacktraceme") @@ -1011,7 +998,7 @@ func TestStacktraceGoroutine(t *testing.T) { t.Fatalf("Main goroutine stack not found %d", mainCount) } - if agoroutineCount < tgtAgoroutineCount { + if agoroutineCount < 10 { t.Fatalf("Goroutine stacks not found (%d)", agoroutineCount) } @@ -1154,7 +1141,7 @@ func evalVariableOrError(p *proc.Target, symbol string) (*proc.Variable, error) var frame proc.Stackframe frame, err = findFirstNonRuntimeFrame(p) if err == nil { - scope = proc.FrameToScope(p.BinInfo(), p.CurrentThread(), nil, frame) + scope = proc.FrameToScope(p.BinInfo(), p.Memory(), nil, frame) } } else { scope, err = proc.GoroutineScope(p.CurrentThread()) @@ -2531,7 +2518,7 @@ func TestStepOnCallPtrInstr(t *testing.T) { regs, err := p.CurrentThread().Registers() assertNoError(err, t, "Registers()") pc := regs.PC() - text, err := proc.Disassemble(p.CurrentThread(), regs, p.Breakpoints(), p.BinInfo(), pc, pc+uint64(p.BinInfo().Arch.MaxInstructionLength())) + text, err := proc.Disassemble(p.Memory(), regs, p.Breakpoints(), p.BinInfo(), pc, pc+uint64(p.BinInfo().Arch.MaxInstructionLength())) assertNoError(err, t, "Disassemble()") if text[0].IsCall() { found = true @@ -3008,9 +2995,6 @@ func TestIssue893(t *testing.T) { if _, ok := err.(*frame.ErrNoFDEForPC); ok { return } - if _, ok := err.(proc.ErrThreadBlocked); ok { - return - } if _, ok := err.(*proc.ErrNoSourceForPC); ok { return } @@ -3041,7 +3025,7 @@ func TestIssue871(t *testing.T) { var frame proc.Stackframe frame, err = findFirstNonRuntimeFrame(p) if err == nil { - scope = proc.FrameToScope(p.BinInfo(), p.CurrentThread(), nil, frame) + scope = proc.FrameToScope(p.BinInfo(), p.Memory(), nil, frame) } } else { scope, err = proc.GoroutineScope(p.CurrentThread()) @@ -3488,7 +3472,7 @@ func TestIssue1034(t *testing.T) { assertNoError(p.Continue(), t, "Continue()") frames, err := p.SelectedGoroutine().Stacktrace(10, 0) assertNoError(err, t, "Stacktrace") - scope := proc.FrameToScope(p.BinInfo(), p.CurrentThread(), nil, frames[2:]...) + scope := proc.FrameToScope(p.BinInfo(), p.Memory(), nil, frames[2:]...) args, _ := scope.FunctionArguments(normalLoadConfig) assertNoError(err, t, "FunctionArguments()") if len(args) > 0 { @@ -3663,7 +3647,7 @@ func TestDisassembleGlobalVars(t *testing.T) { withTestProcess("teststepconcurrent", t, func(p *proc.Target, fixture protest.Fixture) { mainfn := p.BinInfo().LookupFunc["main.main"] regs, _ := p.CurrentThread().Registers() - text, err := proc.Disassemble(p.CurrentThread(), regs, p.Breakpoints(), p.BinInfo(), mainfn.Entry, mainfn.End) + text, err := proc.Disassemble(p.Memory(), regs, p.Breakpoints(), p.BinInfo(), mainfn.Entry, mainfn.End) assertNoError(err, t, "Disassemble") found := false for i := range text { diff --git a/pkg/proc/stack.go b/pkg/proc/stack.go index a6d17ade..4bc454ea 100644 --- a/pkg/proc/stack.go +++ b/pkg/proc/stack.go @@ -100,7 +100,7 @@ func ThreadStacktrace(thread Thread, depth int) ([]Stackframe, error) { return nil, err } so := thread.BinInfo().PCToImage(regs.PC()) - it := newStackIterator(thread.BinInfo(), thread, thread.BinInfo().Arch.RegistersToDwarfRegisters(so.StaticBase, regs), 0, nil, -1, nil, 0) + it := newStackIterator(thread.BinInfo(), thread.ProcessMemory(), thread.BinInfo().Arch.RegistersToDwarfRegisters(so.StaticBase, regs), 0, nil, -1, nil, 0) return it.stacktrace(depth) } return g.Stacktrace(depth, 0) @@ -120,7 +120,7 @@ func (g *G) stackIterator(opts StacktraceOptions) (*stackIterator, error) { } so := bi.PCToImage(regs.PC()) return newStackIterator( - bi, g.Thread, + bi, g.variable.mem, bi.Arch.RegistersToDwarfRegisters(so.StaticBase, regs), g.stack.hi, stkbar, g.stkbarPos, g, opts), nil } diff --git a/pkg/proc/target.go b/pkg/proc/target.go index 1eafc43c..59b91fe7 100644 --- a/pkg/proc/target.go +++ b/pkg/proc/target.go @@ -41,6 +41,9 @@ type Target struct { // case only one will be reported. StopReason StopReason + // currentThread is the thread that will be used by next/step/stepout and to evaluate variables if no goroutine is selected. + currentThread Thread + // Goroutine that will be used by default to set breakpoint, eval variables, etc... // Normally selectedGoroutine is currentThread.GetG, it will not be only if SwitchGoroutine is called with a goroutine that isn't attached to a thread selectedGoroutine *G @@ -108,7 +111,7 @@ func DisableAsyncPreemptEnv() []string { } // NewTarget returns an initialized Target object. -func NewTarget(p Process, cfg NewTargetConfig) (*Target, error) { +func NewTarget(p Process, currentThread Thread, cfg NewTargetConfig) (*Target, error) { entryPoint, err := p.EntryPoint() if err != nil { return nil, err @@ -125,13 +128,14 @@ func NewTarget(p Process, cfg NewTargetConfig) (*Target, error) { } t := &Target{ - Process: p, - proc: p.(ProcessInternal), - fncallForG: make(map[int]*callInjection), - StopReason: cfg.StopReason, + Process: p, + proc: p.(ProcessInternal), + fncallForG: make(map[int]*callInjection), + StopReason: cfg.StopReason, + currentThread: currentThread, } - g, _ := GetG(p.CurrentThread()) + g, _ := GetG(currentThread) t.selectedGoroutine = g t.createUnrecoveredPanicBreakpoint() @@ -171,10 +175,11 @@ func (t *Target) ClearAllGCache() { // Restarting of a normal process happens at a higher level (debugger.Restart). func (t *Target) Restart(from string) error { t.ClearAllGCache() - err := t.proc.Restart(from) + currentThread, err := t.proc.Restart(from) if err != nil { return err } + t.currentThread = currentThread t.selectedGoroutine, _ = GetG(t.CurrentThread()) if from != "" { t.StopReason = StopManual @@ -210,7 +215,7 @@ func (p *Target) SwitchThread(tid int) error { return err } if th, ok := p.FindThread(tid); ok { - p.proc.SetCurrentThread(th) + p.currentThread = th p.selectedGoroutine, _ = GetG(p.CurrentThread()) return nil } @@ -247,7 +252,7 @@ func setAsyncPreemptOff(p *Target, v int64) { return } logger := p.BinInfo().logger - scope := globalScope(p.BinInfo(), p.BinInfo().Images[0], p.CurrentThread()) + scope := globalScope(p.BinInfo(), p.BinInfo().Images[0], p.Memory()) debugv, err := scope.findGlobal("runtime", "debug") if err != nil || debugv.Unreadable != nil { logger.Warnf("could not find runtime/debug variable (or unreadable): %v %v", err, debugv.Unreadable) @@ -295,3 +300,10 @@ func (t *Target) createFatalThrowBreakpoint() { } } } + +// CurrentThread returns the currently selected thread which will be used +// for next/step/stepout and for reading variables, unless a goroutine is +// selected. +func (t *Target) CurrentThread() Thread { + return t.currentThread +} diff --git a/pkg/proc/target_exec.go b/pkg/proc/target_exec.go index 30102819..9aad77c5 100644 --- a/pkg/proc/target_exec.go +++ b/pkg/proc/target_exec.go @@ -142,7 +142,7 @@ func (dbp *Target) Continue() error { if !arch.BreakInstrMovesPC() { bpsize := arch.BreakpointSize() bp := make([]byte, bpsize) - _, err = dbp.CurrentThread().ReadMemory(bp, loc.PC) + _, err = dbp.Memory().ReadMemory(bp, loc.PC) if bytes.Equal(bp, arch.BreakpointInstruction()) { curthread.SetPC(loc.PC + uint64(bpsize)) } @@ -254,7 +254,7 @@ func disassembleCurrentInstruction(p Process, thread Thread, off int64) ([]AsmIn return nil, err } pc := regs.PC() + uint64(off) - return disassemble(thread, regs, p.Breakpoints(), p.BinInfo(), pc, pc+uint64(p.BinInfo().Arch.MaxInstructionLength()), true) + return disassemble(p.Memory(), regs, p.Breakpoints(), p.BinInfo(), pc, pc+uint64(p.BinInfo().Arch.MaxInstructionLength()), true) } // stepInstructionOut repeatedly calls StepInstruction until the current @@ -290,12 +290,8 @@ func (dbp *Target) Step() (err error) { } if err = next(dbp, true, false); err != nil { - switch err.(type) { - case ErrThreadBlocked: // Noop - default: - dbp.ClearInternalBreakpoints() - return - } + _ = dbp.ClearInternalBreakpoints() + return err } if bp := dbp.CurrentThread().Breakpoint().Breakpoint; bp != nil && bp.Kind == StepBreakpoint && dbp.GetDirection() == Backward { @@ -489,10 +485,8 @@ func next(dbp *Target, stepInto, inlinedStepOut bool) error { ext := filepath.Ext(topframe.Current.File) csource := ext != ".go" && ext != ".s" - var thread MemoryReadWriter = curthread var regs Registers if selg != nil && selg.Thread != nil { - thread = selg.Thread regs, err = selg.Thread.Registers() if err != nil { return err @@ -518,13 +512,13 @@ func next(dbp *Target, stepInto, inlinedStepOut bool) error { return nil } - topframe.Ret, err = findCallInstrForRet(dbp, thread, topframe.Ret, retframe.Current.Fn) + topframe.Ret, err = findCallInstrForRet(dbp, dbp.Memory(), topframe.Ret, retframe.Current.Fn) if err != nil { return err } } - text, err := disassemble(thread, regs, dbp.Breakpoints(), dbp.BinInfo(), topframe.Current.Fn.Entry, topframe.Current.Fn.End, false) + text, err := disassemble(dbp.Memory(), regs, dbp.Breakpoints(), dbp.BinInfo(), topframe.Current.Fn.Entry, topframe.Current.Fn.End, false) if err != nil && stepInto { return err } @@ -812,7 +806,7 @@ func skipAutogeneratedWrappersIn(p Process, startfn *Function, startpc uint64) ( // can't exit Go return startfn, startpc } - text, err := Disassemble(p.CurrentThread(), nil, p.Breakpoints(), p.BinInfo(), fn.Entry, fn.End) + text, err := Disassemble(p.Memory(), nil, p.Breakpoints(), p.BinInfo(), fn.Entry, fn.End) if err != nil { break } @@ -876,9 +870,6 @@ func skipAutogeneratedWrappersOut(g *G, thread Thread, startTopframe, startRetfr var err error var frames []Stackframe if g == nil { - if thread.Blocked() { - return - } frames, err = ThreadStacktrace(thread, maxSkipAutogeneratedWrappers) } else { frames, err = g.Stacktrace(maxSkipAutogeneratedWrappers, 0) @@ -961,7 +952,7 @@ func stepOutReverse(p *Target, topframe, retframe Stackframe, sameGCond ast.Expr curthread = selg.Thread } - callerText, err := disassemble(curthread, nil, p.Breakpoints(), p.BinInfo(), retframe.Current.Fn.Entry, retframe.Current.Fn.End, false) + callerText, err := disassemble(p.Memory(), nil, p.Breakpoints(), p.BinInfo(), retframe.Current.Fn.Entry, retframe.Current.Fn.End, false) if err != nil { return err } @@ -969,9 +960,7 @@ func stepOutReverse(p *Target, topframe, retframe Stackframe, sameGCond ast.Expr var frames []Stackframe if selg == nil { - if !curthread.Blocked() { - frames, err = ThreadStacktrace(curthread, 3) - } + frames, err = ThreadStacktrace(curthread, 3) } else { frames, err = selg.Stacktrace(3, 0) } @@ -985,14 +974,14 @@ func stepOutReverse(p *Target, topframe, retframe Stackframe, sameGCond ast.Expr if len(frames) < 4 || frames[3].Current.Fn == nil { return &ErrNoSourceForPC{frames[2].Current.PC} } - callpc, err = findCallInstrForRet(p, curthread, frames[2].Ret, frames[3].Current.Fn) + callpc, err = findCallInstrForRet(p, p.Memory(), frames[2].Ret, frames[3].Current.Fn) if err != nil { return err } } else if ok, pc := isDeferReturnCall(frames, deferReturns); ok { callpc = pc } else { - callpc, err = findCallInstrForRet(p, curthread, topframe.Ret, retframe.Current.Fn) + callpc, err = findCallInstrForRet(p, p.Memory(), topframe.Ret, retframe.Current.Fn) if err != nil { return err } diff --git a/pkg/proc/threads.go b/pkg/proc/threads.go index 8ca17ff3..e08697c2 100644 --- a/pkg/proc/threads.go +++ b/pkg/proc/threads.go @@ -6,7 +6,6 @@ import ( // Thread represents a thread. type Thread interface { - MemoryReadWriter Location() (*Location, error) // Breakpoint will return the breakpoint that this thread is stopped at or // nil if the thread is not stopped at any breakpoint. @@ -24,9 +23,9 @@ type Thread interface { // RestoreRegisters restores saved registers RestoreRegisters(Registers) error BinInfo() *BinaryInfo + // ProcessMemory returns the process memory. + ProcessMemory() MemoryReadWriter StepInstruction() error - // Blocked returns true if the thread is blocked - Blocked() bool // SetCurrentBreakpoint updates the current breakpoint of this thread, if adjustPC is true also checks for breakpoints that were just hit (this should only be passed true after a thread resume) SetCurrentBreakpoint(adjustPC bool) error // Common returns the CommonThread structure for this thread @@ -47,14 +46,6 @@ type Location struct { Fn *Function } -// ErrThreadBlocked is returned when the thread -// is blocked in the scheduler. -type ErrThreadBlocked struct{} - -func (tbe ErrThreadBlocked) Error() string { - return "thread blocked" -} - // CommonThread contains fields used by this package, common to all // implementations of the Thread interface. type CommonThread struct { @@ -75,9 +66,6 @@ func topframe(g *G, thread Thread) (Stackframe, Stackframe, error) { var err error if g == nil { - if thread.Blocked() { - return Stackframe{}, Stackframe{}, ErrThreadBlocked{} - } frames, err = ThreadStacktrace(thread, 1) } else { frames, err = g.Stacktrace(1, StacktraceReadDefers) diff --git a/pkg/proc/variables.go b/pkg/proc/variables.go index d6ecd576..4caf6d72 100644 --- a/pkg/proc/variables.go +++ b/pkg/proc/variables.go @@ -310,16 +310,13 @@ func GoroutinesInfo(dbp *Target, start, count int) ([]*G, int, error) { threads := dbp.ThreadList() for _, th := range threads { - if th.Blocked() { - continue - } g, _ := GetG(th) if g != nil { threadg[g.ID] = g } } - allgptr, allglen, err := dbp.gcache.getRuntimeAllg(dbp.BinInfo(), dbp.CurrentThread()) + allgptr, allglen, err := dbp.gcache.getRuntimeAllg(dbp.BinInfo(), dbp.Memory()) if err != nil { return nil, -1, err } @@ -434,7 +431,7 @@ func getGVariable(thread Thread) (*Variable, error) { gaddr, hasgaddr := regs.GAddr() if !hasgaddr { var err error - gaddr, err = readUintRaw(thread, regs.TLS()+thread.BinInfo().GStructOffset(), int64(thread.BinInfo().Arch.PtrSize())) + gaddr, err = readUintRaw(thread.ProcessMemory(), regs.TLS()+thread.BinInfo().GStructOffset(), int64(thread.BinInfo().Arch.PtrSize())) if err != nil { return nil, err } @@ -568,7 +565,7 @@ func globalScope(bi *BinaryInfo, image *Image, mem MemoryReadWriter) *EvalScope } func newVariableFromThread(t Thread, name string, addr uint64, dwarfType godwarf.Type) *Variable { - return newVariable(name, addr, dwarfType, t.BinInfo(), t) + return newVariable(name, addr, dwarfType, t.BinInfo(), t.ProcessMemory()) } func (v *Variable) newVariable(name string, addr uint64, dwarfType godwarf.Type, mem MemoryReadWriter) *Variable { @@ -922,7 +919,7 @@ var errTracebackAncestorsDisabled = errors.New("tracebackancestors is disabled") // Ancestors returns the list of ancestors for g. func Ancestors(p Process, g *G, n int) ([]Ancestor, error) { - scope := globalScope(p.BinInfo(), p.BinInfo().Images[0], p.CurrentThread()) + scope := globalScope(p.BinInfo(), p.BinInfo().Images[0], p.Memory()) tbav, err := scope.EvalExpression("runtime.debug.tracebackancestors", loadSingleValue) if err == nil && tbav.Unreadable == nil && tbav.Kind == reflect.Int { tba, _ := constant.Int64Val(tbav.Value) diff --git a/pkg/terminal/command_test.go b/pkg/terminal/command_test.go index 61efd3b0..e4adcb8b 100644 --- a/pkg/terminal/command_test.go +++ b/pkg/terminal/command_test.go @@ -342,19 +342,6 @@ func TestScopePrefix(t *testing.T) { const goroutinesCurLinePrefix = "* Goroutine " test.AllowRecording(t) - tgtAgoroutineCount := 10 - - if goversion.VersionAfterOrEqual(runtime.Version(), 1, 14) { - // We try to make sure that all goroutines are stopped at a sensible place - // before reading their stacktrace, but due to the nature of the test - // program there is no guarantee that we always find them in a reasonable - // state. - // Asynchronous preemption in Go 1.14 exacerbates this problem, to avoid - // unnecessary flakiness reduce the target count to 9, allowing one - // goroutine to be in a bad state. - tgtAgoroutineCount = 9 - } - withTestTerminal("goroutinestackprog", t, func(term *FakeTerminal) { term.MustExec("b stacktraceme") term.MustExec("continue") @@ -407,7 +394,7 @@ func TestScopePrefix(t *testing.T) { } } } - if len(agoroutines)+extraAgoroutines < tgtAgoroutineCount { + if len(agoroutines)+extraAgoroutines < 10 { t.Fatalf("Output of goroutines did not have 10 goroutines stopped on main.agoroutine (%d+%d found): %q", len(agoroutines), extraAgoroutines, goroutinesOut) } } @@ -451,12 +438,8 @@ func TestScopePrefix(t *testing.T) { seen[ival] = true } - firsterr := tgtAgoroutineCount != 10 - for i := range seen { - if firsterr { - firsterr = false - } else if !seen[i] { + if !seen[i] { t.Fatalf("goroutine %d not found", i) } } @@ -536,21 +519,8 @@ func TestOnPrefix(t *testing.T) { } } - firsterr := false - if goversion.VersionAfterOrEqual(runtime.Version(), 1, 14) { - // We try to make sure that all goroutines are stopped at a sensible place - // before reading their stacktrace, but due to the nature of the test - // program there is no guarantee that we always find them in a reasonable - // state. - // Asynchronous preemption in Go 1.14 exacerbates this problem, to avoid - // unnecessary flakiness allow a single goroutine to be in a bad state. - firsterr = true - } - for i := range seen { - if firsterr { - firsterr = false - } else if !seen[i] { + if !seen[i] { t.Fatalf("Goroutine %d not seen\n", i) } } @@ -608,21 +578,8 @@ func TestOnPrefixLocals(t *testing.T) { } } - firsterr := false - if goversion.VersionAfterOrEqual(runtime.Version(), 1, 14) { - // We try to make sure that all goroutines are stopped at a sensible place - // before reading their stacktrace, but due to the nature of the test - // program there is no guarantee that we always find them in a reasonable - // state. - // Asynchronous preemption in Go 1.14 exacerbates this problem, to avoid - // unnecessary flakiness allow a single goroutine to be in a bad state. - firsterr = true - } - for i := range seen { - if firsterr { - firsterr = false - } else if !seen[i] { + if !seen[i] { t.Fatalf("Goroutine %d not seen\n", i) } } diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index b4c8398f..a94a20f5 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -381,9 +381,8 @@ func (d *Debugger) FunctionReturnLocations(fnName string) ([]uint64, error) { } var regs proc.Registers - var mem proc.MemoryReadWriter = p.CurrentThread() + mem := p.Memory() if g != nil && g.Thread != nil { - mem = g.Thread regs, _ = g.Thread.Registers() } instructions, err := proc.Disassemble(mem, regs, p.Breakpoints(), p.BinInfo(), fn.Entry, fn.End) @@ -1374,7 +1373,7 @@ func (d *Debugger) convertStacktrace(rawlocs []proc.Stackframe, cfg *proc.LoadCo } if cfg != nil && rawlocs[i].Current.Fn != nil { var err error - scope := proc.FrameToScope(d.target.BinInfo(), d.target.CurrentThread(), nil, rawlocs[i:]...) + scope := proc.FrameToScope(d.target.BinInfo(), d.target.Memory(), nil, rawlocs[i:]...) locals, err := scope.LocalVariables(*cfg) if err != nil { return nil, err @@ -1502,7 +1501,7 @@ func (d *Debugger) Disassemble(goroutineID int, addr1, addr2 uint64) ([]proc.Asm } regs, _ := curthread.Registers() - return proc.Disassemble(curthread, regs, d.target.Breakpoints(), d.target.BinInfo(), addr1, addr2) + return proc.Disassemble(d.target.Memory(), regs, d.target.Breakpoints(), d.target.BinInfo(), addr1, addr2) } func (d *Debugger) AsmInstructionText(inst *proc.AsmInstruction, flavour proc.AssemblyFlavour) string { @@ -1554,9 +1553,9 @@ func (d *Debugger) ExamineMemory(address uint64, length int) ([]byte, error) { d.targetMutex.Lock() defer d.targetMutex.Unlock() - thread := d.target.CurrentThread() + mem := d.target.Memory() data := make([]byte, length) - n, err := thread.ReadMemory(data, address) + n, err := mem.ReadMemory(data, address) if err != nil { return nil, err } diff --git a/service/test/integration1_test.go b/service/test/integration1_test.go index 89b5f413..08966c52 100644 --- a/service/test/integration1_test.go +++ b/service/test/integration1_test.go @@ -751,23 +751,9 @@ func Test1ClientServer_FullStacktrace(t *testing.T) { } } - firsterr := false - if goversion.VersionAfterOrEqual(runtime.Version(), 1, 14) { - // We try to make sure that all goroutines are stopped at a sensible place - // before reading their stacktrace, but due to the nature of the test - // program there is no guarantee that we always find them in a reasonable - // state. - // Asynchronous preemption in Go 1.14 exacerbates this problem, to avoid - // unnecessary flakiness allow a single goroutine to be in a bad state. - firsterr = true - } for i := range found { if !found[i] { - if firsterr { - firsterr = false - } else { - t.Fatalf("Goroutine %d not found", i) - } + t.Fatalf("Goroutine %d not found", i) } } diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go index a3001f04..fa8b8ed0 100644 --- a/service/test/integration2_test.go +++ b/service/test/integration2_test.go @@ -928,23 +928,9 @@ func TestClientServer_FullStacktrace(t *testing.T) { } } - firsterr := false - if goversion.VersionAfterOrEqual(runtime.Version(), 1, 14) { - // We try to make sure that all goroutines are stopped at a sensible place - // before reading their stacktrace, but due to the nature of the test - // program there is no guarantee that we always find them in a reasonable - // state. - // Asynchronous preemption in Go 1.14 exacerbates this problem, to avoid - // unnecessary flakiness allow a single goroutine to be in a bad state. - firsterr = true - } for i := range found { if !found[i] { - if firsterr { - firsterr = false - } else { - t.Fatalf("Goroutine %d not found", i) - } + t.Fatalf("Goroutine %d not found", i) } } @@ -2092,14 +2078,19 @@ func TestIssue2162(t *testing.T) { t.Skip("skip it for stepping into one place where no source for pc when on pie mode or windows") } withTestClient2("issue2162", t, func(c service.Client) { - _, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.main"}) + state, err := c.GetState() + assertNoError(err, t, "GetState()") + if state.CurrentThread.Function == nil { + // Can't call Step if we don't have the source code of the current function + return + } + + _, err = c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.main"}) if err != nil { t.Fatalf("Unexpected error: %v", err) } _, err = c.Step() - if err != nil { - assertNoError(err, t, "Step()") - } + assertNoError(err, t, "Step()") }) } diff --git a/service/test/variables_test.go b/service/test/variables_test.go index 77a269db..1c892754 100644 --- a/service/test/variables_test.go +++ b/service/test/variables_test.go @@ -91,7 +91,7 @@ func evalScope(p *proc.Target) (*proc.EvalScope, error) { if err != nil { return nil, err } - return proc.FrameToScope(p.BinInfo(), p.CurrentThread(), nil, frame), nil + return proc.FrameToScope(p.BinInfo(), p.Memory(), nil, frame), nil } func evalVariable(p *proc.Target, symbol string, cfg proc.LoadConfig) (*proc.Variable, error) { @@ -468,7 +468,7 @@ func TestLocalVariables(t *testing.T) { var frame proc.Stackframe frame, err = findFirstNonRuntimeFrame(p) if err == nil { - scope = proc.FrameToScope(p.BinInfo(), p.CurrentThread(), nil, frame) + scope = proc.FrameToScope(p.BinInfo(), p.Memory(), nil, frame) } } else { scope, err = proc.GoroutineScope(p.CurrentThread())