Skip thread-unsafe Windows heaps from MallocDumpProvider.

base/trace_event/malloc_dump_provider.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/trace_event/malloc_dump_provider.h"
 
 #include <stddef.h>
 
 #include "base/allocator/allocator_extension.h"
 #include "base/allocator/allocator_shim.h"
 #include "base/allocator/features.h"
 #include "base/debug/profiler.h"
 #include "base/trace_event/heap_profiler_allocation_context.h"
 #include "base/trace_event/heap_profiler_allocation_context_tracker.h"
 #include "base/trace_event/heap_profiler_allocation_register.h"
 #include "base/trace_event/heap_profiler_heap_dump_writer.h"
 #include "base/trace_event/process_memory_dump.h"
 #include "base/trace_event/trace_event_argument.h"
 #include "build/build_config.h"
 
 #if defined(OS_MACOSX)
 #include <malloc/malloc.h>
 #else
 #include <malloc.h>
 #endif
 #if defined(OS_WIN)
 #include <windows.h>
+// See https://msdn.microsoft.com/en-us/library/windows/desktop/aa366703 .
+#define HEAP_LFH 2
 #endif
 
 namespace base {
 namespace trace_event {
 
 namespace {
 #if BUILDFLAG(USE_EXPERIMENTAL_ALLOCATOR_SHIM)
 
 using allocator::AllocatorDispatch;
 
@@ skipping 58 matching lines @@
 #if defined(OS_WIN)
 // A structure containing some information about a given heap.
 struct WinHeapInfo {
   HANDLE heap_id;
   size_t committed_size;
   size_t uncommitted_size;
   size_t allocated_size;
   size_t block_count;
 };
 
-bool GetHeapInformation(WinHeapInfo* heap_info,
-                        const std::set<void*>& block_to_skip) {
+bool TryHeapLock(HANDLE heap) {
+  HANDLE main_heap = ::GetProcessHeap();
+
+  // NOTE: crbug.com/665516
+  // We should never try to lock a heap created with HEAP_NO_SERIALIZE flag,
+  // since behaviour of HeapLock is undefined and confirmed to be crashy.
+  // Unfortunately, WinAPI lacks a function to tell the flags heap has been
+  // created with, so we don't account any potentially unsafe heap.
+  ULONG heap_info;
+  if (!::HeapQueryInformation(heap, HeapCompatibilityInformation, &heap_info,
+                              sizeof(heap_info), nullptr)) {
+    return false;
+  }
+  // Low-fragmentation heaps are used by default since Windows Vista and
+  // incompatible with HEAP_NO_SERIALIZE flag, hence there is an indicator.
+  // However, main heap is always accountable even if not LFH because
+  // Windows Runtime should be able to spawn some utility threads implicitly.
+  if (heap_info != HEAP_LFH && heap != main_heap) {
+    return false;
+  }
+
   // NOTE: crbug.com/464430
   // As a part of the Client/Server Runtine Subsystem (CSRSS) lockdown in the
-  // referenced bug, it will invalidate the heap used by CSRSS. The author has
-  // not found a way to clean up an invalid heap handle, so it will be left in
-  // the process's heap list. Therefore we need to support when there is this
-  // invalid heap handle in the heap list.
-  // HeapLock implicitly checks certain aspects of the HEAP structure, such as
-  // the signature. If this passes, we assume that this heap is valid and is
-  // not the one owned by CSRSS.
-  if (!::HeapLock(heap_info->heap_id)) {
+  // referenced bug, it will invalidate the heap used by CSRSS.
+  // HeapLock implicitly checks certain aspects of heap structure.
+  // If this passes, we assume that this heap is valid and is not a heap
+  // owned by CSRSS nor some another corrupted heap.
+  if (!::HeapLock(heap)) {
+    CHECK(heap != main_heap) << "Main WinHeap is not accessible.";
+    return false;
+  } else {
+    return true;
+  }
+}
+
+bool GetWinHeapInformation(WinHeapInfo* heap_info,
+                           const std::set<void*>& block_to_skip) {
+  // Some heaps aren't accountable, see TryHeapLock for details.
+  if (!TryHeapLock(heap_info->heap_id)) {
     return false;
   }
   PROCESS_HEAP_ENTRY heap_entry;
   heap_entry.lpData = nullptr;
   // Walk over all the entries in this heap.
   while (::HeapWalk(heap_info->heap_id, &heap_entry) != FALSE) {
     if (block_to_skip.count(heap_entry.lpData) == 1)
       continue;
     if ((heap_entry.wFlags & PROCESS_HEAP_ENTRY_BUSY) != 0) {
       heap_info->allocated_size += heap_entry.cbData;
       heap_info->block_count++;
     } else if ((heap_entry.wFlags & PROCESS_HEAP_REGION) != 0) {
       heap_info->committed_size += heap_entry.Region.dwCommittedSize;
       heap_info->uncommitted_size += heap_entry.Region.dwUnCommittedSize;
     }
   }
   CHECK(::HeapUnlock(heap_info->heap_id) == TRUE);
   return true;
 }
 
 void WinHeapMemoryDumpImpl(WinHeapInfo* all_heap_info) {
 // This method might be flaky for 2 reasons:
 //   - GetProcessHeaps is racy by design. It returns a snapshot of the
 //     available heaps, but there's no guarantee that that snapshot remains
 //     valid. If a heap disappears between GetProcessHeaps() and HeapWalk()
 //     then chaos should be assumed. This flakyness is acceptable for tracing.
-//   - The MSDN page for HeapLock says: "If the HeapLock function is called on
-//     a heap created with the HEAP_NO_SERIALIZATION flag, the results are
-//     undefined."
 //   - Note that multiple heaps occur on Windows primarily because system and
 //     3rd party DLLs will each create their own private heap. It's possible to
 //     retrieve the heap the CRT allocates from and report specifically on that
 //     heap. It's interesting to report all heaps, as e.g. loading or invoking
 //     on a Windows API may consume memory from a private heap.
 #if defined(SYZYASAN)
   if (base::debug::IsBinaryInstrumented())
     return;
 #endif
 
   // Retrieves the number of heaps in the current process.
   DWORD number_of_heaps = ::GetProcessHeaps(0, NULL);
 
   // Try to retrieve a handle to all the heaps owned by this process. Returns
   // false if the number of heaps has changed.
   //
   // This is inherently racy as is, but it's not something that we observe a lot
   // in Chrome, the heaps tend to be created at startup only.
   std::unique_ptr<HANDLE[]> all_heaps(new HANDLE[number_of_heaps]);
   if (::GetProcessHeaps(number_of_heaps, all_heaps.get()) != number_of_heaps)
     return;
 
   // Skip the pointer to the heap array to avoid accounting the memory used by
   // this dump provider.
   std::set<void*> block_to_skip;
   block_to_skip.insert(all_heaps.get());
 
   // Retrieves some metrics about each heap.
-  size_t heap_info_errors = 0;
   for (size_t i = 0; i < number_of_heaps; ++i) {
     WinHeapInfo heap_info = {0};
     heap_info.heap_id = all_heaps[i];
-    if (GetHeapInformation(&heap_info, block_to_skip)) {
+    if (GetWinHeapInformation(&heap_info, block_to_skip)) {
       all_heap_info->allocated_size += heap_info.allocated_size;
       all_heap_info->committed_size += heap_info.committed_size;
       all_heap_info->uncommitted_size += heap_info.uncommitted_size;
       all_heap_info->block_count += heap_info.block_count;
-    } else {
-      ++heap_info_errors;
-      // See notes in GetHeapInformation() but we only expect 1 heap to not be
-      // able to be read.
-      CHECK_EQ(1u, heap_info_errors);
     }
   }
 }
 #endif  // defined(OS_WIN)
 }  // namespace
 
 // static
 const char MallocDumpProvider::kAllocatedObjects[] = "malloc/allocated_objects";
 
 // static
@@ skipping 175 matching lines @@
       tid_dumping_heap_ == PlatformThread::CurrentId())
     return;
   AutoLock lock(allocation_register_lock_);
   if (!allocation_register_)
     return;
   allocation_register_->Remove(address);
 }
 
 }  // namespace trace_event
 }  // namespace base