win: Attempt to fix unloaded modules list by using RtlGetUnloadEventTraceEx

I haven't been able to reproduce this locally, but we see errors in
crash dumps where the unloaded module list consists of a number of
modules with invalid names and implausible addresses. My assumption is
that RTL_UNLOAD_EVENT_TRACE isn't correct for some OS levels. Instead of
trying to finesse and test that, use RtlGetUnloadEventTraceEx() instead
of RtlGetUnloadEventTrace(), which returns an element size. (This
function is Vista+ which is why it wasn't used the first time around.)

R=mark@chromium.org
BUG=chromium:620175

Change-Id: I4d7080a03623276f9c1c038d6e7329af70e4a64c
Reviewed-on: https://chromium-review.googlesource.com/421564
Reviewed-by: Mark Mentovai <mark@chromium.org>

handler/win/crashy_test_program.cc

@@ -201,8 +201,8 @@ int CrashyMain(int argc, wchar_t* argv[]) {
   AllocateExtraUnsavedMemory(extra_ranges);
 
   // Load and unload some uncommonly used modules so we can see them in the list
-  // reported by `lm`. At least two so that we confirm we got the size of
-  // RTL_UNLOAD_EVENT_TRACE right.
+  // reported by `lm`. At least two so that we confirm we got the element size
+  // advancement of RTL_UNLOAD_EVENT_TRACE correct.
   CHECK(GetModuleHandle(L"lz32.dll") == nullptr);
   CHECK(GetModuleHandle(L"wmerror.dll") == nullptr);
   HMODULE lz32 = LoadLibrary(L"lz32.dll");

snapshot/win/process_snapshot_win.cc

@@ -281,21 +281,43 @@ void ProcessSnapshotWin::InitializeUnloadedModules() {
 #error port
 #endif
 
-  RTL_UNLOAD_EVENT_TRACE<Traits>* unload_event_trace_address =
-      RtlGetUnloadEventTrace<Traits>();
-  WinVMAddress address_in_target_process =
-      reinterpret_cast<WinVMAddress>(unload_event_trace_address);
+  ULONG* element_size;
+  ULONG* element_count;
+  void* event_trace_address;
+  RtlGetUnloadEventTraceEx(&element_size, &element_count, &event_trace_address);
 
-  std::vector<RTL_UNLOAD_EVENT_TRACE<Traits>> events(
-      RTL_UNLOAD_EVENT_TRACE_NUMBER);
-  if (!process_reader_.ReadMemory(address_in_target_process,
-                                  events.size() * sizeof(events[0]),
-                                  &events[0])) {
+  if (*element_size < sizeof(RTL_UNLOAD_EVENT_TRACE<Traits>)) {
+    LOG(ERROR) << "unexpected unloaded module list element size";
     return;
   }
 
-  for (const RTL_UNLOAD_EVENT_TRACE<Traits>& uet : events) {
-    if (uet.ImageName[0]) {
+  const WinVMAddress address_in_target_process =
+      reinterpret_cast<WinVMAddress>(event_trace_address);
+
+  Traits::Pointer pointer_to_array;
+  if (!process_reader_.ReadMemory(address_in_target_process,
+                                  sizeof(pointer_to_array),
+                                  &pointer_to_array)) {
+    LOG(ERROR) << "failed to read target address";
+    return;
+  }
+
+  // No unloaded modules.
+  if (pointer_to_array == 0)
+    return;
+
+  const size_t data_size = *element_size * *element_count;
+  std::vector<uint8_t> data(data_size);
+  if (!process_reader_.ReadMemory(pointer_to_array, data_size, &data[0])) {
+    LOG(ERROR) << "failed to read unloaded module data";
+    return;
+  }
+
+  for (ULONG i = 0; i < *element_count; ++i) {
+    const uint8_t* base_address = &data[i * *element_size];
+    const auto& uet =
+        *reinterpret_cast<const RTL_UNLOAD_EVENT_TRACE<Traits>*>(base_address);
+    if (uet.ImageName[0] != 0) {
       unloaded_modules_.push_back(UnloadedModuleSnapshot(
           uet.BaseAddress,
           uet.SizeOfImage,

util/win/nt_internals.cc

@@ -42,7 +42,9 @@ NTSTATUS NTAPI NtSuspendProcess(HANDLE);
 
 NTSTATUS NTAPI NtResumeProcess(HANDLE);
 
-void* NTAPI RtlGetUnloadEventTrace();
+VOID NTAPI RtlGetUnloadEventTraceEx(PULONG* ElementSize,
+                                    PULONG* ElementCount,
+                                    PVOID* EventTrace);
 
 namespace crashpad {
 
@@ -145,12 +147,12 @@ NTSTATUS NtResumeProcess(HANDLE handle) {
   return nt_resume_process(handle);
 }
 
-template <class Traits>
-RTL_UNLOAD_EVENT_TRACE<Traits>* RtlGetUnloadEventTrace() {
-  static const auto rtl_get_unload_event_trace =
-      GET_FUNCTION_REQUIRED(L"ntdll.dll", ::RtlGetUnloadEventTrace);
-  return reinterpret_cast<RTL_UNLOAD_EVENT_TRACE<Traits>*>(
-      rtl_get_unload_event_trace());
+void RtlGetUnloadEventTraceEx(ULONG** element_size,
+                              ULONG** element_count,
+                              void** event_trace) {
+  static const auto rtl_get_unload_event_trace_ex =
+      GET_FUNCTION_REQUIRED(L"ntdll.dll", ::RtlGetUnloadEventTraceEx);
+  rtl_get_unload_event_trace_ex(element_size, element_count, event_trace);
 }
 
 // Explicit instantiations with the only 2 valid template arguments to avoid
@@ -169,10 +171,4 @@ template NTSTATUS NtOpenThread<process_types::internal::Traits64>(
     const process_types::CLIENT_ID<process_types::internal::Traits64>*
         client_id);
 
-template RTL_UNLOAD_EVENT_TRACE<process_types::internal::Traits32>*
-RtlGetUnloadEventTrace<process_types::internal::Traits32>();
-
-template RTL_UNLOAD_EVENT_TRACE<process_types::internal::Traits64>*
-RtlGetUnloadEventTrace<process_types::internal::Traits64>();
-
 }  // namespace crashpad

util/win/nt_internals.h

@@ -75,10 +75,7 @@ NTSTATUS NtSuspendProcess(HANDLE handle);
 
 NTSTATUS NtResumeProcess(HANDLE handle);
 
-// From https://msdn.microsoft.com/en-us/library/bb432428(VS.85).aspx and
-// http://processhacker.sourceforge.net/doc/struct___r_t_l___u_n_l_o_a_d___e_v_e_n_t___t_r_a_c_e.html
-#define RTL_UNLOAD_EVENT_TRACE_NUMBER 64
-
+// From https://msdn.microsoft.com/en-us/library/cc678403(v=vs.85).aspx.
 template <class Traits>
 struct RTL_UNLOAD_EVENT_TRACE {
   typename Traits::Pointer BaseAddress;
@@ -87,14 +84,10 @@ struct RTL_UNLOAD_EVENT_TRACE {
   ULONG TimeDateStamp;
   ULONG CheckSum;
   WCHAR ImageName[32];
-  ULONG Version0;
-  union {
-    ULONG Version1;
-    typename Traits::Pad alignment_for_x64;
-  };
 };
 
-template <class Traits>
-RTL_UNLOAD_EVENT_TRACE<Traits>* RtlGetUnloadEventTrace();
+void RtlGetUnloadEventTraceEx(ULONG** element_size,
+                              ULONG** element_count,
+                              void** event_trace);
 
 }  // namespace crashpad