Add a fuzzer for V8ScriptValueDeserializer.

Add a fuzzer for V8ScriptValueDeserializer.

Corpus is seeded from all of the values deserialized in Blink and V8 unit
tests, which span the supported features and include a number of edge
cases.

BUG=chromium:148757
Committed: https://crrev.com/c2b1c95b3688cd279bf99977d1323f07b80cce88
Cr-Commit-Position: refs/heads/master@{#424465}

[jbroman, 2016-10-06 19:28:46]

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

[commit-bot: I haz the power, 2016-10-06 19:29:22]

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

[jbroman, 2016-10-06 19:47:47]

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

[jbroman, 2016-10-06 19:47:49]

[cc jsbell because he asked about fuzzing :)]

Running this locally has already caught a number of issues (fixes in separate
CLs), plus one in WebRTC which is now what the fuzzer repeatedly finds:
https://bugs.chromium.org/p/webrtc/issues/detail?id=6488

If I understand correctly, once this lands, ClusterFuzz will automatically begin
running it:
https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/clu...

[commit-bot: I haz the power, 2016-10-06 20:51:19]

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

[commit-bot: I haz the power, 2016-10-06 20:51:20]

Dry run: This issue passed the CQ dry run.

[haraken, 2016-10-07 02:14:53]

(I want to have some fuzzer expert review this CL...)

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
File
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp
(right):

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:95:
// Set up.

Can you use V8TestingScope?

[inferno, 2016-10-07 14:01:35]

inferno@chromium.org changed reviewers:
+ inferno@chromium.org, mmoroz@chromium.org, ochang@chromium.org

[inferno, 2016-10-07 14:01:36]

Max/Oliver - can you please review.

[mmoroz, 2016-10-07 14:16:38]

Glad to see one more V8 fuzzer!

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
File
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp
(right):

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:57:
// Consume a 32-bit fuzzing flags field before the real input.
Do you really want to consume 32-bit from the data? I think we can use std::hash
(some examples:
https://cs.chromium.org/search/?q=%22std::hash%22+file:.*fuzz.*&sq=package:ch...)
and use data for the deserealization only.

Also in this case we can move the check from line 69 to the beginning of
LLVMFuzzerTestOneInput() function.

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:62:
memcpy(&flags, data, sizeof(flags));
Would you mind to use
https://cs.chromium.org/chromium/src/base/test/fuzzed_data_provider.h class to
consume the data?

No need to use that class if you switch to std::hash for flags randomization.

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:92:
blobs = &blobInfoArray;
Looks like this is read-only:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/...

If that's correct, does it make sense to move blobInfoArray to a global variable
initialized int he beginning only (i.e. LLVMFuzzerInitialize)?

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:107:
CHECK(!tryCatch.HasCaught())
Just to clarify: this is something we don't expect to happen, but would like to
catch if it happens, right?

[jbroman, 2016-10-07 15:10:29]

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

[commit-bot: I haz the power, 2016-10-07 15:10:41]

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

[jbroman, 2016-10-07 15:11:14]

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
File
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp
(right):

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:57:
// Consume a 32-bit fuzzing flags field before the real input.
On 2016/10/07 at 14:16:38, mmoroz wrote:
> Do you really want to consume 32-bit from the data? I think we can use
std::hash (some examples:
https://cs.chromium.org/search/?q=%22std::hash%22+file:.*fuzz.*&sq=package:ch...)
and use data for the deserealization only.
> 
> Also in this case we can move the check from line 69 to the beginning of
LLVMFuzzerTestOneInput() function.

It's unintuitive to me why this is better; I would have expected this would
hamper libfuzzer's ability to bias toward unexplored code paths (because part of
the flow control depends on a hash function of the whole input, so how can it
cross-over the choice of flags between inputs?). If it is better (or a
non-issue), then I'm willing to switch to std::hash.

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:62:
memcpy(&flags, data, sizeof(flags));
On 2016/10/07 at 14:16:38, mmoroz wrote:
> Would you mind to use
https://cs.chromium.org/chromium/src/base/test/fuzzed_data_provider.h class to
consume the data?
> 
> No need to use that class if you switch to std::hash for flags randomization.

Ah, wasn't aware of it. base/ is ordinarily banned in core/. Since this is just
a fuzzer it might be okay to ignore that here, but there's also
blink::FuzzedDataProvider (which wraps it, but presently also does an additional
copy of the data -- though it could be adjusted not to).

If you recommend std::hash (above), then I'll use that; otherwise I'll switch to
one of these.

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:92:
blobs = &blobInfoArray;
On 2016/10/07 at 14:16:38, mmoroz wrote:
> Looks like this is read-only:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/...
> 
> If that's correct, does it make sense to move blobInfoArray to a global
variable initialized int he beginning only (i.e. LLVMFuzzerInitialize)?

Done.

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:95:
// Set up.
On 2016/10/07 at 02:14:53, haraken wrote:
> Can you use V8TestingScope?

Since V8TestingScope is STACK_ALLOCATED, this would force me to put it in
LLVMFuzzerTestOneInput, and so the whole DummyPageHolder (and the associated
page, document, frame, clients, etc.) would be instantiated each time. I've
pulled that initialization out so that it only happens once, to speed up fuzzing
time. I haven't measured, but since doing so only costs ~3 LOC, it seemed
worthwhile to extract it.

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:107:
CHECK(!tryCatch.HasCaught())
On 2016/10/07 at 14:16:38, mmoroz wrote:
> Just to clarify: this is something we don't expect to happen, but would like
to catch if it happens, right?

Right. I'd like to know if an exception somehow escapes
SerializedScriptValue::deserialize.

[mmoroz, 2016-10-07 15:44:10]

mmoroz@chromium.org changed reviewers:
+ kcc@chromium.org

[mmoroz, 2016-10-07 15:44:12]

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
File
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp
(right):

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:57:
// Consume a 32-bit fuzzing flags field before the real input.
On 2016/10/07 15:11:14, jbroman wrote:
> On 2016/10/07 at 14:16:38, mmoroz wrote:
> > Do you really want to consume 32-bit from the data? I think we can use
> std::hash (some examples:
>
https://cs.chromium.org/search/?q=%22std::hash%22+file:.*fuzz.*&sq=package:ch...)
> and use data for the deserealization only.
> > 
> > Also in this case we can move the check from line 69 to the beginning of
> LLVMFuzzerTestOneInput() function.
> 
> It's unintuitive to me why this is better; I would have expected this would
> hamper libfuzzer's ability to bias toward unexplored code paths (because part
of
> the flow control depends on a hash function of the whole input, so how can it
> cross-over the choice of flags between inputs?). If it is better (or a
> non-issue), then I'm willing to switch to std::hash.

Yeah, that's a valid concern (I used to prefer this way some time ago too), but
actually:

a) That feels intuitive, but we don't have a real proof that result of the
fuzzing will be biased. And I'm pretty sure it depends on the target.

b) Due to the avalanche effect of a hash function, most likely we will not be
that biased to lose any possible coverage. Assume that we are running fuzzer
without hash (i.e. current patchset) and we have two different inputs with 100%
the same coverage - that's more than real, right? But these two different inputs
will have significantly different hashes (~half of the output bits should
differ), so using hash will allow us to cover different flags combinations
without shrinking the data as well.

c) if we can avoid cutting |data| bytes for flags randomization, we can easily
reuse the corpus, mutate it with another engines or even feed to another fuzzers
(of course if we will have another deserialize() fuzzer :) ).

[jbroman, 2016-10-07 18:23:09]

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

[commit-bot: I haz the power, 2016-10-07 18:23:28]

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

[jbroman, 2016-10-07 18:27:11]

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
File
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp
(right):

