win: Handle binary with embedded CodeView debug record

I considered writing the CodeView records to the minidump, but I didn't
find a ton of docs and debugging is only lightly supported (e.g.
http://www.debuginfo.com/articles/gendebuginfo.html#debuggersandformats
and it doesn't attempt to load at all on more recent Visual Studios).

As we won't be generating symbols in this format, and we don't expect to
have symbols for any weird modules that get injected into us in the
wild, it seems like we don't lose anything by just ignoring them.

R=mark@chromium.org
BUG=crashpad:47

Review URL: https://codereview.chromium.org/1430773003 .

handler/handler.gyp

@@ -110,6 +110,29 @@
           ],
         },
       ],
+      'conditions': [
+        # Cannot create an x64 DLL with embedded debug info.
+        ['target_arch=="ia32"', {
+          'targets': [
+            {
+              'target_name': 'crashy_z7_loader',
+              'type': 'executable',
+              'dependencies': [
+                '../client/client.gyp:crashpad_client',
+                '../test/test.gyp:crashpad_test',
+                '../third_party/mini_chromium/mini_chromium.gyp:base',
+                '../tools/tools.gyp:crashpad_tool_support',
+              ],
+              'include_dirs': [
+                '..',
+              ],
+              'sources': [
+                'win/crashy_test_z7_loader.cc',
+              ],
+            },
+          ],
+        }],
+      ],
     }, {
       'targets': [],
     }],

handler/win/crashy_test_z7_loader.cc

@@ -0,0 +1,73 @@
+// 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,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <stdlib.h>
+#include <windows.h>
+
+#include "base/files/file_path.h"
+#include "base/logging.h"
+#include "build/build_config.h"
+#include "client/crashpad_client.h"
+#include "test/paths.h"
+#include "tools/tool_support.h"
+
+#if !defined(ARCH_CPU_X86)
+#error This test is only supported on x86.
+#endif  // !ARCH_CPU_X86
+
+namespace crashpad {
+namespace {
+
+int CrashyLoadZ7Main(int argc, char* argv[]) {
+  if (argc != 2) {
+    fprintf(stderr, "Usage: %s <server_pipe_name>\n", argv[0]);
+    return EXIT_FAILURE;
+  }
+
+  CrashpadClient client;
+  if (!client.SetHandler(argv[1])) {
+    LOG(ERROR) << "SetHandler";
+    return EXIT_FAILURE;
+  }
+  if (!client.UseHandler()) {
+    LOG(ERROR) << "UseHandler";
+    return EXIT_FAILURE;
+  }
+
+  // The DLL has /Z7 symbols embedded in the binary (rather than in a .pdb).
+  // There's only an x86 version of this dll as newer x64 toolchains can't
+  // generate this format any more.
+  base::FilePath z7_path = test::Paths::TestDataRoot().Append(
+      FILE_PATH_LITERAL("handler/win/z7_test.dll"));
+  HMODULE z7_test = LoadLibrary(z7_path.value().c_str());
+  if (!z7_test) {
+    PLOG(ERROR) << "LoadLibrary";
+    return EXIT_FAILURE;
+  }
+  FARPROC crash_me = GetProcAddress(z7_test, "CrashMe");
+  if (!crash_me) {
+    PLOG(ERROR) << "GetProcAddress";
+    return EXIT_FAILURE;
+  }
+  reinterpret_cast<void(*)()>(crash_me)();
+
+  return EXIT_SUCCESS;
+}
+
+}  // namespace
+}  // namespace crashpad
+
+int wmain(int argc, wchar_t* argv[]) {
+  return crashpad::ToolSupport::Wmain(argc, argv, crashpad::CrashyLoadZ7Main);
+}

handler/win/z7_test.cpp

@@ -0,0 +1,32 @@
+// 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,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Build in VC++6 or older command prompt with:
+//
+// cl /nologo /W4 /MT /Z7 z7_test.cpp /link /dll /out:z7_test.dll /debugtype:cv /pdb:none
+//
+// Given that this is quite tedious to build, the result is also checked in.
+
+#include <windows.h>
+#include <stdio.h>
+
+extern "C" __declspec(dllexport) void CrashMe() {
+  volatile int* foo = reinterpret_cast<volatile int*>(7);
+  *foo = 42;
+}
+
+BOOL WINAPI DllMain(HINSTANCE hinstance, DWORD reason, LPVOID) {
+  printf("%p %d\n", hinstance, reason);
+  return TRUE;
+}

snapshot/win/end_to_end_test.py

@@ -121,6 +121,10 @@ def GetDumpFromSelfDestroyingProgram(out_dir, pipe_name):
   return GetDumpFromProgram(out_dir, pipe_name, 'self_destroying_program.exe')
 
 
+def GetDumpFromZ7Program(out_dir, pipe_name):
+  return GetDumpFromProgram(out_dir, pipe_name, 'crashy_z7_loader.exe')
+
+
 class CdbRun(object):
   """Run cdb.exe passing it a cdb command and capturing the output.
   `Check()` searches for regex patterns in sequence allowing verification of
@@ -155,7 +159,7 @@ class CdbRun(object):
       sys.exit(1)
 
 
-def RunTests(cdb_path, dump_path, destroyed_dump_path, pipe_name):
+def RunTests(cdb_path, dump_path, destroyed_dump_path, z7_dump_path, pipe_name):
   """Runs various tests in sequence. Runs a new cdb instance on the dump for
   each block of tests to reduce the chances that output from one command is
   confused for output from another.
@@ -212,6 +216,15 @@ def RunTests(cdb_path, dump_path, destroyed_dump_path, pipe_name):
                 r'FreeOwnStackAndBreak.*\nquit:',
             'at correct location, no additional stack entries')
 
