Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.

Automatically adding ccs based on OWNERS file / target commit history.

If this is incorrect, please add ClusterFuzz-Wrong label.

Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/v8/v8/+/4ef0e79cba592ee16d279587dc8393bfb229f995 ([snapshot] Remove the builtins snapshot).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.

The signature (NativesExternalStringResource::DecodeForDeserialization) doesn't look directly related to the blamed CL. Maybe it exposed something?

DecodeForDeserialization:

https://cs.chromium.org/chromium/src/v8/src/snapshot/natives.h?l=80&rcl=55d0017cb7b12de4a2f397b60db5b94747a29eb7

Use-site:

https://cs.chromium.org/chromium/src/v8/src/snapshot/deserializer.cc?l=241&rcl=55d0017cb7b12de4a2f397b60db5b94747a29eb7

I don't see us freeing the external string's resource on isolate shutdown. Yang, does this sound familiar?

External resources seem to be freed in Heap::ExternalStringTable::TearDown: 

https://cs.chromium.org/chromium/src/v8/src/heap/heap-inl.h?l=366&rcl=55d0017cb7b12de4a2f397b60db5b94747a29eb7

Native source strings should be registered by Factory::NewNativeSourceString:

https://cs.chromium.org/chromium/src/v8/src/heap/factory.cc?l=1310&rcl=55d0017cb7b12de4a2f397b60db5b94747a29eb7

The leak repros locally at least. Investigating.

The strings are deserialized here: 

(gdb) bt
#0  v8::internal::Deserializer::PostProcessNewObject (this=0x7fffe85a2140, obj=0x7effb2c080a9, space=2)
    at ../../v8/src/snapshot/deserializer.cc:243
[...]
#25 0x0000555564c120cc in blink::BlinkFuzzerTestSupport::BlinkFuzzerTestSupport (
    this=0x5555a5f67ea0 <LLVMFuzzerTestOneInput::test_support>, argc=0, argv=0x0)
    at ../../third_party/blink/renderer/platform/testing/blink_fuzzer_test_support.cc:28
#26 0x0000555564c11d42 in blink::BlinkFuzzerTestSupport::BlinkFuzzerTestSupport (
    this=0x5555a5f67ea0 <LLVMFuzzerTestOneInput::test_support>)
    at ../../third_party/blink/renderer/platform/testing/blink_fuzzer_test_support.cc:16
#27 0x0000555564c05824 in LLVMFuzzerTestOneInput (data=0x0, size=0)
    at ../../third_party/blink/renderer/core/feature_policy/feature_policy_fuzzer.cc:18
#28 0x0000555564c132f8 in ExecuteFilesOnyByOne (argc=2, argv=0x7fffffffdd88) at ../../third_party/libFuzzer/src/afl/afl_driver.cpp:301
#29 0x0000555564c13f7b in main (argc=2, argv=0x7fffffffdd88) at ../../third_party/libFuzzer/src/afl/afl_driver.cpp:339

The comment for BlinkFuzzerTestSupport explicitly says

// Instantiating BlinkFuzzerTestSupport will spin up an environment similar to
// webkit_unit_tests. It should be statically initialized and leaked in fuzzers.

https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/testing/blink_fuzzer_test_support.h?l=10&rcl=39c24c8550d19ff106a2040c582689920ed322eb

No teardown happens at all so i'm not sure the leaks are surprising.

Other similar tests have special handling for LSAN. Once that is added, I don't see the leak warning any more.

diff --git a/third_party/blink/renderer/core/feature_policy/feature_policy_fuzzer.cc b/third_party/blink/renderer/core/feature_policy/feature_policy_fuzzer.cc
index 89610c88b14d..51d3e5e77524 100644
--- a/third_party/blink/renderer/core/feature_policy/feature_policy_fuzzer.cc
+++ b/third_party/blink/renderer/core/feature_policy/feature_policy_fuzzer.cc
@@ -22,5 +22,12 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
       blink::SecurityOrigin::CreateFromString("https://example.com/");
   blink::ParseFeaturePolicyHeader(WTF::String(data, size), origin.get(),
                                   &messages);
+#if defined(ADDRESS_SANITIZER)
+  // LSAN needs unreachable objects to be released to avoid reporting them
+  // incorrectly as a memory leak.
+  blink::ThreadState* currentThreadState = blink::ThreadState::Current();
+  currentThreadState->CollectAllGarbage();
+#endif
+
   return 0;
 }

