Skip to content

Commit

Permalink
cmd/compile: for rangefunc, add checks and tests, fix panic interactions
Browse files Browse the repository at this point in the history
Modify rangefunc #next protocol to make it more robust

Extra-terrible nests of rangefunc iterators caused the
prior implementation to misbehave non-locally (in outer loops).

Add more rangefunc exit flag tests, parallel and tricky

This tests the assertion that a rangefunc iterator running
in parallel can trigger the race detector if any of the
parallel goroutines attempts an early exit.  It also
verifies that if everything else is carefully written,
that it does NOT trigger the race detector if all the
parts run time completion.

Another test tries to rerun a yield function within a loop,
so that any per-line shared checking would be fooled.

Added all the use-of-body/yield-function checking.

These checks handle pathological cases that would cause
rangefunc for loops to behave in surprising ways (compared
to "regular" for loops).  For example, a rangefunc iterator
might defer-recover a panic thrown in the syntactic body
of a loop; this notices the fault and panics with an
explanation

Modified closure naming to ID rangefunc bodies

Add a "-range<N>" suffix to the name of any closure generated for
a rangefunc loop body, as provided in Alessandro Arzilli's CL
(which is merged into this one).

Fix return values for panicky range functions

This removes the delayed implementation of "return x" by
ensuring that return values (in rangefunc-return-containing
functions) always have names and translating the "return x"
into "#rv1 = x" where #rv1 is the synthesized name of the
first result.

Updates #61405.

Change-Id: I933299ecce04ceabcf1c0c2de8e610b2ecd1cfd8
Reviewed-on: https://go-review.googlesource.com/c/go/+/584596
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Tim King <taking@google.com>
  • Loading branch information
