Revert "Add an option to start a Windows client with global hooks disabled."

This reverts commit e17518a9e879f63b578db6c184c6bb17f1b13a06.

Reason for revert:

When trying to roll Crashpad into Chromium, all the new tests in
this CL fail; https://crrev.com/c/4984643?checksRunsSelected=win-rel&tab=checks

Original change's description:
> Add an option to start a Windows client with global hooks disabled.
>
> Change-Id: I645d6136788ca4ccebfc73005c8c2455dc4b2cee
> Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4949671
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Commit-Queue: Rich Mckeever <mckeever@google.com>

Change-Id: I3a41238cf0960899fac19d1e6d0ed0e527dfe13f
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4985124
Reviewed-by: Mark Mentovai <mark@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Avi Drissman <avi@chromium.org>
This commit is contained in:
Avi Drissman 2023-10-27 20:06:46 +00:00 committed by Crashpad LUCI CQ
parent 188ad79298
commit 4a93d7f4c4
3 changed files with 3 additions and 143 deletions

View File

@ -486,21 +486,6 @@ 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
@ -833,7 +818,6 @@ 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)

View File

@ -592,8 +592,7 @@ void CommonInProcessInitialization() {
} // namespace
CrashpadClient::CrashpadClient()
: ipc_pipe_(), handler_start_thread_(), vectored_handler_(),
disable_global_hooks_(false) {}
: ipc_pipe_(), handler_start_thread_(), vectored_handler_() {}
CrashpadClient::~CrashpadClient() {}
@ -668,10 +667,6 @@ bool CrashpadClient::StartHandler(
}
void CrashpadClient::RegisterHandlers() {
if (disable_global_hooks_) {
return;
}
SetUnhandledExceptionFilter(&UnhandledExceptionHandler);
// Windows swallows heap corruption failures but we can intercept them with

View File

@ -17,7 +17,6 @@
#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"
@ -30,10 +29,11 @@ namespace crashpad {
namespace test {
namespace {
void StartAndUseHandler(CrashpadClient& client, const base::FilePath& temp_dir) {
void StartAndUseHandler(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(),
@ -45,11 +45,6 @@ void StartAndUseHandler(CrashpadClient& client, 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() {}
@ -197,120 +192,6 @@ 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