Skip to content

Commit

Permalink
[dev.fuzz] testing: convert seed corpus values where possible
Browse files Browse the repository at this point in the history
The types provided in f.Fuzz will be viewed as the
canonical types for fuzzing. If the type is different
for a seed corpus entry, then the testing package
will attempt to convert it. If it can't convert it,
f.Fuzz will fail.

Currently, this allows converting types that may result
in precision loss or a semantically different value.
For example, an int(-1) can be converted to uint even
though the value could be math.MaxUint64. There is a
TODO to consider improving this in the future.

Updates #45593

Change-Id: I2e752119662f46b68445d42b1ffa46dd30e9faea
Reviewed-on: https://go-review.googlesource.com/c/go/+/325702
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
  • Loading branch information
katiehockman committed Jun 15, 2021
1 parent 965b114 commit 413c125
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 10 deletions.
38 changes: 35 additions & 3 deletions src/cmd/go/testdata/script/test_fuzz.txt
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,23 @@ stdout FAIL
! stdout ^ok
stdout FAIL

# Test that converting compatible value from f.Add successful runs cleanly.
go test -run FuzzConvertType fuzz_add_test.go
stdout ^ok
! stdout FAIL

# Test that converting incompatible value from f.Add fails.
! go test -run FuzzConvertIncompatibleType fuzz_add_test.go
! stdout ^ok
stdout FAIL

# Test that converts value which would lose precision from f.Add.
# Consider making a test like this fail, as it may have unexpected
# consequences for the developer.
go test -v -run FuzzConvertLosePrecision fuzz_add_test.go
stdout ok
! stdout FAIL