dr2chase committed May 21, 2024
1 parent 6438658 commit 5a9dabc
Show file tree
Hide file tree
Showing 16 changed files with 1,723 additions and 572 deletions.
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/inline/inl.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ opSwitch:
case "throw":
v.budget -= inlineExtraThrowCost
break opSwitch
case "panicrangeexit":
case "panicrangestate":
cheap = true
}
// Special case for reflect.noescape. It does just type
Expand Down
46 changes: 32 additions & 14 deletions src/cmd/compile/internal/ir/func.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,19 @@ type Func struct {

Inl *Inline

// funcLitGen and goDeferGen track how many closures have been
// created in this function for function literals and go/defer
// wrappers, respectively. Used by closureName for creating unique
// function names.
//
// RangeParent, if non-nil, is the first non-range body function containing
// the closure for the body of a range function.
RangeParent *Func

// funcLitGen, rangeLitGen and goDeferGen track how many closures have been
// created in this function for function literals, range-over-func loops,
// and go/defer wrappers, respectively. Used by closureName for creating
// unique function names.
// Tracking goDeferGen separately avoids wrappers throwing off
// function literal numbering (e.g., runtime/trace_test.TestTraceSymbolize.func11).
funcLitGen int32
goDeferGen int32
funcLitGen int32
rangeLitGen int32
goDeferGen int32

Label int32 // largest auto-generated label in this function

Expand Down Expand Up @@ -417,20 +421,25 @@ var globClosgen int32

// closureName generates a new unique name for a closure within outerfn at pos.
func closureName(outerfn *Func, pos src.XPos, why Op) *types.Sym {
if outerfn != nil && outerfn.OClosure != nil && outerfn.OClosure.Func.RangeParent != nil {
outerfn = outerfn.OClosure.Func.RangeParent
}
pkg := types.LocalPkg
outer := "glob."
var prefix string
var prefix string = "."
switch why {
default:
base.FatalfAt(pos, "closureName: bad Op: %v", why)
case OCLOSURE:
if outerfn == nil || outerfn.OClosure == nil {
prefix = "func"
prefix = ".func"
}
case ORANGE:
prefix = "-range"
case OGO:
prefix = "gowrap"
prefix = ".gowrap"
case ODEFER:
prefix = "deferwrap"
prefix = ".deferwrap"
}
gen := &globClosgen

Expand All @@ -441,9 +450,12 @@ func closureName(outerfn *Func, pos src.XPos, why Op) *types.Sym {
pkg = outerfn.Sym().Pkg
outer = FuncName(outerfn)

if why == OCLOSURE {
switch why {
case OCLOSURE:
gen = &outerfn.funcLitGen
} else {
case ORANGE:
gen = &outerfn.rangeLitGen
default:
gen = &outerfn.goDeferGen
}
}
Expand All @@ -460,7 +472,7 @@ func closureName(outerfn *Func, pos src.XPos, why Op) *types.Sym {
}

*gen++
return pkg.Lookup(fmt.Sprintf("%s.%s%d", outer, prefix, *gen))
return pkg.Lookup(fmt.Sprintf("%s%s%d", outer, prefix, *gen))
}

// NewClosureFunc creates a new Func to represent a function literal
Expand Down Expand Up @@ -490,6 +502,12 @@ func NewClosureFunc(fpos, cpos src.XPos, why Op, typ *types.Type, outerfn *Func,
clo.pos = cpos
clo.SetType(typ)
clo.SetTypecheck(1)
if why == ORANGE {
clo.Func.RangeParent = outerfn
if outerfn.OClosure != nil && outerfn.OClosure.Func.RangeParent != nil {
clo.Func.RangeParent = outerfn.OClosure.Func.RangeParent
}
}
fn.OClosure = clo

fn.Nname.Defn = fn
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ir/sizeof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestSizeof(t *testing.T) {
_32bit uintptr // size on 32bit platforms
_64bit uintptr // size on 64bit platforms
}{
{Func{}, 168, 288},
{Func{}, 176, 296},
{Name{}, 96, 168},
}

Expand Down
7 changes: 4 additions & 3 deletions src/cmd/compile/internal/noder/irgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ var versionErrorRx = regexp.MustCompile(`requires go[0-9]+\.[0-9]+ or later`)

// checkFiles configures and runs the types2 checker on the given
// parsed source files and then returns the result.
func checkFiles(m posMap, noders []*noder) (*types2.Package, *types2.Info) {
// The map result value indicates which closures are generated from the bodies of range function loops.
func checkFiles(m posMap, noders []*noder) (*types2.Package, *types2.Info, map[*syntax.FuncLit]bool) {
if base.SyntaxErrors() != 0 {
base.ErrorExit()
}
Expand Down Expand Up @@ -150,9 +151,9 @@ func checkFiles(m posMap, noders []*noder) (*types2.Package, *types2.Info) {
// If we do the rewrite in the back end, like between typecheck and walk,
// then the new implicit closure will not have a unified IR inline body,
// and bodyReaderFor will fail.
rangefunc.Rewrite(pkg, info, files)
rangeInfo := rangefunc.Rewrite(pkg, info, files)

return pkg, info
return pkg, info, rangeInfo
}

// A cycleFinder detects anonymous interface cycles (go.dev/issue/56103).
Expand Down
12 changes: 8 additions & 4 deletions src/cmd/compile/internal/noder/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -2704,7 +2704,7 @@ func (r *reader) syntheticClosure(origPos src.XPos, typ *types.Type, ifaceHack b
return false
}

fn := r.inlClosureFunc(origPos, typ)
fn := r.inlClosureFunc(origPos, typ, ir.OCLOSURE)
fn.SetWrapper(true)

clo := fn.OClosure
Expand Down Expand Up @@ -3035,8 +3035,12 @@ func (r *reader) funcLit() ir.Node {
origPos := r.pos()
sig := r.signature(nil)
r.suppressInlPos--
why := ir.OCLOSURE
if r.Bool() {
why = ir.ORANGE
}

fn := r.inlClosureFunc(origPos, sig)
fn := r.inlClosureFunc(origPos, sig, why)

fn.ClosureVars = make([]*ir.Name, 0, r.Len())
for len(fn.ClosureVars) < cap(fn.ClosureVars) {
Expand All @@ -3062,14 +3066,14 @@ func (r *reader) funcLit() ir.Node {

// inlClosureFunc constructs a new closure function, but correctly
// handles inlining.
func (r *reader) inlClosureFunc(origPos src.XPos, sig *types.Type) *ir.Func {
func (r *reader) inlClosureFunc(origPos src.XPos, sig *types.Type, why ir.Op) *ir.Func {
curfn := r.inlCaller
if curfn == nil {
curfn = r.curfn
}

// TODO(mdempsky): Remove hard-coding of typecheck.Target.
return ir.NewClosureFunc(origPos, r.inlPos(origPos), ir.OCLOSURE, sig, curfn, typecheck.Target)
return ir.NewClosureFunc(origPos, r.inlPos(origPos), why, sig, curfn, typecheck.Target)
}

func (r *reader) exprList() []ir.Node {
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/compile/internal/noder/unified.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,9 @@ func readBodies(target *ir.Package, duringInlining bool) {
// writes an export data package stub representing them,
// and returns the result.
func writePkgStub(m posMap, noders []*noder) string {
pkg, info := checkFiles(m, noders)
pkg, info, otherInfo := checkFiles(m, noders)

pw := newPkgWriter(m, pkg, info)
pw := newPkgWriter(m, pkg, info, otherInfo)

pw.collectDecls(noders)

Expand Down
17 changes: 10 additions & 7 deletions src/cmd/compile/internal/noder/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ import (
type pkgWriter struct {
pkgbits.PkgEncoder

m posMap
curpkg *types2.Package
info *types2.Info
m posMap
curpkg *types2.Package
info *types2.Info
rangeFuncBodyClosures map[*syntax.FuncLit]bool // non-public information, e.g., which functions are closures range function bodies?

// Indices for previously written syntax and types2 things.

Expand All @@ -90,13 +91,14 @@ type pkgWriter struct {

// newPkgWriter returns an initialized pkgWriter for the specified
// package.
func newPkgWriter(m posMap, pkg *types2.Package, info *types2.Info) *pkgWriter {
func newPkgWriter(m posMap, pkg *types2.Package, info *types2.Info, otherInfo map[*syntax.FuncLit]bool) *pkgWriter {
return &pkgWriter{
PkgEncoder: pkgbits.NewPkgEncoder(base.Debug.SyncFrames),

m: m,
curpkg: pkg,
info: info,
m: m,
curpkg: pkg,
info: info,
rangeFuncBodyClosures: otherInfo,

pkgsIdx: make(map[*types2.Package]pkgbits.Index),
objsIdx: make(map[types2.Object]pkgbits.Index),
Expand Down Expand Up @@ -2336,6 +2338,7 @@ func (w *writer) funcLit(expr *syntax.FuncLit) {
w.Sync(pkgbits.SyncFuncLit)
w.pos(expr)
w.signature(sig)
w.Bool(w.p.rangeFuncBodyClosures[expr])

w.Len(len(closureVars))
for _, cv := range closureVars {
Expand Down
Loading

0 comments on commit 5a9dabc

Please sign in to comment.