Support GUID-less PDBs (#77). r=bryner

 - Handle MDCVInfoPDB20-based PDBs by outputting a signature instead of a guid
   in the MODULE line.
 - Identify the OS and CPU in the MODULE line.
 - Suppress multiple subsequent identical STACK WIN lines.

http://groups.google.com/group/airbag-dev/browse_thread/thread/0f54e2c33ed5d82d


git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@70 4c0a9323-5329-0410-9bdc-e9ce6186880e

1 files changed, 105 insertions, 15 deletions

diff --git a/src/common/windows/pdb_source_line_writer.cc b/src/common/windows/pdb_source_line_writer.cc
index c6c1d5c4..f2f68b29 100644
--- a/src/common/windows/pdb_source_line_writer.cc
+++ b/src/common/windows/pdb_source_line_writer.cc
@@ -287,6 +287,11 @@ bool PDBSourceLineWriter::PrintFrameData() {
   if (!frame_data_enum)
     return false;
 
+  DWORD last_type = -1;
+  DWORD last_rva = -1;
+  DWORD last_code_size = 0;
+  DWORD last_prolog_size = -1;
+
   CComPtr<IDiaFrameData> frame_data;
   while (SUCCEEDED(frame_data_enum->Next(1, &frame_data, &count)) &&
          count == 1) {
@@ -348,14 +353,27 @@ bool PDBSourceLineWriter::PrintFrameData() {
       }
     }
 
-    fprintf(output_, "STACK WIN %x %x %x %x %x %x %x %x %x %d ",
-            type, rva, code_size, prolog_size, epilog_size,
-            parameter_size, saved_register_size, local_size, max_stack_size,
-            program_string_result == S_OK);
-    if (program_string_result == S_OK) {
-      fprintf(output_, "%ws\n", program_string);
-    } else {
-      fprintf(output_, "%d\n", allocates_base_pointer);
+    // Only print out a line if type, rva, code_size, or prolog_size have
+    // changed from the last line.  It is surprisingly common (especially in
+    // system library PDBs) for DIA to return a series of identical
+    // IDiaFrameData objects.  For kernel32.pdb from Windows XP SP2 on x86,
+    // this check reduces the size of the dumped symbol file by a third.
+    if (type != last_type || rva != last_rva || code_size != last_code_size ||
+        prolog_size != last_prolog_size) {
+      fprintf(output_, "STACK WIN %x %x %x %x %x %x %x %x %x %d ",
+              type, rva, code_size, prolog_size, epilog_size,
+              parameter_size, saved_register_size, local_size, max_stack_size,
+              program_string_result == S_OK);
+      if (program_string_result == S_OK) {
+        fprintf(output_, "%ws\n", program_string);
+      } else {
+        fprintf(output_, "%d\n", allocates_base_pointer);
+      }
+
+      last_type = type;
+      last_rva = rva;
+      last_code_size = code_size;
+      last_prolog_size = prolog_size;
     }
 
     frame_data.Release();
@@ -390,13 +408,17 @@ bool PDBSourceLineWriter::PrintCodePublicSymbol(IDiaSymbol *symbol) {
 }
 
 bool PDBSourceLineWriter::PrintPDBInfo() {
-  wstring guid, filename;
+  wstring guid, filename, cpu;
   int age;
-  if (!GetModuleInfo(&guid, &age, &filename)) {
+  if (!GetModuleInfo(&guid, &age, &filename, &cpu)) {
     return false;
   }
 
-  fprintf(output_, "MODULE %ws %x %ws\n", guid.c_str(), age, filename.c_str());
+  // Hard-code "windows" for the OS because that's the only thing that makes
+  // sense for PDB files.  (This might not be strictly correct for Windows CE
+  // support, but we don't care about that at the moment.)
+  fprintf(output_, "MODULE windows %ws %ws %x %ws\n",
+          cpu.c_str(), guid.c_str(), age, filename.c_str());
 
   return true;
 }
@@ -663,7 +685,7 @@ wstring PDBSourceLineWriter::GetBaseName(const wstring &filename) {
 }
 
 bool PDBSourceLineWriter::GetModuleInfo(wstring *guid, int *age,
-                                        wstring *filename) {
+                                        wstring *filename, wstring *cpu) {
   guid->clear();
   *age = 0;
   filename->clear();
@@ -673,11 +695,44 @@ bool PDBSourceLineWriter::GetModuleInfo(wstring *guid, int *age,
     return false;
   }
 
-  GUID guid_number;
-  if (FAILED(global->get_guid(&guid_number))) {
+  // cpu is permitted to be NULL.
+  if (cpu) {
+    // All CPUs in CV_CPU_TYPE_e (cvconst.h) below 0x10 are x86.  There's no
+    // single specific constant to use.
+    DWORD platform;
+    if (SUCCEEDED(global->get_platform(&platform)) && platform < 0x10) {
+      *cpu = L"x86";
+    } else {
+      // Unexpected, but handle gracefully.
+      *cpu = L"unknown";
+    }
+  }
+
+  bool uses_guid;
+  if (!UsesGUID(&uses_guid)) {
     return false;
   }
-  *guid = GUIDString::GUIDToWString(&guid_number);
+
+  if (uses_guid) {
+    GUID guid_number;
+    if (FAILED(global->get_guid(&guid_number))) {
+      return false;
+    }
+
+    *guid = GUIDString::GUIDToWString(&guid_number);
+  } else {
+    DWORD signature;
+    if (FAILED(global->get_signature(&signature))) {
+      return false;
+    }
+
+    wchar_t signature_string[9];
+    WindowsStringUtils::safe_swprintf(
+        signature_string,
+        sizeof(signature_string) / sizeof(signature_string[0]),
+        L"%08x", signature);
+    *guid = signature_string;
+  }
 
   // DWORD* and int* are not compatible.  This is clean and avoids a cast.
   DWORD age_dword;
@@ -695,4 +750,39 @@ bool PDBSourceLineWriter::GetModuleInfo(wstring *guid, int *age,
   return true;
 }
 
+bool PDBSourceLineWriter::UsesGUID(bool *uses_guid) {
+  if (!uses_guid)
+    return false;
+
+  CComPtr<IDiaSymbol> global;
+  if (FAILED(session_->get_globalScope(&global)))
+    return false;
+
+  GUID guid;
+  if (FAILED(global->get_guid(&guid)))
+    return false;
+
+  DWORD signature;
+  if (FAILED(global->get_signature(&signature)))
+    return false;
+
+  // There are two possibilities for guid: either it's a real 128-bit GUID
+  // as identified in a code module by a new-style CodeView record, or it's
+  // a 32-bit signature (timestamp) as identified by an old-style record.
+  // See MDCVInfoPDB70 and MDCVInfoPDB20 in minidump_format.h.
+  //
+  // Because DIA doesn't provide a way to directly determine whether a module
+  // uses a GUID or a 32-bit signature, this code checks whether the first 32
+  // bits of guid are the same as the signature, and if the rest of guid is
+  // zero.  If so, then with a pretty high degree of certainty, there's an
+  // old-style CodeView record in use.  This method will only falsely find an
+  // an old-style CodeView record if a real 128-bit GUID has its first 32
+  // bits set the same as the module's signature (timestamp) and the rest of
+  // the GUID is set to 0.  This is highly unlikely.
+
+  GUID signature_guid = {signature};  // 0-initializes other members
+  *uses_guid = !IsEqualGUID(guid, signature_guid);
+  return true;
+}
+
 }  // namespace google_airbag