win x86: Grab bag of restructuring to get tests working on x86-on-x86

A few function implementations that were missing, various switches
for functions/functionality that didn't exist on XP, and far too long
figuring out what exactly was wrong with SYSTEM_PROCESS_INFORMATION
on x86 (the "alignment_for_x86" fields).

R=mark@chromium.org
BUG=crashpad:1, crashpad:50, chromium:531663

Review URL: https://codereview.chromium.org/1336823002 .

snapshot/win/cpu_context_win.cc

@@ -28,48 +28,115 @@ void InitializeX86Context(const WOW64_CONTEXT& context, CPUContextX86* out) {
 }
 
 void InitializeX64Context(const CONTEXT& context, CPUContextX86_64* out) {
-  out->rax = context.Rax;
+  memset(out, 0, sizeof(*out));
-  out->rbx = context.Rbx;
-  out->rcx = context.Rcx;
-  out->rdx = context.Rdx;
-  out->rdi = context.Rdi;
-  out->rsi = context.Rsi;
-  out->rbp = context.Rbp;
-  out->rsp = context.Rsp;
-  out->r8 = context.R8;
-  out->r9 = context.R9;
-  out->r10 = context.R10;
-  out->r11 = context.R11;
-  out->r12 = context.R12;
-  out->r13 = context.R13;
-  out->r14 = context.R14;
-  out->r15 = context.R15;
-  out->rip = context.Rip;
-  out->rflags = context.EFlags;
-  out->cs = context.SegCs;
-  out->fs = context.SegFs;
-  out->gs = context.SegGs;
 
-  out->dr0 = context.Dr0;
+  LOG_IF(ERROR, !(context.ContextFlags & CONTEXT_AMD64)) << "non-x64 context";
-  out->dr1 = context.Dr1;
-  out->dr2 = context.Dr2;
-  out->dr3 = context.Dr3;
-  // DR4 and DR5 are obsolete synonyms for DR6 and DR7, see
-  // http://en.wikipedia.org/wiki/X86_debug_register.
-  out->dr4 = context.Dr6;
-  out->dr5 = context.Dr7;
-  out->dr6 = context.Dr6;
-  out->dr7 = context.Dr7;
 
-  static_assert(sizeof(out->fxsave) == sizeof(context.FltSave),
+  if (context.ContextFlags & CONTEXT_CONTROL) {
-                "types must be equivalent");
+    out->cs = context.SegCs;
-  memcpy(&out->fxsave, &context.FltSave.ControlWord, sizeof(out->fxsave));
+    out->rflags = context.EFlags;
+    out->rip = context.Rip;
+    out->rsp = context.Rsp;
+    // SegSs ignored.
+  }
+
+  if (context.ContextFlags & CONTEXT_INTEGER) {
+    out->rax = context.Rax;
+    out->rbx = context.Rbx;
+    out->rcx = context.Rcx;
+    out->rdx = context.Rdx;
+    out->rdi = context.Rdi;
+    out->rsi = context.Rsi;
+    out->rbp = context.Rbp;
+    out->r8 = context.R8;
+    out->r9 = context.R9;
+    out->r10 = context.R10;
+    out->r11 = context.R11;
+    out->r12 = context.R12;
+    out->r13 = context.R13;
+    out->r14 = context.R14;
+    out->r15 = context.R15;
+  }
+
+  if (context.ContextFlags & CONTEXT_SEGMENTS) {
+    out->fs = context.SegFs;
+    out->gs = context.SegGs;
+    // SegDs ignored.
+    // SegEs ignored.
+  }
+
+  if (context.ContextFlags & CONTEXT_DEBUG_REGISTERS) {
+    out->dr0 = context.Dr0;
+    out->dr1 = context.Dr1;
+    out->dr2 = context.Dr2;
+    out->dr3 = context.Dr3;
+    // DR4 and DR5 are obsolete synonyms for DR6 and DR7, see
+    // https://en.wikipedia.org/wiki/X86_debug_register.
+    out->dr4 = context.Dr6;
+    out->dr5 = context.Dr7;
+    out->dr6 = context.Dr6;
+    out->dr7 = context.Dr7;
+  }
+
+  if (context.ContextFlags & CONTEXT_FLOATING_POINT) {
+    static_assert(sizeof(out->fxsave) == sizeof(context.FltSave),
+                  "types must be equivalent");
+    memcpy(&out->fxsave, &context.FltSave.ControlWord, sizeof(out->fxsave));
+  }
 }
 
 #else  // ARCH_CPU_64_BITS
 
 void InitializeX86Context(const CONTEXT& context, CPUContextX86* out) {
-  CHECK(false) << "TODO(scottmg) InitializeX86Context()";
+  memset(out, 0, sizeof(*out));
+
+  LOG_IF(ERROR, !(context.ContextFlags & CONTEXT_i386)) << "non-x86 context";
+
+  if (context.ContextFlags & CONTEXT_CONTROL) {
+    out->ebp = context.Ebp;
+    out->eip = context.Eip;
+    out->cs = static_cast<uint16_t>(context.SegCs);
+    out->eflags = context.EFlags;
+    out->esp = context.Esp;
+    out->ss = static_cast<uint16_t>(context.SegSs);
+  }
+
+  if (context.ContextFlags & CONTEXT_INTEGER) {
+    out->eax = context.Eax;
+    out->ebx = context.Ebx;
+    out->ecx = context.Ecx;
+    out->edx = context.Edx;
+    out->edi = context.Edi;
+    out->esi = context.Esi;
+  }
+
+  if (context.ContextFlags & CONTEXT_SEGMENTS) {
+    out->ds = static_cast<uint16_t>(context.SegDs);
+    out->es = static_cast<uint16_t>(context.SegEs);
+    out->fs = static_cast<uint16_t>(context.SegFs);
+    out->gs = static_cast<uint16_t>(context.SegGs);
+  }
+
+  if (context.ContextFlags & CONTEXT_DEBUG_REGISTERS) {
+    out->dr0 = context.Dr0;
+    out->dr1 = context.Dr1;
+    out->dr2 = context.Dr2;
+    out->dr3 = context.Dr3;
+    // DR4 and DR5 are obsolete synonyms for DR6 and DR7, see
+    // https://en.wikipedia.org/wiki/X86_debug_register.
+    out->dr4 = context.Dr6;
+    out->dr5 = context.Dr7;
+    out->dr6 = context.Dr6;
+    out->dr7 = context.Dr7;
+  }
+
+  if (context.ContextFlags & CONTEXT_EXTENDED_REGISTERS) {
+    static_assert(sizeof(out->fxsave) == sizeof(context.ExtendedRegisters),
+                  "types must be equivalent");
+    memcpy(&out->fxsave, &context.ExtendedRegisters, sizeof(out->fxsave));
+  } else if (context.ContextFlags & CONTEXT_FLOATING_POINT) {
+    CHECK(false) << "TODO(scottmg): extract x87 data";
+  }
 }
 
 #endif  // ARCH_CPU_64_BITS

