[wasm] Write fuzzers for single wasm sections.

[wasm] Write fuzzers for single wasm sections.

This CL adds fuzzers for the wasm module sections 'types', 'names',
'globals', 'imports', 'function signatures', 'memory', and 'data', one
fuzzer per section. No fuzzers are added for the other sections because
either there already exists a fuzzer (e.g. wasm-code), or there exist
inter-section dependencies.

To avoid introducing a bunch executables which would make compilation
with make slow, I introduce a single executable
'v8_simple_wasm_section_fuzzer' which calls the fuzzers mentioned above.
This executable is run by the trybots and ensures that the fuzzers
actually compile. For debugging I introduce commandline parameters which
allow to execute the specific fuzzers from 'v8_simple_wasm_section_fuzzer'.

R=titzer@chromium.org, jochen@chromium.org, mstarzinger@chromium.org

Committed: https://crrev.com/3ff201906e14b40d5c9bdb77ea79cd9fd51b6238
Cr-Commit-Position: refs/heads/master@{#39413}

[ahaas, 2016-09-12 14:52:43]

The CQ bit was checked by ahaas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-12 14:52:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jochen (gone - plz use gerrit), 2016-09-12 14:57:21]

https://codereview.chromium.org/2336603002/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2336603002/diff/1/BUILD.gn#newcode2741
BUILD.gn:2741: "test/cctest/wasm/wasm-module-runner.cc",
can you move this into a separate source_set? we shouldn't have the same file in
multiple targets

https://codereview.chromium.org/2336603002/diff/1/BUILD.gn#newcode2755
BUILD.gn:2755: v8_fuzzer("wasm_section_fuzzer") {
why no fuzzers for the others?

[jochen (gone - plz use gerrit), 2016-09-12 14:58:10]

https://codereview.chromium.org/2336603002/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2336603002/diff/1/BUILD.gn#newcode2728
BUILD.gn:2728: "test/fuzzer/wasm-section-fuzzers.cc",
that's also duplicated across all new targets you added?

[commit-bot: I haz the power, 2016-09-12 14:58:16]

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

[commit-bot: I haz the power, 2016-09-12 14:58:17]

Dry run: Try jobs failed on following builders:
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)

[ahaas, 2016-09-12 16:08:11]

The CQ bit was checked by ahaas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-12 16:08:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ahaas, 2016-09-12 16:10:09]

The CQ bit was checked by ahaas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-12 16:10:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ahaas, 2016-09-12 16:10:25]

https://codereview.chromium.org/2336603002/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2336603002/diff/1/BUILD.gn#newcode2728
BUILD.gn:2728: "test/fuzzer/wasm-section-fuzzers.cc",
On 2016/09/12 at 14:58:10, jochen wrote:
> that's also duplicated across all new targets you added?

I introduced another source_set.

https://codereview.chromium.org/2336603002/diff/1/BUILD.gn#newcode2741
BUILD.gn:2741: "test/cctest/wasm/wasm-module-runner.cc",
On 2016/09/12 at 14:57:21, jochen wrote:
> can you move this into a separate source_set? we shouldn't have the same file
in multiple targets

Done.

https://codereview.chromium.org/2336603002/diff/1/BUILD.gn#newcode2755
BUILD.gn:2755: v8_fuzzer("wasm_section_fuzzer") {
On 2016/09/12 at 14:57:21, jochen wrote:
> why no fuzzers for the others?

This executable calls all the section fuzzers. The reason why I do not add one
executable per fuzzer is that it would increase linking time of 'make' users.

[commit-bot: I haz the power, 2016-09-12 16:19:18]

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

[commit-bot: I haz the power, 2016-09-12 16:19:19]

Dry run: Try jobs failed on following builders:
  v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)

[Michael Starzinger, 2016-09-13 09:27:08]

Looking good from my end. Just some comment nit.

https://codereview.chromium.org/2336603002/diff/40001/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/2336603002/diff/40001/src/flag-definitions.h#...
src/flag-definitions.h:1096: // Parameters for the wasm-section-fuzzer, not used
by V8.
We already employ a similar solution for some "d8-only" flags, that are not
respected for other embedders besides the "d8" binary. The section is currently
called "Dev shell flags" in this file.

