win: Add more memory regions to gathering of PEB

snapshot/win/process_snapshot_win.cc

Index: snapshot/win/process_snapshot_win.cc
diff --git a/snapshot/win/process_snapshot_win.cc b/snapshot/win/process_snapshot_win.cc
index b9df62c0ab4c011109ab9054f5b556141958d658..f42f44ca6a119c269c4f89cfd879e81c9f2aa80d 100644
--- a/snapshot/win/process_snapshot_win.cc
+++ b/snapshot/win/process_snapshot_win.cc
@@ -14,6 +14,8 @@
 
 #include "snapshot/win/process_snapshot_win.h"
 
+#include <algorithm>
+
 #include "base/logging.h"
 #include "snapshot/win/module_snapshot_win.h"
 #include "util/win/registration_protocol_win.h"
@@ -48,10 +50,11 @@ bool ProcessSnapshotWin::Initialize(HANDLE process,
     return false;
 
   system_.Initialize(&process_reader_);
-  WinVMAddress peb_address;
-  WinVMSize peb_size;
-  process_reader_.GetProcessInfo().Peb(&peb_address, &peb_size);
-  peb_.Initialize(&process_reader_, peb_address, peb_size);
+
+  if (process_reader_.Is64Bit())
+    InitializePebData<process_types::internal::Traits64>();
+  else
+    InitializePebData<process_types::internal::Traits32>();
 
   InitializeThreads();
   InitializeModules();
@@ -186,7 +189,8 @@ const ExceptionSnapshot* ProcessSnapshotWin::Exception() const {
 std::vector<const MemorySnapshot*> ProcessSnapshotWin::ExtraMemory() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   std::vector<const MemorySnapshot*> extra_memory;
-  extra_memory.push_back(&peb_);
+  for (const auto& peb_memory : peb_memory_)
+    extra_memory.push_back(peb_memory);
   return extra_memory;
 }
 
@@ -214,4 +218,66 @@ void ProcessSnapshotWin::InitializeModules() {
   }
 }
 
+template <class Traits>
+void ProcessSnapshotWin::InitializePebData() {
+  WinVMAddress peb_address;
+  WinVMSize peb_size;
+  process_reader_.GetProcessInfo().Peb(&peb_address, &peb_size);
+  AddMemorySnapshot(peb_address, peb_size, &peb_memory_);
+
+  process_types::PEB<Traits> peb_data;
+  if (!process_reader_.ReadMemory(peb_address, peb_size, &peb_data)) {
+    LOG(ERROR) << "ReadMemory PEB";
+    return;
+  }
+  AddMemorySnapshot(
+      peb_data.Ldr, sizeof(process_types::PEB_LDR_DATA<Traits>), &peb_memory_);
+
+  process_types::RTL_USER_PROCESS_PARAMETERS<Traits> process_parameters;
+  if (!process_reader_.ReadMemory(peb_data.ProcessParameters,
+                                  sizeof(process_parameters),
+                                  &process_parameters)) {
+    LOG(ERROR) << "ReadMemory RTL_USER_PROCESS_PARAMETERS";
+    return;
+  }
+
+  AddMemorySnapshotForUNICODE_STRING(
+      process_parameters.CurrentDirectory.DosPath, &peb_memory_);
+  AddMemorySnapshotForUNICODE_STRING(process_parameters.DllPath, &peb_memory_);
+  AddMemorySnapshotForUNICODE_STRING(process_parameters.ImagePathName,
+                                     &peb_memory_);
+  AddMemorySnapshotForUNICODE_STRING(process_parameters.CommandLine,
+                                     &peb_memory_);
+  // http://blogs.msdn.com/b/oldnewthing/archive/2010/02/03/9957320.aspx On
+  // newer OSs there's no stated limit, but in practice grabbing 32k should be
+  // more than enough. TODO(scottmg): Determine how big the enviroment block
+  // really is, somehow.
+  const int kMaxEnvironmentBlockSize = 32768;

[Mark Mentovai, 2015/09/25 23:11:59]

32,768 bytes or 32,768 characters? All of the references seem to say characters.

[scottmg, 2015/09/26 00:07:38]

You're right, characters. (That would make more sense as a limit too for
wchar_ts in a short.)

+  AddMemorySnapshot(
+      process_parameters.Environment, kMaxEnvironmentBlockSize, &peb_memory_);
+}
+
+void ProcessSnapshotWin::AddMemorySnapshot(
+    WinVMAddress address,
+    WinVMAddress size,
+    PointerVector<internal::MemorySnapshotWin>* into) {
+  if (size == 0)
+    return;
+  internal::MemorySnapshotWin* memory_snapshot =
+      new internal::MemorySnapshotWin();
+  memory_snapshot->Initialize(&process_reader_, address, size);
+  into->push_back(memory_snapshot);
+}
+
+template <class Traits>
+void ProcessSnapshotWin::AddMemorySnapshotForUNICODE_STRING(
+    const process_types::UNICODE_STRING<Traits>& us,
+    PointerVector<internal::MemorySnapshotWin>* into) {
+  AddMemorySnapshot(
+      us.Buffer,
+      std::min(static_cast<USHORT>(us.Length + sizeof(wchar_t)),

[Mark Mentovai, 2015/09/25 23:11:59]

Y’know, on the second thought, the documentation for UNICODE_STRING actually
says that it may not be NUL-terminated at all. So maybe this should just be
us.Length without the std::min or the extra wchar_t.

If you do that, is windbg OK with reading empty strings that don’t have any
memory or NULs to point to?

[scottmg, 2015/09/26 00:07:38]

It seems fine.

+               us.MaximumLength),
+      into);
+}
+
 }  // namespace crashpad