From: Katie Hockman Date: Thu, 7 Oct 2021 20:26:36 +0000 (-0400) Subject: testing: fix -run behavior with fuzz tests X-Git-Tag: go1.18beta1~937 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=36a265a625a8320fea93aad62da4003b2cc54f72;p=gostls13.git testing: fix -run behavior with fuzz tests This change fixes some issues with -run, and the subsequent command line output when running in verbose mode. It replaces CorpusEntry.Name with CorpusEntry.Path, and refactors the code accordingly. This change also adds a lot of additional tests which check explicit command line output when fuzz targets are run without fuzzing. This will be important to avoid regressions. Updates #48149 Change-Id: If34b1f51db646317b7b51c3c38ae53231d01f568 Reviewed-on: https://go-review.googlesource.com/c/go/+/354632 Trust: Katie Hockman Run-TryBot: Katie Hockman TryBot-Result: Go Bot Reviewed-by: Jay Conrod --- 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 79476ecb28..628e003f41 100644 --- a/src/cmd/go/testdata/script/test_fuzz_mutate_crash.txt +++ b/src/cmd/go/testdata/script/test_fuzz_mutate_crash.txt @@ -21,7 +21,7 @@ go run check_testdata.go FuzzWithBug # Now, the failing bytes should have been added to the seed corpus for # the target, and should fail when run without fuzzing. ! go test -stdout 'testdata[/\\]fuzz[/\\]FuzzWithBug[/\\][a-f0-9]{64}' +stdout 'FuzzWithBug/[a-f0-9]{64}' stdout 'this input caused a crash!' ! go test -run=FuzzWithNilPanic -fuzz=FuzzWithNilPanic -fuzztime=100x -fuzzminimizetime=1000x diff --git a/src/cmd/go/testdata/script/test_fuzz_run.txt b/src/cmd/go/testdata/script/test_fuzz_run.txt new file mode 100644 index 0000000000..e546d997cb --- /dev/null +++ b/src/cmd/go/testdata/script/test_fuzz_run.txt @@ -0,0 +1,145 @@ +# TODO(jayconrod): support shared memory on more platforms. +[!darwin] [!linux] [!windows] skip + +[short] skip +env GOCACHE=$WORK/cache + +# Tests which verify the behavior and command line output when +# running a fuzz target as a unit test. + +# Tests without -run. + +! go test +stdout FAIL +stdout 'error here' + +! go test -v +stdout FAIL +stdout 'error here' +stdout '=== RUN FuzzFoo/thisfails' +stdout '--- FAIL: FuzzFoo/thisfails' +stdout '=== RUN FuzzFoo/thispasses' +stdout '--- PASS: FuzzFoo/thispasses' + +# Tests where -run matches all seed corpora. + +! go test -run FuzzFoo/this +stdout FAIL +stdout 'error here' +! stdout 'no tests to run' + +! go test -run /this +stdout FAIL +stdout 'error here' +! stdout 'no tests to run' + +! go test -v -run FuzzFoo/this +stdout FAIL +stdout 'error here' +stdout '=== RUN FuzzFoo/thisfails' +stdout '--- FAIL: FuzzFoo/thisfails' +stdout '=== RUN FuzzFoo/thispasses' +stdout '--- PASS: FuzzFoo/thispasses' +! stdout 'no tests to run' + +! go test -v -run /this +stdout FAIL +stdout 'error here' +stdout '=== RUN FuzzFoo/thisfails' +stdout '--- FAIL: FuzzFoo/thisfails' +stdout '=== RUN FuzzFoo/thispasses' +stdout '--- PASS: FuzzFoo/thispasses' +! stdout 'no tests to run' + +# Tests where -run only matches one seed corpus which passes. + +go test -run FuzzFoo/thispasses +stdout ok +! stdout 'no tests to run' + +go test -run /thispasses +stdout ok +! stdout 'no tests to run' + +# Same tests in verbose mode +go test -v -run FuzzFoo/thispasses +stdout '=== RUN FuzzFoo/thispasses' +stdout '--- PASS: FuzzFoo/thispasses' +! stdout '=== RUN FuzzFoo/thisfails' +! stdout 'no tests to run' + +go test -v -run /thispasses +stdout '=== RUN FuzzFoo/thispasses' +stdout '--- PASS: FuzzFoo/thispasses' +! stdout '=== RUN FuzzFoo/thisfails' +! stdout 'no tests to run' + +# Tests where -run only matches one seed corpus which fails. + +! go test -run FuzzFoo/thisfails +stdout FAIL +stdout 'error here' +! stdout 'no tests to run' + +! go test -run /thisfails +stdout FAIL +stdout 'error here' +! stdout 'no tests to run' + +! go test -v -run FuzzFoo/thisfails +stdout 'error here' +stdout '=== RUN FuzzFoo/thisfails' +stdout '--- FAIL: FuzzFoo/thisfails' +! stdout '=== RUN FuzzFoo/thispasses' +! stdout 'no tests to run' + +! go test -v -run /thisfails +stdout 'error here' +stdout '=== RUN FuzzFoo/thisfails' +stdout '--- FAIL: FuzzFoo/thisfails' +! stdout '=== RUN FuzzFoo/thispasses' +! stdout 'no tests to run' + +# Tests where -run doesn't match any seed corpora. + +go test -run FuzzFoo/nomatch +stdout ok + +go test -run /nomatch +stdout ok + +go test -v -run FuzzFoo/nomatch +stdout '=== RUN FuzzFoo' +stdout '--- PASS: FuzzFoo' +stdout ok +! stdout 'no tests to run' + +go test -v -run /nomatch +stdout '=== RUN FuzzFoo' +stdout '--- PASS: FuzzFoo' +stdout ok +! stdout 'no tests to run' + +-- go.mod -- +module example.com/x + +go 1.16 +-- x_test.go -- +package x + +import "testing" + +func FuzzFoo(f *testing.F) { + f.Add("this is fine") + f.Fuzz(func(t *testing.T, s string) { + if s == "fails" { + t.Error("error here") + } + }) +} +-- testdata/fuzz/FuzzFoo/thisfails -- +go test fuzz v1 +string("fails") +-- testdata/fuzz/FuzzFoo/thispasses -- +go test fuzz v1 +string("passes") \ No newline at end of file diff --git a/src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt b/src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt index a66d0554b6..5d04d8c022 100644 --- a/src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt +++ b/src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt @@ -26,6 +26,7 @@ stdout ok ! go test -fuzz=FuzzWithTestdata -run=FuzzWithTestdata -fuzztime=1x ! stdout ^ok ! stdout 'Crash written to testdata[/\\]fuzz[/\\]FuzzWithTestdata[/\\]' +stdout 'found a crash while testing seed corpus entry: FuzzWithTestdata/1' stdout FAIL # Test that fuzzing a target with no seed corpus or cache finds a crash, prints diff --git a/src/internal/fuzz/fuzz.go b/src/internal/fuzz/fuzz.go index f660052911..a8bbd60b1c 100644 --- a/src/internal/fuzz/fuzz.go +++ b/src/internal/fuzz/fuzz.go @@ -141,14 +141,14 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err if c.crashMinimizing == nil || crashWritten { return } - fileName, werr := writeToCorpus(c.crashMinimizing.entry.Data, opts.CorpusDir) + werr := writeToCorpus(&c.crashMinimizing.entry, opts.CorpusDir) if werr != nil { err = fmt.Errorf("%w\n%v", err, werr) return } if err == nil { err = &crashError{ - name: filepath.Base(fileName), + path: c.crashMinimizing.entry.Path, err: errors.New(c.crashMinimizing.crasherMsg), } } @@ -230,7 +230,8 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err if result.crasherMsg != "" { if c.warmupRun() && result.entry.IsSeed { - fmt.Fprintf(c.opts.Log, "found a crash while testing seed corpus entry: %q\n", result.entry.Parent) + target := filepath.Base(c.opts.CorpusDir) + fmt.Fprintf(c.opts.Log, "found a crash while testing seed corpus entry: %s/%s\n", target, testName(result.entry.Parent)) stop(errors.New(result.crasherMsg)) break } @@ -249,11 +250,11 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err } else if !crashWritten { // Found a crasher that's either minimized or not minimizable. // Write to corpus and stop. - fileName, err := writeToCorpus(result.entry.Data, opts.CorpusDir) + err := writeToCorpus(&result.entry, opts.CorpusDir) if err == nil { crashWritten = true err = &crashError{ - name: filepath.Base(fileName), + path: result.entry.Path, err: errors.New(result.crasherMsg), } } @@ -262,7 +263,7 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err c.opts.Log, "DEBUG new crasher, elapsed: %s, id: %s, parent: %s, gen: %d, size: %d, exec time: %s\n", c.elapsed(), - fileName, + result.entry.Path, result.entry.Parent, result.entry.Generation, len(result.entry.Data), @@ -315,12 +316,11 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err // Update the coordinator's coverage mask and save the value. inputSize := len(result.entry.Data) if opts.CacheDir != "" { - filename, err := writeToCorpus(result.entry.Data, opts.CacheDir) + err := writeToCorpus(&result.entry, opts.CacheDir) if err != nil { stop(err) } result.entry.Data = nil - result.entry.Name = filename } c.updateCoverage(keepCoverage) c.corpus.entries = append(c.corpus.entries, result.entry) @@ -331,7 +331,7 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err c.opts.Log, "DEBUG new interesting input, elapsed: %s, id: %s, parent: %s, gen: %d, new bits: %d, total bits: %d, size: %d, exec time: %s\n", c.elapsed(), - result.entry.Name, + result.entry.Path, result.entry.Parent, result.entry.Generation, countBits(keepCoverage), @@ -347,7 +347,7 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err c.opts.Log, "DEBUG worker reported interesting input that doesn't expand coverage, elapsed: %s, id: %s, parent: %s, canMinimize: %t\n", c.elapsed(), - result.entry.Name, + result.entry.Path, result.entry.Parent, result.canMinimize, ) @@ -397,7 +397,7 @@ func CoordinateFuzzing(ctx context.Context, opts CoordinateFuzzingOpts) (err err // of the file where the input causing the crasher was saved. The testing // framework uses this to report a command to re-run that specific input. type crashError struct { - name string + path string err error } @@ -409,8 +409,8 @@ func (e *crashError) Unwrap() error { return e.err } -func (e *crashError) CrashName() string { - return e.name +func (e *crashError) CrashPath() string { + return e.path } type corpus struct { @@ -426,14 +426,14 @@ type corpus struct { type CorpusEntry = struct { Parent string - // Name is the name of the corpus file, if the entry was loaded from the - // seed corpus. It can be used with -run. For entries added with f.Add and - // entries generated by the mutator, Name is empty and Data is populated. - Name string + // Path is the path of the corpus file, if the entry was loaded from disk. + // For other entries, including seed values provided by f.Add, Path is the + // name of the test, e.g. seed#0 or its hash. + Path string - // Data is the raw input data. Data should only be populated for initial - // seed values added with f.Add. For on-disk corpus files, Data will - // be nil. + // Data is the raw input data. Data should only be populated for seed + // values. For on-disk corpus files, Data will be nil, as it will be loaded + // from disk using Path. Data []byte // Values is the unmarshaled values from a corpus file. @@ -452,7 +452,7 @@ func CorpusEntryData(ce CorpusEntry) ([]byte, error) { return ce.Data, nil } - return os.ReadFile(ce.Name) + return os.ReadFile(ce.Path) } type fuzzInput struct { @@ -616,7 +616,7 @@ type coordinator struct { } func newCoordinator(opts CoordinateFuzzingOpts) (*coordinator, error) { - // Make sure all of the seed corpus given by f.Add has marshalled data. + // Make sure all of the seed corpus has marshalled data. for i := range opts.Seed { if opts.Seed[i].Data == nil && opts.Seed[i].Values != nil { opts.Seed[i].Data = marshalCorpusFile(opts.Seed[i].Values...) @@ -673,7 +673,7 @@ func newCoordinator(opts CoordinateFuzzingOpts) (*coordinator, error) { data := marshalCorpusFile(vals...) h := sha256.Sum256(data) name := fmt.Sprintf("%x", h[:4]) - c.corpus.entries = append(c.corpus.entries, CorpusEntry{Name: name, Data: data}) + c.corpus.entries = append(c.corpus.entries, CorpusEntry{Path: name, Data: data}) } return c, nil @@ -956,7 +956,7 @@ func ReadCorpus(dir string, types []reflect.Type) ([]CorpusEntry, error) { errs = append(errs, fmt.Errorf("%q: %v", filename, err)) continue } - corpus = append(corpus, CorpusEntry{Name: filename, Values: vals}) + corpus = append(corpus, CorpusEntry{Path: filename, Values: vals}) } if len(errs) > 0 { return corpus, &MalformedCorpusError{errs: errs} @@ -979,7 +979,7 @@ func readCorpusData(data []byte, types []reflect.Type) ([]interface{}, error) { // provided. func CheckCorpus(vals []interface{}, types []reflect.Type) error { if len(vals) != len(types) { - return fmt.Errorf("wrong number of values in corpus entry: %d, want %d", len(vals), len(types)) + return fmt.Errorf("wrong number of values in corpus entry %v: want %v", vals, types) } for i := range types { if reflect.TypeOf(vals[i]) != types[i] { @@ -989,21 +989,25 @@ func CheckCorpus(vals []interface{}, types []reflect.Type) error { return nil } -// writeToCorpus atomically writes the given bytes to a new file in testdata. -// If the directory does not exist, it will create one. If the file already -// exists, writeToCorpus will not rewrite it. writeToCorpus returns the -// file's name, or an error if it failed. -func writeToCorpus(b []byte, dir string) (name string, err error) { - sum := fmt.Sprintf("%x", sha256.Sum256(b)) - name = filepath.Join(dir, sum) +// writeToCorpus atomically writes the given bytes to a new file in testdata. If +// the directory does not exist, it will create one. If the file already exists, +// writeToCorpus will not rewrite it. writeToCorpus sets entry.Path to the new +// file that was just written or an error if it failed. +func writeToCorpus(entry *CorpusEntry, dir string) (err error) { + sum := fmt.Sprintf("%x", sha256.Sum256(entry.Data)) + entry.Path = filepath.Join(dir, sum) if err := os.MkdirAll(dir, 0777); err != nil { - return "", err + return err } - if err := ioutil.WriteFile(name, b, 0666); err != nil { - os.Remove(name) // remove partially written file - return "", err + if err := ioutil.WriteFile(entry.Path, entry.Data, 0666); err != nil { + os.Remove(entry.Path) // remove partially written file + return err } - return name, nil + return nil +} + +func testName(path string) string { + return filepath.Base(path) } func zeroValue(t reflect.Type) interface{} { diff --git a/src/internal/fuzz/worker.go b/src/internal/fuzz/worker.go index 1429decba8..0c428ed832 100644 --- a/src/internal/fuzz/worker.go +++ b/src/internal/fuzz/worker.go @@ -1029,7 +1029,7 @@ func (wc *workerClient) minimize(ctx context.Context, entryIn CorpusEntry, args entryOut.Values, err = unmarshalCorpusFile(entryOut.Data) h := sha256.Sum256(entryOut.Data) name := fmt.Sprintf("%x", h[:4]) - entryOut.Name = name + entryOut.Path = name entryOut.Parent = entryIn.Parent entryOut.Generation = entryIn.Generation if err != nil { @@ -1092,8 +1092,8 @@ func (wc *workerClient) fuzz(ctx context.Context, entryIn CorpusEntry, args fuzz h := sha256.Sum256(dataOut) name := fmt.Sprintf("%x", h[:4]) entryOut = CorpusEntry{ - Name: name, - Parent: entryIn.Name, + Parent: entryIn.Path, + Path: name, Data: dataOut, Generation: entryIn.Generation + 1, } diff --git a/src/testing/fuzz.go b/src/testing/fuzz.go index 60e2603da9..40b77c1331 100644 --- a/src/testing/fuzz.go +++ b/src/testing/fuzz.go @@ -80,7 +80,7 @@ var _ TB = (*F)(nil) // import internal/fuzz from testing. type corpusEntry = struct { Parent string - Name string + Path string Data []byte Values []interface{} Generation int @@ -259,7 +259,7 @@ func (f *F) Add(args ...interface{}) { } values = append(values, args[i]) } - f.corpus = append(f.corpus, corpusEntry{Values: values, IsSeed: true, Name: fmt.Sprintf("seed#%d", len(f.corpus))}) + f.corpus = append(f.corpus, corpusEntry{Values: values, IsSeed: true, Path: fmt.Sprintf("seed#%d", len(f.corpus))}) } // supportedTypes represents all of the supported types which can be fuzzed. @@ -369,16 +369,16 @@ func (f *F) Fuzz(ff interface{}) { // fn is called in its own goroutine. run := func(e corpusEntry) error { if e.Values == nil { - // Every code path should have already unmarshaled Data into Values. - // It's our fault if it didn't. - panic(fmt.Sprintf("corpus file %q was not unmarshaled", e.Name)) + // The corpusEntry must have non-nil Values in order to run the + // test. If Values is nil, it is a bug in our code. + panic(fmt.Sprintf("corpus file %q was not unmarshaled", e.Path)) } if shouldFailFast() { return nil } - testName := f.common.name - if e.Name != "" { - testName = fmt.Sprintf("%s/%s", testName, e.Name) + testName := f.name + if e.Path != "" { + testName = fmt.Sprintf("%s/%s", testName, filepath.Base(e.Path)) } // Record the stack trace at the point of this call so that if the subtest @@ -448,9 +448,10 @@ func (f *F) Fuzz(ff interface{}) { f.Fail() fmt.Fprintf(f.w, "%v\n", err) if crashErr, ok := err.(fuzzCrashError); ok { - crashName := crashErr.CrashName() - fmt.Fprintf(f.w, "Crash written to %s\n", filepath.Join(corpusDir, f.name, crashName)) - fmt.Fprintf(f.w, "To re-run:\ngo test %s -run=%s/%s\n", f.fuzzContext.deps.ImportPath(), f.name, crashName) + crashPath := crashErr.CrashPath() + fmt.Fprintf(f.w, "Crash written to %s\n", crashPath) + testName := filepath.Base(crashPath) + fmt.Fprintf(f.w, "To re-run:\ngo test %s -run=%s/%s\n", f.fuzzContext.deps.ImportPath(), f.name, testName) } } // TODO(jayconrod,katiehockman): Aggregate statistics across workers @@ -470,7 +471,10 @@ func (f *F) Fuzz(ff interface{}) { // Fuzzing is not enabled, or will be done later. Only run the seed // corpus now. for _, e := range f.corpus { - run(e) + name := fmt.Sprintf("%s/%s", f.name, filepath.Base(e.Path)) + if _, ok, _ := f.testContext.match.fullName(nil, name); ok { + run(e) + } } } } @@ -516,11 +520,11 @@ type fuzzCrashError interface { error Unwrap() error - // CrashName returns the name of the subtest that corresponds to the saved - // crash input file in the seed corpus. The test can be re-run with - // go test $pkg -run=$target/$name where $pkg is the package's import path, - // $target is the fuzz target name, and $name is the string returned here. - CrashName() string + // CrashPath returns the path of the subtest that corresponds to the saved + // crash input file in the seed corpus. The test can be re-run with go test + // -run=$target/$name $target is the fuzz target name, and $name is the + // filepath.Base of the string returned here. + CrashPath() string } // fuzzContext holds fields common to all fuzz targets.