From 49588d0c555aaacb1354dced5e1fad0195ded7c6 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Thu, 9 Mar 2023 15:59:17 -0800 Subject: [PATCH] internal/fuzz: avoid deadlock on duplicate entries with exec limit If there was a execution limit enabled, and a result put us beyond that limit, but the result expanded coverage *and* was a duplicate of an entry already in the cache, the check if we were passed the limit would be skipped. Since this check was inside the result check body, and we would no longer send any new inputs, we'd never get to that check again, causing the coordinator to just sit in an infinite loop. This moves the check up to the top of the coordinator loop, so that it is checked after every result is processed. Also add a cmd/go TestScript regression test which triggered this case much more frequently. Updates #51484 Change-Id: I7a2181051177acb853c1009beedd334a40796177 Reviewed-on: https://go-review.googlesource.com/c/go/+/475196 Auto-Submit: Roland Shoemaker Reviewed-by: Bryan Mills Run-TryBot: Roland Shoemaker TryBot-Result: Gopher Robot --- .../script/test_fuzz_limit_dup_entry.txt | 37 +++++++++++++++++++ src/internal/fuzz/fuzz.go | 12 +++--- 2 files changed, 43 insertions(+), 6 deletions(-) create mode 100644 src/cmd/go/testdata/script/test_fuzz_limit_dup_entry.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_limit_dup_entry.txt b/src/cmd/go/testdata/script/test_fuzz_limit_dup_entry.txt new file mode 100644 index 0000000000..83235f4b6d --- /dev/null +++ b/src/cmd/go/testdata/script/test_fuzz_limit_dup_entry.txt @@ -0,0 +1,37 @@ +[!fuzz] skip +[short] skip + +# FuzzA attempts to cause the mutator to create duplicate inputs that generate +# new coverage. Previously this would trigger a corner case when the fuzzer +# had a execution limit, causing it to deadlock and sit in the coordinator +# loop indefinitely, failing to exit once the limit had been exhausted. + +go clean --fuzzcache +go test -fuzz=FuzzA -fuzztime=100x -parallel=1 + +-- go.mod -- +module m + +go 1.16 +-- fuzz_test.go -- +package fuzz_test + +import ( + "fmt" + "testing" +) + +func FuzzA(f *testing.F) { + f.Add([]byte("seed")) + i := 0 + f.Fuzz(func(t *testing.T, b []byte) { + i++ + if string(b) == "seed" { + if i == 0 { + fmt.Println("a") + } else if i > 1 { + fmt.Println("b") + } + } + }) +} diff --git a/src/internal/fuzz/fuzz.go b/src/internal/fuzz/fuzz.go index 7d4fe06198..fd3dad29b4 100644 --- a/src/internal/fuzz/fuzz.go +++ b/src/internal/fuzz/fuzz.go @@ -195,6 +195,11 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err c.logStats() for { + // If there is an execution limit, and we've reached it, stop. + if c.opts.Limit > 0 && c.count >= c.opts.Limit { + stop(nil) + } + var inputC chan fuzzInput input, ok := c.peekInput() if ok && c.crashMinimizing == nil && !stopping { @@ -311,6 +316,7 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err if c.canMinimize() && result.canMinimize && c.crashMinimizing == nil { // Send back to workers to find a smaller value that preserves // at least one new coverage bit. + c.queueForMinimization(result, keepCoverage) } else { // Update the coordinator's coverage mask and save the value. @@ -370,12 +376,6 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err } } - // Once the result has been processed, stop the worker if we - // have reached the fuzzing limit. - if c.opts.Limit > 0 && c.count >= c.opts.Limit { - stop(nil) - } - case inputC <- input: // Sent the next input to a worker. c.sentInput(input) -- 2.48.1