cmd/go: prevent necessary GCFlag from being removed

There are special flags that must be passed to the
compiler at build time in order to instrument the
testing binary for fuzzing.
One potential option would be to add these flags to
p.Internal.Gcflags inside cmd/go/internal/test. However,
future calls to setToolFlags can cause these flags to
get cleared about before the build starts, removing
virtually all coverage guidance. This change moves the
logic to add the flag deeper down the call stack,
preventing it from being cleared.

Change-Id: I40eadb0cacc18f29cee75379cd9380f9e73bb8da
Reviewed-on: https://go-review.googlesource.com/c/go/+/352511
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>

diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go
index 4013330bc47f9dc18c1c7b728ad7625ab3a4bf77..8a5a1a5fe299983bbfa12ddc3c5d11da7e818a8a 100644 (file)
--- a/src/cmd/go/internal/load/pkg.go
+++ b/src/cmd/go/internal/load/pkg.go
@@ -203,6 +203,7 @@ type PackageInternal struct {
        Local             bool                 // imported via local path (./ or ../)
        LocalPrefix       string               // interpret ./ and ../ imports relative to this prefix
        ExeName           string               // desired name for temporary executable
+       FuzzInstrument    bool                 // package should be instrumented for fuzzing
        CoverMode         string               // preprocess Go source files with the coverage tool in this mode
        CoverVars         map[string]*CoverVar // variables created by coverage analysis
        OmitDebug         bool                 // tell linker not to write debug information

diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go
index 7c6f109cc561276cadcca9a75fc9365e2e8f9f2f..a6c8631a3751d7754b502bf8a2e7d639334ff81d 100644 (file)
--- a/src/cmd/go/internal/test/test.go
+++ b/src/cmd/go/internal/test/test.go
@@ -818,12 +818,11 @@ func runTest(ctx context.Context, cmd *base.Command, args []string) {
 
        // Inform the compiler that it should instrument the binary at
        // build-time when fuzzing is enabled.
-       fuzzFlags := work.FuzzInstrumentFlags()
-       if testFuzz != "" && fuzzFlags != nil {
+       if testFuzz != "" {
                // Don't instrument packages which may affect coverage guidance but are
                // unlikely to be useful. Most of these are used by the testing or
                // internal/fuzz packages concurrently with fuzzing.
-               var fuzzNoInstrument = map[string]bool{
+               var skipInstrumentation = map[string]bool{
                        "context":       true,
                        "internal/fuzz": true,
                        "reflect":       true,
@@ -835,10 +834,9 @@ func runTest(ctx context.Context, cmd *base.Command, args []string) {
                        "time":          true,
                }
                for _, p := range load.TestPackageList(ctx, pkgOpts, pkgs) {
-                       if fuzzNoInstrument[p.ImportPath] {
-                               continue
+                       if !skipInstrumentation[p.ImportPath] {
+                               p.Internal.FuzzInstrument = true
                        }
-                       p.Internal.Gcflags = append(p.Internal.Gcflags, fuzzFlags...)
                }
        }
 

diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index f82028aef65d45e8cb69576dde31ea8565f702fe..692d39452051674bb635a41543620627ed5daeec 100644 (file)
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -281,6 +281,11 @@ func (b *Builder) buildActionID(a *Action) cache.ActionID {
        if p.Internal.CoverMode != "" {
                fmt.Fprintf(h, "cover %q %q\n", p.Internal.CoverMode, b.toolID("cover"))
        }
+       if p.Internal.FuzzInstrument {
+               if fuzzFlags := fuzzInstrumentFlags(); fuzzFlags != nil {
+                       fmt.Fprintf(h, "fuzz %q\n", fuzzFlags)
+               }
+       }
        fmt.Fprintf(h, "modinfo %q\n", p.Internal.BuildInfo)
 
        // Configuration specific to compiler toolchain.

diff --git a/src/cmd/go/internal/work/gc.go b/src/cmd/go/internal/work/gc.go
index fe0a45ec2a42afe6c813b5d0dd586522b09a99f8..414736cbd7696111d304c372b66e9b6c52ef7f1b 100644 (file)
--- a/src/cmd/go/internal/work/gc.go
+++ b/src/cmd/go/internal/work/gc.go
@@ -144,6 +144,9 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg
        }
 
        gcflags := str.StringList(forcedGcflags, p.Internal.Gcflags)
+       if p.Internal.FuzzInstrument {
+               gcflags = append(gcflags, fuzzInstrumentFlags()...)
+       }
        if compilingRuntime {
                // Remove -N, if present.
                // It is not possible to build the runtime with no optimizations,

diff --git a/src/cmd/go/internal/work/init.go b/src/cmd/go/internal/work/init.go
index 1f8ec02df182407a03df0f1ee8eb42f527cbb02f..6a29abb03b87b77439ea3b1abff2105bec984b86 100644 (file)
--- a/src/cmd/go/internal/work/init.go
+++ b/src/cmd/go/internal/work/init.go
@@ -60,18 +60,19 @@ func BuildInit() {
        }
 }
 
-// FuzzInstrumentFlags returns compiler flags that enable fuzzing instrumation
+// fuzzInstrumentFlags returns compiler flags that enable fuzzing instrumation
 // on supported platforms.
 //
-// On unsupported platforms, FuzzInstrumentFlags returns nil, meaning no
+// On unsupported platforms, fuzzInstrumentFlags returns nil, meaning no
 // instrumentation is added. 'go test -fuzz' still works without coverage,
 // but it generates random inputs without guidance, so it's much less effective.
-func FuzzInstrumentFlags() []string {
-       // TODO: expand the set of supported platforms, with testing.
-       // Nothing about the instrumentation is OS specific, but only amd64 and arm64
-       // are supported in the runtime. See src/runtime/libfuzzer*.
+func fuzzInstrumentFlags() []string {
+       // TODO: expand the set of supported platforms, with testing. Nothing about
+       // the instrumentation is OS specific, but only amd64 and arm64 are
+       // supported in the runtime. See src/runtime/libfuzzer*.
        //
-       // Keep in sync with build constraints in internal/fuzz/counters_{un,}supported.go
+       // Keep in sync with build constraints in
+       // internal/fuzz/counters_{un,}supported.go
        switch cfg.Goos {
        case "darwin", "freebsd", "linux", "windows":
        default: