From c38725b70e596457c1c0b1685dd2e017966324d1 Mon Sep 17 00:00:00 2001
From: Nelson Billing <nbilling@google.com>
Date: Wed, 19 Jun 2019 16:13:24 -0700
Subject: [PATCH] Fix 'debug_file' in PESourceLineWriter.

- Add a #define to testing.gyp to avoid warnings about TR1 deprecation.
- PESourceLineWriter now reads debug_file from CodeView record instead
  of using code_file value.
- Updated PE-only MD reading unit test.

Change-Id: Ib4e6201df3e3fd651e160f310584b5a67b16c842
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1668347
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
---
 src/client/windows/unittests/testing.gyp      |  8 ++---
 src/common/windows/pe_util.cc                 | 34 +++++++++++++------
 .../testdata/pe_only_symbol_test.sym          |  2 +-
 3 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/src/client/windows/unittests/testing.gyp b/src/client/windows/unittests/testing.gyp
index cd8aca22..0f9f944c 100644
--- a/src/client/windows/unittests/testing.gyp
+++ b/src/client/windows/unittests/testing.gyp
@@ -53,9 +53,9 @@
         # Visual C++ implements variadic templates strangely, and
         # VC++2012 broke Google Test by lowering this value. See
         # http://stackoverflow.com/questions/12558327/google-test-in-visual-studio-2012
-        'defines': ['_VARIADIC_MAX=10'],
+        'defines': ['_VARIADIC_MAX=10', '_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING'],
       },
-      'defines': ['_VARIADIC_MAX=10'],
+      'defines': ['_VARIADIC_MAX=10', '_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING'],
     },
     {
       'target_name': 'gmock',
@@ -81,9 +81,9 @@
           '<(DEPTH)/testing/googlemock',
           '<(DEPTH)/testing',
         ],
-        'defines': ['_VARIADIC_MAX=10'],
+        'defines': ['_VARIADIC_MAX=10', '_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING'],
       },
-      'defines': ['_VARIADIC_MAX=10'],
+      'defines': ['_VARIADIC_MAX=10', '_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING'],
     },
 
   ],
diff --git a/src/common/windows/pe_util.cc b/src/common/windows/pe_util.cc
index 11181954..27f702a3 100644
--- a/src/common/windows/pe_util.cc
+++ b/src/common/windows/pe_util.cc
@@ -95,7 +95,7 @@ struct CV_INFO_PDB70 {
   ULONG cv_signature;
   GUID signature;
   ULONG age;
-  UCHAR pdb_filename[ANYSIZE_ARRAY];
+  CHAR pdb_filename[ANYSIZE_ARRAY];
 };
 
 #define CV_SIGNATURE_RSDS 'SDSR'
@@ -123,8 +123,6 @@ using std::unique_ptr;
 using google_breakpad::GUIDString;
 
 bool ReadModuleInfo(const wstring & pe_file, PDBModuleInfo * info) {
-  info->debug_file = WindowsStringUtils::GetBaseName(pe_file);
-
   // Convert wchar to native charset because ImageLoad only takes
   // a PSTR as input.
   string img_file;
@@ -141,10 +139,10 @@ bool ReadModuleInfo(const wstring & pe_file, PDBModuleInfo * info) {
   }
 
   info->cpu = FileHeaderMachineToCpuString(
-    img->FileHeader->FileHeader.Machine);
+      img->FileHeader->FileHeader.Machine);
 
   PIMAGE_OPTIONAL_HEADER64 optional_header =
-    &(reinterpret_cast<PIMAGE_NT_HEADERS64>(img->FileHeader))->OptionalHeader;
+      &(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;
@@ -164,21 +162,37 @@ bool ReadModuleInfo(const wstring & pe_file, PDBModuleInfo * info) {
 
   for (DWORD i = 0; i < debug_size / sizeof(*debug_directories); i++) {
     if (debug_directories[i].Type != IMAGE_DEBUG_TYPE_CODEVIEW ||
-      debug_directories[i].SizeOfData < sizeof(CV_INFO_PDB70)) {
+        debug_directories[i].SizeOfData < sizeof(CV_INFO_PDB70)) {
       continue;
     }
 
     struct CV_INFO_PDB70* cv_info = static_cast<CV_INFO_PDB70*>(ImageRvaToVa(
-      img->FileHeader,
-      img->MappedAddress,
-      debug_directories[i].AddressOfRawData,
-      &img->LastRvaSection));
+        img->FileHeader,
+        img->MappedAddress,
+        debug_directories[i].AddressOfRawData,
+        &img->LastRvaSection));
     if (cv_info->cv_signature != CV_SIGNATURE_RSDS) {
       continue;
     }
 
     info->debug_identifier = GenerateDebugIdentifier(cv_info->age,
         cv_info->signature);
+
+    // This code assumes that the pdb_filename is stored as ASCII without
+    // multibyte characters, but it's not clear if that's true.
+    size_t debug_file_length = strnlen_s(cv_info->pdb_filename, MAX_PATH);
+    if (debug_file_length < 0 || debug_file_length >= MAX_PATH) {
+      fprintf(stderr, "PE debug directory is corrupt.\n");
+      return false;
+    }
+    std::string debug_file(cv_info->pdb_filename, debug_file_length);
+    if (!WindowsStringUtils::safe_mbstowcs(debug_file, &info->debug_file)) {
+      fprintf(stderr, "PDB filename '%s' contains unrecognized characters.\n",
+          debug_file.c_str());
+      return false;
+    }
+    info->debug_file = WindowsStringUtils::GetBaseName(info->debug_file);
+
     return true;
   }
 
diff --git a/src/tools/windows/dump_syms/testdata/pe_only_symbol_test.sym b/src/tools/windows/dump_syms/testdata/pe_only_symbol_test.sym
index 24a3ddf4..745aa4bb 100644
--- a/src/tools/windows/dump_syms/testdata/pe_only_symbol_test.sym
+++ b/src/tools/windows/dump_syms/testdata/pe_only_symbol_test.sym
@@ -1,4 +1,4 @@
-MODULE windows x86_64 2A5EAB481FAB4A17A9761CDC14FE531A1 pe_only_symbol_test.dll
+MODULE windows x86_64 2A5EAB481FAB4A17A9761CDC14FE531A1 pe_only_symbol_test.pdb
 INFO CODE_ID 5C8AD05F12000 pe_only_symbol_test.dll
 STACK CFI INIT 1440 39 .cfa: $rsp .ra: .cfa 8 - ^
 STACK CFI 1440 .cfa: $rsp 32 +