nit: Can we move this section close to the other, and also clean up the section
headers somehow so that both specifically mention the name of the binary for
which they are used? Something along the lines of ...

//
// Specific flags for {d8}, not used within V8 proper.
//

[...]

//
// Specific flags for {v8_simple_wasm_fuzzer}, not used within V8 proper.
//

[...]

[jochen (gone - plz use gerrit), 2016-09-13 09:33:49]

I don't think multiplexing the different fuzzers like this will work.

make is a thing of the past anyways.

https://codereview.chromium.org/2336603002/diff/40001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2336603002/diff/40001/BUILD.gn#newcode2576
BUILD.gn:2576: source_set("wasm-module-runner") {
why not v8_source_set?

please use underscores instead of dashes

[titzer, 2016-09-13 09:37:48]

On 2016/09/13 09:33:49, jochen wrote:
> I don't think multiplexing the different fuzzers like this will work.
> 

The main thing is that we will have 10-12 fuzzers, each of which is pretty much
identical, and given that each of our debug binaries is > 300mb, it's wasteful
to generate that many binaries.

Another solution that doesn't generate 3gb of new binaries?

> make is a thing of the past anyways.
> 
> https://codereview.chromium.org/2336603002/diff/40001/BUILD.gn
> File BUILD.gn (right):
> 
> https://codereview.chromium.org/2336603002/diff/40001/BUILD.gn#newcode2576
> BUILD.gn:2576: source_set("wasm-module-runner") {
> why not v8_source_set?
> 
> please use underscores instead of dashes

[ahaas, 2016-09-13 09:48:59]

The CQ bit was checked by ahaas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-13 09:49:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ahaas, 2016-09-13 09:50:22]

https://codereview.chromium.org/2336603002/diff/40001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2336603002/diff/40001/BUILD.gn#newcode2576
BUILD.gn:2576: source_set("wasm-module-runner") {
On 2016/09/13 at 09:33:48, jochen wrote:
> why not v8_source_set?
> 
> please use underscores instead of dashes

Done, and I use v8_source_set now.

https://codereview.chromium.org/2336603002/diff/40001/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/2336603002/diff/40001/src/flag-definitions.h#...
src/flag-definitions.h:1096: // Parameters for the wasm-section-fuzzer, not used
by V8.
On 2016/09/13 at 09:27:08, Michael Starzinger wrote:
> We already employ a similar solution for some "d8-only" flags, that are not
respected for other embedders besides the "d8" binary. The section is currently
called "Dev shell flags" in this file.
> 
> nit: Can we move this section close to the other, and also clean up the
section headers somehow so that both specifically mention the name of the binary
for which they are used? Something along the lines of ...
> 
> //
> // Specific flags for {d8}, not used within V8 proper.
> //
> 
> [...]
> 
> //
> // Specific flags for {v8_simple_wasm_fuzzer}, not used within V8 proper.
> //
> 
> [...]

Done.

[jochen (gone - plz use gerrit), 2016-09-13 09:53:26]

I can see the point about the binary size.

Does clusterfuzz support running the same binary with multiple command line
flags?

If yes, it might make sense to merge all fuzzers into one.

We'd also need to teach run-tests.py about running the binary with multiple
flags then.

[commit-bot: I haz the power, 2016-09-13 10:26:15]

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

[commit-bot: I haz the power, 2016-09-13 10:26:16]

Dry run: This issue passed the CQ dry run.

[ahaas, 2016-09-13 10:42:04]

On 2016/09/13 at 09:53:26, jochen wrote:
> I can see the point about the binary size.
> 
> Does clusterfuzz support running the same binary with multiple command line
flags?
> 
> If yes, it might make sense to merge all fuzzers into one.
> 
> We'd also need to teach run-tests.py about running the binary with multiple
flags then.

The purpose of the binary is just to make sure that the fuzzer code compiles and
executes, and to provide debugging support. Note that if no flags are passed to
the binary, as run-tests.py does right now, then the binary executes all section
fuzzers. Therefore the current setup in run-tests.py guarantees some sanity of
the section fuzzers.

The binary does not affect clusterfuzz at all. Clusterfuzz targets are defined
in chromium, and there I plan to define one target per fuzzer. I tried the
fuzzers already in chromium and it works.

[jochen (gone - plz use gerrit), 2016-09-13 11:08:17]

On 2016/09/13 at 10:42:04, ahaas wrote:
> On 2016/09/13 at 09:53:26, jochen wrote:
> > I can see the point about the binary size.
> > 
> > Does clusterfuzz support running the same binary with multiple command line
flags?
> > 
> > If yes, it might make sense to merge all fuzzers into one.
> > 
> > We'd also need to teach run-tests.py about running the binary with multiple
flags then.
> 
> The purpose of the binary is just to make sure that the fuzzer code compiles
and executes, and to provide debugging support. Note that if no flags are passed
to the binary, as run-tests.py does right now, then the binary executes all
section fuzzers. Therefore the current setup in run-tests.py guarantees some
sanity of the section fuzzers.
> 
> The binary does not affect clusterfuzz at all. Clusterfuzz targets are defined
in chromium, and there I plan to define one target per fuzzer. I tried the
fuzzers already in chromium and it works.

that means you add 3GB to chromium's output then...

[ahaas, 2016-09-13 12:03:05]

On 2016/09/13 at 11:08:17, jochen wrote:
> On 2016/09/13 at 10:42:04, ahaas wrote:
> > On 2016/09/13 at 09:53:26, jochen wrote:
> > > I can see the point about the binary size.
> > > 
> > > Does clusterfuzz support running the same binary with multiple command
line flags?
> > > 
> > > If yes, it might make sense to merge all fuzzers into one.
> > > 
> > > We'd also need to teach run-tests.py about running the binary with
multiple flags then.
> > 
> > The purpose of the binary is just to make sure that the fuzzer code compiles
and executes, and to provide debugging support. Note that if no flags are passed
to the binary, as run-tests.py does right now, then the binary executes all
section fuzzers. Therefore the current setup in run-tests.py guarantees some
sanity of the section fuzzers.
> > 
> > The binary does not affect clusterfuzz at all. Clusterfuzz targets are
defined in chromium, and there I plan to define one target per fuzzer. I tried
the fuzzers already in chromium and it works.
> 
> that means you add 3GB to chromium's output then...

yes

[ahaas, 2016-09-13 14:43:11]

The CQ bit was checked by ahaas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-13 14:43:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ahaas, 2016-09-13 14:47:21]

On 2016/09/13 at 12:03:05, ahaas wrote:
> On 2016/09/13 at 11:08:17, jochen wrote:
> > On 2016/09/13 at 10:42:04, ahaas wrote:
> > > On 2016/09/13 at 09:53:26, jochen wrote:
> > > > I can see the point about the binary size.
> > > > 
> > > > Does clusterfuzz support running the same binary with multiple command
line flags?
> > > > 
> > > > If yes, it might make sense to merge all fuzzers into one.
> > > > 
> > > > We'd also need to teach run-tests.py about running the binary with
multiple flags then.
> > > 
> > > The purpose of the binary is just to make sure that the fuzzer code
compiles and executes, and to provide debugging support. Note that if no flags
are passed to the binary, as run-tests.py does right now, then the binary
executes all section fuzzers. Therefore the current setup in run-tests.py
guarantees some sanity of the section fuzzers.
> > > 
> > > The binary does not affect clusterfuzz at all. Clusterfuzz targets are
defined in chromium, and there I plan to define one target per fuzzer. I tried
the fuzzers already in chromium and it works.
> > 
> > that means you add 3GB to chromium's output then...
> 
> yes

I changed the CL to generate one binary per fuzzer.

[commit-bot: I haz the power, 2016-09-13 14:48:45]

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

[commit-bot: I haz the power, 2016-09-13 14:48:46]

Dry run: Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)

