win: Add more memory regions to gathering of PEB

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 a7665c293aaae99ec34c03dd41f81e230c34aa06..9d27051d3d6aad77c1254865aa72e830fd39c785 100644
--- a/snapshot/win/process_reader_win.cc
+++ b/snapshot/win/process_reader_win.cc
@@ -228,6 +228,25 @@ bool ProcessReaderWin::ReadMemory(WinVMAddress at,
   return true;
 }
 
+WinVMSize ProcessReaderWin::ReadAvailableMemory(WinVMAddress at,
+                                                WinVMSize num_bytes,
+                                                void* into) const {
+  auto ranges = process_info_.GetReadableRanges(
+      CheckedRange<WinVMAddress, WinVMSize>(at, num_bytes));
+  // We only read up until the first unavailable block, so we only read from the

[Mark Mentovai, 2015/10/01 19:05:47]

Put some blank lines before these comments that apply to the things that follow
them but not the things that precede them.

[scottmg, 2015/10/01 20:17:01]

Done.

+  // first range.
+  if (ranges.empty())
+    return 0;

[Mark Mentovai, 2015/10/01 19:05:47]

LOG something on the return-0s that don’t pass through ReadMemory()?

[scottmg, 2015/10/01 20:17:01]

I was thinking since we're using this in a trial-and-error way for probing the
environment block, it would be better if it weren't noisy. But I guess it is
useful to know that not even the first byte was accessible because that probably
means the parent structure pointing here is broken.

[Mark Mentovai, 2015/10/01 22:00:52]

scottmg wrote:
It’s probably reasonable to expect there to be something present where we’re
told the environment block is, and to flag it as an oddity if there isn’t
anything there. It’s good now in the latest patch set.

+  // If the start address was adjusted, we couldn't read even the first
+  // requested byte.
+  if (ranges.front().base() != at)
+    return 0;
+  // If we fail on a normal read, then something went very wrong.
+  if (!ReadMemory(ranges.front().base(), ranges.front().size(), into))

[Mark Mentovai, 2015/10/01 19:05:47]

It’s not strictly necessary because the tests should cover GetReadableRanges’
behavior well enough that this shouldn’t ever happen, but consider:

  DCHECK_LE(ranges.front.size(), num_bytes);

before calling ReadMemory().

[scottmg, 2015/10/01 20:17:01]

Done.

+    return 0;
+  return ranges.front().size();
+}
+
 bool ProcessReaderWin::StartTime(timeval* start_time) const {
   FILETIME creation, exit, kernel, user;
   if (!GetProcessTimes(process_, &creation, &exit, &kernel, &user)) {