[libfuzzer] support of custom libfuzzer options via .options file.

[libfuzzer] support of custom libfuzzer options via .options file.

1) remove 'fuzzer_test_launcher', use 'fuzzer_test' for both fuzzers and tests
2) copy dictionary with name like '<fuzzer_name>.dict' (otherwise there are
warnings if one dictionary is used for mor than one fuzzer)
3) remove .sh-warpper for fuzzers because it is platform-dependent
4) copy .options file with name like '<fuzzer_name>.options' if no 'dict' used
and update/generate .options file in the build root if 'dict' is used
5) a couple of tests for updated build template and .options generation

R=aizatsky@chromium.org, inferno@chromium.org, krasin@chromium.org
BUG=539572
Committed: https://crrev.com/344a75ae39ea54533e7185bc98a063f8a54ae531
Cr-Commit-Position: refs/heads/master@{#378761}

[mmoroz, 2016-02-16 16:17:51]

On 2016/02/16 14:30:03, mmoroz wrote:

Alternative idea is to generate one more file when "params" is specified. In
that file we can put any options which we wish, for example, (max_len_min=N,
max_len_max=M). Then parse those new options in run.py script and use them (for
example, generate random max_len in given range [N, M] ) to generate command for
fuzzer launch.

[krasin1, 2016-02-16 17:03:03]

Hi Max,

I will leave this CL for Mike to review.

krasin

[aizatsky, 2016-02-16 18:25:05]

https://codereview.chromium.org/1703523002/diff/1/testing/libfuzzer/fuzzer_te...
File testing/libfuzzer/fuzzer_test.gni (right):

https://codereview.chromium.org/1703523002/diff/1/testing/libfuzzer/fuzzer_te...
testing/libfuzzer/fuzzer_test.gni:95: if (defined(invoker.dict)) {
use forward_variables_from (see below).

https://codereview.chromium.org/1703523002/diff/1/testing/libfuzzer/gen_fuzze...
File testing/libfuzzer/gen_fuzzer_runner.py (right):

https://codereview.chromium.org/1703523002/diff/1/testing/libfuzzer/gen_fuzze...
testing/libfuzzer/gen_fuzzer_runner.py:32: launcher_source_code += "
-dict=$DIR/%(dict)s" % { "dict": args.dict }
build arguments first and insert them into the template, rather than appending
to the result.

[mmoroz, 2016-02-17 10:36:48]

https://codereview.chromium.org/1703523002/diff/1/testing/libfuzzer/fuzzer_te...
File testing/libfuzzer/fuzzer_test.gni (right):

https://codereview.chromium.org/1703523002/diff/1/testing/libfuzzer/fuzzer_te...
testing/libfuzzer/fuzzer_test.gni:95: if (defined(invoker.dict)) {
On 2016/02/16 18:25:05, aizatsky wrote:
> use forward_variables_from (see below).

Done.

Didn't know that, thanks!

https://codereview.chromium.org/1703523002/diff/1/testing/libfuzzer/gen_fuzze...
File testing/libfuzzer/gen_fuzzer_runner.py (right):

https://codereview.chromium.org/1703523002/diff/1/testing/libfuzzer/gen_fuzze...
testing/libfuzzer/gen_fuzzer_runner.py:32: launcher_source_code += "
-dict=$DIR/%(dict)s" % { "dict": args.dict }
On 2016/02/16 18:25:05, aizatsky wrote:
> build arguments first and insert them into the template, rather than appending
> to the result. 

Done.

[aizatsky, 2016-02-17 21:35:16]

The CQ bit was checked by aizatsky@chromium.org

[aizatsky, 2016-02-17 21:35:17]

lgtm

[commit-bot: I haz the power, 2016-02-17 21:37:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1703523002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1703523002/20001

[commit-bot: I haz the power, 2016-02-17 21:37:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-17 21:37:30]

No L-G-T-M from a valid reviewer yet. Only full committers are accepted.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[mmoroz, 2016-02-18 12:03:16]

On 2016/02/17 21:37:30, commit-bot: I haz the power wrote:
> No L-G-T-M from a valid reviewer yet. Only full committers are accepted.
> Even if an L-G-T-M may have been provided, it was from a non-committer,
> _not_ a full super star committer.
> See http://www.chromium.org/getting-involved/become-a-committer
> Note that this has nothing to do with OWNERS files.

Seems that Mike is not committer :(
Ivan or Abhishek, could you please take a look?

[aarya, 2016-02-18 21:49:59]

aarya@google.com changed reviewers:
+ aarya@google.com, kcc@chromium.org, ochang@chromium.org

[aarya, 2016-02-18 21:49:59]

Oliver, can you review this.

I was more thinking about putting individual porperties like maxLen, etc in a
dictionary with their values (than providing customized command line arguments).
That way, we can overwrite over defaults too.

[Oliver Chang, 2016-02-18 22:31:42]

https://codereview.chromium.org/1703523002/diff/20001/testing/libfuzzer/gen_f...
File testing/libfuzzer/gen_fuzzer_runner.py (right):

https://codereview.chromium.org/1703523002/diff/20001/testing/libfuzzer/gen_f...
testing/libfuzzer/gen_fuzzer_runner.py:24: def buildFuzzerArguments(args):
nit: follow chromium python style guide
(https://www.chromium.org/chromium-os/python-style-guidelines) and NameLikeThis
(first letter capitalised).

https://codereview.chromium.org/1703523002/diff/20001/testing/libfuzzer/gen_f...
testing/libfuzzer/gen_fuzzer_runner.py:31: # Put custom parameters after $* to
overwrite default values from ClusterFuzz.
Why do we want to override values from ClusterFuzz?

If specifying the same argument again overwrites the previous value, I think it
would be useful for someone (not just ClusterFuzz) to be able to override a
specific default when running this wrapper.

https://codereview.chromium.org/1703523002/diff/20001/testing/libfuzzer/gen_f...
testing/libfuzzer/gen_fuzzer_runner.py:33: fuzzer_arguments += " %(params)s" % {
"params": args.params[1 : -1]}
why is the quoting in fuzzer_test.gni necessary if you are removing it here in
args.params[1 : -1] ? Does GN quote the strings for you?

[mmoroz, 2016-02-18 22:37:50]

On 2016/02/18 21:49:59, aarya wrote:
> Oliver, can you review this.
> 
> I was more thinking about putting individual porperties like maxLen, etc in a
> dictionary with their values (than providing customized command line
arguments).
> That way, we can overwrite over defaults too.

I wanted to land this to enable some fuzzers which require custom max_len value
(media_container_names needs large length, re2 and v8_regexp need small length,
and so on).

Dictionary with many parameters is obviously better, but it requires an update
in run.py, so we will do it and then update chromium part too. Or do you mean
use dictionary instead of "params" string in .gn config?

[mmoroz, 2016-02-25 17:52:53]

On 2016/02/18 22:37:50, mmoroz wrote:
> On 2016/02/18 21:49:59, aarya wrote:
> > Oliver, can you review this.
> > 
> > I was more thinking about putting individual porperties like maxLen, etc in
a
> > dictionary with their values (than providing customized command line
> arguments).
> > That way, we can overwrite over defaults too.
> 
> I wanted to land this to enable some fuzzers which require custom max_len
value
> (media_container_names needs large length, re2 and v8_regexp need small
length,
> and so on).
> 
> Dictionary with many parameters is obviously better, but it requires an update
> in run.py, so we will do it and then update chromium part too. Or do you mean
> use dictionary instead of "params" string in .gn config?

This version uses .options configuration files for libfuzzer options
customization.
I will add a link to corresponding CL for CF soon. However, if this CL will be
landed first it should not break anything on CF side. Except of dictionaries,
they will not be used until CF gets an update.

[mmoroz, 2016-02-25 18:04:17]

Description was changed from

==========
[libfuzzer] support of custom parameters for fuzzers in fuzzer_test template.

R=aizatsky@chromium.org, inferno@chromium.org, krasin@chromium.org
BUG=539572
==========

to

==========
[libfuzzer] support of custom libfuzzer options via .options file.
    
1) remove 'fuzzer_test_launcher', use 'fuzzer_test' for both fuzzers and tests
2) copy dictionary with name like '<fuzzer_name>.dict' (otherwise there are
warnings if one dictionary is used for mor than one fuzzer)
3) remove .sh-warpper for fuzzers because it is platform-dependent
4) copy .options file with name like '<fuzzer_name>.options' if no 'dict' used
and update/generate .options file in the build root if 'dict' is used
5) a couple of tests for updated build template and .options generation

R=aizatsky@chromium.org, inferno@chromium.org, krasin@chromium.org
BUG=539572
==========

[mmoroz, 2016-02-25 18:09:21]

On 2016/02/25 17:52:53, mmoroz wrote:
> On 2016/02/18 22:37:50, mmoroz wrote:
> > On 2016/02/18 21:49:59, aarya wrote:
> > > Oliver, can you review this.
> > > 
> > > I was more thinking about putting individual porperties like maxLen, etc
in
> a
> > > dictionary with their values (than providing customized command line
> > arguments).
> > > That way, we can overwrite over defaults too.
> > 
> > I wanted to land this to enable some fuzzers which require custom max_len
> value
> > (media_container_names needs large length, re2 and v8_regexp need small
> length,
> > and so on).
> > 
> > Dictionary with many parameters is obviously better, but it requires an
update
> > in run.py, so we will do it and then update chromium part too. Or do you
mean
> > use dictionary instead of "params" string in .gn config?
> 
> This version uses .options configuration files for libfuzzer options
> customization.
> I will add a link to corresponding CL for CF soon. However, if this CL will be
> landed first it should not break anything on CF side. Except of dictionaries,
> they will not be used until CF gets an update.

.options parsing on CF side: https://chromereviews.googleplex.com/364137013

[mmoroz, 2016-02-29 13:26:55]

Mike and Oliver, could you please take a look into latest patch set?

[Oliver Chang, 2016-02-29 18:53:50]

sorry, this fell through the cracks.

lgtm, but I'm not that familiar with GN.

https://codereview.chromium.org/1703523002/diff/40001/testing/libfuzzer/tests...
File testing/libfuzzer/tests/check_fuzzer_config.py (right):

https://codereview.chromium.org/1703523002/diff/40001/testing/libfuzzer/tests...
testing/libfuzzer/tests/check_fuzzer_config.py:21: for option_name in
fuzzer_config.options(OPTIONS_SECTION_LIBFUZZER):
nit: can you use .items() here?

https://codereview.chromium.org/1703523002/diff/40001/testing/libfuzzer/tests...
testing/libfuzzer/tests/check_fuzzer_config.py:23: sys.stdout.write('%s=%s\n\r'
% (option_name, option_value))
Why do you need the \r here?

[mmoroz, 2016-03-01 12:35:20]

https://codereview.chromium.org/1703523002/diff/40001/testing/libfuzzer/tests...
File testing/libfuzzer/tests/check_fuzzer_config.py (right):

https://codereview.chromium.org/1703523002/diff/40001/testing/libfuzzer/tests...
testing/libfuzzer/tests/check_fuzzer_config.py:21: for option_name in
fuzzer_config.options(OPTIONS_SECTION_LIBFUZZER):
On 2016/02/29 18:53:50, Oliver Chang wrote:
> nit: can you use .items() here?

Yes, it would be better, thanks!

Done.

https://codereview.chromium.org/1703523002/diff/40001/testing/libfuzzer/tests...
testing/libfuzzer/tests/check_fuzzer_config.py:23: sys.stdout.write('%s=%s\n\r'
% (option_name, option_value))
On 2016/02/29 18:53:50, Oliver Chang wrote:
> Why do you need the \r here?

I saw that existing test from Mike uses '\n\r' to split lines from script's
output and decided that may be there is any reasonable point for this. He used
|print|, so it could matter for some platforms. Since I'm using
|sys.stdout.write()| I can leave only '\n' in both places. Thanks!

Done.

[inferno, 2016-03-02 16:03:04]

lgtm

[mmoroz, 2016-03-02 16:05:13]

The CQ bit was checked by mmoroz@chromium.org

[mmoroz, 2016-03-02 16:05:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
aizatsky@chromium.org, ochang@chromium.org
Link to the patchset: https://codereview.chromium.org/1703523002/#ps60001
(title: "fix nits in check_fuzzer_config.py script and fuzzer_launcher_test.cc")

[commit-bot: I haz the power, 2016-03-02 16:05:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1703523002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1703523002/60001

[commit-bot: I haz the power, 2016-03-02 16:57:28]

Description was changed from

==========
[libfuzzer] support of custom libfuzzer options via .options file.
    
1) remove 'fuzzer_test_launcher', use 'fuzzer_test' for both fuzzers and tests
2) copy dictionary with name like '<fuzzer_name>.dict' (otherwise there are
warnings if one dictionary is used for mor than one fuzzer)
3) remove .sh-warpper for fuzzers because it is platform-dependent
4) copy .options file with name like '<fuzzer_name>.options' if no 'dict' used
and update/generate .options file in the build root if 'dict' is used
5) a couple of tests for updated build template and .options generation

