From: Katie Hockman <katie@golang.org>
Date: Wed, 3 Mar 2021 21:00:49 +0000 (-0500)
Subject: [dev.fuzz] internal/fuzz: crash if there is no error output
X-Git-Tag: go1.18beta1~1282^2~87
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=354c77a108863a1ccae810c94453870f0678a828;p=gostls13.git

[dev.fuzz] internal/fuzz: crash if there is no error output

Previously, the coordintor used the error string encoded by the worker
to determine whether or not a crash occurred. However, failures caused
by things like t.Fail() which have no output will have an empty error
string, so we can't rely on the error string alone to determine if
something is a crasher or not.

Change-Id: Idcf7f6b1210aa1dc4e8dab222642c87919595693
Reviewed-on: https://go-review.googlesource.com/c/go/+/298351
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
---

diff --git a/src/cmd/go/testdata/script/test_fuzz_mutate_crash.txt b/src/cmd/go/testdata/script/test_fuzz_mutate_crash.txt
index 2f5d069e69..66e1cd8b76 100644
--- a/src/cmd/go/testdata/script/test_fuzz_mutate_crash.txt
+++ b/src/cmd/go/testdata/script/test_fuzz_mutate_crash.txt
@@ -35,6 +35,22 @@ stdout 'testdata[/\\]corpus[/\\]FuzzWithNilPanic[/\\]'
 stdout 'runtime.Goexit'
 go run check_testdata.go FuzzWithNilPanic
 
+! go test -run=FuzzWithFail -fuzz=FuzzWithFail -fuzztime=5s -parallel=1
+stdout 'testdata[/\\]corpus[/\\]FuzzWithFail[/\\]'
+go run check_testdata.go FuzzWithFail
+
+! go test -run=FuzzWithErrorf -fuzz=FuzzWithErrorf -fuzztime=5s -parallel=1
+stdout 'testdata[/\\]corpus[/\\]FuzzWithErrorf[/\\]'
+# TODO: Uncomment this part of the test once it's fixed
+# stdout 'errorf was called here'
+go run check_testdata.go FuzzWithErrorf
+
+! go test -run=FuzzWithFatalf -fuzz=FuzzWithFatalf -fuzztime=5s -parallel=1
+stdout 'testdata[/\\]corpus[/\\]FuzzWithFatalf[/\\]'
+# TODO: Uncomment this part of the test once it's fixed
+# stdout 'fatalf was called here'
+go run check_testdata.go FuzzWithFatalf
+
 ! go test -run=FuzzWithBadExit -fuzz=FuzzWithBadExit -fuzztime=5s -parallel=1
 stdout 'testdata[/\\]corpus[/\\]FuzzWithBadExit[/\\]'
 stdout 'unexpectedly'
@@ -70,6 +86,33 @@ func FuzzWithNilPanic(f *testing.F) {
 	})
 }
 
+func FuzzWithFail(f *testing.F) {
+	f.Add([]byte("aa"))
+	f.Fuzz(func(t *testing.T, b []byte) {
+		if string(b) != "aa" {
+			t.Fail()
+		}
+	})
+}
+
+func FuzzWithErrorf(f *testing.F) {
+	f.Add([]byte("aa"))
+	f.Fuzz(func(t *testing.T, b []byte) {
+		if string(b) != "aa" {
+			t.Errorf("errorf was called here")
+		}
+	})
+}
+
+func FuzzWithFatalf(f *testing.F) {
+	f.Add([]byte("aa"))
+	f.Fuzz(func(t *testing.T, b []byte) {
+		if string(b) != "aa" {
+			t.Fatalf("fatalf was called here")
+		}
+	})
+}
+
 func FuzzWithTwoTypes(f *testing.F) {
 	f.Fuzz(func(t *testing.T, a, b []byte) {
 		if len(a) > 0 && len(b) > 0 {
diff --git a/src/internal/fuzz/worker.go b/src/internal/fuzz/worker.go
index e2b3c3d7a6..b44c321aac 100644
--- a/src/internal/fuzz/worker.go
+++ b/src/internal/fuzz/worker.go
@@ -142,7 +142,7 @@ func (w *worker) runFuzzing() error {
 					}
 					// TODO(jayconrod): what happens if testing.F.Fuzz is never called?
 					// TODO(jayconrod): time out if the test process hangs.
-				} else if resp.Err != "" {
+				} else if resp.Crashed {
 					// The worker found a crasher. Inform the coordinator.
 					crasher := crasherEntry{
 						CorpusEntry: CorpusEntry{Data: value},
@@ -357,7 +357,12 @@ type fuzzResponse struct {
 	// the coordinator (for example, because it expanded coverage).
 	Interesting bool
 
-	// Err is set if the value in shared memory caused a crash.
+	// Crashed indicates the value in shared memory caused a crash.
+	Crashed bool
+
+	// Err is the error string caused by the value in shared memory. This alone
+	// cannot be used to determine whether this value caused a crash, since a
+	// crash can occur without any output (e.g. with t.Fail()).
 	Err string
 }
 
@@ -469,7 +474,7 @@ func (ws *workerServer) fuzz(ctx context.Context, args fuzzArgs) fuzzResponse {
 			mem.setValueLen(len(b))
 			mem.setValue(b)
 			if err := ws.fuzzFn(CorpusEntry{Values: vals}); err != nil {
-				return fuzzResponse{Err: err.Error()}
+				return fuzzResponse{Crashed: true, Err: err.Error()}
 			}
 			// TODO(jayconrod,katiehockman): return early if we find an
 			// interesting value.