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>

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)

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)
 

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 {