]> Cypherpunks.ru repositories - gostls13.git/commitdiff
cmd/compile/internal/typecheck: normalize go/defer statements earlier
authorMatthew Dempsky <mdempsky@google.com>
Wed, 16 Aug 2023 21:39:47 +0000 (14:39 -0700)
committerGopher Robot <gobot@golang.org>
Thu, 17 Aug 2023 19:36:58 +0000 (19:36 +0000)
Normalizing go/defer statements to always use functions with zero
parameters and zero results was added to escape analysis, because that
was the earliest point at which all three frontends converged. Now
that we only have the unified frontend, we can do it during typecheck,
which is where we perform all other desugaring and normalization
rewrites.

Change-Id: Iebf7679b117fd78b1dffee2974bbf85ebc923b23
Reviewed-on: https://go-review.googlesource.com/c/go/+/520260
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/cmd/compile/internal/escape/call.go
src/cmd/compile/internal/ir/func.go
src/cmd/compile/internal/ir/sizeof_test.go
src/cmd/compile/internal/noder/reader.go
src/cmd/compile/internal/typecheck/stmt.go
src/runtime/race/output_test.go
test/fixedbugs/issue24491a.go
test/fixedbugs/issue31573.go

index d87dca23e12a4dafc99c6cb60b2fc57774c1d877..eb87954299e052ace253665938bb5fe668177ec3 100644 (file)
@@ -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
-               }
-       }
 
-       // 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
-               }
+       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)
        }
 
-       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()
index 7efc71d2c73758a5cba46b2fe5e7d2a4d9e98ef6..356d0b070fd868db263691873a10680289e30d19 100644 (file)
@@ -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"
-       gen := &globClosgen
-
-       if outerfn != nil {
-               if outerfn.OClosure != nil {
-                       prefix = ""
+       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
 
+       // 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}
index 307f40d4846710a5a8bad4f93b1f9728dcbfab84..3d2c14318f76e37c870632d336f582151e20e6a6 100644 (file)
@@ -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},
        }
 
index a07fec68ec205588074ab23b46a337aa752b5093..013c73f3d5c5ae79637eb4253f4c260d2507b594 100644 (file)
@@ -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 {
index b902fd9a583de1474c12d91692331bf75fa47938..4c21f045afaea45fca681b2bfac3b530e13ef682 100644 (file)
@@ -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"
-       }
-
-       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
+       call := n.Call
+
+       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)
+       }
+
+       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)
        }
 
-       // 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)
+       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.
index 4c2c3397cf8dec87e1d1ebe837965b081a974d26..0c636ff6c18caeb139f13acd8ae0a37bcfbc21c5 100644 (file)
@@ -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\(\)
       <autogenerated>: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:
index d30b65b233c98c90851fc52067bebec3d03424df..1f748186042cea8a8b5a8cd4099720669fbe4aac 100644 (file)
@@ -74,8 +74,8 @@ func main() {
                        break
                }
        }()
-
        <-done
+
        func() {
                s := &S{}
                defer s.test("method call", uintptr(setup()), uintptr(setup()), uintptr(setup()), uintptr(setup()))
index eaab5634316916a239b1c71d9ea35dafe42533e6..a0cff3099aef3fdd74cb6d6dc5302d98f4157d7a 100644 (file)
@@ -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()