Add PE-only MD support to Windows symbol converter.

- Only 64-bit PEs supported.
- Re-add some scripts that were missed in initial move of code.
- Change msdia120.dll dependency to msdia140.dll.
- Add tests for Intel, AMD, and NVidia Microsoft Symbol Stores.
- Windows symbol converter now attempts to fall back to PE-only metadata
  when it fails to locate a PDB.
- Remove the 'binary' folder under converter_exe. Need to think more
  about how a deployment should look and what tool(s) to use in creating
  one.

Change-Id: I52e42cbe5e759874a25114c2483e8b50d73fdf77
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1670098
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>

12 files changed, 314 insertions, 56 deletions

diff --git a/src/common/windows/pe_util.cc b/src/common/windows/pe_util.cc
index 27f702a3..f599fb53 100644
--- a/src/common/windows/pe_util.cc
+++ b/src/common/windows/pe_util.cc
@@ -143,10 +143,6 @@ bool ReadModuleInfo(const wstring & pe_file, PDBModuleInfo * info) {
 

   PIMAGE_OPTIONAL_HEADER64 optional_header =

       &(reinterpret_cast<PIMAGE_NT_HEADERS64>(img->FileHeader))->OptionalHeader;

-  if (optional_header->Magic != IMAGE_NT_OPTIONAL_HDR64_MAGIC) {

-    fprintf(stderr, "Not a PE32+ image\n");

-    return false;

-  }

 

   // Search debug directories for a guid signature & age

   DWORD debug_rva = optional_header->

diff --git a/src/tools/windows/converter/ms_symbol_server_converter.cc b/src/tools/windows/converter/ms_symbol_server_converter.cc
index 6cc67700..e3215ba5 100644
--- a/src/tools/windows/converter/ms_symbol_server_converter.cc
+++ b/src/tools/windows/converter/ms_symbol_server_converter.cc
@@ -42,6 +42,7 @@
 
 #include "tools/windows/converter/ms_symbol_server_converter.h"
 #include "common/windows/pdb_source_line_writer.h"
+#include "common/windows/pe_source_line_writer.h"
 #include "common/windows/string_utils-inl.h"
 
 // SYMOPT_NO_PROMPTS is not defined in earlier platform SDKs.  Define it
@@ -445,7 +446,10 @@ MSSymbolServerConverter::LocateAndConvertSymbolFile(
   string pdb_file;
   LocateResult result = LocateSymbolFile(missing, &pdb_file);
   if (result != LOCATE_SUCCESS) {
-    return result;
+    fprintf(stderr, "Fallback to PE-only symbol generation for: %s\n",
+        missing.debug_file.c_str());
+    return LocateAndConvertPEFile(missing, keep_pe_file, converted_symbol_file,
+        out_pe_file);
   }
 
   if (symbol_file && keep_symbol_file) {
@@ -525,7 +529,7 @@ MSSymbolServerConverter::LocateAndConvertSymbolFile(
 #if _MSC_VER >= 1400  // MSVC 2005/8
   errno_t err;
   if ((err = fopen_s(&converted_output, converted_symbol_file->c_str(), "w"))
-      != 0) {
+    != 0) {
 #else  // _MSC_VER >= 1400
   // fopen_s and errno_t were introduced in MSVC8.  Use fopen for earlier
   // environments.  Don't use fopen with MSVC8 and later, because it's
@@ -536,12 +540,12 @@ MSSymbolServerConverter::LocateAndConvertSymbolFile(
     err = -1;
 #endif  // _MSC_VER >= 1400
     fprintf(stderr, "LocateAndConvertSymbolFile: "
-            "fopen_s: error %d for %s %s %s %s\n",
-            err,
-            missing.debug_file.c_str(),
-            missing.debug_identifier.c_str(),
-            missing.version.c_str(),
-            converted_symbol_file->c_str());
+        "fopen_s: error %d for %s %s %s %s\n",
+        err,
+        missing.debug_file.c_str(),
+        missing.debug_identifier.c_str(),
+        missing.version.c_str(),
+        converted_symbol_file->c_str());
     return LOCATE_FAILURE;
   }
 
@@ -573,4 +577,121 @@ MSSymbolServerConverter::LocateAndConvertSymbolFile(
   return LOCATE_SUCCESS;
 }
 
+MSSymbolServerConverter::LocateResult
+MSSymbolServerConverter::LocateAndConvertPEFile(
+    const MissingSymbolInfo &missing,
+    bool keep_pe_file,
+    string *converted_symbol_file,
+    string *out_pe_file) {
+  assert(converted_symbol_file);
+  converted_symbol_file->clear();
+
+  string pe_file;
+  MSSymbolServerConverter::LocateResult result = LocatePEFile(missing,
+      &pe_file);
+  if (result != LOCATE_SUCCESS) {
+    fprintf(stderr, "WARNING: Could not download: %s\n", pe_file.c_str());
+    return result;
+  }
+
+  if (out_pe_file && keep_pe_file) {
+    *out_pe_file = pe_file;
+  }
+
+  // Conversion may fail because the file is corrupt.  If a broken file is
+  // kept in the local cache, LocatePEFile will not hit the network again
+  // to attempt to locate it.  To guard against problems like this, the
+  // PE file in the local cache will be removed if conversion fails.
+  AutoDeleter pe_deleter(pe_file);
+
+  // Be sure that it's a .exe or .dll file, since we'll be replacing extension
+  // with .sym for the converted file's name.
+  string pe_extension = pe_file.substr(pe_file.length() - 4);
+  // strcasecmp is called _stricmp here.
+  if (_stricmp(pe_extension.c_str(), ".exe") != 0 &&
+    _stricmp(pe_extension.c_str(), ".dll") != 0) {
+    fprintf(stderr, "LocateAndConvertPEFile: "
+        "no .dll/.exe extension for %s %s %s %s\n",
+        missing.debug_file.c_str(),
+        missing.debug_identifier.c_str(),
+        missing.version.c_str(),
+        pe_file.c_str());
+    return LOCATE_FAILURE;
+  }
+
+  *converted_symbol_file = pe_file.substr(0, pe_file.length() - 4) + ".sym";
+
+  FILE *converted_output = NULL;
+#if _MSC_VER >= 1400  // MSVC 2005/8
+  errno_t err;
+  if ((err = fopen_s(&converted_output, converted_symbol_file->c_str(), "w"))
+      != 0) {
+#else  // _MSC_VER >= 1400
+  // fopen_s and errno_t were introduced in MSVC8.  Use fopen for earlier
+  // environments.  Don't use fopen with MSVC8 and later, because it's
+  // deprecated.  fopen does not provide reliable error codes, so just use
+  // -1 in the event of a failure.
+  int err;
+  if (!(converted_output = fopen(converted_symbol_file->c_str(), "w"))) {
+    err = -1;
+#endif  // _MSC_VER >= 1400
+    fprintf(stderr, "LocateAndConvertPEFile: "
+        "fopen_s: error %d for %s %s %s %s\n",
+        err,
+        missing.debug_file.c_str(),
+        missing.debug_identifier.c_str(),
+        missing.version.c_str(),
+        converted_symbol_file->c_str());
+    return LOCATE_FAILURE;
+  }
+  AutoDeleter sym_deleter(*converted_symbol_file);
+
+  wstring pe_file_w;
+  if (!WindowsStringUtils::safe_mbstowcs(pe_file, &pe_file_w)) {
+    fprintf(stderr,
+        "LocateAndConvertPEFile: "
+        "WindowsStringUtils::safe_mbstowcs failed for %s\n",
+        pe_file.c_str());
+    return LOCATE_FAILURE;
+  }
+  PESourceLineWriter writer(pe_file_w);
+  PDBModuleInfo module_info;
+  if (!writer.GetModuleInfo(&module_info)) {
+    fprintf(stderr, "LocateAndConvertPEFile: "
+        "PESourceLineWriter::GetModuleInfo failed for %s %s %s %s\n",
+        missing.debug_file.c_str(),
+        missing.debug_identifier.c_str(),
+        missing.version.c_str(),
+        pe_file.c_str());
+    return LOCATE_FAILURE;
+  }
+  if (module_info.cpu.compare(L"x86_64") != 0) {
+    // This module is not x64 so we cannot generate Breakpad symbols from the
+    // PE alone. Don't delete PE-- no need to retry download.
+    pe_deleter.Release();
+    return LOCATE_FAILURE;
+  }
+
+  bool success = writer.WriteSymbols(converted_output);
+  fclose(converted_output);
+
+  if (!success) {
+    fprintf(stderr, "LocateAndConvertPEFile: "
+        "PESourceLineWriter::WriteMap failed for %s %s %s %s\n",
+        missing.debug_file.c_str(),
+        missing.debug_identifier.c_str(),
+        missing.version.c_str(),
+        pe_file.c_str());
+    return LOCATE_FAILURE;
+  }
+
+  if (keep_pe_file) {
+    pe_deleter.Release();
+  }
+
+  sym_deleter.Release();
+
+  return LOCATE_SUCCESS;
+}
+
 }  // namespace google_breakpad
diff --git a/src/tools/windows/converter/ms_symbol_server_converter.h b/src/tools/windows/converter/ms_symbol_server_converter.h
index d601b433..401f7c34 100644
--- a/src/tools/windows/converter/ms_symbol_server_converter.h
+++ b/src/tools/windows/converter/ms_symbol_server_converter.h
@@ -177,6 +177,22 @@ class MSSymbolServerConverter {
                                           string *symbol_file,
                                           string *pe_file);
 
+  // Calls LocatePEFile and converts the returned PE file to the
+  // dumped-symbol format, storing it adjacent to the PE file. The
+  // only conversion supported is from PE files. Returns the return
+  // value of LocatePEFile, or if LocatePEFile succeeds but
+  // conversion fails, returns LOCATE_FAILURE. The pathname to the
+  // PE file and to the converted symbol file are returned in
+  // |converted_symbol_file| and |pe_file|. |pe_file| is optional and may be
+  // NULL.  If only the converted symbol file is desired, set |keep_pe_file|
+  // to false to indicate that the executable file (exe, dll) should be deleted
+  // after conversion.
+  // NOTE: Currrently only supports x64 PEs.
+  LocateResult LocateAndConvertPEFile(const MissingSymbolInfo &missing,
+      bool keep_pe_file,
+      string *converted_symbol_file,
+      string *pe_file);
+
  private:
   // Locates the PDB or PE file (DLL or EXE) specified by the identifying
   // information in |debug_or_code_file| and |debug_or_code_id|, by checking
diff --git a/src/tools/windows/converter_exe/binary/configure.cmd b/src/tools/windows/converter_exe/binary/configure.cmd
deleted file mode 100644
index 39b1d2a5..00000000
--- a/src/tools/windows/converter_exe/binary/configure.cmd
+++ /dev/null
@@ -1,33 +0,0 @@
-@if "%ECHOON%"=="" @echo off

-SETLOCAL

-

-REM ******************************************************************

-REM Please, make sure to run this in an Elevated Command Prompt.

-REM Usage:

-REM        configure.cmd

-REM ******************************************************************

-

-REM ******************************************************************

-REM Initialize

-REM ******************************************************************

-SET SCRIPT_LOCATION=%~dp0

-

-REM ******************************************************************

-REM Go to script location

-REM ******************************************************************

-pushd %SCRIPT_LOCATION%

-

-REM ******************************************************************

-REM Register msdia120.dll.

-REM ******************************************************************

-SET MSG=Failed to register msdia120.dll.  Make sure to run this in elevated command prompt.

-%systemroot%\SysWoW64\regsvr32.exe /s msdia120.dll & if errorlevel 1 echo %MSG% & goto :fail

-

-:success

-echo Configuration was successful.

-ENDLOCAL

-exit /b 0

-

-:fail

-ENDLOCAL

-exit /b 1

diff --git a/src/tools/windows/converter_exe/binary/missing_symbols_test.txt b/src/tools/windows/converter_exe/binary/missing_symbols_test.txt
deleted file mode 100644
index 251b4eca..00000000
--- a/src/tools/windows/converter_exe/binary/missing_symbols_test.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-msctf.pdb|6A5BABB8E88644C696530BFE3C90F32F2|6.1.7600.16385|msctf.dll|4A5BDFAA109000

-imm32.pdb|98F27BA5AEE541ECBEE00CD03AD50FEE2|6.1.7600.16385|imm32.dll|4A5BDF402e000

diff --git a/src/tools/windows/converter_exe/binary/sleep.exe b/src/tools/windows/converter_exe/binary/sleep.exe
deleted file mode 100644
index d178e17e..00000000
--- a/src/tools/windows/converter_exe/binary/sleep.exe
+++ /dev/null
diff --git a/src/tools/windows/converter_exe/binary/symsrv.yes b/src/tools/windows/converter_exe/binary/symsrv.yes
deleted file mode 100644
index 1d01dda7..00000000
--- a/src/tools/windows/converter_exe/binary/symsrv.yes
+++ /dev/null
@@ -1,2 +0,0 @@
-See breakpad/tools/windows/converter/ms_symbol_server_converter.h for a

-description of this file's function.

diff --git a/src/tools/windows/converter_exe/configure.cmd b/src/tools/windows/converter_exe/configure.cmd
index 39b1d2a5..5464a61e 100644
--- a/src/tools/windows/converter_exe/configure.cmd
+++ b/src/tools/windows/converter_exe/configure.cmd
@@ -18,10 +18,10 @@ REM ******************************************************************
 pushd %SCRIPT_LOCATION%

 

 REM ******************************************************************

-REM Register msdia120.dll.

+REM Register msdia140.dll.

 REM ******************************************************************

-SET MSG=Failed to register msdia120.dll.  Make sure to run this in elevated command prompt.

-%systemroot%\SysWoW64\regsvr32.exe /s msdia120.dll & if errorlevel 1 echo %MSG% & goto :fail

+SET MSG=Failed to register msdia140.dll.  Make sure to run this in elevated command prompt.

+%systemroot%\SysWoW64\regsvr32.exe /s msdia140.dll & if errorlevel 1 echo %MSG% & goto :fail

 

 :success

 echo Configuration was successful.

diff --git a/src/tools/windows/converter_exe/converter.cc b/src/tools/windows/converter_exe/converter.cc
index 6c30b270..9418ecaa 100644
--- a/src/tools/windows/converter_exe/converter.cc
+++ b/src/tools/windows/converter_exe/converter.cc
@@ -292,8 +292,7 @@ static bool SafeToMakeExternalRequest(const MissingSymbolInfo &missing_info,
 // Converter options derived from command line parameters.

 struct ConverterOptions {

   ConverterOptions()

-      : report_fetch_failures(true),

-        blacklist_regex(nullptr) {

+      : report_fetch_failures(true) {

   }

 

   ~ConverterOptions() {

@@ -704,6 +703,7 @@ int main(int argc, char **argv) {
   if (argc % 2 != 1) {

     return usage(argv[0]);

   }

+

   string blacklist_regex_str;

   bool have_any_msss_servers = false;

   for (int argi = 1; argi < argc; argi += 2) {

@@ -781,8 +781,9 @@ int main(int argc, char **argv) {
     FprintfFlush(stderr, "No upload symbols URL specified.\n");

     return usage(argv[0]);

   }

-  if (options.missing_symbols_url.empty()) {

-    FprintfFlush(stderr, "No missing symbols URL specified.\n");

+  if (options.missing_symbols_url.empty() &&

+      options.missing_symbols_file.empty()) {

+    FprintfFlush(stderr, "No missing symbols URL or file specified.\n");

     return usage(argv[0]);

   }

   if (options.fetch_symbol_failure_url.empty()) {

diff --git a/src/tools/windows/converter_exe/missing_symbols_test.txt b/src/tools/windows/converter_exe/missing_symbols_test.txt
index 251b4eca..91641fca 100644
--- a/src/tools/windows/converter_exe/missing_symbols_test.txt
+++ b/src/tools/windows/converter_exe/missing_symbols_test.txt
@@ -1,2 +1,5 @@
 msctf.pdb|6A5BABB8E88644C696530BFE3C90F32F2|6.1.7600.16385|msctf.dll|4A5BDFAA109000

 imm32.pdb|98F27BA5AEE541ECBEE00CD03AD50FEE2|6.1.7600.16385|imm32.dll|4A5BDF402e000

+amd_opencl64.pdb|3D306D0FCCB14F47AF322A5ACDF5EEA81||amd_opencl64.dll|587901FB1E000

+igd10iumd64.pdb|B2B72475BB0846D8ADE4344FAE0CCE361 ||igd10iumd64.dll|568D69FBD99000

+NvCameraWhitelisting64.pdb|3C364C4D3FBF4180B021D52D469C6DAB1||NvCameraWhitelisting64.dll|5B8ED23485000
\ No newline at end of file
diff --git a/src/tools/windows/converter_exe/winsymconv.cmd b/src/tools/windows/converter_exe/winsymconv.cmd
new file mode 100644
index 00000000..bea84b58
--- /dev/null
+++ b/src/tools/windows/converter_exe/winsymconv.cmd
@@ -0,0 +1,86 @@
+@if "%ECHOON%"=="" @echo off

+SETLOCAL

+

+REM ******************************************************************

+REM Usage:

+REM        winsymconv

+REM ******************************************************************

+

+REM ******************************************************************

+REM Initialize

+REM ******************************************************************

+SET SCRIPT_LOCATION=%~dp0

+SET DBGHELP_WINHTTP=

+SET USE_WINHTTP=

+

+REM ******************************************************************

+REM Go to script location

+REM ******************************************************************

+pushd %SCRIPT_LOCATION%

+

+REM ******************************************************************

+REM  Make sure the symbol file directory exists

+REM ******************************************************************

+SET SYMBOL_DIR=%SCRIPT_LOCATION%symbol

+if NOT EXIST %SYMBOL_DIR% MKDIR %SYMBOL_DIR%

+if NOT EXIST %SYMBOL_DIR% echo Failed to create directory '%SYMBOL_DIR%' & goto :fail

+

+:restart

+

+REM ******************************************************************

+REM Convert missing Windows symbols on the staging instance.

+REM ******************************************************************

+echo Converting missing Windows symbols on staging instance ...

+

+google_converter.exe ^

+    -n http://msdl.microsoft.com/download/symbols ^

+    -n http://symbols.mozilla.org/firefox ^

+    -n http://chromium-browser-symsrv.commondatastorage.googleapis.com ^

+    -n https://download.amd.com/dir/bin ^

+    -n https://driver-symbols.nvidia.com ^

+    -n https://software.intel.com/sites/downloads/symbols ^

+    -l %SYMBOL_DIR% ^

+    -s https://clients2.google.com/cr/staging_symbol ^

+    -m https://clients2.google.com/cr/staging_symbol/missingsymbols ^

+    -t https://clients2.google.com/cr/staging_symbol/fetchfailed ^

+    -b "google|chrome|internal|private" ^

+    > %SCRIPT_LOCATION%last_cycle_staging.txt

+

+REM ******************************************************************

+REM Convert missing Windows symbols on the production instance.

+REM ******************************************************************

+echo Converting missing Windows symbols on production instance ...

+

+google_converter.exe ^

+    -n http://msdl.microsoft.com/download/symbols ^

+    -n http://symbols.mozilla.org/firefox ^

+    -n http://chromium-browser-symsrv.commondatastorage.googleapis.com ^

+    -n https://download.amd.com/dir/bin ^

+    -n https://driver-symbols.nvidia.com ^

+    -n https://software.intel.com/sites/downloads/symbols ^

+    -l %SYMBOL_DIR% ^

+    -s https://clients2.google.com/cr/symbol ^

+    -m https://clients2.google.com/cr/symbol/missingsymbols ^

+    -t https://clients2.google.com/cr/symbol/fetchfailed ^

+    -b "google|chrome|internal|private" ^

+    > %SCRIPT_LOCATION%last_cycle_prod.txt

+

+REM ******************************************************************

+REM Sleep for 5 minutes ...

+REM ******************************************************************

+echo Sleeping for 5 minutes ...

+

+%SCRIPT_LOCATION%sleep.exe 300

+

+REM ******************************************

+REM  Restart work loop ...

+REM ******************************************

+goto :restart

+

+:success

+ENDLOCAL

+exit /b 0

+

+:fail

+ENDLOCAL

+exit /b 1

diff --git a/src/tools/windows/converter_exe/winsymconv_test.cmd b/src/tools/windows/converter_exe/winsymconv_test.cmd
new file mode 100644
index 00000000..c1777066
--- /dev/null
+++ b/src/tools/windows/converter_exe/winsymconv_test.cmd
@@ -0,0 +1,72 @@
+@if "%ECHOON%"=="" @echo off

+SETLOCAL

+

+REM ******************************************************************

+REM Usage:

+REM        winsymconv_test

+REM ******************************************************************

+

+REM ******************************************************************

+REM Initialize

+REM ******************************************************************

+SET SCRIPT_LOCATION=%~dp0

+SET DBGHELP_WINHTTP=

+SET USE_WINHTTP=

+

+REM ******************************************************************

+REM Go to script location

+REM ******************************************************************

+pushd %SCRIPT_LOCATION%

+

+REM ******************************************************************

+REM  Make sure the symbol file directory exists

+REM ******************************************************************

+SET SYMBOL_DIR=%SCRIPT_LOCATION%symbol

+if NOT EXIST %SYMBOL_DIR% MKDIR %SYMBOL_DIR%

+if NOT EXIST %SYMBOL_DIR% echo Failed to create directory '%SYMBOL_DIR%' & goto :fail

+

+REM ******************************************************************

+REM Testing on the staging instance.

+REM ******************************************************************

+echo Testing on the staging instance ...

+

+google_converter.exe ^

+    -n http://msdl.microsoft.com/download/symbols ^

+    -n http://symbols.mozilla.org/firefox ^

+    -n http://chromium-browser-symsrv.commondatastorage.googleapis.com ^

+    -n https://download.amd.com/dir/bin ^

+    -n https://driver-symbols.nvidia.com ^

+    -n https://software.intel.com/sites/downloads/symbols ^

+    -l %SYMBOL_DIR% ^

+    -s https://clients2.google.com/cr/staging_symbol ^

+    -mf %SCRIPT_LOCATION%missing_symbols_test.txt ^

+    -t https://clients2.google.com/cr/staging_symbol/fetchfailed ^

+    -b "google|chrome|internal|private" ^

+    > %SCRIPT_LOCATION%last_cycle_staging.txt

+

+REM ******************************************************************

+REM Testing on the production instance.

+REM ******************************************************************

+echo Testing on the production instance ...

+

+google_converter.exe ^

+    -n http://msdl.microsoft.com/download/symbols ^

+    -n http://symbols.mozilla.org/firefox ^

+    -n http://chromium-browser-symsrv.commondatastorage.googleapis.com ^

+    -n https://download.amd.com/dir/bin ^

+    -n https://driver-symbols.nvidia.com ^

+    -n https://software.intel.com/sites/downloads/symbols ^

+    -l %SYMBOL_DIR% ^

+    -s https://clients2.google.com/cr/symbol ^

+    -mf %SCRIPT_LOCATION%missing_symbols_test.txt ^

+    -t https://clients2.google.com/cr/symbol/fetchfailed ^

+    -b "google|chrome|internal|private" ^

+    > %SCRIPT_LOCATION%last_cycle_prod.txt

+

+:success

+ENDLOCAL

+exit /b 0

+

+:fail

+ENDLOCAL

+exit /b 1