symupload parameters don't match processor expectations (#91). r=bryner

 - Interface change: the "guid" and "age" parameters supplied to a symbol
   server by symupload have been merged into "debug_identifier".  Some
   other parameters have had their names changed.  Additional code_file,
   os, and cpu parameters have been added.
 - Interface change: the format of the MODULE line at the top of dumped .sym
   files has changed slightly.  The fields used for uuid and age have
   merged into a debug_identifier-type field.
 - debug_identifier is formatted the same way as CodeModule::debug_identifier
   for ease of server-side processing.

http://groups.google.com/group/airbag-dev/browse_thread/thread/8022f504cf01f994


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

6 files changed, 136 insertions, 59 deletions

diff --git a/src/common/windows/guid_string.cc b/src/common/windows/guid_string.cc
index 42a0b21a..0adc248a 100644
--- a/src/common/windows/guid_string.cc
+++ b/src/common/windows/guid_string.cc
@@ -52,4 +52,17 @@ wstring GUIDString::GUIDToWString(GUID *guid) {
   return wstring(guid_string);
 }
 
+// static
+wstring GUIDString::GUIDToSymbolServerWString(GUID *guid) {
+  wchar_t guid_string[33];
+  WindowsStringUtils::safe_swprintf(
+      guid_string, sizeof(guid_string) / sizeof(guid_string[0]),
+      L"%08X%04X%04X%02X%02X%02X%02X%02X%02X%02X%02X",
+      guid->Data1, guid->Data2, guid->Data3,
+      guid->Data4[0], guid->Data4[1], guid->Data4[2],
+      guid->Data4[3], guid->Data4[4], guid->Data4[5],
+      guid->Data4[6], guid->Data4[7]);
+  return wstring(guid_string);
+}
+
 }  // namespace google_airbag
diff --git a/src/common/windows/guid_string.h b/src/common/windows/guid_string.h
index df067499..c62a341c 100644
--- a/src/common/windows/guid_string.h
+++ b/src/common/windows/guid_string.h
@@ -45,6 +45,12 @@ class GUIDString {
   // Converts guid to a string in the format recommended by RFC 4122 and
   // returns the string.
   static wstring GUIDToWString(GUID *guid);
+
+  // Converts guid to a string formatted as uppercase hexadecimal, with
+  // no separators, and returns the string.  This is the format used for
+  // symbol server identifiers, although identifiers have an age tacked
+  // on to the string.
+  static wstring GUIDToSymbolServerWString(GUID *guid);
 };
 
 }  // namespace google_airbag