https://codereview.chromium.org/2402503002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:57:
// Consume a 32-bit fuzzing flags field before the real input.
On 2016/10/07 at 15:44:12, mmoroz wrote:
> On 2016/10/07 15:11:14, jbroman wrote:
> > On 2016/10/07 at 14:16:38, mmoroz wrote:
> > > Do you really want to consume 32-bit from the data? I think we can use
> > std::hash (some examples:
> >
https://cs.chromium.org/search/?q=%22std::hash%22+file:.*fuzz.*&sq=package:ch...)
> > and use data for the deserealization only.
> > > 
> > > Also in this case we can move the check from line 69 to the beginning of
> > LLVMFuzzerTestOneInput() function.
> > 
> > It's unintuitive to me why this is better; I would have expected this would
> > hamper libfuzzer's ability to bias toward unexplored code paths (because
part of
> > the flow control depends on a hash function of the whole input, so how can
it
> > cross-over the choice of flags between inputs?). If it is better (or a
> > non-issue), then I'm willing to switch to std::hash.
> 
> Yeah, that's a valid concern (I used to prefer this way some time ago too),
but actually:
> 
> a) That feels intuitive, but we don't have a real proof that result of the
fuzzing will be biased. And I'm pretty sure it depends on the target.
> 
> b) Due to the avalanche effect of a hash function, most likely we will not be
that biased to lose any possible coverage. Assume that we are running fuzzer
without hash (i.e. current patchset) and we have two different inputs with 100%
the same coverage - that's more than real, right? But these two different inputs
will have significantly different hashes (~half of the output bits should
differ), so using hash will allow us to cover different flags combinations
without shrinking the data as well.
> 
> c) if we can avoid cutting |data| bytes for flags randomization, we can easily
reuse the corpus, mutate it with another engines or even feed to another fuzzers
(of course if we will have another deserialize() fuzzer :) ).

OK, switched to a hash. Thanks for the explanation.

[commit-bot: I haz the power, 2016-10-07 20:05:58]

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

[commit-bot: I haz the power, 2016-10-07 20:06:00]

Dry run: This issue passed the CQ dry run.

[mmoroz, 2016-10-07 21:44:55]

LGTM for fuzzing part, thanks!

[esprehn, 2016-10-07 22:02:17]

lgtm, should we have a fuzzer for serializing too?

https://codereview.chromium.org/2402503002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp
(right):

https://codereview.chromium.org/2402503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:58:
"/native/path", "path", "text/plain");
what's with the magic hashes? can you add a comment for what's going on here?

[mmoroz, 2016-10-08 11:00:11]

On 2016/10/07 22:02:17, esprehn wrote:
> lgtm, should we have a fuzzer for serializing too?
> 
>
https://codereview.chromium.org/2402503002/diff/40001/third_party/WebKit/Sour...
> File
>
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp
> (right):
> 
>
https://codereview.chromium.org/2402503002/diff/40001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:58:
> "/native/path", "path", "text/plain");
> what's with the magic hashes? can you add a comment for what's going on here?

Having a fuzzer for serializing sounds good! We also may consider use them
together in a single fuzzer and do a correctness check in the end, like if (data
== deserialize(serialize(data))) { <error> }.
Example:
https://cs.chromium.org/chromium/src/v8/test/fuzzer/wasm-code.cc?q=wasm_code&...

[jbroman, 2016-10-11 14:26:08]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2016-10-11 14:26:17]

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-10-11 14:54:19]

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

[commit-bot: I haz the power, 2016-10-11 14:54:20]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[jbroman, 2016-10-11 16:37:54]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2016-10-11 16:38:20]

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-10-11 17:12:04]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-11 17:14:34]

Description was changed from

==========
Add a fuzzer for V8ScriptValueDeserializer.

Corpus is seeded from all of the values deserialized in Blink and V8 unit
tests, which span the supported features and include a number of edge
cases.

BUG=chromium:148757
==========

to

==========
Add a fuzzer for V8ScriptValueDeserializer.

Corpus is seeded from all of the values deserialized in Blink and V8 unit
tests, which span the supported features and include a number of edge
cases.

BUG=chromium:148757

Committed: https://crrev.com/c2b1c95b3688cd279bf99977d1323f07b80cce88
Cr-Commit-Position: refs/heads/master@{#424465}
==========

[commit-bot: I haz the power, 2016-10-11 17:14:37]

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

[jbroman, 2016-10-14 18:05:06]

Followup on this: when I look at the logs on ClusterFuzz, I see that it's
aborting early due to LeakSanitizer failures, which seems to arise from the fact
that the fuzzer intentionally leaks a PageHolder (and thus its associated
objects), and thus isn't even getting the chance to look for other failures
(like ASAN failures). What's the right way to deal with this?

[kcc2, 2016-10-14 18:08:13]

On 2016/10/14 18:05:06, jbroman wrote:
> Followup on this: when I look at the logs on ClusterFuzz, I see that it's
> aborting early due to LeakSanitizer failures, which seems to arise from the
fact
> that the fuzzer intentionally leaks a PageHolder (and thus its associated
> objects), and thus isn't even getting the chance to look for other failures
> (like ASAN failures). What's the right way to deal with this?

Other than fixing the leaks? 

I am not sure what do you mean by "intentionally leaks a PageHolder".

Is that a one-time leak at initialization? 
If so, why can't we not leak it and have a pointer to it in a global variable?

Or is that a leak on every invocation? 
If so, libFuzzer won't work properly because it will die with OOM after some
number of invocation.

[jbroman, 2016-10-14 18:15:47]

On 2016/10/14 at 18:08:13, kcc wrote:
> On 2016/10/14 18:05:06, jbroman wrote:
> > Followup on this: when I look at the logs on ClusterFuzz, I see that it's
> > aborting early due to LeakSanitizer failures, which seems to arise from the
fact
> > that the fuzzer intentionally leaks a PageHolder (and thus its associated
> > objects), and thus isn't even getting the chance to look for other failures
> > (like ASAN failures). What's the right way to deal with this?
> 
> Other than fixing the leaks? 
> 
> I am not sure what do you mean by "intentionally leaks a PageHolder".
> 
> Is that a one-time leak at initialization? 
> If so, why can't we not leak it and have a pointer to it in a global variable?

There is a pointer to the PageHolder in a global variable (it's never freed),
but I'm still getting complaints like this one:

Direct leak of 216 byte(s) in 1 object(s) allocated from:
    #0 0x4e86cc in __interceptor_malloc
(/mnt/scratch0/clusterfuzz/slave-bot/builds/chromium-browser-libfuzzer_linux-release-asan_ae530a86793cd6b8b56ce9af9159ac101396e802/revisions/libfuzzer-linux-release-425275/v8_serialized_script_value_fuzzer+0x4e86cc)
    #1 0xb254590 in partitionAllocGenericFlags
third_party/WebKit/Source/wtf/allocator/PartitionAlloc.h:821:18
    #2 0xb254590 in partitionAllocGeneric
third_party/WebKit/Source/wtf/allocator/PartitionAlloc.h:851
    #3 0xb254590 in bufferMalloc
third_party/WebKit/Source/wtf/allocator/Partitions.h:91
    #4 0xb254590 in WTF::BitVector::OutOfLineBits::create(unsigned long)
third_party/WebKit/Source/wtf/BitVector.cpp:80
    #5 0xb25491f in WTF::BitVector::resizeOutOfLine(unsigned long)
third_party/WebKit/Source/wtf/BitVector.cpp:92:37
    #6 0x8250d85 in blink::UseCounter::LegacyCounter::LegacyCounter()
third_party/WebKit/Source/core/frame/UseCounter.cpp:1301:7
    #7 0x8f6e90a in blink::Page::Page(blink::Page::PageClients&)
third_party/WebKit/Source/core/page/Page.cpp:127:7
    #8 0x44bcfc1 in blink::Page::create(blink::Page::PageClients&)
third_party/WebKit/Source/core/page/Page.h:99:16
    #9 0x44bcc2c in blink::DummyPageHolder::DummyPageHolder(blink::IntSize
const&, blink::Page::PageClients*, blink::FrameLoaderClient*, void
(*)(blink::Settings&), blink::InterfaceProvider*)
third_party/WebKit/Source/core/testing/DummyPageHolder.cpp:71:12
    #10 0x44bc94a in blink::DummyPageHolder::create(blink::IntSize const&,
blink::Page::PageClients*, blink::FrameLoaderClient*, void
(*)(blink::Settings&), blink::InterfaceProvider*)
third_party/WebKit/Source/core/testing/DummyPageHolder.cpp:52:25
    #11 0x514bfa in LLVMFuzzerInitialize
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:52:16
    #12 0x51f1c3 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char
