-
Notifications
You must be signed in to change notification settings - Fork 551
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
Add Mutator Plugins as a Strategy #286
Conversation
/gcbrun |
Will we add usage documentation for this after we've verified the feature works well? Really excited to see this idea land and be usable. |
Sure. I plan on first seeing if this feature is useful. If it is and we decide to make the feature permanent, I'll add documentation and make it "supported" for other users. |
/gcbrun |
/gcbrun |
/gcbrun |
/gcbrun |
@@ -61,6 +61,11 @@ class FuzzerTest(builtin_test.BaseEngineFuzzerTest): | |||
def test_run(self): | |||
"""Test running libFuzzer fuzzer.""" | |||
libfuzzer = fuzzer.LibFuzzer() | |||
|
|||
# Don't test mutator_plugins here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I test it elsewhere so it isn't needed here. But when the test ran before it would spend a really long time and then error out when talking to GCS. Maybe we should get this working?
/gcbrun |
Sorry for the spam folks, my local tests weren't quite working so I was using the upload/gcbrun workflow a bit too aggressively. |
for OSS-Fuzz, you'll need to add a line here: https://github.com/google/clusterfuzz/blob/master/src/appengine/handlers/cron/oss_fuzz_setup.py#L745 to give workers permissions to access the relevant buckets. |
cc @kcc @afd @paulthomson |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! some suggestions about the implementation
/gcbrun |
This CL is getting huge, best to split into 2-3 parts for faster review and easier understanding.
|
/gcbrun |
/gcbrun |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Some questions and minor nits.
"""Creates directories needed to use mutator plugins.""" | ||
|
||
def recreate_directory(directory_path): | ||
shell.create_directory_if_needed( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very hacky, to do create_directory_if_needed first and then remove_directory. Why do we need to remove and re-download plugin everytime ? Is this for every launcher instance ? We need to make sure we dont even unpack if it was unpacked once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very hacky, to do create_directory_if_needed first and then remove_directory. Why do we need to remove and re-download plugin everytime ?
What if the plugin gets updated?
Is this for every launcher instance ?
Yes.
We need to make sure we dont even unpack if it was unpacked once.
I intentionally do this. I agree it would be less wasteful to unpack once for every 5 runs of launcher.py. I believe the code did this initially, but it was suggested that I change this so that mutator plugins would more easily work on OSS-Fuzz.
/gcbrun |
No description provided.