Move timeout subtraction outside Engine.fuzz.#2108
Conversation
|
/gcbrun |
Currently it's not obvious at all to users of Engine.fuzz how long fuzzing will go for when a max_time is passed. Add Engine.fuzz_additional_processing_timeout so that the caller can explicitly subtract the necessary processing time themselves if needed. Remove engine_common.POSTPROCESSING_TIME. This was a holdout from the launcher days. This is moved to the AFL launcher and can be removed there as well once that's migrated to the Engine interface.
oliverchang
force-pushed
the
engine-timeout
branch
from
3653643 to
fc17bd2
Compare
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
oliverchang
changed the title
| Returns: | ||
| An int representing the number of seconds required. | ||
| """ | ||
| del options |
There was a problem hiding this comment.
What is this needed for?
There was a problem hiding this comment.
This is the new recommended approach to dealing with unused arguments:
https://google.github.io/styleguide/pyguide.html#214-decision
| fuzz_test_timeout = environment.get_value('FUZZ_TEST_TIMEOUT') | ||
| additional_processing_time = engine_impl.fuzz_additional_processing_timeout( | ||
| options) | ||
| fuzz_test_timeout -= additional_processing_time |
There was a problem hiding this comment.
Why do we do -(-X), why not just return positive val from this function and add it here.
There was a problem hiding this comment.
This is a positive value. We negate it in the implementation of libFuzzer's fuzz_additional_processing_timeout because libfuzzer.get_fuzz_timeout() returned a negative value.
We can clean up libfuzzer.get_fuzz_timeout to return a positive value in the first place, but that needs to wait for further refactoring (including AFL->engien refactor) so that we can get rid of some env var overrides used for testing (e.g. HARD_TIMEOUT_OVERRIDE).
| options = engine_impl.prepare(corpus_path, target_path, DATA_DIR) | ||
| options.arguments.append('-runs=10') | ||
| engine_impl.fuzz(target_path, options, TEMP_DIR, 10) | ||
| engine_impl.fuzz(target_path, options, TEMP_DIR, get_fuzz_timeout(5)) |
There was a problem hiding this comment.
This get_fuzz_timeout call is confusing, isnt it this signature ?
src/python/bot/fuzzers/libfuzzer.py:def get_fuzz_timeout(is_mutations_run, total_timeout=None):
There was a problem hiding this comment.
This is a different get_fuzz_timeout, local to this file:
3 participants
Part of #2101
Currently it's not obvious at all to users of Engine.fuzz how long
fuzzing will go for when a max_time is passed.
Add Engine.fuzz_additional_processing_timeout so that the caller can
explicitly subtract the necessary processing time themselves if needed.
Remove engine_common.POSTPROCESSING_TIME. This was a holdout from the
launcher days. This is moved to the AFL launcher and can be removed
there as well once that's migrated to the Engine interface.