Skip to content

Commit

Permalink
Make chrome_elf exports explicit in .def files instead of using __dec…
Browse files Browse the repository at this point in the history
…lspec(dllexport).

The __declspec(dllexport) directives were bleeding across targets, so chrome.dll and
chrome_child.dll ended up exporting all the same things as chrome_elf.dll. This leads
to a small size regression - on the order of 100K in the 32 bit binaries.

Bug: 753363
Cq-Include-Trybots: master.tryserver.chromium.win:win10_chromium_x64_rel_ng
Change-Id: I614c82dcb41f0372373b3942bff36215f057c4a1
Reviewed-on: https://chromium-review.googlesource.com/611963
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Reviewed-by: Robert Shield <robertshield@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494214}
  • Loading branch information
sigurasg authored and Commit Bot committed Aug 14, 2017
1 parent f2f5c73 commit a36b8aa
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 33 deletions.
22 changes: 18 additions & 4 deletions chrome/browser/chrome_browser_main_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,25 @@ typedef HRESULT (STDAPICALLTYPE* RegisterApplicationRestartProc)(
const wchar_t* command_line,
DWORD flags);

// TODO(siggi): Replace this with link-time binding to the right code.
// See https://crbug.com/753363.
int UnhandledExceptionFilterWrapper(EXCEPTION_POINTERS* info) {
return UnhandledExceptionFilter(info);
}

