mirror of
https://github.com/golang/go
synced 2025-02-19 03:19:07 +00:00
cmd/compile/internal/staticinit: make staticopy safe
Currently, cmd/compile optimizes `var a = true; var b = a` into `var a = true; var b = true`. But this may not be safe if we need to initialize any other global variables between `a` and `b`, and the initialization involves calling a user-defined function that reassigns `a`. This CL changes staticinit to keep track of the initialization expressions that we've seen so far, and to stop applying the staticcopy optimization once we've seen an initialization expression that might have modified another global variable within this package. To help identify affected initializers, this CL adds a -d=staticcopy flag to warn when a staticcopy is suppressed and turned into a dynamic copy. Currently, `go build -gcflags=all=-d=staticcopy std` reports only four instances: ``` encoding/xml/xml.go:1600:5: skipping static copy of HTMLEntity+0 with map[string]string{...} encoding/xml/xml.go:1869:5: skipping static copy of HTMLAutoClose+0 with []string{...} net/net.go:661:5: skipping static copy of .stmp_31+0 with poll.ErrNetClosing net/http/transport.go:2566:5: skipping static copy of errRequestCanceled+0 with ~R0 ``` Fixes #51913. Change-Id: Iab41cf6f84c44f7f960e4e62c28a8aeaade4fbcf Reviewed-on: https://go-review.googlesource.com/c/go/+/395541 Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: David Chase <drchase@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
This commit is contained in:
committed by
Gopher Robot
parent
1cdabf0c8b
commit
afa3f8e104
@ -47,6 +47,7 @@ type DebugFlags struct {
|
||||
Shapify int `help:"print information about shaping recursive types"`
|
||||
Slice int `help:"print information about slice compilation"`
|
||||
SoftFloat int `help:"force compiler to emit soft-float code" concurrent:"ok"`
|
||||
StaticCopy int `help:"print information about missed static copies" concurrent:"ok"`
|
||||
SyncFrames int `help:"how many writer stack frames to include at sync points in unified export data"`
|
||||
TypeAssert int `help:"print information about type assertion inlining"`
|
||||
WB int `help:"print information about write barriers"`
|
||||
|
@ -445,3 +445,13 @@ func NewClosureFunc(fpos, cpos src.XPos, why Op, typ *types.Type, outerfn *Func,
|
||||
|
||||
return fn
|
||||
}
|
||||
|
||||
// IsFuncPCIntrinsic returns whether n is a direct call of internal/abi.FuncPCABIxxx functions.
|
||||
func IsFuncPCIntrinsic(n *CallExpr) bool {
|
||||
if n.Op() != OCALLFUNC || n.X.Op() != ONAME {
|
||||
return false
|
||||
}
|
||||
fn := n.X.(*Name).Sym()
|
||||
return (fn.Name == "FuncPCABI0" || fn.Name == "FuncPCABIInternal") &&
|
||||
fn.Pkg.Path == "internal/abi"
|
||||
}
|
||||
|
@ -42,6 +42,11 @@ type Schedule struct {
|
||||
|
||||
Plans map[ir.Node]*Plan
|
||||
Temps map[ir.Node]*ir.Name
|
||||
|
||||
// seenMutation tracks whether we've seen an initialization
|
||||
// expression that may have modified other package-scope variables
|
||||
// within this package.
|
||||
seenMutation bool
|
||||
}
|
||||
|
||||
func (s *Schedule) append(n ir.Node) {
|
||||
@ -80,26 +85,57 @@ func recordFuncForVar(v *ir.Name, fn *ir.Func) {
|
||||
MapInitToVar[fn] = v
|
||||
}
|
||||
|
||||
// allBlank reports whether every node in exprs is blank.
|
||||
func allBlank(exprs []ir.Node) bool {
|
||||
for _, expr := range exprs {
|
||||
if !ir.IsBlank(expr) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// tryStaticInit attempts to statically execute an initialization
|
||||
// statement and reports whether it succeeded.
|
||||
func (s *Schedule) tryStaticInit(nn ir.Node) bool {
|
||||
// Only worry about simple "l = r" assignments. Multiple
|
||||
// variable/expression OAS2 assignments have already been
|
||||
// replaced by multiple simple OAS assignments, and the other
|
||||
// OAS2* assignments mostly necessitate dynamic execution
|
||||
// anyway.
|
||||
if nn.Op() != ir.OAS {
|
||||
func (s *Schedule) tryStaticInit(n ir.Node) bool {
|
||||
var lhs []ir.Node
|
||||
var rhs ir.Node
|
||||
|
||||
switch n.Op() {
|
||||
default:
|
||||
base.FatalfAt(n.Pos(), "unexpected initialization statement: %v", n)
|
||||
case ir.OAS:
|
||||
n := n.(*ir.AssignStmt)
|
||||
lhs, rhs = []ir.Node{n.X}, n.Y
|
||||
case ir.OAS2DOTTYPE, ir.OAS2FUNC, ir.OAS2MAPR, ir.OAS2RECV:
|
||||
n := n.(*ir.AssignListStmt)
|
||||
if len(n.Lhs) < 2 || len(n.Rhs) != 1 {
|
||||
base.FatalfAt(n.Pos(), "unexpected shape for %v: %v", n.Op(), n)
|
||||
}
|
||||
lhs, rhs = n.Lhs, n.Rhs[0]
|
||||
case ir.OCALLFUNC:
|
||||
return false // outlined map init call; no mutations
|
||||
}
|
||||
|
||||
if !s.seenMutation {
|
||||
s.seenMutation = mayModifyPkgVar(rhs)
|
||||
}
|
||||
|
||||
if allBlank(lhs) && !AnySideEffects(rhs) {
|
||||
return true // discard
|
||||
}
|
||||
|
||||
// Only worry about simple "l = r" assignments. The OAS2*
|
||||
// assignments mostly necessitate dynamic execution anyway.
|
||||
if len(lhs) > 1 {
|
||||
return false
|
||||
}
|
||||
n := nn.(*ir.AssignStmt)
|
||||
if ir.IsBlank(n.X) && !AnySideEffects(n.Y) {
|
||||
// Discard.
|
||||
return true
|
||||
}
|
||||
|
||||
lno := ir.SetPos(n)
|
||||
defer func() { base.Pos = lno }()
|
||||
nam := n.X.(*ir.Name)
|
||||
return s.StaticAssign(nam, 0, n.Y, nam.Type())
|
||||
|
||||
nam := lhs[0].(*ir.Name)
|
||||
return s.StaticAssign(nam, 0, rhs, nam.Type())
|
||||
}
|
||||
|
||||
// like staticassign but we are copying an already
|
||||
@ -134,6 +170,15 @@ func (s *Schedule) staticcopy(l *ir.Name, loff int64, rn *ir.Name, typ *types.Ty
|
||||
base.Fatalf("unexpected initializer: %v", rn.Defn)
|
||||
}
|
||||
|
||||
// Variable may have been reassigned by a user-written function call
|
||||
// that was invoked to initialize another global variable (#51913).
|
||||
if s.seenMutation {
|
||||
if base.Debug.StaticCopy != 0 {
|
||||
base.WarnfAt(l.Pos(), "skipping static copy of %v+%v with %v", l, loff, r)
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
for r.Op() == ir.OCONVNOP && !types.Identical(r.Type(), typ) {
|
||||
r = r.(*ir.ConvExpr).X
|
||||
}
|
||||
@ -830,6 +875,43 @@ func AnySideEffects(n ir.Node) bool {
|
||||
return ir.Any(n, isSideEffect)
|
||||
}
|
||||
|
||||
// mayModifyPkgVar reports whether expression n may modify any
|
||||
// package-scope variables declared within the current package.
|
||||
func mayModifyPkgVar(n ir.Node) bool {
|
||||
// safeLHS reports whether the assigned-to variable lhs is either a
|
||||
// local variable or a global from another package.
|
||||
safeLHS := func(lhs ir.Node) bool {
|
||||
v, ok := ir.OuterValue(lhs).(*ir.Name)
|
||||
return ok && v.Op() == ir.ONAME && !(v.Class == ir.PEXTERN && v.Sym().Pkg == types.LocalPkg)
|
||||
}
|
||||
|
||||
return ir.Any(n, func(n ir.Node) bool {
|
||||
switch n.Op() {
|
||||
case ir.OCALLFUNC, ir.OCALLINTER:
|
||||
return !ir.IsFuncPCIntrinsic(n.(*ir.CallExpr))
|
||||
|
||||
case ir.OAPPEND, ir.OCLEAR, ir.OCOPY:
|
||||
return true // could mutate a global array
|
||||
|
||||
case ir.OAS:
|
||||
n := n.(*ir.AssignStmt)
|
||||
if !safeLHS(n.X) {
|
||||
return true
|
||||
}
|
||||
|
||||
case ir.OAS2, ir.OAS2DOTTYPE, ir.OAS2FUNC, ir.OAS2MAPR, ir.OAS2RECV:
|
||||
n := n.(*ir.AssignListStmt)
|
||||
for _, lhs := range n.Lhs {
|
||||
if !safeLHS(lhs) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
})
|
||||
}
|
||||
|
||||
// canRepeat reports whether executing n multiple times has the same effect as
|
||||
// assigning n to a single variable and using that variable multiple times.
|
||||
func canRepeat(n ir.Node) bool {
|
||||
|
@ -549,7 +549,7 @@ func walkCall(n *ir.CallExpr, init *ir.Nodes) ir.Node {
|
||||
directClosureCall(n)
|
||||
}
|
||||
|
||||
if isFuncPCIntrinsic(n) {
|
||||
if ir.IsFuncPCIntrinsic(n) {
|
||||
// For internal/abi.FuncPCABIxxx(fn), if fn is a defined function, rewrite
|
||||
// it to the address of the function of the ABI fn is defined.
|
||||
name := n.X.(*ir.Name).Sym().Name
|
||||
|
@ -538,7 +538,7 @@ func (o *orderState) call(nn ir.Node) {
|
||||
n := nn.(*ir.CallExpr)
|
||||
typecheck.AssertFixedCall(n)
|
||||
|
||||
if isFuncPCIntrinsic(n) && isIfaceOfFunc(n.Args[0]) {
|
||||
if ir.IsFuncPCIntrinsic(n) && isIfaceOfFunc(n.Args[0]) {
|
||||
// For internal/abi.FuncPCABIxxx(fn), if fn is a defined function,
|
||||
// do not introduce temporaries here, so it is easier to rewrite it
|
||||
// to symbol address reference later in walk.
|
||||
@ -1500,16 +1500,6 @@ func (o *orderState) as2ok(n *ir.AssignListStmt) {
|
||||
o.stmt(typecheck.Stmt(as))
|
||||
}
|
||||
|
||||
// isFuncPCIntrinsic returns whether n is a direct call of internal/abi.FuncPCABIxxx functions.
|
||||
func isFuncPCIntrinsic(n *ir.CallExpr) bool {
|
||||
if n.Op() != ir.OCALLFUNC || n.X.Op() != ir.ONAME {
|
||||
return false
|
||||
}
|
||||
fn := n.X.(*ir.Name).Sym()
|
||||
return (fn.Name == "FuncPCABI0" || fn.Name == "FuncPCABIInternal") &&
|
||||
fn.Pkg.Path == "internal/abi"
|
||||
}
|
||||
|
||||
// isIfaceOfFunc returns whether n is an interface conversion from a direct reference of a func.
|
||||
func isIfaceOfFunc(n ir.Node) bool {
|
||||
return n.Op() == ir.OCONVIFACE && n.(*ir.ConvExpr).X.Op() == ir.ONAME && n.(*ir.ConvExpr).X.(*ir.Name).Class == ir.PFUNC
|
||||
|
21
test/fixedbugs/issue51913.go
Normal file
21
test/fixedbugs/issue51913.go
Normal file
@ -0,0 +1,21 @@
|
||||
// run
|
||||
|
||||
// Copyright 2022 The Go Authors. All rights reserved.
|
||||
// Use of this source code is governed by a BSD-style
|
||||
// license that can be found in the LICENSE file.
|
||||
|
||||
package main
|
||||
|
||||
var _ = func() int {
|
||||
a = false
|
||||
return 0
|
||||
}()
|
||||
|
||||
var a = true
|
||||
var b = a
|
||||
|
||||
func main() {
|
||||
if b {
|
||||
panic("FAIL")
|
||||
}
|
||||
}
|
Reference in New Issue
Block a user