From 93fa375b580a647904925cb741266f2d679cb448 Mon Sep 17 00:00:00 2001 From: mmentovai Date: Wed, 6 Dec 2006 20:18:26 +0000 Subject: [PATCH] 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 --- src/common/windows/guid_string.cc | 13 +++ src/common/windows/guid_string.h | 6 ++ src/common/windows/pdb_source_line_writer.cc | 90 ++++++++++---------- src/common/windows/pdb_source_line_writer.h | 35 +++++--- src/common/windows/string_utils-inl.h | 7 ++ src/common/windows/string_utils.cc | 44 ++++++++++ src/tools/windows/dump_syms/dump_syms.vcproj | 4 + src/tools/windows/symupload/symupload.cc | 60 +++++++------ src/tools/windows/symupload/symupload.vcproj | 4 + 9 files changed, 177 insertions(+), 86 deletions(-) create mode 100644 src/common/windows/string_utils.cc 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 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; + identifier_string, + sizeof(identifier_string) / sizeof(identifier_string[0]), + L"%08x%x", signature, age); + info->debug_identifier = identifier_string; } - // DWORD* and int* are not compatible. This is clean and avoids a cast. - DWORD age_dword; - if (FAILED(global->get_age(&age_dword))) { + CComBSTR debug_file_string; + if (FAILED(global->get_symbolsFileName(&debug_file_string))) { return false; } - *age = age_dword; - - CComBSTR filename_string; - if (FAILED(global->get_symbolsFileName(&filename_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 #include +#include + // 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 diff --git a/src/tools/windows/dump_syms/dump_syms.vcproj b/src/tools/windows/dump_syms/dump_syms.vcproj index 360b7d9b..f9ac45d6 100644 --- a/src/tools/windows/dump_syms/dump_syms.vcproj +++ b/src/tools/windows/dump_syms/dump_syms.vcproj @@ -211,6 +211,10 @@ RelativePath="..\..\..\common\windows\pdb_source_line_writer.cc" > + + diff --git a/src/tools/windows/symupload/symupload.cc b/src/tools/windows/symupload/symupload.cc index 21545e11..d829aa91 100644 --- a/src/tools/windows/symupload/symupload.cc +++ b/src/tools/windows/symupload/symupload.cc @@ -31,11 +31,16 @@ // The PDB file is located automatically, using the path embedded in the // executable. The upload is sent as a multipart/form-data POST request, // with the following parameters: -// module: the name of the module, e.g. app.exe -// ver: the file version of the module, e.g. 1.2.3.4 -// guid: the GUID string embedded in the module pdb, -// e.g. 11111111-2222-3333-4444-555555555555 -// symbol_file: the airbag-format symbol file +// code_file: the basename of the module, e.g. "app.exe" +// debug_file: the basename of the debugging file, e.g. "app.pdb" +// debug_identifier: the debug file's identifier, usually consisting of +// the guid and age embedded in the pdb, e.g. +// "11111111BBBB3333DDDD555555555555F" +// version: the file version of the module, e.g. "1.2.3.4" +// os: the operating system that the module was built for, always +// "windows" in this implementation. +// cpu: the CPU that the module was built for, typically "x86". +// symbol_file: the contents of the airbag-format symbol file #include #include @@ -56,6 +61,7 @@ using std::wstring; using std::vector; using std::map; using google_airbag::HTTPUpload; +using google_airbag::PDBModuleInfo; using google_airbag::PDBSourceLineWriter; using google_airbag::WindowsStringUtils; @@ -97,15 +103,16 @@ static bool GetFileVersionString(const wchar_t *filename, wstring *version) { } // Creates a new temporary file and writes the symbol data from the given -// exe/dll file to it. Returns the path to the temp file in temp_file_path, -// and the unique identifier (GUID) for the pdb in module_guid. +// exe/dll file to it. Returns the path to the temp file in temp_file_path +// and information about the pdb in pdb_info. static bool DumpSymbolsToTempFile(const wchar_t *file, wstring *temp_file_path, - wstring *module_guid, - int *module_age, - wstring *module_filename) { + PDBModuleInfo *pdb_info) { google_airbag::PDBSourceLineWriter writer; - if (!writer.Open(file, PDBSourceLineWriter::ANY_FILE)) { + // Use EXE_FILE to get information out of the exe/dll in addition to the + // pdb. The name and version number of the exe/dll are of value, and + // there's no way to locate an exe/dll given a pdb. + if (!writer.Open(file, PDBSourceLineWriter::EXE_FILE)) { return false; } @@ -139,34 +146,31 @@ static bool DumpSymbolsToTempFile(const wchar_t *file, *temp_file_path = temp_filename; - return writer.GetModuleInfo(module_guid, module_age, module_filename, NULL); + return writer.GetModuleInfo(pdb_info); } int wmain(int argc, wchar_t *argv[]) { if (argc < 3) { - wprintf(L"Usage: %s file.[pdb|exe|dll] \n", argv[0]); + wprintf(L"Usage: %s \n", argv[0]); return 0; } const wchar_t *module = argv[1], *url = argv[2]; - wstring symbol_file, module_guid, module_basename; - int module_age; - if (!DumpSymbolsToTempFile(module, &symbol_file, - &module_guid, &module_age, &module_basename)) { + wstring symbol_file; + PDBModuleInfo pdb_info; + if (!DumpSymbolsToTempFile(module, &symbol_file, &pdb_info)) { fwprintf(stderr, L"Could not get symbol data from %s\n", module); return 1; } - wchar_t module_age_string[11]; - WindowsStringUtils::safe_swprintf( - module_age_string, - sizeof(module_age_string) / sizeof(module_age_string[0]), - L"0x%x", module_age); + wstring code_file = WindowsStringUtils::GetBaseName(wstring(module)); map parameters; - parameters[L"module"] = module_basename; - parameters[L"guid"] = module_guid; - parameters[L"age"] = module_age_string; + parameters[L"code_file"] = code_file; + parameters[L"debug_file"] = pdb_info.debug_file; + parameters[L"debug_identifier"] = pdb_info.debug_identifier; + parameters[L"os"] = L"windows"; // This version of symupload is Windows-only + parameters[L"cpu"] = pdb_info.cpu; // Don't make a missing version a hard error. Issue a warning, and let the // server decide whether to reject files without versions. @@ -186,7 +190,9 @@ int wmain(int argc, wchar_t *argv[]) { return 1; } - wprintf(L"Uploaded symbols for %s/%s/%s\n", - module_basename.c_str(), file_version.c_str(), module_guid.c_str()); + wprintf(L"Uploaded symbols for windows-%s/%s/%s (%s %s)\n", + pdb_info.cpu.c_str(), pdb_info.debug_file.c_str(), + pdb_info.debug_identifier.c_str(), code_file.c_str(), + file_version.c_str()); return 0; } diff --git a/src/tools/windows/symupload/symupload.vcproj b/src/tools/windows/symupload/symupload.vcproj index c1458854..49f6e518 100644 --- a/src/tools/windows/symupload/symupload.vcproj +++ b/src/tools/windows/symupload/symupload.vcproj @@ -216,6 +216,10 @@ RelativePath="..\..\..\common\windows\pdb_source_line_writer.cc" > + +