win: support x64 reading x86 (wow64)

snapshot/win/exception_snapshot_win.cc

Index: snapshot/win/exception_snapshot_win.cc
diff --git a/snapshot/win/exception_snapshot_win.cc b/snapshot/win/exception_snapshot_win.cc
index 1156baf35d4b37852d20521a6ea91fb87d2b30d0..f6f723ae506b64f3cd205d7af3a898b17655173e 100644
--- a/snapshot/win/exception_snapshot_win.cc
+++ b/snapshot/win/exception_snapshot_win.cc
@@ -17,7 +17,6 @@
 #include "snapshot/win/cpu_context_win.h"
 #include "snapshot/win/process_reader_win.h"
 #include "util/win/nt_internals.h"
-#include "util/win/process_structs.h"
 
 namespace crashpad {
 namespace internal {
@@ -51,26 +50,24 @@ bool ExceptionSnapshotWin::Initialize(ProcessReaderWin* process_reader,
   }
 
   if (!found_thread) {
-    LOG(ERROR) << "thread ID " << thread_id << "not found in process";
+    LOG(ERROR) << "thread ID " << thread_id << " not found in process";
     return false;
   } else {
     thread_id_ = thread_id;
   }
 
-  EXCEPTION_POINTERS exception_pointers;
-  if (!process_reader->ReadMemory(exception_pointers_address,
-                                  sizeof(EXCEPTION_POINTERS),
-                                  &exception_pointers)) {
-    LOG(ERROR) << "EXCEPTION_POINTERS read failed";
-    return false;
-  }
-  if (!exception_pointers.ExceptionRecord) {
-    LOG(ERROR) << "null ExceptionRecord";
-    return false;
-  }
+  using ExceptionPointers32 =
+      process_types::EXCEPTION_POINTERS<process_types::internal::Traits32>;
+  using ExceptionPointers64 =

[Mark Mentovai, 2015/09/18 15:44:06]

Goes in ARCH_CPU_64_BITS?

[scottmg, 2015/09/18 19:45:26]

I moved them to process_structs because they're basically just "missing" as
compared with the availability of EXCEPTION_RECORD32/64, etc.

+      process_types::EXCEPTION_POINTERS<process_types::internal::Traits64>;
 
 #if defined(ARCH_CPU_64_BITS)
   if (process_reader->Is64Bit()) {
+    ExceptionPointers64 exception_pointers;
+    if (!ReadExceptionPointers(
+            *process_reader, exception_pointers_address, &exception_pointers)) {
+      return false;
+    }
     CONTEXT context_record;
     if (!InitializeFromExceptionPointers<EXCEPTION_RECORD64>(
             *process_reader, exception_pointers, &context_record)) {
@@ -80,18 +77,24 @@ bool ExceptionSnapshotWin::Initialize(ProcessReaderWin* process_reader,
     context_.x86_64 = &context_union_.x86_64;
     InitializeX64Context(context_record, context_.x86_64);
   } else {
-    CHECK(false) << "TODO(scottmg) WOW64";
-    return false;
+    ExceptionPointers32 exception_pointers;
+    if (!ReadExceptionPointers(
+            *process_reader, exception_pointers_address, &exception_pointers)) {
+      return false;
+    }
+    if (!CommonX86Initialize<WOW64_CONTEXT>(*process_reader,
+                                            exception_pointers)) {
+      return false;
+    }
   }
 #else
-  CONTEXT context_record;
-  if (!InitializeFromExceptionPointers<EXCEPTION_RECORD32>(
-          *process_reader, exception_pointers, &context_record)) {
+  ExceptionPointers32 exception_pointers;

[Mark Mentovai, 2015/09/18 15:44:06]

It’s only a small amount of duplication, but it’d be nice to share a little
more.

#if defined(ARCH_CPU_32_BITS)
  const bool is_64_bit = false;
  using NativeContext32 = CONTEXT;
#elif defined(ARCH_CPU_64_BITS)
  const bool is_64_bit = process_reader->Is64Bit();
  using NativeContext32 = WOW64_CONTEXT;
  if (is_64_bit) {
    ExceptionPointers64 …
  }
#endif
  if (!is_64_bit) {
    ExceptionPointers32 …
    CommonX86Initialize<NativeContext32>(…)
  }

but maybe we can get away with using WOW64_CONTEXT even for the native 32-bit
case?

[scottmg, 2015/09/18 19:45:26]

Done, and that made it clearer how to collapse the two helper functions too.

I didn't use WOW64_CONTEXT on x86, they're the same of course, but it just feels
wrong and only saves a couple using lines.

+  if (!ReadExceptionPointers(
+          *process_reader, exception_pointers_address, &exception_pointers)) {
     return false;
   }
-  context_.architecture = kCPUArchitectureX86;
-  context_.x86 = &context_union_.x86;
-  InitializeX86Context(context_record, context_.x86);
+  if (!CommonX86Initialize<CONTEXT>(*process_reader, exception_pointers))
+    return false;
 #endif  // ARCH_CPU_64_BITS
 
   INITIALIZATION_STATE_SET_VALID(initialized_);
@@ -128,14 +131,16 @@ const std::vector<uint64_t>& ExceptionSnapshotWin::Codes() const {
   return codes_;
 }
 
-template <class ExceptionRecordType, class ContextType>
+template <class ExceptionRecordType,
+          class ExceptionPointersType,
+          class ContextType>
 bool ExceptionSnapshotWin::InitializeFromExceptionPointers(
     const ProcessReaderWin& process_reader,
-    const EXCEPTION_POINTERS& exception_pointers,
+    const ExceptionPointersType& exception_pointers,
     ContextType* context_record) {
   ExceptionRecordType first_record;
   if (!process_reader.ReadMemory(
-          reinterpret_cast<WinVMAddress>(exception_pointers.ExceptionRecord),
+          static_cast<WinVMAddress>(exception_pointers.ExceptionRecord),
           sizeof(first_record),
           &first_record)) {
     LOG(ERROR) << "ExceptionRecord";
@@ -152,7 +157,7 @@ bool ExceptionSnapshotWin::InitializeFromExceptionPointers(
   }
 
   if (!process_reader.ReadMemory(
-          reinterpret_cast<WinVMAddress>(exception_pointers.ContextRecord),
+          static_cast<WinVMAddress>(exception_pointers.ContextRecord),
           sizeof(*context_record),
           context_record)) {
     LOG(ERROR) << "ContextRecord";
@@ -162,5 +167,39 @@ bool ExceptionSnapshotWin::InitializeFromExceptionPointers(
   return true;
 }
 
+template <class ContextRecordType>
+bool ExceptionSnapshotWin::CommonX86Initialize(
+    const ProcessReaderWin& process_reader,
+    const process_types::EXCEPTION_POINTERS<process_types::internal::Traits32>&
+        exception_pointers) {
+  ContextRecordType 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);
+  return true;
+}
+
+template <class ExceptionPointersType>
+bool ExceptionSnapshotWin::ReadExceptionPointers(
+    const ProcessReaderWin& process_reader,
+    WinVMAddress exception_pointers_address,
+    ExceptionPointersType* exception_pointers) {
+  if (!process_reader.ReadMemory(exception_pointers_address,
+                                 sizeof(EXCEPTION_POINTERS),
+                                 exception_pointers)) {
+    LOG(ERROR) << "EXCEPTION_POINTERS read failed";
+    return false;
+  }
+  if (!exception_pointers->ExceptionRecord) {
+    LOG(ERROR) << "null ExceptionRecord";
+    return false;
+  }
+  return true;
+}
+
 }  // namespace internal
 }  // namespace crashpad