]> Cypherpunks repositories - gostls13.git/commitdiff
internal/fuzz: fix bugs with minimization
authorKatie Hockman <katie@golang.org>
Wed, 13 Oct 2021 20:49:27 +0000 (16:49 -0400)
committerKatie Hockman <katie@golang.org>
Fri, 15 Oct 2021 19:03:33 +0000 (19:03 +0000)
This pulls in some code and tests from CL 353355.

This change makes some refactors for when we read
to and write from memory during minimization.
That fixes a bug when minimizing interesting inputs.
Now, if an error occurs while minimizing an interesting
input, that value will continue to be minimized as a
crash, and returned to the user.

This change also allows minimization of a crash that
occurred during the warmup phase. We don't want to
minimize failures in the seed corpus, but if an entry
in the cache causes a new failure, then there's no
compelling reason why we shouldn't try to minimize it.

Fixes #48731

Change-Id: I7262cecd8ea7ae6fdf932f3a36db55fb062a1f2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/355691
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
src/cmd/go/testdata/script/test_fuzz_minimize.txt
src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt
src/internal/fuzz/fuzz.go
src/internal/fuzz/minimize_test.go
src/internal/fuzz/worker.go

index 0a0359fabb8431597b5c4c94ef2f6d7a16d6c260..56abc681042fda8890f8d6f0da0dbffc48be38ac 100644 (file)
@@ -7,56 +7,72 @@
 env GOCACHE=$WORK/gocache
 
 # Test that fuzzminimizetime cannot be negative seconds
-! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=10000x -fuzzminimizetime=-1ms minimizer_test.go
+! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=10000x -fuzzminimizetime=-1ms .
 ! stdout '^ok'
 ! stdout 'contains a non-zero byte'
 stdout 'invalid duration'
 stdout FAIL
 
 # Test that fuzzminimizetime cannot be negative times
-! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=10000x -fuzzminimizetime=-1x minimizer_test.go
+! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=10000x -fuzzminimizetime=-1x .
 ! stdout '^ok'
 ! stdout 'contains a non-zero byte'
 stdout 'invalid count'
 stdout FAIL
 
 # Test that fuzzminimizetime can be zero seconds, and minimization is disabled
-! go test -fuzz=FuzzMinimizeZeroDurationSet -run=FuzzMinimizeZeroDurationSet -fuzztime=10000x -fuzzminimizetime=0s minimizer_test.go
+! go test -fuzz=FuzzMinimizeZeroDurationSet -run=FuzzMinimizeZeroDurationSet -fuzztime=10000x -fuzzminimizetime=0s .
 ! stdout '^ok'
 ! stdout 'minimizing'
 stdout 'there was an Error'
 stdout FAIL
 
 # Test that fuzzminimizetime can be zero times, and minimization is disabled
-! go test -fuzz=FuzzMinimizeZeroLimitSet -run=FuzzMinimizeZeroLimitSet -fuzztime=10000x -fuzzminimizetime=0x minimizer_test.go
+! go test -fuzz=FuzzMinimizeZeroLimitSet -run=FuzzMinimizeZeroLimitSet -fuzztime=10000x -fuzzminimizetime=0x .
 ! stdout '^ok'
 ! stdout 'minimizing'
 stdout 'there was an Error'
 stdout FAIL
 
+# Test that minimization occurs for a crash that appears while minimizing a
+# newly found interesting input. There must be only one worker for this test to
+# be flaky like we want.
+! go test -fuzz=FuzzMinimizerCrashInMinimization -run=FuzzMinimizerCrashInMinimization -fuzztime=10000x -parallel=1 .
+! stdout '^ok'
+stdout 'got the minimum size!'
+stdout 'flaky failure'
+stdout FAIL
+
+# Make sure the crash that was written will fail when run with go test
+! go test -run=FuzzMinimizerCrashInMinimization .
+
+# Clear testdata.
+rm testdata
+
 # Test that minimization is working for recoverable errors.
-! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=10000x minimizer_test.go
+! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=10000x .
 ! stdout '^ok'
 stdout 'got the minimum size!'