const*, unsigned long)) third_party/libFuzzer/src/FuzzerDriver.cpp:359:5
    #13 0x53cb58 in main third_party/libFuzzer/src/FuzzerMain.cpp:20:10
    #14 0x7f1af010bf44 in __libc_start_main
/build/eglibc-oGUzwX/eglibc-2.19/csu/libc-start.c:287

> Or is that a leak on every invocation?

As best I can tell, they're from either the initialize step, or the first input
(for some things which are lazy-initialized).

> If so, libFuzzer won't work properly because it will die with OOM after some
number of invocation.

It has run successfully for me locally with ASAN enabled.

[kcc2, 2016-10-14 18:21:20]

On 2016/10/14 18:15:47, jbroman wrote:
> On 2016/10/14 at 18:08:13, kcc wrote:
> > On 2016/10/14 18:05:06, jbroman wrote:
> > > Followup on this: when I look at the logs on ClusterFuzz, I see that it's
> > > aborting early due to LeakSanitizer failures, which seems to arise from
the
> fact
> > > that the fuzzer intentionally leaks a PageHolder (and thus its associated
> > > objects), and thus isn't even getting the chance to look for other
failures
> > > (like ASAN failures). What's the right way to deal with this?
> > 
> > Other than fixing the leaks? 
> > 
> > I am not sure what do you mean by "intentionally leaks a PageHolder".
> > 
> > Is that a one-time leak at initialization? 
> > If so, why can't we not leak it and have a pointer to it in a global
variable?
> 
> There is a pointer to the PageHolder in a global variable (it's never freed),

Which global? 
In such case lsan should not complain. If a pointer is accessible from a global
state it is not considered a leak. 


> but I'm still getting complaints like this one:
> 
> Direct leak of 216 byte(s) in 1 object(s) allocated from:
>     #0 0x4e86cc in __interceptor_malloc
>
(/mnt/scratch0/clusterfuzz/slave-bot/builds/chromium-browser-libfuzzer_linux-release-asan_ae530a86793cd6b8b56ce9af9159ac101396e802/revisions/libfuzzer-linux-release-425275/v8_serialized_script_value_fuzzer+0x4e86cc)
>     #1 0xb254590 in partitionAllocGenericFlags
> third_party/WebKit/Source/wtf/allocator/PartitionAlloc.h:821:18
>     #2 0xb254590 in partitionAllocGeneric
> third_party/WebKit/Source/wtf/allocator/PartitionAlloc.h:851
>     #3 0xb254590 in bufferMalloc
> third_party/WebKit/Source/wtf/allocator/Partitions.h:91
>     #4 0xb254590 in WTF::BitVector::OutOfLineBits::create(unsigned long)
> third_party/WebKit/Source/wtf/BitVector.cpp:80
>     #5 0xb25491f in WTF::BitVector::resizeOutOfLine(unsigned long)
> third_party/WebKit/Source/wtf/BitVector.cpp:92:37
>     #6 0x8250d85 in blink::UseCounter::LegacyCounter::LegacyCounter()
> third_party/WebKit/Source/core/frame/UseCounter.cpp:1301:7
>     #7 0x8f6e90a in blink::Page::Page(blink::Page::PageClients&)
> third_party/WebKit/Source/core/page/Page.cpp:127:7
>     #8 0x44bcfc1 in blink::Page::create(blink::Page::PageClients&)
> third_party/WebKit/Source/core/page/Page.h:99:16
>     #9 0x44bcc2c in blink::DummyPageHolder::DummyPageHolder(blink::IntSize
> const&, blink::Page::PageClients*, blink::FrameLoaderClient*, void
> (*)(blink::Settings&), blink::InterfaceProvider*)
> third_party/WebKit/Source/core/testing/DummyPageHolder.cpp:71:12
>     #10 0x44bc94a in blink::DummyPageHolder::create(blink::IntSize const&,
> blink::Page::PageClients*, blink::FrameLoaderClient*, void
> (*)(blink::Settings&), blink::InterfaceProvider*)
> third_party/WebKit/Source/core/testing/DummyPageHolder.cpp:52:25
>     #11 0x514bfa in LLVMFuzzerInitialize
>
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:52:16
>     #12 0x51f1c3 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char
> const*, unsigned long)) third_party/libFuzzer/src/FuzzerDriver.cpp:359:5
>     #13 0x53cb58 in main third_party/libFuzzer/src/FuzzerMain.cpp:20:10
>     #14 0x7f1af010bf44 in __libc_start_main
> /build/eglibc-oGUzwX/eglibc-2.19/csu/libc-start.c:287
> 
> > Or is that a leak on every invocation?
> 
> As best I can tell, they're from either the initialize step, or the first
input
> (for some things which are lazy-initialized).

Either is fine. 
Maybe the code actually initializes it twice (and so leaks the first object)?



> 
> > If so, libFuzzer won't work properly because it will die with OOM after some
> number of invocation.
> 
> It has run successfully for me locally with ASAN enabled.

By default lsan is turned off on chrome builds (for historical reasons). 
ClusterFuzz enables it back. 
You can do it with 
ASAN_OPTIONS=detect_leaks=1  ./your-binary

[kcc2, 2016-10-14 18:21:25]

On 2016/10/14 18:15:47, jbroman wrote:
> On 2016/10/14 at 18:08:13, kcc wrote:
> > On 2016/10/14 18:05:06, jbroman wrote:
> > > Followup on this: when I look at the logs on ClusterFuzz, I see that it's
> > > aborting early due to LeakSanitizer failures, which seems to arise from
the
> fact
> > > that the fuzzer intentionally leaks a PageHolder (and thus its associated
> > > objects), and thus isn't even getting the chance to look for other
failures
> > > (like ASAN failures). What's the right way to deal with this?
> > 
> > Other than fixing the leaks? 
> > 
> > I am not sure what do you mean by "intentionally leaks a PageHolder".
> > 
> > Is that a one-time leak at initialization? 
> > If so, why can't we not leak it and have a pointer to it in a global
variable?
> 
> There is a pointer to the PageHolder in a global variable (it's never freed),

Which global? 
In such case lsan should not complain. If a pointer is accessible from a global
state it is not considered a leak. 


> but I'm still getting complaints like this one:
> 
> Direct leak of 216 byte(s) in 1 object(s) allocated from:
>     #0 0x4e86cc in __interceptor_malloc
>
(/mnt/scratch0/clusterfuzz/slave-bot/builds/chromium-browser-libfuzzer_linux-release-asan_ae530a86793cd6b8b56ce9af9159ac101396e802/revisions/libfuzzer-linux-release-425275/v8_serialized_script_value_fuzzer+0x4e86cc)
>     #1 0xb254590 in partitionAllocGenericFlags
> third_party/WebKit/Source/wtf/allocator/PartitionAlloc.h:821:18
>     #2 0xb254590 in partitionAllocGeneric
> third_party/WebKit/Source/wtf/allocator/PartitionAlloc.h:851
>     #3 0xb254590 in bufferMalloc
> third_party/WebKit/Source/wtf/allocator/Partitions.h:91
>     #4 0xb254590 in WTF::BitVector::OutOfLineBits::create(unsigned long)
> third_party/WebKit/Source/wtf/BitVector.cpp:80
>     #5 0xb25491f in WTF::BitVector::resizeOutOfLine(unsigned long)
> third_party/WebKit/Source/wtf/BitVector.cpp:92:37
>     #6 0x8250d85 in blink::UseCounter::LegacyCounter::LegacyCounter()
> third_party/WebKit/Source/core/frame/UseCounter.cpp:1301:7
>     #7 0x8f6e90a in blink::Page::Page(blink::Page::PageClients&)
> third_party/WebKit/Source/core/page/Page.cpp:127:7
>     #8 0x44bcfc1 in blink::Page::create(blink::Page::PageClients&)
> third_party/WebKit/Source/core/page/Page.h:99:16
>     #9 0x44bcc2c in blink::DummyPageHolder::DummyPageHolder(blink::IntSize
> const&, blink::Page::PageClients*, blink::FrameLoaderClient*, void
> (*)(blink::Settings&), blink::InterfaceProvider*)
> third_party/WebKit/Source/core/testing/DummyPageHolder.cpp:71:12
>     #10 0x44bc94a in blink::DummyPageHolder::create(blink::IntSize const&,
> blink::Page::PageClients*, blink::FrameLoaderClient*, void
> (*)(blink::Settings&), blink::InterfaceProvider*)
> third_party/WebKit/Source/core/testing/DummyPageHolder.cpp:52:25
>     #11 0x514bfa in LLVMFuzzerInitialize
>
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:52:16
>     #12 0x51f1c3 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char
> const*, unsigned long)) third_party/libFuzzer/src/FuzzerDriver.cpp:359:5
>     #13 0x53cb58 in main third_party/libFuzzer/src/FuzzerMain.cpp:20:10
>     #14 0x7f1af010bf44 in __libc_start_main
> /build/eglibc-oGUzwX/eglibc-2.19/csu/libc-start.c:287
> 
> > Or is that a leak on every invocation?
> 
> As best I can tell, they're from either the initialize step, or the first
input
> (for some things which are lazy-initialized).

