From: Than McIntosh Date: Tue, 30 May 2023 14:44:45 +0000 (-0400) Subject: cmd/{cover,go}: revise fix for pkg init order change with -cover X-Git-Tag: go1.21rc1~187 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=89a3bb69cec781177d3967b557e0ce14b2b854f9;p=gostls13.git 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 Run-TryBot: Than McIntosh TryBot-Result: Gopher Robot Reviewed-by: Bryan Mills --- diff --git a/src/cmd/cover/cfg_test.go b/src/cmd/cover/cfg_test.go index 0a2956784b..6782ec89a4 100644 --- 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 49d3f580bc..b86d777ffe 100644 --- 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 a832b6c359..3303b7c211 100644 --- 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= -mode=b.coverMode -var="varName" -o -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 {