# Test fatal with testdata seed corpus
! go test -run FuzzFail corpustesting/fuzz_testdata_corpus_test.go
! stdout ^ok
Expand Down Expand Up @@ -344,19 +361,34 @@ func FuzzNilPanic(f *testing.F) {
func FuzzUnsupported(f *testing.F) {
m := make(map[string]bool)
f.Add(m)
f.Fuzz(func(t *testing.T, b []byte) {})
f.Fuzz(func(*testing.T, []byte) {})
}

func FuzzAddDifferentNumber(f *testing.F) {
f.Add([]byte("a"))
f.Add([]byte("a"), []byte("b"))
f.Fuzz(func(t *testing.T, b []byte) {})
f.Fuzz(func(*testing.T, []byte) {})
}

func FuzzAddDifferentType(f *testing.F) {
f.Add(false)
f.Add(1234)
f.Fuzz(func(t *testing.T, b []byte) {})
f.Fuzz(func(*testing.T, []byte) {})
}

func FuzzConvertIncompatibleType(f *testing.F) {
f.Add("abcde")
f.Fuzz(func(*testing.T, int64) {})
}

func FuzzConvertLosePrecision(f *testing.F) {
f.Add(-1)
f.Fuzz(func(*testing.T, uint) {})
}

func FuzzConvertType(f *testing.F) {
f.Add(1, "hello")
f.Fuzz(func(*testing.T, uint, []byte) {})
}

-- corpustesting/fuzz_testdata_corpus_test.go --
Expand Down
45 changes: 41 additions & 4 deletions src/internal/fuzz/fuzz.go
Original file line number Diff line number Diff line change
Expand Up @@ -727,15 +727,52 @@ func readCorpusData(data []byte, types []reflect.Type) ([]interface{}, error) {
if err != nil {
return nil, fmt.Errorf("unmarshal: %v", err)
}
if err = CheckCorpus(vals, types); err != nil {
return nil, err
}
return vals, nil
}

// CheckCorpus verifies that the types in vals match the expected types
// provided. If not, attempt to convert them. If that's not possible, return an
// error.
func CheckCorpus(vals []interface{}, types []reflect.Type) error {
if len(vals) != len(types) {
return nil, fmt.Errorf("wrong number of values in corpus file: %d, want %d", len(vals), len(types))
return fmt.Errorf("wrong number of values in corpus file: %d, want %d", len(vals), len(types))
}
for i := range types {
if reflect.TypeOf(vals[i]) != types[i] {
return nil, fmt.Errorf("mismatched types in corpus file: %v, want %v", vals, types)
orig := reflect.ValueOf(vals[i])
origType := orig.Type()
wantType := types[i]
if origType == wantType {
continue // already the same type
}
// Attempt to convert the corpus value to the expected type
if !origType.ConvertibleTo(wantType) {
return fmt.Errorf("cannot convert %v to %v", origType, wantType)
}
convertedVal, ok := convertToType(orig, wantType)
if !ok {
return fmt.Errorf("error converting %v to %v", origType, wantType)
}
// TODO: Check that the value didn't change.
// e.g. val went from int64(-1) -> uint(0) -> int64(0) which should fail

// Updates vals to use the newly converted value of the expected type.
vals[i] = convertedVal.Interface()
}
return vals, nil
return nil
}

func convertToType(orig reflect.Value, t reflect.Type) (converted reflect.Value, ok bool) {
// Convert might panic even if ConvertibleTo returns true, so catch
// that panic and return false.
defer func() {
if r := recover(); r != nil {
ok = false
}
}()
return orig.Convert(t), true
}

// writeToCorpus atomically writes the given bytes to a new file in testdata.
Expand Down
14 changes: 13 additions & 1 deletion src/testing/fuzz.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func (f *F) TempDir() string {

// Add will add the arguments to the seed corpus for the fuzz target. This will
// be a no-op if called after or within the Fuzz function. The args must match
// those in the Fuzz function.
// or be convertible to those in the Fuzz function.
func (f *F) Add(args ...interface{}) {
var values []interface{}
for i := range args {
Expand Down Expand Up @@ -291,6 +291,15 @@ func (f *F) Fuzz(ff interface{}) {
types = append(types, t)
}

// Check the corpus provided by f.Add
for _, c := range f.corpus {
if err := f.fuzzContext.checkCorpus(c.Values, types); err != nil {
// TODO: Is there a way to save which line number is associated
// with the f.Add call that failed?
f.Fatal(err)
}
}

// Load seed corpus
c, err := f.fuzzContext.readCorpus(filepath.Join(corpusDir, f.name), types)
if err != nil {
Expand Down Expand Up @@ -470,6 +479,7 @@ type fuzzContext struct {
coordinateFuzzing func(time.Duration, int64, time.Duration, int64, int, []corpusEntry, []reflect.Type, string, string) error
runFuzzWorker func(func(corpusEntry) error) error
readCorpus func(string, []reflect.Type) ([]corpusEntry, error)
checkCorpus func(vals []interface{}, types []reflect.Type) error
resetCoverage func()
snapshotCoverage func()
}
Expand All @@ -487,6 +497,7 @@ func runFuzzTargets(deps testDeps, fuzzTargets []InternalFuzzTarget) (ran, ok bo
fctx := &fuzzContext{
importPath: deps.ImportPath,
readCorpus: deps.ReadCorpus,
checkCorpus: deps.CheckCorpus,
resetCoverage: deps.ResetCoverage,
snapshotCoverage: deps.SnapshotCoverage,
}
Expand Down Expand Up @@ -543,6 +554,7 @@ func runFuzzing(deps testDeps, fuzzTargets []InternalFuzzTarget) (ran, ok bool)
fctx := &fuzzContext{
importPath: deps.ImportPath,
readCorpus: deps.ReadCorpus,
checkCorpus: deps.CheckCorpus,
resetCoverage: deps.ResetCoverage,
snapshotCoverage: deps.SnapshotCoverage,
}
Expand Down
4 changes: 4 additions & 0 deletions src/testing/internal/testdeps/deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ func (TestDeps) ReadCorpus(dir string, types []reflect.Type) ([]fuzz.CorpusEntry
return fuzz.ReadCorpus(dir, types)
}

func (TestDeps) CheckCorpus(vals []interface{}, types []reflect.Type) error {
return fuzz.CheckCorpus(vals, types)
}

func (TestDeps) ResetCoverage() {
fuzz.ResetCoverage()
}
Expand Down
6 changes: 4 additions & 2 deletions src/testing/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -1463,8 +1463,9 @@ func (f matchStringOnly) RunFuzzWorker(func(corpusEntry) error) error { return e
func (f matchStringOnly) ReadCorpus(string, []reflect.Type) ([]corpusEntry, error) {
return nil, errMain
}
func (f matchStringOnly) ResetCoverage() {}
func (f matchStringOnly) SnapshotCoverage() {}
func (f matchStringOnly) CheckCorpus([]interface{}, []reflect.Type) error { return nil }
func (f matchStringOnly) ResetCoverage() {}
func (f matchStringOnly) SnapshotCoverage() {}

// 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.
Expand Down Expand Up @@ -1510,6 +1511,7 @@ type testDeps interface {
CoordinateFuzzing(time.Duration, int64, time.Duration, int64, int, []corpusEntry, []reflect.Type, string, string) error
RunFuzzWorker(func(corpusEntry) error) error
ReadCorpus(string, []reflect.Type) ([]corpusEntry, error)
CheckCorpus([]interface{}, []reflect.Type) error
ResetCoverage()
SnapshotCoverage()
}
Expand Down

0 comments on commit 413c125

Please sign in to comment.