Uploaded https://crrev.com/c/1341522.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5afa5d59a01834926272f8a27b13547411b6f716

commit 5afa5d59a01834926272f8a27b13547411b6f716
Author: Jakob Gruber <jgruber@chromium.org>
Date: Thu Nov 22 02:04:05 2018

Fix LSAN leak warnings in feature_policy_fuzzer

This moves logic to handle LSAN leak warnings into ~BlinkFuzzerTestSupport.
Two related call sites are updated.

Bug: chromium:906425
Change-Id: I2b7c6d5145e67a3d9d919e5f17e51a28eacee7bc
Reviewed-on: https://chromium-review.googlesource.com/c/1341522
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Abhishek Arya <inferno@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Commit-Queue: Abhishek Arya <inferno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610280}
[modify] https://crrev.com/5afa5d59a01834926272f8a27b13547411b6f716/third_party/blink/renderer/core/css/parser/css_parser_fast_paths_fuzzer.cc
[modify] https://crrev.com/5afa5d59a01834926272f8a27b13547411b6f716/third_party/blink/renderer/core/css/style_sheet_contents_fuzzer.cc
[modify] https://crrev.com/5afa5d59a01834926272f8a27b13547411b6f716/third_party/blink/renderer/platform/testing/blink_fuzzer_test_support.cc

ClusterFuzz testcase 5084131509403648 is still reproducing on tip-of-tree build (trunk).

Please re-test your fix against this testcase and if the fix was incorrect or incomplete, please re-open the bug. Otherwise, ignore this notification and add ClusterFuzz-Wrong label.

Unfortunately #13 is correct, there's still / again a leak on ToT.

Repro:

$ ASAN_OPTIONS=redzone=32:strict_string_check=1:print_suppressions=0:strict_memcmp=1:allow_user_segv_handler=0:allocator_may_return_null=1:handle_sigfpe=1:handle_sigbus=1:detect_stack_use_after_return=1:alloc_dealloc_mismatch=0:print_scariness=1:max_uar_stack_size_log=16:quarantine_size_mb=256:detect_odr_violation=0:handle_sigill=1:allocator_release_to_os_interval_ms=500:use_sigaltstack=1:fast_unwind_on_fatal=1:detect_leaks=1:print_summary=1:handle_abort=1:check_malloc_usable_size=0:detect_container_overflow=1:symbolize=1:handle_segv=1:external_symbolizer_path=third_party/llvm-build/Release+Asserts/bin/llvm-symbolizer out/crbug-906425/feature_policy_fuzzer ~/.clusterfuzz/cache/testcases/5084131509403648_testcase/fuzz-0

