]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: move remaining instrumentation logic into ssagen
authorMatthew Dempsky <mdempsky@google.com>
Thu, 14 Sep 2023 05:48:43 +0000 (22:48 -0700)
committerGopher Robot <gobot@golang.org>
Thu, 14 Sep 2023 13:21:53 +0000 (13:21 +0000)
Allows removing Func.{Enter,Exit}, which were only ever used to hold
at most one function call each.

Change-Id: I8b629c82e90bac3fcbe54db89900492406c7dca6
Reviewed-on: https://go-review.googlesource.com/c/go/+/528319
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Than McIntosh <thanm@google.com>
src/cmd/compile/internal/ir/fmt.go
src/cmd/compile/internal/ir/func.go
src/cmd/compile/internal/ir/sizeof_test.go
src/cmd/compile/internal/ir/symtab.go
src/cmd/compile/internal/loopvar/loopvar.go
src/cmd/compile/internal/ssagen/ssa.go
src/cmd/compile/internal/walk/race.go [deleted file]
src/cmd/compile/internal/walk/walk.go

index 81a25cd461e5d3cb713b888576e79d8b51cffa00..35bfbc7a1c21ce54456aee7bf824621769026e0b 100644 (file)
@@ -1129,11 +1129,6 @@ func dumpNode(w io.Writer, n Node, depth int) {
                                dumpNode(w, cv, depth+1)
                        }
                }
