Skip to content

Commit

Permalink
Remove dependency on base/win/win_util.cc from crashpad.
Browse files Browse the repository at this point in the history
Fixing the official ASAN builder which fails when
chrome_crash_reporter_client_win.cc is compiled with chrome_elf.dll

Crashpad is now included as part of chrome_elf and hence it cannot depend on functionality in base or elsewhere which pulls in user32

The ElfImportsTest.ChromeElfSanityCheck is disabled for
component builds to fix Dr Memory issues.

This reverts commit f546a78.

BUG=621460
TBR=robertshield

Review-Url: https://codereview.chromium.org/2077323004
Cr-Commit-Position: refs/heads/master@{#400839}
  • Loading branch information
ananta authored and Commit bot committed Jun 20, 2016
1 parent e93a0d2 commit e10c457
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 3 deletions.
7 changes: 7 additions & 0 deletions chrome_elf/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ windows_manifest("chrome_elf_manifest") {
# in the world.
shared_library("chrome_elf") {
sources = [
"//chrome/app/chrome_crash_reporter_client_win.cc",
"//chrome/app/chrome_crash_reporter_client_win.h",
"//chrome/common/chrome_result_codes.h",
"chrome_elf.def",
"chrome_elf_main.cc",
"chrome_elf_main.h",
Expand All @@ -36,8 +39,12 @@ shared_library("chrome_elf") {
":chrome_elf_manifest",
":chrome_elf_resources",
":common",
"//base",
"//build/config/sanitizers:deps",
"//chrome/install_static:install_static_util",
"//components/crash/content/app",
"//components/crash/core/common",
"//content/public/common:result_codes",
]
configs += [ "//build/config/win:windowed" ]
configs -= [ "//build/config/win:console" ]
Expand Down
1 change: 1 addition & 0 deletions chrome_elf/DEPS
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
include_rules = [
"+sandbox",
"+breakpad/src/client",
"+chrome/app/chrome_crash_reporter_client_win.h",
"+chrome/common/chrome_version.h",
"+chrome/install_static/install_util.h"
]
4 changes: 4 additions & 0 deletions chrome_elf/chrome_elf.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,17 @@
'chrome_elf.def',
'chrome_elf_main.cc',
'chrome_elf_main.h',
'../chrome/app/chrome_crash_reporter_client_win.cc',
'../chrome/app/chrome_crash_reporter_client_win.h',
'<(SHARED_INTERMEDIATE_DIR)/chrome_elf/chrome_elf_version.rc',
],
'dependencies': [
'blacklist',
'chrome_elf_breakpad',
'chrome_elf_resources',
'../chrome/chrome.gyp:install_static_util',
'../components/components.gyp:crash_component',
'../components/components.gyp:crash_core_common',
],
'msvs_settings': {
'VCLinkerTool': {
Expand Down
2 changes: 1 addition & 1 deletion chrome_elf/elf_imports_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class ELFImportsTest : public testing::Test {
//
// If you break this test, you may have changed base or the Windows sandbox
// such that more system imports are required to link.
#ifdef NDEBUG
#if defined(NDEBUG) && !defined(COMPONENT_BUILD)
TEST_F(ELFImportsTest, ChromeElfSanityCheck) {
base::FilePath dll;
ASSERT_TRUE(PathService::Get(base::DIR_EXE, &dll));
Expand Down
52 changes: 50 additions & 2 deletions components/crash/content/app/crashpad.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@

#if BUILDFLAG(ENABLE_KASKO)
#include "base/win/scoped_handle.h"
#include "base/win/win_util.h"
#include "third_party/crashpad/crashpad/snapshot/api/module_annotations_win.h"
#endif

Expand Down Expand Up @@ -105,10 +104,59 @@ void DumpWithoutCrashing() {
}

#if BUILDFLAG(ENABLE_KASKO)
// TODO(ananta)
// We cannot depend on functionality in base which pulls in dependencies on
// user32 directly or indirectly. The GetLoadedModulesSnapshot is a copy of the
// function in base/win/win_util.cc. Depending on the base function pulls in
// dependencies on user32 due to other functionality in win_util.cc. This
// function should be removed when KASKO is removed.
bool GetLoadedModulesSnapshot(HANDLE process, std::vector<HMODULE>* snapshot) {
DCHECK(snapshot);
DCHECK_EQ(0u, snapshot->size());
snapshot->resize(128);

// We will retry at least once after first determining |bytes_required|. If
// the list of modules changes after we receive |bytes_required| we may retry
// more than once.
int retries_remaining = 5;
do {
DWORD bytes_required = 0;
// EnumProcessModules returns 'success' even if the buffer size is too
// small.
DCHECK_GE(std::numeric_limits<DWORD>::max(),
snapshot->size() * sizeof(HMODULE));
if (!::EnumProcessModules(
process, &(*snapshot)[0],
static_cast<DWORD>(snapshot->size() * sizeof(HMODULE)),
&bytes_required)) {
DPLOG(ERROR) << "::EnumProcessModules failed.";
return false;
}
DCHECK_EQ(0u, bytes_required % sizeof(HMODULE));
size_t num_modules = bytes_required / sizeof(HMODULE);
if (num_modules <= snapshot->size()) {
// Buffer size was too big, presumably because a module was unloaded.
snapshot->erase(snapshot->begin() + num_modules, snapshot->end());
return true;
} else if (num_modules == 0) {
DLOG(ERROR) << "Can't determine the module list size.";
return false;
} else {
// Buffer size was too small. Try again with a larger buffer. A little
// more room is given to avoid multiple expensive calls to
// ::EnumProcessModules() just because one module has been added.
snapshot->resize(num_modules + 8, NULL);
}
} while (--retries_remaining);

DLOG(ERROR) << "Failed to enumerate modules.";
return false;
}

HMODULE GetModuleInProcess(base::ProcessHandle process,
const wchar_t* module_name) {
std::vector<HMODULE> modules_snapshot;
if (!base::win::GetLoadedModulesSnapshot(process, &modules_snapshot))
if (!GetLoadedModulesSnapshot(process, &modules_snapshot))
return nullptr;

for (HMODULE module : modules_snapshot) {
Expand Down

0 comments on commit e10c457

Please sign in to comment.