Skip to content

Commit

Permalink
proc: use .closureptr for stepping through range-over-func statements
Browse files Browse the repository at this point in the history
Uses special variables .closureptr and #yieldN to correctly identify
the parent frame of a range-over-func body closure call.

Updates go-delve#3733
  • Loading branch information
aarzilli committed Jun 25, 2024
1 parent 79d9ceb commit ee86773
Show file tree
Hide file tree
Showing 7 changed files with 268 additions and 61 deletions.
46 changes: 20 additions & 26 deletions _fixtures/rangeoverfunc.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,20 @@ B:
fmt.Println(result)
}

func TestRecur(n int) {
result := []int{}
if n > 0 {
TestRecur(n - 1)
}
for _, x := range OfSliceIndex([]int{10, 20, 30}) {
result = append(result, x)
if n == 3 {
TestRecur(0)
}
}
fmt.Println(result)
}

func main() {
TestTrickyIterAll()
TestTrickyIterAll2()
Expand All @@ -240,34 +254,22 @@ func main() {
TestLongReturnWrapper()
TestGotoA1()
TestGotoB1()
TestRecur(3)
}

type Seq[T any] func(yield func(T) bool)
type Seq2[T1, T2 any] func(yield func(T1, T2) bool)

type TrickyIterator struct {
yield func(int, int) bool
}

func (ti *TrickyIterator) iterEcho(s []int) Seq2[int, int] {
return func(yield func(int, int) bool) {
for i, v := range s {
if !yield(i, v) {
ti.yield = yield
return
}
if ti.yield != nil && !ti.yield(i, v) {
return
}
}
ti.yield = yield
return
}
yield func()
}

func (ti *TrickyIterator) iterAll(s []int) Seq2[int, int] {
return func(yield func(int, int) bool) {
ti.yield = yield // Save yield for future abuse
// NOTE: storing the yield func in the iterator has been removed because
// it make the closure escape to the heap which breaks the .closureptr
// heuristic. Eventually we will need to figure out what to do when that
// happens.
for i, v := range s {
if !yield(i, v) {
return
Expand All @@ -277,14 +279,6 @@ func (ti *TrickyIterator) iterAll(s []int) Seq2[int, int] {
}
}

func (ti *TrickyIterator) iterZero(s []int) Seq2[int, int] {
return func(yield func(int, int) bool) {
ti.yield = yield // Save yield for future abuse
// Don't call it at all, maybe it won't escape
return
}
}

// OfSliceIndex returns a Seq2 over the elements of s. It is equivalent
// to range s.
func OfSliceIndex[T any, S ~[]T](s S) Seq2[int, T] {
Expand Down
80 changes: 59 additions & 21 deletions pkg/proc/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ const (
// If localsNoDeclLineCheck the declaration line isn't checked at
// all to determine if the variable is in scope.
localsNoDeclLineCheck

// If localsOnlyRangeBodyClosures is set simpleLocals only returns
// variables containing the range body closure.
localsOnlyRangeBodyClosures
)

// ConvertEvalScope returns a new EvalScope in the context of the
Expand Down Expand Up @@ -330,12 +334,10 @@ func (scope *EvalScope) Locals(flags localsFlags, wantedName string) ([]*Variabl
scopes = append(scopes, vars0)

if scope.rangeFrames == nil {
scope.rangeFrames, err = rangeFuncStackTrace(scope.target, scope.g)
err := scope.setupRangeFrames()
if err != nil {
return nil, err
}
scope.rangeFrames = scope.rangeFrames[1:]
scope.enclosingRangeScopes = make([]*EvalScope, len(scope.rangeFrames))
}
for i, scope2 := range scope.enclosingRangeScopes {
if i == len(scope.enclosingRangeScopes)-1 {
Expand Down Expand Up @@ -373,6 +375,17 @@ func (scope *EvalScope) Locals(flags localsFlags, wantedName string) ([]*Variabl
return vars, nil
}

func (scope *EvalScope) setupRangeFrames() error {
var err error
scope.rangeFrames, err = rangeFuncStackTrace(scope.target, scope.g)
if err != nil {
return err
}
scope.rangeFrames = scope.rangeFrames[1:]
scope.enclosingRangeScopes = make([]*EvalScope, len(scope.rangeFrames))
return nil
}

// simpleLocals returns all local variables in 'scope'.
// This function does not try to merge the scopes of range-over-func closure
// bodies with their enclosing function, for that use (*EvalScope).Locals or
Expand Down Expand Up @@ -406,23 +419,7 @@ func (scope *EvalScope) simpleLocals(flags localsFlags, wantedName string) ([]*V

// look for dictionary entry
if scope.dictAddr == 0 {
for _, entry := range varEntries {
name, _ := entry.Val(dwarf.AttrName).(string)
if name == goDictionaryName {
dictVar, err := extractVarInfoFromEntry(scope.target, scope.BinInfo, scope.image(), scope.Regs, scope.Mem, entry.Tree, 0)
if err != nil {
logflags.DebuggerLogger().Errorf("could not load %s variable: %v", name, err)
} else if dictVar.Unreadable != nil {
logflags.DebuggerLogger().Errorf("could not load %s variable: %v", name, dictVar.Unreadable)
} else {
scope.dictAddr, err = readUintRaw(dictVar.mem, dictVar.Addr, int64(scope.BinInfo.Arch.PtrSize()))
if err != nil {
logflags.DebuggerLogger().Errorf("could not load %s variable: %v", name, err)
}
}
break
}
}
scope.dictAddr = readLocalPtrVar(dwarfTree, goDictionaryName, scope.target, scope.BinInfo, scope.image(), scope.Regs, scope.Mem)
}

vars := make([]*Variable, 0, len(varEntries))
Expand All @@ -434,8 +431,12 @@ func (scope *EvalScope) simpleLocals(flags localsFlags, wantedName string) ([]*V
if name != wantedName && name != "&"+wantedName {
continue
}
case flags&localsOnlyRangeBodyClosures != 0:
if !strings.HasPrefix(name, "#yield") && !strings.HasPrefix(name, "&#yield") {
continue
}
default:
if name == goDictionaryName || name == goClosurePtr || strings.HasPrefix(name, "#state") || strings.HasPrefix(name, "&#state") || strings.HasPrefix(name, "#next") || strings.HasPrefix(name, "&#next") || strings.HasPrefix(name, "#yield") {
if name == goDictionaryName || name == goClosurePtr || strings.HasPrefix(name, "#state") || strings.HasPrefix(name, "&#state") || strings.HasPrefix(name, "#next") || strings.HasPrefix(name, "&#next") || strings.HasPrefix(name, "#yield") || strings.HasPrefix(name, "&#yield") {
continue
}
}
Expand Down Expand Up @@ -522,6 +523,31 @@ func afterLastArgAddr(vars []*Variable) uint64 {
return 0
}

// readLocalPtrVar reads the value of the local pointer variable vname. This
// is a low level helper function, it does not support nested scopes, range
// resolution across range bodies, type parameters, &c...
func readLocalPtrVar(dwarfTree *godwarf.Tree, vname string, tgt *Target, bi *BinaryInfo, image *Image, regs op.DwarfRegisters, mem MemoryReadWriter) uint64 {
for _, entry := range dwarfTree.Children {
name, _ := entry.Val(dwarf.AttrName).(string)
if name == vname {
v, err := extractVarInfoFromEntry(tgt, bi, image, regs, mem, entry, 0)
if err != nil {
logflags.DebuggerLogger().Errorf("could not load %s variable: %v", name, err)
} else if v.Unreadable != nil {
logflags.DebuggerLogger().Errorf("could not load %s variable: %v", name, v.Unreadable)
} else {
r, err := readUintRaw(v.mem, v.Addr, int64(bi.Arch.PtrSize()))
if err != nil {
logflags.DebuggerLogger().Errorf("could not load %s variable: %v", name, err)
}
return r
}
break
}
}
return 0
}

// setValue writes the value of srcv to dstv.
// - If srcv is a numerical literal constant and srcv is of a compatible type
// the necessary type conversion is performed.
Expand Down Expand Up @@ -887,6 +913,8 @@ func (stack *evalStack) resume(g *G) {
scope.Regs.FrameBase = stack.fboff + int64(scope.g.stack.hi)
scope.Regs.CFA = scope.frameOffset + int64(scope.g.stack.hi)
stack.curthread = g.Thread
scope.rangeFrames = nil
scope.enclosingRangeScopes = nil

finished := funcCallStep(scope, stack, g.Thread)
if finished {
Expand Down Expand Up @@ -1002,6 +1030,16 @@ func (stack *evalStack) executeOp() {
case *evalop.PushFrameoff:
stack.push(newConstant(constant.MakeInt64(scope.frameOffset), scope.Mem))

case *evalop.PushRangeParentOffset:
if scope.rangeFrames == nil {
stack.err = scope.setupRangeFrames()
}
if len(scope.rangeFrames) > 0 {
stack.push(newConstant(constant.MakeInt64(scope.rangeFrames[len(scope.rangeFrames)-2].FrameOffset()), scope.Mem))
} else {
stack.push(newConstant(constant.MakeInt64(0), scope.Mem))
}

case *evalop.PushThreadID:
stack.push(newConstant(constant.MakeInt64(int64(scope.threadID)), scope.Mem))

Expand Down
3 changes: 3 additions & 0 deletions pkg/proc/evalop/evalcompile.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,9 @@ func (ctx *compileCtx) compileAST(t ast.Expr) error {
case x.Name == "runtime" && node.Sel.Name == "threadid":
ctx.pushOp(&PushThreadID{})

case x.Name == "runtime" && node.Sel.Name == "rangeParentOffset":
ctx.pushOp(&PushRangeParentOffset{})

default:
ctx.pushOp(&PushPackageVarOrSelect{Name: x.Name, Sel: node.Sel.Name})
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/proc/evalop/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ type PushThreadID struct {

func (*PushThreadID) depthCheck() (npop, npush int) { return 0, 1 }

// PushRangeParentOffset pushes the frame offset of the range-over-func closure body parent.
type PushRangeParentOffset struct {
}

func (*PushRangeParentOffset) depthCheck() (npop, npush int) { return 0, 1 }

// PushConst pushes a constant on the stack.
type PushConst struct {
Value constant.Value
Expand Down
Loading

0 comments on commit ee86773

Please sign in to comment.