-               if len(fn.Enter) > 0 {
-                       indent(w, depth)
-                       fmt.Fprintf(w, "%+v-Enter", n.Op())
-                       dumpNodes(w, fn.Enter, depth+1)
-               }
                if len(fn.Body) > 0 {
                        indent(w, depth)
                        fmt.Fprintf(w, "%+v-body", n.Op())
index ded9acced26092d496ada70b2bd47c6c9ff31430..ea575a420643a548ff43c8d80df6dc648fb21ef2 100644 (file)
@@ -56,11 +56,6 @@ type Func struct {
        Nname    *Name        // ONAME node
        OClosure *ClosureExpr // OCLOSURE node
 
-       // Extra entry code for the function. For example, allocate and initialize
-       // memory for escaping parameters.
-       Enter Nodes
-       Exit  Nodes
-
        // ONAME nodes for all params/locals for this func/closure, does NOT
        // include closurevars until transforming closures during walk.
        // Names must be listed PPARAMs, PPARAMOUTs, then PAUTOs,
@@ -235,7 +230,6 @@ const (
        funcNilCheckDisabled         // disable nil checks when compiling this function
        funcInlinabilityChecked      // inliner has already determined whether the function is inlinable
        funcNeverReturns             // function never returns (in most cases calls panic(), os.Exit(), or equivalent)
-       funcInstrumentBody           // add race/msan/asan instrumentation during SSA construction
        funcOpenCodedDeferDisallowed // can't do open-coded defers
        funcClosureResultsLost       // closure is called indirectly and we lost track of its results; used by escape analysis
        funcPackageInit              // compiler emitted .init func for package
@@ -257,7 +251,6 @@ func (f *Func) HasDefer() bool                 { return f.flags&funcHasDefer !=
 func (f *Func) NilCheckDisabled() bool         { return f.flags&funcNilCheckDisabled != 0 }
 func (f *Func) InlinabilityChecked() bool      { return f.flags&funcInlinabilityChecked != 0 }
 func (f *Func) NeverReturns() bool             { return f.flags&funcNeverReturns != 0 }
-func (f *Func) InstrumentBody() bool           { return f.flags&funcInstrumentBody != 0 }
 func (f *Func) OpenCodedDeferDisallowed() bool { return f.flags&funcOpenCodedDeferDisallowed != 0 }
 func (f *Func) ClosureResultsLost() bool       { return f.flags&funcClosureResultsLost != 0 }
 func (f *Func) IsPackageInit() bool            { return f.flags&funcPackageInit != 0 }
@@ -273,7 +266,6 @@ func (f *Func) SetHasDefer(b bool)                 { f.flags.set(funcHasDefer, b
 func (f *Func) SetNilCheckDisabled(b bool)         { f.flags.set(funcNilCheckDisabled, b) }
 func (f *Func) SetInlinabilityChecked(b bool)      { f.flags.set(funcInlinabilityChecked, b) }
 func (f *Func) SetNeverReturns(b bool)             { f.flags.set(funcNeverReturns, b) }
-func (f *Func) SetInstrumentBody(b bool)           { f.flags.set(funcInstrumentBody, b) }
 func (f *Func) SetOpenCodedDeferDisallowed(b bool) { f.flags.set(funcOpenCodedDeferDisallowed, b) }
 func (f *Func) SetClosureResultsLost(b bool)       { f.flags.set(funcClosureResultsLost, b) }
 func (f *Func) SetIsPackageInit(b bool)            { f.flags.set(funcPackageInit, b) }
index 005ecff314f4fef53d97422384cb21cd29e56d1e..3b6823895cf3b54feda0aa577655c9f4d3e82fa4 100644 (file)
@@ -20,7 +20,7 @@ func TestSizeof(t *testing.T) {
                _32bit uintptr     // size on 32bit platforms
                _64bit uintptr     // size on 64bit platforms
        }{
-               {Func{}, 192, 336},
+               {Func{}, 168, 288},
                {Name{}, 96, 168},
        }
 
index 6ee832e18dcabba369e19708d2326a8ce967dfa2..1e2fc8d97448a4bd4766f832c8249ab4fa238b01 100644 (file)
@@ -40,6 +40,8 @@ var Syms struct {
        PanicdottypeI     *obj.LSym
        Panicnildottype   *obj.LSym
        Panicoverflow     *obj.LSym
+       Racefuncenter     *obj.LSym
+       Racefuncexit      *obj.LSym
        Raceread          *obj.LSym
        Racereadrange     *obj.LSym
        Racewrite         *obj.LSym
index ecf9401eeb16bf22a5535c358b14dcf3a2466eff..030fc04c1369d8da7c1e946dfcc64e709dca2a3d 100644 (file)
@@ -487,8 +487,6 @@ func rewriteNodes(fn *ir.Func, editNodes func(c ir.Nodes) ir.Nodes) {
                switch x := n.(type) {
                case *ir.Func:
                        x.Body = editNodes(x.Body)
-                       x.Enter = editNodes(x.Enter)
-                       x.Exit = editNodes(x.Exit)
                case *ir.InlinedCallExpr:
                        x.Body = editNodes(x.Body)
 
index 4660d050e5a9077610052d3d042e117a655af54e..d79c6fdd00b9c3a5a1f24c4eb4c0d5c37100f556 100644 (file)
@@ -27,6 +27,7 @@ import (
        "cmd/compile/internal/typecheck"
        "cmd/compile/internal/types"
        "cmd/internal/obj"
+       "cmd/internal/objabi"
        "cmd/internal/src"
        "cmd/internal/sys"
 
@@ -130,6 +131,8 @@ func InitConfig() {
        ir.Syms.Panicnildottype = typecheck.LookupRuntimeFunc("panicnildottype")
        ir.Syms.Panicoverflow = typecheck.LookupRuntimeFunc("panicoverflow")
        ir.Syms.Panicshift = typecheck.LookupRuntimeFunc("panicshift")
+       ir.Syms.Racefuncenter = typecheck.LookupRuntimeFunc("racefuncenter")
+       ir.Syms.Racefuncexit = typecheck.LookupRuntimeFunc("racefuncexit")
        ir.Syms.Raceread = typecheck.LookupRuntimeFunc("raceread")
        ir.Syms.Racereadrange = typecheck.LookupRuntimeFunc("racereadrange")
        ir.Syms.Racewrite = typecheck.LookupRuntimeFunc("racewrite")
@@ -319,9 +322,7 @@ func buildssa(fn *ir.Func, worker int) *ssa.Func {
        var astBuf *bytes.Buffer
        if printssa {
                astBuf = &bytes.Buffer{}
-               ir.FDumpList(astBuf, "buildssa-enter", fn.Enter)
                ir.FDumpList(astBuf, "buildssa-body", fn.Body)
-               ir.FDumpList(astBuf, "buildssa-exit", fn.Exit)
                if ssaDumpStdout {
                        fmt.Println("generating SSA for", name)
                        fmt.Print(astBuf.String())
@@ -338,6 +339,15 @@ func buildssa(fn *ir.Func, worker int) *ssa.Func {
        }
        s.checkPtrEnabled = ir.ShouldCheckPtr(fn, 1)
 
+       if base.Flag.Cfg.Instrumenting && fn.Pragma&ir.Norace == 0 && !fn.Linksym().ABIWrapper() {
+               if !base.Flag.Race || !objabi.LookupPkgSpecial(fn.Sym().Pkg.Path).NoRaceFunc {
+                       s.instrumentMemory = true
+               }
+               if base.Flag.Race {
+                       s.instrumentEnterExit = true
+               }
+       }
+
        fe := ssafn{
                curfn: fn,
                log:   printssa && ssaDumpStdout,
@@ -395,10 +405,10 @@ func buildssa(fn *ir.Func, worker int) *ssa.Func {
                // preceding the deferreturn/ret code that we don't track correctly.
                s.hasOpenDefers = false
        }
-       if s.hasOpenDefers && len(s.curfn.Exit) > 0 {
-               // Skip doing open defers if there is any extra exit code (likely
-               // race detection), since we will not generate that code in the
-               // case of the extra deferreturn/ret segment.
+       if s.hasOpenDefers && s.instrumentEnterExit {
+               // Skip doing open defers if we need to instrument function
+               // returns for the race detector, since we will not generate that
+               // code in the case of the extra deferreturn/ret segment.
                s.hasOpenDefers = false
        }
        if s.hasOpenDefers {
@@ -541,7 +551,9 @@ func buildssa(fn *ir.Func, worker int) *ssa.Func {
        }
 
        // Convert the AST-based IR to the SSA-based IR
-       s.stmtList(fn.Enter)
+       if s.instrumentEnterExit {
+               s.rtcall(ir.Syms.Racefuncenter, true, nil, s.newValue0(ssa.OpGetCallerPC, types.Types[types.TUINTPTR]))
+       }
        s.zeroResults()
        s.paramsToHeap()
        s.stmtList(fn.Body)
@@ -881,11 +893,13 @@ type state struct {
        // Used to deduplicate panic calls.
        panics map[funcLine]*ssa.Block
 
-       cgoUnsafeArgs   bool
-       hasdefer        bool // whether the function contains a defer statement
-       softFloat       bool
-       hasOpenDefers   bool // whether we are doing open-coded defers
-       checkPtrEnabled bool // whether to insert checkptr instrumentation
+       cgoUnsafeArgs       bool
+       hasdefer            bool // whether the function contains a defer statement
+       softFloat           bool
+       hasOpenDefers       bool // whether we are doing open-coded defers
+       checkPtrEnabled     bool // whether to insert checkptr instrumentation
+       instrumentEnterExit bool // whether to instrument function enter/exit
+       instrumentMemory    bool // whether to instrument memory operations
 
        // If doing open-coded defers, list of info about the defer calls in
        // scanning order. Hence, at exit we should run these defers in reverse
@@ -1262,7 +1276,7 @@ func (s *state) instrumentMove(t *types.Type, dst, src *ssa.Value) {
 }
 
 func (s *state) instrument2(t *types.Type, addr, addr2 *ssa.Value, kind instrumentKind) {
-       if !s.curfn.InstrumentBody() {
+       if !s.instrumentMemory {
                return
        }
 
@@ -2023,13 +2037,10 @@ func (s *state) exit() *ssa.Block {
                }
        }
 
-       var b *ssa.Block
-       var m *ssa.Value
        // Do actual return.
        // These currently turn into self-copies (in many cases).
        resultFields := s.curfn.Type().Results()
        results := make([]*ssa.Value, len(resultFields)+1, len(resultFields)+1)
-       m = s.newValue0(ssa.OpMakeResult, s.f.OwnAux.LateExpansionResultType())
        // Store SSAable and heap-escaped PPARAMOUT variables back to stack locations.
        for i, f := range resultFields {
                n := f.Nname.(*ir.Name)
@@ -2055,15 +2066,18 @@ func (s *state) exit() *ssa.Block {
                }
        }
 
-       // Run exit code. Today, this is just racefuncexit, in -race mode.
-       // TODO(register args) this seems risky here with a register-ABI, but not clear it is right to do it earlier either.
-       // Spills in register allocation might just fix it.
-       s.stmtList(s.curfn.Exit)
+       // In -race mode, we need to call racefuncexit.
+       // Note: This has to happen after we load any heap-allocated results,
+       // otherwise races will be attributed to the caller instead.
+       if s.instrumentEnterExit {
+               s.rtcall(ir.Syms.Racefuncexit, true, nil)
+       }
 
        results[len(results)-1] = s.mem()
+       m := s.newValue0(ssa.OpMakeResult, s.f.OwnAux.LateExpansionResultType())
        m.AddArgs(results...)
 
-       b = s.endBlock()
+       b := s.endBlock()
        b.Kind = ssa.BlockRet
        b.SetControl(m)
        if s.hasdefer && s.hasOpenDefers {
diff --git a/src/cmd/compile/internal/walk/race.go b/src/cmd/compile/internal/walk/race.go
deleted file mode 100644 (file)
index 972c878..0000000
+++ /dev/null
@@ -1,55 +0,0 @@
-// Copyright 2012 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 walk
-
-import (
-       "cmd/compile/internal/base"
-       "cmd/compile/internal/ir"
-       "cmd/compile/internal/types"
-       "cmd/internal/objabi"
-       "cmd/internal/src"
-)
-
-// The racewalk pass is currently handled in three parts.
-//
-// First, for flag_race, it inserts calls to racefuncenter and
-// racefuncexit at the start and end (respectively) of each
-// function. This is handled below.
-//
-// Second, during buildssa, it inserts appropriate instrumentation
-// calls immediately before each memory load or store. This is handled
-// by the (*state).instrument method in ssa.go, so here we just set
-// the Func.InstrumentBody flag as needed. For background on why this
-// is done during SSA construction rather than a separate SSA pass,
-// see issue #19054.
-//
-// Third, we remove calls to racefuncenter and racefuncexit, for leaf
-// functions without instrumented operations. This is done as part of
-// ssa opt pass via special rule.
-
-// TODO(dvyukov): do not instrument initialization as writes:
-// a := make([]int, 10)
-
-func instrument(fn *ir.Func) {
-       if fn.Pragma&ir.Norace != 0 || (fn.Linksym() != nil && fn.Linksym().ABIWrapper()) {
-               return
-       }
-
-       if !base.Flag.Race || !objabi.LookupPkgSpecial(base.Ctxt.Pkgpath).NoRaceFunc {
-               fn.SetInstrumentBody(true)
-       }
-
-       if base.Flag.Race {
-               lno := base.Pos
-               base.Pos = src.NoXPos
-               var init ir.Nodes
-               fn.Enter.Prepend(mkcallstmt("racefuncenter", mkcall("getcallerpc", types.Types[types.TUINTPTR], &init)))
-               if len(init) != 0 {
-                       base.Fatalf("race walk: unexpected init for getcallerpc")
-               }
-               fn.Exit.Append(mkcallstmt("racefuncexit"))
-               base.Pos = lno
-       }
-}
index b09f7f1732d409468f34ef567f30527f8d502f26..8be5804616bea628b9b76cdd6eb5a8bd0a23f4e2 100644 (file)
@@ -45,10 +45,6 @@ func Walk(fn *ir.Func) {
                ir.DumpList(s, ir.CurFunc.Body)
        }
 
-       if base.Flag.Cfg.Instrumenting {
-               instrument(fn)
-       }
-
        // Eagerly compute sizes of all variables for SSA.
        for _, n := range fn.Dcl {
                types.CalcSize(n.Type())