win: Add thread snapshot and memory snapshot for stacks

snapshot/win/process_reader_win.cc

Index: snapshot/win/process_reader_win.cc
diff --git a/snapshot/win/process_reader_win.cc b/snapshot/win/process_reader_win.cc
index 84e176b8739dcdee035210dcdeb6850d44d12db6..4775bceb1156530b4ed8a9cd1ddc91d9a4ad4118 100644
--- a/snapshot/win/process_reader_win.cc
+++ b/snapshot/win/process_reader_win.cc
@@ -14,11 +14,126 @@
 
 #include "snapshot/win/process_reader_win.h"
 
+#include <winternl.h>
+
+#include "base/memory/scoped_ptr.h"
 #include "base/numerics/safe_conversions.h"
+#include "util/win/process_structs.h"
 #include "util/win/time.h"
 
 namespace crashpad {
 
+namespace {
+
+NTSTATUS NtQuerySystemInformation(
+    SYSTEM_INFORMATION_CLASS system_information_class,
+    PVOID system_information,
+    ULONG system_information_length,
+    PULONG return_length) {
+  static decltype(::NtQuerySystemInformation)* nt_query_system_information =
+      reinterpret_cast<decltype(::NtQuerySystemInformation)*>(GetProcAddress(
+          LoadLibrary(L"ntdll.dll"), "NtQuerySystemInformation"));
+  DCHECK(nt_query_system_information);
+  return nt_query_system_information(system_information_class,
+                                     system_information,
+                                     system_information_length,
+                                     return_length);
+}
+
+// Copied from ntstatus.h because um/winnt.h conflicts with general inclusion of
+// ntstatus.h.
+#define STATUS_BUFFER_TOO_SMALL          ((NTSTATUS)0xC0000023L)

[Mark Mentovai, 2015/05/08 16:54:20]

Drop the extra whitespace. Even though it’s just a copy-paste job, it leads to
ugliness if we later needed to copy-paste something like, say,
STATUS_PRIMARY_TRANSPORT_CONNECT_FAILED.

[scottmg, 2015/05/08 18:24:01]

Done.

+#define STATUS_INFO_LENGTH_MISMATCH      ((NTSTATUS)0xC0000004L)
+
+// Kernel mode enumerations for thread state.
+enum class KTHREAD_STATE : uint32_t {

[Mark Mentovai, 2015/05/08 16:54:19]

If this isn’t supposed to match names defined elsewhere (and I don’t think it
is), use something more like KThreadState, kInitialized, etc. per our style
guide.

[Mark Mentovai, 2015/05/08 16:54:19]

If there’s a good reference for where this came from, put it in a comment.

[scottmg, 2015/05/08 18:24:01]

Done.

[scottmg, 2015/05/08 18:24:02]

Done.

+  Initialized,
+  Ready,
+  Running,
+  Standby,
+  Terminated,
+  Waiting,
+  Transition,
+  DeferredReady,
+  GateWait,
+  MaximumThreadState

[Mark Mentovai, 2015/05/08 16:54:20]

I don’t like these “max” things in enums unless there’s a good reason, because
when you switch over an enum, it becomes a valid value that you need to handle
or have a default: for. You don’t do that here, but you also don’t use this
value at all.

[scottmg, 2015/05/08 18:24:02]

Done.

+};
+
+// Gets a pointer to the process information structure after a given one, or
+// null when iteration is complete, assuming they've been retrieved in a block
+// via NtQuerySystemInformation().
+template <class Traits>
+process_types::SYSTEM_PROCESS_INFORMATION<Traits>* NextProcess(
+    process_types::SYSTEM_PROCESS_INFORMATION<Traits>* process) {
+  ULONG offset = process->NextEntryOffset;
+  if (offset == 0)
+    return nullptr;
+  return reinterpret_cast<process_types::SYSTEM_PROCESS_INFORMATION<Traits>*>(
+      reinterpret_cast<uint8_t*>(process) + offset);

[Mark Mentovai, 2015/05/08 16:54:20]

Assure no overflow. Ideally, this function would know where the area returned by
NtQuerySystemInformation() ended, and would refuse to walk outside of that area.
It may be easier for you to do this check in GetProcessInformation() proper.

[Mark Mentovai, 2015/05/08 18:14:14]

Mark Mentovai wrote:
On the second thought, don’t do any of that.

When I initially read this, I thought for some reason that this information was
coming from user space (like ReadProcessMemory()) and I wanted to guard against
corruption. But it comes from kernel space, and I think we should be able to
trust the offsets without paranoia, absent evidence to the contrary.

[scottmg, 2015/05/08 18:24:02]

Done.

[scottmg, 2015/05/08 18:27:10]

(Un)Done.

+}
+
+//! \brief Retrieves the SYSTEM_PROCESS_INFORMATION for a given process.
+//!
+//! The returned pointer points into the memory block stored by \a buffer.
+//! Ownership of \a buffer is transferred to the caller.
+//!
+//! \return Pointer to the process' data, or nullptr if it was not found or on
+//! error. On error, a message will be logged.

[Mark Mentovai, 2015/05/08 16:54:20]

Hanging indent.

[scottmg, 2015/05/08 18:24:01]

Done.

+template <class Traits>
+process_types::SYSTEM_PROCESS_INFORMATION<Traits>* GetProcessInformation(
+    HANDLE process_handle,
+    scoped_ptr<uint8_t[]>* buffer) {
+  ULONG buffer_size = 16384;
+  buffer->reset(new uint8_t[buffer_size]);
+  NTSTATUS status;
+  // 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>(
+            kSystemExtendedProcessInformation),
+        reinterpret_cast<void*>(buffer->get()),
+        buffer_size,
+        &buffer_size);
+    if (status == STATUS_BUFFER_TOO_SMALL ||
+        status == STATUS_INFO_LENGTH_MISMATCH) {
+      buffer->reset(new uint8_t[buffer_size]);

[Mark Mentovai, 2015/05/08 16:54:19]

Maybe leave a little extra slop in here so that if the size grows a little bit
before the next call, there’s a better chance of accommodating it.

[scottmg, 2015/05/08 18:24:01]

Done.

+    } else {
+      break;
+    }
+  }
+
+  if (!NT_SUCCESS(status)) {
+    LOG(ERROR) << "NtQuerySystemInformation failed: " << status;

[Mark Mentovai, 2015/05/08 16:54:20]

These are more sensible to print in hex, although the stream operations to set
that up are a pain.

[scottmg, 2015/05/08 18:24:02]

Done.

+    return nullptr;
+  }
+
+  process_types::SYSTEM_PROCESS_INFORMATION<Traits>* process =
+      reinterpret_cast<process_types::SYSTEM_PROCESS_INFORMATION<Traits>*>(
+          buffer->get());
+  DWORD process_id = GetProcessId(process_handle);
+  do {
+    if (process->UniqueProcessId == process_id)
+      return process;
+  } while (process = NextProcess(process));
+
+  LOG(ERROR) << "process " << process_id << " not found";
+  return nullptr;
+}
+
+}  // namespace
+
+ProcessReaderWin::Thread::Thread()
+    : id(0),
+      suspend_count(0),
+      priority_class(0),
+      priority(0),
+      teb(0),
+      stack_region_address(0),
+      stack_region_size(0) {
+}
+
 ProcessReaderWin::ProcessReaderWin()
     : process_(INVALID_HANDLE_VALUE),
       process_info_(),
