[dev.fuzz] internal/fuzz: move coverage capture closer to function

When instrumented packages intersect with the packages used by the
testing or internal/fuzz packages the coverage counters become noisier,
as counters will be triggered by non-fuzzed harness code.

Ideally counters would be deterministic, as there are many advanced
fuzzing strategies that require mutating the input while maintaining
static coverage.

The simplest way to mitigate this noise is to capture the coverage
counters as closely as possible to the invocation of the fuzz target
in the testing package. In order to do this add a new function which
captures the current values of the counters, SnapshotCoverage. This
function copies the current counters into a static buffer,
coverageSnapshot, which workerServer.fuzz can then inspect when it
comes time to check if new coverage has been found.

This method is not foolproof. As the fuzz target is called in a
goroutine, harness code can still cause counters to be incremented
while the target is being executed. Despite this we do see
significant reduction in churn via this approach. For example,
running a  basic target that causes strconv to be instrumented for
500,000 iterations causes ~800 unique sets of coverage counters,
whereas by capturing the counters closer to the target we get ~40
unique sets.

It may be possible to make counters completely deterministic, but
likely this would require rewriting testing/F.Fuzz to not use tRunner
in a goroutine, and instead use it in a blocking manner (which I
couldn't figure out an obvious way to do), or by doing something even
more complex.

Change-Id: I95c2f3b1d7089c3e6885fc7628a0d3a8ac1a99cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/320329
Trust: Roland Shoemaker <roland@golang.org>
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Katie Hockman <katie@golang.org>

3 files changed, 24 insertions, 4 deletions

diff --git a/src/testing/fuzz.go b/src/testing/fuzz.go
index 7afd24d258..d81796b4fc 100644
--- a/src/testing/fuzz.go
+++ b/src/testing/fuzz.go
@@ -338,7 +338,9 @@ func (f *F) Fuzz(ff interface{}) {
 			for _, v := range e.Values {
 				args = append(args, reflect.ValueOf(v))
 			}
+			f.fuzzContext.resetCoverage()
 			fn.Call(args)
+			f.fuzzContext.snapshotCoverage()
 		})
 		<-t.signal
 		f.inFuzzFn = false
@@ -452,6 +454,8 @@ type fuzzContext struct {
 	coordinateFuzzing func(time.Duration, int64, int, []corpusEntry, []reflect.Type, string, string) error
 	runFuzzWorker     func(func(corpusEntry) error) error
 	readCorpus        func(string, []reflect.Type) ([]corpusEntry, error)
+	resetCoverage     func()
+	snapshotCoverage  func()
 }
 
 // runFuzzTargets runs the fuzz targets matching the pattern for -run. This will
@@ -465,8 +469,10 @@ func runFuzzTargets(deps testDeps, fuzzTargets []InternalFuzzTarget) (ran, ok bo
 	m := newMatcher(deps.MatchString, *match, "-test.run")
 	tctx := newTestContext(*parallel, m)
 	fctx := &fuzzContext{
-		importPath: deps.ImportPath,
-		readCorpus: deps.ReadCorpus,
+		importPath:       deps.ImportPath,
+		readCorpus:       deps.ReadCorpus,
+		resetCoverage:    deps.ResetCoverage,
+		snapshotCoverage: deps.SnapshotCoverage,
 	}
 	root := common{w: os.Stdout} // gather output in one place
 	if Verbose() {
@@ -519,8 +525,10 @@ func runFuzzing(deps testDeps, fuzzTargets []InternalFuzzTarget) (ran, ok bool)
 	m := newMatcher(deps.MatchString, *matchFuzz, "-test.fuzz")
 	tctx := newTestContext(1, m)
 	fctx := &fuzzContext{
-		importPath: deps.ImportPath,
-		readCorpus: deps.ReadCorpus,
+		importPath:       deps.ImportPath,
+		readCorpus:       deps.ReadCorpus,
+		resetCoverage:    deps.ResetCoverage,
+		snapshotCoverage: deps.SnapshotCoverage,
 	}
 	if *isFuzzWorker {
 		fctx.runFuzzWorker = deps.RunFuzzWorker
diff --git a/src/testing/internal/testdeps/deps.go b/src/testing/internal/testdeps/deps.go
index b9c110100f..24ef7c4d62 100644
--- a/src/testing/internal/testdeps/deps.go
+++ b/src/testing/internal/testdeps/deps.go
@@ -174,3 +174,11 @@ func (TestDeps) RunFuzzWorker(fn func(fuzz.CorpusEntry) error) error {
 func (TestDeps) ReadCorpus(dir string, types []reflect.Type) ([]fuzz.CorpusEntry, error) {
 	return fuzz.ReadCorpus(dir, types)
 }
+
+func (TestDeps) ResetCoverage() {
+	fuzz.ResetCoverage()
+}
+
+func (TestDeps) SnapshotCoverage() {
+	fuzz.SnapshotCoverage()
+}
diff --git a/src/testing/testing.go b/src/testing/testing.go
index 1877ce3ba4..63dcc62597 100644
--- a/src/testing/testing.go
+++ b/src/testing/testing.go
@@ -1397,6 +1397,8 @@ func (f matchStringOnly) RunFuzzWorker(func(corpusEntry) error) error { return e
 func (f matchStringOnly) ReadCorpus(string, []reflect.Type) ([]corpusEntry, error) {
 	return nil, errMain
 }
+func (f matchStringOnly) ResetCoverage()    {}
+func (f matchStringOnly) SnapshotCoverage() {}
 
 // Main is an internal function, part of the implementation of the "go test" command.
 // It was exported because it is cross-package and predates "internal" packages.
@@ -1442,6 +1444,8 @@ type testDeps interface {
 	CoordinateFuzzing(time.Duration, int64, int, []corpusEntry, []reflect.Type, string, string) error
 	RunFuzzWorker(func(corpusEntry) error) error
 	ReadCorpus(string, []reflect.Type) ([]corpusEntry, error)
+	ResetCoverage()
+	SnapshotCoverage()
 }
 
 // MainStart is meant for use by tests generated by 'go test'.