Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support range-over-func #3732

Closed
wants to merge 1 commit into from
Closed

Support range-over-func #3732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion pkg/proc/breakpoints.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ const (
// goroutine.
StepIntoNewProcBreakpoint

steppingMask = NextBreakpoint | NextDeferBreakpoint | StepBreakpoint | StepIntoNewProcBreakpoint
// NextInactivatedBreakpoint a NextBreakpoint that has been inactivated, see rangeFrameInactivateNextBreakpoints
NextInactivatedBreakpoint

steppingMask = NextBreakpoint | NextDeferBreakpoint | StepBreakpoint | StepIntoNewProcBreakpoint | NextInactivatedBreakpoint
)

// WatchType is the watchpoint type
Expand Down Expand Up @@ -221,6 +224,8 @@ func (bp *Breakpoint) VerboseDescr() []string {
r = append(r, "PluginOpenBreakpoint")
case StepIntoNewProcBreakpoint:
r = append(r, "StepIntoNewProcBreakpoint")
case NextInactivatedBreakpoint:
r = append(r, "NextInactivatedBreakpoint")
default:
r = append(r, fmt.Sprintf("Unknown %d", breaklet.Kind))
}
Expand Down Expand Up @@ -318,6 +323,9 @@ func (bpstate *BreakpointState) checkCond(tgt *Target, breaklet *Breaklet, threa
case StackResizeBreakpoint, PluginOpenBreakpoint, StepIntoNewProcBreakpoint:
// no further checks

case NextInactivatedBreakpoint:
active = false

default:
bpstate.CondError = fmt.Errorf("internal error unknown breakpoint kind %v", breaklet.Kind)
}
Expand Down
10 changes: 3 additions & 7 deletions pkg/proc/eval.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,12 +340,8 @@ func (scope *EvalScope) Locals(flags localsFlags, wantedName string) ([]*Variabl
}
}
for i, scope2 := range scope.enclosingRangeScopes {
if i == len(scope.enclosingRangeScopes)-1 {
// Last one is the caller frame, we shouldn't check it
break
}
if scope2 == nil {
scope2 = FrameToScope(scope.target, scope.target.Memory(), scope.g, scope.threadID, scope.rangeFrames[i:]...)
scope2 = FrameToScope(scope.target, scope.target.Memory(), scope.g, scope.threadID, scope.rangeFrames[2*i:]...)
scope.enclosingRangeScopes[i] = scope2
}
vars, err := scope2.simpleLocals(flags, wantedName)
Expand Down Expand Up @@ -381,8 +377,8 @@ func (scope *EvalScope) setupRangeFrames() error {
if err != nil {
return err
}
scope.rangeFrames = scope.rangeFrames[1:]
scope.enclosingRangeScopes = make([]*EvalScope, len(scope.rangeFrames))
scope.rangeFrames = scope.rangeFrames[2:] // skip the first frame and its return frame
scope.enclosingRangeScopes = make([]*EvalScope, len(scope.rangeFrames)/2)
return nil
}

Expand Down
63 changes: 22 additions & 41 deletions pkg/proc/proc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6299,25 +6299,6 @@ func TestRangeOverFuncNext(t *testing.T) {
return seqTest{contNext, n}
}

nx2 := func(t *testing.T, n int) seqTest {
return seqTest{contNothing, func(grp *proc.TargetGroup, p *proc.Target) {
_, ln1 := currentLineNumber(p, t)
assertNoError(grp.Next(), t, "Next() returned an error")
f, ln2 := currentLineNumber(p, t)
if ln2 == n {
return
}
if ln2 != ln1 {
t.Fatalf("Program did not continue to correct next location (expected %d or %d) was %s:%d", ln1, n, f, ln2)
}
assertNoError(grp.Next(), t, "Next() returned an error")
f, ln2 = currentLineNumber(p, t)
if ln2 != n {
t.Fatalf("Program did not continue to correct next location (expected %d) was %s:%d", n, f, ln2)
}
}}
}

assertLocals := func(t *testing.T, varnames ...string) seqTest {
return seqTest{
contNothing,
Expand Down Expand Up @@ -6659,20 +6640,20 @@ func TestRangeOverFuncNext(t *testing.T) {
nx(118), // if z == 4
nx(121),

nx(116), // for _, z := range (z == 2)
nx2(t, 117), // result = append(result, z)
nx(118), // if z == 4
nx(116), // for _, z := range (z == 2)
nx(117), // result = append(result, z)
nx(118), // if z == 4
nx(121),

nx(116), // for _, z := range (z == 3)
nx2(t, 117), // result = append(result, z)
nx(118), // if z == 4
nx(116), // for _, z := range (z == 3)
nx(117), // result = append(result, z)
nx(118), // if z == 4
nx(121),

nx(116), // for _, z := range (z == 4)
nx2(t, 117), // result = append(result, z)
nx(118), // if z == 4
nx(119), // break
nx(116), // for _, z := range (z == 4)
nx(117), // result = append(result, z)
nx(118), // if z == 4
nx(119), // break

nx(112), // defer func()
nx(113), // r := recover()
Expand Down Expand Up @@ -6773,14 +6754,14 @@ func TestRangeOverFuncNext(t *testing.T) {
nx(203), // result = append(result, y)
nx(204),

nx(199), // for _, y := range (y == 2)
nx2(t, 200), // if y == 3
nx(203), // result = append(result, y)
nx(199), // for _, y := range (y == 2)
nx(200), // if y == 3
nx(203), // result = append(result, y)
nx(204),

nx(199), // for _, y := range (y == 3)
nx2(t, 200), // if y == 3
nx(201), // goto A
nx(199), // for _, y := range (y == 3)
nx(200), // if y == 3
nx(201), // goto A
nx(204),
nx(206), // result = append(result, x)
nx(207),
Expand Down Expand Up @@ -6809,14 +6790,14 @@ func TestRangeOverFuncNext(t *testing.T) {
nx(222), // result = append(result, y)
nx(223),

nx(218), // for _, y := range (y == 2)
nx2(t, 219), // if y == 3
nx(222), // result = append(result, y)
nx(218), // for _, y := range (y == 2)
nx(219), // if y == 3
nx(222), // result = append(result, y)
nx(223),

nx(218), // for _, y := range (y == 3)
nx2(t, 219), // if y == 3
nx(220), // goto B
nx(218), // for _, y := range (y == 3)
nx(219), // if y == 3
nx(220), // goto B
nx(223),
nx(225),
nx(227), // result = append(result, 999)
Expand Down
63 changes: 43 additions & 20 deletions pkg/proc/stack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ func (g *G) readDefers(frames []Stackframe) {
frames[i].TopmostDefer = curdefer.topdefer()
}

if frames[i].SystemStack || curdefer.SP >= uint64(frames[i].Regs.CFA) {
if frames[i].SystemStack || frames[i].Inlined || curdefer.SP >= uint64(frames[i].Regs.CFA) {
// frames[i].Regs.CFA is the value that SP had before the function of
// frames[i] was called.
// This means that when curdefer.SP == frames[i].Regs.CFA then curdefer
Expand Down Expand Up @@ -900,8 +900,8 @@ func ruleString(rule *frame.DWRule, regnumToString func(uint64) string) string {

// rangeFuncStackTrace, if the topmost frame of the stack is a the body of a
// range-over-func statement, returns a slice containing the stack of range
// bodies on the stack, the frame of the function containing them and
// finally the function that called it.
// bodies on the stack, interleaved with their return frames, the frame of
// the function containing them and finally the function that called it.
//
// For example, given:
//
Expand All @@ -916,9 +916,11 @@ func ruleString(rule *frame.DWRule, regnumToString func(uint64) string) string {
// It will return the following frames:
//
// 0. f-range2()
// 1. f-range1()
// 2. f()
// 3. function that called f()
// 1. function that called f-range2
// 2. f-range1()
// 3. function that called f-range1
// 4. f()
// 5. function that called f()
//
// If the topmost frame of the stack is *not* the body closure of a
// range-over-func statement then nothing is returned.
Expand All @@ -931,7 +933,17 @@ func rangeFuncStackTrace(tgt *Target, g *G) ([]Stackframe, error) {
return nil, err
}
frames := []Stackframe{}
stage := 0

const (
startStage = iota
normalStage
lastFrameStage
doneStage
)

stage := startStage
addretframe := false

var rangeParent *Function
nonMonotonicSP := false
var closurePtr int64
Expand All @@ -941,6 +953,7 @@ func rangeFuncStackTrace(tgt *Target, g *G) ([]Stackframe, error) {
if fr.closurePtr != 0 {
closurePtr = fr.closurePtr
}
addretframe = true
}

closurePtrOk := func(fr *Stackframe) bool {
Expand Down Expand Up @@ -976,33 +989,40 @@ func rangeFuncStackTrace(tgt *Target, g *G) ([]Stackframe, error) {
}
}

if addretframe {
addretframe = false
frames = append(frames, fr)
}

switch stage {
case 0:
case startStage:
appendFrame(fr)
rangeParent = fr.Call.Fn.extra(tgt.BinInfo()).rangeParent
stage++
stage = normalStage
if rangeParent == nil || closurePtr == 0 {
frames = nil
stage = 3
addretframe = false
stage = doneStage
return false
}
case 1:
if fr.Call.Fn.offset == rangeParent.offset && closurePtrOk(&fr) {
appendFrame(fr)
stage++
} else if fr.Call.Fn.extra(tgt.BinInfo()).rangeParent == rangeParent && closurePtrOk(&fr) {
case normalStage:
if fr.Call.Fn.offset == rangeParent.offset && closureptrok(&fr) {
frames = append(frames, fr)
stage = lastFrameStage
} else if fr.Call.Fn.extra(tgt.BinInfo()).rangeParent == rangeParent && closureptrok(&fr) {
appendFrame(fr)
if closurePtr == 0 {
frames = nil
stage = 3
addretframe = false
stage = doneStage
return false
}
}
case 2:
case lastFrameStage:
frames = append(frames, fr)
stage++
stage = doneStage
return false
case 3:
case doneStage:
return false
}
return true
Expand All @@ -1013,9 +1033,12 @@ func rangeFuncStackTrace(tgt *Target, g *G) ([]Stackframe, error) {
if nonMonotonicSP {
return nil, errors.New("corrupted stack (SP not monotonically decreasing)")
}
if stage != 3 {
if stage != doneStage {
return nil, errors.New("could not find range-over-func closure parent on the stack")
}
if len(frames)%2 != 0 {
return nil, errors.New("incomplete range-over-func stacktrace")
}
g.readDefers(frames)
return frames, nil
}
49 changes: 46 additions & 3 deletions pkg/proc/target_exec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -847,8 +847,10 @@ func next(dbp *Target, stepInto, inlinedStepOut bool) error {

// Set step-out breakpoints for range-over-func body closures
if !stepInto && selg != nil && topframe.Current.Fn.extra(bi).rangeParent != nil && len(rangeFrames) > 0 {
for _, fr := range rangeFrames[:len(rangeFrames)-1] {
retframecond := astutil.And(sameGCond, frameoffCondition(&fr))
// Set step-out breakpoint for every range-over-func body currently on the stack so that we stop on them.
for i := 2; i < len(rangeFrames); i += 2 {
fr := &rangeFrames[i]
retframecond := astutil.And(sameGCond, frameoffCondition(fr))
if !fr.hasInlines {
dbp.SetBreakpoint(0, fr.Current.PC, NextBreakpoint, retframecond)
} else {
Expand All @@ -857,7 +859,7 @@ func next(dbp *Target, stepInto, inlinedStepOut bool) error {
if err != nil {
return err
}
pcs, err = removeInlinedCalls(pcs, &fr, bi)
pcs, err = removeInlinedCalls(pcs, fr, bi)
if err != nil {
return err
}
Expand All @@ -866,6 +868,24 @@ func next(dbp *Target, stepInto, inlinedStepOut bool) error {
}
}
}

// Set a step-out breakpoint for the first range-over-func body on the
// stack, this breakpoint will never cause a stop because the associated
// callback always returns false.
// Its purpose is to inactivate all the breakpoints for the current
// range-over-func body function so that if the iterator re-calls it we
// don't end up inside the prologue.
if !rangeFrames[0].Inlined {
bp, err := dbp.SetBreakpoint(0, rangeFrames[1].Call.PC, NextBreakpoint, astutil.And(sameGCond, frameoffCondition(&rangeFrames[1])))
if err == nil {
bplet := bp.Breaklets[len(bp.Breaklets)-1]
bplet.callback = func(th Thread, p *Target) (bool, error) {
rangeFrameInactivateNextBreakpoints(p, rangeFrames[0].Call.Fn)
return false, nil
}
}
}

topframe, retframe = rangeFrames[len(rangeFrames)-2], rangeFrames[len(rangeFrames)-1]
}

Expand Down Expand Up @@ -1657,3 +1677,26 @@ func (t *Target) handleHardcodedBreakpoints(grp *TargetGroup, trapthread Thread,
}
return nil
}

func rangeFrameInactivateNextBreakpoints(p *Target, fn *Function) {
pc, err := FirstPCAfterPrologue(p, fn, false)
if err != nil {
logflags.DebuggerLogger().Errorf("Error inactivating next breakpoints after exiting a range-over-func body: %v", err)
return
}

for _, bp := range p.Breakpoints().M {
if bp.Addr < fn.Entry || bp.Addr >= fn.End || bp.Addr == pc {
continue
}
for _, bplet := range bp.Breaklets {
if bplet.Kind != NextBreakpoint {
continue
}
// We set to NextInactivatedBreakpoint instead of deleting them because
// we can't delete breakpoints (or breakpointlets) while breakpoint
// conditions are being evaluated.
bplet.Kind = NextInactivatedBreakpoint
}
}
}