Refactor: Rename all elf-exported crash thunks with an ExportThunk suffix.

This should be unique enough that the link will fail if the dependencies
are goofed. Turns out the Visual Studio linker is overly "helpful"
and tries very hard to make the link succeed even in the absence of
the functions named in a .DEF file. See https://crbug.com/760385#c11.
This also fixes the "upload now" link in chrome://crashes, which
regressed at 86ac09aa98

TBR=brettw@chromium.org

Bug: 753363
Cq-Include-Trybots: master.tryserver.chromium.win:win10_chromium_x64_rel_ng
Change-Id: Ic5a0e2352e9f7956af3d57668a16b7df5397b9e8
Reviewed-on: https://chromium-review.googlesource.com/647332
Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499254}

chrome/browser/chrome_browser_main_win.cc

@@ -90,7 +90,7 @@ typedef HRESULT (STDAPICALLTYPE* RegisterApplicationRestartProc)(
 
 void InitializeWindowProcExceptions() {
   base::win::WinProcExceptionFilter exception_filter =
-      base::win::SetWinProcExceptionFilter(&CrashForException);
+      base::win::SetWinProcExceptionFilter(&CrashForException_ExportThunk);
   DCHECK(!exception_filter);
 }
 

chrome/browser/hang_monitor/hang_crash_dump_win.cc

@@ -65,10 +65,10 @@ void CrashDumpAndTerminateHungChildProcess(
 
   HANDLE remote_thread = nullptr;
   if (send_remote_memory) {
-    remote_thread = InjectDumpForHungInput(hprocess, remote_memory);
+    remote_thread = InjectDumpForHungInput_ExportThunk(hprocess, remote_memory);
   } else {
-    remote_thread =
+    remote_thread = InjectDumpForHungInputNoCrashKeys_ExportThunk(
-        InjectDumpForHungInputNoCrashKeys(hprocess, crash_key_failure);
+        hprocess, crash_key_failure);
   }
   DCHECK(remote_thread) << "Failed creating remote thread: error "
                         << GetLastError();

chrome/browser/metrics/chrome_metrics_services_manager_client.cc

@@ -265,7 +265,7 @@ void ChromeMetricsServicesManagerClient::UpdateRunningServices(
   // Crashpad will use the kRegUsageStatsInSample registry value to apply
   // sampling correctly, but may_record already reflects the sampling state.
   // This isn't a problem though, since they will be consistent.
-  SetUploadConsentImpl(may_record && may_upload);
+  SetUploadConsent_ExportThunk(may_record && may_upload);
 }
 #endif  // defined(OS_WIN)
 

chrome/child/v8_breakpad_support_win.cc

@@ -16,8 +16,10 @@ namespace v8_breakpad_support {
 
 void SetUp() {
 #if defined(ARCH_CPU_X86_64)
-  gin::Debug::SetCodeRangeCreatedCallback(&RegisterNonABICompliantCodeRange);
+  gin::Debug::SetCodeRangeCreatedCallback(
-  gin::Debug::SetCodeRangeDeletedCallback(&UnregisterNonABICompliantCodeRange);
+      &RegisterNonABICompliantCodeRange_ExportThunk);
+  gin::Debug::SetCodeRangeDeletedCallback(
+      &UnregisterNonABICompliantCodeRange_ExportThunk);
 #endif
 }
 

chrome/common/child_process_logging_win.cc

@@ -22,11 +22,11 @@ namespace {
 
 void SetCrashKeyValueTrampoline(const base::StringPiece& key,
                                 const base::StringPiece& value) {
-  SetCrashKeyValueImplEx(key.data(), value.data());
+  SetCrashKeyValueEx_ExportThunk(key.data(), value.data());
 }
 
 void ClearCrashKeyValueTrampoline(const base::StringPiece& key) {
-  ClearCrashKeyValueImplEx(key.data());
+  ClearCrashKeyValueEx_ExportThunk(key.data());
 }
 
 }  // namespace

chrome_elf/chrome_elf_x64.def

@@ -7,21 +7,27 @@ EXPORTS
   ; When functions are added to this file, they must also be added to
   ; chrome_elf_x86.def
 
-  ; From components/crash/content/app/crashpad.cc
+  ; From components/crash/content/app/crash_export_stubs.cc
-  ClearCrashKeyValueImpl
+  ClearCrashKeyValueEx_ExportThunk
-  ClearCrashKeyValueImplEx
+  ClearCrashKeyValue_ExportThunk
-  RequestSingleCrashUploadImpl
+  CrashForException_ExportThunk
-  SetCrashKeyValueImpl
+  GetCrashReports_ExportThunk
-  SetCrashKeyValueImplEx
+  InjectDumpForHungInputNoCrashKeys_ExportThunk
-  SetUploadConsentImpl
+  InjectDumpForHungInput_ExportThunk
+  RequestSingleCrashUpload_ExportThunk
+  SetCrashKeyValueEx_ExportThunk
+  SetCrashKeyValue_ExportThunk
+  SetUploadConsent_ExportThunk
 
-  ; From components/crash/content/app/crashpad_win.cc
+  ; X64-only exports
-  CrashForException
+  RegisterNonABICompliantCodeRange_ExportThunk
-  InjectDumpForHungInput
+  UnregisterNonABICompliantCodeRange_ExportThunk
-  InjectDumpForHungInputNoCrashKeys
+
+  ; This export is used by SyzyASAN, do not rename or remove without talking
+  ; to syzygy-team@chromium.org first.
+  SetCrashKeyValue = SetCrashKeyValue_ExportThunk
 
   ; From chrome_elf/crash_helper.cc
-  GetCrashReportsImpl
   SetMetricsClientId
 
   ; From chrome_elf_main.cc
@@ -38,8 +44,3 @@ EXPORTS
   GetBlacklistIndex
   IsBlacklistInitialized
   SuccessfullyBlocked
-
-  ; X64-only exports
-  ; From components/crash/content/app/crashpad_win.cc
-  RegisterNonABICompliantCodeRange
-  UnregisterNonABICompliantCodeRange

chrome_elf/chrome_elf_x86.def

@@ -1,28 +1,29 @@
 ; Copyright 2013 The Chromium Authors. All rights reserved.
 ; Use of this source code is governed by a BSD-style license that can be
 ; found in the LICENSE file.
-
 LIBRARY  "chrome_elf.dll"
 
 EXPORTS
   ; When functions are added to this file, they must also be added to
-  ; chrome_elf_x64.def
+  ; chrome_elf_x86.def
 
-  ; From components/crash/content/app/crashpad.cc
+  ; From components/crash/content/app/crash_export_stubs.cc
-  ClearCrashKeyValueImpl
+  ClearCrashKeyValueEx_ExportThunk
-  ClearCrashKeyValueImplEx
+  ClearCrashKeyValue_ExportThunk
-  RequestSingleCrashUploadImpl
+  CrashForException_ExportThunk
-  SetCrashKeyValueImpl
+  GetCrashReports_ExportThunk
-  SetCrashKeyValueImplEx
+  InjectDumpForHungInputNoCrashKeys_ExportThunk
-  SetUploadConsentImpl
+  InjectDumpForHungInput_ExportThunk
+  RequestSingleCrashUpload_ExportThunk
+  SetCrashKeyValueEx_ExportThunk
+  SetCrashKeyValue_ExportThunk
+  SetUploadConsent_ExportThunk
 
-  ; From components/crash/content/app/crashpad_win.cc
+  ; This export is used by SyzyASAN, do not rename or remove without talking
-  CrashForException
+  ; to syzygy-team@chromium.org first.
-  InjectDumpForHungInput
+  SetCrashKeyValue = SetCrashKeyValue_ExportThunk
-  InjectDumpForHungInputNoCrashKeys
 
   ; From chrome_elf/crash_helper.cc
-  GetCrashReportsImpl
   SetMetricsClientId
 
   ; From chrome_elf_main.cc

components/crash/content/app/crash_export_stubs.cc

@@ -12,41 +12,44 @@
 #include "components/crash/content/app/crash_export_thunks.h"
 #include "components/crash/content/app/crashpad.h"
 
-void RequestSingleCrashUploadThunk(const char* local_id) {}
+void RequestSingleCrashUpload_ExportThunk(const char* local_id) {}
 
-size_t GetCrashReportsImpl(crash_reporter::Report* reports,
+size_t GetCrashReports_ExportThunk(crash_reporter::Report* reports,
-                           size_t reports_size) {
+                                   size_t reports_size) {
   return 0;
 }
 
-int CrashForException(EXCEPTION_POINTERS* info) {
+int CrashForException_ExportThunk(EXCEPTION_POINTERS* info) {
   // Make sure to properly crash the process by dispatching directly to the
   // Windows unhandled exception filter.
   return UnhandledExceptionFilter(info);
 }
 
-void SetUploadConsentImpl(bool consent) {}
+void SetUploadConsent_ExportThunk(bool consent) {}
 
-void SetCrashKeyValueImpl(const wchar_t* key, const wchar_t* value) {}
+void SetCrashKeyValue_ExportThunk(const wchar_t* key, const wchar_t* value) {}
 
-void ClearCrashKeyValueImpl(const wchar_t* key) {}
+void ClearCrashKeyValue_ExportThunk(const wchar_t* key) {}
 
-void SetCrashKeyValueImplEx(const char* key, const char* value) {}
+void SetCrashKeyValueEx_ExportThunk(const char* key, const char* value) {}
 
-void ClearCrashKeyValueImplEx(const char* key) {}
+void ClearCrashKeyValueEx_ExportThunk(const char* key) {}
 
-HANDLE InjectDumpForHungInput(HANDLE process, void* serialized_crash_keys) {
+HANDLE InjectDumpForHungInput_ExportThunk(HANDLE process,
+                                          void* serialized_crash_keys) {
   return nullptr;
 }
 
-HANDLE InjectDumpForHungInputNoCrashKeys(HANDLE process, int reason) {
+HANDLE InjectDumpForHungInputNoCrashKeys_ExportThunk(HANDLE process,
+                                                     int reason) {
   return nullptr;
 }
 
 #if defined(ARCH_CPU_X86_64)
 
-void RegisterNonABICompliantCodeRange(void* start, size_t size_in_bytes) {}
+void RegisterNonABICompliantCodeRange_ExportThunk(void* start,
+                                                  size_t size_in_bytes) {}
 
-void UnregisterNonABICompliantCodeRange(void* start) {}
+void UnregisterNonABICompliantCodeRange_ExportThunk(void* start) {}
 
 #endif  // defined(ARCH_CPU_X86_64)

components/crash/content/app/crash_export_thunks.cc

@@ -12,12 +12,12 @@
 #include "components/crash/content/app/crashpad.h"
 #include "third_party/crashpad/crashpad/client/crashpad_client.h"
 
-void RequestSingleCrashUploadImpl(const char* local_id) {
+void RequestSingleCrashUpload_ExportThunk(const char* local_id) {
   crash_reporter::RequestSingleCrashUploadImpl(local_id);
 }
 
-size_t GetCrashReportsImpl(crash_reporter::Report* reports,
+size_t GetCrashReports_ExportThunk(crash_reporter::Report* reports,
-                           size_t reports_size) {
+                                   size_t reports_size) {
   static_assert(std::is_pod<crash_reporter::Report>::value,
                 "crash_reporter::Report must be POD");
   // Since this could be called across module boundaries, retrieve the full
@@ -36,7 +36,7 @@ size_t GetCrashReportsImpl(crash_reporter::Report* reports,
   return crash_reports.size();
 }
 
-int CrashForException(EXCEPTION_POINTERS* info) {
+int CrashForException_ExportThunk(EXCEPTION_POINTERS* info) {
   crash_reporter::GetCrashpadClient().DumpAndCrash(info);
   return EXCEPTION_CONTINUE_SEARCH;
 }
@@ -48,7 +48,7 @@ int CrashForException(EXCEPTION_POINTERS* info) {
 // be consistent with
 // crash_reporter::GetCrashReporterClient()->GetCollectStatsConsent(), but it's
 // not enforced to avoid blocking startup code on synchronizing them.
-void SetUploadConsentImpl(bool consent) {
+void SetUploadConsent_ExportThunk(bool consent) {
   crash_reporter::SetUploadConsent(consent);
 }
 
@@ -56,31 +56,33 @@ void SetUploadConsentImpl(bool consent) {
 // change the name or signature of this function you will break SyzyASAN
 // instrumented releases of Chrome. Please contact syzygy-team@chromium.org
 // before doing so! See also http://crbug.com/567781.
-void SetCrashKeyValueImpl(const wchar_t* key, const wchar_t* value) {
+void SetCrashKeyValue_ExportThunk(const wchar_t* key, const wchar_t* value) {
   crash_reporter::SetCrashKeyValue(base::UTF16ToUTF8(key),
                                    base::UTF16ToUTF8(value));
 }
 
-void ClearCrashKeyValueImpl(const wchar_t* key) {
+void ClearCrashKeyValue_ExportThunk(const wchar_t* key) {
   crash_reporter::ClearCrashKey(base::UTF16ToUTF8(key));
 }
 
-void SetCrashKeyValueImplEx(const char* key, const char* value) {
+void SetCrashKeyValueEx_ExportThunk(const char* key, const char* value) {
   crash_reporter::SetCrashKeyValue(key, value);
 }
 
-void ClearCrashKeyValueImplEx(const char* key) {
+void ClearCrashKeyValueEx_ExportThunk(const char* key) {
   crash_reporter::ClearCrashKey(key);
 }
 
-HANDLE InjectDumpForHungInput(HANDLE process, void* serialized_crash_keys) {
+HANDLE InjectDumpForHungInput_ExportThunk(HANDLE process,
+                                          void* serialized_crash_keys) {
   return CreateRemoteThread(
       process, nullptr, 0,
       crash_reporter::internal::DumpProcessForHungInputThread,
       serialized_crash_keys, 0, nullptr);
 }
 
-HANDLE InjectDumpForHungInputNoCrashKeys(HANDLE process, int reason) {
+HANDLE InjectDumpForHungInputNoCrashKeys_ExportThunk(HANDLE process,
+                                                     int reason) {
   return CreateRemoteThread(
       process, nullptr, 0,
       crash_reporter::internal::DumpProcessForHungInputNoCrashKeysThread,
@@ -89,12 +91,13 @@ HANDLE InjectDumpForHungInputNoCrashKeys(HANDLE process, int reason) {
 
 #if defined(ARCH_CPU_X86_64)
 
-void RegisterNonABICompliantCodeRange(void* start, size_t size_in_bytes) {
+void RegisterNonABICompliantCodeRange_ExportThunk(void* start,
+                                                  size_t size_in_bytes) {
   crash_reporter::internal::RegisterNonABICompliantCodeRangeImpl(start,
                                                                  size_in_bytes);
 }
 
-void UnregisterNonABICompliantCodeRange(void* start) {
+void UnregisterNonABICompliantCodeRange_ExportThunk(void* start) {
   crash_reporter::internal::UnregisterNonABICompliantCodeRangeImpl(start);
 }
 

components/crash/content/app/crash_export_thunks.h

@@ -16,19 +16,22 @@ struct Report;
 
 extern "C" {
 
-// TODO(siggi): Rename these functions to something descriptive and unique,
+// All the functions in this file are named with a suffix of _ExportThunk to
-//   once all the thunks have been converted to import binding.
+// make sure their names cannot easily collide with random other functions.
+// The Microsoft Visual Studio linker has a misfeature where it searches rather
+// aggressively for functions to match exports in .DEF files.
+// See https://crbug.com/760385#c11 for how this can be bad.
 
 // This function may be invoked across module boundaries to request a single
 // crash report upload. See CrashUploadListCrashpad.
-void RequestSingleCrashUploadImpl(const char* local_id);
+void RequestSingleCrashUpload_ExportThunk(const char* local_id);
 
 // This function may be invoked across module boundaries to retrieve the crash
 // list. It copies up to |report_count| reports into |reports| and returns the
 // number of reports available. If the return value is less than or equal to
 // |reports_size|, then |reports| contains all the available reports.
-size_t GetCrashReportsImpl(crash_reporter::Report* reports,
+size_t GetCrashReports_ExportThunk(crash_reporter::Report* reports,
-                           size_t reports_size);
+                                   size_t reports_size);
 
 // Crashes the process after generating a dump for the provided exception. Note
 // that the crash reporter should be initialized before calling this function
@@ -36,7 +39,7 @@ size_t GetCrashReportsImpl(crash_reporter::Report* reports,
 // NOTE: This function is used by SyzyASAN to invoke a crash. If you change the
 // the name or signature of this function you will break SyzyASAN instrumented
 // releases of Chrome. Please contact syzygy-team@chromium.org before doing so!
-int CrashForException(EXCEPTION_POINTERS* info);
+int CrashForException_ExportThunk(EXCEPTION_POINTERS* info);
 
 // This function is used in chrome_metrics_services_manager_client.cc to trigger
 // changes to the upload-enabled state. This is done when the metrics services
@@ -45,19 +48,19 @@ int CrashForException(EXCEPTION_POINTERS* info);
 // be consistent with
 // crash_reporter::GetCrashReporterClient()->GetCollectStatsConsent(), but it's
 // not enforced to avoid blocking startup code on synchronizing them.
-void SetUploadConsentImpl(bool consent);
+void SetUploadConsent_ExportThunk(bool consent);
 
 // NOTE: This function is used by SyzyASAN to annotate crash reports. If you
 // change the name or signature of this function you will break SyzyASAN
 // instrumented releases of Chrome. Please contact syzygy-team@chromium.org
 // before doing so! See also http://crbug.com/567781.
-void SetCrashKeyValueImpl(const wchar_t* key, const wchar_t* value);
+void SetCrashKeyValue_ExportThunk(const wchar_t* key, const wchar_t* value);
 
-void ClearCrashKeyValueImpl(const wchar_t* key);
+void ClearCrashKeyValue_ExportThunk(const wchar_t* key);
 
-void SetCrashKeyValueImplEx(const char* key, const char* value);
+void SetCrashKeyValueEx_ExportThunk(const char* key, const char* value);
 
-void ClearCrashKeyValueImplEx(const char* key);
+void ClearCrashKeyValueEx_ExportThunk(const char* key);
 
 // Injects a thread into a remote process to dump state when there is no crash.
 // |serialized_crash_keys| is a nul terminated string in the address space of
@@ -65,17 +68,20 @@ void ClearCrashKeyValueImplEx(const char* key);
 // Keys and values are separated by ':', and key/value pairs are separated by
 // ','. All keys should be previously registered as crash keys.
 // This method is used solely to classify hung input.
-HANDLE InjectDumpForHungInput(HANDLE process, void* serialized_crash_keys);
+HANDLE InjectDumpForHungInput_ExportThunk(HANDLE process,
+                                          void* serialized_crash_keys);
 
 // Injects a thread into a remote process to dump state when there is no crash.
 // This method provides |reason| which will interpreted as an integer and logged
 // as a crash key.
-HANDLE InjectDumpForHungInputNoCrashKeys(HANDLE process, int reason);
+HANDLE InjectDumpForHungInputNoCrashKeys_ExportThunk(HANDLE process,
+                                                     int reason);
 
 #if defined(ARCH_CPU_X86_64)
 // V8 support functions.
-void RegisterNonABICompliantCodeRange(void* start, size_t size_in_bytes);
+void RegisterNonABICompliantCodeRange_ExportThunk(void* start,
-void UnregisterNonABICompliantCodeRange(void* start);
+                                                  size_t size_in_bytes);
+void UnregisterNonABICompliantCodeRange_ExportThunk(void* start);
 #endif  // defined(ARCH_CPU_X86_64)
 
 }  // extern "C"

components/crash/content/app/crashpad.cc

@@ -261,7 +261,7 @@ void GetReports(std::vector<Report>* reports) {
   reports->resize(25);
   while (true) {
     size_t available_reports =
-        GetCrashReportsImpl(&reports->at(0), reports->size());
+        GetCrashReports_ExportThunk(&reports->at(0), reports->size());
     if (available_reports <= reports->size()) {
       // The input size was large enough to capture all available crashes.
       // Trim the vector to the actual number of reports returned and return.
@@ -283,7 +283,7 @@ void RequestSingleCrashUpload(const std::string& local_id) {
 #if defined(OS_WIN)
   // On Windows, crash reporting may be implemented in another module, which is
   // why this can't call crash_reporter::RequestSingleCrashUpload directly.
-  RequestSingleCrashUploadImpl(local_id);
+  RequestSingleCrashUpload_ExportThunk(local_id.c_str());
 #else
   crash_reporter::RequestSingleCrashUploadImpl(local_id);
 #endif

components/crash/content/app/crashpad_win.cc

@@ -205,7 +205,7 @@ static int CrashForExceptionInNonABICompliantCodeRange(
     PCONTEXT ContextRecord,
     PDISPATCHER_CONTEXT DispatcherContext) {
   EXCEPTION_POINTERS info = { ExceptionRecord, ContextRecord };
-  return CrashForException(&info);
+  return CrashForException_ExportThunk(&info);
 }
 
 // See https://msdn.microsoft.com/en-us/library/ddssxxy8.aspx
@@ -225,9 +225,7 @@ struct ExceptionHandlerRecord {
   unsigned char thunk[12];
 };
 
-// These are GetProcAddress()d from V8 binding code.
+void RegisterNonABICompliantCodeRangeImpl(void* start, size_t size_in_bytes) {
-void __cdecl RegisterNonABICompliantCodeRangeImpl(void* start,
-                                                  size_t size_in_bytes) {
   ExceptionHandlerRecord* record =
       reinterpret_cast<ExceptionHandlerRecord*>(start);