Either is fine. 
Maybe the code actually initializes it twice (and so leaks the first object)?



> 
> > If so, libFuzzer won't work properly because it will die with OOM after some
> number of invocation.
> 
> It has run successfully for me locally with ASAN enabled.

By default lsan is turned off on chrome builds (for historical reasons). 
ClusterFuzz enables it back. 
You can do it with 
ASAN_OPTIONS=detect_leaks=1  ./your-binary

[kcc2, 2016-10-14 18:25:59]

BTW, running this target locally w/o lsan I get an assertion failure: 

ASAN_OPTIONS=detect_odr_violation=0:detect_leaks=0:quarantine_size_mb=1
./out/libfuzzer/./v8_serialized_script_value_fuzzer

#
# Fatal error in ../../third_party/webrtc/base/rtccertificate.cc, line 26
# last system error: 110
# Check failed: identity_
# 
#
==32564== ERROR: libFuzzer: deadly signal
    #0 0x51faa0 in __sanitizer_print_stack_trace
(/usr/local/google/home/kcc/chromium/src/out/libfuzzer/v8_serialized_script_value_fuzzer+0x51faa0)
    #1 0x5be642 in fuzzer::Fuzzer::CrashCallback()
third_party/libFuzzer/src/FuzzerLoop.cpp:228:5
    #2 0x5be556 in fuzzer::Fuzzer::StaticCrashSignalCallback()
third_party/libFuzzer/src/FuzzerLoop.cpp:217:6
    #3 0x61f2f7 in fuzzer::CrashHandler(int, siginfo*, void*)
third_party/libFuzzer/src/FuzzerUtil.cpp:80:3
    #4 0x7f605bdec32f  (/lib/x86_64-linux-gnu/libpthread.so.0+0x1032f)
    #5 0x12f6946 in rtc::RTCCertificate::RTCCertificate(rtc::SSLIdentity*)
third_party/webrtc/base/rtccertificate.cc:26:3
    #6 0x12f7dfb in
rtc::RefCountedObject<rtc::RTCCertificate>::RefCountedObject<rtc::SSLIdentity*>(rtc::SSLIdentity*&&)
third_party/webrtc/base/refcount.h:40:38
    #7 0x12f7880 in rtc::RTCCertificate::FromPEM(rtc::RTCCertificatePEM const&)
third_party/webrtc/base/rtccertificate.cc:57:14
    #8 0x11098d9 in content::(anonymous
namespace)::TestWebRTCCertificateGenerator::fromPEM(blink::WebString,
blink::WebString) content/test/test_blink_web_unit_test_support.cc:334:9
    #9 0x7f6064674ca8 in
blink::V8ScriptValueDeserializerForModules::readDOMObject(blink::SerializationTag)
third_party/WebKit/Source/bindings/modules/v8/serialization/V8ScriptValueDeserializerForModules.cpp:30:33
    #10 0x7f6094a56aff in
blink::V8ScriptValueDeserializer::ReadHostObject(v8::Isolate*)
third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueDeserializer.cpp:329:17
    #11 0x7f608bee508d in v8::internal::ValueDeserializer::ReadHostObject()
v8/src/value-serializer.cc:1439:19
    #12 0x7f608beda527 in v8::internal::ValueDeserializer::ReadObjectInternal()
v8/src/value-serializer.cc:1035:14
    #13 0x7f608bed8c6a in v8::internal::ValueDeserializer::ReadObject()
v8/src/value-serializer.cc:940:32
    #14 0x7f608beeb14a in
v8::internal::ValueDeserializer::ReadObjectUsingEntireBufferForLegacyFormat()
v8/src/value-serializer.cc:1720:14
    #15 0x7f6089b68398 in
v8::ValueDeserializer::ReadValue(v8::Local<v8::Context>) v8/src/api.cc:3071:32
    #16 0x7f6094a4ff2e in blink::V8ScriptValueDeserializer::deserialize()
third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueDeserializer.cpp:62:23
    #17 0x7f60646915d6 in
blink::SerializedScriptValueForModulesFactory::deserialize(blink::SerializedScriptValue*,
v8::Isolate*, blink::HeapVector<blink::Member<blink::MessagePort>, 0ul>*,
WTF::Vector<blink::WebBlobInfo, 0ul, WTF::PartitionAllocator> const*)
third_party/WebKit/Source/bindings/modules/v8/SerializedScriptValueForModulesFactory.cpp:43:25
    #18 0x7f609487049b in
blink::SerializedScriptValue::deserialize(v8::Isolate*,
blink::HeapVector<blink::Member<blink::MessagePort>, 0ul>*,
WTF::Vector<blink::WebBlobInfo, 0ul, WTF::PartitionAllocator> const*)
third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp:302:51
    #19 0x546e08 in LLVMFuzzerTestOneInput
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:95:26

[jbroman, 2016-10-14 18:36:20]

On 2016/10/14 at 18:21:20, kcc wrote:
> Which global? 
> In such case lsan should not complain. If a pointer is accessible from a
global state it is not considered a leak.

The one on line 28:
DummyPageHolder* pageHolder = nullptr;

(The rest of the state is indirectly owned by that object.)

