Continuous fuzzing#944
Conversation
|
Looks like that test build did succeed. |
Am I reading this correctly as: tried one input smaller than 4096 bytes? |
|
Thanks for looking at it @bjorn3. This comes from how the fuzzer binary is produced. Fuzzit docs give the That default max length is worth thinking about. If you think it's too small I can increase. |
Is there anywhere I can see the fuzzing job? |
|
I'll send the links, I'm not sure if they're private to the account or what. https://app.fuzzit.dev/orgs/bookmoons/targets/cranelift-reader-parse |
|
You have to login :( |
|
Shoot. They're in my queue anyway. Let me cancel everything and let them run a little while, then I think I can pull the log to send. |
|
There's the beginning of a worker log. Starting to build up a corpus. |
|
You can also upload a seed corpus to Fuzzit. If you had a bunch of valid data laying around you could give it a kickstart. |
|
Looks like it found a crash, in the |
|
Took a little longer but it also found a crash in |
|
I think the last one just ran out of memory. I reduced it to: which will attempt to allocate 477777777 ebbs. Maybe we should add a reader config to deny such pathological inputs. Edit: created issue #951 for this. |
I don't seem to be able to reproduce the crash for |
|
Strange. Maybe it's also a memory error and the platform is running with less. Fuzzit shows the same return code 76 and labels it |
|
Thanks! More fuzzing is always great. We've now moved over to Azure pipelines, is there a chance you could update your PR, please? |
|
Will do! Thanks for looking at it. Just did that for wasmer so I have an idea what it takes. |
|
This is updated. I see fuzzing runs happening in Fuzzit. There's a successful test build: Fuzzit have kindly set up an org that I've preconfigured with targets. If you'd like to sign in to make an account I can get you added to the org. Setup through Azure is now like this:
|
|
I had to update the reader_parse target. d755002 modifies the targeted function. I updated to use the new signature. |
bnjbvr
left a comment
There was a problem hiding this comment.
Thanks! That seems correct, but I'll defer to Till for review, as well as setting up the Azure pipelines if we wanted to do it.
Two remarks:
- I'd like to make sure the commits are squashed before merging, either manually (by you) or by using the squash and merge button when we accept this.
- Can you comment on the CLI options "fuzzing" vs "local-regression", somewhere in the code, please?
Conforms parse_test usage to the new signature introduced in d755002
c56764d to
adf64d8
Compare
|
Made these changes. I kept the target update separate. Put the type option docs into the usage message. |
|
This looks great to me, thank you so much, @bookmoons! I'm traveling right now, and won't be able to thoroughly investigate the setup fully, unfortunately. @alexcrichton, could you take a look and see if this setup seems good? Perhaps also compared to what kind of fuzzing is done for Rust? |
|
I'm seeing a little bug in the fuzz regression stage. Just tested and it's showing on this setup. Probably worth holding off merging until it's resolved. Reported it in fuzzitdev/fuzzit#38 |
9cd4ae9 to
be6197d
Compare
|
Upgrading the Fuzzit client resolved that. This is ready for review again. There's a successful test build: The latest seems to require an API key, so the PR build will fail until that's in. |
|
While I haven't done much continuous fuzzing myself in Rust yet, this seems good to me! Is there a way that API keys aren't required on PRs? Or is there perhaps a readonly key available for PRs and the read/write key could only be used for branches? |
|
👋 I am glad to see this! @bookmoons do you happen to know where reports will be sent when the fuzzer detects an issue? I took a quick look at the fuzzit.dev docs but it was not obvious to me. |
|
Thanks a lot guys. I think the reports go out to everyone on the account. There's a list of admins for the org, so if anyone who should be administering wants to sign up I can get you all added.
I'm going ask Fuzzit about this. The recommendation was that you make the corpus public so it can run in PRs without a key. This seems to break that. I see in settings you can make a readonly key, so maybe that's the right track. |
When they get sent they go out by email, to the address on the GitHub account. Then you can pop in and download the crashing input. |
38d288c to
e975990
Compare
Enables automated fuzzing on Fuzzit. Runs fuzz regression tests every push and PR. Runs full fuzzing every push. Fuzzit emails if it finds crashes. Uses the existing fuzz targets: * translate-module - Fuzz valid WebAssembly modules. * reader-parse - Fuzz IR text format parsing.
|
The latest Fuzzit client brings back PR fuzzing. Made the upgrade to get that working again. |
tschneidereit
left a comment
There was a problem hiding this comment.
Thank you again for doing this, @bookmoons!
I've defined the FUZZIT_API_KEY variable in Pipelines, so let's see what the results will look like :)
|
Ah, I see. @bookmoons, can you add me to the org on Fuzzit? I'll revert the PR for now, because it causes the build to fail. |
This reverts commit 374fd23.
|
Got you added! |
|
If it helps I could open a new PR. |
|
@bookmoons can you add me to the org as well? Thanks! |
|
Thanks, @bookmoons! I just updated the API key. It does indeed seem like we'll have to have a new PR, so could you open one? |
|
Opened #1042 to replace this. |
Proposing to enable continuous fuzzing of the good fuzzing targets. I see this was mentioned in #306 as a possible future improvement.
This patch runs both existing targets on Fuzzit. There's a build under my Travis account I'm expecting to succeed.
The PR build will fail due to missing an API key. Setup is like this:
cranelift-translate-modulecranelift-reader-parseFUZZIT_API_KEY.Thank you for considering.