[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>

diff --git a/src/internal/fuzz/coverage.go b/src/internal/fuzz/coverage.go
index e039d68d9a4afce5ca097fea2f76ee8c6b4f2025..316aa147839c81dfd0a91ffd2982fb915d5b1637 100644 (file)
--- a/src/internal/fuzz/coverage.go
+++ b/src/internal/fuzz/coverage.go
@@ -26,25 +26,27 @@ func coverage() []byte {
        return res
 }
 
-// coverageCopy returns a copy of the current bytes provided by coverage().
-// TODO(jayconrod,katiehockman): consider using a shared buffer instead, to
-// make fewer costly allocations.
-func coverageCopy() []byte {
-       cov := coverage()
-       ret := make([]byte, len(cov))
-       copy(ret, cov)
-       return ret
-}
-
-// resetCovereage sets all of the counters for each edge of the instrumented
+// ResetCovereage sets all of the counters for each edge of the instrumented
 // source code to 0.
-func resetCoverage() {
+func ResetCoverage() {
        cov := coverage()
        for i := range cov {
                cov[i] = 0
        }
 }
 
+// SnapshotCoverage copies the current counter values into coverageSnapshot,
+// preserving them for later inspection.
+func SnapshotCoverage() {
+       cov := coverage()
+       if coverageSnapshot == nil {
+               coverageSnapshot = make([]byte, len(cov))
+       }
+       copy(coverageSnapshot, cov)
+}
+
+var coverageSnapshot []byte
+
 // _counters and _ecounters mark the start and end, respectively, of where
 // the 8-bit coverage counters reside in memory. They're known to cmd/link,
 // which specially assigns their addresses for this purpose.

diff --git a/src/internal/fuzz/fuzz.go b/src/internal/fuzz/fuzz.go
index b4145f58dc56e86c4d596399e4f255b125199867..3bb2da872c47598f3addaba174eb0b8589df4332 100644 (file)
--- a/src/internal/fuzz/fuzz.go
+++ b/src/internal/fuzz/fuzz.go
@@ -439,13 +439,13 @@ func newCoordinator(opts CoordinateFuzzingOpts) (*coordinator, error) {
                covOnlyInputs: covOnlyInputs,
        }
 
-       cov := coverageCopy()
-       if len(cov) == 0 {
+       covSize := len(coverage())
+       if covSize == 0 {
                fmt.Fprintf(c.opts.Log, "warning: coverage-guided fuzzing is not supported on this platform\n")
                c.covOnlyInputs = 0
        } else {
                // Set c.coverageData to a clean []byte full of zeros.
-               c.coverageData = make([]byte, len(cov))
+               c.coverageData = make([]byte, covSize)
        }
 
        if c.covOnlyInputs > 0 {

diff --git a/src/internal/fuzz/worker.go b/src/internal/fuzz/worker.go
index 61d3226b309d1c2bc73b21338d21f612cf4a0510..15b1f89daa2af0eab34840e79af09dd2fa6f23f1 100644 (file)
--- a/src/internal/fuzz/worker.go
+++ b/src/internal/fuzz/worker.go
@@ -615,15 +615,12 @@ func (ws *workerServer) fuzz(ctx context.Context, args fuzzArgs) (resp fuzzRespo
        }
 
        if args.CoverageOnly {
-               // Reset the coverage each time before running the fuzzFn.
-               resetCoverage()
                ws.fuzzFn(CorpusEntry{Values: vals})
-               resp.CoverageData = coverageCopy()
+               resp.CoverageData = coverageSnapshot
                return resp
        }
 
-       cov := coverage()
-       if len(cov) != len(ws.coverageData) {
+       if cov := coverage(); len(cov) != len(ws.coverageData) {
                panic(fmt.Sprintf("num edges changed at runtime: %d, expected %d", len(cov), len(ws.coverageData)))
        }
        for {
@@ -635,7 +632,6 @@ func (ws *workerServer) fuzz(ctx context.Context, args fuzzArgs) (resp fuzzRespo
                        resp.Count++
                        ws.m.mutate(vals, cap(mem.valueRef()))
                        writeToMem(vals, mem)
-                       resetCoverage()
                        if err := ws.fuzzFn(CorpusEntry{Values: vals}); err != nil {
                                // TODO(jayconrod,katiehockman): consider making the maximum
                                // minimization time customizable with a go command flag.
@@ -651,12 +647,10 @@ func (ws *workerServer) fuzz(ctx context.Context, args fuzzArgs) (resp fuzzRespo
                                }
                                return resp
                        }
-                       for i := range cov {
-                               if ws.coverageData[i] == 0 && cov[i] > ws.coverageData[i] {
+                       for i := range coverageSnapshot {
+                               if ws.coverageData[i] == 0 && coverageSnapshot[i] > ws.coverageData[i] {
                                        // TODO(jayconrod,katie): minimize this.
-                                       // This run hit a new edge. Only allocate a new slice as a
-                                       // copy of cov if we are returning, since it is expensive.
-                                       resp.CoverageData = coverageCopy()
+                                       resp.CoverageData = coverageSnapshot
                                        return resp
                                }
                        }

diff --git a/src/testing/fuzz.go b/src/testing/fuzz.go
index 7afd24d2582c7afbca5576cd33a6766b1e83f159..d81796b4fc236363d11b1d08815a7a26b92f01b8 100644 (file)
--- 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 b9c110100f2cb0acb8686207a2fb236a19334d0a..24ef7c4d620a5fa641014ac9bc975ade6c781ab8 100644 (file)
--- 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 1877ce3ba4549754bbc059457c6b93494c200f40..63dcc62597b07d2e4fcdc9120f94e4c78d85dbfe 100644 (file)
--- 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'.