[ahaas, 2016-09-13 14:51:12]

The CQ bit was checked by ahaas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-13 14:51:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-13 15:17:44]

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

[commit-bot: I haz the power, 2016-09-13 15:17:45]

Dry run: This issue passed the CQ dry run.

[titzer, 2016-09-13 16:35:01]

On 2016/09/13 14:47:21, ahaas wrote:
> On 2016/09/13 at 12:03:05, ahaas wrote:
> > On 2016/09/13 at 11:08:17, jochen wrote:
> > > On 2016/09/13 at 10:42:04, ahaas wrote:
> > > > On 2016/09/13 at 09:53:26, jochen wrote:
> > > > > I can see the point about the binary size.
> > > > > 
> > > > > Does clusterfuzz support running the same binary with multiple command
> line flags?
> > > > > 
> > > > > If yes, it might make sense to merge all fuzzers into one.
> > > > > 
> > > > > We'd also need to teach run-tests.py about running the binary with
> multiple flags then.
> > > > 
> > > > The purpose of the binary is just to make sure that the fuzzer code
> compiles and executes, and to provide debugging support. Note that if no flags
> are passed to the binary, as run-tests.py does right now, then the binary
> executes all section fuzzers. Therefore the current setup in run-tests.py
> guarantees some sanity of the section fuzzers.
> > > > 
> > > > The binary does not affect clusterfuzz at all. Clusterfuzz targets are
> defined in chromium, and there I plan to define one target per fuzzer. I tried
> the fuzzers already in chromium and it works.
> > > 
> > > that means you add 3GB to chromium's output then...
> > 
> > yes
> 
> I changed the CL to generate one binary per fuzzer.

