[dev.fuzz] internal/fuzz: refactor CorpusEntry type

CorpusEntry is now a struct type with Name and Data fields. In the
future, it may have more fields describing multiple values with
different types added with f.Add.

CorpusEntry must be the same type in testing and
internal/fuzz. However, we don't want to export it from testing, and
testing can't import internal/fuzz. We define it to be a type alias of
a struct type instead of a defined type. We need to define it to the
same thing in both places. We'll get a type error when building cmd/go
if there's a difference.

Change-Id: I9df6cd7aed67a6aa48b77ffb3a84bd302d2e5d94
Reviewed-on: https://go-review.googlesource.com/c/go/+/288534
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
This commit is contained in:
Jay Conrod 2021-02-01 18:00:37 -05:00
parent 671dba6c89
commit 5ef7357b50
5 changed files with 59 additions and 57 deletions

View File

@ -39,7 +39,7 @@ import (
//
// If a crash occurs, the function will return an error containing information
// about the crash, which can be reported to the user.
func CoordinateFuzzing(ctx context.Context, parallel int, seed [][]byte, corpusDir, cacheDir string) (err error) {
func CoordinateFuzzing(ctx context.Context, parallel int, seed []CorpusEntry, corpusDir, cacheDir string) (err error) {
if err := ctx.Err(); err != nil {
return err
}
@ -53,7 +53,7 @@ func CoordinateFuzzing(ctx context.Context, parallel int, seed [][]byte, corpusD
return err
}
if len(corpus.entries) == 0 {
corpus.entries = []corpusEntry{{b: []byte{}}}
corpus.entries = []CorpusEntry{{Data: []byte{}}}
}
// TODO(jayconrod): do we want to support fuzzing different binaries?
@ -64,8 +64,8 @@ func CoordinateFuzzing(ctx context.Context, parallel int, seed [][]byte, corpusD
c := &coordinator{
doneC: make(chan struct{}),
inputC: make(chan corpusEntry),
interestingC: make(chan corpusEntry),
inputC: make(chan CorpusEntry),
interestingC: make(chan CorpusEntry),
crasherC: make(chan crasherEntry),
errC: make(chan error),
}
@ -141,7 +141,7 @@ func CoordinateFuzzing(ctx context.Context, parallel int, seed [][]byte, corpusD
case crasher := <-c.crasherC:
// A worker found a crasher. Write it to testdata and return it.
fileName, err := writeToCorpus(crasher.b, corpusDir)
fileName, err := writeToCorpus(crasher.Data, corpusDir)
if err == nil {
err = fmt.Errorf(" Crash written to %s\n%s", fileName, crasher.errMsg)
}
@ -160,7 +160,7 @@ func CoordinateFuzzing(ctx context.Context, parallel int, seed [][]byte, corpusD
// in the corpus.
corpus.entries = append(corpus.entries, entry)
if cacheDir != "" {
if _, err := writeToCorpus(entry.b, cacheDir); err != nil {
if _, err := writeToCorpus(entry.Data, cacheDir); err != nil {
return err
}
}
@ -182,17 +182,32 @@ func CoordinateFuzzing(ctx context.Context, parallel int, seed [][]byte, corpusD
}
type corpus struct {
entries []corpusEntry
entries []CorpusEntry
}
// TODO(jayconrod,katiehockman): decide whether and how to unify this type
// with the equivalent in testing.
type corpusEntry struct {
b []byte
// CorpusEntry represents an individual input for fuzzing.
//
// We must use an equivalent type in the testing and testing/internal/testdeps
// packages, but testing can't import this package directly, and we don't want
// to export this type from testing. Instead, we use the same struct type and
// use a type alias (not a defined type) for convenience.
type CorpusEntry = struct {
// 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.
Name string
// Data is the raw data loaded from a corpus file.
Data []byte
// TODO(jayconrod,katiehockman): support multiple values of different types
// added with f.Add with a Values []interface{} field. We'll need marhsalling
// and unmarshalling functions, and we'll need to figure out what to do
// in the mutator.
}
type crasherEntry struct {
corpusEntry
CorpusEntry
errMsg string
}
@ -206,12 +221,12 @@ type coordinator struct {
// inputC is sent values to fuzz by the coordinator. Any worker may receive
// values from this channel.
inputC chan corpusEntry
inputC chan CorpusEntry
// interestingC is sent interesting values by the worker, which is received
// by the coordinator. Values are usually interesting because they
// increase coverage.
interestingC chan corpusEntry
interestingC chan CorpusEntry
// crasherC is sent values that crashed the code being fuzzed. These values
// should be saved in the corpus, and we may want to stop fuzzing after
@ -231,33 +246,29 @@ type coordinator struct {
// the same package at a different version or in a different module.
// TODO(jayconrod,katiehockman): need a mechanism that can remove values that
// aren't useful anymore, for example, because they have the wrong type.
func readCorpusAndCache(seed [][]byte, corpusDir, cacheDir string) (corpus, error) {
func readCorpusAndCache(seed []CorpusEntry, corpusDir, cacheDir string) (corpus, error) {
var c corpus
for _, b := range seed {
c.entries = append(c.entries, corpusEntry{b: b})
}
c.entries = append(c.entries, seed...)
for _, dir := range []string{corpusDir, cacheDir} {
bs, err := ReadCorpus(dir)
entries, err := ReadCorpus(dir)
if err != nil {
return corpus{}, err
}
for _, b := range bs {
c.entries = append(c.entries, corpusEntry{b: b})
}
c.entries = append(c.entries, entries...)
}
return c, nil
}
// ReadCorpus reads the corpus from the testdata directory in this target's
// package.
func ReadCorpus(dir string) ([][]byte, error) {
func ReadCorpus(dir string) ([]CorpusEntry, error) {
files, err := ioutil.ReadDir(dir)
if os.IsNotExist(err) {
return nil, nil // No corpus to read
} else if err != nil {
return nil, fmt.Errorf("testing: reading seed corpus from testdata: %v", err)
}
var corpus [][]byte
var corpus []CorpusEntry
for _, file := range files {
// TODO(jayconrod,katiehockman): determine when a file is a fuzzing input
// based on its name. We should only read files created by writeToCorpus.
@ -271,7 +282,7 @@ func ReadCorpus(dir string) ([][]byte, error) {
if err != nil {
return nil, fmt.Errorf("testing: failed to read corpus file: %v", err)
}
corpus = append(corpus, bytes)
corpus = append(corpus, CorpusEntry{Name: file.Name(), Data: bytes})
}
return corpus, nil
}

View File

@ -104,7 +104,7 @@ func (w *worker) runFuzzing() error {
w.memMu <- mem
message := fmt.Sprintf("fuzzing process terminated unexpectedly: %v", w.waitErr)
crasher := crasherEntry{
corpusEntry: corpusEntry{b: value},
CorpusEntry: CorpusEntry{Data: value},
errMsg: message,
}
w.coordinator.crasherC <- crasher
@ -119,7 +119,7 @@ func (w *worker) runFuzzing() error {
inputC = nil // block new inputs until we finish with this one.
go func() {
args := fuzzArgs{Duration: workerFuzzDuration}
value, resp, err := w.client.fuzz(input.b, args)
value, resp, err := w.client.fuzz(input.Data, args)
if err != nil {
// Error communicating with worker.
select {
@ -144,7 +144,7 @@ func (w *worker) runFuzzing() error {
} else if resp.Err != "" {
// The worker found a crasher. Inform the coordinator.
crasher := crasherEntry{
corpusEntry: corpusEntry{b: value},
CorpusEntry: CorpusEntry{Data: value},
errMsg: resp.Err,
}
w.coordinator.crasherC <- crasher
@ -152,7 +152,7 @@ func (w *worker) runFuzzing() error {
// Inform the coordinator that fuzzing found something
// interesting (i.e. new coverage).
if resp.Interesting {
w.coordinator.interestingC <- corpusEntry{b: value}
w.coordinator.interestingC <- CorpusEntry{Data: value}
}
// Continue fuzzing.

View File

@ -48,17 +48,12 @@ type F struct {
fuzzFunc func(f *F) // fuzzFunc is the function which makes up the fuzz target
}
// corpus corpusEntry
type corpusEntry struct {
b []byte
}
func bytesToCorpus(bytes [][]byte) []corpusEntry {
c := make([]corpusEntry, len(bytes))
for i, b := range bytes {
c[i].b = b
}
return c
// corpusEntry is an alias to the same type as internal/fuzz.CorpusEntry.
// We use a type alias because we don't want to export this type, and we can't
// importing internal/fuzz from testing.
type corpusEntry = struct {
Name string
Data []byte
}
// Add will add the arguments to the seed corpus for the fuzz target. This will
@ -74,7 +69,7 @@ func (f *F) Add(args ...interface{}) {
}
switch v := args[0].(type) {
case []byte:
f.corpus = append(f.corpus, corpusEntry{v})
f.corpus = append(f.corpus, corpusEntry{Data: v})
// TODO: support other types
default:
panic("testing: Add only supports []byte currently")
@ -100,7 +95,7 @@ func (f *F) Fuzz(ff interface{}) {
if err != nil {
f.Fatal(err)
}
f.corpus = append(f.corpus, bytesToCorpus(c)...)
f.corpus = append(f.corpus, c...)
// TODO(jayconrod,katiehockman): dedupe testdata corpus with entries from f.Add
var errStr string
@ -132,13 +127,9 @@ func (f *F) Fuzz(ff interface{}) {
// Fuzzing is enabled, and this is the test process started by 'go test'.
// Act as the coordinator process, and coordinate workers to perform the
// actual fuzzing.
seed := make([][]byte, len(f.corpus))
for i, e := range f.corpus {
seed[i] = e.b
}
corpusTargetDir := filepath.Join(corpusDir, f.name)
cacheTargetDir := filepath.Join(*fuzzCacheDir, f.name)
err := f.context.coordinateFuzzing(*fuzzDuration, *parallel, seed, corpusTargetDir, cacheTargetDir)
err := f.context.coordinateFuzzing(*fuzzDuration, *parallel, f.corpus, corpusTargetDir, cacheTargetDir)
if err != nil {
f.Fail()
f.result = FuzzResult{Error: err}
@ -188,7 +179,7 @@ func (f *F) Fuzz(ff interface{}) {
},
context: newTestContext(1, nil),
}
go run(t, c.b)
go run(t, c.Data)
<-t.signal
if t.Failed() {
f.Fail()
@ -281,9 +272,9 @@ func (r FuzzResult) String() string {
type fuzzContext struct {
runMatch *matcher
fuzzMatch *matcher
coordinateFuzzing func(time.Duration, int, [][]byte, string, string) error
coordinateFuzzing func(time.Duration, int, []corpusEntry, string, string) error
runFuzzWorker func(func([]byte) error) error
readCorpus func(string) ([][]byte, error)
readCorpus func(string) ([]corpusEntry, error)
}
// runFuzzTargets runs the fuzz targets matching the pattern for -run. This will

View File

@ -132,7 +132,7 @@ func (TestDeps) SetPanicOnExit0(v bool) {
testlog.SetPanicOnExit0(v)
}
func (TestDeps) CoordinateFuzzing(timeout time.Duration, parallel int, seed [][]byte, corpusDir, cacheDir string) error {
func (TestDeps) CoordinateFuzzing(timeout time.Duration, parallel int, seed []fuzz.CorpusEntry, corpusDir, cacheDir string) error {
// Fuzzing may be interrupted with a timeout or if the user presses ^C.
// In either case, we'll stop worker processes gracefully and save
// crashers and interesting values.
@ -178,6 +178,6 @@ func (TestDeps) RunFuzzWorker(fn func([]byte) error) error {
return nil
}
func (TestDeps) ReadCorpus(dir string) ([][]byte, error) {
func (TestDeps) ReadCorpus(dir string) ([]fuzz.CorpusEntry, error) {
return fuzz.ReadCorpus(dir)
}

View File

@ -1361,11 +1361,11 @@ func (f matchStringOnly) ImportPath() string { return "
func (f matchStringOnly) StartTestLog(io.Writer) {}
func (f matchStringOnly) StopTestLog() error { return errMain }
func (f matchStringOnly) SetPanicOnExit0(bool) {}
func (f matchStringOnly) CoordinateFuzzing(time.Duration, int, [][]byte, string, string) error {
func (f matchStringOnly) CoordinateFuzzing(time.Duration, int, []corpusEntry, string, string) error {
return errMain
}
func (f matchStringOnly) RunFuzzWorker(func([]byte) error) error { return errMain }
func (f matchStringOnly) ReadCorpus(string) ([][]byte, error) { return nil, errMain }
func (f matchStringOnly) ReadCorpus(string) ([]corpusEntry, error) { return nil, errMain }
// Main is an internal function, part of the implementation of the "go test" command.
// It was exported because it is cross-package and predates "internal" packages.
@ -1408,9 +1408,9 @@ type testDeps interface {
StartTestLog(io.Writer)
StopTestLog() error
WriteProfileTo(string, io.Writer, int) error
CoordinateFuzzing(time.Duration, int, [][]byte, string, string) error
CoordinateFuzzing(time.Duration, int, []corpusEntry, string, string) error
RunFuzzWorker(func([]byte) error) error
ReadCorpus(string) ([][]byte, error)
ReadCorpus(string) ([]corpusEntry, error)
}
// MainStart is meant for use by tests generated by 'go test'.