From 1e2338d2337c3c0f7863e3f30981a2a57ad1fc0f Mon Sep 17 00:00:00 2001 From: Derek Parker Date: Tue, 24 Oct 2023 09:57:39 -0700 Subject: [PATCH] proc: allow evaluator to reference previous frames (#3534) Fixes #3515 --- _fixtures/condframe.go | 17 ++++++++++ pkg/proc/eval.go | 14 +++++++- pkg/proc/evalop/evalcompile.go | 26 +++++++++++++-- pkg/proc/evalop/ops.go | 3 +- pkg/proc/native/threads_linux_ppc64le.go | 42 +++++++++++------------- pkg/proc/proc_test.go | 35 ++++++++++++++++++-- pkg/terminal/command_test.go | 13 ++++++++ 7 files changed, 121 insertions(+), 29 deletions(-) create mode 100644 _fixtures/condframe.go diff --git a/_fixtures/condframe.go b/_fixtures/condframe.go new file mode 100644 index 00000000..5807a46d --- /dev/null +++ b/_fixtures/condframe.go @@ -0,0 +1,17 @@ +package main + +import "fmt" + +func callme() { + for i := 0; i < 10; i++ { + callme2() + } +} + +func callme2() { + fmt.Println("called again!!") +} + +func main() { + callme() +} diff --git a/pkg/proc/eval.go b/pkg/proc/eval.go index ef582af6..41dd16d4 100644 --- a/pkg/proc/eval.go +++ b/pkg/proc/eval.go @@ -963,7 +963,19 @@ func (stack *evalStack) executeOp() { stack.push(newConstant(op.Value, scope.Mem)) case *evalop.PushLocal: - vars, err := scope.Locals(0) + var vars []*Variable + var err error + if op.Frame != 0 { + var frameScope *EvalScope + frameScope, err = ConvertEvalScope(scope.target, scope.g.ID, int(op.Frame), 0) + if err != nil { + stack.err = err + return + } + vars, err = frameScope.Locals(0) + } else { + vars, err = scope.Locals(0) + } if err != nil { stack.err = err return diff --git a/pkg/proc/evalop/evalcompile.go b/pkg/proc/evalop/evalcompile.go index 6bb15d81..f7521208 100644 --- a/pkg/proc/evalop/evalcompile.go +++ b/pkg/proc/evalop/evalcompile.go @@ -193,7 +193,7 @@ func (ctx *compileCtx) compileAST(t ast.Expr) error { ctx.pushOp(&PushThreadID{}) case ctx.HasLocal(x.Name): - ctx.pushOp(&PushLocal{x.Name}) + ctx.pushOp(&PushLocal{Name: x.Name}) ctx.pushOp(&Select{node.Sel.Name}) case ctx.HasGlobal(x.Name, node.Sel.Name): @@ -203,6 +203,27 @@ func (ctx *compileCtx) compileAST(t ast.Expr) error { return ctx.compileUnary(node.X, &Select{node.Sel.Name}) } + case *ast.CallExpr: + ident, ok := x.Fun.(*ast.SelectorExpr) + if ok { + f, ok := ident.X.(*ast.Ident) + if ok && f.Name == "runtime" && ident.Sel.Name == "frame" { + switch arg := x.Args[0].(type) { + case *ast.BasicLit: + fr, err := strconv.ParseInt(arg.Value, 10, 8) + if err != nil { + return err + } + // Push local onto the stack to be evaluated in the new frame context. + ctx.pushOp(&PushLocal{Name: node.Sel.Name, Frame: fr}) + return nil + default: + return fmt.Errorf("expected integer value for frame, got %v", arg) + } + } + } + return ctx.compileUnary(node.X, &Select{node.Sel.Name}) + case *ast.BasicLit: // try to accept "package/path".varname syntax for package variables s, err := strconv.Unquote(x.Value) if err != nil { @@ -271,7 +292,6 @@ func (ctx *compileCtx) compileAST(t ast.Expr) error { default: return fmt.Errorf("expression %T not implemented", t) - } return nil } @@ -392,7 +412,7 @@ func (ctx *compileCtx) compileBuiltinCall(builtin string, args []ast.Expr) error func (ctx *compileCtx) compileIdent(node *ast.Ident) error { switch { case ctx.HasLocal(node.Name): - ctx.pushOp(&PushLocal{node.Name}) + ctx.pushOp(&PushLocal{Name: node.Name}) case ctx.HasGlobal("", node.Name): ctx.pushOp(&PushPackageVar{"", node.Name}) case node.Name == "true" || node.Name == "false": diff --git a/pkg/proc/evalop/ops.go b/pkg/proc/evalop/ops.go index 1bd25729..3a74ef7b 100644 --- a/pkg/proc/evalop/ops.go +++ b/pkg/proc/evalop/ops.go @@ -39,7 +39,8 @@ func (*PushConst) depthCheck() (npop, npush int) { return 0, 1 } // PushLocal pushes the local variable with the given name on the stack. type PushLocal struct { - Name string + Name string + Frame int64 } func (*PushLocal) depthCheck() (npop, npush int) { return 0, 1 } diff --git a/pkg/proc/native/threads_linux_ppc64le.go b/pkg/proc/native/threads_linux_ppc64le.go index aaa2d769..94ce21ff 100644 --- a/pkg/proc/native/threads_linux_ppc64le.go +++ b/pkg/proc/native/threads_linux_ppc64le.go @@ -1,13 +1,12 @@ package native import ( - "fmt" "debug/elf" - "syscall" - "unsafe" - - sys "golang.org/x/sys/unix" + "fmt" + "syscall" + "unsafe" + sys "golang.org/x/sys/unix" "github.com/go-delve/delve/pkg/proc" "github.com/go-delve/delve/pkg/proc/linutil" @@ -27,22 +26,21 @@ func (t *nativeThread) fpRegisters() ([]proc.Register, []byte, error) { } func (t *nativeThread) restoreRegisters(savedRegs proc.Registers) error { - sr := savedRegs.(*linutil.PPC64LERegisters) + sr := savedRegs.(*linutil.PPC64LERegisters) - var restoreRegistersErr error - t.dbp.execPtraceFunc(func() { - restoreRegistersErr = ptraceSetGRegs(t.ID, sr.Regs) - if restoreRegistersErr != syscall.Errno(0) && restoreRegistersErr != nil { - return - } - if sr.Fpregset != nil { - iov := sys.Iovec{Base: &sr.Fpregset[0], Len: _PPC64LE_FPREGS_SIZE} - _, _, restoreRegistersErr = syscall.Syscall6(syscall.SYS_PTRACE, sys.PTRACE_SETREGSET, uintptr(t.ID), uintptr(elf.NT_FPREGSET), uintptr(unsafe.Pointer(&iov)), 0, 0) - } - }) - if restoreRegistersErr == syscall.Errno(0) { - restoreRegistersErr = nil - } - return restoreRegistersErr + var restoreRegistersErr error + t.dbp.execPtraceFunc(func() { + restoreRegistersErr = ptraceSetGRegs(t.ID, sr.Regs) + if restoreRegistersErr != syscall.Errno(0) && restoreRegistersErr != nil { + return + } + if sr.Fpregset != nil { + iov := sys.Iovec{Base: &sr.Fpregset[0], Len: _PPC64LE_FPREGS_SIZE} + _, _, restoreRegistersErr = syscall.Syscall6(syscall.SYS_PTRACE, sys.PTRACE_SETREGSET, uintptr(t.ID), uintptr(elf.NT_FPREGSET), uintptr(unsafe.Pointer(&iov)), 0, 0) + } + }) + if restoreRegistersErr == syscall.Errno(0) { + restoreRegistersErr = nil + } + return restoreRegistersErr } - diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 714a4d1d..db359da1 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -7,6 +7,7 @@ import ( "fmt" "go/ast" "go/constant" + "go/parser" "go/token" "io" "math/rand" @@ -1770,6 +1771,36 @@ func TestCondBreakpoint(t *testing.T) { }) } +func TestCondBreakpointWithFrame(t *testing.T) { + protest.AllowRecording(t) + withTestProcess("condframe", t, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) { + bp := setFileBreakpoint(p, t, fixture.Source, 12) + parsed, err := parser.ParseExpr("runtime.frame(1).i == 3") + if err != nil { + t.Fatalf("failed to parse expression: %v", err) + } + bp.UserBreaklet().Cond = parsed + + assertNoError(grp.Continue(), t, "Continue()") + + g := p.SelectedGoroutine() + scope, err := proc.ConvertEvalScope(p, g.ID, 1, 0) + if err != nil { + t.Fatal(err) + } + + v, err := scope.EvalExpression("i", normalLoadConfig) + if err != nil { + t.Fatal(err) + } + + vval, _ := constant.Int64Val(v.Value) + if vval != 3 { + t.Fatalf("Incorrect value for frame variable: %v", vval) + } + }) +} + func TestCondBreakpointError(t *testing.T) { protest.AllowRecording(t) withTestProcess("parallel_next", t, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) { @@ -3381,7 +3412,7 @@ func TestCgoStacktrace(t *testing.T) { logStacktrace(t, p, frames) m := stacktraceCheck(t, tc, frames) - mismatch := (m == nil) + mismatch := m == nil for i, j := range m { if strings.HasPrefix(tc[i], "C.hellow") { @@ -5085,7 +5116,7 @@ func TestStepOutPreservesGoroutine(t *testing.T) { } pc := currentPC(p, t) f, l, fn := p.BinInfo().PCToLine(pc) - var fnname string = "???" + var fnname = "???" if fn != nil { fnname = fn.Name } diff --git a/pkg/terminal/command_test.go b/pkg/terminal/command_test.go index 57b62105..02b8746f 100644 --- a/pkg/terminal/command_test.go +++ b/pkg/terminal/command_test.go @@ -1242,6 +1242,19 @@ func TestHitCondBreakpoint(t *testing.T) { }) } +func TestCondBreakpointWithFrame(t *testing.T) { + withTestTerminal("condframe", t, func(term *FakeTerminal) { + term.MustExec("break bp1 callme2") + term.MustExec("condition bp1 runtime.frame(1).i == 3") + term.MustExec("continue") + out := term.MustExec("frame 1 print i") + t.Logf("%q", out) + if !strings.Contains(out, "3\n") { + t.Fatalf("wrong value of i") + } + }) +} + func TestClearCondBreakpoint(t *testing.T) { withTestTerminal("break", t, func(term *FakeTerminal) { term.MustExec("break main.main:4")