void InitializeWindowProcExceptions() {
// Get the breakpad pointer from chrome.exe
base::win::WinProcExceptionFilter exception_filter =
reinterpret_cast<base::win::WinProcExceptionFilter>(::GetProcAddress(
::GetModuleHandle(chrome::kChromeElfDllName), "CrashForException"));
// Get the exception filter from chrome_elf.dll, if present. In tests,
// chrome_elf won't be present, in which case use the UnhandledExceptionFilter
// to ensure the process crashes.
HMODULE chrome_elf = ::GetModuleHandle(chrome::kChromeElfDllName);
base::win::WinProcExceptionFilter exception_filter = nullptr;
if (chrome_elf) {
exception_filter = reinterpret_cast<base::win::WinProcExceptionFilter>(
::GetProcAddress(chrome_elf, "CrashForException"));
} else {
exception_filter = &UnhandledExceptionFilterWrapper;
}

CHECK(exception_filter);
exception_filter = base::win::SetWinProcExceptionFilter(exception_filter);
DCHECK(!exception_filter);
Expand Down
4 changes: 4 additions & 0 deletions chrome_elf/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ shared_library("chrome_elf") {
"chrome_elf_main.cc",
"chrome_elf_main.h",
]
if (target_cpu == "x64") {
sources += [ "chrome_elf_x64.def" ]
}

deps = [
":blacklist",
":chrome_elf_manifest",
Expand Down
30 changes: 25 additions & 5 deletions chrome_elf/chrome_elf.def
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,32 @@
LIBRARY "chrome_elf.dll"

EXPORTS
AddDllToBlacklist
GetBlacklistIndex
GetInstallDetailsPayload
; From components/crash/content/app/crashpad.cc
ClearCrashKeyValueImpl
RequestSingleCrashUploadImpl
SetCrashKeyValueImpl
SetUploadConsentImpl

; From components/crash/content/app/crashpad_win.cc
CrashForException
InjectDumpForHungInput
InjectDumpForHungInputNoCrashKeys

; From chrome_elf/crash_helper.cc
GetCrashReportsImpl
SetMetricsClientId

; From chrome_elf_main.cc
DumpProcessWithoutCrash
GetUserDataDirectoryThunk
IsBlacklistInitialized
SignalChromeElf
SignalInitializeCrashReporting

; From chrome/install_static
GetInstallDetailsPayload

; From chrome_elf/blacklist.cc
AddDllToBlacklist
GetBlacklistIndex
IsBlacklistInitialized
SuccessfullyBlocked
DumpProcessWithoutCrash
10 changes: 10 additions & 0 deletions chrome_elf/chrome_elf_x64.def
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
; 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
; From components/crash/content/app/crashpad_win.cc
RegisterNonABICompliantCodeRange
UnregisterNonABICompliantCodeRange
12 changes: 7 additions & 5 deletions chrome_elf/crash/crash_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,10 @@ void DumpWithoutCrashing() {
// NOTE: Since the returned pointer references read-only memory that will be
// cleaned up when this DLL unloads, be careful not to reference the memory
// beyond that point (e.g. during tests).
extern "C" __declspec(dllexport) void GetCrashReportsImpl(
const crash_reporter::Report** reports,
size_t* report_count) {
extern "C" {

void GetCrashReportsImpl(const crash_reporter::Report** reports,
size_t* report_count) {
if (!g_crash_helper_enabled)
return;
crash_reporter::GetReports(g_crash_reports);
Expand All @@ -144,10 +145,11 @@ extern "C" __declspec(dllexport) void GetCrashReportsImpl(

// This helper is invoked by debugging code in chrome to register the client
// id.
extern "C" __declspec(dllexport) void SetMetricsClientId(
const char* client_id) {
void SetMetricsClientId(const char* client_id) {
if (!g_crash_helper_enabled)
return;
if (client_id)
crash_keys::SetMetricsClientIdFromGUID(client_id);
}

} // extern "C"
11 changes: 5 additions & 6 deletions components/crash/content/app/crashpad.cc
Original file line number Diff line number Diff line change
Expand Up @@ -319,30 +319,29 @@ extern "C" {
// be consistent with
// crash_reporter::GetCrashReporterClient()->GetCollectStatsConsent(), but it's
// not enforced to avoid blocking startup code on synchronizing them.
void __declspec(dllexport) __cdecl SetUploadConsentImpl(bool consent) {
void SetUploadConsentImpl(bool consent) {
crash_reporter::SetUploadConsent(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 __declspec(dllexport) __cdecl SetCrashKeyValueImpl(const wchar_t* key,
const wchar_t* value) {
void SetCrashKeyValueImpl(const wchar_t* key, const wchar_t* value) {
crash_reporter::SetCrashKeyValue(base::UTF16ToUTF8(key),
base::UTF16ToUTF8(value));
}

void __declspec(dllexport) __cdecl ClearCrashKeyValueImpl(const wchar_t* key) {
void ClearCrashKeyValueImpl(const wchar_t* key) {
crash_reporter::ClearCrashKey(base::UTF16ToUTF8(key));
}

// This helper is invoked by code in chrome.dll to request a single crash report
// upload. See CrashUploadListCrashpad.
void __declspec(dllexport)
RequestSingleCrashUploadImpl(const std::string& local_id) {
void RequestSingleCrashUploadImpl(const std::string& local_id) {
crash_reporter::RequestSingleCrashUpload(local_id);
}

} // extern "C"

#endif // OS_WIN
20 changes: 7 additions & 13 deletions components/crash/content/app/crashpad_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,7 @@ extern "C" {
// 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 __declspec(dllexport) CrashForException(
EXCEPTION_POINTERS* info) {
int CrashForException(EXCEPTION_POINTERS* info) {
crash_reporter::GetCrashpadClient().DumpAndCrash(info);
return EXCEPTION_CONTINUE_SEARCH;
}
Expand All @@ -221,9 +220,8 @@ int __declspec(dllexport) CrashForException(
// crash keys sent from the browser. 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 __declspec(dllexport) __cdecl InjectDumpForHungInput(
HANDLE process,
void* serialized_crash_keys) {
HANDLE __cdecl InjectDumpForHungInput(HANDLE process,
void* serialized_crash_keys) {
return CreateRemoteThread(
process, nullptr, 0,
crash_reporter::internal::DumpProcessForHungInputThread,
Expand All @@ -233,9 +231,7 @@ HANDLE __declspec(dllexport) __cdecl InjectDumpForHungInput(
// 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 __declspec(dllexport) __cdecl InjectDumpForHungInputNoCrashKeys(
HANDLE process,
int reason) {
HANDLE __cdecl InjectDumpForHungInputNoCrashKeys(HANDLE process, int reason) {
return CreateRemoteThread(
process, nullptr, 0,
crash_reporter::internal::DumpProcessForHungInputNoCrashKeysThread,
Expand Down Expand Up @@ -271,9 +267,8 @@ struct ExceptionHandlerRecord {
};

// These are GetProcAddress()d from V8 binding code.
void __declspec(dllexport) __cdecl RegisterNonABICompliantCodeRange(
void* start,
size_t size_in_bytes) {
void __cdecl RegisterNonABICompliantCodeRange(void* start,
size_t size_in_bytes) {
ExceptionHandlerRecord* record =
reinterpret_cast<ExceptionHandlerRecord*>(start);

Expand Down Expand Up @@ -316,8 +311,7 @@ void __declspec(dllexport) __cdecl RegisterNonABICompliantCodeRange(
&record->runtime_function, 1, reinterpret_cast<DWORD64>(start)));
}

void __declspec(dllexport) __cdecl UnregisterNonABICompliantCodeRange(
void* start) {
void __cdecl UnregisterNonABICompliantCodeRange(void* start) {
ExceptionHandlerRecord* record =
reinterpret_cast<ExceptionHandlerRecord*>(start);

Expand Down

0 comments on commit a36b8aa

Please sign in to comment.