Fuzzer for TextCodecs

Fuzzer for TextCodecs

This introduces a libFuzzer-based fuzzer (which can be run locally
or via ClusterFuzz) for the WTF::TextCodec implementations. It
exercises the codecs - some of which are implemented in blink,
like UTF-8, UTF-16, Latin1, and some of which come wrap ICU - with
all the argument permutations for encoding and decoding.

Fuzzer docs: https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/getting_started.md

A generated corpus was uploaded to Google Cloud Storage per
the docs; no dictionary is added since any byte stream is 
useful. Running the fuzzer locally, three bugs were already
found and fixed.

[jsbell, 2016-12-02 22:22:29]

jsbell@chromium.org changed reviewers:
+ csharrison@chromium.org, haraken@chromium.org

[jsbell, 2016-12-02 22:22:30]

This found bugs 661367, 661823, 662146 - seems worth having.

But... the CL as is introduces a dependency from wtf/ on platform/ which is
incorrect. Dropping the use of blink_fuzzer_test_support gives me PartitionAlloc
errors.

csharrison@ or haraken@ - do you have any advice on adding fuzzers into wtf/ or
suggestions for other contacts?

[Charlie Harrison, 2016-12-02 22:27:33]

This seems fine to me, wtf doesn't have a dependency on platform, just this
fuzzer (which is built independently).

Let me defer to haraken though, as I'm not an expert in blink architecture.

[haraken, 2016-12-03 02:14:21]

haraken@chromium.org changed reviewers:
+ esprehn@chromium.org, yutak@chromium.org

[haraken, 2016-12-03 02:14:21]

Yeah... BlinkFuzzerTestSupport.{h,cpp} has dependency on content/ and
platform::SchemeRegistry, so BlinkFuzzerTestSupport needs to stay in platform/.
This means that we cannot add a fuzzer to wtf/.

In middle term, the problem will be resolved by merging wtf/ into platform/.

In short term, maybe can we add the TextCodec fuzzer test to platform/ with a
TODO comment?

[jsbell, 2016-12-06 23:51:18]

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

[commit-bot: I haz the power, 2016-12-06 23:52:00]

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

[jsbell, 2016-12-06 23:56:24]

Description was changed from

==========
DO NOT LAND - Fuzzer for TextCodecs
==========

to

==========
Fuzzer for TextCodecs

This introduces a libFuzzer-based fuzzer (which can be run locally
or via ClusterFuzz) for the WTF::TextCodec implementations. It
exercises the codecs - some of which are implemented in blink,
like UTF-8, UTF-16, Latin1, and some of which come wrap ICU - with
all the argument permutations for encoding and decoding.

Fuzzer docs:
https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/get...

A generated corpus was uploaded to Google Cloud Storage per
the docs; no dictionary is added since any byte stream is 
useful. Running the fuzzer locally, three bugs were already
found and fixed.
==========

[jsbell, 2016-12-06 23:57:53]

csharrison@, haraken@ - please take a look?

I'm not thrilled with the getEncodingNamesForTesting() addition so cleaner
alternatives welcome.

[commit-bot: I haz the power, 2016-12-07 00:01:53]

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

[commit-bot: I haz the power, 2016-12-07 00:01:54]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[haraken, 2016-12-07 00:21:30]

Dependency-wise LGTM

[Charlie Harrison, 2016-12-07 16:07:16]

https://codereview.chromium.org/2546233002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/TextCodecFuzzer.cpp (right):

https://codereview.chromium.org/2546233002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/TextCodecFuzzer.cpp:27: Vector<String>
encodings = WTF::getEncodingNamesForTesting();
Can you make all these Vectors static? It is inefficient to populate them for
every run of the fuzzer.\

Another technique is to have a test harness class with these values using
something like base::Singleton.

https://codereview.chromium.org/2546233002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/TextCodecFuzzer.cpp:45: WTF::TextEncoding
encoding(encodingName);
Maybe have a Vector of TextEncodings instead of names, as it looks like the
constructor is not trivial.

https://codereview.chromium.org/2546233002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/TextCodecFuzzer.cpp:49:
std::unique_ptr<TextCodec> codec = newTextCodec(encoding);
It would be good to avoid all the allocations of TextCodecs. I can't see an easy
way to do that though, as it looks like it is hard to avoid statefulness.

[jsbell, 2017-03-02 20:37:51]

FYI, I haven't had a chance to get back to this. If anyone wants to pick it up,
please go for it!

[Charlie Harrison, 2017-03-03 01:39:03]

I can try landing this, as it seems high value.

[jsbell, 2017-03-03 17:39:25]

On 2017/03/03 01:39:03, Charlie Harrison wrote:
> I can try landing this, as it seems high value.

Awesome! Closing this out then.