[dev.fuzz] cmd/go/internal: instrument imports of the test

Previously, the packages that were imported by the
test were not instrumented for coverage. This meant
that a fuzz target in a stand-alone test file would
not be able to perform coverage-guided fuzzing.

The fix uses all of the imports, including those
from the test files, when determining which packages
to instrument. However, certain package should
be ignored when walking the import graph. Otherwise,
packages like internal/fuzz, and its imports, may be
instrumented, which could lead to false positives for
"interesting" corpus values.

There was an additional bug which needed to be fixed
in order for this to work. The bug was in the fact that
the GcFlags which held `-d=libfuzzer` were being
overwritten in some cases. The fix updates the way these
flags are set in order to prevent this behavior.

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

src/cmd/go/internal/load/flag.go

@@ -22,8 +22,9 @@ var (
 // that allows specifying different effective flags for different packages.
 // See 'go help build' for more details about per-package flags.
 type PerPackageFlag struct {
-	present bool
-	values  []ppfValue
+	present      bool
+	values       []ppfValue
+	seenPackages map[*Package]bool // the packages for which the flags have already been set
 }
 
 // A ppfValue is a single <pattern>=<flags> per-package flag value.

src/cmd/go/internal/load/pkg.go

@@ -210,7 +210,6 @@ type PackageInternal struct {
 	BuildInfo         string               // add this info to package main
 	TestmainGo        *[]byte              // content for _testmain.go
 	Embed             map[string][]string  // //go:embed comment mapping
-	FlagsSet          bool                 // whether the flags have been set
 	OrigImportPath    string               // original import path before adding '_test' suffix
 
 	Asmflags   []string // -asmflags for this package
@@ -2628,18 +2627,20 @@ func (e *mainPackageError) ImportPath() string {
 
 func setToolFlags(pkgs ...*Package) {
 	for _, p := range PackageList(pkgs) {
-		// TODO(jayconrod,katiehockman): See if there's a better way to do this.
-		if p.Internal.FlagsSet {
-			// The flags have already been set, so don't re-run this and
-			// potentially clear existing flags.
-			continue
-		} else {
-			p.Internal.FlagsSet = true
+		appendFlags(p, &p.Internal.Asmflags, &BuildAsmflags)
+		appendFlags(p, &p.Internal.Gcflags, &BuildGcflags)
+		appendFlags(p, &p.Internal.Ldflags, &BuildLdflags)
+		appendFlags(p, &p.Internal.Gccgoflags, &BuildGccgoflags)
+	}
+}
+
+func appendFlags(p *Package, flags *[]string, packageFlag *PerPackageFlag) {
+	if !packageFlag.seenPackages[p] {
+		if packageFlag.seenPackages == nil {
+			packageFlag.seenPackages = make(map[*Package]bool)
 		}
-		p.Internal.Asmflags = BuildAsmflags.For(p)
-		p.Internal.Gcflags = BuildGcflags.For(p)
-		p.Internal.Ldflags = BuildLdflags.For(p)
-		p.Internal.Gccgoflags = BuildGccgoflags.For(p)
+		packageFlag.seenPackages[p] = true
+		*flags = append(*flags, packageFlag.For(p)...)
 	}
 }
 

src/cmd/go/internal/test/test.go

@@ -826,11 +826,21 @@ 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 {
-		// Inform the compiler that it should instrument the binary at
-		// build-time when fuzzing is enabled.
-		for _, p := range load.PackageList(pkgs) {
+		// Don't instrument packages which may affect coverage guidance but are
+		// unlikely to be useful.
+		var fuzzNoInstrument = map[string]bool{
+			"testing":       true,
+			"internal/fuzz": true,
+			"runtime":       true,
+		}
+		for _, p := range load.TestPackageList(ctx, pkgOpts, pkgs) {
+			if fuzzNoInstrument[p.ImportPath] {
+				continue
+			}
 			p.Internal.Gcflags = append(p.Internal.Gcflags, fuzzFlags...)
 		}
 	}