On 2016/10/14 at 18:25:59, kcc wrote:
> BTW, running this target locally w/o lsan I get an assertion failure: 
> 
> ASAN_OPTIONS=detect_odr_violation=0:detect_leaks=0:quarantine_size_mb=1
./out/libfuzzer/./v8_serialized_script_value_fuzzer
> 
> #
> # Fatal error in ../../third_party/webrtc/base/rtccertificate.cc, line 26
> # last system error: 110
> # Check failed: identity_
> # 
> #
> ==32564== ERROR: libFuzzer: deadly signal
>     #0 0x51faa0 in __sanitizer_print_stack_trace
(/usr/local/google/home/kcc/chromium/src/out/libfuzzer/v8_serialized_script_value_fuzzer+0x51faa0)
>     #1 0x5be642 in fuzzer::Fuzzer::CrashCallback()
third_party/libFuzzer/src/FuzzerLoop.cpp:228:5
>     #2 0x5be556 in fuzzer::Fuzzer::StaticCrashSignalCallback()
third_party/libFuzzer/src/FuzzerLoop.cpp:217:6
>     #3 0x61f2f7 in fuzzer::CrashHandler(int, siginfo*, void*)
third_party/libFuzzer/src/FuzzerUtil.cpp:80:3
>     #4 0x7f605bdec32f  (/lib/x86_64-linux-gnu/libpthread.so.0+0x1032f)
>     #5 0x12f6946 in rtc::RTCCertificate::RTCCertificate(rtc::SSLIdentity*)
third_party/webrtc/base/rtccertificate.cc:26:3
>     #6 0x12f7dfb in
rtc::RefCountedObject<rtc::RTCCertificate>::RefCountedObject<rtc::SSLIdentity*>(rtc::SSLIdentity*&&)
third_party/webrtc/base/refcount.h:40:38
>     #7 0x12f7880 in rtc::RTCCertificate::FromPEM(rtc::RTCCertificatePEM
const&) third_party/webrtc/base/rtccertificate.cc:57:14
>     #8 0x11098d9 in content::(anonymous
namespace)::TestWebRTCCertificateGenerator::fromPEM(blink::WebString,
blink::WebString) content/test/test_blink_web_unit_test_support.cc:334:9
>     #9 0x7f6064674ca8 in
blink::V8ScriptValueDeserializerForModules::readDOMObject(blink::SerializationTag)
third_party/WebKit/Source/bindings/modules/v8/serialization/V8ScriptValueDeserializerForModules.cpp:30:33
>     #10 0x7f6094a56aff in
blink::V8ScriptValueDeserializer::ReadHostObject(v8::Isolate*)
third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueDeserializer.cpp:329:17
>     #11 0x7f608bee508d in v8::internal::ValueDeserializer::ReadHostObject()
v8/src/value-serializer.cc:1439:19
>     #12 0x7f608beda527 in
v8::internal::ValueDeserializer::ReadObjectInternal()
v8/src/value-serializer.cc:1035:14
>     #13 0x7f608bed8c6a in v8::internal::ValueDeserializer::ReadObject()
v8/src/value-serializer.cc:940:32
>     #14 0x7f608beeb14a in
v8::internal::ValueDeserializer::ReadObjectUsingEntireBufferForLegacyFormat()
v8/src/value-serializer.cc:1720:14
>     #15 0x7f6089b68398 in
v8::ValueDeserializer::ReadValue(v8::Local<v8::Context>) v8/src/api.cc:3071:32
>     #16 0x7f6094a4ff2e in blink::V8ScriptValueDeserializer::deserialize()
third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueDeserializer.cpp:62:23
>     #17 0x7f60646915d6 in
blink::SerializedScriptValueForModulesFactory::deserialize(blink::SerializedScriptValue*,
v8::Isolate*, blink::HeapVector<blink::Member<blink::MessagePort>, 0ul>*,
WTF::Vector<blink::WebBlobInfo, 0ul, WTF::PartitionAllocator> const*)
third_party/WebKit/Source/bindings/modules/v8/SerializedScriptValueForModulesFactory.cpp:43:25
>     #18 0x7f609487049b in
blink::SerializedScriptValue::deserialize(v8::Isolate*,
blink::HeapVector<blink::Member<blink::MessagePort>, 0ul>*,
WTF::Vector<blink::WebBlobInfo, 0ul, WTF::PartitionAllocator> const*)
third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp:302:51
>     #19 0x546e08 in LLVMFuzzerTestOneInput
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:95:26

Yes, that one is a real bug (and exists in our current serialization code, too);
I've previously filed it as
https://bugs.chromium.org/p/webrtc/issues/detail?id=6488.

[kcc2, 2016-10-14 18:38:55]

On 2016/10/14 18:36:20, jbroman wrote:
> On 2016/10/14 at 18:21:20, kcc wrote:
> > Which global? 
> > In such case lsan should not complain. If a pointer is accessible from a
> global state it is not considered a leak.
> 
> The one on line 28:
> DummyPageHolder* pageHolder = nullptr;
> 
> (The rest of the state is indirectly owned by that object.)

This is totally fine. 
But the leak reports I see look different. 
Is that the same you see? 

Direct leak of 624 byte(s) in 1 object(s) allocated from:
    #0 0x5433ab in operator new[](unsigned long)
(/usr/local/google/home/kcc/chromium/src/out/libfuzzer/v8_serialized_script_value_fuzzer+0x5433ab)
    #1 0x7fde4f5ea82c in v8::internal::AllocateSlotSet(unsigned long, unsigned
char*) v8/src/heap/spaces.cc:1076:23
    #2 0x7fde4f5ea764 in v8::internal::MemoryChunk::AllocateOldToNewSlots()
v8/src/heap/spaces.cc:1085:30
    #3 0x7fde4e12b8a2 in AllocateSlotSet v8/src/heap/remembered-set.h:240:14
    #4 0x7fde4e12b8a2 in Insert v8/src/heap/remembered-set.h:28
    #5 0x7fde4e12b8a2 in v8::internal::Heap::RecordWrite(v8::internal::Object*,
int, v8::internal::Object*) v8/src/heap/heap-inl.h:504
    #6 0x7fde4f97bb04 in set_instance_descriptors v8/src/objects-inl.h:5520:1
    #7 0x7fde4f97bb04 in
v8::internal::Map::InitializeDescriptors(v8::internal::DescriptorArray*,
v8::internal::LayoutDescriptor*) v8/src/objects-inl.h:5502
    #8 0x7fde4f97edb1 in
