From 24806f2a30c7b8c75986030e30ea91c4c1e51ce9 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Tue, 8 Mar 2022 16:10:30 -0700 Subject: [PATCH] go/analysis: add tests check for calling *F methods in fuzz func The only *F methods that are allowed to be called from the function passed to (*F).Fuzz are Named and Failed. This check looks for other methods and will report errors. Change-Id: I30ec546d11f9e9b38ee673c5a0e7cf0fde8ca922 Reviewed-on: https://go-review.googlesource.com/c/tools/+/390974 Trust: Suzy Mueller Run-TryBot: Suzy Mueller Reviewed-by: Hyang-Ah Hana Kim gopls-CI: kokoro TryBot-Result: Gopher Robot --- .../passes/tests/testdata/src/a/go118_test.go | 17 ++++++++----- go/analysis/passes/tests/tests.go | 24 ++++++++++++++++--- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/go/analysis/passes/tests/testdata/src/a/go118_test.go b/go/analysis/passes/tests/testdata/src/a/go118_test.go index c00e71d0d7c..dc898daca0b 100644 --- a/go/analysis/passes/tests/testdata/src/a/go118_test.go +++ b/go/analysis/passes/tests/testdata/src/a/go118_test.go @@ -20,12 +20,17 @@ func FuzzFunc(f *testing.F) { } func FuzzFuncWithArgs(f *testing.F) { - f.Add() // want `wrong number of values in call to \(\*testing.F\)\.Add: 0, fuzz target expects 2` - f.Add(1, 2, 3, 4) // want `wrong number of values in call to \(\*testing.F\)\.Add: 4, fuzz target expects 2` - f.Add(5, 5) // want `mismatched type in call to \(\*testing.F\)\.Add: int, fuzz target expects \[\]byte` - f.Add([]byte("hello"), 5) // want `mismatched types in call to \(\*testing.F\)\.Add: \[\[\]byte int\], fuzz target expects \[int \[\]byte\]` - f.Add(5, []byte("hello")) // OK - f.Fuzz(func(t *testing.T, i int, b []byte) {}) // OK "arguments in func are allowed" + f.Add() // want `wrong number of values in call to \(\*testing.F\)\.Add: 0, fuzz target expects 2` + f.Add(1, 2, 3, 4) // want `wrong number of values in call to \(\*testing.F\)\.Add: 4, fuzz target expects 2` + f.Add(5, 5) // want `mismatched type in call to \(\*testing.F\)\.Add: int, fuzz target expects \[\]byte` + f.Add([]byte("hello"), 5) // want `mismatched types in call to \(\*testing.F\)\.Add: \[\[\]byte int\], fuzz target expects \[int \[\]byte\]` + f.Add(5, []byte("hello")) // OK + f.Fuzz(func(t *testing.T, i int, b []byte) { // OK "arguments in func are allowed" + f.Add(5, []byte("hello")) // want `fuzz target must not call any \*F methods` + f.Name() // OK "calls to (*F).Failed and (*F).Name are allowed" + f.Failed() // OK "calls to (*F).Failed and (*F).Name are allowed" + f.Fuzz(func(t *testing.T) {}) // want `fuzz target must not call any \*F methods` + }) } func FuzzArgFunc(f *testing.F) { diff --git a/go/analysis/passes/tests/tests.go b/go/analysis/passes/tests/tests.go index bfa55d27502..ffa5205dd77 100644 --- a/go/analysis/passes/tests/tests.go +++ b/go/analysis/passes/tests/tests.go @@ -99,6 +99,8 @@ func checkFuzz(pass *analysis.Pass, fn *ast.FuncDecl) { // 4. Second argument onwards should be of type []byte, string, bool, byte, // rune, float32, float64, int, int8, int16, int32, int64, uint, uint8, uint16, // uint32, uint64 +// 5. func() must not call any *F methods, e.g. (*F).Log, (*F).Error, (*F).Skip +// The only *F methods that are allowed in the (*F).Fuzz function are (*F).Failed and (*F).Name. // Returns the list of parameters to the fuzz function, if they are valid fuzz parameters. func checkFuzzCall(pass *analysis.Pass, fn *ast.FuncDecl) (params *types.Tuple) { ast.Inspect(fn, func(n ast.Node) bool { @@ -121,7 +123,7 @@ func checkFuzzCall(pass *analysis.Pass, fn *ast.FuncDecl) (params *types.Tuple) // Argument should be a function if !argOk { pass.ReportRangef(expr, "argument to Fuzz must be a function") - return true + return false } // ff Argument function should not return if tSign.Results().Len() != 0 { @@ -130,12 +132,28 @@ func checkFuzzCall(pass *analysis.Pass, fn *ast.FuncDecl) (params *types.Tuple) // ff Argument function should have 1 or more argument if tSign.Params().Len() == 0 { pass.ReportRangef(expr, "fuzz target must have 1 or more argument") - return true + return false } ok := validateFuzzArgs(pass, tSign.Params(), expr) if ok && params == nil { params = tSign.Params() } + // Inspect the function that was passed as an argument to make sure that + // there are no calls to *F methods, except for Name and Failed. + ast.Inspect(expr, func(n ast.Node) bool { + if call, ok := n.(*ast.CallExpr); ok { + if !isFuzzTargetDot(pass, call, "") { + return true + } + if !isFuzzTargetDot(pass, call, "Name") && !isFuzzTargetDot(pass, call, "Failed") { + pass.ReportRangef(call, "fuzz target must not call any *F methods") + } + } + return true + }) + // We do not need to look at any calls to f.Fuzz inside of a Fuzz call, + // since they are not allowed. + return false } return true }) @@ -202,7 +220,7 @@ func isFuzzTargetDot(pass *analysis.Pass, call *ast.CallExpr, name string) bool if !isTestingType(pass.TypesInfo.Types[selExpr.X].Type, "F") { return false } - if selExpr.Sel.Name == name { + if name == "" || selExpr.Sel.Name == name { return true } }