diff --git a/doc/support/generate_asciidoc.sh b/doc/support/generate_asciidoc.sh index ea5bd467..b636fe70 100755 --- a/doc/support/generate_asciidoc.sh +++ b/doc/support/generate_asciidoc.sh @@ -122,7 +122,7 @@ for input in \ done for input in \ - handler/mac/crashpad_handler.ad \ + handler/crashpad_handler.ad \ tools/*.ad \ tools/mac/*.ad; do generate "${input}" "man" diff --git a/handler/mac/crashpad_handler.ad b/handler/crashpad_handler.ad similarity index 94% rename from handler/mac/crashpad_handler.ad rename to handler/crashpad_handler.ad index b53d0977..cc335ded 100644 --- a/handler/mac/crashpad_handler.ad +++ b/handler/crashpad_handler.ad @@ -75,7 +75,12 @@ of 'PATH' exists. *--handshake-fd*='FD':: Perform the handshake with the initial client on the file descriptor at 'FD'. -This option is required. +This option is required. This option is only valid on Mac OS X. + +*--pipe-name*='PIPE':: +Listen on the given pipe name for connections from clients. 'PIPE' must be of +the form +\\.\pipe\+. This option is required. This option is only +valid on Windows. *--url*='URL':: If uploads are enabled, sends crash reports to the Breakpad-type crash report diff --git a/handler/handler.gyp b/handler/handler.gyp index 33c7fd5b..8f8324c0 100644 --- a/handler/handler.gyp +++ b/handler/handler.gyp @@ -17,34 +17,36 @@ '../build/crashpad.gypi', '../build/crashpad_in_chromium.gypi', ], - 'conditions': [ - ['OS=="mac"', { - 'targets': [ - { - 'target_name': 'crashpad_handler', - 'type': 'executable', - 'dependencies': [ - '../client/client.gyp:crashpad_client', - '../compat/compat.gyp:crashpad_compat', - '../minidump/minidump.gyp:crashpad_minidump', - '../snapshot/snapshot.gyp:crashpad_snapshot', - '../third_party/mini_chromium/mini_chromium.gyp:base', - '../tools/tools.gyp:crashpad_tool_support', - '../util/util.gyp:crashpad_util', - ], - 'include_dirs': [ - '..', - ], - 'sources': [ - 'crash_report_upload_thread.cc', - 'crash_report_upload_thread.h', - 'mac/crash_report_exception_handler.cc', - 'mac/crash_report_exception_handler.h', - 'mac/exception_handler_server.cc', - 'mac/exception_handler_server.h', - 'mac/main.cc', - ], + 'targets': [ + { + 'target_name': 'crashpad_handler', + 'type': 'executable', + 'dependencies': [ + '../client/client.gyp:crashpad_client', + '../compat/compat.gyp:crashpad_compat', + '../minidump/minidump.gyp:crashpad_minidump', + '../snapshot/snapshot.gyp:crashpad_snapshot', + '../third_party/mini_chromium/mini_chromium.gyp:base', + '../tools/tools.gyp:crashpad_tool_support', + '../util/util.gyp:crashpad_util', + ], + 'include_dirs': [ + '..', + ], + 'sources': [ + 'crash_report_upload_thread.cc', + 'crash_report_upload_thread.h', + 'mac/crash_report_exception_handler.cc', + 'mac/crash_report_exception_handler.h', + 'mac/exception_handler_server.cc', + 'mac/exception_handler_server.h', + 'main.cc', + 'win/crash_report_exception_handler.cc', + 'win/crash_report_exception_handler.h', + ], + 'conditions': [ + ['OS=="mac"', { # In an in-Chromium build with component=shared_library, # crashpad_handler will depend on shared libraries such as # libbase.dylib located in out/{Debug,Release} via the @rpath @@ -66,26 +68,26 @@ }, }], ], - }, + }], ], - },], + }, + ], + 'conditions': [ ['OS=="win"', { 'targets': [ { - 'target_name': 'crashpad_handler', - # TODO(scottmg): This will soon be an executable, once main.cc exists. - 'type': 'static_library', + 'target_name': 'crashy_program', + 'type': 'executable', 'dependencies': [ - '../compat/compat.gyp:crashpad_compat', + '../client/client.gyp:crashpad_client', '../third_party/mini_chromium/mini_chromium.gyp:base', - '../util/util.gyp:crashpad_util', + '../tools/tools.gyp:crashpad_tool_support', ], 'include_dirs': [ '..', ], 'sources': [ - 'crash_report_upload_thread.cc', - 'crash_report_upload_thread.h', + 'win/crashy_test_program.cc', ], }, ], diff --git a/handler/mac/main.cc b/handler/main.cc similarity index 74% rename from handler/mac/main.cc rename to handler/main.cc index 39e5c8c4..0f3c3643 100644 --- a/handler/mac/main.cc +++ b/handler/main.cc @@ -13,7 +13,6 @@ // limitations under the License. #include -#include #include #include @@ -22,46 +21,66 @@ #include "base/files/file_path.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" +#include "base/strings/utf_string_conversions.h" +#include "build/build_config.h" #include "client/crash_report_database.h" #include "tools/tool_support.h" #include "handler/crash_report_upload_thread.h" -#include "handler/mac/crash_report_exception_handler.h" -#include "handler/mac/exception_handler_server.h" -#include "util/mach/child_port_handshake.h" -#include "util/posix/close_stdio.h" #include "util/stdlib/map_insert.h" #include "util/stdlib/string_number_conversion.h" #include "util/string/split_string.h" #include "util/synchronization/semaphore.h" +#if defined(OS_MACOSX) +#include +#include "handler/mac/crash_report_exception_handler.h" +#include "handler/mac/exception_handler_server.h" +#include "util/mach/child_port_handshake.h" +#include "util/posix/close_stdio.h" +#elif defined(OS_WIN) +#include +#include "handler/win/crash_report_exception_handler.h" +#include "util/win/exception_handler_server.h" +#endif // OS_MACOSX + namespace crashpad { namespace { -void Usage(const std::string& me) { +void Usage(const base::FilePath& me) { fprintf(stderr, -"Usage: %s [OPTION]...\n" +"Usage: %" PRFilePath " [OPTION]...\n" "Crashpad's exception handler server.\n" "\n" " --annotation=KEY=VALUE set a process annotation in each crash report\n" " --database=PATH store the crash report database at PATH\n" +#if defined(OS_MACOSX) " --handshake-fd=FD establish communication with the client over FD\n" +#elif defined(OS_WIN) +" --pipe-name=PIPE communicate with the client over PIPE\n" +#endif // OS_MACOSX " --url=URL send crash reports to this Breakpad server URL,\n" " only if uploads are enabled for the database\n" " --help display this help and exit\n" " --version output version information and exit\n", - me.c_str()); + me.value().c_str()); ToolSupport::UsageTail(me); } int HandlerMain(int argc, char* argv[]) { - const std::string me(basename(argv[0])); + const base::FilePath argv0( + ToolSupport::CommandLineArgumentToFilePathStringType(argv[0])); + const base::FilePath me(argv0.BaseName()); enum OptionFlags { // Long options without short equivalents. kOptionLastChar = 255, kOptionAnnotation, kOptionDatabase, +#if defined(OS_MACOSX) kOptionHandshakeFD, +#elif defined(OS_WIN) + kOptionPipeName, +#endif // OS_MACOSX kOptionURL, // Standard options. @@ -73,14 +92,24 @@ int HandlerMain(int argc, char* argv[]) { std::map annotations; std::string url; const char* database; +#if defined(OS_MACOSX) int handshake_fd; +#elif defined(OS_WIN) + std::string pipe_name; +#endif } options = {}; +#if defined(OS_MACOSX) options.handshake_fd = -1; +#endif const option long_options[] = { {"annotation", required_argument, nullptr, kOptionAnnotation}, {"database", required_argument, nullptr, kOptionDatabase}, +#if defined(OS_MACOSX) {"handshake-fd", required_argument, nullptr, kOptionHandshakeFD}, +#elif defined(OS_WIN) + {"pipe-name", required_argument, nullptr, kOptionPipeName}, +#endif {"url", required_argument, nullptr, kOptionURL}, {"help", no_argument, nullptr, kOptionHelp}, {"version", no_argument, nullptr, kOptionVersion}, @@ -108,6 +137,7 @@ int HandlerMain(int argc, char* argv[]) { options.database = optarg; break; } +#if defined(OS_MACOSX) case kOptionHandshakeFD: { if (!StringToNumber(optarg, &options.handshake_fd) || options.handshake_fd < 0) { @@ -117,6 +147,12 @@ int HandlerMain(int argc, char* argv[]) { } break; } +#elif defined(OS_WIN) + case kOptionPipeName: { + options.pipe_name = optarg; + break; + } +#endif // OS_MACOSX case kOptionURL: { options.url = optarg; break; @@ -138,10 +174,17 @@ int HandlerMain(int argc, char* argv[]) { argc -= optind; argv += optind; +#if defined(OS_MACOSX) if (options.handshake_fd < 0) { ToolSupport::UsageHint(me, "--handshake-fd is required"); return EXIT_FAILURE; } +#elif defined(OS_WIN) + if (options.pipe_name.empty()) { + ToolSupport::UsageHint(me, "--pipe-name is required"); + return EXIT_FAILURE; + } +#endif if (!options.database) { ToolSupport::UsageHint(me, "--database is required"); @@ -153,16 +196,23 @@ int HandlerMain(int argc, char* argv[]) { return EXIT_FAILURE; } - CloseStdinAndStdout(); - ExceptionHandlerServer exception_handler_server; +#if defined(OS_MACOSX) + CloseStdinAndStdout(); ChildPortHandshake::RunClient(options.handshake_fd, exception_handler_server.receive_port(), MACH_MSG_TYPE_MAKE_SEND); +#endif // OS_MACOSX scoped_ptr database( - CrashReportDatabase::Initialize(base::FilePath(options.database))); + CrashReportDatabase::Initialize(base::FilePath( +#if defined(OS_MACOSX) + options.database +#elif defined(OS_WIN) + base::UTF8ToUTF16(options.database) +#endif + ))); if (!database) { return EXIT_FAILURE; } @@ -173,7 +223,11 @@ int HandlerMain(int argc, char* argv[]) { CrashReportExceptionHandler exception_handler( database.get(), &upload_thread, &options.annotations); +#if defined(OS_MACOSX) exception_handler_server.Run(&exception_handler); +#elif defined(OS_WIN) + exception_handler_server.Run(&exception_handler, options.pipe_name); +#endif // OS_MACOSX upload_thread.Stop(); @@ -183,6 +237,12 @@ int HandlerMain(int argc, char* argv[]) { } // namespace } // namespace crashpad +#if defined(OS_MACOSX) int main(int argc, char* argv[]) { return crashpad::HandlerMain(argc, argv); } +#elif defined(OS_WIN) +int wmain(int argc, wchar_t* argv[]) { + return crashpad::ToolSupport::Wmain(argc, argv, crashpad::HandlerMain); +} +#endif // OS_MACOSX diff --git a/handler/win/crash_report_exception_handler.cc b/handler/win/crash_report_exception_handler.cc new file mode 100644 index 00000000..faf03671 --- /dev/null +++ b/handler/win/crash_report_exception_handler.cc @@ -0,0 +1,117 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "handler/win/crash_report_exception_handler.h" + +#include "client/crash_report_database.h" +#include "client/settings.h" +#include "handler/crash_report_upload_thread.h" +#include "minidump/minidump_file_writer.h" +#include "snapshot/win/process_snapshot_win.h" +#include "util/file/file_writer.h" +#include "util/win/registration_protocol_win.h" + +namespace crashpad { + +CrashReportExceptionHandler::CrashReportExceptionHandler( + CrashReportDatabase* database, + CrashReportUploadThread* upload_thread, + const std::map* process_annotations) + : database_(database), + upload_thread_(upload_thread), + process_annotations_(process_annotations) { +} + +CrashReportExceptionHandler::~CrashReportExceptionHandler() { +} + +void CrashReportExceptionHandler::ExceptionHandlerServerStarted() { +} + +unsigned int CrashReportExceptionHandler::ExceptionHandlerServerException( + HANDLE process, + WinVMAddress exception_information_address) { + const unsigned int kFailedTerminationCode = 0xffff7002; + + // TODO(scottmg): ScopedProcessSuspend + + ProcessSnapshotWin process_snapshot; + if (!process_snapshot.Initialize(process)) { + LOG(WARNING) << "ProcessSnapshotWin::Initialize failed"; + return kFailedTerminationCode; + } + + if (!process_snapshot.InitializeException(exception_information_address)) { + LOG(WARNING) << "ProcessSnapshotWin::InitializeException failed"; + return kFailedTerminationCode; + } + + // Now that we have the exception information, even if something else fails we + // can terminate the process with the correct exit code. + const unsigned int termination_code = + process_snapshot.Exception()->Exception(); + + CrashpadInfoClientOptions client_options; + process_snapshot.GetCrashpadOptions(&client_options); + if (client_options.crashpad_handler_behavior != TriState::kDisabled) { + UUID client_id; + Settings* const settings = database_->GetSettings(); + if (settings) { + // If GetSettings() or GetClientID() fails, something else will log a + // message and client_id will be left at its default value, all zeroes, + // which is appropriate. + settings->GetClientID(&client_id); + } + + process_snapshot.SetClientID(client_id); + process_snapshot.SetAnnotationsSimpleMap(*process_annotations_); + + CrashReportDatabase::NewReport* new_report; + CrashReportDatabase::OperationStatus database_status = + database_->PrepareNewCrashReport(&new_report); + if (database_status != CrashReportDatabase::kNoError) { + LOG(ERROR) << "PrepareNewCrashReport failed"; + return termination_code; + } + + process_snapshot.SetReportID(new_report->uuid); + + CrashReportDatabase::CallErrorWritingCrashReport + call_error_writing_crash_report(database_, new_report); + + WeakFileHandleFileWriter file_writer(new_report->handle); + + MinidumpFileWriter minidump; + minidump.InitializeFromSnapshot(&process_snapshot); + if (!minidump.WriteEverything(&file_writer)) { + LOG(ERROR) << "WriteEverything failed"; + return termination_code; + } + + call_error_writing_crash_report.Disarm(); + + UUID uuid; + database_status = database_->FinishedWritingCrashReport(new_report, &uuid); + if (database_status != CrashReportDatabase::kNoError) { + LOG(ERROR) << "FinishedWritingCrashReport failed"; + return termination_code; + } + + upload_thread_->ReportPending(); + } + + return termination_code; +} + +} // namespace crashpad diff --git a/handler/win/crash_report_exception_handler.h b/handler/win/crash_report_exception_handler.h new file mode 100644 index 00000000..60e6b6ad --- /dev/null +++ b/handler/win/crash_report_exception_handler.h @@ -0,0 +1,75 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_HANDLER_WIN_CRASH_REPORT_EXCEPTION_HANDLER_H_ +#define CRASHPAD_HANDLER_WIN_CRASH_REPORT_EXCEPTION_HANDLER_H_ + +#include + +#include +#include + +#include "util/win/exception_handler_server.h" + +namespace crashpad { + +class CrashReportDatabase; +class CrashReportUploadThread; + +//! \brief An exception handler that writes crash reports for exception messages +//! to a CrashReportDatabase. +class CrashReportExceptionHandler : public ExceptionHandlerServer::Delegate { + public: + //! \brief Creates a new object that will store crash reports in \a database. + //! + //! \param[in] database The database to store crash reports in. Weak. + //! \param[in] upload_thread The upload thread to notify when a new crash + //! report is written into \a database. + //! \param[in] process_annotations A map of annotations to insert as + //! process-level annotations into each crash report that is written. Do + //! not confuse this with module-level annotations, which are under the + //! control of the crashing process, and are used to implement Chrome's + //! "crash keys." Process-level annotations are those that are beyond the + //! control of the crashing process, which must reliably be set even if + //! the process crashes before it's able to establish its own annotations. + //! To interoperate with Breakpad servers, the recommended practice is to + //! specify values for the `"prod"` and `"ver"` keys as process + //! annotations. + CrashReportExceptionHandler( + CrashReportDatabase* database, + CrashReportUploadThread* upload_thread, + const std::map* process_annotations); + + ~CrashReportExceptionHandler() override; + + // ExceptionHandlerServer::Delegate: + + //! \brief Processes an exception message by writing a crash report to this + //! object's CrashReportDatabase. + void ExceptionHandlerServerStarted() override; + unsigned int ExceptionHandlerServerException( + HANDLE process, + WinVMAddress exception_information_address) override; + + private: + CrashReportDatabase* database_; // weak + CrashReportUploadThread* upload_thread_; // weak + const std::map* process_annotations_; // weak + + DISALLOW_COPY_AND_ASSIGN(CrashReportExceptionHandler); +}; + +} // namespace crashpad + +#endif // CRASHPAD_HANDLER_WIN_CRASH_REPORT_EXCEPTION_HANDLER_H_ diff --git a/handler/win/crashy_test_program.cc b/handler/win/crashy_test_program.cc new file mode 100644 index 00000000..07c9cf10 --- /dev/null +++ b/handler/win/crashy_test_program.cc @@ -0,0 +1,54 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "client/crashpad_client.h" + +#include "base/logging.h" +#include "tools/tool_support.h" + +namespace crashpad { +namespace { + +void SomeCrashyFunction() { + volatile int* foo = reinterpret_cast(7); + *foo = 42; +} + +int CrashyMain(int argc, char* argv[]) { + if (argc != 2) { + fprintf(stderr, "Usage: %s \n", argv[0]); + return 1; + } + + CrashpadClient client; + if (!client.SetHandler(argv[1])) { + LOG(ERROR) << "SetHandler"; + return 1; + } + if (!client.UseHandler()) { + LOG(ERROR) << "UseHandler"; + return 1; + } + + SomeCrashyFunction(); + + return 0; +} + +} // namespace +} // namespace crashpad + +int wmain(int argc, wchar_t* argv[]) { + return crashpad::ToolSupport::Wmain(argc, argv, crashpad::CrashyMain); +} diff --git a/snapshot/win/exception_snapshot_win_test.cc b/snapshot/win/exception_snapshot_win_test.cc index dbed3f80..f6509a51 100644 --- a/snapshot/win/exception_snapshot_win_test.cc +++ b/snapshot/win/exception_snapshot_win_test.cc @@ -20,7 +20,6 @@ #include "base/strings/utf_string_conversions.h" #include "client/crashpad_client.h" #include "gtest/gtest.h" -#include "snapshot/win/process_reader_win.h" #include "snapshot/win/process_snapshot_win.h" #include "test/win/win_child_process.h" #include "util/thread/thread.h" @@ -63,22 +62,9 @@ class ExceptionSnapshotWinTest : public testing::Test { unsigned int ExceptionHandlerServerException( HANDLE process, WinVMAddress exception_information_address) override { - // Snapshot the process and exception. - ProcessReaderWin process_reader; - EXPECT_TRUE(process_reader.Initialize(process)); - if (HasFatalFailure()) - return 0xffffffff; - ExceptionInformation exception_information; - EXPECT_TRUE( - process_reader.ReadMemory(exception_information_address, - sizeof(exception_information), - &exception_information)); - if (HasFatalFailure()) - return 0xffffffff; ProcessSnapshotWin snapshot; snapshot.Initialize(process); - snapshot.InitializeException(exception_information.thread_id, - exception_information.exception_pointers); + snapshot.InitializeException(exception_information_address); // Confirm the exception record was read correctly. EXPECT_NE(snapshot.Exception()->ThreadID(), 0u); @@ -113,17 +99,19 @@ class ExceptionSnapshotWinTest : public testing::Test { // Runs the ExceptionHandlerServer on a background thread. class RunServerThread : public Thread { public: - // Instantiates a thread which will invoke server->Run(pipe_name); - explicit RunServerThread(ExceptionHandlerServer* server, - const std::string& pipe_name) - : server_(server), pipe_name_(pipe_name) {} + // Instantiates a thread which will invoke server->Run(delegate, pipe_name); + RunServerThread(ExceptionHandlerServer* server, + ExceptionHandlerServer::Delegate* delegate, + const std::string& pipe_name) + : server_(server), delegate_(delegate), pipe_name_(pipe_name) {} ~RunServerThread() override {} private: // Thread: - void ThreadMain() override { server_->Run(pipe_name_); } + void ThreadMain() override { server_->Run(delegate_, pipe_name_); } ExceptionHandlerServer* server_; + ExceptionHandlerServer::Delegate* delegate_; std::string pipe_name_; DISALLOW_COPY_AND_ASSIGN(RunServerThread); @@ -133,8 +121,7 @@ class RunServerThread : public Thread { // thread joined. class ScopedStopServerAndJoinThread { public: - explicit ScopedStopServerAndJoinThread(ExceptionHandlerServer* server, - Thread* thread) + ScopedStopServerAndJoinThread(ExceptionHandlerServer* server, Thread* thread) : server_(server), thread_(thread) {} ~ScopedStopServerAndJoinThread() { server_->Stop(); @@ -199,8 +186,9 @@ TEST_F(ExceptionSnapshotWinTest, ChildCrash) { ScopedKernelHANDLE completed(CreateEvent(nullptr, false, false, nullptr)); Delegate delegate(server_ready.get(), completed.get()); - ExceptionHandlerServer exception_handler_server(&delegate); - RunServerThread server_thread(&exception_handler_server, pipe_name); + ExceptionHandlerServer exception_handler_server; + RunServerThread server_thread( + &exception_handler_server, &delegate, pipe_name); server_thread.Start(); ScopedStopServerAndJoinThread scoped_stop_server_and_join_thread( &exception_handler_server, &server_thread); diff --git a/snapshot/win/process_snapshot_win.cc b/snapshot/win/process_snapshot_win.cc index cb0500ab..cb0c0d3a 100644 --- a/snapshot/win/process_snapshot_win.cc +++ b/snapshot/win/process_snapshot_win.cc @@ -16,6 +16,7 @@ #include "base/logging.h" #include "snapshot/win/module_snapshot_win.h" +#include "util/win/registration_protocol_win.h" #include "util/win/time.h" namespace crashpad { @@ -55,14 +56,22 @@ bool ProcessSnapshotWin::Initialize(HANDLE process) { } bool ProcessSnapshotWin::InitializeException( - DWORD thread_id, - WinVMAddress exception_pointers) { + WinVMAddress exception_information_address) { INITIALIZATION_STATE_DCHECK_VALID(initialized_); DCHECK(!exception_); + ExceptionInformation exception_information; + if (!process_reader_.ReadMemory(exception_information_address, + sizeof(exception_information), + &exception_information)) { + LOG(WARNING) << "ReadMemory ExceptionInformation failed"; + return false; + } + exception_.reset(new internal::ExceptionSnapshotWin()); - if (!exception_->Initialize( - &process_reader_, thread_id, exception_pointers)) { + if (!exception_->Initialize(&process_reader_, + exception_information.thread_id, + exception_information.exception_pointers)) { exception_.reset(); return false; } diff --git a/snapshot/win/process_snapshot_win.h b/snapshot/win/process_snapshot_win.h index 9e668ef9..6cb4976d 100644 --- a/snapshot/win/process_snapshot_win.h +++ b/snapshot/win/process_snapshot_win.h @@ -59,17 +59,18 @@ class ProcessSnapshotWin final : public ProcessSnapshot { //! \brief Initializes the object's exception. //! - //! This populates the data to be returned by Exception(). The parameters may - //! be passed directly through from a Windows exception handler. + //! This populates the data to be returned by Exception(). //! //! This method must not be called until after a successful call to //! Initialize(). //! + //! \param[in] exception_information_address The address in the client + //! process's address space of an ExceptionInformation structure. + //! //! \return `true` if the exception information could be initialized, `false` //! otherwise with an appropriate message logged. When this method returns //! `false`, the ProcessSnapshotWin object's validity remains unchanged. - bool InitializeException(DWORD thread_id, - WinVMAddress exception_pointers); + bool InitializeException(WinVMAddress exception_information_address); //! \brief Sets the value to be returned by ReportID(). //! diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc index a8312664..278d6e1d 100644 --- a/util/win/exception_handler_server.cc +++ b/util/win/exception_handler_server.cc @@ -184,9 +184,8 @@ class ClientData { ExceptionHandlerServer::Delegate::~Delegate() { } -ExceptionHandlerServer::ExceptionHandlerServer(Delegate* delegate) - : delegate_(delegate), - port_(CreateIoCompletionPort(INVALID_HANDLE_VALUE, nullptr, 0, 1)), +ExceptionHandlerServer::ExceptionHandlerServer() + : port_(CreateIoCompletionPort(INVALID_HANDLE_VALUE, nullptr, 0, 1)), clients_lock_(), clients_() { } @@ -194,7 +193,8 @@ ExceptionHandlerServer::ExceptionHandlerServer(Delegate* delegate) ExceptionHandlerServer::~ExceptionHandlerServer() { } -void ExceptionHandlerServer::Run(const std::string& pipe_name) { +void ExceptionHandlerServer::Run(Delegate* delegate, + const std::string& pipe_name) { uint64_t shutdown_token = base::RandUint64(); // We create two pipe instances, so that there's one listening while the // PipeServiceProc is processing a registration. @@ -217,7 +217,7 @@ void ExceptionHandlerServer::Run(const std::string& pipe_name) { internal::PipeServiceContext* context = new internal::PipeServiceContext(port_.get(), pipe, - delegate_, + delegate, &clients_lock_, &clients_, shutdown_token); @@ -225,7 +225,7 @@ void ExceptionHandlerServer::Run(const std::string& pipe_name) { CreateThread(nullptr, 0, &PipeServiceProc, context, 0, nullptr)); } - delegate_->ExceptionHandlerServerStarted(); + delegate->ExceptionHandlerServerStarted(); // This is the main loop of the server. Most work is done on the threadpool, // other than process end handling which is posted back to this main thread, diff --git a/util/win/exception_handler_server.h b/util/win/exception_handler_server.h index 42642f55..fd943acd 100644 --- a/util/win/exception_handler_server.h +++ b/util/win/exception_handler_server.h @@ -57,18 +57,16 @@ class ExceptionHandlerServer { }; //! \brief Constructs the exception handling server. - //! - //! \param[in] delegate The interface to which the exceptions are delegated - //! when they are caught in Run(). Ownership is not transferred and it - //! must outlive this object. - explicit ExceptionHandlerServer(Delegate* delegate); + ExceptionHandlerServer(); ~ExceptionHandlerServer(); //! \brief Runs the exception-handling server. //! + //! \param[in] delegate The interface to which the exceptions are delegated + //! when they are caught in Run(). Ownership is not transferred. //! \param[in] pipe_name The name of the pipe to listen on. Must be of the //! form "\\.\pipe\". - void Run(const std::string& pipe_name); + void Run(Delegate* delegate, const std::string& pipe_name); //! \brief Stops the exception-handling server. Returns immediately. The //! object must not be destroyed until Run() returns. @@ -81,7 +79,6 @@ class ExceptionHandlerServer { static void __stdcall OnDumpEvent(void* ctx, BOOLEAN); static void __stdcall OnProcessEnd(void* ctx, BOOLEAN); - Delegate* delegate_; // Weak. ScopedKernelHANDLE port_; base::Lock clients_lock_; diff --git a/util/win/exception_handler_server_test.cc b/util/win/exception_handler_server_test.cc index fd366b03..0c6a4799 100644 --- a/util/win/exception_handler_server_test.cc +++ b/util/win/exception_handler_server_test.cc @@ -36,16 +36,19 @@ namespace { // Runs the ExceptionHandlerServer on a background thread. class RunServerThread : public Thread { public: - // Instantiates a thread which will invoke server->Run(pipe_name). - RunServerThread(ExceptionHandlerServer* server, const std::string& pipe_name) - : server_(server), pipe_name_(pipe_name) {} + // Instantiates a thread which will invoke server->Run(delegate, pipe_name). + RunServerThread(ExceptionHandlerServer* server, + ExceptionHandlerServer::Delegate* delegate, + const std::string& pipe_name) + : server_(server), delegate_(delegate), pipe_name_(pipe_name) {} ~RunServerThread() override {} private: // Thread: - void ThreadMain() override { server_->Run(pipe_name_); } + void ThreadMain() override { server_->Run(delegate_, pipe_name_); } ExceptionHandlerServer* server_; + ExceptionHandlerServer::Delegate* delegate_; std::string pipe_name_; DISALLOW_COPY_AND_ASSIGN(RunServerThread); @@ -81,8 +84,8 @@ class ExceptionHandlerServerTest : public testing::Test { base::StringPrintf("%08x", GetCurrentProcessId())), server_ready_(CreateEvent(nullptr, false, false, nullptr)), delegate_(server_ready_.get()), - server_(&delegate_), - server_thread_(&server_, pipe_name_) {} + server_(), + server_thread_(&server_, &delegate_, pipe_name_) {} TestDelegate& delegate() { return delegate_; } ExceptionHandlerServer& server() { return server_; }