R=aizatsky@chromium.org, inferno@chromium.org, krasin@chromium.org
BUG=539572
==========

to

==========
[libfuzzer] support of custom libfuzzer options via .options file.
    
1) remove 'fuzzer_test_launcher', use 'fuzzer_test' for both fuzzers and tests
2) copy dictionary with name like '<fuzzer_name>.dict' (otherwise there are
warnings if one dictionary is used for mor than one fuzzer)
3) remove .sh-warpper for fuzzers because it is platform-dependent
4) copy .options file with name like '<fuzzer_name>.options' if no 'dict' used
and update/generate .options file in the build root if 'dict' is used
5) a couple of tests for updated build template and .options generation

R=aizatsky@chromium.org, inferno@chromium.org, krasin@chromium.org
BUG=539572
==========

[commit-bot: I haz the power, 2016-03-02 16:57:30]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-02 16:59:56]

Description was changed from

==========
[libfuzzer] support of custom libfuzzer options via .options file.
    
1) remove 'fuzzer_test_launcher', use 'fuzzer_test' for both fuzzers and tests
2) copy dictionary with name like '<fuzzer_name>.dict' (otherwise there are
warnings if one dictionary is used for mor than one fuzzer)
3) remove .sh-warpper for fuzzers because it is platform-dependent
4) copy .options file with name like '<fuzzer_name>.options' if no 'dict' used
and update/generate .options file in the build root if 'dict' is used
5) a couple of tests for updated build template and .options generation

R=aizatsky@chromium.org, inferno@chromium.org, krasin@chromium.org
BUG=539572
==========

to

==========
[libfuzzer] support of custom libfuzzer options via .options file.

1) remove 'fuzzer_test_launcher', use 'fuzzer_test' for both fuzzers and tests
2) copy dictionary with name like '<fuzzer_name>.dict' (otherwise there are
warnings if one dictionary is used for mor than one fuzzer)
3) remove .sh-warpper for fuzzers because it is platform-dependent
4) copy .options file with name like '<fuzzer_name>.options' if no 'dict' used
and update/generate .options file in the build root if 'dict' is used
5) a couple of tests for updated build template and .options generation

R=aizatsky@chromium.org, inferno@chromium.org, krasin@chromium.org
BUG=539572

Committed: https://crrev.com/344a75ae39ea54533e7185bc98a063f8a54ae531
Cr-Commit-Position: refs/heads/master@{#378761}
==========

[commit-bot: I haz the power, 2016-03-02 16:59:57]

Patchset 4 (id:??) landed as
https://crrev.com/344a75ae39ea54533e7185bc98a063f8a54ae531
Cr-Commit-Position: refs/heads/master@{#378761}