WinHeap test

base/trace_event/malloc_dump_provider.cc

Index: base/trace_event/malloc_dump_provider.cc
diff --git a/base/trace_event/malloc_dump_provider.cc b/base/trace_event/malloc_dump_provider.cc
index 7d0cb579315d4884e00327fa5175b01c5a3824bd..305def6c367924ddb78e95ade5c7b04e02a13b1b 100644
--- a/base/trace_event/malloc_dump_provider.cc
+++ b/base/trace_event/malloc_dump_provider.cc
@@ -25,6 +25,8 @@
 #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 {
@@ -103,18 +105,99 @@ struct WinHeapInfo {
   size_t block_count;
 };
 
-bool GetHeapInformation(WinHeapInfo* heap_info,
-                        const std::set<void*>& block_to_skip) {
+bool TryHeapLock(HANDLE heap) {
+  HANDLE main_heap = ::GetProcessHeap();
+  LOG(ERROR) << "Trying to lock the heap...";
+
+  // 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)) {
+    LOG(ERROR) << "Unable to get heap info.";
+    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 could spawn some utility threads implicitly.
+  if (heap_info != HEAP_LFH && heap != main_heap) {
+    LOG(ERROR) << "Non-LFH heap skipped.";
+    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 the heap
+  // owned by CSRSS nor some another corrupted heap.
+  if (!::HeapLock(heap)) {
+    LOG(ERROR) << "Unable to lock the heap.";
+    CHECK(heap != main_heap) << "Main WinHeap is not accessible.";
+    return false;
+  } else {
+    return true;
+  }
+}
+
+void TestWinHeapOps() {
+  HANDLE temp_heap;
+  ULONG heap_info;
+  LOG(ERROR) << "=== Testing WinHeap operations ===";
+
+  temp_heap = ::HeapCreate(0, 102400, 10240000);
+  ::HeapDestroy(temp_heap);
+  LOG(ERROR) << "Test 1: Heap created and destroyed (OK)";
+  if (!::HeapQueryInformation(temp_heap, HeapCompatibilityInformation,
+                              &heap_info, sizeof(heap_info), nullptr)) {
+    LOG(ERROR) << "Test 1: HeapQueryInformation failed on destroyed heap (OK)";
+  }
+  if (!::HeapLock(temp_heap)) {
+    LOG(ERROR) << "Test 1: HeapLock failed on destroyed heap (OK)";
+  }
+
+  // NOTE: Surprise! It crashes.
+  std::unique_ptr<HANDLE[]> all_heaps(new HANDLE[100]);
+  int num_of_heaps = ::GetProcessHeaps(100, all_heaps.get());
+  LOG(ERROR) << "Test 1: GetProcessHeaps succeeded (OK)";
+  for (int i = 0; i < num_of_heaps; i++) {
+    if (temp_heap == all_heaps[i]) {
+      LOG(ERROR) << "Test 1: Destroyed heap found among process heaps (OK)";
+    }
+  }
+
+  // NOTE: This test crashes too.
+  // So, let's trust MSDN that LFH and HEAP_NO_SERIALIZE can't be enabled
+  // together.
+  //
+  // temp_heap = ::HeapCreate(HEAP_NO_SERIALIZE, 102400, 10240000);
+  // heap_info = HEAP_LFH;
+  // if (!::HeapSetInformation(temp_heap, HeapCompatibilityInformation,
+  //                           &heap_info, sizeof(heap_info))) {
+  //   LOG(ERROR) << "Test 2: Unable to set LFH mode for non-serialized heap (OK)";
+  // }
+
+  // NOTE: This test crashes as well.
+  //
+  // if (!::HeapQueryInformation((HANDLE) 0xDEADBEEF, HeapCompatibilityInformation,
+  //                             &heap_info, sizeof(heap_info), nullptr)) {
+  //   LOG(ERROR) << "Test 3: HeapQueryInformation is okay with wrong handle (OK)";
+  // }
+  // if (!::HeapLock((HANDLE) 0xDEADBEEF)) {
+  //   LOG(ERROR) << "Test 3: HeapLock is okay with wrong handle (OK)";
+  // }
+
+  LOG(ERROR) << "=== [END] Testing WinHeap operations [END] ===";
+}
+
+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;
@@ -141,9 +224,6 @@ void WinHeapMemoryDumpImpl(WinHeapInfo* all_heap_info) {
 //     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
@@ -154,6 +234,10 @@ void WinHeapMemoryDumpImpl(WinHeapInfo* all_heap_info) {
     return;
 #endif
 
+  TestWinHeapOps();
+
+  LOG(ERROR) << "=== Dumping WinHeaps ===";
+
   // Retrieves the number of heaps in the current process.
   DWORD number_of_heaps = ::GetProcessHeaps(0, NULL);
 
@@ -171,23 +255,27 @@ void WinHeapMemoryDumpImpl(WinHeapInfo* all_heap_info) {
   std::set<void*> block_to_skip;
   block_to_skip.insert(all_heaps.get());
 
+  HANDLE main_heap = ::GetProcessHeap();
+  size_t committed_size_main = 0;
   // 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);
+      if (heap_info.heap_id == main_heap) {
+        committed_size_main = heap_info.committed_size;
+      }
     }
   }
+
+  LOG(ERROR) << "Allocated in secondary heaps: "
+             << std::to_string(all_heap_info->committed_size -
+                               committed_size_main)
+             << " bytes.";
 }
 #endif  // defined(OS_WIN)
 }  // namespace