From ce6e5029b3914882ff14ca582d3eecf46435c2f7 Mon Sep 17 00:00:00 2001 From: hmdsefi Date: Sun, 1 Nov 2026 00:10:11 +0400 Subject: [PATCH] feat(runner/toolexec): rewrite sync method values, interface locks, conds and once functions --- .../toolexec/cmd/faultline-toolexec/main.go | 2 +- .../cmd/faultline-toolexec/rewrite.go | 140 ++++++------ .../cmd/faultline-toolexec/rewrite_sync.go | 165 ++++++++++++++ .../faultline-toolexec/rewrite_sync_test.go | 208 ++++++++++++++++++ .../cmd/faultline-toolexec/version_test.go | 4 +- runner/toolexec/sched/sync.go | 4 +- 6 files changed, 441 insertions(+), 82 deletions(-) create mode 100644 runner/toolexec/cmd/faultline-toolexec/rewrite_sync.go create mode 100644 runner/toolexec/cmd/faultline-toolexec/rewrite_sync_test.go diff --git a/runner/toolexec/cmd/faultline-toolexec/main.go b/runner/toolexec/cmd/faultline-toolexec/main.go index 5add29d..c059c62 100644 --- a/runner/toolexec/cmd/faultline-toolexec/main.go +++ b/runner/toolexec/cmd/faultline-toolexec/main.go @@ -39,7 +39,7 @@ import ( // The line is part of the go command's tool IDs, so change version whenever the rewriter, the // rewrite set or a generated file changes (XTE-022): otherwise cached packages compiled by an // older rewriter are reused. -const version = "3" +const version = "4" func fail(format string, args ...any) { fmt.Fprintf(os.Stderr, "faultline-toolexec: "+format+"\n", args...) diff --git a/runner/toolexec/cmd/faultline-toolexec/rewrite.go b/runner/toolexec/cmd/faultline-toolexec/rewrite.go index 9067ac7..0ae3603 100644 --- a/runner/toolexec/cmd/faultline-toolexec/rewrite.go +++ b/runner/toolexec/cmd/faultline-toolexec/rewrite.go @@ -38,7 +38,7 @@ type rewriter struct { n int // temporary-name counter sched bool // the file now uses sched reflect bool // the file now uses reflect - rands map[string]bool // local names of rewritten math/rand imports + keeps map[string]string // local name -> declaration that keeps a replaced import used points map[int]bool // offsets where a point was inserted gotos map[types.Object]bool // labels a goto statement names } @@ -81,12 +81,13 @@ const ( ) // render returns src[lo:hi] with the edits inside it applied. Edits nested in another edit are -// applied by that edit's text function (through render on sub-ranges). +// applied by that edit's text function (through render on sub-ranges). An insertion at either end +// of a sub-range belongs to the enclosing text, which applies it around the edit. func (r *rewriter) render(lo, hi int) string { var b strings.Builder cur := lo for _, e := range r.edits { - if e.pos < cur || e.end > hi || (e.pos == e.end && e.pos >= hi) { + if e.pos < cur || e.end > hi || (e.pos == e.end && (e.pos == lo || e.pos >= hi)) { continue } b.Write(r.src[cur:e.pos]) @@ -131,13 +132,13 @@ func (r *rewriter) output() string { imports = append(imports, `import __fl_reflect "reflect"`) } var tail string - names := make([]string, 0, len(r.rands)) - for n := range r.rands { + names := make([]string, 0, len(r.keeps)) + for n := range r.keeps { names = append(names, n) } slices.Sort(names) for _, n := range names { - tail += "\nvar _ " + n + ".Source\n" + tail += "\nvar _ " + r.keeps[n] + "\n" } if len(gaps) > 0 { tail += "\nvar _ = __fl_sched.Gaps(" + strconv.Quote(strings.Join(gaps, "\n")) + ")\n" @@ -167,7 +168,7 @@ func (r *rewriter) sortedGaps() []string { // rewrite walks the file once and records the edits of EXB-034. func (r *rewriter) rewrite() { - r.rands, r.points, r.gotos = map[string]bool{}, map[int]bool{}, map[types.Object]bool{} + r.keeps, r.points, r.gotos = map[string]string{}, map[int]bool{}, map[types.Object]bool{} ast.Inspect(r.file, func(n ast.Node) bool { if b, ok := n.(*ast.BranchStmt); ok && b.Tok == token.GOTO && b.Label != nil { if obj := r.info.Uses[b.Label]; obj != nil { @@ -195,11 +196,7 @@ func (r *rewriter) rewrite() { r.call(n, sel) } case *ast.SelectorExpr: - if !calls[n] { - if name, ok := r.syncMethod(n); ok { - r.gap(n, "method value "+name) - } - } + r.selector(n, calls[n]) } return true }) @@ -321,11 +318,10 @@ func (r *rewriter) deferStmt(start token.Pos, st *ast.DeferStmt) { return "{ " + d + " := " + r.text(call.Args[0]) + "; defer func() { __fl_sched.Point(); close(" + d + ") }() }" }) } else if sel, ok := call.Fun.(*ast.SelectorExpr); ok { - if name, ok := r.syncMethod(sel); ok && name == "WaitGroup.Wait" { + if name, ok := r.syncMethod(sel); ok && name == "WaitGroup.Wait" && !r.recvGap(st, sel) { d = r.tmp("d") r.add(r.off(st.Pos()), r.off(st.End()), func() string { - addr, _ := r.recvAddr(sel) - return "{ " + d + " := " + addr + "; defer func() { __fl_sched.Point(); " + d + ".Wait() }() }" + return "{ " + d + " := " + r.recvAddr(sel) + "; defer func() { __fl_sched.Point(); " + d + ".Wait() }() }" }) } } @@ -392,11 +388,18 @@ func (r *rewriter) goStmt(st *ast.GoStmt) { } } } - // The spec's form binds a sync method as a method value, which then runs outside sched. - if sel, ok := ast.Unparen(call.Fun).(*ast.SelectorExpr); ok { - if name, ok := r.syncMethod(sel); ok { - r.gap(st, "method value "+name) - } + // A sync method is bound as its rewritten method value (XTE-041); an interface Lock or Unlock, + // or a sync method whose receiver the file cannot name, is bound as a method value, which runs + // outside sched (XTE-042). + sel, _ := ast.Unparen(call.Fun).(*ast.SelectorExpr) + name, _ := r.syncMethod(sel) + helper := syncFuncs[name] + s := r.info.Selections[sel] + iface := s != nil && s.Kind() == types.MethodVal && r.ifaceLock(s) + if (helper != "" || iface) && r.recvGap(st, sel) { + helper = "" + } else if iface { + r.gap(st, "method value of an interface Lock or Unlock") } r.sched = true names := make([]string, len(ops)) @@ -413,6 +416,14 @@ func (r *rewriter) goStmt(st *ast.GoStmt) { continue } val := r.text(e) + if i == 0 && helper != "" { + val = "__fl_sched." + helper + "(" + r.recvAddr(sel) + ")" + } + if i == 2 && sel != nil { // AfterFunc binds its function wrapped, as in a call (XTE-041) + if p, n, _ := r.pkgFunc(sel); (p == "time" || p == "context") && n == "AfterFunc" && len(call.Args) == 2 { + val = "__fl_sched.Func(" + val + ")" + } + } if conv[i] != "" { val = conv[i] + "(" + val + ")" } @@ -923,54 +934,25 @@ func (r *rewriter) syncMethod(sel *ast.SelectorExpr) (string, bool) { if !ok || s.Kind() != types.MethodVal { return "", false } - fn, ok := s.Obj().(*types.Func) - if !ok || fn.Pkg() == nil || (fn.Pkg().Path() != "sync" && fn.Pkg().Path() != "sync/atomic") { - return "", false - } - sig, ok := fn.Type().(*types.Signature) - if !ok || sig.Recv() == nil { - return "", false - } - recv := sig.Recv().Type() - if p, ok := recv.(*types.Pointer); ok { - recv = p.Elem() - } - named, ok := recv.(*types.Named) - if !ok { - return "", false - } - return named.Obj().Name() + "." + fn.Name(), true + return r.syncMethodOf(s) } -// recvAddr returns the address expression of the sync value a method selection is called on. -func (r *rewriter) recvAddr(sel *ast.SelectorExpr) (string, bool) { - s := r.info.Selections[sel] - expr := "(" + r.text(sel.X) + ")" - t := r.info.TypeOf(sel.X) - idx := s.Index() - for _, i := range idx[:len(idx)-1] { - u := t - if p, ok := u.Underlying().(*types.Pointer); ok { - u = p.Elem() - } - st, ok := u.Underlying().(*types.Struct) - if !ok { - return "", false - } - f := st.Field(i) - expr += "." + f.Name() - t = f.Type() - } +// recvAddr returns the address expression of the sync value a method selection is called on, +// for a selection whose receiver the file can name (recvGap). +func (r *rewriter) recvAddr(sel *ast.SelectorExpr) string { + path, t, _ := r.fieldPath(sel) + expr := "(" + r.text(sel.X) + ")" + path if _, ok := t.Underlying().(*types.Pointer); ok { - return expr, true + return expr } - return "&" + expr, true + return "&" + expr } var syncCalls = map[string]string{ "Mutex.Lock": "Lock", "Mutex.Unlock": "Unlock", "RWMutex.Lock": "WLock", "RWMutex.Unlock": "WUnlock", "RWMutex.RLock": "RLock", "RWMutex.RUnlock": "RUnlock", - "Once.Do": "OnceDo", + "Once.Do": "OnceDo", "RWMutex.RLocker": "RLocker", "WaitGroup.Go": "WaitGroupGo", + "Cond.Wait": "CondWait", "Cond.Signal": "CondSignal", "Cond.Broadcast": "CondBroadcast", } var randV1 = map[string]bool{"ExpFloat64": true, "Float32": true, "Float64": true, "Int": true, "Int31": true, @@ -981,28 +963,27 @@ var randV2 = map[string]bool{"ExpFloat64": true, "Float32": true, "Float64": tru "Int32N": true, "Int64": true, "Int64N": true, "IntN": true, "NormFloat64": true, "Perm": true, "Shuffle": true, "Uint": true, "Uint32": true, "Uint32N": true, "Uint64": true, "Uint64N": true, "UintN": true} -// call rewrites sync method calls and math/rand package-level calls. +// call rewrites sync method calls, Lock and Unlock calls through interfaces, math/rand +// package-level calls and the function arguments of AfterFunc; a sync method or interface lock +// call whose receiver the file cannot name is a gap. func (r *rewriter) call(call *ast.CallExpr, sel *ast.SelectorExpr) { - if name, ok := r.syncMethod(sel); ok { - switch { - case strings.HasPrefix(name, "Cond."): - r.gap(call, "sync.Cond") - return - case r.info.Selections[sel].Obj().Pkg().Path() == "sync/atomic": - r.gap(call, "sync/atomic") + if s := r.info.Selections[sel]; s != nil && s.Kind() == types.MethodVal && r.ifaceLock(s) { + if r.recvGap(call, sel) { return } + fn := map[string]string{"Lock": "ILock", "Unlock": "IUnlock"}[sel.Sel.Name] + r.sched = true + r.add(r.off(call.Pos()), r.off(call.End()), func() string { return "__fl_sched." + fn + "(" + r.ifaceRecv(sel) + ")" }) + return + } + if name, ok := r.syncMethod(sel); ok { fn, ok := syncCalls[name] - if !ok { + if !ok || r.recvGap(call, sel) { return } r.sched = true r.add(r.off(call.Pos()), r.off(call.End()), func() string { - addr, ok := r.recvAddr(sel) - if !ok { - return r.render(r.off(call.Pos()), r.off(call.End())) - } - args := addr + args := r.recvAddr(sel) for _, a := range call.Args { args += ", " + r.text(a) } @@ -1020,15 +1001,20 @@ func (r *rewriter) call(call *ast.CallExpr, sel *ast.SelectorExpr) { } switch path := pn.Imported().Path(); { case path == "math/rand" && randV1[sel.Sel.Name]: - r.sched, r.rands[id.Name] = true, true + r.sched = true + r.keepImport(path, id) r.add(r.off(sel.Pos()), r.off(sel.End()), func() string { return "__fl_sched.RandV1()." + sel.Sel.Name }) case path == "math/rand/v2" && randV2[sel.Sel.Name]: - r.sched, r.rands[id.Name] = true, true + r.sched = true + r.keepImport(path, id) r.add(r.off(sel.Pos()), r.off(sel.End()), func() string { return "__fl_sched.Rand()." + sel.Sel.Name }) case path == "math/rand/v2" && sel.Sel.Name == "N": r.gap(call, "math/rand/v2.N") - case path == "sync/atomic": - r.gap(call, "sync/atomic") + case (path == "time" || path == "context") && sel.Sel.Name == "AfterFunc" && len(call.Args) == 2: + arg := call.Args[1] + r.sched = true + r.add(r.off(arg.Pos()), r.off(arg.Pos()), func() string { return "__fl_sched.Func(" }) + r.add(r.off(arg.End()), r.off(arg.End()), func() string { return ")" }) case path == "reflect" && sel.Sel.Name == "Select": r.gap(call, "reflect.Select") } diff --git a/runner/toolexec/cmd/faultline-toolexec/rewrite_sync.go b/runner/toolexec/cmd/faultline-toolexec/rewrite_sync.go new file mode 100644 index 0000000..04978d7 --- /dev/null +++ b/runner/toolexec/cmd/faultline-toolexec/rewrite_sync.go @@ -0,0 +1,165 @@ +// Copyright 2026 Hamed Yousefi +// SPDX-License-Identifier: MPL-2.0 + +package main + +import ( + "fmt" + "go/ast" + "go/types" +) + +// syncFuncs maps the sync methods whose method values are rewritten to sched's helpers. +var syncFuncs = map[string]string{ + "Mutex.Lock": "LockFunc", "Mutex.Unlock": "UnlockFunc", + "RWMutex.RLock": "RLockFunc", "RWMutex.RUnlock": "RUnlockFunc", "RWMutex.Lock": "WLockFunc", "RWMutex.Unlock": "WUnlockFunc", + "Once.Do": "OnceDoFunc", "WaitGroup.Wait": "WaitFunc", "WaitGroup.Go": "GoFunc", + "Cond.Wait": "CondWaitFunc", "Cond.Signal": "CondSignalFunc", "Cond.Broadcast": "CondBroadcastFunc", +} + +// pkgFuncs maps package-level functions to the sched functions that replace them, called or as +// values. +var pkgFuncs = map[string]string{ + "runtime.Gosched": "Point", + "sync.OnceFunc": "OnceFunc", "sync.OnceValue": "OnceValue", "sync.OnceValues": "OnceValues", +} + +// importKeep is, per import path, the declaration that keeps an import used after the rewriter +// replaced its last use (%s is the import's local name). +var importKeep = map[string]string{ + "runtime": "= %s.Gosched", "sync": "%s.Mutex", "math/rand": "%s.Source", "math/rand/v2": "%s.Source", + "reflect": "%s.Value", "maps": "= %s.Copy[map[int]int, map[int]int]", +} + +// keepImport records that the file must keep the import id names in use. +func (r *rewriter) keepImport(path string, id *ast.Ident) { + r.keeps[id.Name] = fmt.Sprintf(importKeep[path], id.Name) +} + +// pkgFunc returns the import path and name of a package-level function sel names, and the +// package identifier, or "" when sel is not a qualified identifier. +func (r *rewriter) pkgFunc(sel *ast.SelectorExpr) (path, name string, id *ast.Ident) { + id, ok := sel.X.(*ast.Ident) + if !ok { + return "", "", nil + } + pn, ok := r.info.Uses[id].(*types.PkgName) + if !ok { + return "", "", nil + } + return pn.Imported().Path(), sel.Sel.Name, id +} + +// selector rewrites the functions of pkgFuncs, called or not, and, for a selector that is not +// called, the method values of syncFuncs; method expressions of the methods of syncFuncs and +// syncCalls, called or not, and method values of interface Lock and Unlock are gaps, and so is a +// method value of syncFuncs or of an interface Lock or Unlock whose receiver the file cannot name +// (XTE-041, XTE-042). +func (r *rewriter) selector(sel *ast.SelectorExpr, called bool) { + if path, name, id := r.pkgFunc(sel); id != nil { + if repl, ok := pkgFuncs[path+"."+name]; ok { + r.sched = true + r.keepImport(path, id) + r.add(r.off(sel.Pos()), r.off(sel.End()), func() string { return "__fl_sched." + repl }) + } + return + } + s, ok := r.info.Selections[sel] + if !ok || (called && s.Kind() != types.MethodExpr) { + return + } + if name, ok := r.syncMethodOf(s); ok { + helper := syncFuncs[name] + switch { + case s.Kind() == types.MethodExpr && (helper != "" || syncCalls[name] != ""): + r.gap(sel, "method expression "+name) + case helper == "", r.recvGap(sel, sel): + default: + r.sched = true + r.add(r.off(sel.Pos()), r.off(sel.End()), func() string { return "__fl_sched." + helper + "(" + r.recvAddr(sel) + ")" }) + } + return + } + if s.Kind() == types.MethodVal && r.ifaceLock(s) && !r.recvGap(sel, sel) { + r.gap(sel, "method value of an interface Lock or Unlock") + } +} + +// syncMethodOf names the method a selection selects ("Mutex.Lock"), as a method value or a +// method expression, when its receiver is a sync or sync/atomic type other than an interface. +func (r *rewriter) syncMethodOf(s *types.Selection) (string, bool) { + if s.Kind() != types.MethodVal && s.Kind() != types.MethodExpr { + return "", false + } + fn, ok := s.Obj().(*types.Func) + if !ok || fn.Pkg() == nil || (fn.Pkg().Path() != "sync" && fn.Pkg().Path() != "sync/atomic") { + return "", false + } + sig, ok := fn.Type().(*types.Signature) + if !ok || sig.Recv() == nil { + return "", false + } + recv := sig.Recv().Type() + if p, ok := recv.(*types.Pointer); ok { + recv = p.Elem() + } + named, ok := recv.(*types.Named) + if !ok || types.IsInterface(named) { + return "", false + } + return named.Obj().Name() + "." + fn.Name(), true +} + +// ifaceLock reports whether s selects a method Lock or Unlock, without parameters and results, +// of an interface. +func (r *rewriter) ifaceLock(s *types.Selection) bool { + fn, ok := s.Obj().(*types.Func) + if !ok || (fn.Name() != "Lock" && fn.Name() != "Unlock") { + return false + } + sig, ok := fn.Type().(*types.Signature) + if !ok || sig.Recv() == nil || sig.Params().Len() != 0 || sig.Results().Len() != 0 { + return false + } + return types.IsInterface(sig.Recv().Type()) +} + +// ifaceRecv returns the interface value a Lock or Unlock call selects, following embedded +// struct fields. +func (r *rewriter) ifaceRecv(sel *ast.SelectorExpr) string { + path, _, _ := r.fieldPath(sel) + return "(" + r.text(sel.X) + ")" + path +} + +// fieldPath returns the embedded fields a method selection is promoted through (".T.Locker") +// and the type of the last one, or of sel.X when there is none, and true; or false when the file +// cannot name one of them: an unexported field of another package. +func (r *rewriter) fieldPath(sel *ast.SelectorExpr) (string, types.Type, bool) { + path, t := "", r.info.TypeOf(sel.X) + idx := r.info.Selections[sel].Index() + for _, i := range idx[:len(idx)-1] { + if p, ok := t.Underlying().(*types.Pointer); ok { + t = p.Elem() + } + st, ok := t.Underlying().(*types.Struct) + if !ok { + return "", nil, false + } + f := st.Field(i) + if !f.Exported() && f.Pkg() != r.pkg { + return "", nil, false + } + path, t = path+"."+f.Name(), f.Type() + } + return path, t, true +} + +// recvGap records a gap at n and reports true when the file cannot name the receiver of the +// method selection sel, so that its sync row leaves it as written (XTE-042). +func (r *rewriter) recvGap(n ast.Node, sel *ast.SelectorExpr) bool { + if _, _, ok := r.fieldPath(sel); ok { + return false + } + r.gap(n, "method receiver not nameable in this file") + return true +} diff --git a/runner/toolexec/cmd/faultline-toolexec/rewrite_sync_test.go b/runner/toolexec/cmd/faultline-toolexec/rewrite_sync_test.go new file mode 100644 index 0000000..03212dc --- /dev/null +++ b/runner/toolexec/cmd/faultline-toolexec/rewrite_sync_test.go @@ -0,0 +1,208 @@ +// Copyright 2026 Hamed Yousefi +// SPDX-License-Identifier: MPL-2.0 + +package main + +import ( + "os" + "path/filepath" + "slices" + "strconv" + "strings" + "testing" +) + +// AT-XTE-11: the sync rows of XTE-041. +func TestATXTE11_SyncRows(t *testing.T) { + for i, c := range []struct{ src, want string }{ + { // method values of sync methods + src: "package p\n\nimport \"sync\"\n\ntype S struct {\n\tmu sync.Mutex\n\twg sync.WaitGroup\n\tc *sync.Cond\n}\n\n" + + "func f(s *S, rw *sync.RWMutex, o *sync.Once) []any {\n\treturn []any{s.mu.Lock, s.mu.Unlock, rw.RLock, rw.Lock, o.Do, s.wg.Wait, s.wg.Go, s.c.Signal}\n}\n", + want: header + "\nimport \"sync\"\n\ntype S struct {\n\tmu sync.Mutex\n\twg sync.WaitGroup\n\tc *sync.Cond\n}\n\n" + + "func f(s *S, rw *sync.RWMutex, o *sync.Once) []any {\n\treturn []any{__fl_sched.LockFunc(&(s.mu)), __fl_sched.UnlockFunc(&(s.mu)), " + + "__fl_sched.RLockFunc((rw)), __fl_sched.WLockFunc((rw)), __fl_sched.OnceDoFunc((o)), __fl_sched.WaitFunc(&(s.wg)), " + + "__fl_sched.GoFunc(&(s.wg)), __fl_sched.CondSignalFunc((s.c))}\n}\n", + }, + { // go of a sync method binds the rewritten method value + src: "package p\n\nimport \"sync\"\n\nfunc f(mu *sync.Mutex, o *sync.Once, fn func()) {\n\tgo mu.Lock()\n\tgo o.Do(fn)\n}\n", + want: header + "\nimport \"sync\"\n\nfunc f(mu *sync.Mutex, o *sync.Once, fn func()) {\n\t{ __fl_g1 := __fl_sched.LockFunc((mu)); __fl_sched.Go(func() { __fl_g1() }) }\n\t{ __fl_g2, __fl_g3 := __fl_sched.OnceDoFunc((o)), fn; __fl_sched.Go(func() { __fl_g2(__fl_g3) }) }\n}\n", + }, + { // Lock and Unlock through interface values, promoted ones too, and RLocker + src: "package p\n\nimport \"sync\"\n\ntype locker interface {\n\tLock()\n\tUnlock()\n\tName() string\n}\n\ntype S struct{ sync.Locker }\n\n" + + "func f(l sync.Locker, m locker, s S, rw *sync.RWMutex) {\n\tl.Lock()\n\tl.Unlock()\n\tm.Lock()\n\tdefer m.Unlock()\n\ts.Lock()\n\tr := rw.RLocker()\n\tr.Lock()\n\tr.Unlock()\n}\n", + want: header + "\nimport \"sync\"\n\ntype locker interface {\n\tLock()\n\tUnlock()\n\tName() string\n}\n\ntype S struct{ sync.Locker }\n\n" + + "func f(l sync.Locker, m locker, s S, rw *sync.RWMutex) {\n\t__fl_sched.ILock((l))\n\t__fl_sched.IUnlock((l))\n\t__fl_sched.ILock((m))\n\tdefer __fl_sched.IUnlock((m))\n" + + "\t__fl_sched.ILock((s).Locker)\n\tr := __fl_sched.RLocker((rw))\n\t__fl_sched.ILock((r))\n\t__fl_sched.IUnlock((r))\n}\n", + }, + { // Cond, WaitGroup.Go, the Once functions, AfterFunc and Gosched + src: "package p\n\nimport (\n\t\"context\"\n\t\"runtime\"\n\t\"sync\"\n\t\"time\"\n)\n\n" + + "func f(ctx context.Context, c *sync.Cond, wg *sync.WaitGroup, mu *sync.Mutex) {\n\tc.Wait()\n\tc.Signal()\n\tc.Broadcast()\n\twg.Go(func() {})\n" + + "\tg := sync.OnceFunc(func() {})\n\tv := sync.OnceValue(func() int { return 1 })\n\tvs := sync.OnceValues[int, error](func() (int, error) { return 1, nil })\n\t_, _, _ = g, v, vs\n" + + "\ttime.AfterFunc(time.Second, mu.Unlock)\n\tcontext.AfterFunc(ctx, func() {})\n\truntime.Gosched()\n\tyield := runtime.Gosched\n\tyield()\n}\n", + want: header + "\nimport (\n\t\"context\"\n\t\"runtime\"\n\t\"sync\"\n\t\"time\"\n)\n\n" + + "func f(ctx context.Context, c *sync.Cond, wg *sync.WaitGroup, mu *sync.Mutex) {\n\t__fl_sched.CondWait((c))\n\t__fl_sched.CondSignal((c))\n\t__fl_sched.CondBroadcast((c))\n\t__fl_sched.WaitGroupGo((wg), func() {})\n" + + "\tg := __fl_sched.OnceFunc(func() {})\n\tv := __fl_sched.OnceValue(func() int { return 1 })\n\tvs := __fl_sched.OnceValues[int, error](func() (int, error) { return 1, nil })\n\t_, _, _ = g, v, vs\n" + + "\ttime.AfterFunc(time.Second, __fl_sched.Func(__fl_sched.UnlockFunc((mu))))\n\tcontext.AfterFunc(ctx, __fl_sched.Func(func() {}))\n\t__fl_sched.Point()\n\tyield := __fl_sched.Point\n\tyield()\n}\n" + + "\nvar _ = runtime.Gosched\n\nvar _ sync.Mutex\n", + }, + } { + got, gaps := rewriteSource(t, c.src, "") + if got != c.want || !slices.Equal(gaps, nil) { + t.Errorf("case %d: got\n%s\ngaps %q\nwant\n%s", i, got, gaps, c.want) + } + } +} + +// The XTE-042 gaps next to the sync rows: a called method expression, go of an interface Lock, an +// interface Unlock method value and a method expression of RLocker; and a rewritten Lock whose +// receiver starts the statement that got a point, so the point stays out of the receiver. +func TestSyncRowsGaps(t *testing.T) { + src := "package p\n\nimport \"sync\"\n\ntype S struct{ mu sync.Mutex }\n\n" + + "func f(ch chan *S, mu *sync.Mutex, l sync.Locker) any {\n\t(<-ch).mu.Lock()\n\t(*sync.Mutex).Lock(mu)\n\tgo l.Lock()\n\treturn l.Unlock\n}\n" + + "\nvar rl = (*sync.RWMutex).RLocker\n" + wantGaps := []string{"/src/p/a.go:9: method expression Mutex.Lock", "/src/p/a.go:10: method value of an interface Lock or Unlock", + "/src/p/a.go:11: method value of an interface Lock or Unlock", "/src/p/a.go:14: method expression RWMutex.RLocker"} + want := header + "\nimport \"sync\"\n\ntype S struct{ mu sync.Mutex }\n\n" + + "func f(ch chan *S, mu *sync.Mutex, l sync.Locker) any {\n\t__fl_sched.Point(); __fl_sched.Lock(&((<-ch).mu))\n\t(*sync.Mutex).Lock(mu)\n" + + "\t{ __fl_g1 := l.Lock; __fl_sched.Go(func() { __fl_g1() }) }\n\treturn l.Unlock\n}\n\nvar rl = (*sync.RWMutex).RLocker\n" + + "\nvar _ = __fl_sched.Gaps(" + strconv.Quote(strings.Join(wantGaps, "\n")) + ")\n" + if got, gaps := rewriteSource(t, src, ""); got != want || !slices.Equal(gaps, wantGaps) { + t.Errorf("got\n%s\ngaps %q\nwant\n%s\ngaps %q", got, gaps, want, wantGaps) + } +} + +// The sync rows AT-XTE-11 leaves unpinned: the method values of RUnlock, Unlock on an RWMutex, Wait +// and Broadcast; Lock calls and method values through interfaces whose Lock takes an argument or +// returns a result, which stay as written; an interface Lock reached through an embedded pointer; +// a file whose only rewrite is one of Gosched, OnceFunc, an interface Lock or AfterFunc, which +// still imports sched; AfterFunc under go, which wraps the function it binds; and an AfterFunc of +// another package, which stays as written. +func TestSyncRowsPinned(t *testing.T) { + for i, c := range []struct{ src, dep, want string }{ + { + src: "package p\n\nimport (\n\t\"context\"\n\t\"sync\"\n)\n\ntype T struct{ sync.Locker }\n\ntype S struct{ *T }\n\n" + + "type errLocker interface{ Lock() error }\n\ntype argLocker interface{ Lock(context.Context) }\n\n" + + "func f(ctx context.Context, rw *sync.RWMutex, c *sync.Cond, p *S, e errLocker, a argLocker) []any {\n" + + "\tp.Lock()\n\t_ = e.Lock()\n\ta.Lock(ctx)\n\treturn []any{rw.RUnlock, rw.Unlock, c.Wait, c.Broadcast, e.Lock, a.Lock}\n}\n", + want: header + "\nimport (\n\t\"context\"\n\t\"sync\"\n)\n\ntype T struct{ sync.Locker }\n\ntype S struct{ *T }\n\n" + + "type errLocker interface{ Lock() error }\n\ntype argLocker interface{ Lock(context.Context) }\n\n" + + "func f(ctx context.Context, rw *sync.RWMutex, c *sync.Cond, p *S, e errLocker, a argLocker) []any {\n" + + "\t__fl_sched.ILock((p).T.Locker)\n\t_ = e.Lock()\n\ta.Lock(ctx)\n\treturn []any{__fl_sched.RUnlockFunc((rw)), " + + "__fl_sched.WUnlockFunc((rw)), __fl_sched.CondWaitFunc((c)), __fl_sched.CondBroadcastFunc((c)), e.Lock, a.Lock}\n}\n", + }, + { + src: "package p\n\nimport \"runtime\"\n\nfunc f() {\n\truntime.Gosched()\n}\n", + want: header + "\nimport \"runtime\"\n\nfunc f() {\n\t__fl_sched.Point()\n}\n\nvar _ = runtime.Gosched\n", + }, + { + src: "package p\n\nimport \"sync\"\n\nvar g = sync.OnceFunc(func() {})\n", + want: header + "\nimport \"sync\"\n\nvar g = __fl_sched.OnceFunc(func() {})\n\nvar _ sync.Mutex\n", + }, + { + src: "package p\n\nimport \"sync\"\n\nfunc f(l sync.Locker) {\n\tl.Lock()\n}\n", + want: header + "\nimport \"sync\"\n\nfunc f(l sync.Locker) {\n\t__fl_sched.ILock((l))\n}\n", + }, + { + src: "package p\n\nimport \"time\"\n\nfunc f(g func()) {\n\ttime.AfterFunc(time.Second, g)\n}\n", + want: header + "\nimport \"time\"\n\nfunc f(g func()) {\n\ttime.AfterFunc(time.Second, __fl_sched.Func(g))\n}\n", + }, + { + src: "package p\n\nimport (\n\t\"context\"\n\t\"sync\"\n\t\"time\"\n)\n\nfunc f(ctx context.Context, g func(), mu *sync.Mutex) {\n" + + "\tgo time.AfterFunc(time.Second, g)\n\tgo context.AfterFunc(ctx, func() { g() })\n\tgo time.AfterFunc(time.Second, mu.Unlock)\n}\n", + want: header + "\nimport (\n\t\"context\"\n\t\"sync\"\n\t\"time\"\n)\n\nfunc f(ctx context.Context, g func(), mu *sync.Mutex) {\n" + + "\t{ __fl_g1, __fl_g2 := time.AfterFunc, __fl_sched.Func(g); __fl_sched.Go(func() { __fl_g1(time.Second, __fl_g2) }) }\n" + + "\t{ __fl_g3, __fl_g4, __fl_g5 := context.AfterFunc, ctx, __fl_sched.Func(func() { g() }); __fl_sched.Go(func() { __fl_g3(__fl_g4, __fl_g5) }) }\n" + + "\t{ __fl_g6, __fl_g7 := time.AfterFunc, __fl_sched.Func(__fl_sched.UnlockFunc((mu))); __fl_sched.Go(func() { __fl_g6(time.Second, __fl_g7) }) }\n}\n", + }, + { // an AfterFunc of another package is not wrapped, called or under go + src: "package p\n\nimport \"example.com/q\"\n\nfunc f(s string) {\n\tq.AfterFunc(1, s)\n\tgo q.AfterFunc(1, s)\n}\n", + dep: "package q\n\nfunc AfterFunc(n int, s string) {}\n", + want: header + "\nimport \"example.com/q\"\n\nfunc f(s string) {\n\tq.AfterFunc(1, s)\n" + + "\t{ __fl_g1, __fl_g2 := q.AfterFunc, s; __fl_sched.Go(func() { __fl_g1(1, __fl_g2) }) }\n}\n", + }, + { // Lock and Unlock through interfaces that are not a sync.Locker, and a type parameter + src: "package p\n\n" + narrowLockers + "\n" + narrowLockersFunc, + want: header + "\n" + narrowLockers + "\n" + strings.NewReplacer("l.Lock()", "__fl_sched.ILock((l))", "c.Unlock()", "__fl_sched.IUnlock((c))", "e.Unlock()", "__fl_sched.IUnlock((e))", "t.Lock()", "__fl_sched.ILock((t))").Replace(narrowLockersFunc), + }, + } { + got, gaps := rewriteSource(t, c.src, c.dep) + if got != c.want || !slices.Equal(gaps, nil) { + t.Errorf("case %d: got\n%s\ngaps %q\nwant\n%s", i, got, gaps, c.want) + } + } +} + +// narrowLockers declares interfaces with a Lock or an Unlock that are not a sync.Locker: Lock +// only, Unlock next to another method, and Unlock next to a Lock with a result. +const narrowLockers = "type onlyLock interface{ Lock() }\n\ntype unlockCloser interface {\n\tUnlock()\n\tClose() error\n}\n\n" + + "type errLock interface {\n\tLock() error\n\tUnlock()\n}\n" + +// narrowLockersFunc locks and unlocks through narrowLockers and a type parameter. +const narrowLockersFunc = "func F[T onlyLock](l onlyLock, c unlockCloser, e errLock, t T) {\n\tl.Lock()\n\tc.Unlock()\n\te.Unlock()\n\tt.Lock()\n}\n" + +// receiverDep is package example.com/q, whose types promote Lock, Unlock and Wait through +// unexported embedded fields; E promotes them through an exported one. +const receiverDep = "package q\n\nimport \"sync\"\n\ntype locker interface {\n\tLock()\n\tUnlock()\n}\n\n" + + "type inner struct{ sync.Mutex }\n\ntype group struct{ sync.WaitGroup }\n\n" + + "type I struct{ locker }\n\ntype M struct{ inner }\n\ntype W struct{ group }\n\ntype E struct{ sync.Mutex }\n" + +// receiverUse calls the sync rows on receiverDep's types. +const receiverUse = "func G(i q.I, m *q.M, w *q.W) func() {\n\ti.Lock()\n\tm.Lock()\n\tgo i.Unlock()\n\tgo m.Unlock()\n\tgo (i.Lock)()\n" + + "\tdefer w.Wait()\n\t_ = i.Unlock\n\treturn m.Lock\n}\n" + +// AT-XTE-13, the row of XTE-042 for receivers the file cannot name: a sync method or an interface +// Lock or Unlock reached through an unexported embedded field of another package is left as +// written (go binds the method value as written) and records exactly its kind once per site; +// an exported field of another package and an unexported field of the file's own package are +// rewritten. +func TestATXTE13_MethodReceiverNotNameable(t *testing.T) { + src := "package p\n\nimport (\n\t\"sync\"\n\n\t\"example.com/q\"\n)\n\ntype own struct{ sync.Mutex }\n\ntype T struct{ own }\n\n" + + receiverUse + "\nfunc H(e *q.E, t *T) {\n\te.Lock()\n\tt.Lock()\n}\n" + const kind = "method receiver not nameable in this file" + var wantGaps []string + for _, l := range []int{14, 15, 16, 17, 18, 19, 20, 21} { + wantGaps = append(wantGaps, "/src/p/a.go:"+strconv.Itoa(l)+": "+kind) + } + want := header + "\nimport (\n\t\"sync\"\n\n\t\"example.com/q\"\n)\n\ntype own struct{ sync.Mutex }\n\ntype T struct{ own }\n\n" + + "func G(i q.I, m *q.M, w *q.W) func() {\n\ti.Lock()\n\tm.Lock()\n\t{ __fl_g1 := i.Unlock; __fl_sched.Go(func() { __fl_g1() }) }\n" + + "\t{ __fl_g2 := m.Unlock; __fl_sched.Go(func() { __fl_g2() }) }\n\t{ __fl_g3 := (i.Lock); __fl_sched.Go(func() { __fl_g3() }) }\n" + + "\tdefer w.Wait()\n\t_ = i.Unlock\n\treturn m.Lock\n}\n" + + "\nfunc H(e *q.E, t *T) {\n\t__fl_sched.Lock(&(e).Mutex)\n\t__fl_sched.Lock(&(t).own.Mutex)\n}\n" + + "\nvar _ = __fl_sched.Gaps(" + strconv.Quote(strings.Join(wantGaps, "\n")) + ")\n" + if got, gaps := rewriteSource(t, src, receiverDep); got != want || !slices.Equal(gaps, wantGaps) { + t.Errorf("got\n%s\ngaps %q\nwant\n%s\ngaps %q", got, gaps, want, wantGaps) + } +} + +// The sync rows compile through the wrapper for receivers the rewriter cannot pass as a +// sync.Locker or cannot spell: narrowLockers and a type parameter go to ILock and IUnlock, and +// receiverDep's unexported embedded fields are gaps. +func TestSyncRowsCompileThroughTheWrapper(t *testing.T) { + tc := currentToolchain(t) + state := stateCopy(t, tc) + prepared := readPrepared(t, filepath.Join(state, tc.key()+".importcfg")) + dir := userModule(t, "q/q.go", receiverDep, + "u.go", "package u\n\nimport \"example.com/u/q\"\n\n"+narrowLockers+"\n"+narrowLockersFunc+"\n"+receiverUse) + in := func(rel string) string { return filepath.Join(dir, rel) } + cfg := "packagefile sync=" + prepared["sync"] + "\npackagefile example.com/u/q=" + in("q.a") + "\n" + if err := os.WriteFile(in("importcfg"), []byte(cfg), 0o600); err != nil { + t.Fatal(err) + } + keep := t.TempDir() + env := []string{"FAULTLINE_TOOLEXEC_STATE=" + state, "FAULTLINE_TOOLEXEC_KEEP=" + keep} + for _, c := range []struct{ ip, out, file string }{{"example.com/u/q", "q.a", "q/q.go"}, {"example.com/u", "u.a", "u.go"}} { + args := compileArgs(tc, c.ip, in("importcfg"), in(c.out), in(c.file)) + if out, code := wrapperRun(t, append(env, "TOOLEXEC_IMPORTPATH="+c.ip), args...); code != 0 { + t.Fatalf("compile %s: exit %d\n%s", c.ip, code, out) + } + } + kept, err := os.ReadFile(filepath.Join(keep, "example.com_u", "u.go")) + if err != nil { + t.Fatal(err) + } + for _, s := range []string{"__fl_sched.ILock((l))", "__fl_sched.IUnlock((c))", "__fl_sched.IUnlock((e))", "__fl_sched.ILock((t))", "method receiver not nameable in this file"} { + if !strings.Contains(string(kept), s) { + t.Errorf("kept u.go has no %q:\n%s", s, kept) + } + } +} diff --git a/runner/toolexec/cmd/faultline-toolexec/version_test.go b/runner/toolexec/cmd/faultline-toolexec/version_test.go index 4773130..c890d8d 100644 --- a/runner/toolexec/cmd/faultline-toolexec/version_test.go +++ b/runner/toolexec/cmd/faultline-toolexec/version_test.go @@ -18,8 +18,8 @@ import ( // file of this package needs a new version: raise version in main.go, then set these two to the // values the failing test prints. const ( - stampVersion = "3" - stampHash = "4dcd2df755235960f2502880d61276530c01d0b01e15af74a87a7732e3a56859" + stampVersion = "4" + stampHash = "315bd4e6a110c015de4491b86b5b0c93c69d71d8671c1006269d3d2d6a01d6ac" ) // AT-XTE-04 diff --git a/runner/toolexec/sched/sync.go b/runner/toolexec/sched/sync.go index e820bc1..34115bf 100644 --- a/runner/toolexec/sched/sync.go +++ b/runner/toolexec/sched/sync.go @@ -10,7 +10,7 @@ import ( // ILock replaces a Lock call through an interface value: mutexes go through Lock and WLock, // other lockers are called directly. -func ILock(l sync.Locker) { +func ILock(l interface{ Lock() }) { switch m := l.(type) { case *sync.Mutex: Lock(m) @@ -22,7 +22,7 @@ func ILock(l sync.Locker) { } // IUnlock replaces an Unlock call through an interface value. -func IUnlock(l sync.Locker) { +func IUnlock(l interface{ Unlock() }) { switch m := l.(type) { case *sync.Mutex: Unlock(m)