snapshot/win/cpu_context_win_test.cc

@@ -27,10 +27,12 @@ namespace {
 #if defined(ARCH_CPU_X86_64)
 
 TEST(CPUContextWin, InitializeX64Context) {
-  CONTEXT context;
+  CONTEXT context = {0};
   context.Rax = 10;
   context.FltSave.TagWord = 11;
   context.Dr0 = 12;
+  context.ContextFlags =
+      CONTEXT_INTEGER | CONTEXT_FLOATING_POINT | CONTEXT_DEBUG_REGISTERS;
 
   // Test the simple case, where everything in the CPUContextX86_64 argument is
   // set directly from the supplied thread, float, and debug state parameters.
@@ -43,9 +45,26 @@ TEST(CPUContextWin, InitializeX64Context) {
   }
 }
 
-#else  // ARCH_CPU_X86_64
+#else
 
-#error ARCH_CPU_X86
+TEST(CPUContextWin, InitializeX86Context) {
+  CONTEXT context = {0};
+  context.ContextFlags =
+      CONTEXT_INTEGER | CONTEXT_EXTENDED_REGISTERS | CONTEXT_DEBUG_REGISTERS;
+  context.Eax = 1;
+  context.ExtendedRegisters[4] = 2;  // FTW.
+  context.Dr0 = 3;
+
+  // Test the simple case, where everything in the CPUContextX86 argument is
+  // set directly from the supplied thread, float, and debug state parameters.
+  {
+    CPUContextX86 cpu_context_x86 = {};
+    InitializeX86Context(context, &cpu_context_x86);
+    EXPECT_EQ(1u, cpu_context_x86.eax);
+    EXPECT_EQ(2u, cpu_context_x86.fxsave.ftw);
+    EXPECT_EQ(3u, cpu_context_x86.dr0);
+  }
+}
 
 #endif  // ARCH_CPU_X86_64
 

snapshot/win/exception_snapshot_win.cc

@@ -69,42 +69,30 @@ bool ExceptionSnapshotWin::Initialize(ProcessReaderWin* process_reader,
     return false;
   }
 
+#if defined(ARCH_CPU_64_BITS)
   if (process_reader->Is64Bit()) {
-    EXCEPTION_RECORD64 first_record;
+    CONTEXT context_record;
-    if (!process_reader->ReadMemory(
+    if (!InitializeFromExceptionPointers<EXCEPTION_RECORD64>(
-            reinterpret_cast<WinVMAddress>(exception_pointers.ExceptionRecord),
+            *process_reader, exception_pointers, &context_record)) {
-            sizeof(first_record),
-            &first_record)) {
-      LOG(ERROR) << "ExceptionRecord";
       return false;
     }
-    exception_code_ = first_record.ExceptionCode;
-    exception_flags_ = first_record.ExceptionFlags;
-    exception_address_ = first_record.ExceptionAddress;
-    for (DWORD i = 0; i < first_record.NumberParameters; ++i)
-      codes_.push_back(first_record.ExceptionInformation[i]);
-    if (first_record.ExceptionRecord) {
-      // https://code.google.com/p/crashpad/issues/detail?id=43
-      LOG(WARNING) << "dropping chained ExceptionRecord";
-    }
-
     context_.architecture = kCPUArchitectureX86_64;
     context_.x86_64 = &context_union_.x86_64;
-    // We assume 64-on-64 here in that we're relying on the CONTEXT definition
-    // to be the x64 one.
-    CONTEXT context_record;
-    if (!process_reader->ReadMemory(
-            reinterpret_cast<WinVMAddress>(exception_pointers.ContextRecord),
-            sizeof(context_record),
-            &context_record)) {
-      LOG(ERROR) << "ContextRecord";
-      return false;
-    }
     InitializeX64Context(context_record, context_.x86_64);
   } else {
-    CHECK(false) << "TODO(scottmg) x86";
+    CHECK(false) << "TODO(scottmg) WOW64";
     return false;
   }
+#else
+  CONTEXT context_record;
+  if (!InitializeFromExceptionPointers<EXCEPTION_RECORD32>(
+          *process_reader, exception_pointers, &context_record)) {
+    return false;
+  }
+  context_.architecture = kCPUArchitectureX86;
+  context_.x86 = &context_union_.x86;
+  InitializeX86Context(context_record, context_.x86);
+#endif  // ARCH_CPU_64_BITS
 
   INITIALIZATION_STATE_SET_VALID(initialized_);
   return true;
@@ -140,5 +128,39 @@ const std::vector<uint64_t>& ExceptionSnapshotWin::Codes() const {
   return codes_;
 }
 
+template <class ExceptionRecordType, class ContextType>
+bool ExceptionSnapshotWin::InitializeFromExceptionPointers(
+    const ProcessReaderWin& process_reader,
+    const EXCEPTION_POINTERS& exception_pointers,
+    ContextType* context_record) {
+  ExceptionRecordType first_record;
+  if (!process_reader.ReadMemory(
+          reinterpret_cast<WinVMAddress>(exception_pointers.ExceptionRecord),
+          sizeof(first_record),
+          &first_record)) {
+    LOG(ERROR) << "ExceptionRecord";
+    return false;
+  }
+  exception_code_ = first_record.ExceptionCode;
+  exception_flags_ = first_record.ExceptionFlags;
+  exception_address_ = first_record.ExceptionAddress;
+  for (DWORD i = 0; i < first_record.NumberParameters; ++i)
+    codes_.push_back(first_record.ExceptionInformation[i]);
+  if (first_record.ExceptionRecord) {
+    // https://code.google.com/p/crashpad/issues/detail?id=43
+    LOG(WARNING) << "dropping chained ExceptionRecord";
+  }
+
+  if (!process_reader.ReadMemory(
+          reinterpret_cast<WinVMAddress>(exception_pointers.ContextRecord),
+          sizeof(*context_record),
+          context_record)) {
+    LOG(ERROR) << "ContextRecord";
+    return false;
+  }
+
+  return true;
+}
+
 }  // namespace internal
 }  // namespace crashpad

