internal/fuzz: don't use dirty coverage maps during minimization

When minimizing a value, if the value cannot be minimized (i.e. it is
the final value is the same value as was sent for minimization) return
the initial coverage map, rather than the coverageSnapshot, which is
actually the coverage map for the final minimization step and may not
accurately reflect whether the input actually expands the coverage set
or not.

Updates #48326

Change-Id: I01f0eebe5841e808b6799647d2e5fe3aa45cd2e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/391614
Reviewed-by: Bryan Mills <bcmills@google.com>
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/cmd/go/testdata/script/test_fuzz_minimize_dirty_cov.txt b/src/cmd/go/testdata/script/test_fuzz_minimize_dirty_cov.txt
new file mode 100644 (file)
index 0000000..571bf75
--- /dev/null
+++ b/src/cmd/go/testdata/script/test_fuzz_minimize_dirty_cov.txt
@@ -0,0 +1,84 @@
+# Test that minimization doesn't use dirty coverage snapshots when it
+# is unable to actually minimize the input. We do this by checking that
+# a expected value appears in the cache. If a dirty coverage map is used
+# (i.e. the coverage map generated during the last minimization step,
+# rather than the map provided with the initial input) then this value
+# is unlikely to appear in the cache, since the map generated during
+# the last minimization step should not increase the coverage.
+
+[short] skip
+[!fuzz-instrumented] skip
+
+env GOCACHE=$WORK/gocache
+go test -fuzz=FuzzCovMin -fuzztime=25s -test.fuzzcachedir=$GOCACHE/fuzz
+go run check_file/main.go $GOCACHE/fuzz/FuzzCovMin abcd
+
+-- go.mod --
+module test
+
+-- covmin_test.go --
+package covmin
+
+import "testing"
+
+func FuzzCovMin(f *testing.F) {
+       f.Fuzz(func(t *testing.T, data []byte) {
+               if len(data) >= 4 && data[0] == 'a' && data[1] == 'b' && data[2] == 'c' && data[3] == 'd' {
+                       return
+               }
+       })
+}
+
+-- check_file/main.go --
+package main
+
+import (
+       "bytes"
+       "fmt"
+       "os"
+       "path/filepath"
+       "regexp"
+       "strconv"
+)
+
+func checkFile(name, expected string) (bool, error) {
+       data, err := os.ReadFile(name)
+       if err != nil {
+               return false, err
+       }
+       for _, line := range bytes.Split(data, []byte("\n")) {
+               m := valRe.FindSubmatch(line)
+               if m == nil {
+                       continue
+               }
+               fmt.Println(strconv.Unquote(string(m[1])))
+               if s, err := strconv.Unquote(string(m[1])); err != nil {
+                       return false, err
+               } else if s == expected {
+                       return true, nil
+               }
+       }
+       return false, nil
+}
+
+var valRe = regexp.MustCompile(`^\[\]byte\(([^)]+)\)$`)
+
+func main() {
+       dir, expected := os.Args[1], os.Args[2]
+       ents, err := os.ReadDir(dir)
+       if err != nil {
+               fmt.Fprintln(os.Stderr, err)
+               os.Exit(1)
+       }
+       for _, ent := range ents {
+               name := filepath.Join(dir, ent.Name())
+               if good, err := checkFile(name, expected); err != nil {
+                       fmt.Fprintln(os.Stderr, err)
+                       os.Exit(1)
+               } else if good {
+                       os.Exit(0)
+               }
+       }
+       fmt.Fprintln(os.Stderr, "input over minimized")
+       os.Exit(1)
+}

diff --git a/src/internal/fuzz/worker.go b/src/internal/fuzz/worker.go
index e984ba73b201f0437329daa090117020daae5dc6..83d937ee6d163003c78232438ee6eb743e9ea960 100644 (file)
--- a/src/internal/fuzz/worker.go
+++ b/src/internal/fuzz/worker.go
@@ -800,6 +800,7 @@ func (ws *workerServer) minimize(ctx context.Context, args minimizeArgs) (resp m
        if err != nil {
                panic(err)
        }
+       inpHash := sha256.Sum256(mem.valueCopy())
        if args.Timeout != 0 {
                var cancel func()
                ctx, cancel = context.WithTimeout(ctx, args.Timeout)
@@ -811,12 +812,22 @@ func (ws *workerServer) minimize(ctx context.Context, args minimizeArgs) (resp m
        success, err := ws.minimizeInput(ctx, vals, mem, args)
        if success {
                writeToMem(vals, mem)
+               outHash := sha256.Sum256(mem.valueCopy())
                mem.header().rawInMem = false
                resp.WroteToMem = true
                if err != nil {
                        resp.Err = err.Error()
                } else {
-                       resp.CoverageData = coverageSnapshot
+                       // If the values didn't change during minimization then coverageSnapshot is likely
+                       // a dirty snapshot which represents the very last step of minimization, not the
+                       // coverage for the initial input. In that case just return the coverage we were
+                       // given initially, since it more accurately represents the coverage map for the
+                       // input we are returning.
+                       if outHash != inpHash {
+                               resp.CoverageData = coverageSnapshot
+                       } else {
+                               resp.CoverageData = args.KeepCoverage
+                       }
                }
        }
        return resp