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

2 files changed, 120 insertions, 19 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
diff --git a/src/common/windows/pdb_source_line_writer.h b/src/common/windows/pdb_source_line_writer.h
index 5a8fee1d..b4db5bc7 100644
--- a/src/common/windows/pdb_source_line_writer.h
+++ b/src/common/windows/pdb_source_line_writer.h
@@ -75,10 +75,21 @@ class PDBSourceLineWriter {
   void Close();
 
   // Sets guid to the GUID for the module, as a string,
-  // e.g. "11111111-2222-3333-4444-555555555555".  age will be set to the
-  // age of the pdb file, and filename will be set to the basename of the
-  // PDB's file name.  Returns true on success and false on failure.
-  bool GetModuleInfo(wstring *guid, int *age, wstring *filename);
+  // e.g. "11111111-2222-3333-4444-555555555555".  If the module has no guid,
+  // guid will instead be set to the module's 32-bit signature value, in
+  // zero-padded hexadecimal form, such as "0004beef".  age will be set to the
+  // age of the pdb file, filename will be set to the basename of the pdb's
+  // file name, and cpu will be set to a string identifying the associated CPU
+  // architecture.  cpu is permitted to be NULL, in which case CPU information
+  // will not be returned.  Returns true on success and false on failure.
+  bool GetModuleInfo(wstring *guid, int *age, wstring *filename, wstring *cpu);
+
+  // Sets uses_guid to true if the opened file uses a new-style CodeView
+  // record with a 128-bit GUID, or false if the opened file uses an old-style
+  // CodeView record.  When no GUID is available, a 32-bit signature should be
+  // used to identify the module instead.  If the information cannot be
+  // determined, this method returns false.
+  bool UsesGUID(bool *uses_guid);
 
  private:
   // Outputs the line/address pairs for each line in the enumerator.