-stdout 'contains a non-zero byte'
+# The error message that was printed should be for the one written to testdata.
+stdout 'contains a non-zero byte of length 50'
 stdout FAIL
 
 # Check that the bytes written to testdata are of length 50 (the minimum size)
-go run check_testdata.go FuzzMinimizerRecoverable 50
+go run ./check_testdata FuzzMinimizerRecoverable 50
 
 # Test that re-running the minimized value causes a crash.
-! go test -run=FuzzMinimizerRecoverable minimizer_test.go
+! go test -run=FuzzMinimizerRecoverable .
 rm testdata
 
 # Test that minimization doesn't run for non-recoverable errors.
-! go test -fuzz=FuzzMinimizerNonrecoverable -run=FuzzMinimizerNonrecoverable -fuzztime=10000x minimizer_test.go
+! go test -fuzz=FuzzMinimizerNonrecoverable -run=FuzzMinimizerNonrecoverable -fuzztime=10000x .
 ! stdout '^ok'
 ! stdout 'minimizing'
 stdout 'fuzzing process terminated unexpectedly: exit status 99'
 stdout FAIL
 
 # Check that re-running the value causes a crash.
-! go test -run=FuzzMinimizerNonrecoverable minimizer_test.go
+! go test -run=FuzzMinimizerNonrecoverable .
 rm testdata
 
 # Clear the fuzzing cache. There may already be minimized inputs that would
@@ -65,35 +81,43 @@ go clean -fuzzcache
 
 # Test that minimization can be cancelled by fuzzminimizetime and the latest
 # crash will still be logged and written to testdata.
-! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=100x -fuzzminimizetime=1x minimizer_test.go
+! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=100x -fuzzminimizetime=1x .
 ! stdout '^ok'
 stdout 'testdata[/\\]fuzz[/\\]FuzzMinimizerRecoverable[/\\]'
 ! stdout 'got the minimum size!'  # it shouldn't have had enough time to minimize it
 stdout FAIL
 
 # Test that re-running the unminimized value causes a crash.
-! go test -run=FuzzMinimizerRecoverable minimizer_test.go
-
-! go test -fuzz=FuzzMinimizerTooSlow -run=FuzzMinimizerTooSlow -fuzzminimizetime=3s minimizer_test.go
-stdout 'fuzz: minimizing'
-stdout 'fuzz: elapsed: \d+s, minimizing'
-stdout 'testdata[/\\]fuzz[/\\]FuzzMinimizerTooSlow[/\\]'
-stdout FAIL
+! go test -run=FuzzMinimizerRecoverable .
 
 # TODO(jayconrod,katiehockman): add a test which verifies that the right bytes
 # are written to testdata in the case of an interrupt during minimization.
 
 -- go.mod --
-module m
+module example.com/y
 
 go 1.16
--- minimizer_test.go --
-package fuzz_test
+-- y.go --
+package y
 
 import (
+       "bytes"
+       "io"
+)
+
+func Y(w io.Writer, b []byte) {
+       if !bytes.Equal(b, []byte("y")) {
+               w.Write([]byte("not equal"))
+       }
+}
+-- y_test.go --
+package y
+
+import (
+       "bytes"
+       "io"
        "os"
        "testing"
-       "time"
 )
 
 func FuzzMinimizeZeroDurationSet(f *testing.F) {
@@ -126,42 +150,42 @@ func FuzzMinimizerRecoverable(f *testing.F) {
                                if len(b) == 50 {
                                        t.Log("got the minimum size!")
                                }
-                               t.Fatal("contains a non-zero byte")
+                               t.Fatalf("contains a non-zero byte of length %d", len(b))
                        }
                }
        })
 }
 
 func FuzzMinimizerNonrecoverable(f *testing.F) {
-       f.Add(make([]byte, 100))
        f.Fuzz(func(t *testing.T, b []byte) {
-               if len(b) < 50 {
-                       // Make sure that b is large enough that it can be minimized
-                       return
-               }
-               // Given the randomness of the mutations, this should allow the
-               // minimizer to trim down the value a bit.
-               for _, n := range b {
-                       if n != 0 {
-                               t.Log("contains a non-zero byte")
-                               os.Exit(99)
-                       }
-               }
+               os.Exit(99)
        })
 }
 
