win: Save contents of TEBs allowing !teb and !gle to work in windbg

snapshot/win/thread_snapshot_win.cc

Index: snapshot/win/thread_snapshot_win.cc
diff --git a/snapshot/win/thread_snapshot_win.cc b/snapshot/win/thread_snapshot_win.cc
index ce4b309b9d97972fa529be6ac7383e546c2f4139..4093700b4a21349ee4ce94d7e52087e1f2ad50af 100644
--- a/snapshot/win/thread_snapshot_win.cc
+++ b/snapshot/win/thread_snapshot_win.cc
@@ -14,6 +14,8 @@
 
 #include "snapshot/win/thread_snapshot_win.h"
 
+#include <vector>
+
 #include "base/logging.h"
 #include "snapshot/win/cpu_context_win.h"
 #include "snapshot/win/process_reader_win.h"
@@ -22,7 +24,12 @@ namespace crashpad {
 namespace internal {
 
 ThreadSnapshotWin::ThreadSnapshotWin()
-    : ThreadSnapshot(), context_(), stack_(), thread_(), initialized_() {
+    : ThreadSnapshot(),
+      context_(),
+      stack_(),
+      teb_(),
+      thread_(),
+      initialized_() {
 }
 
 ThreadSnapshotWin::~ThreadSnapshotWin() {
@@ -36,6 +43,7 @@ bool ThreadSnapshotWin::Initialize(
   thread_ = process_reader_thread;
   stack_.Initialize(
       process_reader, thread_.stack_region_address, thread_.stack_region_size);
+  teb_.Initialize(process_reader, thread_.teb_address, thread_.teb_size);
 
 #if defined(ARCH_CPU_X86_64)
   if (process_reader->Is64Bit()) {
@@ -84,7 +92,14 @@ int ThreadSnapshotWin::Priority() const {
 
 uint64_t ThreadSnapshotWin::ThreadSpecificDataAddress() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  return thread_.teb;
+  return thread_.teb_address;
+}
+
+std::vector<const MemorySnapshot*> ThreadSnapshotWin::ExtraMemory() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  std::vector<const MemorySnapshot*> extra_memory;

[Mark Mentovai, 2015/10/01 20:28:05]

(1, &teb_)

[scottmg, 2015/10/01 21:01:07]

Done.

+  extra_memory.push_back(&teb_);

[Mark Mentovai, 2015/10/01 20:28:05]

I’ll give you a pass on this for now, as long as you leave behind a link to
crashpad:59.

I guess I’m a little more fearful of this than you. Since a process’ address
space is basically within its own control, it can be busted in pretty much any
way imaginable, intentionally or not. Yeah, it’s true that only a sick thread
would have a screwed-up TEB, but we’re in the crash-reporting business, so we
can’t just throw our hands up in the air because a thread or process is sick.
“Unreadable TEB” may wind up being the reason for a crash some day.

I mention “intentionally” because we should also be aware that garbageware that
injects itself into Chrome might try to break internal structures to prevent it
from being debugged effectively, or to prevent crash dumps from being sent at
all. We’ve got a very strong interest in minimizing that attack area.

Anyway, yeah, we do need to be able to read the TEB ourselves to do our job
here, but we only consult a few of its early fields, and if they’re messed up,
we only bail on reporting the thread altogether. Maybe we should report
something for the thread in that case too, but it’s kind of important that we
don’t toss the whole dump out. With this change, a problem later in the TEB
would be fatal for the dump as a whole, but the thread itself might not appear
to have anything wrong with it until it attempted a TLS operation.

[scottmg, 2015/10/01 21:01:07]

I'm probably not fearful enough. Added TODO links. 

+  return extra_memory;
 }
 
 }  // namespace internal