From 514ebaec358488d1dcf7253b9d05b4ad8c76c390 Mon Sep 17 00:00:00 2001 From: Katie Hockman Date: Fri, 22 Oct 2021 15:59:15 -0400 Subject: [PATCH] internal/fuzz: don't deflake coverage found while fuzzing Previously, the worker would attempt to deflake an input that was reported to have caused new coverage. The chances of a flake causing new coverage seem pretty low to me, and even if it was a flake that caused it, adding that input to the cache doesn't seem like a bad thing. The input is already going to be deflaked during minimization anyway. If by some off-chance the code is causing a lot of flaky coverage increases, and the user doesn't want minimization to occur, then setting -fuzzminimizetime=1x will deflake in the way they want without minimizing. This can be documented as needed. This fixes a bug where the mem.header().count could have been one too large if an unrecoverable crash occured while deflaking an input that caused code coverage. Fixes #49047 Change-Id: Ibdf893d7a89a46dd700702afb09e35623615390e Reviewed-on: https://go-review.googlesource.com/c/go/+/358094 Trust: Katie Hockman Run-TryBot: Katie Hockman TryBot-Result: Go Bot Reviewed-by: Julie Qiu Reviewed-by: Roland Shoemaker Reviewed-by: Bryan C. Mills --- .../script/test_fuzz_mutator_repeat.txt | 24 ++++++++++++------- src/internal/fuzz/worker.go | 17 +++---------- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/cmd/go/testdata/script/test_fuzz_mutator_repeat.txt b/src/cmd/go/testdata/script/test_fuzz_mutator_repeat.txt index f2952c349b..15d7cb6b32 100644 --- a/src/cmd/go/testdata/script/test_fuzz_mutator_repeat.txt +++ b/src/cmd/go/testdata/script/test_fuzz_mutator_repeat.txt @@ -1,5 +1,3 @@ -skip # Flaky: https://golang.org/issue/49047 - # TODO(jayconrod): support shared memory on more platforms. [!darwin] [!linux] [!windows] skip @@ -9,11 +7,11 @@ skip # Flaky: https://golang.org/issue/49047 [short] skip -# Start fuzzing. The worker crashes after ~100 iterations. +# Start fuzzing. The worker crashes after 100 iterations. # The fuzz function writes the crashing input to "want" before exiting. # The fuzzing engine reconstructs the crashing input and saves it to testdata. ! exists want -! go test -fuzz=. -parallel=1 +! go test -fuzz=. -parallel=1 -fuzztime=110x -fuzzminimizetime=10x -v stdout 'fuzzing process terminated unexpectedly' stdout 'Crash written to testdata' @@ -46,12 +44,20 @@ func FuzzRepeat(f *testing.F) { if i == 100 { f, err := os.OpenFile("want", os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0666) if err != nil { - // Couldn't create the file, probably because it already exists, - // and we're minimizing now. Return without crashing. - return + // Couldn't create the file. Return without crashing, and try + // again. + i-- + t.Skip(err) + } + if _, err := f.Write(b); err != nil { + // We already created the file, so if we failed to write it + // there's not much we can do. The test will fail anyway, but + // at least make sure the error is logged to stdout. + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) } - f.Write(b) - f.Close() os.Exit(1) // crash without communicating } diff --git a/src/internal/fuzz/worker.go b/src/internal/fuzz/worker.go index b36ebe2a7e..388675f713 100644 --- a/src/internal/fuzz/worker.go +++ b/src/internal/fuzz/worker.go @@ -738,20 +738,9 @@ func (ws *workerServer) fuzz(ctx context.Context, args fuzzArgs) (resp fuzzRespo return resp } if cov != nil { - // Found new coverage. Before reporting to the coordinator, - // run the same values once more to deflake. - if !shouldStop() { - dur, cov, errMsg = fuzzOnce(entry) - if errMsg != "" { - resp.Err = errMsg - return resp - } - } - if cov != nil { - resp.CoverageData = cov - resp.InterestingDuration = dur - return resp - } + resp.CoverageData = cov + resp.InterestingDuration = dur + return resp } if shouldStop() { return resp -- 2.50.0