@@ -35,6 +150,8 @@ bool ProcessReaderWin::Initialize(HANDLE process) {
   process_ = process;
   process_info_.Initialize(process);
 
+  InitializeThreads();

[Mark Mentovai, 2015/05/08 16:54:20]

You can do this in Threads() at the expense of another member variable to track
whether it’s been called already.

[scottmg, 2015/05/08 18:24:01]

Better? I guess? Done.

+
   INITIALIZATION_STATE_SET_VALID(initialized_);
   return true;
 }
@@ -77,6 +194,11 @@ bool ProcessReaderWin::CPUTimes(timeval* user_time,
   return true;
 }
 
+const std::vector<ProcessReaderWin::Thread>& ProcessReaderWin::Threads() {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return threads_;
+}
+
 const std::vector<ProcessInfo::Module>& ProcessReaderWin::Modules() {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
 
@@ -87,4 +209,57 @@ const std::vector<ProcessInfo::Module>& ProcessReaderWin::Modules() {
   return modules_;
 }
 
+void ProcessReaderWin::InitializeThreads() {
+  DCHECK(threads_.empty());
+
+#if ARCH_CPU_32_BITS
+  using SizeTraits = process_types::internal::Traits32;
+#else
+  using SizeTraits = process_types::internal::Traits64;
+#endif
+  process_types::SYSTEM_PROCESS_INFORMATION<SizeTraits>* process_information;
+  scoped_ptr<uint8_t[]> buffer;
+  process_information =
+      GetProcessInformation<process_types::internal::Traits64>(process_,

[Mark Mentovai, 2015/05/08 16:54:19]

I don’t think that this will work for ARCH_CPU_32_BITS.

Does GetProcessInformation() return SYSTEM_PROCESS_INFORMATION objects in the
bitness of the caller or the bitness of the processes being described?

[scottmg, 2015/05/08 18:24:01]

Quite right, wouldn't even compile. I was fiddling around getting the structure
layout for x64 and had hardcoded it temporarily to avoid getting confused. I've
tried retrieving from both x64 and x86 processes now, but haven't tried
compiling as x86 recently.

+                                                               &buffer);
+  if (!process_information)
+    return;
+
+  for (unsigned 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;
+    // TODO(scottmg): I'm not sure how to get SuspendCount. A rough version is

[Mark Mentovai, 2015/05/08 16:54:20]

Can you add some blank lines before these comments so that they’re easier to
spot?

[scottmg, 2015/05/08 18:24:02]

Done.

+    // to set a suspended count if not currently Running.

[Mark Mentovai, 2015/05/08 16:54:20]

The ntexapi.h snippets I’ve found online make it seem like you may need to check
WaitReason (if the state is actually waiting) to know that a thread has been
explicitly suspended.

[scottmg, 2015/05/08 18:24:02]

Looking at
https://msdn.microsoft.com/en-us/library/system.diagnostics.processthread.wai...
it seems like ThreadState is the controlling one, and WaitReason is extra info.
But WaitReason is only valid when in the Wait state (as opposed to != Running),
so switched the sense to be == Wait instead.

+    thread.suspend_count =
+        thread_info.ThreadState != static_cast<ULONG>(KTHREAD_STATE::Running)
+            ? 0
+            : 1;
+    // 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;

[Mark Mentovai, 2015/05/08 16:54:20]

And also a blank line after this so it’s obvious that the TODO comment only
applies to priority_class and not to priority or teb.

[scottmg, 2015/05/08 18:24:01]

Done.

+    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

[Mark Mentovai, 2015/05/08 16:54:19]

I’m not totally surprised, the stack is more of a user-space concern. Maybe one
of these fields got recycled for the suspend count. :)

[scottmg, 2015/05/08 18:24:01]

They're funny, they sort of "seem" like the right size and are in plausible
locations for a stack, but they're normally not even mapped into the process'
address space. It might just be a matter of translating them relative to some
base.

+    // 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)) {
+      thread.stack_region_address = 0;

[Mark Mentovai, 2015/05/08 16:54:19]

The constructor already zeroed out these fields.

[scottmg, 2015/05/08 18:24:01]

Oops, yes. Forgot to remove them when I added Thread::Thread(). Done.

+      thread.stack_region_size = 0;
+    } else {
+      thread.stack_region_address = tib.StackBase;
+      // Note, "backwards" because of direction of stack growth.
+      thread.stack_region_size = tib.StackBase - tib.StackLimit;
+    }
+    threads_.push_back(thread);
+  }
+}
+
 }  // namespace crashpad