From bd71dee2b4c0a44f9c32e9eb23d990b54ce06bae Mon Sep 17 00:00:00 2001
From: Roland Shoemaker <roland@golang.org>
Date: Thu, 10 Mar 2022 12:16:33 -0800
Subject: [PATCH] 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>
---
 .../script/test_fuzz_minimize_dirty_cov.txt   | 84 +++++++++++++++++++
 src/internal/fuzz/worker.go                   | 13 ++-
 2 files changed, 96 insertions(+), 1 deletion(-)
 create mode 100644 src/cmd/go/testdata/script/test_fuzz_minimize_dirty_cov.txt

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
index 0000000000..571bf752d0
--- /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 e984ba73b2..83d937ee6d 100644
--- 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