-func FuzzMinimizerTooSlow(f *testing.F) {
+func FuzzMinimizerCrashInMinimization(f *testing.F) {
+       seed := make([]byte, 1000)
+       f.Add(seed)
        f.Fuzz(func(t *testing.T, b []byte) {
-               if len(b) > 50 {
-                       t.Error("error here")
-                       time.Sleep(2 * time.Second)
+               if len(b) < 50 || len(b) > 1100 {
+                       // Make sure that b is large enough that it can be minimized
+                       return
+               }
+               if !bytes.Equal(b, seed) {
+                       // This should have hit a new edge, and the interesting input
+                       // should be attempting minimization
+                       Y(io.Discard, b)
+               }
+               if len(b) < 350 {
+                       t.Error("flaky failure")
+               }
+               if len(b) == 50 {
+                       t.Log("got the minimum size!")
                }
        })
 }
-
--- check_testdata.go --
-// +build ignore
-
+-- empty/empty.go --
+package empty
+-- check_testdata/check_testdata.go --
 package main
 
 import (
index 5d04d8c0223ec424a91b40392b73ef86acc7b2ec..18f634a3b64d3b1cd6eb1209191602b427a354c9 100644 (file)
@@ -47,6 +47,23 @@ cp cache-file $GOCACHE/fuzz/example.com/x/FuzzWithCache/1
 stdout 'Crash written to testdata[/\\]fuzz[/\\]FuzzWithCache[/\\]'
 stdout FAIL
 
+# Write a crashing input to the cache
+mkdir $GOCACHE/fuzz/example.com/x/FuzzWithMinimizableCache
+cp cache-file-bytes $GOCACHE/fuzz/example.com/x/FuzzWithMinimizableCache/1
+
+# Test that fuzzing a target with a failure in the cache minimizes it and writes
+# the new crash to testdata/fuzz
+! go test -fuzz=FuzzWithMinimizableCache -run=FuzzWithMinimizableCache -fuzztime=10000x
+! stdout ^ok
+stdout 'gathering baseline coverage'
+stdout 'got the minimum size!'
+stdout 'contains a non-zero byte of length 10'
+stdout 'Crash written to testdata[/\\]fuzz[/\\]FuzzWithMinimizableCache[/\\]'
+stdout FAIL
+# Make sure this crash didn't come from fuzzing
+# (the log line that states fuzzing began shouldn't have printed)
+! stdout 'execs'
+
 # Clear the fuzz cache and make sure it's gone
 go clean -fuzzcache
 ! exists $GOCACHE/fuzz
@@ -158,6 +175,22 @@ func FuzzWithCache(f *testing.F) {
     })
 }
 
+func FuzzWithMinimizableCache(f *testing.F) {
+    f.Fuzz(func(t *testing.T, b []byte) {
+               if len(b) < 10 {
+                       return
+               }
+               for _, n := range b {
+                       if n != 0 {
+                               if len(b) == 10 {
+                                       t.Log("got the minimum size!")
+                               }
+                               t.Fatalf("contains a non-zero byte of length %d", len(b))
+                       }
+               }
+    })
+}
+
 func FuzzRunNoneWithCache(f *testing.F) {
     f.Fuzz(func(t *testing.T, i int) {
         if i == 10 {
@@ -170,4 +203,7 @@ go test fuzz v1
 int(10)
 -- cache-file --
 go test fuzz v1
-int(10)
\ No newline at end of file
+int(10)
+-- cache-file-bytes --
+go test fuzz v1
+[]byte("11111111111111111111")
\ No newline at end of file
index 03071d552140103602e1cb89e0d19f982218bb7a..5b3819be752c6cc33cde8ead940d30d17005ce2e 100644 (file)
@@ -875,12 +875,10 @@ func (c *coordinator) updateCoverage(newCoverage []byte) int {
 }
 
 // canMinimize returns whether the coordinator should attempt to find smaller
-// inputs that reproduce a crash or new coverage. It shouldn't do this if it
-// is in the warmup phase.
+// inputs that reproduce a crash or new coverage.
 func (c *coordinator) canMinimize() bool {
        return c.minimizationAllowed &&
-               (c.opts.Limit == 0 || c.count+c.countWaiting < c.opts.Limit) &&
-               !c.warmupRun()
+               (c.opts.Limit == 0 || c.count+c.countWaiting < c.opts.Limit)
 }
 
 func (c *coordinator) elapsed() time.Duration {
index 410b78310be998679e3804aa3a91f19301b864b2..dd76baff51f6290c85851c1c694203a6dec5019a 100644 (file)
@@ -262,13 +262,12 @@ func TestMinimizeInput(t *testing.T) {
        }
 }
 
-// TestMinimizeInputCoverageError checks that if we're minimizing an interesting
-// input (one that we don't expect to cause an error), and the fuzz function
-// returns an error, minimizing fails, and we return the error quickly.
-func TestMinimizeInputCoverageError(t *testing.T) {
-       errOhNo := errors.New("ohno")
+// TestMinimizeFlaky checks that if we're minimizing an interesting
+// input and a flaky failure occurs, that minimization was not indicated
+// to be successful, and the error isn't returned (since it's flaky).
+func TestMinimizeFlaky(t *testing.T) {
        ws := &workerServer{fuzzFn: func(e CorpusEntry) error {
-               return errOhNo
+               return errors.New("ohno")
        }}
        keepCoverage := make([]byte, len(coverageSnapshot))
        count := int64(0)
@@ -277,7 +276,7 @@ func TestMinimizeInputCoverageError(t *testing.T) {
        if success {
                t.Error("unexpected success")
        }
-       if err != errOhNo {
+       if err != nil {
                t.Errorf("unexpected error: %v", err)
        }
        if count != 1 {
index 0c428ed832d3bacacbce68a389ace2e158512429..e3827b112a88dd18ea06c8982b9a72bf0579b0ec 100644 (file)
@@ -261,8 +261,8 @@ func (w *worker) minimize(ctx context.Context, input fuzzMinimizeInput) (min fuz
                return fuzzResult{}, fmt.Errorf("fuzzing process terminated unexpectedly while minimizing: %w", w.waitErr)
        }
 
-       if input.crasherMsg != "" && resp.Err == "" && !resp.Success {
-               return fuzzResult{}, fmt.Errorf("attempted to minimize but could not reproduce")
+       if input.crasherMsg != "" && resp.Err == "" {
+               return fuzzResult{}, fmt.Errorf("attempted to minimize a crash but could not reproduce")
        }
 
        return fuzzResult{
@@ -509,12 +509,11 @@ type minimizeArgs struct {
 
 // minimizeResponse contains results from workerServer.minimize.
 type minimizeResponse struct {
-       // Success is true if the worker found a smaller input, stored in shared
-       // memory, that was "interesting" for the same reason as the original input.
-       // If minimizeArgs.KeepCoverage was set, the minimized input preserved at
-       // least one coverage bit and did not cause an error. Otherwise, the
-       // minimized input caused some error, recorded in Err.
-       Success bool
+       // WroteToMem is true if the worker found a smaller input and wrote it to
+       // shared memory. If minimizeArgs.KeepCoverage was set, the minimized input
+       // preserved at least one coverage bit and did not cause an error.
+       // Otherwise, the minimized input caused some error, recorded in Err.
+       WroteToMem bool
 
        // Err is the error string caused by the value in shared memory, if any.
        Err string
@@ -777,32 +776,31 @@ func (ws *workerServer) minimize(ctx context.Context, args minimizeArgs) (resp m
        }
 
        // Minimize the values in vals, then write to shared memory. We only write
-       // to shared memory after completing minimization. If the worker terminates
-       // unexpectedly before then, the coordinator will use the original input.
-       resp.Success, err = ws.minimizeInput(ctx, vals, &mem.header().count, args.Limit, args.KeepCoverage)
-       if resp.Success {
+       // to shared memory after completing minimization.
+       // TODO(48165): If the worker terminates unexpectedly during minimization,
+       // the coordinator has no way of retrieving the crashing input.
+       success, err := ws.minimizeInput(ctx, vals, &mem.header().count, args.Limit, args.KeepCoverage)
+       if success {
                writeToMem(vals, mem)
-       }
-       if err != nil {
-               resp.Err = err.Error()
-       } else if resp.Success {
-               resp.CoverageData = coverageSnapshot
+               resp.WroteToMem = true
+               if err != nil {
+                       resp.Err = err.Error()
+               } else {
+                       resp.CoverageData = coverageSnapshot
+               }
        }
        return resp
 }
 
 // minimizeInput applies a series of minimizing transformations on the provided
-// vals, ensuring that each minimization still causes an error in fuzzFn. Before
-// every call to fuzzFn, it marshals the new vals and writes it to the provided
-// mem just in case an unrecoverable error occurs. It uses the context to
-// determine how long to run, stopping once closed. It returns a bool
-// indicating whether minimization was successful and an error if one was found.
+// vals, ensuring that each minimization still causes an error in fuzzFn. It
+// uses the context to determine how long to run, stopping once closed. It
+// returns a bool indicating whether minimization was successful and an error if
+// one was found.
 func (ws *workerServer) minimizeInput(ctx context.Context, vals []interface{}, count *int64, limit int64, keepCoverage []byte) (success bool, retErr error) {
-       wantError := keepCoverage == nil
        shouldStop := func() bool {
                return ctx.Err() != nil ||
-                       (limit > 0 && *count >= limit) ||
-                       (retErr != nil && !wantError)
+                       (limit > 0 && *count >= limit)
        }
        if shouldStop() {
                return false, nil
@@ -812,11 +810,12 @@ func (ws *workerServer) minimizeInput(ctx context.Context, vals []interface{}, c
        // If not, then whatever caused us to think the value was interesting may
        // have been a flake, and we can't minimize it.
        *count++
-       if retErr = ws.fuzzFn(CorpusEntry{Values: vals}); retErr == nil && wantError {
-               return false, nil
-       } else if retErr != nil && !wantError {
-               return false, retErr
-       } else if keepCoverage != nil && !hasCoverageBit(keepCoverage, coverageSnapshot) {
+       retErr = ws.fuzzFn(CorpusEntry{Values: vals})
+       if keepCoverage != nil {
+               if !hasCoverageBit(keepCoverage, coverageSnapshot) || retErr != nil {
+                       return false, nil
+               }
+       } else if retErr == nil {
                return false, nil
        }
 
@@ -881,7 +880,13 @@ func (ws *workerServer) minimizeInput(ctx context.Context, vals []interface{}, c
                err := ws.fuzzFn(CorpusEntry{Values: vals})
                if err != nil {
                        retErr = err
-                       return wantError
+                       if keepCoverage != nil {
+                               // Now that we've found a crash, that's more important than any
+                               // minimization of interesting inputs that was being done. Clear out
+                               // keepCoverage to only minimize the crash going forward.
+                               keepCoverage = nil
+                       }
+                       return true
                }
                if keepCoverage != nil && hasCoverageBit(keepCoverage, coverageSnapshot) {
                        return true
@@ -939,7 +944,7 @@ func (ws *workerServer) minimizeInput(ctx context.Context, vals []interface{}, c
                        panic("unreachable")
                }
        }
-       return (wantError || retErr == nil), retErr
+       return true, retErr
 }
 
 func writeToMem(vals []interface{}, mem *sharedMem) {
@@ -1024,7 +1029,7 @@ func (wc *workerClient) minimize(ctx context.Context, entryIn CorpusEntry, args
        }
        defer func() { wc.memMu <- mem }()
        resp.Count = mem.header().count
-       if resp.Success {
+       if resp.WroteToMem {
                entryOut.Data = mem.valueCopy()
                entryOut.Values, err = unmarshalCorpusFile(entryOut.Data)
                h := sha256.Sum256(entryOut.Data)