lgtm, short and sweet

[jochen (gone - plz use gerrit), 2016-09-14 08:31:07]

lgtm

[ahaas, 2016-09-14 10:39:38]

The CQ bit was checked by ahaas@chromium.org

[ahaas, 2016-09-14 10:39:38]

The patchset sent to the CQ was uploaded after l-g-t-m from titzer@chromium.org,
jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/2336603002/#ps120001
(title: "Rebase")

[commit-bot: I haz the power, 2016-09-14 10:39:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-14 11:17:18]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-09-14 11:17:58]

Description was changed from

==========
[wasm] Write fuzzers for single wasm sections.

This CL adds fuzzers for the wasm module sections 'types', 'names',
'globals', 'imports', 'function signatures', 'memory', and 'data', one
fuzzer per section. No fuzzers are added for the other sections because
either there already exists a fuzzer (e.g. wasm-code), or there exist
inter-section dependencies.

To avoid introducing a bunch executables which would make compilation
with make slow, I introduce a single executable
'v8_simple_wasm_section_fuzzer' which calls the fuzzers mentioned above.
This executable is run by the trybots and ensures that the fuzzers
actually compile. For debugging I introduce commandline parameters which
allow to execute the specific fuzzers from 'v8_simple_wasm_section_fuzzer'.

R=titzer@chromium.org, jochen@chromium.org, mstarzinger@chromium.org
==========

to

==========
[wasm] Write fuzzers for single wasm sections.

This CL adds fuzzers for the wasm module sections 'types', 'names',
'globals', 'imports', 'function signatures', 'memory', and 'data', one
fuzzer per section. No fuzzers are added for the other sections because
either there already exists a fuzzer (e.g. wasm-code), or there exist
inter-section dependencies.

To avoid introducing a bunch executables which would make compilation
with make slow, I introduce a single executable
'v8_simple_wasm_section_fuzzer' which calls the fuzzers mentioned above.
This executable is run by the trybots and ensures that the fuzzers
actually compile. For debugging I introduce commandline parameters which
allow to execute the specific fuzzers from 'v8_simple_wasm_section_fuzzer'.

R=titzer@chromium.org, jochen@chromium.org, mstarzinger@chromium.org

Committed: https://crrev.com/3ff201906e14b40d5c9bdb77ea79cd9fd51b6238
Cr-Commit-Position: refs/heads/master@{#39413}
==========

[commit-bot: I haz the power, 2016-09-14 11:18:00]

Patchset 7 (id:??) landed as
https://crrev.com/3ff201906e14b40d5c9bdb77ea79cd9fd51b6238
Cr-Commit-Position: refs/heads/master@{#39413}