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>

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

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 != "" {
-	if testFuzz != "" && fuzzFlags != nil {
 		// 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] {
+			if !skipInstrumentation[p.ImportPath] {
-				continue
+				p.Internal.FuzzInstrument = true
 			}
-			p.Internal.Gcflags = append(p.Internal.Gcflags, fuzzFlags...)
 		}
 	}
 

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.

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,

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 {
+func fuzzInstrumentFlags() []string {
-	// TODO: expand the set of supported platforms, with testing.
+	// TODO: expand the set of supported platforms, with testing. Nothing about
-	// Nothing about the instrumentation is OS specific, but only amd64 and arm64
+	// the instrumentation is OS specific, but only amd64 and arm64 are
-	// are supported in the runtime. See src/runtime/libfuzzer*.
+	// 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: