Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/vet: checks for malformed fuzz targets #46218

Open
katiehockman opened this issue May 18, 2021 · 5 comments
Open

cmd/vet: checks for malformed fuzz targets #46218

katiehockman opened this issue May 18, 2021 · 5 comments

Comments

@katiehockman
Copy link
Member

@katiehockman katiehockman commented May 18, 2021

There are several things that vet can do to support native fuzzing. This issue tracks all of the potential checks that could be added.

Vet should fail if...

  • the inputs to any f.Add call don't match the ones in f.Fuzz
  • there is any code after f.Fuzz
  • the f.Fuzz function is missing a *testing.T as its first parameter
  • there is any call to a *testing.F method from within the f.Fuzz function
@timothy-king
Copy link
Contributor

@timothy-king timothy-king commented May 21, 2021

Maybe a dumb question, but can "the f.Fuzz function is missing a *testing.T as its first parameter" be enforced by the typechecker/compiler instead?

Loading

@zpavlinovic
Copy link
Contributor

@zpavlinovic zpavlinovic commented May 25, 2021

There are several things that vet can do to support native fuzzing. This issue tracks all of the potential checks that could be added.

Vet should fail if...

  • the inputs to any f.Add call don't match the ones in f.Fuzz

Could you perhaps elaborate on this one? Do you have a specific pattern in mind?

Loading

@katiehockman
Copy link
Member Author

@katiehockman katiehockman commented May 25, 2021

Maybe a dumb question, but can "the f.Fuzz function is missing a *testing.T as its first parameter" be enforced by the typechecker/compiler instead?

@timothy-king Not a dumb question! I don't have an answer to this question though. Possibly? The use of generics might make things easier here as well, but I haven't looked too deeply into this yet.
However, in general, the more issues like this that we can detect at compile-time the better.

Could you perhaps elaborate on this one? Do you have a specific pattern in mind?

@zpavlinovic Yes, I'm thinking of something like this:

func FuzzFoo(f *testing.F) {
  f.Add("some string")
  f.Fuzz(func(*testing.T, int) { })
}

In the case above, f.Add is attempting to add a value to the corpus which is a single string. However, the f.Fuzz function accepts an int as its type. It would be nice if this can fail at compile time, but a vet check would be helpful if that's not feasible. That code should be corrected to something like this:

func FuzzFoo(f *testing.F) {
  f.Add(50)
  f.Fuzz(func(*testing.T, int) { })
}

or

func FuzzFoo(f *testing.F) {
  f.Add("some string")
  f.Fuzz(func(*testing.T, string) { })
}

Loading

@guodongli-google
Copy link

@guodongli-google guodongli-google commented Sep 11, 2021

All these four checks can be covered by a simple vet checker. My main question is whether this checker mets the frequency requirement. After all, running a malformed target will result in panic, so there may be few malformed targets in the tested code.

I am more interested in the bugs that fail "silently", e.g. the expected mutations are not performed. Are there some patterns that are malformed but the related tests won't crash?

Loading

@rsc rsc changed the title [dev.fuzz] cmd/vet: checks for malformed fuzz targets cmd/vet: checks for malformed fuzz targets Sep 21, 2021
@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Oct 15, 2021

For anyone looking at this later, golang.org/x/tools/go/analysis/passes/tests is a vet analysis that detects problems with tests, examples, and benchmarks, for example, a missing *T parameter, or an // Output comment in the wrong place.

It's probably a good place to check fuzz tests here, rather than defining a new pass.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants