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

x/tools/gopls: check f.Fuzz arguments based on f.Add calls #51398

Closed
findleyr opened this issue Feb 28, 2022 · 4 comments
Closed

x/tools/gopls: check f.Fuzz arguments based on f.Add calls #51398

findleyr opened this issue Feb 28, 2022 · 4 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@findleyr
Copy link
Contributor

Spin-off of #50198: our analysis of Fuzz targets can be improved by comparing their signatures to the signatures of f.Add.

Moving to a separate issue for milestone tracking.

CC @hyangah @ansaba

@findleyr findleyr added this to the gopls/v0.8.1 milestone Feb 28, 2022
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Feb 28, 2022
@findleyr
Copy link
Contributor Author

findleyr commented Mar 4, 2022

@ansaba do you want @suzmue to pick this up next week?

@ansaba
Copy link

ansaba commented Mar 7, 2022

Yes, that would be helpful.

@gopherbot
Copy link

Change https://go.dev/cl/390614 mentions this issue: go/analysis: add analyzer for f.Add

gopherbot pushed a commit to golang/tools that referenced this issue Mar 10, 2022
Calls to f.Add must have arguments of the same types
as the function signature passed to f.Fuzz (excluding testing.T).
This checks that all calls to f.Add in a function declaration
have the same number and types.

This will have some false positives, because it does not check
to see if the call to f.Add is a no-op.
This happens if f.Add is called after or in f.Fuzz.

This can be fixed by adding a heuristic for when f.Add is a noop.
There also should maybe be an analyzer that reports these noop
calls to the user.

Updates golang/go#51398

Change-Id: I49b2ed19cb55347abb9a48960b83b8376b7e6a1d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/390614
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@suzmue
Copy link
Contributor

suzmue commented Mar 14, 2022

There is now an analyzer that checks the arguments to (*testing.F).Add against the arguments expected by the fuzz target in (*testing.F).Fuzz.

The analyzer checks the number of arguments:
Screen Shot 2022-03-14 at 1 53 09 PM

The analyzer checks the type of the arguments:
Screen Shot 2022-03-14 at 1 55 49 PM
Screen Shot 2022-03-14 at 1 56 01 PM

@suzmue suzmue closed this as completed Mar 14, 2022
@rsc rsc unassigned suzmue Jun 22, 2022
@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants