diff --git a/src/cmd/compile/internal/escape/call.go b/src/cmd/compile/internal/escape/call.go index d87dca23e1..eb87954299 100644 --- a/src/cmd/compile/internal/escape/call.go +++ b/src/cmd/compile/internal/escape/call.go @@ -210,17 +210,6 @@ func (e *escape) callCommon(ks []hole, call ir.Node, init *ir.Nodes, wrapper *ir } // goDeferStmt analyzes a "go" or "defer" statement. -// -// In the process, it also normalizes the statement to always use a -// simple function call with no arguments and no results. For example, -// it rewrites: -// -// defer f(x, y) -// -// into: -// -// x1, y1 := x, y -// defer func() { f(x1, y1) }() func (e *escape) goDeferStmt(n *ir.GoDeferStmt) { k := e.heapHole() if n.Op() == ir.ODEFER && e.loopDepth == 1 { @@ -233,57 +222,26 @@ func (e *escape) goDeferStmt(n *ir.GoDeferStmt) { n.SetEsc(ir.EscNever) } - call := n.Call - - init := n.PtrInit() - init.Append(ir.TakeInit(call)...) - e.stmts(*init) - // If the function is already a zero argument/result function call, // just escape analyze it normally. // // Note that the runtime is aware of this optimization for // "go" statements that start in reflect.makeFuncStub or // reflect.methodValueCall. - if call, ok := call.(*ir.CallExpr); ok && call.Op() == ir.OCALLFUNC { - if sig := call.X.Type(); sig.NumParams()+sig.NumResults() == 0 { - if clo, ok := call.X.(*ir.ClosureExpr); ok && n.Op() == ir.OGO { - clo.IsGoWrap = true - } - e.expr(k, call.X) - return - } + + call, ok := n.Call.(*ir.CallExpr) + if !ok || call.Op() != ir.OCALLFUNC { + base.FatalfAt(n.Pos(), "expected function call: %v", n.Call) + } + if sig := call.X.Type(); sig.NumParams()+sig.NumResults() != 0 { + base.FatalfAt(n.Pos(), "expected signature without parameters or results: %v", sig) } - // Create a new no-argument function that we'll hand off to defer. - fn := ir.NewClosureFunc(n.Pos(), n.Pos(), types.NewSignature(nil, nil, nil), e.curfn, typecheck.Target) - fn.SetWrapper(true) - fn.SetEsc(escFuncTagged) // no params; effectively tagged already - fn.Body = []ir.Node{call} - if call, ok := call.(*ir.CallExpr); ok && call.Op() == ir.OCALLFUNC { - // If the callee is a named function, link to the original callee. - x := call.X - if x.Op() == ir.ONAME && x.(*ir.Name).Class == ir.PFUNC { - fn.WrappedFunc = call.X.(*ir.Name).Func - } else if x.Op() == ir.OMETHEXPR && ir.MethodExprFunc(x).Nname != nil { - fn.WrappedFunc = ir.MethodExprName(x).Func - } - } - - clo := fn.OClosure - - if n.Op() == ir.OGO { + if clo, ok := call.X.(*ir.ClosureExpr); ok && n.Op() == ir.OGO { clo.IsGoWrap = true } - e.callCommon(nil, call, init, fn) - e.closures = append(e.closures, closure{e.spill(k, clo), clo}) - - // Create new top level call to closure. - n.Call = ir.NewCallExpr(call.Pos(), ir.OCALL, clo, nil) - ir.WithFunc(e.curfn, func() { - typecheck.Stmt(n.Call) - }) + e.expr(k, call.X) } // rewriteArgument rewrites the argument *argp of the given call expression. @@ -317,6 +275,10 @@ func (e *escape) rewriteArgument(argp *ir.Node, init *ir.Nodes, call ir.Node, fn } // Create and declare a new pointer-typed temp variable. + // + // TODO(mdempsky): This potentially violates the Go spec's order + // of evaluations, by evaluating arg.X before any other + // operands. tmp := e.wrapExpr(arg.Pos(), &arg.X, init, call, wrapper) k := e.mutatorHole() diff --git a/src/cmd/compile/internal/ir/func.go b/src/cmd/compile/internal/ir/func.go index 7efc71d2c7..356d0b070f 100644 --- a/src/cmd/compile/internal/ir/func.go +++ b/src/cmd/compile/internal/ir/func.go @@ -96,10 +96,15 @@ type Func struct { Inl *Inline - // Closgen tracks how many closures have been generated within - // this function. Used by closurename for creating unique + // 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. - Closgen int32 + // + // Tracking goDeferGen separately avoids wrappers throwing off + // function literal numbering (e.g., runtime/trace_test.TestTraceSymbolize.func11). + funcLitGen int32 + goDeferGen int32 Label int32 // largest auto-generated label in this function @@ -358,25 +363,35 @@ func IsTrivialClosure(clo *ClosureExpr) bool { var globClosgen int32 // closureName generates a new unique name for a closure within outerfn at pos. -func closureName(outerfn *Func, pos src.XPos) *types.Sym { +func closureName(outerfn *Func, pos src.XPos, why Op) *types.Sym { pkg := types.LocalPkg outer := "glob." - prefix := "func" + var prefix string + switch why { + default: + base.FatalfAt(pos, "closureName: bad Op: %v", why) + case OCLOSURE: + if outerfn == nil || outerfn.OClosure == nil { + prefix = "func" + } + case OGO: + prefix = "gowrap" + case ODEFER: + prefix = "deferwrap" + } gen := &globClosgen - if outerfn != nil { - if outerfn.OClosure != nil { - prefix = "" - } - + // There may be multiple functions named "_". In those + // cases, we can't use their individual Closgens as it + // would lead to name clashes. + if outerfn != nil && !IsBlank(outerfn.Nname) { pkg = outerfn.Sym().Pkg outer = FuncName(outerfn) - // There may be multiple functions named "_". In those - // cases, we can't use their individual Closgens as it - // would lead to name clashes. - if !IsBlank(outerfn.Nname) { - gen = &outerfn.Closgen + if why == OCLOSURE { + gen = &outerfn.funcLitGen + } else { + gen = &outerfn.goDeferGen } } @@ -406,8 +421,12 @@ func closureName(outerfn *Func, pos src.XPos) *types.Sym { // // outerfn is the enclosing function, if any. The returned function is // appending to pkg.Funcs. -func NewClosureFunc(fpos, cpos src.XPos, typ *types.Type, outerfn *Func, pkg *Package) *Func { - fn := NewFunc(fpos, fpos, closureName(outerfn, cpos), typ) +// +// why is the reason we're generating this Func. It can be OCLOSURE +// (for a normal function literal) or OGO or ODEFER (for wrapping a +// call expression that has parameters or results). +func NewClosureFunc(fpos, cpos src.XPos, why Op, typ *types.Type, outerfn *Func, pkg *Package) *Func { + fn := NewFunc(fpos, fpos, closureName(outerfn, cpos, why), typ) fn.SetIsHiddenClosure(outerfn != nil) clo := &ClosureExpr{Func: fn} diff --git a/src/cmd/compile/internal/ir/sizeof_test.go b/src/cmd/compile/internal/ir/sizeof_test.go index 307f40d484..3d2c14318f 100644 --- a/src/cmd/compile/internal/ir/sizeof_test.go +++ b/src/cmd/compile/internal/ir/sizeof_test.go @@ -20,7 +20,7 @@ func TestSizeof(t *testing.T) { _32bit uintptr // size on 32bit platforms _64bit uintptr // size on 64bit platforms }{ - {Func{}, 188, 328}, + {Func{}, 192, 336}, {Name{}, 100, 176}, } diff --git a/src/cmd/compile/internal/noder/reader.go b/src/cmd/compile/internal/noder/reader.go index a07fec68ec..013c73f3d5 100644 --- a/src/cmd/compile/internal/noder/reader.go +++ b/src/cmd/compile/internal/noder/reader.go @@ -3143,7 +3143,7 @@ func (r *reader) inlClosureFunc(origPos src.XPos, sig *types.Type) *ir.Func { } // TODO(mdempsky): Remove hard-coding of typecheck.Target. - return ir.NewClosureFunc(origPos, r.inlPos(origPos), sig, curfn, typecheck.Target) + return ir.NewClosureFunc(origPos, r.inlPos(origPos), ir.OCLOSURE, sig, curfn, typecheck.Target) } func (r *reader) exprList() []ir.Node { diff --git a/src/cmd/compile/internal/typecheck/stmt.go b/src/cmd/compile/internal/typecheck/stmt.go index b902fd9a58..4c21f045af 100644 --- a/src/cmd/compile/internal/typecheck/stmt.go +++ b/src/cmd/compile/internal/typecheck/stmt.go @@ -198,58 +198,181 @@ func tcFor(n *ir.ForStmt) ir.Node { return n } +// tcGoDefer typechecks an OGO/ODEFER statement. +// +// Really, this means normalizing the statement to always use a simple +// function call with no arguments and no results. For example, it +// rewrites: +// +// defer f(x, y) +// +// into: +// +// x1, y1 := x, y +// defer func() { f(x1, y1) }() func tcGoDefer(n *ir.GoDeferStmt) { - what := "defer" - if n.Op() == ir.OGO { - what = "go" - } + call := n.Call - switch n.Call.Op() { - // ok - case ir.OCALLINTER, - ir.OCALLMETH, - ir.OCALLFUNC, - ir.OCLEAR, - ir.OCLOSE, - ir.OCOPY, - ir.ODELETE, - ir.OMAX, - ir.OMIN, - ir.OPANIC, - ir.OPRINT, - ir.OPRINTN, - ir.ORECOVER, - ir.ORECOVERFP: - return + init := n.PtrInit() + init.Append(ir.TakeInit(call)...) - case ir.OAPPEND, - ir.OCAP, - ir.OCOMPLEX, - ir.OIMAG, - ir.OLEN, - ir.OMAKE, - ir.OMAKESLICE, - ir.OMAKECHAN, - ir.OMAKEMAP, - ir.ONEW, - ir.OREAL, - ir.OLITERAL: // conversion or unsafe.Alignof, Offsetof, Sizeof - if orig := ir.Orig(n.Call); orig.Op() == ir.OCONV { - break + if call, ok := n.Call.(*ir.CallExpr); ok && call.Op() == ir.OCALLFUNC { + if sig := call.X.Type(); sig.NumParams()+sig.NumResults() == 0 { + return // already in normal form } - base.ErrorfAt(n.Pos(), errors.UnusedResults, "%s discards result of %v", what, n.Call) - return } - // type is broken or missing, most likely a method call on a broken type - // we will warn about the broken type elsewhere. no need to emit a potentially confusing error - if n.Call.Type() == nil { - return + // Create a new wrapper function without parameters or results. + wrapperFn := ir.NewClosureFunc(n.Pos(), n.Pos(), n.Op(), types.NewSignature(nil, nil, nil), ir.CurFunc, Target) + wrapperFn.SetWrapper(true) + + // argps collects the list of operands within the call expression + // that must be evaluated at the go/defer statement. + var argps []*ir.Node + + var visit func(argp *ir.Node) + visit = func(argp *ir.Node) { + arg := *argp + if arg == nil { + return + } + + // Recognize a few common expressions that can be evaluated within + // the wrapper, so we don't need to allocate space for them within + // the closure. + switch arg.Op() { + case ir.OLITERAL, ir.ONIL, ir.OMETHEXPR: + return + case ir.ONAME: + arg := arg.(*ir.Name) + if arg.Class == ir.PFUNC { + return // reference to global function + } + case ir.OADDR: + arg := arg.(*ir.AddrExpr) + if arg.X.Op() == ir.OLINKSYMOFFSET { + return // address of global symbol + } + + case ir.OCONVNOP: + arg := arg.(*ir.ConvExpr) + + // For unsafe.Pointer->uintptr conversion arguments, save the + // unsafe.Pointer argument. This is necessary to handle cases + // like fixedbugs/issue24491a.go correctly. + // + // TODO(mdempsky): Limit to static callees with + // //go:uintptr{escapes,keepalive}? + if arg.Type().IsUintptr() && arg.X.Type().IsUnsafePtr() { + visit(&arg.X) + return + } + + case ir.OARRAYLIT, ir.OSLICELIT, ir.OSTRUCTLIT: + // TODO(mdempsky): For very large slices, it may be preferable + // to construct them at the go/defer statement instead. + list := arg.(*ir.CompLitExpr).List + for i, el := range list { + switch el := el.(type) { + case *ir.KeyExpr: + visit(&el.Value) + case *ir.StructKeyExpr: + visit(&el.Value) + default: + visit(&list[i]) + } + } + return + } + + argps = append(argps, argp) } - // The syntax made sure it was a call, so this must be - // a conversion. - base.FatalfAt(n.Pos(), "%s requires function call, not conversion", what) + visitList := func(list []ir.Node) { + for i := range list { + visit(&list[i]) + } + } + + switch call.Op() { + default: + base.Fatalf("unexpected call op: %v", call.Op()) + + case ir.OCALLFUNC: + call := call.(*ir.CallExpr) + + // If the callee is a named function, link to the original callee. + if wrapped := ir.StaticCalleeName(call.X); wrapped != nil { + wrapperFn.WrappedFunc = wrapped.Func + } + + visit(&call.X) + visitList(call.Args) + + case ir.OCALLINTER: + call := call.(*ir.CallExpr) + argps = append(argps, &call.X.(*ir.SelectorExpr).X) // must be first for OCHECKNIL; see below + visitList(call.Args) + + case ir.OAPPEND, ir.ODELETE, ir.OPRINT, ir.OPRINTN, ir.ORECOVERFP: + call := call.(*ir.CallExpr) + visitList(call.Args) + visit(&call.RType) + + case ir.OCOPY: + call := call.(*ir.BinaryExpr) + visit(&call.X) + visit(&call.Y) + visit(&call.RType) + + case ir.OCLEAR, ir.OCLOSE, ir.OPANIC: + call := call.(*ir.UnaryExpr) + visit(&call.X) + } + + if len(argps) != 0 { + // Found one or more operands that need to be evaluated upfront + // and spilled to temporary variables, which can be captured by + // the wrapper function. + + stmtPos := base.Pos + callPos := base.Pos + + as := ir.NewAssignListStmt(callPos, ir.OAS2, make([]ir.Node, len(argps)), make([]ir.Node, len(argps))) + for i, argp := range argps { + arg := *argp + + pos := callPos + if ir.HasUniquePos(arg) { + pos = arg.Pos() + } + + // tmp := arg + tmp := TempAt(pos, ir.CurFunc, arg.Type()) + init.Append(Stmt(ir.NewDecl(pos, ir.ODCL, tmp))) + tmp.Defn = as + as.Lhs[i] = tmp + as.Rhs[i] = arg + + // Rewrite original expression to use/capture tmp. + *argp = ir.NewClosureVar(pos, wrapperFn, tmp) + } + init.Append(Stmt(as)) + + // For "go/defer iface.M()", if iface is nil, we need to panic at + // the point of the go/defer statement. + if call.Op() == ir.OCALLINTER { + iface := as.Lhs[0] + init.Append(Stmt(ir.NewUnaryExpr(stmtPos, ir.OCHECKNIL, ir.NewUnaryExpr(iface.Pos(), ir.OITAB, iface)))) + } + } + + // Move call into the wrapper function, now that it's safe to + // evaluate there. + wrapperFn.Body = []ir.Node{call} + + // Finally, rewrite the go/defer statement to call the wrapper. + n.Call = Call(call.Pos(), wrapperFn.OClosure, nil, false) } // tcIf typechecks an OIF node. diff --git a/src/runtime/race/output_test.go b/src/runtime/race/output_test.go index 4c2c3397cf..0c636ff6c1 100644 --- a/src/runtime/race/output_test.go +++ b/src/runtime/race/output_test.go @@ -439,7 +439,7 @@ Goroutine [0-9] \(running\) created at: main\.main\(\) .*/main.go:[0-9]+ \+0x[0-9,a-f]+ ==================`}}, - // Test symbolizing wrappers. Both (*T).f and main.func1 are wrappers. + // Test symbolizing wrappers. Both (*T).f and main.gowrap1 are wrappers. // go.dev/issue/60245 {"wrappersym", "run", "", "atexit_sleep_ms=0", ` package main @@ -465,7 +465,7 @@ Write at 0x[0-9,a-f]+ by goroutine [0-9]: .*/main.go:15 \+0x[0-9,a-f]+ main\.\(\*T\)\.f\(\) :1 \+0x[0-9,a-f]+ - main\.main\.func1\(\) + main\.main\.gowrap1\(\) .*/main.go:9 \+0x[0-9,a-f]+ Previous write at 0x[0-9,a-f]+ by main goroutine: diff --git a/test/fixedbugs/issue24491a.go b/test/fixedbugs/issue24491a.go index d30b65b233..1f74818604 100644 --- a/test/fixedbugs/issue24491a.go +++ b/test/fixedbugs/issue24491a.go @@ -74,8 +74,8 @@ func main() { break } }() - <-done + func() { s := &S{} defer s.test("method call", uintptr(setup()), uintptr(setup()), uintptr(setup()), uintptr(setup())) diff --git a/test/fixedbugs/issue31573.go b/test/fixedbugs/issue31573.go index eaab563431..a0cff3099a 100644 --- a/test/fixedbugs/issue31573.go +++ b/test/fixedbugs/issue31573.go @@ -1,4 +1,4 @@ -// errorcheck -0 -m +// errorcheck -0 -m -l // Copyright 2019 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style @@ -6,7 +6,7 @@ package p -func f(...*int) {} // ERROR "can inline f$" +func f(...*int) {} func g() { defer f()