adds the afl instrumenter.

adds the afl instrumenter.

BUG=
R=chrisha@chromium.org

Review-Url: https://codereview.chromium.org/2951713003
Committed: https://github.com/google/syzygy/commit/e94470d3ee05aff00d26e174307821288263a3ee

[chrisha, 2017-06-28 18:20:04]

lgtm with nits!

Thanks for taking the time to land this cleanly, it's nice to have outside
contributors to Syzygy!

https://codereview.chromium.org/2951713003/diff/1/syzygy/instrument/instrumen...
File syzygy/instrument/instrumenters/afl_instrumenter.cc (right):

https://codereview.chromium.org/2951713003/diff/1/syzygy/instrument/instrumen...
syzygy/instrument/instrumenters/afl_instrumenter.cc:39: }
You could fold these two checks into a single check and a single message:

if (!value || value->IsType(...)) {
  LOG(ERROR) << "...";
  return false;
}

https://codereview.chromium.org/2951713003/diff/1/syzygy/instrument/instrumen...
syzygy/instrument/instrumenters/afl_instrumenter.cc:45: * to_parse_list =
nullptr;
Strong preference for repeating the const "base::ListValue*" type, and putting
one variable per line (more readable).

https://codereview.chromium.org/2951713003/diff/1/syzygy/instrument/instrumen...
syzygy/instrument/instrumenters/afl_instrumenter.cc:71: LOG(WARNING) << "The
list must be composed of strings only, ignoring.";
If this is truly a warning, you probably only want to emit it once, otherwise
you could flood the screen with these warnings. I'd prefer this be a hard
failure, as malformed input.

https://codereview.chromium.org/2951713003/diff/1/syzygy/instrument/instrumen...
File syzygy/instrument/instrumenters/afl_instrumenter_unittest.cc (right):

https://codereview.chromium.org/2951713003/diff/1/syzygy/instrument/instrumen...
syzygy/instrument/instrumenters/afl_instrumenter_unittest.cc:1: // Axel '0//
Copyright 2017 Google Inc. All Rights Reserved.
Stray edit?

https://codereview.chromium.org/2951713003/diff/1/syzygy/instrument/instrumen...
syzygy/instrument/instrumenters/afl_instrumenter_unittest.cc:29:
LR"(syzygy\instrument\test_data\afl-good-whitelist.json)";
Well then... I didn't know about string literals in C++11. Learn something every
day!

[0vercl0k, 2017-06-29 02:39:14]

Thanks for the review Chris - really appreciated all your time that went into
this :).

Addressed the feedback in next PS.

Cheers

https://codereview.chromium.org/2951713003/diff/1/syzygy/instrument/instrumen...
File syzygy/instrument/instrumenters/afl_instrumenter.cc (right):

https://codereview.chromium.org/2951713003/diff/1/syzygy/instrument/instrumen...
syzygy/instrument/instrumenters/afl_instrumenter.cc:39: }
On 2017/06/28 18:20:03, chrisha wrote:
> You could fold these two checks into a single check and a single message:
> 
> if (!value || value->IsType(...)) {
>   LOG(ERROR) << "...";
>   return false;
> }

Done.

https://codereview.chromium.org/2951713003/diff/1/syzygy/instrument/instrumen...
syzygy/instrument/instrumenters/afl_instrumenter.cc:45: * to_parse_list =
nullptr;
On 2017/06/28 18:20:04, chrisha wrote:
> Strong preference for repeating the const "base::ListValue*" type, and putting
> one variable per line (more readable).

Done.

https://codereview.chromium.org/2951713003/diff/1/syzygy/instrument/instrumen...
syzygy/instrument/instrumenters/afl_instrumenter.cc:71: LOG(WARNING) << "The
list must be composed of strings only, ignoring.";
On 2017/06/28 18:20:03, chrisha wrote:
> If this is truly a warning, you probably only want to emit it once, otherwise
> you could flood the screen with these warnings. I'd prefer this be a hard
> failure, as malformed input.

Done.

https://codereview.chromium.org/2951713003/diff/1/syzygy/instrument/instrumen...
File syzygy/instrument/instrumenters/afl_instrumenter_unittest.cc (right):

https://codereview.chromium.org/2951713003/diff/1/syzygy/instrument/instrumen...
syzygy/instrument/instrumenters/afl_instrumenter_unittest.cc:1: // Axel '0//
Copyright 2017 Google Inc. All Rights Reserved.
On 2017/06/28 18:20:04, chrisha wrote:
> Stray edit?

Oh interesting - thanks for catching this Chris.

https://codereview.chromium.org/2951713003/diff/1/syzygy/instrument/instrumen...
syzygy/instrument/instrumenters/afl_instrumenter_unittest.cc:29:
LR"(syzygy\instrument\test_data\afl-good-whitelist.json)";
On 2017/06/28 18:20:04, chrisha wrote:
> Well then... I didn't know about string literals in C++11. Learn something
every
> day!

They usually are pretty handy :-).

[0vercl0k, 2017-06-29 02:39:29]

The CQ bit was checked by axelscht@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-29 02:39:31]

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, 2017-06-29 03:28:23]

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

[commit-bot: I haz the power, 2017-06-29 03:28:24]

Dry run: This issue passed the CQ dry run.

[0vercl0k, 2017-06-29 06:20:00]

The CQ bit was checked by axelscht@gmail.com

[0vercl0k, 2017-06-29 06:20:01]

The patchset sent to the CQ was uploaded after l-g-t-m from chrisha@chromium.org
Link to the patchset: https://codereview.chromium.org/2951713003/#ps20001
(title: "addressed feedback")

[commit-bot: I haz the power, 2017-06-29 06:20:08]

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, 2017-06-29 06:20:11]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498717200750850,
"parent_rev": "376c47c5a4c6ff96f748543affcb3d2c9661507b", "commit_rev":
"e94470d3ee05aff00d26e174307821288263a3ee"}

[commit-bot: I haz the power, 2017-06-29 06:20:14]

Description was changed from

==========
adds the afl instrumenter.

BUG=
R=chrisha@chromium.org
==========

to

==========
adds the afl instrumenter.

BUG=
R=chrisha@chromium.org

Review-Url: https://codereview.chromium.org/2951713003
Committed:
https://github.com/google/syzygy/commit/e94470d3ee05aff00d26e174307821288263a3ee
==========

[commit-bot: I haz the power, 2017-06-29 06:20:15]

Committed patchset #2 (id:20001) as
https://github.com/google/syzygy/commit/e94470d3ee05aff00d26e174307821288263a3ee