+  if z7_dump_path:
+    out = CdbRun(cdb_path, z7_dump_path, '.ecxr;lm')
+    out.Check('This dump file has an exception of interest stored in it',
+              'captured exception in z7 module')
+    out.Check(r'z7_test\+0x[0-8a-f]+:', 'exception in z7 at correct location')
+    out.Check(r'z7_test  C \(codeview symbols\)     z7_test.dll',
+              'expected non-pdb symbol format')
+
+
 def main(args):
   try:
     if len(args) != 1:
@@ -242,7 +255,14 @@ def main(args):
     if not destroyed_dump_path:
       return 1
 
-    RunTests(cdb_path, crashy_dump_path, destroyed_dump_path, pipe_name)
+    z7_dump_path = None
+    if not args[0].endswith('x64'):
+      z7_dump_path = GetDumpFromZ7Program(args[0], pipe_name)
+      if not z7_dump_path:
+        return 1
+
+    RunTests(cdb_path, crashy_dump_path, destroyed_dump_path, z7_dump_path,
+             pipe_name)
 
     return 0
   finally:

snapshot/win/module_snapshot_win.cc

@@ -60,6 +60,14 @@ bool ModuleSnapshotWin::Initialize(
           &uuid_, &age_dword, &pdb_name_)) {
     static_assert(sizeof(DWORD) == sizeof(uint32_t), "unexpected age size");
     age_ = age_dword;
+  } else {
+    // If we fully supported all old debugging formats, we would want to extract
+    // and emit a different type of CodeView record here (as old Microsoft tools
+    // would do). As we don't expect to ever encounter a module that wouldn't be
+    // be using .PDB that we actually have symbols for, we simply set a
+    // plausible name here, but this will never correspond to symbols that we
+    // have.
+    pdb_name_ = base::UTF16ToUTF8(name_);
   }
 
   INITIALIZATION_STATE_SET_VALID(initialized_);

snapshot/win/pe_image_reader.cc

@@ -176,35 +176,41 @@ bool PEImageReader::ReadDebugDirectoryInformation(UUID* uuid,
     if (debug_directory.Type != IMAGE_DEBUG_TYPE_CODEVIEW)
       continue;
 
-    if (debug_directory.SizeOfData < sizeof(CodeViewRecordPDB70)) {
-      LOG(WARNING) << "CodeView debug entry of unexpected size";
-      continue;
-    }
-    scoped_ptr<char[]> data(new char[debug_directory.SizeOfData]);
-    if (!CheckedReadMemory(Address() + debug_directory.AddressOfRawData,
-                           debug_directory.SizeOfData,
-                           data.get())) {
-      LOG(WARNING) << "could not read debug directory";
-      return false;
-    }
+    if (debug_directory.AddressOfRawData) {
+      if (debug_directory.SizeOfData < sizeof(CodeViewRecordPDB70)) {
+        LOG(WARNING) << "CodeView debug entry of unexpected size";
+        continue;
+      }
 
-    if (*reinterpret_cast<DWORD*>(data.get()) !=
-        CodeViewRecordPDB70::kSignature) {
-      // TODO(scottmg): Consider supporting other record types, see
+      scoped_ptr<char[]> data(new char[debug_directory.SizeOfData]);
+      if (!CheckedReadMemory(Address() + debug_directory.AddressOfRawData,
+                             debug_directory.SizeOfData,
+                             data.get())) {
+        LOG(WARNING) << "could not read debug directory";
+        return false;
+      }
+
+      if (*reinterpret_cast<DWORD*>(data.get()) !=
+          CodeViewRecordPDB70::kSignature) {
+        LOG(WARNING) << "encountered non-7.0 CodeView debug record";
+        continue;
+      }
+
+      CodeViewRecordPDB70* codeview =
+          reinterpret_cast<CodeViewRecordPDB70*>(data.get());
+      *uuid = codeview->uuid;
+      *age = codeview->age;
+      // This is a NUL-terminated string encoded in the codepage of the system
+      // where the binary was linked. We have no idea what that was, so we just
+      // assume ASCII.
+      *pdbname = std::string(reinterpret_cast<char*>(&codeview->pdb_name[0]));
+      return true;
+    } else if (debug_directory.PointerToRawData) {
+      // This occurs for non-PDB based debug information. We simply ignore these
+      // as we don't expect to encounter modules that will be in this format
+      // for which we'll actually have symbols. See
       // https://crashpad.chromium.org/bug/47.
-      LOG(WARNING) << "encountered non-7.0 CodeView debug record";
-      continue;
     }
-
-    CodeViewRecordPDB70* codeview =
-        reinterpret_cast<CodeViewRecordPDB70*>(data.get());
-    *uuid = codeview->uuid;
-    *age = codeview->age;
-    // This is a NUL-terminated string encoded in the codepage of the system
-    // where the binary was linked. We have no idea what that was, so we just
-    // assume ASCII.
-    *pdbname = std::string(reinterpret_cast<char*>(&codeview->pdb_name[0]));
-    return true;
   }
 
   return false;

util/win/process_info.cc

@@ -514,14 +514,14 @@ bool ProcessInfo::Initialize(HANDLE process) {
         system_info.wProcessorArchitecture == PROCESSOR_ARCHITECTURE_AMD64;
   }
 
-#if ARCH_CPU_32_BITS
+#if defined(ARCH_CPU_32_BITS)
   if (is_64_bit_) {
     LOG(ERROR) << "Reading x64 process from x86 process not supported";
     return false;
   }
-#endif
+#endif  // ARCH_CPU_32_BITS
 
-#if ARCH_CPU_64_BITS
+#if defined(ARCH_CPU_64_BITS)
   bool result = GetProcessBasicInformation<process_types::internal::Traits64>(
       process, is_wow64_, this, &peb_address_, &peb_size_);
 #else