snapshot/win/exception_snapshot_win.h

@@ -61,6 +61,12 @@ class ExceptionSnapshotWin final : public ExceptionSnapshot {
   const std::vector<uint64_t>& Codes() const override;
 
  private:
+  template <class ExceptionRecordType, class ContextType>
+  bool InitializeFromExceptionPointers(
+      const ProcessReaderWin& process_reader,
+      const EXCEPTION_POINTERS& exception_pointers,
+      ContextType* context_record);
+
 #if defined(ARCH_CPU_X86_FAMILY)
   union {
     CPUContextX86 x86;

snapshot/win/pe_image_reader_test.cc

@@ -14,6 +14,7 @@
 
 #include "snapshot/win/pe_image_reader.h"
 
+#define PSAPI_VERSION 1
 #include <psapi.h>
 
 #include "gtest/gtest.h"
@@ -25,6 +26,17 @@ namespace crashpad {
 namespace test {
 namespace {
 
+BOOL CrashpadGetModuleInformation(HANDLE process,
+                          HMODULE module,
+                          MODULEINFO* module_info,
+                          DWORD cb) {
+  static decltype(GetModuleInformation)* get_module_information =
+      reinterpret_cast<decltype(GetModuleInformation)*>(
+          GetProcAddress(LoadLibrary(L"psapi.dll"), "GetModuleInformation"));
+  DCHECK(get_module_information);
+  return get_module_information(process, module, module_info, cb);
+}
+
 TEST(PEImageReader, DebugDirectory) {
   PEImageReader pe_image_reader;
   ProcessReaderWin process_reader;
@@ -32,7 +44,7 @@ TEST(PEImageReader, DebugDirectory) {
                                         ProcessSuspensionState::kRunning));
   HMODULE self = reinterpret_cast<HMODULE>(&__ImageBase);
   MODULEINFO module_info;
-  ASSERT_TRUE(GetModuleInformation(
+  ASSERT_TRUE(CrashpadGetModuleInformation(
       GetCurrentProcess(), self, &module_info, sizeof(module_info)));
   EXPECT_EQ(self, module_info.lpBaseOfDll);
   EXPECT_TRUE(pe_image_reader.Initialize(&process_reader,

snapshot/win/process_reader_win.cc

@@ -19,6 +19,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/numerics/safe_conversions.h"
 #include "util/win/nt_internals.h"
+#include "util/win/ntstatus_logging.h"
 #include "util/win/process_structs.h"
 #include "util/win/scoped_handle.h"
 #include "util/win/time.h"
@@ -57,10 +58,8 @@ process_types::SYSTEM_PROCESS_INFORMATION<Traits>* GetProcessInformation(
   // This must be in retry loop, as we're racing with process creation on the
   // system to find a buffer large enough to hold all process information.
   for (int tries = 0; tries < 20; ++tries) {
-    const int kSystemExtendedProcessInformation = 57;
     status = crashpad::NtQuerySystemInformation(
-        static_cast<SYSTEM_INFORMATION_CLASS>(
+        SystemProcessInformation,
-            kSystemExtendedProcessInformation),
         reinterpret_cast<void*>(buffer->get()),
         buffer_size,
         &buffer_size);
@@ -77,7 +76,7 @@ process_types::SYSTEM_PROCESS_INFORMATION<Traits>* GetProcessInformation(
   }
 
   if (!NT_SUCCESS(status)) {
-    LOG(ERROR) << "NtQuerySystemInformation failed: " << std::hex << status;
+    NTSTATUS_LOG(ERROR, status) << "NtQuerySystemInformation";
     return nullptr;
   }
 
@@ -95,16 +94,17 @@ process_types::SYSTEM_PROCESS_INFORMATION<Traits>* GetProcessInformation(
 }
 
 template <class Traits>
-HANDLE OpenThread(const process_types::SYSTEM_EXTENDED_THREAD_INFORMATION<
+HANDLE OpenThread(
-    Traits>& thread_info) {
+    const process_types::SYSTEM_THREAD_INFORMATION<Traits>& thread_info) {
   HANDLE handle;
-  ACCESS_MASK query_access = THREAD_GET_CONTEXT | THREAD_SUSPEND_RESUME;
+  ACCESS_MASK query_access =
+      THREAD_GET_CONTEXT | THREAD_SUSPEND_RESUME | THREAD_QUERY_INFORMATION;
   OBJECT_ATTRIBUTES object_attributes;
   InitializeObjectAttributes(&object_attributes, nullptr, 0, nullptr, nullptr);
   NTSTATUS status = crashpad::NtOpenThread(
       &handle, query_access, &object_attributes, &thread_info.ClientId);
   if (!NT_SUCCESS(status)) {
-    LOG(ERROR) << "NtOpenThread failed";
+    NTSTATUS_LOG(ERROR, status) << "NtOpenThread";
     return nullptr;
   }
   return handle;
@@ -114,19 +114,15 @@ HANDLE OpenThread(const process_types::SYSTEM_EXTENDED_THREAD_INFORMATION<
 // side-effect of returning the SuspendCount of the thread on success, so we
 // fill out these two pieces of semi-unrelated data in the same function.
 template <class Traits>
-void FillThreadContextAndSuspendCount(
+bool FillThreadContextAndSuspendCount(HANDLE thread_handle,
-    const process_types::SYSTEM_EXTENDED_THREAD_INFORMATION<Traits>&
+                                      ProcessReaderWin::Thread* thread,
-        thread_info,
+                                      ProcessSuspensionState suspension_state) {
-    ProcessReaderWin::Thread* thread,
-    ProcessSuspensionState suspension_state) {
   // Don't suspend the thread if it's this thread. This is really only for test
   // binaries, as we won't be walking ourselves, in general.
-  bool is_current_thread = thread_info.ClientId.UniqueThread ==
+  bool is_current_thread = thread->id ==
                            reinterpret_cast<process_types::TEB<Traits>*>(
                                NtCurrentTeb())->ClientId.UniqueThread;
 
-  ScopedKernelHANDLE thread_handle(OpenThread(thread_info));
-
   // TODO(scottmg): Handle cross-bitness in this function.
 
   if (is_current_thread) {
@@ -134,10 +130,10 @@ void FillThreadContextAndSuspendCount(
     thread->suspend_count = 0;
     RtlCaptureContext(&thread->context);
   } else {
-    DWORD previous_suspend_count = SuspendThread(thread_handle.get());
+    DWORD previous_suspend_count = SuspendThread(thread_handle);
     if (previous_suspend_count == -1) {
       PLOG(ERROR) << "SuspendThread failed";
-      return;
+      return false;
     }
     DCHECK(previous_suspend_count > 0 ||
            suspension_state == ProcessSuspensionState::kRunning);
@@ -147,15 +143,18 @@ void FillThreadContextAndSuspendCount(
 
     memset(&thread->context, 0, sizeof(thread->context));
     thread->context.ContextFlags = CONTEXT_ALL;
-    if (!GetThreadContext(thread_handle.get(), &thread->context)) {
+    if (!GetThreadContext(thread_handle, &thread->context)) {
       PLOG(ERROR) << "GetThreadContext failed";
-      return;
+      return false;
     }
 
-    if (!ResumeThread(thread_handle.get())) {
+    if (!ResumeThread(thread_handle)) {
       PLOG(ERROR) << "ResumeThread failed";
+      return false;
     }
   }
+
+  return true;
 }
 
 }  // namespace
@@ -198,7 +197,7 @@ bool ProcessReaderWin::Initialize(HANDLE process,
 
 bool ProcessReaderWin::ReadMemory(WinVMAddress at,
                                   WinVMSize num_bytes,
-                                  void* into) {
+                                  void* into) const {
   SIZE_T bytes_read;
   if (!ReadProcessMemory(process_,
                          reinterpret_cast<void*>(at),
@@ -243,50 +242,10 @@ const std::vector<ProcessReaderWin::Thread>& ProcessReaderWin::Threads() {
 
   initialized_threads_ = true;
 
-  DCHECK(threads_.empty());
+  if (process_info_.Is64Bit())
-
+    ReadThreadData<process_types::internal::Traits64>();
-#if ARCH_CPU_32_BITS
+  else
-  using SizeTraits = process_types::internal::Traits32;
+    ReadThreadData<process_types::internal::Traits32>();
-#else
-  using SizeTraits = process_types::internal::Traits64;
-#endif
-  scoped_ptr<uint8_t[]> buffer;
-  process_types::SYSTEM_PROCESS_INFORMATION<SizeTraits>* process_information =
-      GetProcessInformation<SizeTraits>(process_, &buffer);
-  if (!process_information)
-    return threads_;
-
-  for (unsigned long i = 0; i < process_information->NumberOfThreads; ++i) {
-    const process_types::SYSTEM_EXTENDED_THREAD_INFORMATION<SizeTraits>&
-        thread_info = process_information->Threads[i];
-    Thread thread;
-    thread.id = thread_info.ClientId.UniqueThread;
-
-    FillThreadContextAndSuspendCount(thread_info, &thread, suspension_state_);
-
-    // TODO(scottmg): I believe we could reverse engineer the PriorityClass from
-    // the Priority, BasePriority, and
-    // https://msdn.microsoft.com/en-us/library/windows/desktop/ms685100 .
-    // MinidumpThreadWriter doesn't handle it yet in any case, so investigate
-    // both of those at the same time if it's useful.
-    thread.priority_class = NORMAL_PRIORITY_CLASS;
-
-    thread.priority = thread_info.Priority;
-    thread.teb = thread_info.TebBase;
-
-    // While there are semi-documented fields in the thread structure called
-    // StackBase and StackLimit, they don't appear to be correct in practice (or
-    // at least, I don't know how to interpret them). Instead, read the TIB
-    // (Thread Information Block) which is the first element of the TEB, and use
-    // its stack fields.
-    process_types::NT_TIB<SizeTraits> tib;
-    if (ReadMemory(thread_info.TebBase, sizeof(tib), &tib)) {
-      // Note, "backwards" because of direction of stack growth.
-      thread.stack_region_address = tib.StackLimit;
-      thread.stack_region_size = tib.StackBase - tib.StackLimit;
-    }
-    threads_.push_back(thread);
-  }
 
   return threads_;
 }
@@ -301,4 +260,68 @@ const std::vector<ProcessInfo::Module>& ProcessReaderWin::Modules() {
   return modules_;
 }
 
+template <class Traits>
+void ProcessReaderWin::ReadThreadData() {
+  DCHECK(threads_.empty());
+
+  scoped_ptr<uint8_t[]> buffer;
+  process_types::SYSTEM_PROCESS_INFORMATION<Traits>* process_information =
+      GetProcessInformation<Traits>(process_, &buffer);
+  if (!process_information)
+    return;
+
+  for (unsigned long i = 0; i < process_information->NumberOfThreads; ++i) {
+    const process_types::SYSTEM_THREAD_INFORMATION<Traits>& thread_info =
+        process_information->Threads[i];
+    ProcessReaderWin::Thread thread;
+    thread.id = thread_info.ClientId.UniqueThread;
+
+    ScopedKernelHANDLE thread_handle(OpenThread(thread_info));
+    if (!thread_handle.is_valid())
+      continue;
+
+    if (!FillThreadContextAndSuspendCount<Traits>(
+            thread_handle.get(), &thread, suspension_state_)) {
+      continue;
+    }
+
+    // TODO(scottmg): I believe we could reverse engineer the PriorityClass from
+    // the Priority, BasePriority, and
+    // https://msdn.microsoft.com/en-us/library/windows/desktop/ms685100 .
+    // MinidumpThreadWriter doesn't handle it yet in any case, so investigate
+    // both of those at the same time if it's useful.
+    thread.priority_class = NORMAL_PRIORITY_CLASS;
+
+    thread.priority = thread_info.Priority;
+
+    process_types::THREAD_BASIC_INFORMATION<Traits> thread_basic_info;
+    NTSTATUS status = crashpad::NtQueryInformationThread(
+        thread_handle.get(),
+        static_cast<THREADINFOCLASS>(ThreadBasicInformation),
+        &thread_basic_info,
+        sizeof(thread_basic_info),
+        nullptr);
+    if (!NT_SUCCESS(status)) {
+      NTSTATUS_LOG(ERROR, status) << "NtQueryInformationThread";
+      continue;
+    }
+
+    // Read the TIB (Thread Information Block) which is the first element of the
+    // TEB, for its stack fields.
+    process_types::NT_TIB<Traits> tib;
+    if (ReadMemory(thread_basic_info.TebBaseAddress, sizeof(tib), &tib)) {
+      // Note, "backwards" because of direction of stack growth.
+      thread.stack_region_address = tib.StackLimit;
+      if (tib.StackLimit > tib.StackBase) {
+        LOG(ERROR) << "invalid stack range: " << tib.StackBase << " - "
+                   << tib.StackLimit;
+        thread.stack_region_size = 0;
+      } else {
+        thread.stack_region_size = tib.StackBase - tib.StackLimit;
+      }
+    }
+    threads_.push_back(thread);
+  }
+}
+
 }  // namespace crashpad

snapshot/win/process_reader_win.h

@@ -79,7 +79,7 @@ class ProcessReaderWin {
   pid_t ProcessID() const { return process_info_.ProcessID(); }
   pid_t ParentProcessID() const { return process_info_.ParentProcessID(); }
 
-  bool ReadMemory(WinVMAddress at, WinVMSize num_bytes, void* into);
+  bool ReadMemory(WinVMAddress at, WinVMSize num_bytes, void* into) const;
 
   //! \brief Determines the target process' start time.
   //!
@@ -107,6 +107,9 @@ class ProcessReaderWin {
   const std::vector<ProcessInfo::Module>& Modules();
 
  private:
+  template <class Traits>
+  void ReadThreadData();
+
   HANDLE process_;
   ProcessInfo process_info_;
   std::vector<Thread> threads_;

snapshot/win/process_reader_win_test.cc

@@ -19,6 +19,8 @@
 
 #include "gtest/gtest.h"
 #include "test/win/win_multiprocess.h"
+#include "util/synchronization/semaphore.h"
+#include "util/thread/thread.h"
 #include "util/win/scoped_process_suspend.h"
 
 namespace crashpad {
@@ -104,7 +106,7 @@ TEST(ProcessReaderWin, SelfOneThread) {
   // thread, not exactly one thread.
   ASSERT_GE(threads.size(), 1u);
 
-  EXPECT_EQ(GetThreadId(GetCurrentThread()), threads[0].id);
+  EXPECT_EQ(GetCurrentThreadId(), threads[0].id);
 #if defined(ARCH_CPU_64_BITS)
   EXPECT_NE(0, threads[0].context.Rip);
 #else
@@ -120,14 +122,36 @@ class ProcessReaderChildThreadSuspendCount final : public WinMultiprocess {
   ~ProcessReaderChildThreadSuspendCount() {}
 
  private:
+  enum : unsigned int { kCreatedThreads = 3 };
+
+  class SleepingThread : public Thread {
+   public:
+    SleepingThread() : done_(nullptr) {}
+
+    void SetHandle(Semaphore* done) {
+      done_= done;
+    }
+
+    void ThreadMain() override {
+      done_->Wait();
+    };
+
+   private:
+    Semaphore* done_;
+  };
+
   void WinMultiprocessParent() override {
+    char c;
+    CheckedReadFile(ReadPipeHandle(), &c, sizeof(c));
+    ASSERT_EQ(' ', c);
+
     {
       ProcessReaderWin process_reader;
       ASSERT_TRUE(process_reader.Initialize(ChildProcess(),
                                             ProcessSuspensionState::kRunning));
 
       const auto& threads = process_reader.Threads();
-      ASSERT_FALSE(threads.empty());
+      ASSERT_GE(threads.size(), kCreatedThreads + 1);
       for (const auto& thread : threads)
         EXPECT_EQ(0u, thread.suspend_count);
     }
@@ -142,19 +166,33 @@ class ProcessReaderChildThreadSuspendCount final : public WinMultiprocess {
       // Confirm that thread counts are adjusted correctly for the process being
       // suspended.
       const auto& threads = process_reader.Threads();
-      ASSERT_FALSE(threads.empty());
+      ASSERT_GE(threads.size(), kCreatedThreads + 1);
       for (const auto& thread : threads)
         EXPECT_EQ(0u, thread.suspend_count);
     }
   }
 
   void WinMultiprocessChild() override {
-    WinVMAddress address = reinterpret_cast<WinVMAddress>(kTestMemory);
+    // Create three dummy threads so we can confirm we read successfully read
-    CheckedWriteFile(WritePipeHandle(), &address, sizeof(address));
+    // more than just the main thread.
+    SleepingThread threads[kCreatedThreads];
+    Semaphore done(0);
+    for (auto& thread : threads)
+      thread.SetHandle(&done);
+    for (auto& thread : threads)
+      thread.Start();
+
+    char c = ' ';
+    CheckedWriteFile(WritePipeHandle(), &c, sizeof(c));
 
     // Wait for the parent to signal that it's OK to exit by closing its end of
     // the pipe.
     CheckedReadFileAtEOF(ReadPipeHandle());
+
+    for (int i = 0; i < arraysize(threads); ++i)
+      done.Signal();
+    for (auto& thread : threads)
+      thread.Join();
   }
 
   DISALLOW_COPY_AND_ASSIGN(ProcessReaderChildThreadSuspendCount);

snapshot/win/thread_snapshot_win.cc

@@ -50,7 +50,11 @@ bool ThreadSnapshotWin::Initialize(
         context_.x86);
   }
 #else
-#error ARCH_CPU_X86
+  context_.architecture = kCPUArchitectureX86;
+  context_.x86 = &context_union_.x86;
+  InitializeX86Context(
+      *reinterpret_cast<const CONTEXT*>(&process_reader_thread.context),
+      context_.x86);
 #endif  // ARCH_CPU_X86_64
 
   INITIALIZATION_STATE_SET_VALID(initialized_);

test/win/win_child_process_test.cc

@@ -14,6 +14,7 @@
 
 #include "test/win/win_child_process.h"
 
+#include <stdlib.h>
 #include <windows.h>
 
 #include "base/basictypes.h"
@@ -49,7 +50,7 @@ class TestWinChildProcess final : public WinChildProcess {
   int Run() override {
     int value = ReadInt(ReadPipeHandle());
     WriteInt(WritePipeHandle(), value);
-    return EXIT_SUCCESS;
+    return testing::Test::HasFailure() ? EXIT_FAILURE : EXIT_SUCCESS;
   }
 
   DISALLOW_COPY_AND_ASSIGN(TestWinChildProcess);
@@ -58,7 +59,9 @@ class TestWinChildProcess final : public WinChildProcess {
 TEST(WinChildProcessTest, WinChildProcess) {
   WinChildProcess::EntryPoint<TestWinChildProcess>();
 
-  WinChildProcess::Launch();
+  scoped_ptr<WinChildProcess::Handles> handles = WinChildProcess::Launch();
+  WriteInt(handles->write.get(), 1);
+  ASSERT_EQ(1, ReadInt(handles->read.get()));
 }
 
 TEST(WinChildProcessTest, MultipleChildren) {

util/util_test.gyp

@@ -126,6 +126,7 @@
                 '/BASE:0x78000000',
                 '/FIXED',
               ],
+              'MinimumRequiredVersion': '5.02',  # Server 2003.
               'TargetMachine': '17',  # x64.
             },
           },

util/win/process_info.cc

@@ -17,7 +17,10 @@
 #include <winternl.h>
 
 #include "base/logging.h"
+#include "base/strings/stringprintf.h"
+#include "build/build_config.h"
 #include "util/numeric/safe_assignment.h"
+#include "util/win/ntstatus_logging.h"
 #include "util/win/process_structs.h"
 
 namespace crashpad {
@@ -80,7 +83,8 @@ bool ReadUnicodeString(HANDLE process,
   return true;
 }
 
-template <class T> bool ReadStruct(HANDLE process, WinVMAddress at, T* into) {
+template <class T>
+bool ReadStruct(HANDLE process, WinVMAddress at, T* into) {
   SIZE_T bytes_read;
   if (!ReadProcessMemory(process,
                          reinterpret_cast<const void*>(at),
@@ -101,14 +105,72 @@ template <class T> bool ReadStruct(HANDLE process, WinVMAddress at, T* into) {
 
 }  // namespace
 
+template <class Traits>
+bool GetProcessBasicInformation(HANDLE process,
+                                bool is_wow64,
+                                ProcessInfo* process_info,
+                                WinVMAddress* peb_address) {
+  ULONG bytes_returned;
+  process_types::PROCESS_BASIC_INFORMATION<Traits> process_basic_information;
+  NTSTATUS status =
+      crashpad::NtQueryInformationProcess(process,
+                                          ProcessBasicInformation,
+                                          &process_basic_information,
+                                          sizeof(process_basic_information),
+                                          &bytes_returned);
+  if (!NT_SUCCESS(status)) {
+    NTSTATUS_LOG(ERROR, status) << "NtQueryInformationProcess";
+    return false;
+  }
+  if (bytes_returned != sizeof(process_basic_information)) {
+    LOG(ERROR) << "NtQueryInformationProcess incorrect size";
+    return false;
+  }
+
+  // See https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203 on
+  // 32 bit being the correct size for HANDLEs for proceses, even on Windows
+  // x64. API functions (e.g. OpenProcess) take only a DWORD, so there's no
+  // sense in maintaining the top bits.
+  process_info->process_id_ =
+      static_cast<DWORD>(process_basic_information.UniqueProcessId);
+  process_info->inherited_from_process_id_ = static_cast<DWORD>(
+      process_basic_information.InheritedFromUniqueProcessId);
+
+  // We now want to read the PEB to gather the rest of our information. The
+  // PebBaseAddress as returned above is what we want for 64-on-64 and 32-on-32,
+  // but for Wow64, we want to read the 32 bit PEB (a Wow64 process has both).
+  // The address of this is found by a second call to NtQueryInformationProcess.
+  if (!is_wow64) {
+    *peb_address = process_basic_information.PebBaseAddress;
+  } else {
+    ULONG_PTR wow64_peb_address;
+    status = crashpad::NtQueryInformationProcess(process,
+                                                 ProcessWow64Information,
+                                                 &wow64_peb_address,
+                                                 sizeof(wow64_peb_address),
+                                                 &bytes_returned);
+    if (!NT_SUCCESS(status)) {
+      NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess";
+      return false;
+    }
+    if (bytes_returned != sizeof(wow64_peb_address)) {
+      LOG(ERROR) << "NtQueryInformationProcess incorrect size";
+      return false;
+    }
+    *peb_address = wow64_peb_address;
+  }
+
+  return true;
+}
+
 template <class Traits>
 bool ReadProcessData(HANDLE process,
                      WinVMAddress peb_address_vmaddr,
                      ProcessInfo* process_info) {
   Traits::Pointer peb_address;
   if (!AssignIfInRange(&peb_address, peb_address_vmaddr)) {
-    LOG(ERROR) << "peb_address_vmaddr " << peb_address_vmaddr
+    LOG(ERROR) << base::StringPrintf("peb address 0x%x out of range",
-               << " out of range";
+                                     peb_address_vmaddr);
     return false;
   }
 
@@ -232,73 +294,28 @@ bool ProcessInfo::Initialize(HANDLE process) {
   }
 #endif
 
-  ULONG bytes_returned;
+  WinVMAddress peb_address;
-  // We assume this process is not running on Wow64. The
+#if ARCH_CPU_64_BITS
-  // PROCESS_BASIC_INFORMATION uses the OS size (that is, even Wow64 has a 64
+  bool result = GetProcessBasicInformation<process_types::internal::Traits64>(
-  // bit one.)
+      process, is_wow64_, this, &peb_address);
-  // TODO(scottmg): Either support running as Wow64, or check/resolve this at a
-  // higher level.
-#if ARCH_CPU_32_BITS
-  process_types::PROCESS_BASIC_INFORMATION<process_types::internal::Traits32>
-      process_basic_information;
 #else
-  process_types::PROCESS_BASIC_INFORMATION<process_types::internal::Traits64>
+  bool result = GetProcessBasicInformation<process_types::internal::Traits32>(
-      process_basic_information;
+      process, false, this, &peb_address);
-#endif
+#endif  // ARCH_CPU_64_BITS
-  NTSTATUS status =
+
-      crashpad::NtQueryInformationProcess(process,
+  if (!result) {
-                                          ProcessBasicInformation,
+    LOG(ERROR) << "GetProcessBasicInformation failed";
-                                          &process_basic_information,
-                                          sizeof(process_basic_information),
-                                          &bytes_returned);
-  if (status < 0) {
-    LOG(ERROR) << "NtQueryInformationProcess: status=" << status;
-    return false;
-  }
-  if (bytes_returned != sizeof(process_basic_information)) {
-    LOG(ERROR) << "NtQueryInformationProcess incorrect size";
     return false;
   }
 
-  // See https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203 on
+  result = is_64_bit_ ? ReadProcessData<process_types::internal::Traits64>(
-  // 32 bit being the correct size for HANDLEs for proceses, even on Windows
-  // x64. API functions (e.g. OpenProcess) take only a DWORD, so there's no
-  // sense in maintaining the top bits.
-  process_id_ = static_cast<DWORD>(process_basic_information.UniqueProcessId);
-  inherited_from_process_id_ = static_cast<DWORD>(
-      process_basic_information.InheritedFromUniqueProcessId);
-
-  // We now want to read the PEB to gather the rest of our information. The
-  // PebBaseAddress as returned above is what we want for 64-on-64 and 32-on-32,
-  // but for Wow64, we want to read the 32 bit PEB (a Wow64 process has both).
-  // The address of this is found by a second call to NtQueryInformationProcess.
-  WinVMAddress peb_address = process_basic_information.PebBaseAddress;
-  if (is_wow64_) {
-    ULONG_PTR wow64_peb_address;
-    status =
-        crashpad::NtQueryInformationProcess(process,
-                                            ProcessWow64Information,
-                                            &wow64_peb_address,
-                                            sizeof(wow64_peb_address),
-                                            &bytes_returned);
-    if (status < 0) {
-      LOG(ERROR) << "NtQueryInformationProcess: status=" << status;
-      return false;
-    }
-    if (bytes_returned != sizeof(wow64_peb_address)) {
-      LOG(ERROR) << "NtQueryInformationProcess incorrect size";
-      return false;
-    }
-    peb_address = wow64_peb_address;
-  }
-
-  // Read the PEB data using the correct word size.
-  bool result = is_64_bit_ ? ReadProcessData<process_types::internal::Traits64>(
                                  process, peb_address, this)
                            : ReadProcessData<process_types::internal::Traits32>(
                                  process, peb_address, this);
-  if (!result)
+  if (!result) {
+    LOG(ERROR) << "ReadProcessData failed";
     return false;
+  }
 
   INITIALIZATION_STATE_SET_VALID(initialized_);
   return true;

util/win/process_info.h

@@ -86,7 +86,12 @@ class ProcessInfo {
   bool Modules(std::vector<Module>* modules) const;
 
  private:
-  template <class T>
+  template <class Traits>
+  friend bool GetProcessBasicInformation(HANDLE process,
+                                         bool is_wow64,
+                                         ProcessInfo* process_info,
+                                         WinVMAddress* peb_address);
+  template <class Traits>
   friend bool ReadProcessData(HANDLE process,
                               WinVMAddress peb_address_vmaddr,
                               ProcessInfo* process_info);

util/win/process_structs.h

@@ -311,36 +311,70 @@ struct TEB {
   CLIENT_ID<Traits> ClientId;
 };
 
-// See https://msdn.microsoft.com/en-us/library/gg750724.aspx for the base
+// See https://msdn.microsoft.com/en-us/library/gg750724.aspx.
-// structure, and
-// http://processhacker.sourceforge.net/doc/struct___s_y_s_t_e_m___e_x_t_e_n_d_e_d___t_h_r_e_a_d___i_n_f_o_r_m_a_t_i_o_n.html
-// for the extension part.
 template <class Traits>
-struct SYSTEM_EXTENDED_THREAD_INFORMATION {
+struct SYSTEM_THREAD_INFORMATION {
-  LARGE_INTEGER KernelTime;
-  LARGE_INTEGER UserTime;
-  LARGE_INTEGER CreateTime;
   union {
-    ULONG WaitTime;
+    struct {
-    typename Traits::Pad padding_for_x64_0;
+      LARGE_INTEGER KernelTime;
+      LARGE_INTEGER UserTime;
+      LARGE_INTEGER CreateTime;
+      union {
+        ULONG WaitTime;
+        typename Traits::Pad padding_for_x64_0;
+      };
+      typename Traits::Pointer StartAddress;
+      CLIENT_ID<Traits> ClientId;
+      LONG Priority;
+      LONG BasePriority;
+      ULONG ContextSwitches;
+      ULONG ThreadState;
+      union {
+        ULONG WaitReason;
+        typename Traits::Pad padding_for_x64_1;
+      };
+    };
+    LARGE_INTEGER alignment_for_x86[8];
   };
-  typename Traits::Pointer StartAddress;
+};
-  CLIENT_ID<Traits> ClientId;
+
-  LONG Priority;
+// There's an extra field in the x64 VM_COUNTERS (or maybe it's VM_COUNTERS_EX,
-  LONG BasePriority;
+// it's not clear), so we just make separate specializations for 32/64.
-  ULONG ContextSwitches;
+template <class Traits>
-  ULONG ThreadState;
+class VM_COUNTERS {};
+
+template <>
+class VM_COUNTERS<internal::Traits32> {
+  SIZE_T PeakVirtualSize;
+  SIZE_T VirtualSize;
+  ULONG PageFaultCount;
+  SIZE_T PeakWorkingSetSize;
+  SIZE_T WorkingSetSize;
+  SIZE_T QuotaPeakPagedPoolUsage;
+  SIZE_T QuotaPagedPoolUsage;
+  SIZE_T QuotaPeakNonPagedPoolUsage;
+  SIZE_T QuotaNonPagedPoolUsage;
+  SIZE_T PagefileUsage;
+  SIZE_T PeakPagefileUsage;
+};
+
+template <>
+class VM_COUNTERS<internal::Traits64> {
+  SIZE_T PeakVirtualSize;
+  SIZE_T VirtualSize;
   union {
-    ULONG WaitReason;
+    ULONG PageFaultCount;
-    typename Traits::Pad padding_for_x64_1;
+    internal::Traits64::Pad padding_for_x64;
   };
-  typename Traits::Pointer StackBase;  // These don't appear to be correct.
+  SIZE_T PeakWorkingSetSize;
-  typename Traits::Pointer StackLimit;
+  SIZE_T WorkingSetSize;
-  typename Traits::Pointer Win32StartAddress;
+  SIZE_T QuotaPeakPagedPoolUsage;
-  typename Traits::Pointer TebBase;
+  SIZE_T QuotaPagedPoolUsage;
-  typename Traits::Pointer Reserved;
+  SIZE_T QuotaPeakNonPagedPoolUsage;
-  typename Traits::Pointer Reserved2;
+  SIZE_T QuotaNonPagedPoolUsage;
-  typename Traits::Pointer Reserved3;
+  SIZE_T PagefileUsage;
+  SIZE_T PeakPagefileUsage;
+  SIZE_T PrivateUsage;
 };
 
 // See http://undocumented.ntinternals.net/source/usermode/undocumented%20functions/system%20information/structures/system_process_information.html
@@ -348,7 +382,10 @@ template <class Traits>
 struct SYSTEM_PROCESS_INFORMATION {
   ULONG NextEntryOffset;
   ULONG NumberOfThreads;
-  LARGE_INTEGER Reserved[3];
+  LARGE_INTEGER WorkingSetPrivateSize;
+  ULONG HardFaultCount;
+  ULONG NumberOfThreadsHighWatermark;
+  ULONGLONG CycleTime;
   LARGE_INTEGER CreateTime;
   LARGE_INTEGER UserTime;
   LARGE_INTEGER KernelTime;
@@ -366,29 +403,28 @@ struct SYSTEM_PROCESS_INFORMATION {
     typename Traits::Pad padding_for_x64_2;
   };
   ULONG HandleCount;
-  ULONG Reserved2[3];
+  ULONG SessionId;
-  SIZE_T PeakVirtualSize;
+  typename Traits::Pointer UniqueProcessKey;
-  SIZE_T VirtualSize;
   union {
-    ULONG PageFaultCount;
+    VM_COUNTERS<Traits> VirtualMemoryCounters;
-    typename Traits::Pad padding_for_x64_3;
+    LARGE_INTEGER alignment_for_x86[6];
   };
-  SIZE_T PeakWorkingSetSize;
+  IO_COUNTERS IoCounters;
-  SIZE_T WorkingSetSize;
+  SYSTEM_THREAD_INFORMATION<Traits> Threads[1];
-  SIZE_T QuotaPeakPagedPoolUsage;
+};
-  SIZE_T QuotaPagedPoolUsage;
+
-  SIZE_T QuotaPeakNonPagedPoolUsage;
+// http://undocumented.ntinternals.net/source/usermode/structures/thread_basic_information.html
-  SIZE_T QuotaNonPagedPoolUsage;
+template <class Traits>
-  SIZE_T PagefileUsage;
+struct THREAD_BASIC_INFORMATION {
-  SIZE_T PeakPagefileUsage;
+  union {
-  SIZE_T PrivatePageCount;
+    NTSTATUS ExitStatus;
-  LARGE_INTEGER ReadOperationCount;
+    typename Traits::Pad padding_for_x64_0;
-  LARGE_INTEGER WriteOperationCount;
+  };
-  LARGE_INTEGER OtherOperationCount;
+  typename Traits::Pointer TebBaseAddress;
-  LARGE_INTEGER ReadTransferCount;
+  CLIENT_ID<Traits> ClientId;
-  LARGE_INTEGER WriteTransferCount;
+  typename Traits::Pointer AffinityMask;
-  LARGE_INTEGER OtherTransferCount;
+  ULONG Priority;
-  SYSTEM_EXTENDED_THREAD_INFORMATION<Traits> Threads[1];
+  LONG BasePriority;
 };
 
 #pragma pack(pop)