cmd/{cover,go}: revise fix for pkg init order change with -cover

This patch contains a revised fix for issue #56293, switching to a
scheme in which coverage counter variables and meta-data variables are
written to a separate output file as opposed to being tacked onto the
end of an existing rewritten source file.

The advantage of writing counter vars to a separate file is that the
Go command can then present that file as the first source file to the
compiler when the package is built; this will ensure that counter
variable are treated as lexically "before" any other variable that
might call an instrumented function as part of its initializer.

Updates #56293.

Change-Id: Iccb8a6532b976d36ccbd5a2a339882d1f5d19477
Reviewed-on: https://go-review.googlesource.com/c/go/+/499215
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>

diff --git a/src/cmd/cover/cfg_test.go b/src/cmd/cover/cfg_test.go
index 0a2956784b2ae4cf3929698472f0549556d572d2..6782ec89a4779e30c72978934baef084a04253d7 100644 (file)
--- a/src/cmd/cover/cfg_test.go
+++ b/src/cmd/cover/cfg_test.go
@@ -41,7 +41,9 @@ func writePkgConfig(t *testing.T, outdir, tag, ppath, pname string, gran string)
 func writeOutFileList(t *testing.T, infiles []string, outdir, tag string) ([]string, string) {
        outfilelist := filepath.Join(outdir, tag+"outfilelist.txt")
        var sb strings.Builder
-       outfs := []string{}
+       cv := filepath.Join(outdir, "covervars.go")
+       outfs := []string{cv}
+       fmt.Fprintf(&sb, "%s\n", cv)
        for _, inf := range infiles {
                base := filepath.Base(inf)
                of := filepath.Join(outdir, tag+".cov."+base)

diff --git a/src/cmd/cover/cover.go b/src/cmd/cover/cover.go
index 49d3f580bcf2932a62dc81899e1835b2bf9bb3dc..b86d777ffe43485805ae318c6a451431c1c55853 100644 (file)
--- a/src/cmd/cover/cover.go
+++ b/src/cmd/cover/cover.go
@@ -74,7 +74,14 @@ var (
 
 var pkgconfig coverage.CoverPkgConfig
 
-var outputfiles []string // set when -pkgcfg is in use
+// outputfiles is the list of *.cover.go instrumented outputs to write,
+// one per input (set when -pkgcfg is in use)
+var outputfiles []string
+
+// covervarsoutfile is an additional Go source file into which we'll
+// write definitions of coverage counter variables + meta data variables
+// (set when -pkgcfg is in use).
+var covervarsoutfile string
 
 var profile string // The profile to read; the value of -html or -func
 
@@ -165,6 +172,8 @@ func parseFlags() error {
                                if outputfiles, err = readOutFileList(*outfilelist); err != nil {
                                        return err
                                }
+                               covervarsoutfile = outputfiles[0]
+                               outputfiles = outputfiles[1:]
                                numInputs := len(flag.Args())
                                numOutputs := len(outputfiles)
                                if numOutputs != numInputs {
@@ -568,11 +577,6 @@ func annotate(names []string) {
        }
        // TODO: process files in parallel here if it matters.
        for k, name := range names {
-               last := false
-               if k == len(names)-1 {
-                       last = true
-               }
-
                fd := os.Stdout
                isStdout := true
                if *pkgcfg != "" {
@@ -590,16 +594,27 @@ func annotate(names []string) {
                        }
                        isStdout = false
                }
-               p.annotateFile(name, fd, last)
+               p.annotateFile(name, fd)
                if !isStdout {
                        if err := fd.Close(); err != nil {
                                log.Fatalf("cover: %s", err)
                        }
                }
        }
+
+       if *pkgcfg != "" {
+               fd, err := os.Create(covervarsoutfile)
+               if err != nil {
+                       log.Fatalf("cover: %s", err)
+               }
+               p.emitMetaData(fd)
+               if err := fd.Close(); err != nil {
+                       log.Fatalf("cover: %s", err)
+               }
+       }
 }
 
-func (p *Package) annotateFile(name string, fd io.Writer, last bool) {
+func (p *Package) annotateFile(name string, fd io.Writer) {
        fset := token.NewFileSet()
        content, err := os.ReadFile(name)
        if err != nil {
@@ -658,11 +673,6 @@ func (p *Package) annotateFile(name string, fd io.Writer, last bool) {
        if *mode == "atomic" {
                fmt.Fprintf(fd, "\nvar _ = %sLoadUint32\n", atomicPackagePrefix())
        }
-
-       // Last file? Emit meta-data and converage config.
-       if last {
-               p.emitMetaData(fd)
-       }
 }
 
 // setCounterStmt returns the expression: __count[23] = 1.
@@ -1073,6 +1083,9 @@ func (p *Package) emitMetaData(w io.Writer) {
                panic("internal error: seen functions with regonly/testmain")
        }
 
+       // Emit package name.
+       fmt.Fprintf(w, "\npackage %s\n\n", pkgconfig.PkgName)
+
        // Emit package ID var.
        fmt.Fprintf(w, "\nvar %sP uint32\n", *varVar)
 

diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index a832b6c359bc9dfb37f51e23c5e88b2e93133270..3303b7c21158988bb2981733e9ad09b83f25a51f 100644 (file)
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -691,10 +691,11 @@ OverlayLoop:
                                if mode == "" {
                                        panic("covermode should be set at this point")
                                }
-                               pkgcfg := a.Objdir + "pkgcfg.txt"
-                               covoutfiles := a.Objdir + "coveroutfiles.txt"
-                               if err := b.cover2(a, pkgcfg, covoutfiles, infiles, outfiles, coverVar, mode); err != nil {
+                               if newoutfiles, err := b.cover2(a, infiles, outfiles, coverVar, mode); err != nil {
                                        return err
+                               } else {
+                                       outfiles = newoutfiles
+                                       gofiles = append([]string{newoutfiles[0]}, gofiles...)
                                }
                        } else {
                                // If there are no input files passed to cmd/cover,
@@ -2027,9 +2028,19 @@ func (b *Builder) cover(a *Action, dst, src string, varName string) error {
 // cover2 runs, in effect,
 //
 //     go tool cover -pkgcfg=<config file> -mode=b.coverMode -var="varName" -o <outfiles> <infiles>
-func (b *Builder) cover2(a *Action, pkgcfg, covoutputs string, infiles, outfiles []string, varName string, mode string) error {
+//
+// Return value is an updated output files list; in addition to the
+// regular outputs (instrumented source files) the cover tool also
+// writes a separate file (appearing first in the list of outputs)
+// that will contain coverage counters and meta-data.
+func (b *Builder) cover2(a *Action, infiles, outfiles []string, varName string, mode string) ([]string, error) {
+       pkgcfg := a.Objdir + "pkgcfg.txt"
+       covoutputs := a.Objdir + "coveroutfiles.txt"
+       odir := filepath.Dir(outfiles[0])
+       cv := filepath.Join(odir, "covervars.go")
+       outfiles = append([]string{cv}, outfiles...)
        if err := b.writeCoverPkgInputs(a, pkgcfg, covoutputs, outfiles); err != nil {
-               return err
+               return nil, err
        }
        args := []string{base.Tool("cover"),
                "-pkgcfg", pkgcfg,
@@ -2038,8 +2049,11 @@ func (b *Builder) cover2(a *Action, pkgcfg, covoutputs string, infiles, outfiles
                "-outfilelist", covoutputs,
        }
        args = append(args, infiles...)
-       return b.run(a, a.Objdir, "cover "+a.Package.ImportPath, nil,
-               cfg.BuildToolexec, args)
+       if err := b.run(a, a.Objdir, "cover "+a.Package.ImportPath, nil,
+               cfg.BuildToolexec, args); err != nil {
+               return nil, err
+       }
+       return outfiles, nil
 }
 
 func (b *Builder) writeCoverPkgInputs(a *Action, pconfigfile string, covoutputsfile string, outfiles []string) error {