diff --git a/pkg/proc/dwarf_expr_test.go b/pkg/proc/dwarf_expr_test.go index b2f11bed..34aeb7c1 100644 --- a/pkg/proc/dwarf_expr_test.go +++ b/pkg/proc/dwarf_expr_test.go @@ -80,7 +80,7 @@ func uintExprCheck(t *testing.T, scope *proc.EvalScope, expr string, tgt uint64) } func dwarfExprCheck(t *testing.T, mem proc.MemoryReadWriter, regs op.DwarfRegisters, bi *proc.BinaryInfo, testCases map[string]uint16, fn *proc.Function) *proc.EvalScope { - scope := &proc.EvalScope{Location: proc.Location{PC: 0x40100, Fn: fn}, Regs: regs, Mem: mem, Gvar: nil, BinInfo: bi} + scope := &proc.EvalScope{Location: proc.Location{PC: 0x40100, Fn: fn}, Regs: regs, Mem: mem, BinInfo: bi} for name, value := range testCases { uintExprCheck(t, scope, name, uint64(value)) } @@ -213,7 +213,7 @@ func TestDwarfExprLoclist(t *testing.T) { mem := newFakeMemory(defaultCFA, uint16(before), uint16(after)) regs := linutil.AMD64Registers{Regs: &linutil.AMD64PtraceRegs{}} - scope := &proc.EvalScope{Location: proc.Location{PC: 0x40100, Fn: mainfn}, Regs: dwarfRegisters(bi, ®s), Mem: mem, Gvar: nil, BinInfo: bi} + scope := &proc.EvalScope{Location: proc.Location{PC: 0x40100, Fn: mainfn}, Regs: dwarfRegisters(bi, ®s), Mem: mem, BinInfo: bi} uintExprCheck(t, scope, "a", before) scope.PC = 0x40800 @@ -247,7 +247,7 @@ func TestIssue1419(t *testing.T) { mem := newFakeMemory(defaultCFA) - scope := &proc.EvalScope{Location: proc.Location{PC: 0x40100, Fn: mainfn}, Regs: op.DwarfRegisters{}, Mem: mem, Gvar: nil, BinInfo: bi} + scope := &proc.EvalScope{Location: proc.Location{PC: 0x40100, Fn: mainfn}, Regs: op.DwarfRegisters{}, Mem: mem, BinInfo: bi} va, err := scope.EvalExpression("a", normalLoadConfig) assertNoError(err, t, "EvalExpression(a)") diff --git a/pkg/proc/eval.go b/pkg/proc/eval.go index b016d361..b3490d46 100644 --- a/pkg/proc/eval.go +++ b/pkg/proc/eval.go @@ -214,10 +214,10 @@ func (scope *EvalScope) evalAST(t ast.Expr) (*Variable, error) { // try to interpret the selector as a package variable if maybePkg, ok := node.X.(*ast.Ident); ok { if maybePkg.Name == "runtime" && node.Sel.Name == "curg" { - if scope.Gvar == nil { + if scope.g == nil { return nilVariable, nil } - return scope.Gvar.clone(), nil + return scope.g.variable.clone(), nil } else if maybePkg.Name == "runtime" && node.Sel.Name == "frameoff" { return newConstant(constant.MakeInt64(scope.frameOffset), scope.Mem), nil } else if v, err := scope.findGlobal(maybePkg.Name + "." + node.Sel.Name); err == nil { diff --git a/pkg/proc/fncall.go b/pkg/proc/fncall.go index c5f901ec..5ebcd174 100644 --- a/pkg/proc/fncall.go +++ b/pkg/proc/fncall.go @@ -11,6 +11,7 @@ import ( "reflect" "sort" "strconv" + "strings" "github.com/go-delve/delve/pkg/dwarf/godwarf" "github.com/go-delve/delve/pkg/dwarf/op" @@ -105,7 +106,7 @@ type callContext struct { // To signal that evaluation is completed a value will be written to // continueRequest having cont == false and the return values in ret. continueRequest chan<- continueRequest - continueCompleted <-chan struct{} + continueCompleted <-chan *G } type continueRequest struct { @@ -114,9 +115,9 @@ type continueRequest struct { ret *Variable } -func (callCtx *callContext) doContinue() { +func (callCtx *callContext) doContinue() *G { callCtx.continueRequest <- continueRequest{cont: true} - <-callCtx.continueCompleted + return <-callCtx.continueCompleted } func (callCtx *callContext) doReturn(ret *Variable, err error) { @@ -129,12 +130,21 @@ func (callCtx *callContext) doReturn(ret *Variable, err error) { // EvalExpressionWithCalls is like EvalExpression but allows function calls in 'expr'. // Because this can only be done in the current goroutine, unlike // EvalExpression, EvalExpressionWithCalls is not a method of EvalScope. -func EvalExpressionWithCalls(p Process, expr string, retLoadCfg LoadConfig, checkEscape bool) error { +func EvalExpressionWithCalls(p Process, g *G, expr string, retLoadCfg LoadConfig, checkEscape bool) error { bi := p.BinInfo() if !p.Common().fncallEnabled { return errFuncCallUnsupportedBackend } - if p.Common().continueCompleted != nil { + + // check that the target goroutine is running + if g == nil { + return errNoGoroutine + } + if g.Status != Grunning || g.Thread == nil { + return errGoroutineNotRunning + } + + if callinj := p.Common().fncallForG[g.ID]; callinj != nil && callinj.continueCompleted != nil { return errFuncCallInProgress } @@ -143,22 +153,13 @@ func EvalExpressionWithCalls(p Process, expr string, retLoadCfg LoadConfig, chec return errFuncCallUnsupported } - // check that the selected goroutine is running - g := p.SelectedGoroutine() - if g == nil { - return errNoGoroutine - } - if g.Status != Grunning || g.Thread == nil { - return errGoroutineNotRunning - } - - scope, err := GoroutineScope(p.CurrentThread()) + scope, err := GoroutineScope(g.Thread) if err != nil { return err } continueRequest := make(chan continueRequest) - continueCompleted := make(chan struct{}) + continueCompleted := make(chan *G) scope.callCtx = &callContext{ p: p, @@ -168,8 +169,10 @@ func EvalExpressionWithCalls(p Process, expr string, retLoadCfg LoadConfig, chec continueCompleted: continueCompleted, } - p.Common().continueRequest = continueRequest - p.Common().continueCompleted = continueCompleted + p.Common().fncallForG[g.ID] = &callInjection{ + continueCompleted, + continueRequest, + } go scope.EvalExpression(expr, retLoadCfg) @@ -178,35 +181,35 @@ func EvalExpressionWithCalls(p Process, expr string, retLoadCfg LoadConfig, chec return Continue(p) } - return finishEvalExpressionWithCalls(p, contReq, ok) + return finishEvalExpressionWithCalls(p, g, contReq, ok) } -func finishEvalExpressionWithCalls(p Process, contReq continueRequest, ok bool) error { +func finishEvalExpressionWithCalls(p Process, g *G, contReq continueRequest, ok bool) error { + fncallLog("stashing return values for %d in thread=%d\n", g.ID, g.Thread.ThreadID()) var err error if !ok { err = errors.New("internal error EvalExpressionWithCalls didn't return anything") } else if contReq.err != nil { if fpe, ispanic := contReq.err.(fncallPanicErr); ispanic { - p.CurrentThread().Common().returnValues = []*Variable{fpe.panicVar} + g.Thread.Common().returnValues = []*Variable{fpe.panicVar} } else { err = contReq.err } } else if contReq.ret == nil { - p.CurrentThread().Common().returnValues = nil - } else if contReq.ret.Addr == 0 && contReq.ret.DwarfType == nil { + g.Thread.Common().returnValues = nil + } else if contReq.ret.Addr == 0 && contReq.ret.DwarfType == nil && contReq.ret.Kind == reflect.Invalid { // this is a variable returned by a function call with multiple return values r := make([]*Variable, len(contReq.ret.Children)) for i := range contReq.ret.Children { r[i] = &contReq.ret.Children[i] } - p.CurrentThread().Common().returnValues = r + g.Thread.Common().returnValues = r } else { - p.CurrentThread().Common().returnValues = []*Variable{contReq.ret} + g.Thread.Common().returnValues = []*Variable{contReq.ret} } - p.Common().continueRequest = nil - close(p.Common().continueCompleted) - p.Common().continueCompleted = nil + close(p.Common().fncallForG[g.ID].continueCompleted) + delete(p.Common().fncallForG, g.ID) return err } @@ -240,22 +243,13 @@ func (scope *EvalScope) evalFunctionCall(node *ast.CallExpr) (*Variable, error) return nil, errFuncCallUnsupported } - // check that the selected goroutine is running - g := p.SelectedGoroutine() - if g == nil { - return nil, errNoGoroutine - } - if g.Status != Grunning || g.Thread == nil { - return nil, errGoroutineNotRunning - } - // check that there are at least 256 bytes free on the stack - regs, err := g.Thread.Registers(true) + regs, err := scope.g.Thread.Registers(true) if err != nil { return nil, err } regs = regs.Copy() - if regs.SP()-256 <= g.stacklo { + if regs.SP()-256 <= scope.g.stacklo { return nil, errNotEnoughStack } _, err = regs.Get(int(x86asm.RAX)) @@ -273,42 +267,39 @@ func (scope *EvalScope) evalFunctionCall(node *ast.CallExpr) (*Variable, error) return nil, err } - if err := callOP(bi, g.Thread, regs, dbgcallfn.Entry); err != nil { + if err := callOP(bi, scope.g.Thread, regs, dbgcallfn.Entry); err != nil { 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, g.Thread, regs.SP()-3*uint64(bi.Arch.PtrSize()), uint64(fncall.argFrameSize)); err != nil { + if err := writePointer(bi, scope.g.Thread, regs.SP()-3*uint64(bi.Arch.PtrSize()), uint64(fncall.argFrameSize)); err != nil { return nil, err } - fncallLog("function call initiated %v frame size %d\n", fncall.fn, fncall.argFrameSize) + fncallLog("function call initiated %v frame size %d", fncall.fn, fncall.argFrameSize) - spoff := int64(scope.Regs.Uint64Val(scope.Regs.SPRegNum)) - int64(g.stackhi) - bpoff := int64(scope.Regs.Uint64Val(scope.Regs.BPRegNum)) - int64(g.stackhi) - fboff := scope.Regs.FrameBase - int64(g.stackhi) + spoff := int64(scope.Regs.Uint64Val(scope.Regs.SPRegNum)) - int64(scope.g.stackhi) + bpoff := int64(scope.Regs.Uint64Val(scope.Regs.BPRegNum)) - int64(scope.g.stackhi) + fboff := scope.Regs.FrameBase - int64(scope.g.stackhi) for { - scope.callCtx.doContinue() + scope.g = scope.callCtx.doContinue() - g = p.SelectedGoroutine() - if g != nil { - // adjust the value of registers inside scope - for regnum := range scope.Regs.Regs { - switch uint64(regnum) { - case scope.Regs.PCRegNum, scope.Regs.SPRegNum, scope.Regs.BPRegNum: - // leave these alone - default: - // every other register is dirty and unrecoverable - scope.Regs.Regs[regnum] = nil - } + // adjust the value of registers inside scope + for regnum := range scope.Regs.Regs { + switch uint64(regnum) { + case scope.Regs.PCRegNum, scope.Regs.SPRegNum, scope.Regs.BPRegNum: + // leave these alone + default: + // every other register is dirty and unrecoverable + scope.Regs.Regs[regnum] = nil } - - scope.Regs.Regs[scope.Regs.SPRegNum].Uint64Val = uint64(spoff + int64(g.stackhi)) - scope.Regs.Regs[scope.Regs.BPRegNum].Uint64Val = uint64(bpoff + int64(g.stackhi)) - scope.Regs.FrameBase = fboff + int64(g.stackhi) - scope.Regs.CFA = scope.frameOffset + int64(g.stackhi) } + scope.Regs.Regs[scope.Regs.SPRegNum].Uint64Val = uint64(spoff + int64(scope.g.stackhi)) + scope.Regs.Regs[scope.Regs.BPRegNum].Uint64Val = uint64(bpoff + int64(scope.g.stackhi)) + scope.Regs.FrameBase = fboff + int64(scope.g.stackhi) + scope.Regs.CFA = scope.frameOffset + int64(scope.g.stackhi) + finished := funcCallStep(scope, &fncall) if finished { break @@ -464,14 +455,13 @@ type funcCallArg struct { // funcCallEvalArgs evaluates the arguments of the function call, copying // the into the argument frame starting at argFrameAddr. func funcCallEvalArgs(scope *EvalScope, fncall *functionCallState, argFrameAddr uint64) error { - g := scope.callCtx.p.SelectedGoroutine() - if g == nil { + if scope.g == nil { // this should never happen return errNoGoroutine } if fncall.receiver != nil { - err := funcCallCopyOneArg(g, scope, fncall, fncall.receiver, &fncall.formalArgs[0], argFrameAddr) + err := funcCallCopyOneArg(scope, fncall, fncall.receiver, &fncall.formalArgs[0], argFrameAddr) if err != nil { return err } @@ -487,7 +477,7 @@ func funcCallEvalArgs(scope *EvalScope, fncall *functionCallState, argFrameAddr } actualArg.Name = exprToString(fncall.expr.Args[i]) - err = funcCallCopyOneArg(g, scope, fncall, actualArg, formalArg, argFrameAddr) + err = funcCallCopyOneArg(scope, fncall, actualArg, formalArg, argFrameAddr) if err != nil { return err } @@ -496,10 +486,10 @@ func funcCallEvalArgs(scope *EvalScope, fncall *functionCallState, argFrameAddr return nil } -func funcCallCopyOneArg(g *G, scope *EvalScope, fncall *functionCallState, actualArg *Variable, formalArg *funcCallArg, argFrameAddr uint64) error { +func funcCallCopyOneArg(scope *EvalScope, fncall *functionCallState, actualArg *Variable, formalArg *funcCallArg, argFrameAddr uint64) error { if scope.callCtx.checkEscape { //TODO(aarzilli): only apply the escapeCheck to leaking parameters. - if err := escapeCheck(actualArg, formalArg.name, g); err != nil { + if err := escapeCheck(actualArg, formalArg.name, scope.g); err != nil { return fmt.Errorf("cannot use %s as argument %s in function %s: %v", actualArg.Name, formalArg.name, fncall.fn.Name, err) } } @@ -631,7 +621,7 @@ func funcCallStep(callScope *EvalScope, fncall *functionCallState) bool { p := callScope.callCtx.p bi := p.BinInfo() - thread := p.CurrentThread() + thread := callScope.g.Thread regs, err := thread.Registers(false) if err != nil { fncall.err = err @@ -651,7 +641,7 @@ func funcCallStep(callScope *EvalScope, fncall *functionCallState) bool { fnname = loc.Fn.Name } } - fncallLog("function call interrupt rax=%#x (PC=%#x in %s)\n", rax, pc, fnname) + fncallLog("function call interrupt gid=%d thread=%d rax=%#x (PC=%#x in %s)", callScope.g.ID, thread.ThreadID(), rax, pc, fnname) } switch rax { @@ -869,3 +859,49 @@ func (scope *EvalScope) allocString(v *Variable) error { _, err = scope.Mem.WriteMemory(v.Base, []byte(constant.StringVal(v.Value))) return err } + +func isCallInjectionStop(loc *Location) bool { + if loc.Fn == nil { + return false + } + return strings.HasPrefix(loc.Fn.Name, debugCallFunctionNamePrefix1) || strings.HasPrefix(loc.Fn.Name, debugCallFunctionNamePrefix2) +} + +// callInjectionProtocol is the function called from Continue to progress +// the injection protocol for all threads. +// Returns true if a call injection terminated +func callInjectionProtocol(p Process, threads []Thread) (done bool, err error) { + if len(p.Common().fncallForG) == 0 { + // we aren't injecting any calls, no need to check the threads. + return false, nil + } + for _, thread := range threads { + loc, err := thread.Location() + if err != nil { + continue + } + if !isCallInjectionStop(loc) { + continue + } + + g, err := GetG(thread) + if err != nil { + return done, fmt.Errorf("could not determine running goroutine for thread %#x currently executing the function call injection protocol: %v", thread.ThreadID(), err) + } + callinj := p.Common().fncallForG[g.ID] + if callinj == nil || callinj.continueCompleted == nil { + return false, fmt.Errorf("could not recover call injection state for goroutine %d", g.ID) + } + fncallLog("step for injection on goroutine %d thread=%d (location %s)", g.ID, thread.ThreadID(), loc.Fn.Name) + callinj.continueCompleted <- g + contReq, ok := <-callinj.continueRequest + if !contReq.cont { + err := finishEvalExpressionWithCalls(p, g, contReq, ok) + if err != nil { + return done, err + } + done = true + } + } + return done, nil +} diff --git a/pkg/proc/interface.go b/pkg/proc/interface.go index a54b4327..30ca5bec 100644 --- a/pkg/proc/interface.go +++ b/pkg/proc/interface.go @@ -117,17 +117,21 @@ type CommonProcess struct { allGCache []*G fncallEnabled bool + fncallForG map[int]*callInjection +} + +type callInjection struct { // if continueCompleted is not nil it means we are in the process of // executing an injected function call, see comments throughout // pkg/proc/fncall.go for a description of how this works. - continueCompleted chan<- struct{} + continueCompleted chan<- *G continueRequest <-chan continueRequest } // NewCommonProcess returns a struct with fields common across // all process implementations. func NewCommonProcess(fncallEnabled bool) CommonProcess { - return CommonProcess{fncallEnabled: fncallEnabled} + return CommonProcess{fncallEnabled: fncallEnabled, fncallForG: make(map[int]*callInjection)} } // ClearAllGCache clears the cached contents of the cache for runtime.allgs. diff --git a/pkg/proc/proc.go b/pkg/proc/proc.go index bc2bcc8f..1dff68fe 100644 --- a/pkg/proc/proc.go +++ b/pkg/proc/proc.go @@ -8,7 +8,6 @@ import ( "go/token" "path/filepath" "strconv" - "strings" ) // ErrNotExecutable is returned after attempting to execute a non-executable file @@ -190,6 +189,11 @@ func Continue(dbp Process) error { threads := dbp.ThreadList() + callInjectionDone, err := callInjectionProtocol(dbp, threads) + if err != nil { + return err + } + if err := pickCurrentThread(dbp, trapthread, threads); err != nil { return err } @@ -209,6 +213,7 @@ func Continue(dbp Process) error { if err != nil || loc.Fn == nil { return conditionErrors(threads) } + g, _ := GetG(curthread) switch { case loc.Fn.Name == "runtime.breakpoint": @@ -220,22 +225,8 @@ func Continue(dbp Process) error { return err } return conditionErrors(threads) - case strings.HasPrefix(loc.Fn.Name, debugCallFunctionNamePrefix1) || strings.HasPrefix(loc.Fn.Name, debugCallFunctionNamePrefix2): - continueCompleted := dbp.Common().continueCompleted - if continueCompleted == nil { - return conditionErrors(threads) - } - continueCompleted <- struct{}{} - contReq, ok := <-dbp.Common().continueRequest - if !contReq.cont { - // only stop execution if the expression evaluation with calls finished - err := finishEvalExpressionWithCalls(dbp, contReq, ok) - if err != nil { - return err - } - return conditionErrors(threads) - } - default: + case g == nil || dbp.Common().fncallForG[g.ID] == nil: + // a hardcoded breakpoint somewhere else in the code (probably cgo) return conditionErrors(threads) } case curbp.Active && curbp.Internal: @@ -285,6 +276,11 @@ func Continue(dbp Process) error { default: // not a manual stop, not on runtime.Breakpoint, not on a breakpoint, just repeat } + if callInjectionDone { + // a call injection was finished, don't let a breakpoint with a failed + // condition or a step breakpoint shadow this. + return conditionErrors(threads) + } } } @@ -334,8 +330,10 @@ func stepInstructionOut(dbp Process, curthread Thread, fnname1, fnname2 string) } loc, err := curthread.Location() if err != nil || loc.Fn == nil || (loc.Fn.Name != fnname1 && loc.Fn.Name != fnname2) { - if g := dbp.SelectedGoroutine(); g != nil { - g.CurrentLoc = *loc + g, _ := GetG(curthread) + selg := dbp.SelectedGoroutine() + if g != nil && selg != nil && g.ID == selg.ID { + selg.CurrentLoc = *loc } return curthread.SetCurrentBreakpoint() } @@ -706,11 +704,6 @@ func ConvertEvalScope(dbp Process, gid, frame, deferCall int) (*EvalScope, error // Otherwise all memory between frames[0].Regs.SP() and frames[0].Regs.CFA // will be cached. func FrameToScope(bi *BinaryInfo, thread MemoryReadWriter, g *G, frames ...Stackframe) *EvalScope { - var gvar *Variable - if g != nil { - gvar = g.variable - } - // Creates a cacheMem that will preload the entire stack frame the first // time any local variable is read. // Remember that the stack grows downward in memory. @@ -725,7 +718,7 @@ func FrameToScope(bi *BinaryInfo, thread MemoryReadWriter, g *G, frames ...Stack thread = cacheMemory(thread, uintptr(minaddr), int(maxaddr-minaddr)) } - s := &EvalScope{Location: frames[0].Call, Regs: frames[0].Regs, Mem: thread, Gvar: gvar, BinInfo: bi, frameOffset: frames[0].FrameOffset()} + s := &EvalScope{Location: frames[0].Call, Regs: frames[0].Regs, Mem: thread, g: g, BinInfo: bi, frameOffset: frames[0].FrameOffset()} s.PC = frames[0].lastpc return s } diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 0789ea81..50fdf1f3 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -4144,7 +4144,7 @@ func TestIssue1374(t *testing.T) { setFileBreakpoint(p, t, fixture, 7) assertNoError(proc.Continue(p), t, "First Continue") assertLineNumber(p, t, 7, "Did not continue to correct location (first continue),") - assertNoError(proc.EvalExpressionWithCalls(p, "getNum()", normalLoadConfig, true), t, "Call") + assertNoError(proc.EvalExpressionWithCalls(p, p.SelectedGoroutine(), "getNum()", normalLoadConfig, true), t, "Call") err := proc.Continue(p) if _, isexited := err.(proc.ErrProcessExited); !isexited { regs, _ := p.CurrentThread().Registers(false) @@ -4325,18 +4325,33 @@ func TestAncestors(t *testing.T) { }) } -func testCallConcurrentCheckReturns(p proc.Process, t *testing.T, gid1 int) bool { +func testCallConcurrentCheckReturns(p proc.Process, t *testing.T, gid1, gid2 int) int { + found := 0 for _, thread := range p.ThreadList() { g, _ := proc.GetG(thread) - if g == nil || g.ID != gid1 { + if g == nil || (g.ID != gid1 && g.ID != gid2) { continue } retvals := thread.Common().ReturnValues(normalLoadConfig) - if len(retvals) != 0 { - return true + if len(retvals) == 0 { + continue + } + n, _ := constant.Int64Val(retvals[0].Value) + t.Logf("injection on goroutine %d (thread %d) returned %v\n", g.ID, thread.ThreadID(), n) + switch g.ID { + case gid1: + if n != 11 { + t.Errorf("wrong return value for goroutine %d", g.ID) + } + found++ + case gid2: + if n != 12 { + t.Errorf("wrong return value for goroutine %d", g.ID) + } + found++ } } - return false + return found } func TestCallConcurrent(t *testing.T) { @@ -4344,26 +4359,34 @@ func TestCallConcurrent(t *testing.T) { withTestProcess("teststepconcurrent", t, func(p proc.Process, fixture protest.Fixture) { bp := setFileBreakpoint(p, t, fixture, 24) assertNoError(proc.Continue(p), t, "Continue()") - _, err := p.ClearBreakpoint(bp.Addr) - assertNoError(err, t, "ClearBreakpoint() returned an error") + //_, err := p.ClearBreakpoint(bp.Addr) + //assertNoError(err, t, "ClearBreakpoint() returned an error") gid1 := p.SelectedGoroutine().ID t.Logf("starting injection in %d / %d", p.SelectedGoroutine().ID, p.CurrentThread().ThreadID()) - assertNoError(proc.EvalExpressionWithCalls(p, "Foo(10, 1)", normalLoadConfig, false), t, "EvalExpressionWithCalls()") + assertNoError(proc.EvalExpressionWithCalls(p, p.SelectedGoroutine(), "Foo(10, 1)", normalLoadConfig, false), t, "EvalExpressionWithCalls()") - returned := testCallConcurrentCheckReturns(p, t, gid1) + returned := testCallConcurrentCheckReturns(p, t, gid1, -1) curthread := p.CurrentThread() - if curbp := curthread.Breakpoint(); curbp.Breakpoint == nil || curbp.ID != bp.ID || returned { + if curbp := curthread.Breakpoint(); curbp.Breakpoint == nil || curbp.ID != bp.ID || returned > 0 { + t.Logf("skipping test, the call injection terminated before we hit a breakpoint in a different thread") return } + _, err := p.ClearBreakpoint(bp.Addr) + assertNoError(err, t, "ClearBreakpoint() returned an error") + + gid2 := p.SelectedGoroutine().ID + t.Logf("starting second injection in %d / %d", p.SelectedGoroutine().ID, p.CurrentThread().ThreadID()) + assertNoError(proc.EvalExpressionWithCalls(p, p.SelectedGoroutine(), "Foo(10, 2)", normalLoadConfig, false), t, "EvalExpressioniWithCalls") + for { - returned = testCallConcurrentCheckReturns(p, t, gid1) - if returned { + returned += testCallConcurrentCheckReturns(p, t, gid1, gid2) + if returned >= 2 { break } - t.Logf("Continuing... %v", returned) + t.Logf("Continuing... %d", returned) assertNoError(proc.Continue(p), t, "Continue()") } diff --git a/pkg/proc/variables.go b/pkg/proc/variables.go index 00549960..d6240053 100644 --- a/pkg/proc/variables.go +++ b/pkg/proc/variables.go @@ -218,7 +218,7 @@ type EvalScope struct { Location Regs op.DwarfRegisters Mem MemoryReadWriter // Target's memory - Gvar *Variable + g *G BinInfo *BinaryInfo frameOffset int64 @@ -250,7 +250,7 @@ func (err *IsNilErr) Error() string { } func globalScope(bi *BinaryInfo, image *Image, mem MemoryReadWriter) *EvalScope { - return &EvalScope{Location: Location{}, Regs: op.DwarfRegisters{StaticBase: image.StaticBase}, Mem: mem, Gvar: nil, BinInfo: bi, frameOffset: 0} + return &EvalScope{Location: Location{}, Regs: op.DwarfRegisters{StaticBase: image.StaticBase}, Mem: mem, g: nil, BinInfo: bi, frameOffset: 0} } func (scope *EvalScope) newVariable(name string, addr uintptr, dwarfType godwarf.Type, mem MemoryReadWriter) *Variable { diff --git a/pkg/terminal/command.go b/pkg/terminal/command.go index 459a71c7..b1ad7281 100644 --- a/pkg/terminal/command.go +++ b/pkg/terminal/command.go @@ -1035,7 +1035,7 @@ func (c *Commands) call(t *Term, ctx callContext, args string) error { unsafe = true args = args[len(unsafePrefix):] } - state, err := exitedToError(t.client.Call(args, unsafe)) + state, err := exitedToError(t.client.Call(ctx.Scope.GoroutineID, args, unsafe)) c.frame = 0 if err != nil { printcontextNoState(t) diff --git a/service/api/types.go b/service/api/types.go index 2bd6cf6f..3fa24196 100644 --- a/service/api/types.go +++ b/service/api/types.go @@ -304,7 +304,7 @@ type DebuggerCommand struct { // command. ThreadID int `json:"threadID,omitempty"` // GoroutineID is used to specify which thread to use with the SwitchGoroutine - // command. + // and Call commands. GoroutineID int `json:"goroutineID,omitempty"` // When ReturnInfoLoadConfig is not nil it will be used to load the value // of any return variables. diff --git a/service/client.go b/service/client.go index a15c0e28..03e0eb9d 100644 --- a/service/client.go +++ b/service/client.go @@ -39,7 +39,7 @@ type Client interface { // StepOut continues to the return address of the current function StepOut() (*api.DebuggerState, error) // Call resumes process execution while making a function call. - Call(expr string, unsafe bool) (*api.DebuggerState, error) + Call(goroutineID int, expr string, unsafe bool) (*api.DebuggerState, error) // SingleStep will step a single cpu instruction. StepInstruction() (*api.DebuggerState, error) diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index 611cd888..1340df53 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -598,7 +598,14 @@ func (d *Debugger) Command(command *api.DebuggerCommand) (*api.DebuggerState, er if command.ReturnInfoLoadConfig == nil { return nil, errors.New("can not call function with nil ReturnInfoLoadConfig") } - err = proc.EvalExpressionWithCalls(d.target, command.Expr, *api.LoadConfigToProc(command.ReturnInfoLoadConfig), !command.UnsafeCall) + g := d.target.SelectedGoroutine() + if command.GoroutineID > 0 { + g, err = proc.FindGoroutine(d.target, command.GoroutineID) + if err != nil { + return nil, err + } + } + err = proc.EvalExpressionWithCalls(d.target, g, command.Expr, *api.LoadConfigToProc(command.ReturnInfoLoadConfig), !command.UnsafeCall) case api.Rewind: d.log.Debug("rewinding") if err := d.target.Direction(proc.Backward); err != nil { diff --git a/service/rpc2/client.go b/service/rpc2/client.go index 12424345..2b8da0f8 100644 --- a/service/rpc2/client.go +++ b/service/rpc2/client.go @@ -148,9 +148,9 @@ func (c *RPCClient) StepOut() (*api.DebuggerState, error) { return &out.State, err } -func (c *RPCClient) Call(expr string, unsafe bool) (*api.DebuggerState, error) { +func (c *RPCClient) Call(goroutineID int, expr string, unsafe bool) (*api.DebuggerState, error) { var out CommandOut - err := c.call("Command", api.DebuggerCommand{Name: api.Call, ReturnInfoLoadConfig: c.retValLoadCfg, Expr: expr, UnsafeCall: unsafe}, &out) + err := c.call("Command", api.DebuggerCommand{Name: api.Call, ReturnInfoLoadConfig: c.retValLoadCfg, Expr: expr, UnsafeCall: unsafe, GoroutineID: goroutineID}, &out) return &out.State, err } diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go index 5300a47e..e9cf28cc 100644 --- a/service/test/integration2_test.go +++ b/service/test/integration2_test.go @@ -1555,7 +1555,7 @@ func TestClientServerFunctionCall(t *testing.T) { state := <-c.Continue() assertNoError(state.Err, t, "Continue()") beforeCallFn := state.CurrentThread.Function.Name() - state, err := c.Call("call1(one, two)", false) + state, err := c.Call(-1, "call1(one, two)", false) assertNoError(err, t, "Call()") t.Logf("returned to %q", state.CurrentThread.Function.Name()) if state.CurrentThread.Function.Name() != beforeCallFn { @@ -1598,7 +1598,7 @@ func TestClientServerFunctionCallBadPos(t *testing.T) { assertNoError(state.Err, t, "Continue()") c.SetReturnValuesLoadConfig(&normalLoadConfig) - state, err = c.Call("main.call1(main.zero, main.zero)", false) + state, err = c.Call(-1, "main.call1(main.zero, main.zero)", false) if err == nil || err.Error() != "call not at safe point" { t.Fatalf("wrong error or no error: %v", err) } @@ -1612,7 +1612,7 @@ func TestClientServerFunctionCallPanic(t *testing.T) { c.SetReturnValuesLoadConfig(&normalLoadConfig) state := <-c.Continue() assertNoError(state.Err, t, "Continue()") - state, err := c.Call("callpanic()", false) + state, err := c.Call(-1, "callpanic()", false) assertNoError(err, t, "Call()") t.Logf("at: %s:%d", state.CurrentThread.File, state.CurrentThread.Line) if state.CurrentThread.ReturnValues == nil { @@ -1638,7 +1638,7 @@ func TestClientServerFunctionCallStacktrace(t *testing.T) { c.SetReturnValuesLoadConfig(&api.LoadConfig{false, 0, 2048, 0, 0}) state := <-c.Continue() assertNoError(state.Err, t, "Continue()") - state, err := c.Call("callstacktrace()", false) + state, err := c.Call(-1, "callstacktrace()", false) assertNoError(err, t, "Call()") t.Logf("at: %s:%d", state.CurrentThread.File, state.CurrentThread.Line) if state.CurrentThread.ReturnValues == nil { diff --git a/service/test/variables_test.go b/service/test/variables_test.go index bb22cb3b..5811abe9 100644 --- a/service/test/variables_test.go +++ b/service/test/variables_test.go @@ -1155,6 +1155,10 @@ func TestCallFunction(t *testing.T) { // Escape tests {"escapeArg(&a2)", nil, errors.New("cannot use &a2 as argument pa2 in function main.escapeArg: stack object passed to escaping pointer: pa2")}, + + // Issue 1577 + {"1+2", []string{`::3`}, nil}, + {`"de"+"mo"`, []string{`::"demo"`}, nil}, } var testcases112 = []testCaseCallFunction{ @@ -1224,7 +1228,7 @@ func testCallFunction(t *testing.T, p proc.Process, tc testCaseCallFunction) { checkEscape = false } t.Logf("call %q", tc.expr) - err := proc.EvalExpressionWithCalls(p, callExpr, pnormalLoadConfig, checkEscape) + err := proc.EvalExpressionWithCalls(p, p.SelectedGoroutine(), callExpr, pnormalLoadConfig, checkEscape) if tc.err != nil { t.Logf("\terr = %v\n", err) if err == nil {