Direct leak of 24576 byte(s) in 3 object(s) allocated from:
    #0 0x55d8c117f9fa in __interceptor_calloc /b/swarming/w/ir/kitchen-workdir/src/third_party/llvm/compiler-rt/lib/asan/asan_malloc_linux.cc:155:3
    #1 0x55d8cbd1732a in v8::internal::MemoryChunk::AllocateMarkingBitmap() v8/src/heap/spaces.cc:1429:42
    #2 0x55d8cbd168d2 in v8::internal::MemoryChunk::Initialize(v8::internal::Heap*, unsigned long, unsigned long, unsigned long, unsigned long, v8::internal::Executability, v8::internal::Space*, v8::internal::VirtualMemory) v8/src/heap/spaces.cc:637:10
    #3 0x55d8cbd01272 in v8::internal::MemoryAllocator::AllocateChunk(unsigned long, unsigned long, v8::internal::Executability, v8::internal::Space*) v8/src/heap/spaces.cc:892:7
    #4 0x55d8cbcffe24 in v8::internal::Page* v8::internal::MemoryAllocator::AllocatePage<(v8::internal::MemoryAllocator::AllocationMode)0, v8::internal::PagedSpace>(unsigned long, v8::internal::PagedSpace*, v8::internal::Executability) v8/src/heap/spaces.cc:1144:13
    #5 0x55d8cbd3075b in v8::internal::PagedSpace::Expand() v8/src/heap/spaces.cc:1682:35
    #6 0x55d8cbd4c400 in v8::internal::PagedSpace::RawSlowRefillLinearAllocationArea(int) v8/src/heap/spaces.cc:3230:62
    #7 0x55d8cbd4ba79 in v8::internal::PagedSpace::SlowRefillLinearAllocationArea(int) v8/src/heap/spaces.cc:3177:10
    #8 0x55d8cb916f30 in v8::internal::PagedSpace::EnsureLinearAllocationArea(int) v8/src/heap/spaces-inl.h:339:10
    #9 0x55d8cb915785 in v8::internal::PagedSpace::AllocateRawUnaligned(int, v8::internal::PagedSpace::UpdateSkipList) v8/src/heap/spaces-inl.h:371:8
    #10 0x55d8cb97e233 in v8::internal::Heap::ReserveSpace(std::__1::vector<v8::internal::Heap::Chunk, std::__1::allocator<v8::internal::Heap::Chunk> >*, std::__1::vector<unsigned long, std::__1::allocator<unsigned long> >*) v8/src/heap/heap.cc:1545:46
    #11 0x55d8cd1cf09a in v8::internal::DeserializerAllocator::ReserveSpace() v8/src/snapshot/deserializer-allocator.cc:138:27
    #12 0x55d8cd290725 in v8::internal::StartupDeserializer::DeserializeInto(v8::internal::Isolate*) v8/src/snapshot/startup-deserializer.cc:24:21
    #13 0x55d8cc17eb67 in v8::internal::Isolate::Init(v8::internal::StartupDeserializer*) v8/src/isolate.cc:3336:36
    #14 0x55d8cd284ad3 in v8::internal::Snapshot::Initialize(v8::internal::Isolate*) v8/src/snapshot/snapshot-common.cc:47:27
    #15 0x55d8c99e1dc0 in v8::Isolate::Initialize(v8::Isolate*, v8::Isolate::CreateParams const&) v8/src/api.cc:8280:8
    #16 0x55d8e5bfbb0f in gin::IsolateHolder::IsolateHolder(scoped_refptr<base::SingleThreadTaskRunner>, gin::IsolateHolder::AccessMode, gin::IsolateHolder::AllowAtomicsWaitMode, gin::IsolateHolder::IsolateType, gin::IsolateHolder::IsolateCreationMode) gin/isolate_holder.cc:84:5
    #17 0x55d8dac0bb7f in blink::V8PerIsolateData::V8PerIsolateData(scoped_refptr<base::SingleThreadTaskRunner>, blink::V8PerIsolateData::V8ContextSnapshotMode) third_party/blink/renderer/platform/bindings/v8_per_isolate_data.cc:71:7
    #18 0x55d8dac0ec1a in blink::V8PerIsolateData::Initialize(scoped_refptr<base::SingleThreadTaskRunner>, blink::V8PerIsolateData::V8ContextSnapshotMode) third_party/blink/renderer/platform/bindings/v8_per_isolate_data.cc:137:16
    #19 0x55d8eacbf7d4 in blink::V8Initializer::InitializeMainThread(long const*) third_party/blink/renderer/bindings/core/v8/v8_initializer.cc:719:26
    #20 0x55d8eab48ac0 in blink::(anonymous namespace)::InitializeCommon(blink::Platform*, service_manager::BinderRegistryWithArgs<>*) third_party/blink/renderer/controller/blink_initializer.cc:121:3
    #21 0x55d8eab487a4 in blink::Initialize(blink::Platform*, service_manager::BinderRegistryWithArgs<>*, blink::scheduler::WebThreadScheduler*) third_party/blink/renderer/controller/blink_initializer.cc:149:3
    #22 0x55d8e7dba455 in content::TestBlinkWebUnitTestSupport::TestBlinkWebUnitTestSupport() content/test/test_blink_web_unit_test_support.cc:178:3
    #23 0x55d8e7d71ceb in content::(anonymous namespace)::TestEnvironment::TestEnvironment() content/public/test/blink_test_environment.cc:40:3
    #24 0x55d8e7d71bca in content::SetUpBlinkTestEnvironment() content/public/test/blink_test_environment.cc:97:26
    #25 0x55d8c11bf63b in blink::BlinkFuzzerTestSupport::BlinkFuzzerTestSupport(int, char**) third_party/blink/renderer/platform/testing/blink_fuzzer_test_support.cc:29:3
    #26 0x55d8c11bf2b1 in blink::BlinkFuzzerTestSupport::BlinkFuzzerTestSupport() third_party/blink/renderer/platform/testing/blink_fuzzer_test_support.cc:17:7
    #27 0x55d8c11b1823 in LLVMFuzzerTestOneInput third_party/blink/renderer/core/feature_policy/feature_policy_fuzzer.cc:18:7
    #28 0x55d8c11bfc8f in ExecuteFilesOnyByOne(int, char**) third_party/libFuzzer/src/afl/afl_driver.cpp:301:5
    #29 0x55d8c11bfff4 in main third_party/libFuzzer/src/afl/afl_driver.cpp:339:12