v8::internal::Map::CopyReplaceDescriptors(v8::internal::Handle<v8::internal::Map>,
v8::internal::Handle<v8::internal::DescriptorArray>,
v8::internal::Handle<v8::internal:»
    #9 0x7fde4f96f411 in
v8::internal::Map::CopyAddDescriptor(v8::internal::Handle<v8::internal::Map>,
v8::internal::Descriptor*, v8::internal::TransitionFlag)
v8/src/objects.cc:9690:10
    #10 0x7fde4f9ffdb0 in CopyWithConstant v8/src/objects.cc:3108:10
    #11 0x7fde4f9ffdb0 in
v8::internal::Map::TransitionToDataProperty(v8::internal::Handle<v8::internal::Map>,
v8::internal::Handle<v8::internal::Name>,
v8::internal::Handle<v8::internal::Object>»
    #12 0x7fde4f885045 in
v8::internal::LookupIterator::PrepareTransitionToDataProperty(v8::internal::Handle<v8::internal::JSObject>,
v8::internal::Handle<v8::internal::Object>, v8::internal::Pro»
    #13 0x7fde4f99d703 in
v8::internal::Object::AddDataProperty(v8::internal::LookupIterator*,
v8::internal::Handle<v8::internal::Object>, v8::internal::PropertyAttributes,
v8::internal::Object::»
    #14 0x7fde4f9b0ee7 in
v8::internal::JSObject::AddProperty(v8::internal::Handle<v8::internal::JSObject>,
v8::internal::Handle<v8::internal::Name>,
v8::internal::Handle<v8::internal::Object>, v»
    #15 0x7fde4f2ce9a8 in
v8::internal::Factory::NewFunctionPrototype(v8::internal::Handle<v8::internal::JSFunction>)
v8/src/factory.cc:1404:5
    #16 0x7fde4e145aea in
v8::internal::ApiNatives::CreateApiFunction(v8::internal::Isolate*,
v8::internal::Handle<v8::internal::FunctionTemplateInfo>,
v8::internal::Handle<v8::internal::Object>,»
    #17 0x7fde4e3e098e in
v8::internal::Genesis::CreateNewGlobals(v8::Local<v8::ObjectTemplate>,
v8::internal::Handle<v8::internal::JSGlobalProxy>) v8/src/bootstrapper.cc:948:33
    #18 0x7fde4e422794 in v8::internal::Genesis::Genesis(v8::internal::Isolate*,
v8::internal::MaybeHandle<v8::internal::JSGlobalProxy>,
v8::Local<v8::ObjectTemplate>, v8::ExtensionConfiguration*»
    #19 0x7fde4e3d75a1 in
v8::internal::Bootstrapper::CreateEnvironment(v8::internal::MaybeHandle<v8::internal::JSGlobalProxy>,
v8::Local<v8::ObjectTemplate>, v8::ExtensionConfiguration*, unsigne»
    #20 0x7fde4e1f621f in Invoke v8/src/api.cc:5998:37
    #21 0x7fde4e1f621f in CreateEnvironment<v8::internal::Context>
v8/src/api.cc:6068
    #22 0x7fde4e1f621f in v8::NewContext(v8::Isolate*,
v8::ExtensionConfiguration*, v8::MaybeLocal<v8::ObjectTemplate>,
v8::MaybeLocal<v8::Value>, unsigned long) v8/src/api.cc:6098
    #23 0x7fde5904db1e in blink::WindowProxy::createContext()
third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp:325:15
    #24 0x7fde5904bd27 in blink::WindowProxy::initialize()
third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp:228:3
    #25 0x7fde5904b415 in blink::WindowProxy::initializeIfNeeded()
third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp:217:10
    #26 0x7fde58d848c5 in
blink::ScriptController::windowProxy(blink::DOMWrapperWorld&)
third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp:182:20
    #27 0x7fde5b25bd4f in
blink::LocalFrame::windowProxy(blink::DOMWrapperWorld&)
third_party/WebKit/Source/core/frame/LocalFrame.cpp:349:20
    #28 0x7fde58f0b249 in blink::toV8ContextEvenIfDetached(blink::Frame*,
blink::DOMWrapperWorld&)
third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp:855:17
    #29 0x7fde58f0ac49 in blink::toV8Context(blink::Frame*,
blink::DOMWrapperWorld&)
third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp:841:36
    #30 0x7fde58deb139 in blink::ScriptState::forWorld(blink::LocalFrame*,
blink::DOMWrapperWorld&)
third_party/WebKit/Source/bindings/core/v8/ScriptState.cpp:98:36
    #31 0x7fde58deaefd in blink::ScriptState::forMainWorld(blink::LocalFrame*)
third_party/WebKit/Source/bindings/core/v8/ScriptState.cpp:92:10
    #32 0x546d77 in LLVMFuzzerTestOneInput
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:87:30

[jbroman, 2016-10-14 18:47:07]

On 2016/10/14 at 18:38:55, kcc wrote:
> On 2016/10/14 18:36:20, jbroman wrote:
> > On 2016/10/14 at 18:21:20, kcc wrote:
> > > Which global? 
> > > In such case lsan should not complain. If a pointer is accessible from a
> > global state it is not considered a leak.
> > 
> > The one on line 28:
> > DummyPageHolder* pageHolder = nullptr;
> > 
> > (The rest of the state is indirectly owned by that object.)
> 
> This is totally fine. 
> But the leak reports I see look different. 
> Is that the same you see? 
> 
> Direct leak of 624 byte(s) in 1 object(s) allocated from:
>     #0 0x5433ab in operator new[](unsigned long)
(/usr/local/google/home/kcc/chromium/src/out/libfuzzer/v8_serialized_script_value_fuzzer+0x5433ab)
>     #1 0x7fde4f5ea82c in v8::internal::AllocateSlotSet(unsigned long, unsigned
char*) v8/src/heap/spaces.cc:1076:23
>     #2 0x7fde4f5ea764 in v8::internal::MemoryChunk::AllocateOldToNewSlots()
v8/src/heap/spaces.cc:1085:30
>     #3 0x7fde4e12b8a2 in AllocateSlotSet v8/src/heap/remembered-set.h:240:14
>     #4 0x7fde4e12b8a2 in Insert v8/src/heap/remembered-set.h:28
>     #5 0x7fde4e12b8a2 in
v8::internal::Heap::RecordWrite(v8::internal::Object*, int,
v8::internal::Object*) v8/src/heap/heap-inl.h:504
>     #6 0x7fde4f97bb04 in set_instance_descriptors v8/src/objects-inl.h:5520:1
>     #7 0x7fde4f97bb04 in
v8::internal::Map::InitializeDescriptors(v8::internal::DescriptorArray*,
v8::internal::LayoutDescriptor*) v8/src/objects-inl.h:5502
>     #8 0x7fde4f97edb1 in
v8::internal::Map::CopyReplaceDescriptors(v8::internal::Handle<v8::internal::Map>,
v8::internal::Handle<v8::internal::DescriptorArray>,
v8::internal::Handle<v8::internal:»
>     #9 0x7fde4f96f411 in
v8::internal::Map::CopyAddDescriptor(v8::internal::Handle<v8::internal::Map>,
v8::internal::Descriptor*, v8::internal::TransitionFlag)
v8/src/objects.cc:9690:10
>     #10 0x7fde4f9ffdb0 in CopyWithConstant v8/src/objects.cc:3108:10
>     #11 0x7fde4f9ffdb0 in
v8::internal::Map::TransitionToDataProperty(v8::internal::Handle<v8::internal::Map>,
v8::internal::Handle<v8::internal::Name>,
v8::internal::Handle<v8::internal::Object>»
>     #12 0x7fde4f885045 in
v8::internal::LookupIterator::PrepareTransitionToDataProperty(v8::internal::Handle<v8::internal::JSObject>,
v8::internal::Handle<v8::internal::Object>, v8::internal::Pro»
>     #13 0x7fde4f99d703 in
v8::internal::Object::AddDataProperty(v8::internal::LookupIterator*,
v8::internal::Handle<v8::internal::Object>, v8::internal::PropertyAttributes,
v8::internal::Object::»
>     #14 0x7fde4f9b0ee7 in
v8::internal::JSObject::AddProperty(v8::internal::Handle<v8::internal::JSObject>,
v8::internal::Handle<v8::internal::Name>,
v8::internal::Handle<v8::internal::Object>, v»
>     #15 0x7fde4f2ce9a8 in
v8::internal::Factory::NewFunctionPrototype(v8::internal::Handle<v8::internal::JSFunction>)
v8/src/factory.cc:1404:5
>     #16 0x7fde4e145aea in
v8::internal::ApiNatives::CreateApiFunction(v8::internal::Isolate*,
v8::internal::Handle<v8::internal::FunctionTemplateInfo>,
v8::internal::Handle<v8::internal::Object>,»
>     #17 0x7fde4e3e098e in
v8::internal::Genesis::CreateNewGlobals(v8::Local<v8::ObjectTemplate>,
v8::internal::Handle<v8::internal::JSGlobalProxy>) v8/src/bootstrapper.cc:948:33
>     #18 0x7fde4e422794 in
v8::internal::Genesis::Genesis(v8::internal::Isolate*,
v8::internal::MaybeHandle<v8::internal::JSGlobalProxy>,
v8::Local<v8::ObjectTemplate>, v8::ExtensionConfiguration*»
>     #19 0x7fde4e3d75a1 in
v8::internal::Bootstrapper::CreateEnvironment(v8::internal::MaybeHandle<v8::internal::JSGlobalProxy>,
v8::Local<v8::ObjectTemplate>, v8::ExtensionConfiguration*, unsigne»
>     #20 0x7fde4e1f621f in Invoke v8/src/api.cc:5998:37
>     #21 0x7fde4e1f621f in CreateEnvironment<v8::internal::Context>
v8/src/api.cc:6068
>     #22 0x7fde4e1f621f in v8::NewContext(v8::Isolate*,
v8::ExtensionConfiguration*, v8::MaybeLocal<v8::ObjectTemplate>,
v8::MaybeLocal<v8::Value>, unsigned long) v8/src/api.cc:6098
>     #23 0x7fde5904db1e in blink::WindowProxy::createContext()
third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp:325:15
>     #24 0x7fde5904bd27 in blink::WindowProxy::initialize()
third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp:228:3
>     #25 0x7fde5904b415 in blink::WindowProxy::initializeIfNeeded()
third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp:217:10
>     #26 0x7fde58d848c5 in
blink::ScriptController::windowProxy(blink::DOMWrapperWorld&)
third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp:182:20
>     #27 0x7fde5b25bd4f in
blink::LocalFrame::windowProxy(blink::DOMWrapperWorld&)
third_party/WebKit/Source/core/frame/LocalFrame.cpp:349:20
>     #28 0x7fde58f0b249 in blink::toV8ContextEvenIfDetached(blink::Frame*,
blink::DOMWrapperWorld&)
third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp:855:17
>     #29 0x7fde58f0ac49 in blink::toV8Context(blink::Frame*,
blink::DOMWrapperWorld&)
third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp:841:36
>     #30 0x7fde58deb139 in blink::ScriptState::forWorld(blink::LocalFrame*,
blink::DOMWrapperWorld&)
third_party/WebKit/Source/bindings/core/v8/ScriptState.cpp:98:36
>     #31 0x7fde58deaefd in blink::ScriptState::forMainWorld(blink::LocalFrame*)
third_party/WebKit/Source/bindings/core/v8/ScriptState.cpp:92:10
>     #32 0x546d77 in LLVMFuzzerTestOneInput
third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp:87:30

This is one of the lazy-initialized objects (I see complaints about this as
well). This backtrace is the V8 context starting up in what is presumably the
first call of LLVMFuzzerTestOneInput. While it's possible this has found a bug
in V8 startup (this is before the deserializer I'm testing is constructed at
all, when we're just asking for a reference to the V8 context), it seems
unlikely to me.

Given that the DummyPageHolder is not freed (but a global pointer to it exists),
it's unsurprising to me that data creating during initialization of JavaScript
globals also persists.

[kcc2, 2016-10-14 18:54:59]

I don't know this code and it's hard for me to judge it, but lsan typically has
no false positives. 

The codeallocates something here
ScriptState* scriptState = ScriptState::forMainWorld(&pageHolder->frame()); 
does it put the memory back into pageHolder?

The usual way to [dis]prove a leak is to run a single input 1000000000 times and
see if the RSS grows. 
Started the run.

[kcc2, 2016-10-14 20:42:36]

On 2016/10/14 18:54:59, kcc2 wrote:
> I don't know this code and it's hard for me to judge it, but lsan typically
has
> no false positives. 
> 
> The codeallocates something here
> ScriptState* scriptState = ScriptState::forMainWorld(&pageHolder->frame()); 
> does it put the memory back into pageHolder?
> 
> The usual way to [dis]prove a leak is to run a single input 1000000000 times
and
> see if the RSS grows. 
> Started the run.

Running this (using an arbitrary file as an input):

ASAN_OPTIONS=detect_odr_violation=0:detect_leaks=0:quarantine_size_mb=1
./out/libfuzzer/./v8_serialized_script_value_fuzzer
C/00885b0dbb9d7c109e1e73733cb29125068ba196 -runs=10000000


#1024	pulse  cov: 10910 bits: 13440 indir: 920 exec/s: 341 rss: 429Mb
#2048	pulse  cov: 11750 bits: 14782 indir: 945 exec/s: 204 rss: 441Mb
#4096	pulse  cov: 11905 bits: 15543 indir: 950 exec/s: 132 rss: 463Mb
#8192	pulse  cov: 12499 bits: 17215 indir: 1002 exec/s: 77 rss: 473Mb
#16384	pulse  cov: 12505 bits: 17350 indir: 1002 exec/s: 42 rss: 488Mb
#32768	pulse  cov: 17048 bits: 22831 indir: 1185 exec/s: 21 rss: 519Mb
<stuck>

RSS grows, also the run times degrade over time.
Looks like we are leaking on ever invocation.

[jbroman, 2016-10-17 15:23:04]

On 2016/10/14 at 20:42:36, kcc wrote:
> On 2016/10/14 18:54:59, kcc2 wrote:
> > I don't know this code and it's hard for me to judge it, but lsan typically
has
> > no false positives. 
> > 
> > The codeallocates something here
> > ScriptState* scriptState = ScriptState::forMainWorld(&pageHolder->frame()); 
> > does it put the memory back into pageHolder?
> > 
> > The usual way to [dis]prove a leak is to run a single input 1000000000 times
and
> > see if the RSS grows. 
> > Started the run.
> 
> Running this (using an arbitrary file as an input):
> 
> ASAN_OPTIONS=detect_odr_violation=0:detect_leaks=0:quarantine_size_mb=1
./out/libfuzzer/./v8_serialized_script_value_fuzzer
C/00885b0dbb9d7c109e1e73733cb29125068ba196 -runs=10000000
> 
> 
> #1024	pulse  cov: 10910 bits: 13440 indir: 920 exec/s: 341 rss: 429Mb
> #2048	pulse  cov: 11750 bits: 14782 indir: 945 exec/s: 204 rss: 441Mb
> #4096	pulse  cov: 11905 bits: 15543 indir: 950 exec/s: 132 rss: 463Mb
> #8192	pulse  cov: 12499 bits: 17215 indir: 1002 exec/s: 77 rss: 473Mb
> #16384	pulse  cov: 12505 bits: 17350 indir: 1002 exec/s: 42 rss: 488Mb
> #32768	pulse  cov: 17048 bits: 22831 indir: 1185 exec/s: 21 rss: 519Mb
> <stuck>
> 
> RSS grows, also the run times degrade over time.
> Looks like we are leaking on ever invocation.

Not sure about RSS growth (could just be an issue with V8 or Oilpan heap not
being perfect, hard to say without further investigation).

But I finally figured out what's going on with the LSAN failures. It looks like
the fuzzer infrastructure is building the binaries with is_asan=true but not
is_lsan=true, so various Chromium/Blink code that is aware of LeakSanitizer
isn't prepared for it, because it's compiled with is_lsan _off_, we get
erroneous reports. For example, the BitVector one I mentioned above should be
suppressed here:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/wtf/BitVector....

Without defined(LEAK_SANITIZER), we don't compile the LSAN fixes. It seems to me
that either these binaries should be running with LeakSanitizer off, or they
should be compiled with LeakSanitizer support.

Currently, the flags seem to be:
enable_nacl = false
ffmpeg_branding = "ChromeOS"
is_asan = true
is_debug = false
optimize_for_fuzzing = true
pdf_enable_xfa = true
proprietary_codecs = true
use_libfuzzer = true

If I add is_lsan=true, I get no LSAN failures.

[kcc2, 2016-10-17 18:30:34]

On 2016/10/17 15:23:04, jbroman wrote:
> On 2016/10/14 at 20:42:36, kcc wrote:
> > On 2016/10/14 18:54:59, kcc2 wrote:
> > > I don't know this code and it's hard for me to judge it, but lsan
typically
> has
> > > no false positives. 
> > > 
> > > The codeallocates something here
> > > ScriptState* scriptState =
ScriptState::forMainWorld(&pageHolder->frame()); 
> > > does it put the memory back into pageHolder?
> > > 
> > > The usual way to [dis]prove a leak is to run a single input 1000000000
times
> and
> > > see if the RSS grows. 
> > > Started the run.
> > 
> > Running this (using an arbitrary file as an input):
> > 
> > ASAN_OPTIONS=detect_odr_violation=0:detect_leaks=0:quarantine_size_mb=1
> ./out/libfuzzer/./v8_serialized_script_value_fuzzer
> C/00885b0dbb9d7c109e1e73733cb29125068ba196 -runs=10000000
> > 
> > 
> > #1024	pulse  cov: 10910 bits: 13440 indir: 920 exec/s: 341 rss: 429Mb
> > #2048	pulse  cov: 11750 bits: 14782 indir: 945 exec/s: 204 rss: 441Mb
> > #4096	pulse  cov: 11905 bits: 15543 indir: 950 exec/s: 132 rss: 463Mb
> > #8192	pulse  cov: 12499 bits: 17215 indir: 1002 exec/s: 77 rss: 473Mb
> > #16384	pulse  cov: 12505 bits: 17350 indir: 1002 exec/s: 42 rss: 488Mb
> > #32768	pulse  cov: 17048 bits: 22831 indir: 1185 exec/s: 21 rss: 519Mb
> > <stuck>
> > 
> > RSS grows, also the run times degrade over time.
> > Looks like we are leaking on ever invocation.
> 
> Not sure about RSS growth (could just be an issue with V8 or Oilpan heap not
> being perfect, hard to say without further investigation).
> 
> But I finally figured out what's going on with the LSAN failures. It looks
like
> the fuzzer infrastructure is building the binaries with is_asan=true but not
> is_lsan=true, so various Chromium/Blink code that is aware of LeakSanitizer
> isn't prepared for it, because it's compiled with is_lsan _off_, we get
> erroneous reports. For example, the BitVector one I mentioned above should be
> suppressed here:
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/wtf/BitVector....
> 
> Without defined(LEAK_SANITIZER), we don't compile the LSAN fixes. It seems to
me
> that either these binaries should be running with LeakSanitizer off, or they
> should be compiled with LeakSanitizer support.
> 
> Currently, the flags seem to be:
> enable_nacl = false
> ffmpeg_branding = "ChromeOS"
> is_asan = true
> is_debug = false
> optimize_for_fuzzing = true
> pdf_enable_xfa = true
> proprietary_codecs = true
> use_libfuzzer = true
> 
> If I add is_lsan=true, I get no LSAN failures.

Thanks for digging into this. 
Indeed with  is_lsan=true (already added to head) no leaks are reported.

I would still encourage you to dig into why the targets slows down over time. 
If you simply run it in a loop with the same input the speed will gradually
degrade: 
ASAN_OPTIONS=detect_odr_violation=0:detect_leaks=0:quarantine_size_mb=1 perf
record  ./out/libfuzzer/./v8_serialized_script_value_fuzzer
C/00885b0dbb9d7c109e1e73733cb29125068ba196 -runs=10000000
./out/libfuzzer/./v8_serialized_script_value_fuzzer: Running 1 inputs 10000000
time(s) each.
Running: C/00885b0dbb9d7c109e1e73733cb29125068ba196
#1024	pulse  cov: 10904 bits: 13440 indir: 920 exec/s: 341 rss: 431Mb
#2048	pulse  cov: 11751 bits: 14478 indir: 945 exec/s: 204 rss: 439Mb
#4096	pulse  cov: 11916 bits: 15447 indir: 950 exec/s: 141 rss: 457Mb
#8192	pulse  cov: 12518 bits: 17189 indir: 1002 exec/s: 79 rss: 468Mb

(see exec/s numbers)

I will also check if this happens w/o asan

[Oliver Chang, 2016-10-17 18:33:28]

On 2016/10/17 15:23:04, jbroman wrote:
> On 2016/10/14 at 20:42:36, kcc wrote:
> > On 2016/10/14 18:54:59, kcc2 wrote:
> > > I don't know this code and it's hard for me to judge it, but lsan
typically
> has
> > > no false positives. 
> > > 
> > > The codeallocates something here
> > > ScriptState* scriptState =
ScriptState::forMainWorld(&pageHolder->frame()); 
> > > does it put the memory back into pageHolder?
> > > 
> > > The usual way to [dis]prove a leak is to run a single input 1000000000
times
> and
> > > see if the RSS grows. 
> > > Started the run.
> > 
> > Running this (using an arbitrary file as an input):
> > 
> > ASAN_OPTIONS=detect_odr_violation=0:detect_leaks=0:quarantine_size_mb=1
> ./out/libfuzzer/./v8_serialized_script_value_fuzzer
> C/00885b0dbb9d7c109e1e73733cb29125068ba196 -runs=10000000
> > 
> > 
> > #1024	pulse  cov: 10910 bits: 13440 indir: 920 exec/s: 341 rss: 429Mb
> > #2048	pulse  cov: 11750 bits: 14782 indir: 945 exec/s: 204 rss: 441Mb
> > #4096	pulse  cov: 11905 bits: 15543 indir: 950 exec/s: 132 rss: 463Mb
> > #8192	pulse  cov: 12499 bits: 17215 indir: 1002 exec/s: 77 rss: 473Mb
> > #16384	pulse  cov: 12505 bits: 17350 indir: 1002 exec/s: 42 rss: 488Mb
> > #32768	pulse  cov: 17048 bits: 22831 indir: 1185 exec/s: 21 rss: 519Mb
> > <stuck>
> > 
> > RSS grows, also the run times degrade over time.
> > Looks like we are leaking on ever invocation.
> 
> Not sure about RSS growth (could just be an issue with V8 or Oilpan heap not
> being perfect, hard to say without further investigation).
> 
> But I finally figured out what's going on with the LSAN failures. It looks
like
> the fuzzer infrastructure is building the binaries with is_asan=true but not
> is_lsan=true, so various Chromium/Blink code that is aware of LeakSanitizer
> isn't prepared for it, because it's compiled with is_lsan _off_, we get
> erroneous reports. For example, the BitVector one I mentioned above should be
> suppressed here:
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/wtf/BitVector....
> 
> Without defined(LEAK_SANITIZER), we don't compile the LSAN fixes. It seems to
me
> that either these binaries should be running with LeakSanitizer off, or they
> should be compiled with LeakSanitizer support.
> 
> Currently, the flags seem to be:
> enable_nacl = false
> ffmpeg_branding = "ChromeOS"
> is_asan = true
> is_debug = false
> optimize_for_fuzzing = true
> pdf_enable_xfa = true
> proprietary_codecs = true
> use_libfuzzer = true
> 
> If I add is_lsan=true, I get no LSAN failures.

is_lsan should be automatically set with is_asan and use_libfuzzer after
https://codereview.chromium.org/2428643002/.

[kcc2, 2016-10-17 20:47:31]

Tried this target with ubsan instead of asan: 
 gn gen out/libfuzzer-ubsan '--args=use_libfuzzer=true is_ubsan_security=true
enable_nacl=false is_debug=false' --check

./out/libfuzzer-ubsan/v8_serialized_script_value_fuzzer
C/00885b0dbb9d7c109e1e73733cb29125068ba196 -runs=10000000 -use_counters=0 

As you can see the heap usage grows, so the leaks are real. 

#32768	pulse  cov: 9848 indir: 1078 exec/s: 16384 rss: 177Mb
#65536	pulse  cov: 12336 indir: 1242 exec/s: 16384 rss: 197Mb
#131072	pulse  cov: 12855 indir: 1300 exec/s: 13107 rss: 243Mb
#262144	pulse  cov: 12857 indir: 1301 exec/s: 13107 rss: 318Mb
#524288	pulse  cov: 12857 indir: 1301 exec/s: 13107 rss: 459Mb
#1048576	pulse  cov: 12873 indir: 1301 exec/s: 13107 rss: 761Mb

[jbroman, 2016-10-17 23:52:30]

On 2016/10/17 at 20:47:31, kcc wrote:
> Tried this target with ubsan instead of asan: 
>  gn gen out/libfuzzer-ubsan '--args=use_libfuzzer=true is_ubsan_security=true
enable_nacl=false is_debug=false' --check
> 
> ./out/libfuzzer-ubsan/v8_serialized_script_value_fuzzer
C/00885b0dbb9d7c109e1e73733cb29125068ba196 -runs=10000000 -use_counters=0 
> 
> As you can see the heap usage grows, so the leaks are real. 
> 
> #32768	pulse  cov: 9848 indir: 1078 exec/s: 16384 rss: 177Mb
> #65536	pulse  cov: 12336 indir: 1242 exec/s: 16384 rss: 197Mb
> #131072	pulse  cov: 12855 indir: 1300 exec/s: 13107 rss: 243Mb
> #262144	pulse  cov: 12857 indir: 1301 exec/s: 13107 rss: 318Mb
> #524288	pulse  cov: 12857 indir: 1301 exec/s: 13107 rss: 459Mb
> #1048576	pulse  cov: 12873 indir: 1301 exec/s: 13107 rss: 761Mb

Indeed I do see that. It's not what LeakSanitizer was reporting, but it is an
issue that should be resolved.

The issue is that the fuzzer, as currently constructed, doesn't give the Blink
GC ("Oilpan") enough room to do collections. I have a patch that should address
this issue. It could be yet more aggressive by forcing full collections, but
this should be cheaper and closer to production behaviour.

https://codereview.chromium.org/2422943003