diff --git a/src/common/windows/pdb_source_line_writer.cc b/src/common/windows/pdb_source_line_writer.cc
index f2f68b29..5be053dc 100644
--- a/src/common/windows/pdb_source_line_writer.cc
+++ b/src/common/windows/pdb_source_line_writer.cc
@@ -408,17 +408,17 @@ bool PDBSourceLineWriter::PrintCodePublicSymbol(IDiaSymbol *symbol) {
 }
 
 bool PDBSourceLineWriter::PrintPDBInfo() {
-  wstring guid, filename, cpu;
-  int age;
-  if (!GetModuleInfo(&guid, &age, &filename, &cpu)) {
+  PDBModuleInfo info;
+  if (!GetModuleInfo(&info)) {
     return false;
   }
 
   // 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());
+  fprintf(output_, "MODULE windows %ws %ws %ws\n",
+          info.cpu.c_str(), info.debug_identifier.c_str(),
+          info.debug_file.c_str());
 
   return true;
 }
@@ -674,38 +674,34 @@ void PDBSourceLineWriter::Close() {
   session_.Release();
 }
 
-// static
-wstring PDBSourceLineWriter::GetBaseName(const wstring &filename) {
-  wstring base_name(filename);
-  size_t slash_pos = base_name.find_last_of(L"/\\");
-  if (slash_pos != wstring::npos) {
-    base_name.erase(0, slash_pos + 1);
+bool PDBSourceLineWriter::GetModuleInfo(PDBModuleInfo *info) {
+  if (!info) {
+    return false;
   }
-  return base_name;
-}
 
-bool PDBSourceLineWriter::GetModuleInfo(wstring *guid, int *age,
-                                        wstring *filename, wstring *cpu) {
-  guid->clear();
-  *age = 0;
-  filename->clear();
+  info->debug_file.clear();
+  info->debug_identifier.clear();
+  info->cpu.clear();
 
   CComPtr<IDiaSymbol> global;
   if (FAILED(session_->get_globalScope(&global))) {
     return false;
   }
 
-  // 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";
-    }
+  // 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) {
+    info->cpu = L"x86";
+  } else {
+    // Unexpected, but handle gracefully.
+    info->cpu = L"unknown";
+  }
+
+  // DWORD* and int* are not compatible.  This is clean and avoids a cast.
+  DWORD age;
+  if (FAILED(global->get_age(&age))) {
+    return false;
   }
 
   bool uses_guid;
@@ -714,38 +710,38 @@ bool PDBSourceLineWriter::GetModuleInfo(wstring *guid, int *age,
   }
 
   if (uses_guid) {
-    GUID guid_number;
-    if (FAILED(global->get_guid(&guid_number))) {
+    GUID guid;
+    if (FAILED(global->get_guid(&guid))) {
       return false;
     }
 
-    *guid = GUIDString::GUIDToWString(&guid_number);
+    wchar_t age_string[9];
+    WindowsStringUtils::safe_swprintf(
+        age_string, sizeof(age_string) / sizeof(age_string[0]),
+        L"%X", age);
+
+    info->debug_identifier = GUIDString::GUIDToSymbolServerWString(&guid);
+    info->debug_identifier.append(age_string);
   } else {
     DWORD signature;
     if (FAILED(global->get_signature(&signature))) {
       return false;
     }
 
-    wchar_t signature_string[9];
+    wchar_t identifier_string[17];
     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;
-  if (FAILED(global->get_age(&age_dword))) {
-    return false;
+        identifier_string,
+        sizeof(identifier_string) / sizeof(identifier_string[0]),
+        L"%08x%x", signature, age);
+    info->debug_identifier = identifier_string;
   }
-  *age = age_dword;
 
-  CComBSTR filename_string;
-  if (FAILED(global->get_symbolsFileName(&filename_string))) {
+  CComBSTR debug_file_string;
+  if (FAILED(global->get_symbolsFileName(&debug_file_string))) {
     return false;
   }
-  *filename = GetBaseName(wstring(filename_string));
+  info->debug_file =
+      WindowsStringUtils::GetBaseName(wstring(debug_file_string));
 
   return true;
 }
diff --git a/src/common/windows/pdb_source_line_writer.h b/src/common/windows/pdb_source_line_writer.h
index b4db5bc7..a2818263 100644
--- a/src/common/windows/pdb_source_line_writer.h
+++ b/src/common/windows/pdb_source_line_writer.h
@@ -45,6 +45,26 @@ namespace google_airbag {
 
 using std::wstring;
 
+// A structure that carries information that identifies a pdb file.
+struct PDBModuleInfo {
+ public:
+  // The basename of the pdb file from which information was loaded.
+  wstring debug_file;
+
+  // The pdb's identifier.  For recent pdb files, the identifier consists
+  // of the pdb's guid, in uppercase hexadecimal form without any dashes
+  // or separators, followed immediately by the pdb's age, also in
+  // uppercase hexadecimal form.  For older pdb files which have no guid,
+  // the identifier is the pdb's 32-bit signature value, in zero-padded
+  // hexadecimal form, followed immediately by the pdb's age, in lowercase
+  // hexadecimal form.
+  wstring debug_identifier;
+
+  // A string identifying the cpu that the pdb is associated with.
+  // Currently, this may be "x86" or "unknown".
+  wstring cpu;
+};
+
 class PDBSourceLineWriter {
  public:
   enum FileFormat {
@@ -74,15 +94,9 @@ class PDBSourceLineWriter {
   // Closes the current pdb file and its associated resources.
   void Close();
 
-  // Sets guid to the GUID for the module, as a string,
-  // 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);
+  // Retrieves information about the module's debugging file.  Returns
+  // true on success and false on failure.
+  bool GetModuleInfo(PDBModuleInfo *info);
 
   // 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
@@ -120,9 +134,6 @@ class PDBSourceLineWriter {
   // its uuid and age.
   bool PrintPDBInfo();
 
-  // Returns the base name of a file, e.g. strips off the path.
-  static wstring GetBaseName(const wstring &filename);
-
   // Returns the function name for a symbol.  If possible, the name is
   // undecorated.  If the symbol's decorated form indicates the size of
   // parameters on the stack, this information is returned in stack_param_size.
diff --git a/src/common/windows/string_utils-inl.h b/src/common/windows/string_utils-inl.h
index 62ce7ac3..b3e496ca 100644
--- a/src/common/windows/string_utils-inl.h
+++ b/src/common/windows/string_utils-inl.h
@@ -36,6 +36,8 @@
 #include <stdarg.h>
 #include <wchar.h>
 
+#include <string>
+
 // The "ll" printf format size specifier corresponding to |long long| was
 // intrudced in MSVC8.  Earlier versions did not provide this size specifier,
 // but "I64" can be used to print 64-bit types.  Don't use "I64" where "ll"
@@ -49,6 +51,8 @@
 
 namespace google_airbag {
 
+using std::wstring;
+
 class WindowsStringUtils {
  public:
   // Equivalent to MSVC8's swprintf, which always 0-terminates buffer.
@@ -68,6 +72,9 @@ class WindowsStringUtils {
   static void safe_wcsncpy(wchar_t *destination, size_t destination_size,
                            const wchar_t *source, size_t count);
 
+  // Returns the base name of a file, e.g. strips off the path.
+  static wstring GetBaseName(const wstring &filename);
+
  private:
   // Disallow instantiation and other object-based operations.
   WindowsStringUtils();
diff --git a/src/common/windows/string_utils.cc b/src/common/windows/string_utils.cc
new file mode 100644
index 00000000..ec2073ef
--- /dev/null
+++ b/src/common/windows/string_utils.cc
@@ -0,0 +1,44 @@
+// Copyright (c) 2006, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#include "common/windows/string_utils-inl.h"
+
+namespace google_airbag {
+
+// static
+wstring WindowsStringUtils::GetBaseName(const wstring &filename) {
+  wstring base_name(filename);
+  size_t slash_pos = base_name.find_last_of(L"/\\");
+  if (slash_pos != wstring::npos) {
+    base_name.erase(0, slash_pos + 1);
+  }
+  return base_name;
+}
+
+}  // namespace google_airbag