[.. snip ..]

Looks like everything allocated during isolate setup is being detected as a leak.

printf debugging confirms Isolate::Deinit is never called :{

Punting for now & setting reminder for next week.

ClusterFuzz testcase 5084131509403648 appears to be flaky, updating reproducibility label.

ClusterFuzz testcase 5084131509403648 is flaky and no longer crashes, so closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

The NextAction date has arrived: 2018-12-06

Still busy.

The NextAction date has arrived: 2018-12-13

Digging a bit now, noticed a few things:

- ~BlinkFuzzerTestSupport() doesn't call content::TearDownBlinkTestEnvironment().
- content::TearDownBlinkTestEnvironment(), if called, doesn't tear down the Isolate. Thus Isolate::Deinit() is never called and all Isolate-associated memory is leaked from the fuzzer's POV.

~IsolateHolder() is also never called.

So the root cause seems to be that v8::Isolate::Dispose (https://cs.chromium.org/chromium/src/v8/src/api.cc?l=8308&rcl=79b441fd88a58ff78d081b55a6cab9fe32b9343d) is never called. I gotta admit I got lost in all the Chrome-internal setup & teardown logic. 

tkent@, do you mind if I hand this off to you? Summarizing repro instructions:

gn args:

enable_nacl = false
is_asan = true
is_component_build = false
is_debug = false
strip_absolute_paths_from_debug_symbols = true
use_afl = true
use_goma = true

build & run:

$ autoninja -C out/release-asan feature_policy_fuzzer
$ echo "" > tmpfile
$ ASAN_OPTIONS=redzone=32:strict_string_check=1:print_suppressions=0:strict_memcmp=1:allow_user_segv_handler=0:allocator_may_return_null=1:handle_sigfpe=1:handle_sigbus=1:detect_stack_use_after_return=1:alloc_dealloc_mismatch=0:print_scariness=1:max_uar_stack_size_log=16:quarantine_size_mb=256:detect_odr_violation=0:handle_sigill=1:allocator_release_to_os_interval_ms=500:use_sigaltstack=1:fast_unwind_on_fatal=1:detect_leaks=1:print_summary=1:handle_abort=1:check_malloc_usable_size=0:detect_container_overflow=1:symbolize=1:handle_segv=1:external_symbolizer_path=third_party/llvm-build/Release+Asserts/bin/llvm-symbolizer out/release-asan/feature_policy_fuzzer tmpfile

And here's my V8 instrumentation that shows Deinit is not called:

diff --git a/src/isolate.cc b/src/isolate.cc
index 3f75251e09..b3742ce4ec 100644
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -2843,6 +2843,8 @@ bool Isolate::LogObjectRelocation() {
 void Isolate::Deinit() {
   TRACE_ISOLATE(deinit);
 
+  fprintf(stderr, "Isolate::Deinit %p\n", this);
+
   tracing_cpu_profiler_.reset();
   if (FLAG_stress_sampling_allocation_profiler > 0) {
     heap_profiler()->StopSamplingHeapProfiler();
@@ -3179,6 +3181,8 @@ void Isolate::TearDownEmbeddedBlob() {
 bool Isolate::Init(StartupDeserializer* des) {
   TRACE_ISOLATE(init);
 
+  fprintf(stderr, "Isolate::Init %p\n", this);
+
   base::ElapsedTimer timer;
   if (des == nullptr && FLAG_profile_deserialization) timer.Start();


At some point, somewhere, we need to make sure to tear down the Isolate created by V8PerIsolateData::Initialize (which in turn holds a gin::IsolateHolder).

I don't have enough time for this area now.