Skip to content

Commit

Permalink
Add an option to start a Windows client with global hooks disabled.
Browse files Browse the repository at this point in the history
Change-Id: I645d6136788ca4ccebfc73005c8c2455dc4b2cee
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4949671
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Rich Mckeever <[email protected]>
  • Loading branch information
richmckeever authored and Crashpad LUCI CQ committed Oct 26, 2023
1 parent 376e8c0 commit e17518a
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 3 deletions.
16 changes: 16 additions & 0 deletions client/crashpad_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,21 @@ class CrashpadClient {
#endif // BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_ANDROID) ||
// BUILDFLAG(IS_CHROMEOS) || DOXYGEN

#if BUILDFLAG(IS_WIN) || DOXYGEN
//! \brief Configures this client to not install any process-global hooks,
//! such as an unhandled exception filter or vectored exception handler.
//!
//! This may be useful if this client is being used in the context of an
//! extension library, which only wants to capture crashes in its own code,
//! via catch blocks, and not all crashes in the host process.
//!
//! This method must be called before calling StartHandler(),
//! SetHandlerSocket(), or other methods that install global hooks.
void DisableGlobalHooks() {
disable_global_hooks_ = true;
}
#endif // BUILDFLAG(IS_WIN) || DOXYGEN

#if BUILDFLAG(IS_IOS) || DOXYGEN
//! \brief Observation callback invoked each time this object finishes
//! processing and attempting to upload on-disk crash reports (whether or
Expand Down Expand Up @@ -818,6 +833,7 @@ class CrashpadClient {
std::wstring ipc_pipe_;
ScopedKernelHANDLE handler_start_thread_;
ScopedVectoredExceptionRegistration vectored_handler_;
bool disable_global_hooks_;
#elif BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_ANDROID)
std::set<int> unhandled_signals_;
#endif // BUILDFLAG(IS_APPLE)
Expand Down
7 changes: 6 additions & 1 deletion client/crashpad_client_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,8 @@ void CommonInProcessInitialization() {
} // namespace

CrashpadClient::CrashpadClient()
: ipc_pipe_(), handler_start_thread_(), vectored_handler_() {}
: ipc_pipe_(), handler_start_thread_(), vectored_handler_(),
disable_global_hooks_(false) {}

CrashpadClient::~CrashpadClient() {}

Expand Down Expand Up @@ -667,6 +668,10 @@ bool CrashpadClient::StartHandler(
}

void CrashpadClient::RegisterHandlers() {
if (disable_global_hooks_) {
return;
}

SetUnhandledExceptionFilter(&UnhandledExceptionHandler);

// Windows swallows heap corruption failures but we can intercept them with
Expand Down
123 changes: 121 additions & 2 deletions client/crashpad_client_win_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <vector>

#include "base/files/file_path.h"
#include "client/crash_report_database.h"
#include "gtest/gtest.h"
#include "test/test_paths.h"
#include "test/scoped_temp_dir.h"
Expand All @@ -29,11 +30,10 @@ namespace crashpad {
namespace test {
namespace {

void StartAndUseHandler(const base::FilePath& temp_dir) {
void StartAndUseHandler(CrashpadClient& client, const base::FilePath& temp_dir) {
base::FilePath handler_path = TestPaths::Executable().DirName().Append(
FILE_PATH_LITERAL("crashpad_handler.com"));

CrashpadClient client;
ASSERT_TRUE(client.StartHandler(handler_path,
temp_dir,
base::FilePath(),
Expand All @@ -45,6 +45,11 @@ void StartAndUseHandler(const base::FilePath& temp_dir) {
ASSERT_TRUE(client.WaitForHandlerStart(INFINITE));
}

void StartAndUseHandler(const base::FilePath& temp_dir) {
CrashpadClient client;
StartAndUseHandler(client, temp_dir);
}

class StartWithInvalidHandles final : public WinMultiprocessWithTempDir {
public:
StartWithInvalidHandles() : WinMultiprocessWithTempDir() {}
Expand Down Expand Up @@ -192,6 +197,120 @@ TEST(CrashpadClient, HandlerLaunchFailureDumpWithoutCrash) {
WinMultiprocess::Run<HandlerLaunchFailureDumpWithoutCrash>();
}

class NoDumpExpected : public WinMultiprocessWithTempDir {
private:
void WinMultiprocessParentAfterChild(HANDLE child) override {
// Make sure no dump was generated.
std::unique_ptr<CrashReportDatabase> database(
CrashReportDatabase::Initialize(GetTempDirPath()));
ASSERT_TRUE(database);

std::vector<CrashReportDatabase::Report> reports;
ASSERT_EQ(database->GetPendingReports(&reports),
CrashReportDatabase::kNoError);
ASSERT_EQ(reports.size(), 0u);
}
};

// Crashing the process under test does not result in a crashed status as an
// exit code in debug builds, so we only verify this behavior in release
// builds.
#if defined(NDEBUG)
class CrashWithDisabledGlobalHooks final : public NoDumpExpected {
public:
CrashWithDisabledGlobalHooks() : NoDumpExpected() {}
~CrashWithDisabledGlobalHooks() {}

private:
void WinMultiprocessParent() override {
SetExpectedChildExitCode(STATUS_ACCESS_VIOLATION);
}

void WinMultiprocessChild() override {
CrashpadClient client;
client.DisableGlobalHooks();
StartAndUseHandler(client, GetTempDirPath());
int* bad = nullptr;
*bad = 1;
}
};

TEST(CrashpadClient, CrashWithDisabledGlobalHooks) {
WinMultiprocessWithTempDir::Run<CrashWithDisabledGlobalHooks>();
}
#endif // defined(NDEBUG)

class DumpAndCrashWithDisabledGlobalHooks final
: public WinMultiprocessWithTempDir {
public:
DumpAndCrashWithDisabledGlobalHooks() : WinMultiprocessWithTempDir() {}
~DumpAndCrashWithDisabledGlobalHooks() {}

private:
static constexpr DWORD kExpectedExitCode = 0x1CEB00DA;

void WinMultiprocessParent() override {
SetExpectedChildExitCode(kExpectedExitCode);
}

void WinMultiprocessChild() override {
CrashpadClient client;
client.DisableGlobalHooks();
StartAndUseHandler(client, GetTempDirPath());
EXCEPTION_RECORD exception_record = {kExpectedExitCode,
EXCEPTION_NONCONTINUABLE};
CONTEXT context;
CaptureContext(&context);
EXCEPTION_POINTERS exception_pointers = {&exception_record, &context};
CrashpadClient::DumpAndCrash(&exception_pointers);
}

void WinMultiprocessParentAfterChild(HANDLE child) override {
// Make sure the dump was generated.
std::unique_ptr<CrashReportDatabase> database(
CrashReportDatabase::Initialize(GetTempDirPath()));
ASSERT_TRUE(database);

std::vector<CrashReportDatabase::Report> reports;
ASSERT_EQ(database->GetPendingReports(&reports),
CrashReportDatabase::kNoError);
ASSERT_EQ(reports.size(), 1u);

// Delegate the cleanup to the superclass.
WinMultiprocessWithTempDir::WinMultiprocessParentAfterChild(child);
}
};

TEST(CrashpadClient, DumpAndCrashWithDisabledGlobalHooks) {
WinMultiprocessWithTempDir::Run<DumpAndCrashWithDisabledGlobalHooks>();
}

#if !defined(ADDRESS_SANITIZER)
class HeapCorruptionWithDisabledGlobalHooks final : public NoDumpExpected {
public:
HeapCorruptionWithDisabledGlobalHooks() : NoDumpExpected() {}
~HeapCorruptionWithDisabledGlobalHooks() {}

private:
void WinMultiprocessParent() override {
SetExpectedChildExitCode(STATUS_HEAP_CORRUPTION);
}

void WinMultiprocessChild() override {
CrashpadClient client;
client.DisableGlobalHooks();
StartAndUseHandler(client, GetTempDirPath());
int* bad = reinterpret_cast<int*>(1);
delete bad;
}
};

TEST(CrashpadClient, HeapCorruptionWithDisabledGlobalHooks) {
WinMultiprocessWithTempDir::Run<HeapCorruptionWithDisabledGlobalHooks>();
}

#endif // !defined(ADDRESS_SANITIZER)

} // namespace
} // namespace test
} // namespace crashpad

0 comments on commit e17518a

Please sign in to comment.