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

Added go-fuzz for parser #253

Merged
merged 1 commit into from
Oct 17, 2020
Merged

Added go-fuzz for parser #253

merged 1 commit into from
Oct 17, 2020

Conversation

tie
Copy link
Contributor

@tie tie commented Oct 11, 2020

It’d be nice to integrate continuous fuzzing too, but I’m not aware of any easy to set up (and free) services.

See also #252

@codecov-io
Copy link

codecov-io commented Oct 11, 2020

Codecov Report

Merging #253 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #253   +/-   ##
=======================================
  Coverage   61.35%   61.35%           
=======================================
  Files          68       68           
  Lines        6345     6345           
=======================================
  Hits         3893     3893           
  Misses       1936     1936           
  Partials      516      516           
Impacted Files Coverage Δ
database/config.go 68.59% <0.00%> (ø)
sql/query/reindex.go 81.81% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca94a13...55a87ed. Read the comment docs.

@asdine
Copy link
Collaborator

asdine commented Oct 12, 2020

Could we run the fuzzer on a Github Action when a branch is merged on master for example?

@tie
Copy link
Contributor Author

tie commented Oct 12, 2020

Yes, we can do that. I think it’s better to run a scheduled nightly fuzz test.

I’ll write a workflow that stores corpus in cache, but we’d probably need a separate repo or some kind of file storage for corpus data if we don’t want to lose it.

@asdine
Copy link
Collaborator

asdine commented Oct 13, 2020

@tie Is this something that must be generated once and stored somewhere? If so, how about storing it in the Genji repo directly?

@tie
Copy link
Contributor Author

tie commented Oct 13, 2020

For each Fuzz function we need corpus data. Ideally, we should also update it since running go-fuzz may find new inputs that increase coverage (we can git push from GitHub Actions workflow with zero setup).

See libFuzzerTutorial.md#Continuous fuzzing.

@asdine
Copy link
Collaborator

asdine commented Oct 14, 2020

@tie I have created a repo for storing the corpus data: https://github.com/genjidb/go-fuzz-corpus

.github/workflows/test.yaml Outdated Show resolved Hide resolved
fuzz/fuzz.go Outdated Show resolved Hide resolved
@asdine
Copy link
Collaborator

asdine commented Oct 15, 2020

@tie Is it ready for review?

@tie
Copy link
Contributor Author

tie commented Oct 15, 2020

@asdine yes!

There would be another PR to go-fuzz-corpus repo with actions workflow to update the corpus on nightly schedule but the changes for main repo are ready.

Note that the (1.15.x, windows, genji/engine/badgerengine) combo fails with -race flag (see the latest workflow run on the PR source branch). Should we (temporarily) exclude it from the matrix?

@tie tie force-pushed the fuzz branch 2 times, most recently from 4b4a54e to 063eae0 Compare October 16, 2020 16:23
@tie
Copy link
Contributor Author

tie commented Oct 16, 2020

I’ve removed go test workflows since they are still work-in-progress. I’ll submit them as a new PR, and will also add tinygo test runner (currently we only run tests with tinygo tag in Travis config).

Copy link
Collaborator

@asdine asdine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @tie ! That's great job! Let's iterate on this for tinygo and race on subsequent PRs 👍🏼

@asdine asdine merged commit 44b30e3 into chaisql:master Oct 17, 2020
@tie
Copy link
Contributor Author

tie commented Oct 17, 2020

Neat, it’s working. Found another bug too!

@tie tie deleted the fuzz branch October 17, 2020 11:52
@asdine asdine added this to the v0.9.0 milestone Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants