From def0688e7ab336acc7332e4fca2dc4c4798a1d58 Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Tue, 24 Sep 2024 19:22:04 +0200 Subject: [PATCH] proc: step into coroutine (#3791) The step command is changed such that when the function being currently called is a coroutine switch function it will move to the associated coroutine. Functions that switch coroutines are currently the next, stop and yield closures produced by the iter.Pull function. --- Documentation/backend_test_health.md | 3 +- _fixtures/backwardsiter.go | 24 +++++ pkg/proc/stepping_test.go | 24 +++++ pkg/proc/target_exec.go | 139 +++++++++++++++++++++++++-- pkg/proc/variables.go | 2 +- 5 files changed, 184 insertions(+), 8 deletions(-) create mode 100644 _fixtures/backwardsiter.go diff --git a/Documentation/backend_test_health.md b/Documentation/backend_test_health.md index ec4e9f01..9ea1f6f3 100644 --- a/Documentation/backend_test_health.md +++ b/Documentation/backend_test_health.md @@ -1,9 +1,10 @@ Tests skipped by each supported backend: -* 386 skipped = 8 +* 386 skipped = 9 * 1 broken * 3 broken - cgo stacktraces * 4 not implemented + * 1 not working due to optimizations * arm64 skipped = 1 * 1 broken - global variable symbolication * darwin skipped = 3 diff --git a/_fixtures/backwardsiter.go b/_fixtures/backwardsiter.go new file mode 100644 index 00000000..a38de057 --- /dev/null +++ b/_fixtures/backwardsiter.go @@ -0,0 +1,24 @@ +package main + +import ( + "fmt" + "iter" +) + +func backwards(s []int) func(func(int) bool) { + return func(yield func(int) bool) { + for i := len(s) - 1; i >= 0; i-- { + if !yield(s[i]) { + break + } + } + } +} + +func main() { + next, stop := iter.Pull(backwards([]int{10, 20, 30, 40})) + fmt.Println(next()) + fmt.Println(next()) + fmt.Println(next()) + stop() +} diff --git a/pkg/proc/stepping_test.go b/pkg/proc/stepping_test.go index 20f37905..38e3b04e 100644 --- a/pkg/proc/stepping_test.go +++ b/pkg/proc/stepping_test.go @@ -1762,3 +1762,27 @@ func TestRangeOverFuncNextInlined(t *testing.T) { }) }) } + +func TestStepIntoCoroutine(t *testing.T) { + if !goversion.VersionAfterOrEqual(runtime.Version(), 1, 23) { + t.Skip("N/A") + } + skipOn(t, "not working due to optimizations", "386") + withTestProcessArgs("backwardsiter", t, ".", []string{}, 0, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) { + testseq2intl(t, fixture, grp, p, nil, []seqTest{ + {contContinueToBreakpoint, 20}, // fmt.Println(next()) -- first call + {contStep, 9}, // func(yield) + {contNext, 10}, // for... + {contNext, 11}, // if !yield + {contStep, 20}, // fmt.Println(next()) -- first call (returning from next) + {contNext, 21}, // fmt.Println(next()) -- second call + {contStep, 11}, // if !yield + {contNext, 10}, // for... + {contNext, 11}, // if !yield + {contStep, 21}, // fmt.Println(next()) -- second call (returning from next) + {contNext, 22}, // fmt.Println(next()) -- third call + {contNext, 23}, // stop() + {contStep, 11}, // if !yield + }) + }) +} diff --git a/pkg/proc/target_exec.go b/pkg/proc/target_exec.go index 4cc02266..cbc9389b 100644 --- a/pkg/proc/target_exec.go +++ b/pkg/proc/target_exec.go @@ -982,6 +982,12 @@ func stepIntoCallback(curthread Thread, p *Target) (bool, error) { if err != nil { return false, err } + + ok, err := stepIntoCoroutineMaybe(curthread, p, text) + if ok || err != nil { + return false, err + } + var fn *Function if loc, _ := curthread.Location(); loc != nil { fn = loc.Fn @@ -1261,13 +1267,14 @@ func setStepIntoNewProcBreakpoint(p *Target, sameGCond ast.Expr) { // Temp breakpoints must be cleared because the current goroutine could // hit one of them before the new goroutine manages to start. - err = p.ClearSteppingBreakpoints() - if err != nil { - return false, err + for _, bp := range p.Breakpoints().M { + for _, bplet := range bp.Breaklets { + if bplet.Kind&steppingMask != 0 { + bplet.Kind = NextInactivatedBreakpoint + } + } } - newGCond := astutil.Eql(astutil.Sel(astutil.PkgVar("runtime", "curg"), "goid"), astutil.Int(newGGoID)) - // We don't want to use startpc directly because it will be an // autogenerated wrapper on some versions of Go. Addditionally, once we // have the correct function we must also skip to prologue. @@ -1281,11 +1288,15 @@ func setStepIntoNewProcBreakpoint(p *Target, sameGCond ast.Expr) { // The new breakpoint must have 'NextBreakpoint' kind because we want to // stop on it. - _, err = p.SetBreakpoint(0, uint64(startpc), NextBreakpoint, newGCond) + _, err = p.SetBreakpoint(0, uint64(startpc), NextBreakpoint, goroutineCondition(newGGoID)) return false, err // we don't want to stop at this breakpoint if there is no error } } +func goroutineCondition(goid int64) ast.Expr { + return astutil.Eql(astutil.Sel(astutil.PkgVar("runtime", "curg"), "goid"), astutil.Int(goid)) +} + func allowDuplicateBreakpoint(bp *Breakpoint, err error) (*Breakpoint, error) { if err != nil { //lint:ignore S1020 this is clearer @@ -1721,3 +1732,119 @@ func rangeFrameInactivateNextBreakpoints(p *Target, fn *Function) { } } } + +// stepIntoCoroutineMaybe: if the current instruction is a call to a closure +// defined into iter.Pull (i.e. next, yield and stop) stepIntoCoroutineMaybe +// will set up a new breakpoint to step into the associated coroutine code +// and returns true. +// In every other case it returns false. +func stepIntoCoroutineMaybe(curthread Thread, p *Target, text []AsmInstruction) (bool, error) { + if len(text) == 0 || !text[0].IsCall() || text[0].DestLoc == nil || text[0].DestLoc.Fn == nil || !strings.HasPrefix(text[0].DestLoc.Fn.Name, "iter.Pull") || !strings.Contains(text[0].DestLoc.Fn.Name, ".func") { + return false, nil + } + bi := p.BinInfo() + + // Read the closure that we are going to call currently + + regs, err := curthread.Registers() + if err != nil { + return false, fmt.Errorf("could not get registers trying to step into coroutine: %v", err) + } + dregs := bi.Arch.RegistersToDwarfRegisters(0, regs) + cst := text[0].DestLoc.Fn.extra(bi).closureStructType + clos := newVariable("", dregs.Uint64Val(bi.Arch.ContextRegNum), cst, p.BinInfo(), p.Memory()) + + // Get variable 'c' from the current closure, change its type to + // runtime.coro (it is normally iter.coro, which is an internal + // placeholder). + + cvar, err := clos.structMember("c") + if err != nil { + logflags.DebuggerLogger().Errorf("iter.Pull problems accessing captured 'c' variable in closure: %v", err) + return false, nil + } + cvar = cvar.maybeDereference() + if cvar.Unreadable != nil { + return false, fmt.Errorf("could not read coroutine: %v", cvar.Unreadable) + } + typRuntimeCoro, err := bi.findType("runtime.coro") + if err != nil { + logflags.DebuggerLogger().Errorf("could not find runtime.coro type: %v", err) + return false, nil + } + cvar = newVariable("", cvar.Addr, typRuntimeCoro, p.BinInfo(), p.Memory()) + + // Set a breakpoint on the first user frame of goroutine c.gp (but + // something special needs to happen if c.gp is executing + // runtime.corostart). + + gp := cvar.loadFieldNamed("gp") + if gp == nil { + logflags.DebuggerLogger().Errorf("could not load runtime.coro.gp field (unreadable: %v)", cvar.Unreadable) + return false, nil + } + gaddr, _ := constant.Uint64Val(gp.Value) + + gvar, err := newGVariable(curthread, gaddr, false) + if err != nil { + logflags.DebuggerLogger().Errorf("could not load runtime.coro.gp: %v", err) + return false, nil + } + g, err := gvar.parseG() + if err != nil { + logflags.DebuggerLogger().Errorf("could not load runtime.coro.gp: %v", err) + return false, nil + } + + if g.CurrentLoc.Fn == nil { + logflags.DebuggerLogger().Errorf("could not determine target location of coroutine") + return false, nil + } + + var bploc Location + + if g.CurrentLoc.Fn.Name == "runtime.corostart" { + // If the associated goroutine is on runtime.corostart that means the + // coroutine hasn't started yet, to give a smooth user experience we + // shouldn't just switch to the goroutine, but instead put a breakpoint on + // the entry point of the sequence function. + // + // This function is stored in the captured variable 'seq' in 'c.f' + + f := cvar.loadFieldNamed("f") + if f == nil || f.Unreadable != nil { + logflags.DebuggerLogger().Errorf("could not determine target location of coroutine (corostart)") + return false, nil + } + seq := f.fieldVariable("seq") + if seq == nil || seq.Unreadable != nil { + logflags.DebuggerLogger().Errorf("could not determine target location of coroutine (corostart -- seq)") + return false, nil + } + fn := bi.PCToFunc(seq.Base) + if fn == nil { + logflags.DebuggerLogger().Errorf("could not determine target location of coroutine (corostart), no function for PC: %#x", seq.Base) + return false, nil + } + pc, err := FirstPCAfterPrologue(p, fn, false) + if err != nil { + logflags.DebuggerLogger().Errorf("FirstPCAfterPrologue error: %v", err) + pc = fn.Entry + } + bploc = Location{PC: pc, Fn: fn} + } else { + bploc = g.UserCurrent() + } + + // Invalidate all current temp breakpoints + for _, bp := range p.Breakpoints().M { + for _, bplet := range bp.Breaklets { + if bplet.Kind&steppingMask != 0 { + bplet.Kind = NextInactivatedBreakpoint + } + } + } + + _, err = p.SetBreakpoint(0, bploc.PC, NextBreakpoint, goroutineCondition(g.ID)) + return true, err +} diff --git a/pkg/proc/variables.go b/pkg/proc/variables.go index 724eeca1..95c1222d 100644 --- a/pkg/proc/variables.go +++ b/pkg/proc/variables.go @@ -509,7 +509,7 @@ func (g *G) UserCurrent() Location { frame := it.Frame() if frame.Call.Fn != nil { name := frame.Call.Fn.Name - if strings.Contains(name, ".") && (!strings.HasPrefix(name, "runtime.") || frame.Call.Fn.exportedRuntime()) && !strings.HasPrefix(name, "internal/") && !strings.HasPrefix(name, "runtime/internal") { + if strings.Contains(name, ".") && (!strings.HasPrefix(name, "runtime.") || frame.Call.Fn.exportedRuntime()) && !strings.HasPrefix(name, "internal/") && !strings.HasPrefix(name, "runtime/internal") && !strings.HasPrefix(name, "iter.") { return frame.Call } }