win x86: Grab bag of restructuring to get tests working on x86-on-x86

snapshot/win/exception_snapshot_win.cc

 // Copyright 2015 The Crashpad Authors. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
 //     http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
@@ skipping 51 matching lines @@
                                   sizeof(EXCEPTION_POINTERS),
                                   &exception_pointers)) {
     LOG(ERROR) << "EXCEPTION_POINTERS read failed";
     return false;
   }
   if (!exception_pointers.ExceptionRecord) {
     LOG(ERROR) << "null ExceptionRecord";
     return false;
   }
 
+#if defined(ARCH_CPU_64_BITS)
   if (process_reader->Is64Bit()) {
     EXCEPTION_RECORD64 first_record;
     if (!process_reader->ReadMemory(
             reinterpret_cast<WinVMAddress>(exception_pointers.ExceptionRecord),
             sizeof(first_record),
             &first_record)) {
       LOG(ERROR) << "ExceptionRecord";
       return false;
     }
     exception_code_ = first_record.ExceptionCode;
@@ skipping 16 matching lines @@
             sizeof(context_record),
             &context_record)) {
       LOG(ERROR) << "ContextRecord";
       return false;
     }
     InitializeX64Context(context_record, context_.x86_64);
   } else {
     CHECK(false) << "TODO(scottmg) x86";
     return false;
   }
+#else
+  EXCEPTION_RECORD32 first_record;

[Mark Mentovai, 2015/09/16 02:57:19]

This is so similar to what’s above that it’s kind of unfortunate to have so much
duplication. The TODO for the WoW64 case above will probably be a third
nearly-identical branch. Is there a decent way to share? Templatized helper
function with traits?

[scottmg, 2015/09/16 18:05:06]

Done.

+  if (!process_reader->ReadMemory(
+          reinterpret_cast<WinVMAddress>(exception_pointers.ExceptionRecord),
+          sizeof(first_record),
+          &first_record)) {
+    LOG(ERROR) << "ExceptionRecord";
+    return false;
+  }
+  exception_code_ = first_record.ExceptionCode;
+  exception_flags_ = first_record.ExceptionFlags;
+  exception_address_ = first_record.ExceptionAddress;
+  for (DWORD i = 0; i < first_record.NumberParameters; ++i)
+    codes_.push_back(first_record.ExceptionInformation[i]);
+  if (first_record.ExceptionRecord) {
+    // https://code.google.com/p/crashpad/issues/detail?id=43
+    LOG(WARNING) << "dropping chained ExceptionRecord";
+  }
+
+  context_.architecture = kCPUArchitectureX86;
+  context_.x86 = &context_union_.x86;
+  // We assume 64-on-64 here in that we're relying on the CONTEXT definition

[Mark Mentovai, 2015/09/16 02:57:19]

Well, the comment shouldn’t have been copied and pasted as-is.

[scottmg, 2015/09/16 18:05:06]

Oops, I noticed that too, but forgot to upload the last snapshot. Anyway,
templated now.

+  // to be the x64 one.
+  CONTEXT context_record;
+  if (!process_reader->ReadMemory(
+          reinterpret_cast<WinVMAddress>(exception_pointers.ContextRecord),
+          sizeof(context_record),
+          &context_record)) {
+    LOG(ERROR) << "ContextRecord";
+    return false;
+  }
+  InitializeX86Context(context_record, context_.x86);
+#endif  // ARCH_CPU_64_BITS
 
   INITIALIZATION_STATE_SET_VALID(initialized_);
   return true;
 }
 
 const CPUContext* ExceptionSnapshotWin::Context() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   return &context_;
 }
 
@@ skipping 17 matching lines @@
   return exception_address_;
 }
 
 const std::vector<uint64_t>& ExceptionSnapshotWin::Codes() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   return codes_;
 }
 
 }  // namespace internal
 }  // namespace crashpad