Based on comments in CL 348469.
Note that with this change, F.Fuzz no longer calls
runtime.Goexit. This simplifies our logic and makes F.Fuzz more
predictable.
Change-Id: I6c3c65b0e8e8f261621cbe2f17375e8164ef60a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/351316
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
// dependencies.
//
// The -fuzzcache flag causes clean to remove files stored in the Go build
-// cache for fuzz testing. Files stored in source testdata directories
-// are left in place.
+// cache for fuzz testing. The fuzzing engine caches files that expand
+// code coverage, so removing them may make fuzzing less effective until
+// new inputs are found that provide the same coverage. These files are
+// distinct from those stored in testdata directory; clean does not remove
+// those files.
//
// For more about build flags, see 'go help build'.
//
// See 'go help test' for details. Running 'go clean -testcache' removes
// all cached test results (but not cached build results).
//
+// The go command also caches values used in fuzzing with 'go test -fuzz',
+// specifically, values that expanded code coverage when passed to a
+// fuzz function. These values are not used for regular building and
+// testing, but they're stored in a subdirectory of the build cache.
+// Running 'go clean -fuzzcache' removes all cached fuzzing values.
+// This may make fuzzing less effective, temporarily.
+//
// The GODEBUG environment variable can enable printing of debugging
// information about the state of the cache:
//
// This directory is managed by the internal/fuzz package. Files in this
// directory aren't removed by the 'go clean -cache' command or by Trim.
// They may be removed with 'go clean -fuzzcache'.
+//
+// TODO(#48526): make Trim remove unused files from this directory.
func (c *Cache) FuzzDir() string {
return filepath.Join(c.dir, "fuzz")
}
dependencies.
The -fuzzcache flag causes clean to remove files stored in the Go build
-cache for fuzz testing. Files stored in source testdata directories
-are left in place.
+cache for fuzz testing. The fuzzing engine caches files that expand
+code coverage, so removing them may make fuzzing less effective until
+new inputs are found that provide the same coverage. These files are
+distinct from those stored in testdata directory; clean does not remove
+those files.
For more about build flags, see 'go help build'.
}
if !cfg.BuildN {
if err := os.RemoveAll(fuzzDir); err != nil {
- base.Errorf("go clean -fuzzcache: %v", err)
+ base.Errorf("go: %v", err)
}
}
}
See 'go help test' for details. Running 'go clean -testcache' removes
all cached test results (but not cached build results).
+The go command also caches values used in fuzzing with 'go test -fuzz',
+specifically, values that expanded code coverage when passed to a
+fuzz function. These values are not used for regular building and
+testing, but they're stored in a subdirectory of the build cache.
+Running 'go clean -fuzzcache' removes all cached fuzzing values.
+This may make fuzzing less effective, temporarily.
+
The GODEBUG environment variable can enable printing of debugging
information about the state of the cache:
// that allows specifying different effective flags for different packages.
// See 'go help build' for more details about per-package flags.
type PerPackageFlag struct {
- present bool
- values []ppfValue
- seenPackages map[*Package]bool // the packages for which the flags have already been set
+ present bool
+ values []ppfValue
}
// A ppfValue is a single <pattern>=<flags> per-package flag value.
func setToolFlags(pkgs ...*Package) {
for _, p := range PackageList(pkgs) {
- appendFlags(p, &p.Internal.Asmflags, &BuildAsmflags)
- appendFlags(p, &p.Internal.Gcflags, &BuildGcflags)
- appendFlags(p, &p.Internal.Ldflags, &BuildLdflags)
- appendFlags(p, &p.Internal.Gccgoflags, &BuildGccgoflags)
- }
-}
-
-func appendFlags(p *Package, flags *[]string, packageFlag *PerPackageFlag) {
- if !packageFlag.seenPackages[p] {
- if packageFlag.seenPackages == nil {
- packageFlag.seenPackages = make(map[*Package]bool)
- }
- packageFlag.seenPackages[p] = true
- *flags = append(*flags, packageFlag.For(p)...)
+ p.Internal.Asmflags = BuildAsmflags.For(p)
+ p.Internal.Gcflags = BuildGcflags.For(p)
+ p.Internal.Ldflags = BuildLdflags.For(p)
+ p.Internal.Gccgoflags = BuildGccgoflags.For(p)
}
}
if testFuzz != "" && fuzzFlags != nil {
// Don't instrument packages which may affect coverage guidance but are
// unlikely to be useful. Most of these are used by the testing or
- // internal/fuzz concurrently with fuzzing.
+ // internal/fuzz packages concurrently with fuzzing.
var fuzzNoInstrument = map[string]bool{
"context": true,
"internal/fuzz": true,
}
// FuzzInstrumentFlags returns compiler flags that enable fuzzing instrumation
-// on supported platforms. On unsupported platforms, FuzzInstrumentFlags returns
-// nil.
+// on supported platforms.
+//
+// On unsupported platforms, FuzzInstrumentFlags returns nil, meaning no
+// instrumentation is added. 'go test -fuzz' still works without coverage,
+// but it generates random inputs without guidance, so it's much less effective.
func FuzzInstrumentFlags() []string {
// TODO: expand the set of supported platforms, with testing.
// Nothing about the instrumentation is OS specific, but only amd64 and arm64
stdout FAIL
stdout 'f.Fuzz function'
-# Test that a call to f.Fatal after the Fuzz func is never executed.
-go test fatal_after_fuzz_func_fuzz_test.go
-stdout ok
-! stdout FAIL
+# Test that runtime.Goexit within the fuzz function is an error.
+! go test goexit_fuzz_fn_fuzz_test.go
+! stdout ^ok
+stdout FAIL
+
+# Test that a call to f.Fatal after the Fuzz func is executed.
+! go test fatal_after_fuzz_func_fuzz_test.go
+! stdout ok
+stdout FAIL
# Test that missing *T in f.Fuzz causes a non-zero exit status.
! go test incomplete_fuzz_call_fuzz_test.go
})
}
+-- goexit_fuzz_fn_fuzz_test.go --
+package goexit_fuzz_fn_fuzz
+
+import "testing"
+
+func Fuzz(f *testing.F) {
+ f.Add([]byte("aa"))
+ f.Fuzz(func(t *testing.T, b []byte) {
+ runtime.Goexit()
+ })
+}
+
-- fatal_after_fuzz_func_fuzz_test.go --
package fatal_after_fuzz_func_fuzz
stdout 'runtime.Goexit'
go run check_testdata.go FuzzWithNilPanic
+! go test -run=FuzzWithGoexit -fuzz=FuzzWithGoexit -fuzztime=100x -fuzzminimizetime=1000x
+stdout 'testdata[/\\]fuzz[/\\]FuzzWithGoexit[/\\]'
+stdout 'runtime.Goexit'
+go run check_testdata.go FuzzWithGoexit
+
! go test -run=FuzzWithFail -fuzz=FuzzWithFail -fuzztime=100x -fuzzminimizetime=1000x
stdout 'testdata[/\\]fuzz[/\\]FuzzWithFail[/\\]'
go run check_testdata.go FuzzWithFail
package fuzz_crash
import (
- "os"
+ "os"
+ "runtime"
"testing"
)
})
}
+func FuzzWithGoexit(f *testing.F) {
+ f.Add([]byte("aa"))
+ f.Fuzz(func(t *testing.T, b []byte) {
+ if string(b) != "aa" {
+ runtime.Goexit()
+ }
+ })
+}
+
func FuzzWithFail(f *testing.F) {
f.Add([]byte("aa"))
f.Fuzz(func(t *testing.T, b []byte) {
// from testdata.
corpus []corpusEntry
- result FuzzResult
+ result fuzzResult
fuzzCalled bool
}
// whose remaining arguments are the types to be fuzzed.
// For example:
//
-// f.Fuzz(func(t *testing.T, b []byte, i int) { ... })
+// f.Fuzz(func(t *testing.T, b []byte, i int) { ... })
//
-// This function should be fast, deterministic, and stateless.
+// The following types are allowed: []byte, string, bool, byte, rune, float32,
+// float64, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64.
+// More types may be supported in the future.
//
-// No mutatable input arguments, or pointers to them, should be retained between
-// executions of the fuzz function, as the memory backing them may be mutated
-// during a subsequent invocation.
+// This function sould be fast and deterministic, and its behavior should not
+// depend on shared state. No mutatable input arguments, or pointers to them,
+// should be retained between executions of the fuzz function, as the memory
+// backing them may be mutated during a subsequent invocation.
//
-// This is a terminal function which will terminate the currently running fuzz
-// target by calling runtime.Goexit.
-// To run any code after fuzzing stops, use (*F).Cleanup.
+// When fuzzing, F.Fuzz does not return until a problem is found, time runs
+// out (set with -fuzztime), or the test process is interrupted by a signal.
+// F.Fuzz should be called exactly once unless F.Skip or F.Fail is called.
func (f *F) Fuzz(ff interface{}) {
if f.fuzzCalled {
panic("testing: F.Fuzz called more than once")
corpusTargetDir,
cacheTargetDir)
if err != nil {
- f.result = FuzzResult{Error: err}
+ f.result = fuzzResult{Error: err}
f.Fail()
fmt.Fprintf(f.w, "%v\n", err)
if crashErr, ok := err.(fuzzCrashError); ok {
run(e)
}
}
-
- // Record that the fuzz function (or coordinateFuzzing or runFuzzWorker)
- // returned normally. This is used to distinguish runtime.Goexit below
- // from panic(nil).
- f.finished = true
-
- // Terminate the goroutine. F.Fuzz should not return.
- // We cannot call runtime.Goexit from a deferred function: if there is a
- // panic, that would replace the panic value with nil.
- runtime.Goexit()
}
func (f *F) report() {
}
}
-// FuzzResult contains the results of a fuzz run.
-type FuzzResult struct {
+// fuzzResult contains the results of a fuzz run.
+type fuzzResult struct {
N int // The number of iterations.
T time.Duration // The total time taken.
Error error // Error is the error from the crash
}
-func (r FuzzResult) String() string {
+func (r fuzzResult) String() string {
s := ""
if r.Error == nil {
return s
atomic.AddUint32(&numFailed, 1)
}
err := recover()
- f.mu.RLock()
- ok := f.skipped || f.failed || (f.fuzzCalled && f.finished)
- f.mu.RUnlock()
- if err == nil && !ok {
- err = errNilPanicOrGoexit
+ if err == nil {
+ f.mu.RLock()
+ fuzzNotCalled := !f.fuzzCalled && !f.skipped && !f.failed
+ if !f.finished && !f.skipped && !f.failed {
+ err = errNilPanicOrGoexit
+ }
+ f.mu.RUnlock()
+ if fuzzNotCalled && err == nil {
+ f.Error("returned without calling F.Fuzz, F.Fail, or F.Skip")
+ }
}
// Use a deferred call to ensure that we report that the test is
- // complete even if a cleanup function calls t.FailNow. See issue 41355.
+ // complete even if a cleanup function calls F.FailNow. See issue 41355.
didPanic := false
defer func() {
- if didPanic {
- return
+ if !didPanic {
+ // Only report that the test is complete if it doesn't panic,
+ // as otherwise the test binary can exit before the panic is
+ // reported to the user. See issue 41479.
+ f.signal <- true
}
- if err != nil {
- panic(err)
- }
- // Only report that the test is complete if it doesn't panic,
- // as otherwise the test binary can exit before the panic is
- // reported to the user. See issue 41479.
- f.signal <- true
}()
// If we recovered a panic or inappropriate runtime.Goexit, fail the test,
if len(f.sub) > 0 {
// Unblock inputs that called T.Parallel while running the seed corpus.
- // T.Parallel has no effect while fuzzing, so this only affects fuzz
- // targets run as normal tests.
+ // This only affects fuzz targets run as normal tests.
+ // While fuzzing, T.Parallel has no effect, so f.sub is empty, and this
+ // branch is not taken. f.barrier is nil in that case.
close(f.barrier)
// Wait for the subtests to complete.
for _, sub := range f.sub {
f.start = time.Now()
fn(f)
- // Code beyond this point is only executed if fn returned normally.
- // That means fn did not call F.Fuzz or F.Skip. It should have called F.Fail.
+ // Code beyond this point will not be executed when FailNow or SkipNow
+ // is invoked.
f.mu.Lock()
- defer f.mu.Unlock()
- if !f.failed {
- panic(f.name + " returned without calling F.Fuzz, F.Fail, or F.Skip")
- }
+ f.finished = true
+ f.mu.Unlock()
}
//
// For example:
//
-// func FuzzHex(f *testing.F) {
-// for _, seed := range [][]byte{{}, {0}, {9}, {0xa}, {0xf}, {1, 2, 3, 4}} {
-// f.Add(seed)
-// }
-// f.Fuzz(func(t *testing.T, in []byte) {
-// enc := hex.EncodeToString(in)
-// out, err := hex.DecodeString(enc)
-// if err != nil {
-// t.Fatalf("%v: decode: %v", in, err)
-// }
-// if !bytes.Equal(in, out) {
-// t.Fatalf("%v: not equal after round trip: %v", in, out)
+// func FuzzHex(f *testing.F) {
+// for _, seed := range [][]byte{{}, {0}, {9}, {0xa}, {0xf}, {1, 2, 3, 4}} {
+// f.Add(seed)
+// }
+// f.Fuzz(func(t *testing.T, in []byte) {
+// enc := hex.EncodeToString(in)
+// out, err := hex.DecodeString(enc)
+// if err != nil {
+// t.Fatalf("%v: decode: %v", in, err)
+// }
+// if !bytes.Equal(in, out) {
+// t.Fatalf("%v: not equal after round trip: %v", in, out)
+// }
+// })
// }
-// })
-// }
//
// Seed inputs may be registered by calling F.Add or by storing files in the
// directory testdata/fuzz/<Name> (where <Name> is the name of the fuzz target)
name string // Name of test or benchmark.
start time.Time // Time test or benchmark started
duration time.Duration
- barrier chan bool // To signal parallel subtests they may start.
+ barrier chan bool // To signal parallel subtests they may start. Nil when T.Parallel is not present (B) or not usable (when fuzzing).
signal chan bool // To signal a test is done.
sub []*T // Queue of subtests to be run in parallel.
// and inserts the final newline if needed and indentation spaces for formatting.
// This function must be called with c.mu held.
func (c *common) decorate(s string, skip int) string {
- if c.helperNames == nil {
- c.helperNames = make(map[string]struct{})
- for pc := range c.helperPCs {
- c.helperNames[pcToName(pc)] = struct{}{}
- }
- }
-
frame := c.frameSkip(skip)
file := frame.File
line := frame.Line
err := recover()
signal := true
- if err != nil && t.isFuzzing() {
- t.Errorf("panic: %s\n%s\n", err, string(debug.Stack()))
- t.mu.Lock()
- t.finished = true
- t.mu.Unlock()
- err = nil
- }
-
t.mu.RLock()
finished := t.finished
t.mu.RUnlock()
}
}
+ if err != nil && t.isFuzzing() {
+ prefix := "panic: "
+ if err == errNilPanicOrGoexit {
+ prefix = ""
+ }
+ t.Errorf("%s%s\n%s\n", prefix, err, string(debug.Stack()))
+ t.mu.Lock()
+ t.finished = true
+ t.mu.Unlock()
+ err = nil
+ }
+
// Use a deferred call to ensure that we report that the test is
// complete even if a cleanup function calls t.FailNow. See issue 41355.
didPanic := false