diff --git a/client/crashpad_client.h b/client/crashpad_client.h index cefa179b..18b7b131 100644 --- a/client/crashpad_client.h +++ b/client/crashpad_client.h @@ -51,9 +51,6 @@ class CrashpadClient { //! send right corresponding to a receive right held by the handler process. //! The handler process runs an exception server on this port. //! - //! On Windows, SetHandler() is normally used instead since the handler is - //! started by other means. - //! //! \param[in] handler The path to a Crashpad handler executable. //! \param[in] database The path to a Crashpad database. The handler will be //! started with this path as its `--database` argument. @@ -75,20 +72,32 @@ class CrashpadClient { const std::vector& arguments); #if defined(OS_WIN) || DOXYGEN - //! \brief Sets the IPC port of a presumably-running Crashpad handler process + //! \brief Sets the IPC pipe of a presumably-running Crashpad handler process //! which was started with StartHandler() or by other compatible means //! and does an IPC message exchange to register this process with the //! handler. However, just like StartHandler(), crashes are not serviced //! until UseHandler() is called. //! - //! The IPC port name (somehow) encodes enough information so that - //! registration is done with a crash handler using the appropriate database - //! and upload server. - //! - //! \param[in] ipc_port The full name of the crash handler IPC port. + //! \param[in] ipc_pipe The full name of the crash handler IPC pipe. This is + //! a string of the form `"\\.\pipe\NAME"`. //! //! \return `true` on success and `false` on failure. - bool SetHandler(const std::string& ipc_port); + bool SetHandlerIPCPipe(const std::wstring& ipc_pipe); + + //! \brief Retrieves the IPC pipe name used to register with the Crashpad + //! handler. + //! + //! This method retrieves the IPC pipe name set by SetHandlerIPCPipe(), or a + //! suitable IPC pipe name chosen by StartHandler(). It is intended to be used + //! to obtain the IPC pipe name so that it may be passed to other processes, + //! so that they may register with an existing Crashpad handler by calling + //! SetHandlerIPCPipe(). + //! + //! This method is only defined on Windows. + //! + //! \return The full name of the crash handler IPC pipe, a string of the form + //! `"\\.\pipe\NAME"`. + std::wstring GetHandlerIPCPipe() const; //! \brief Requests that the handler capture a dump even though there hasn't //! been a crash. @@ -101,7 +110,7 @@ class CrashpadClient { //! \brief Configures the process to direct its crashes to a Crashpad handler. //! //! The Crashpad handler must previously have been started by StartHandler() - //! or configured by SetHandler(). + //! or configured by SetHandlerIPCPipe(). //! //! On Mac OS X, this method sets the task’s exception port for `EXC_CRASH`, //! `EXC_RESOURCE`, and `EXC_GUARD` exceptions to the Mach send right obtained @@ -146,6 +155,8 @@ class CrashpadClient { private: #if defined(OS_MACOSX) base::mac::ScopedMachSendRight exception_port_; +#elif defined(OS_WIN) + std::wstring ipc_pipe_; #endif DISALLOW_COPY_AND_ASSIGN(CrashpadClient); diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index 92529ae1..3f23dde1 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -19,10 +19,13 @@ #include "base/atomicops.h" #include "base/logging.h" +#include "base/rand_util.h" #include "base/strings/string16.h" +#include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" #include "base/synchronization/lock.h" #include "util/file/file_io.h" +#include "util/win/command_line.h" #include "util/win/critical_section_with_debug_info.h" #include "util/win/registration_protocol_win.h" #include "util/win/scoped_handle.h" @@ -102,11 +105,17 @@ LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) { return EXCEPTION_CONTINUE_SEARCH; } +std::wstring FormatArgumentString(const std::string& name, + const std::wstring& value) { + return std::wstring(L"--") + base::UTF8ToUTF16(name) + L"=" + value; +} + } // namespace namespace crashpad { -CrashpadClient::CrashpadClient() { +CrashpadClient::CrashpadClient() + : ipc_pipe_() { } CrashpadClient::~CrashpadClient() { @@ -118,11 +127,86 @@ bool CrashpadClient::StartHandler( const std::string& url, const std::map& annotations, const std::vector& arguments) { - LOG(FATAL) << "SetHandler should be used on Windows"; - return false; + DCHECK(ipc_pipe_.empty()); + + std::string ipc_pipe = + base::StringPrintf("\\\\.\\pipe\\crashpad_%d_", GetCurrentProcessId()); + for (int index = 0; index < 16; ++index) { + ipc_pipe.append(1, static_cast(base::RandInt('A', 'Z'))); + } + ipc_pipe_ = base::UTF8ToUTF16(ipc_pipe); + + std::wstring command_line; + AppendCommandLineArgument(handler.value(), &command_line); + for (const std::string& argument : arguments) { + AppendCommandLineArgument(base::UTF8ToUTF16(argument), &command_line); + } + if (!database.value().empty()) { + AppendCommandLineArgument(FormatArgumentString("database", + database.value()), + &command_line); + } + if (!url.empty()) { + AppendCommandLineArgument(FormatArgumentString("url", + base::UTF8ToUTF16(url)), + &command_line); + } + for (const auto& kv : annotations) { + AppendCommandLineArgument( + FormatArgumentString("annotation", + base::UTF8ToUTF16(kv.first + '=' + kv.second)), + &command_line); + } + AppendCommandLineArgument(FormatArgumentString("pipe-name", ipc_pipe_), + &command_line); + + STARTUPINFO startup_info = {}; + startup_info.cb = sizeof(startup_info); + startup_info.dwFlags = STARTF_USESTDHANDLES; + startup_info.hStdInput = GetStdHandle(STD_INPUT_HANDLE); + startup_info.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE); + startup_info.hStdError = GetStdHandle(STD_ERROR_HANDLE); + PROCESS_INFORMATION process_info; + BOOL rv = CreateProcess(handler.value().c_str(), + &command_line[0], + nullptr, + nullptr, + true, + 0, + nullptr, + nullptr, + &startup_info, + &process_info); + if (!rv) { + PLOG(ERROR) << "CreateProcess"; + return false; + } + + rv = CloseHandle(process_info.hThread); + PLOG_IF(WARNING, !rv) << "CloseHandle thread"; + + rv = CloseHandle(process_info.hProcess); + PLOG_IF(WARNING, !rv) << "CloseHandle process"; + + return true; } -bool CrashpadClient::SetHandler(const std::string& ipc_port) { +bool CrashpadClient::SetHandlerIPCPipe(const std::wstring& ipc_pipe) { + DCHECK(ipc_pipe_.empty()); + DCHECK(!ipc_pipe.empty()); + + ipc_pipe_ = ipc_pipe; + + return true; +} + +std::wstring CrashpadClient::GetHandlerIPCPipe() const { + DCHECK(!ipc_pipe_.empty()); + return ipc_pipe_; +} + +bool CrashpadClient::UseHandler() { + DCHECK(!ipc_pipe_.empty()); DCHECK_EQ(g_signal_exception, INVALID_HANDLE_VALUE); DCHECK_EQ(g_signal_non_crash_dump, INVALID_HANDLE_VALUE); DCHECK_EQ(g_non_crash_dump_done, INVALID_HANDLE_VALUE); @@ -153,8 +237,7 @@ bool CrashpadClient::SetHandler(const std::string& ipc_port) { ServerToClientMessage response = {0}; - if (!SendToCrashHandlerServer( - base::UTF8ToUTF16(ipc_port), message, &response)) { + if (!SendToCrashHandlerServer(ipc_pipe_, message, &response)) { return false; } @@ -168,16 +251,6 @@ bool CrashpadClient::SetHandler(const std::string& ipc_port) { g_non_crash_dump_lock = new base::Lock(); - return true; -} - -bool CrashpadClient::UseHandler() { - if (g_signal_exception == INVALID_HANDLE_VALUE || - g_signal_non_crash_dump == INVALID_HANDLE_VALUE || - g_non_crash_dump_done == INVALID_HANDLE_VALUE) { - return false; - } - // In theory we could store the previous handler but it is not clear what // use we have for it. SetUnhandledExceptionFilter(&UnhandledExceptionHandler); @@ -188,7 +261,7 @@ bool CrashpadClient::UseHandler() { void CrashpadClient::DumpWithoutCrash(const CONTEXT& context) { if (g_signal_non_crash_dump == INVALID_HANDLE_VALUE || g_non_crash_dump_done == INVALID_HANDLE_VALUE) { - LOG(ERROR) << "haven't called SetHandler()"; + LOG(ERROR) << "haven't called UseHandler()"; return; } diff --git a/client/simulate_crash_win.h b/client/simulate_crash_win.h index 81c0af93..a20f3dad 100644 --- a/client/simulate_crash_win.h +++ b/client/simulate_crash_win.h @@ -20,6 +20,8 @@ #include "client/crashpad_client.h" #include "util/win/capture_context.h" +//! \file + //! \brief Captures the CPU context and captures a dump without an exception. #define CRASHPAD_SIMULATE_CRASH() \ do { \ diff --git a/compat/non_win/dbghelp.h b/compat/non_win/dbghelp.h index 08f07ae8..ecf8bab8 100644 --- a/compat/non_win/dbghelp.h +++ b/compat/non_win/dbghelp.h @@ -564,7 +564,9 @@ struct __attribute__((packed, aligned(4))) MINIDUMP_MODULE { //! CodeView 4.1 format. Signatures seen in the wild include “NB09” //! (0x3930424e) for CodeView 4.1 and “NB11” (0x3131424e) for CodeView 5.0. //! This form of debugging information within the module, as opposed to a link - //! to an external `.pdb` file, is chosen by building with `/Z7`. + //! to an external `.pdb` file, is chosen by building with `/Z7` in Visual + //! Studio 6.0 (1998) and earlier. This embedded form of debugging information + //! is now considered obsolete. //! //! On Windows, the CodeView record is taken from a module’s //! IMAGE_DEBUG_DIRECTORY entry whose Type field has the value diff --git a/handler/crashpad_handler.ad b/handler/crashpad_handler.ad index f00ca054..a97e2088 100644 --- a/handler/crashpad_handler.ad +++ b/handler/crashpad_handler.ad @@ -33,14 +33,23 @@ collection server. Uploads are disabled by default, and can only be enabled by a Crashpad client using the Crashpad client library, typically in response to a user requesting this behavior. -This server is normally started by its initial client, and it performs a -handshake with this client via a pipe established by the client that is +On OS X, this server is normally started by its initial client, and it performs +a handshake with this client via a pipe established by the client that is inherited by the server, referenced by the *--handshake-fd* argument. During the handshake, the server furnishes the client with a send right that the client may use as an exception port. The server retains the corresponding receive right, which it monitors for exception messages. When the receive right loses all senders, the server exits after allowing any upload in progress to complete. +On Windows, clients register with this server by communicating with it via the +named pipe identified by the *--pipe-name* argument. During registration, a +client provides the server with an OS event object that it will signal should it +crash. The server obtains the client’s process handle and waits on the crash +event object for a crash, as well as the client’s process handle for the client +to exit cleanly without crashing. When the server loses all clients and +*--persistent* is not specified, it exits after allowing any upload in progress +to complete. + It is not normally appropriate to invoke this program directly. Usually, it will be invoked by a Crashpad client using the Crashpad client library. Arbitrary programs may be run with a Crashpad handler by using @@ -77,6 +86,12 @@ of 'PATH' exists. Perform the handshake with the initial client on the file descriptor at 'FD'. This option is required. This option is only valid on Mac OS X. +*--persistent*:: +Continue running after the last client exits. If this option is not specified, +this server will exit as soon as it has no clients, although on startup, it +always waits for at least one client to connect. This option is only valid on +Windows. + *--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 diff --git a/handler/handler.gyp b/handler/handler.gyp index 04433ac8..20df3d73 100644 --- a/handler/handler.gyp +++ b/handler/handler.gyp @@ -81,7 +81,6 @@ 'dependencies': [ '../client/client.gyp:crashpad_client', '../third_party/mini_chromium/mini_chromium.gyp:base', - '../tools/tools.gyp:crashpad_tool_support', '../util/util.gyp:crashpad_util', ], 'include_dirs': [ @@ -99,7 +98,6 @@ '../compat/compat.gyp:crashpad_compat', '../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': [ @@ -110,6 +108,28 @@ ], }, ], + 'conditions': [ + # Cannot create an x64 DLL with embedded debug info. + ['target_arch=="ia32"', { + 'targets': [ + { + 'target_name': 'crashy_z7_loader', + 'type': 'executable', + 'dependencies': [ + '../client/client.gyp:crashpad_client', + '../test/test.gyp:crashpad_test', + '../third_party/mini_chromium/mini_chromium.gyp:base', + ], + 'include_dirs': [ + '..', + ], + 'sources': [ + 'win/crashy_test_z7_loader.cc', + ], + }, + ], + }], + ], }, { 'targets': [], }], diff --git a/handler/main.cc b/handler/main.cc index de3171ed..9a9b6f5d 100644 --- a/handler/main.cc +++ b/handler/main.cc @@ -60,6 +60,7 @@ void Usage(const base::FilePath& me) { " --reset-own-crash-exception-port-to-system-default\n" " reset the server's exception handler to default\n" #elif defined(OS_WIN) +" --persistent continue running after all clients exit\n" " --pipe-name=PIPE communicate with the client over PIPE\n" #endif // OS_MACOSX " --url=URL send crash reports to this Breakpad server URL,\n" @@ -84,6 +85,7 @@ int HandlerMain(int argc, char* argv[]) { kOptionHandshakeFD, kOptionResetOwnCrashExceptionPortToSystemDefault, #elif defined(OS_WIN) + kOptionPersistent, kOptionPipeName, #endif // OS_MACOSX kOptionURL, @@ -101,8 +103,9 @@ int HandlerMain(int argc, char* argv[]) { int handshake_fd; bool reset_own_crash_exception_port_to_system_default; #elif defined(OS_WIN) + bool persistent; std::string pipe_name; -#endif +#endif // OS_MACOSX } options = {}; #if defined(OS_MACOSX) options.handshake_fd = -1; @@ -119,8 +122,9 @@ int HandlerMain(int argc, char* argv[]) { nullptr, kOptionResetOwnCrashExceptionPortToSystemDefault}, #elif defined(OS_WIN) + {"persistent", no_argument, nullptr, kOptionPersistent}, {"pipe-name", required_argument, nullptr, kOptionPipeName}, -#endif +#endif // OS_MACOSX {"url", required_argument, nullptr, kOptionURL}, {"help", no_argument, nullptr, kOptionHelp}, {"version", no_argument, nullptr, kOptionVersion}, @@ -163,6 +167,10 @@ int HandlerMain(int argc, char* argv[]) { break; } #elif defined(OS_WIN) + case kOptionPersistent: { + options.persistent = true; + break; + } case kOptionPipeName: { options.pipe_name = optarg; break; @@ -228,7 +236,8 @@ int HandlerMain(int argc, char* argv[]) { ExceptionHandlerServer exception_handler_server(receive_right.Pass()); #elif defined(OS_WIN) - ExceptionHandlerServer exception_handler_server(options.pipe_name); + ExceptionHandlerServer exception_handler_server(options.pipe_name, + options.persistent); #endif // OS_MACOSX scoped_ptr database(CrashReportDatabase::Initialize( diff --git a/handler/win/crashy_test_program.cc b/handler/win/crashy_test_program.cc index 66504048..7d61c256 100644 --- a/handler/win/crashy_test_program.cc +++ b/handler/win/crashy_test_program.cc @@ -12,18 +12,24 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include +#include #include #include +#include +#include +#include + // ntstatus.h conflicts with windows.h so define this locally. #ifndef STATUS_NO_SUCH_FILE #define STATUS_NO_SUCH_FILE static_cast(0xC000000F) #endif #include "base/basictypes.h" +#include "base/files/file_path.h" #include "base/logging.h" #include "client/crashpad_client.h" -#include "tools/tool_support.h" #include "util/win/critical_section_with_debug_info.h" #include "util/win/get_function.h" @@ -87,20 +93,32 @@ void SomeCrashyFunction() { *foo = 42; } -int CrashyMain(int argc, char* argv[]) { - if (argc != 2) { +int CrashyMain(int argc, wchar_t* argv[]) { + CrashpadClient client; + + if (argc == 2) { + if (!client.SetHandlerIPCPipe(argv[1])) { + LOG(ERROR) << "SetHandler"; + return EXIT_FAILURE; + } + } else if (argc == 3) { + if (!client.StartHandler(base::FilePath(argv[1]), + base::FilePath(argv[2]), + std::string(), + std::map(), + std::vector())) { + LOG(ERROR) << "StartHandler"; + return EXIT_FAILURE; + } + } else { fprintf(stderr, "Usage: %s \n", argv[0]); - return 1; + fprintf(stderr, " %s \n", argv[0]); + return EXIT_FAILURE; } - CrashpadClient client; - if (!client.SetHandler(argv[1])) { - LOG(ERROR) << "SetHandler"; - return 1; - } if (!client.UseHandler()) { LOG(ERROR) << "UseHandler"; - return 1; + return EXIT_FAILURE; } AllocateMemoryOfVariousProtections(); @@ -112,12 +130,12 @@ int CrashyMain(int argc, char* argv[]) { SomeCrashyFunction(); - return 0; + return EXIT_SUCCESS; } } // namespace } // namespace crashpad int wmain(int argc, wchar_t* argv[]) { - return crashpad::ToolSupport::Wmain(argc, argv, crashpad::CrashyMain); + return crashpad::CrashyMain(argc, argv); } diff --git a/handler/win/crashy_test_z7_loader.cc b/handler/win/crashy_test_z7_loader.cc new file mode 100644 index 00000000..284afa86 --- /dev/null +++ b/handler/win/crashy_test_z7_loader.cc @@ -0,0 +1,72 @@ +// 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 +#include + +#include "base/files/file_path.h" +#include "base/logging.h" +#include "build/build_config.h" +#include "client/crashpad_client.h" +#include "test/paths.h" + +#if !defined(ARCH_CPU_X86) +#error This test is only supported on x86. +#endif // !ARCH_CPU_X86 + +namespace crashpad { +namespace { + +int CrashyLoadZ7Main(int argc, wchar_t* argv[]) { + if (argc != 2) { + fprintf(stderr, "Usage: %ls \n", argv[0]); + return EXIT_FAILURE; + } + + CrashpadClient client; + if (!client.SetHandlerIPCPipe(argv[1])) { + LOG(ERROR) << "SetHandler"; + return EXIT_FAILURE; + } + if (!client.UseHandler()) { + LOG(ERROR) << "UseHandler"; + return EXIT_FAILURE; + } + + // The DLL has /Z7 symbols embedded in the binary (rather than in a .pdb). + // There's only an x86 version of this dll as newer x64 toolchains can't + // generate this format any more. + base::FilePath z7_path = test::Paths::TestDataRoot().Append( + FILE_PATH_LITERAL("handler/win/z7_test.dll")); + HMODULE z7_test = LoadLibrary(z7_path.value().c_str()); + if (!z7_test) { + PLOG(ERROR) << "LoadLibrary"; + return EXIT_FAILURE; + } + FARPROC crash_me = GetProcAddress(z7_test, "CrashMe"); + if (!crash_me) { + PLOG(ERROR) << "GetProcAddress"; + return EXIT_FAILURE; + } + reinterpret_cast(crash_me)(); + + return EXIT_SUCCESS; +} + +} // namespace +} // namespace crashpad + +int wmain(int argc, wchar_t* argv[]) { + return crashpad::CrashyLoadZ7Main(argc, argv); +} diff --git a/handler/win/self_destroying_test_program.cc b/handler/win/self_destroying_test_program.cc index d00fa476..ac358ef0 100644 --- a/handler/win/self_destroying_test_program.cc +++ b/handler/win/self_destroying_test_program.cc @@ -21,7 +21,6 @@ #include "base/strings/stringprintf.h" #include "client/crashpad_client.h" #include "snapshot/win/process_reader_win.h" -#include "tools/tool_support.h" namespace crashpad { namespace { @@ -65,14 +64,14 @@ bool FreeOwnStackAndBreak() { return true; } -int SelfDestroyingMain(int argc, char* argv[]) { +int SelfDestroyingMain(int argc, wchar_t* argv[]) { if (argc != 2) { - fprintf(stderr, "Usage: %s \n", argv[0]); + fprintf(stderr, "Usage: %ls \n", argv[0]); return EXIT_FAILURE; } CrashpadClient client; - if (!client.SetHandler(argv[1])) { + if (!client.SetHandlerIPCPipe(argv[1])) { LOG(ERROR) << "SetHandler"; return EXIT_FAILURE; } @@ -93,5 +92,5 @@ int SelfDestroyingMain(int argc, char* argv[]) { } // namespace crashpad int wmain(int argc, wchar_t* argv[]) { - return crashpad::ToolSupport::Wmain(argc, argv, crashpad::SelfDestroyingMain); + return crashpad::SelfDestroyingMain(argc, argv); } diff --git a/handler/win/z7_test.cpp b/handler/win/z7_test.cpp new file mode 100644 index 00000000..ad7b5d98 --- /dev/null +++ b/handler/win/z7_test.cpp @@ -0,0 +1,32 @@ +// 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. + +// Build in VC++6 or older command prompt with: +// +// cl /nologo /W4 /MT /Z7 z7_test.cpp /link /dll /out:z7_test.dll /debugtype:cv /pdb:none +// +// Given that this is quite tedious to build, the result is also checked in. + +#include +#include + +extern "C" __declspec(dllexport) void CrashMe() { + volatile int* foo = reinterpret_cast(7); + *foo = 42; +} + +BOOL WINAPI DllMain(HINSTANCE hinstance, DWORD reason, LPVOID) { + printf("%p %d\n", hinstance, reason); + return TRUE; +} diff --git a/handler/win/z7_test.dll b/handler/win/z7_test.dll new file mode 100644 index 00000000..d470742e Binary files /dev/null and b/handler/win/z7_test.dll differ diff --git a/snapshot/win/crashpad_snapshot_test_crashing_child.cc b/snapshot/win/crashpad_snapshot_test_crashing_child.cc index 7f4b8e5b..d4023c65 100644 --- a/snapshot/win/crashpad_snapshot_test_crashing_child.cc +++ b/snapshot/win/crashpad_snapshot_test_crashing_child.cc @@ -14,20 +14,25 @@ #include +#include "base/files/file_path.h" #include "base/logging.h" #include "client/crashpad_client.h" #include "util/file/file_io.h" #include "util/win/address_types.h" +namespace { + __declspec(noinline) crashpad::WinVMAddress CurrentAddress() { return reinterpret_cast(_ReturnAddress()); } -int main(int argc, char* argv[]) { +} // namespace + +int wmain(int argc, wchar_t* argv[]) { CHECK_EQ(argc, 2); crashpad::CrashpadClient client; - CHECK(client.SetHandler(argv[1])); + CHECK(client.SetHandlerIPCPipe(argv[1])); CHECK(client.UseHandler()); HANDLE out = GetStdHandle(STD_OUTPUT_HANDLE); diff --git a/snapshot/win/crashpad_snapshot_test_dump_without_crashing.cc b/snapshot/win/crashpad_snapshot_test_dump_without_crashing.cc index 1ecc18ed..cf5ab950 100644 --- a/snapshot/win/crashpad_snapshot_test_dump_without_crashing.cc +++ b/snapshot/win/crashpad_snapshot_test_dump_without_crashing.cc @@ -20,15 +20,19 @@ #include "util/file/file_io.h" #include "util/win/address_types.h" +namespace { + __declspec(noinline) crashpad::WinVMAddress CurrentAddress() { return reinterpret_cast(_ReturnAddress()); } -int main(int argc, char* argv[]) { +} // namespace + +int wmain(int argc, wchar_t* argv[]) { CHECK_EQ(argc, 2); crashpad::CrashpadClient client; - CHECK(client.SetHandler(argv[1])); + CHECK(client.SetHandlerIPCPipe(argv[1])); CHECK(client.UseHandler()); HANDLE out = GetStdHandle(STD_OUTPUT_HANDLE); diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py index 9ed5fda7..3bbcdb98 100644 --- a/snapshot/win/end_to_end_test.py +++ b/snapshot/win/end_to_end_test.py @@ -41,6 +41,7 @@ def CleanUpTempDirs(): def FindInstalledWindowsApplication(app_path): search_paths = [os.getenv('PROGRAMFILES(X86)'), os.getenv('PROGRAMFILES'), + os.getenv('PROGRAMW6432'), os.getenv('LOCALAPPDATA')] search_paths += os.getenv('PATH', '').split(os.pathsep) @@ -77,9 +78,11 @@ def GetCdbPath(): def GetDumpFromProgram(out_dir, pipe_name, executable_name): - """Initialize a crash database, run crashpad_handler, run |executable_name| - connecting to the crash_handler. Returns the minidump generated by - crash_handler for further testing. + """Initialize a crash database, and run |executable_name| connecting to a + crash handler. If pipe_name is set, crashpad_handler will be started first. If + pipe_name is empty, the executable is responsible for starting + crashpad_handler. Returns the minidump generated by crashpad_handler for + further testing. """ test_database = MakeTempDir() handler = None @@ -91,13 +94,18 @@ def GetDumpFromProgram(out_dir, pipe_name, executable_name): print 'could not initialize report database' return None - handler = subprocess.Popen([ - os.path.join(out_dir, 'crashpad_handler.exe'), - '--pipe-name=' + pipe_name, - '--database=' + test_database - ]) + if pipe_name is not None: + handler = subprocess.Popen([ + os.path.join(out_dir, 'crashpad_handler.exe'), + '--pipe-name=' + pipe_name, + '--database=' + test_database + ]) - subprocess.call([os.path.join(out_dir, executable_name), pipe_name]) + subprocess.call([os.path.join(out_dir, executable_name), pipe_name]) + else: + subprocess.call([os.path.join(out_dir, executable_name), + os.path.join(out_dir, 'crashpad_handler.exe'), + test_database]) out = subprocess.check_output([ os.path.join(out_dir, 'crashpad_database_util.exe'), @@ -121,6 +129,10 @@ def GetDumpFromSelfDestroyingProgram(out_dir, pipe_name): return GetDumpFromProgram(out_dir, pipe_name, 'self_destroying_program.exe') +def GetDumpFromZ7Program(out_dir, pipe_name): + return GetDumpFromProgram(out_dir, pipe_name, 'crashy_z7_loader.exe') + + class CdbRun(object): """Run cdb.exe passing it a cdb command and capturing the output. `Check()` searches for regex patterns in sequence allowing verification of @@ -155,7 +167,12 @@ class CdbRun(object): sys.exit(1) -def RunTests(cdb_path, dump_path, destroyed_dump_path, pipe_name): +def RunTests(cdb_path, + dump_path, + start_handler_dump_path, + destroyed_dump_path, + z7_dump_path, + pipe_name): """Runs various tests in sequence. Runs a new cdb instance on the dump for each block of tests to reduce the chances that output from one command is confused for output from another. @@ -167,6 +184,13 @@ def RunTests(cdb_path, dump_path, destroyed_dump_path, pipe_name): 'crashy_program!crashpad::`anonymous namespace\'::SomeCrashyFunction', 'exception at correct location') + out = CdbRun(cdb_path, start_handler_dump_path, '.ecxr') + out.Check('This dump file has an exception of interest stored in it', + 'captured exception (using StartHandler())') + out.Check( + 'crashy_program!crashpad::`anonymous namespace\'::SomeCrashyFunction', + 'exception at correct location (using StartHandler())') + out = CdbRun(cdb_path, dump_path, '!peb') out.Check(r'PEB at', 'found the PEB') out.Check(r'Ldr\.InMemoryOrderModuleList:.*\d+ \. \d+', 'PEB_LDR_DATA saved') @@ -212,6 +236,18 @@ def RunTests(cdb_path, dump_path, destroyed_dump_path, pipe_name): r'FreeOwnStackAndBreak.*\nquit:', 'at correct location, no additional stack entries') + if z7_dump_path: + out = CdbRun(cdb_path, z7_dump_path, '.ecxr;lm') + out.Check('This dump file has an exception of interest stored in it', + 'captured exception in z7 module') + # Older versions of cdb display relative to exports for /Z7 modules, newer + # ones just display the offset. + out.Check(r'z7_test(!CrashMe\+0xe|\+0x100e):', + 'exception in z7 at correct location') + out.Check(r'z7_test C \(codeview symbols\) z7_test.dll', + 'expected non-pdb symbol format') + + def main(args): try: if len(args) != 1: @@ -238,11 +274,26 @@ def main(args): if not crashy_dump_path: return 1 + start_handler_dump_path = GetDumpFromCrashyProgram(args[0], None) + if not start_handler_dump_path: + return 1 + destroyed_dump_path = GetDumpFromSelfDestroyingProgram(args[0], pipe_name) if not destroyed_dump_path: return 1 - RunTests(cdb_path, crashy_dump_path, destroyed_dump_path, pipe_name) + z7_dump_path = None + if not args[0].endswith('x64'): + z7_dump_path = GetDumpFromZ7Program(args[0], pipe_name) + if not z7_dump_path: + return 1 + + RunTests(cdb_path, + crashy_dump_path, + start_handler_dump_path, + destroyed_dump_path, + z7_dump_path, + pipe_name) return 0 finally: diff --git a/snapshot/win/exception_snapshot_win_test.cc b/snapshot/win/exception_snapshot_win_test.cc index fe626a38..8e2bc61b 100644 --- a/snapshot/win/exception_snapshot_win_test.cc +++ b/snapshot/win/exception_snapshot_win_test.cc @@ -128,7 +128,7 @@ void TestCrashingChild(const base::string16& directory_modification) { ScopedKernelHANDLE completed(CreateEvent(nullptr, false, false, nullptr)); CrashingDelegate delegate(server_ready.get(), completed.get()); - ExceptionHandlerServer exception_handler_server(pipe_name); + ExceptionHandlerServer exception_handler_server(pipe_name, true); RunServerThread server_thread(&exception_handler_server, &delegate); server_thread.Start(); ScopedStopServerAndJoinThread scoped_stop_server_and_join_thread( @@ -230,7 +230,7 @@ void TestDumpWithoutCrashingChild( ScopedKernelHANDLE completed(CreateEvent(nullptr, false, false, nullptr)); SimulateDelegate delegate(server_ready.get(), completed.get()); - ExceptionHandlerServer exception_handler_server(pipe_name); + ExceptionHandlerServer exception_handler_server(pipe_name, true); RunServerThread server_thread(&exception_handler_server, &delegate); server_thread.Start(); ScopedStopServerAndJoinThread scoped_stop_server_and_join_thread( diff --git a/snapshot/win/module_snapshot_win.cc b/snapshot/win/module_snapshot_win.cc index 3876fb1e..194d15ac 100644 --- a/snapshot/win/module_snapshot_win.cc +++ b/snapshot/win/module_snapshot_win.cc @@ -60,6 +60,14 @@ bool ModuleSnapshotWin::Initialize( &uuid_, &age_dword, &pdb_name_)) { static_assert(sizeof(DWORD) == sizeof(uint32_t), "unexpected age size"); age_ = age_dword; + } else { + // If we fully supported all old debugging formats, we would want to extract + // and emit a different type of CodeView record here (as old Microsoft tools + // would do). As we don't expect to ever encounter a module that wouldn't be + // be using .PDB that we actually have symbols for, we simply set a + // plausible name here, but this will never correspond to symbols that we + // have. + pdb_name_ = base::UTF16ToUTF8(name_); } INITIALIZATION_STATE_SET_VALID(initialized_); diff --git a/snapshot/win/pe_image_reader.cc b/snapshot/win/pe_image_reader.cc index b6bb7aa6..82bce064 100644 --- a/snapshot/win/pe_image_reader.cc +++ b/snapshot/win/pe_image_reader.cc @@ -176,35 +176,41 @@ bool PEImageReader::ReadDebugDirectoryInformation(UUID* uuid, if (debug_directory.Type != IMAGE_DEBUG_TYPE_CODEVIEW) continue; - if (debug_directory.SizeOfData < sizeof(CodeViewRecordPDB70)) { - LOG(WARNING) << "CodeView debug entry of unexpected size"; - continue; - } - scoped_ptr data(new char[debug_directory.SizeOfData]); - if (!CheckedReadMemory(Address() + debug_directory.AddressOfRawData, - debug_directory.SizeOfData, - data.get())) { - LOG(WARNING) << "could not read debug directory"; - return false; - } + if (debug_directory.AddressOfRawData) { + if (debug_directory.SizeOfData < sizeof(CodeViewRecordPDB70)) { + LOG(WARNING) << "CodeView debug entry of unexpected size"; + continue; + } - if (*reinterpret_cast(data.get()) != - CodeViewRecordPDB70::kSignature) { - // TODO(scottmg): Consider supporting other record types, see + scoped_ptr data(new char[debug_directory.SizeOfData]); + if (!CheckedReadMemory(Address() + debug_directory.AddressOfRawData, + debug_directory.SizeOfData, + data.get())) { + LOG(WARNING) << "could not read debug directory"; + return false; + } + + if (*reinterpret_cast(data.get()) != + CodeViewRecordPDB70::kSignature) { + LOG(WARNING) << "encountered non-7.0 CodeView debug record"; + continue; + } + + CodeViewRecordPDB70* codeview = + reinterpret_cast(data.get()); + *uuid = codeview->uuid; + *age = codeview->age; + // This is a NUL-terminated string encoded in the codepage of the system + // where the binary was linked. We have no idea what that was, so we just + // assume ASCII. + *pdbname = std::string(reinterpret_cast(&codeview->pdb_name[0])); + return true; + } else if (debug_directory.PointerToRawData) { + // This occurs for non-PDB based debug information. We simply ignore these + // as we don't expect to encounter modules that will be in this format + // for which we'll actually have symbols. See // https://crashpad.chromium.org/bug/47. - LOG(WARNING) << "encountered non-7.0 CodeView debug record"; - continue; } - - CodeViewRecordPDB70* codeview = - reinterpret_cast(data.get()); - *uuid = codeview->uuid; - *age = codeview->age; - // This is a NUL-terminated string encoded in the codepage of the system - // where the binary was linked. We have no idea what that was, so we just - // assume ASCII. - *pdbname = std::string(reinterpret_cast(&codeview->pdb_name[0])); - return true; } return false; diff --git a/test/multiprocess_exec_win.cc b/test/multiprocess_exec_win.cc index 23853dd0..2a58f846 100644 --- a/test/multiprocess_exec_win.cc +++ b/test/multiprocess_exec_win.cc @@ -17,7 +17,7 @@ #include "base/logging.h" #include "base/strings/utf_string_conversions.h" #include "gtest/gtest.h" -#include "test/win/child_launcher.h" +#include "util/win/command_line.h" namespace crashpad { namespace test { @@ -119,7 +119,6 @@ void MultiprocessExec::PreFork() { command_line_.clear(); AppendCommandLineArgument(base::UTF8ToUTF16(command_), &command_line_); for (size_t i = 0; i < arguments_.size(); ++i) { - command_line_ += L" "; AppendCommandLineArgument(base::UTF8ToUTF16(arguments_[i]), &command_line_); } diff --git a/test/win/child_launcher.cc b/test/win/child_launcher.cc index 2b82e057..e35f1cb8 100644 --- a/test/win/child_launcher.cc +++ b/test/win/child_launcher.cc @@ -15,6 +15,7 @@ #include "test/win/child_launcher.h" #include "gtest/gtest.h" +#include "util/win/command_line.h" namespace crashpad { namespace test { @@ -96,39 +97,5 @@ DWORD ChildLauncher::WaitForExit() { return exit_code; } -// Ref: http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx -void AppendCommandLineArgument(const std::wstring& argument, - std::wstring* command_line) { - // Don't bother quoting if unnecessary. - if (!argument.empty() && - argument.find_first_of(L" \t\n\v\"") == std::wstring::npos) { - command_line->append(argument); - } else { - command_line->push_back(L'"'); - for (std::wstring::const_iterator i = argument.begin();; ++i) { - size_t backslash_count = 0; - while (i != argument.end() && *i == L'\\') { - ++i; - ++backslash_count; - } - if (i == argument.end()) { - // Escape all backslashes, but let the terminating double quotation mark - // we add below be interpreted as a metacharacter. - command_line->append(backslash_count * 2, L'\\'); - break; - } else if (*i == L'"') { - // Escape all backslashes and the following double quotation mark. - command_line->append(backslash_count * 2 + 1, L'\\'); - command_line->push_back(*i); - } else { - // Backslashes aren't special here. - command_line->append(backslash_count, L'\\'); - command_line->push_back(*i); - } - } - command_line->push_back(L'"'); - } -} - } // namespace test } // namespace crashpad diff --git a/test/win/child_launcher.h b/test/win/child_launcher.h index 456d969d..22463fa5 100644 --- a/test/win/child_launcher.h +++ b/test/win/child_launcher.h @@ -42,7 +42,7 @@ class ChildLauncher { void Start(); //! \brief Waits for the child process to exit. - //! + //! //! \return The process exit code. DWORD WaitForExit(); @@ -67,14 +67,6 @@ class ChildLauncher { ScopedFileHANDLE stdin_write_handle_; }; -//! \brief Utility function for building escaped command lines. -//! -//! \param[in] argument Appended to \a command_line surrounded by properly -//! escaped quotation marks, if necessary. -//! \param[inout] command_line The command line being constructed. -void AppendCommandLineArgument(const std::wstring& argument, - std::wstring* command_line); - } // namespace test } // namespace crashpad diff --git a/util/util.gyp b/util/util.gyp index e8206b91..65791b5a 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -152,6 +152,8 @@ 'win/capture_context.asm', 'win/capture_context.h', 'win/checked_win_address_range.h', + 'win/command_line.cc', + 'win/command_line.h', 'win/critical_section_with_debug_info.cc', 'win/critical_section_with_debug_info.h', 'win/exception_handler_server.cc', diff --git a/util/util_test.gyp b/util/util_test.gyp index 6517e0b4..40bb64fd 100644 --- a/util/util_test.gyp +++ b/util/util_test.gyp @@ -80,6 +80,7 @@ 'thread/thread_log_messages_test.cc', 'thread/thread_test.cc', 'win/capture_context_test.cc', + 'win/command_line_test.cc', 'win/critical_section_with_debug_info_test.cc', 'win/exception_handler_server_test.cc', 'win/get_function_test.cc', diff --git a/util/win/command_line.cc b/util/win/command_line.cc new file mode 100644 index 00000000..5829c12d --- /dev/null +++ b/util/win/command_line.cc @@ -0,0 +1,58 @@ +// 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 "util/win/command_line.h" + +namespace crashpad { + +// Ref: +// http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx +void AppendCommandLineArgument(const std::wstring& argument, + std::wstring* command_line) { + if (!command_line->empty()) { + command_line->push_back(L' '); + } + + // Don’t bother quoting if unnecessary. + if (!argument.empty() && + argument.find_first_of(L" \t\n\v\"") == std::wstring::npos) { + command_line->append(argument); + } else { + command_line->push_back(L'"'); + for (std::wstring::const_iterator i = argument.begin();; ++i) { + size_t backslash_count = 0; + while (i != argument.end() && *i == L'\\') { + ++i; + ++backslash_count; + } + if (i == argument.end()) { + // Escape all backslashes, but let the terminating double quotation mark + // we add below be interpreted as a metacharacter. + command_line->append(backslash_count * 2, L'\\'); + break; + } else if (*i == L'"') { + // Escape all backslashes and the following double quotation mark. + command_line->append(backslash_count * 2 + 1, L'\\'); + command_line->push_back(*i); + } else { + // Backslashes aren’t special here. + command_line->append(backslash_count, L'\\'); + command_line->push_back(*i); + } + } + command_line->push_back(L'"'); + } +} + +} // namespace crashpad diff --git a/util/win/command_line.h b/util/win/command_line.h new file mode 100644 index 00000000..eb3f7124 --- /dev/null +++ b/util/win/command_line.h @@ -0,0 +1,38 @@ +// 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_UTIL_WIN_COMMAND_LINE_H_ +#define CRASHPAD_UTIL_WIN_COMMAND_LINE_H_ + +#include + +namespace crashpad { + +//! \brief Utility function for building escaped command lines. +//! +//! This builds a command line so that individual arguments can be reliably +//! decoded by `CommandLineToArgvW()`. +//! +//! \a argument is appended to \a command_line. If necessary, it will be placed +//! in quotation marks and escaped properly. If \a command_line is initially +//! non-empty, a space will precede \a argument. +//! +//! \param[in] argument The argument to append to \a command_line. +//! \param[inout] command_line The command line being constructed. +void AppendCommandLineArgument(const std::wstring& argument, + std::wstring* command_line); + +} // namespace crashpad + +#endif // CRASHPAD_UTIL_WIN_COMMAND_LINE_H_ diff --git a/util/win/command_line_test.cc b/util/win/command_line_test.cc new file mode 100644 index 00000000..5dc9e901 --- /dev/null +++ b/util/win/command_line_test.cc @@ -0,0 +1,177 @@ +// 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 "util/win/command_line.h" + +#include +#include + +#include "base/basictypes.h" +#include "base/logging.h" +#include "base/scoped_generic.h" +#include "gtest/gtest.h" +#include "test/errors.h" + +namespace crashpad { +namespace test { +namespace { + +struct LocalAllocTraits { + static HLOCAL InvalidValue() { + return nullptr; + } + + static void Free(HLOCAL memory) { + PLOG_IF(ERROR, LocalFree(memory) != nullptr) << "LocalFree"; + } +}; +using ScopedLocalAlloc = base::ScopedGeneric; + +// Calls AppendCommandLineArgument() for every argument in argv, then calls +// CommandLineToArgvW() to decode the string into a vector again, and compares +// the input and output. +void AppendCommandLineArgumentTest(size_t argc, const wchar_t* argv[]) { + std::wstring command_line; + for (size_t index = 0; index < argc; ++index) { + AppendCommandLineArgument(argv[index], &command_line); + } + + int test_argc; + wchar_t** test_argv = CommandLineToArgvW(command_line.c_str(), &test_argc); + + ASSERT_TRUE(test_argv) << ErrorMessage("CommandLineToArgvW"); + ScopedLocalAlloc test_argv_owner(test_argv); + ASSERT_EQ(argc, test_argc); + + for (size_t index = 0; index < argc; ++index) { + EXPECT_STREQ(argv[index], test_argv[index]) << "index " << index; + } + EXPECT_FALSE(test_argv[argc]); +} + +TEST(CommandLine, AppendCommandLineArgument) { + // Most of these test cases come from + // http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx, + // which was also a reference for the implementation of + // AppendCommandLineArgument(). + + { + SCOPED_TRACE("simple"); + + const wchar_t* kArguments[] = { + L"child.exe", + L"argument 1", + L"argument 2", + }; + AppendCommandLineArgumentTest(arraysize(kArguments), kArguments); + } + + { + SCOPED_TRACE("path with spaces"); + + const wchar_t* kArguments[] = { + L"child.exe", + L"argument1", + L"argument 2", + L"\\some\\path with\\spaces", + }; + AppendCommandLineArgumentTest(arraysize(kArguments), kArguments); + } + + { + SCOPED_TRACE("argument with embedded quotation marks"); + + const wchar_t* kArguments[] = { + L"child.exe", + L"argument1", + L"she said, \"you had me at hello\"", + L"\\some\\path with\\spaces", + }; + AppendCommandLineArgumentTest(arraysize(kArguments), kArguments); + } + + { + SCOPED_TRACE("argument with unbalanced quotation marks"); + + const wchar_t* kArguments[] = { + L"child.exe", + L"argument1", + L"argument\"2", + L"argument3", + L"argument4", + }; + AppendCommandLineArgumentTest(arraysize(kArguments), kArguments); + } + + { + SCOPED_TRACE("argument ending with backslash"); + + const wchar_t* kArguments[] = { + L"child.exe", + L"\\some\\directory with\\spaces\\", + L"argument2", + }; + AppendCommandLineArgumentTest(arraysize(kArguments), kArguments); + } + + { + SCOPED_TRACE("empty argument"); + + const wchar_t* kArguments[] = { + L"child.exe", + L"", + L"argument2", + }; + AppendCommandLineArgumentTest(arraysize(kArguments), kArguments); + } + + { + SCOPED_TRACE("funny nonprintable characters"); + + const wchar_t* kArguments[] = { + L"child.exe", + L"argument 1", + L"argument\t2", + L"argument\n3", + L"argument\v4", + L"argument\"5", + L" ", + L"\t", + L"\n", + L"\v", + L"\"", + L" x", + L"\tx", + L"\nx", + L"\vx", + L"\"x", + L"x ", + L"x\t", + L"x\n", + L"x\v", + L"x\"", + L" ", + L"\t\t", + L"\n\n", + L"\v\v", + L"\"\"", + L" \t\n\v\"", + }; + AppendCommandLineArgumentTest(arraysize(kArguments), kArguments); + } +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc index 234d3149..50daa39a 100644 --- a/util/win/exception_handler_server.cc +++ b/util/win/exception_handler_server.cc @@ -234,11 +234,13 @@ class ClientData { ExceptionHandlerServer::Delegate::~Delegate() { } -ExceptionHandlerServer::ExceptionHandlerServer(const std::string& pipe_name) +ExceptionHandlerServer::ExceptionHandlerServer(const std::string& pipe_name, + bool persistent) : pipe_name_(pipe_name), port_(CreateIoCompletionPort(INVALID_HANDLE_VALUE, nullptr, 0, 1)), clients_lock_(), - clients_() { + clients_(), + persistent_(persistent) { } ExceptionHandlerServer::~ExceptionHandlerServer() { @@ -297,11 +299,11 @@ void ExceptionHandlerServer::Run(Delegate* delegate) { // outstanding threadpool waits are complete. This is important because the // process handle can be signalled *before* the dump request is signalled. internal::ClientData* client = reinterpret_cast(key); - { - base::AutoLock lock(clients_lock_); - clients_.erase(client); - } + base::AutoLock lock(clients_lock_); + clients_.erase(client); delete client; + if (!persistent_ && clients_.empty()) + break; } // Signal to the named pipe instances that they should terminate. diff --git a/util/win/exception_handler_server.h b/util/win/exception_handler_server.h index d1243406..d4a81f16 100644 --- a/util/win/exception_handler_server.h +++ b/util/win/exception_handler_server.h @@ -64,7 +64,10 @@ class ExceptionHandlerServer { //! //! \param[in] pipe_name The name of the pipe to listen on. Must be of the //! form "\\.\pipe\". - explicit ExceptionHandlerServer(const std::string& pipe_name); + //! \param[in] persistent `true` if Run() should not return until Stop() is + //! called. If `false`, Run() will return when all clients have exited, + //! although Run() will always wait for the first client to connect. + ExceptionHandlerServer(const std::string& pipe_name, bool persistent); ~ExceptionHandlerServer(); @@ -92,6 +95,8 @@ class ExceptionHandlerServer { base::Lock clients_lock_; std::set clients_; + bool persistent_; + DISALLOW_COPY_AND_ASSIGN(ExceptionHandlerServer); }; diff --git a/util/win/exception_handler_server_test.cc b/util/win/exception_handler_server_test.cc index 9fe593b5..8cfbb45f 100644 --- a/util/win/exception_handler_server_test.cc +++ b/util/win/exception_handler_server_test.cc @@ -21,6 +21,7 @@ #include "base/basictypes.h" #include "base/strings/stringprintf.h" +#include "base/strings/utf_string_conversions.h" #include "client/crashpad_client.h" #include "gtest/gtest.h" #include "test/win/win_child_process.h" @@ -83,7 +84,7 @@ class ExceptionHandlerServerTest : public testing::Test { base::StringPrintf("%08x", GetCurrentProcessId())), server_ready_(CreateEvent(nullptr, false, false, nullptr)), delegate_(server_ready_.get()), - server_(pipe_name_), + server_(pipe_name_, true), server_thread_(&server_, &delegate_) {} TestDelegate& delegate() { return delegate_; } @@ -134,7 +135,7 @@ TEST_F(ExceptionHandlerServerTest, StopWhileConnected) { &server(), &server_thread()); ASSERT_NO_FATAL_FAILURE(delegate().WaitForStart()); CrashpadClient client; - client.SetHandler(pipe_name()); // Connect to server. + client.SetHandlerIPCPipe(base::UTF8ToUTF16(pipe_name())); // Leaving this scope causes the server to be stopped, while the connection // is still open. } @@ -161,9 +162,9 @@ class TestClient final : public WinChildProcess { private: int Run() override { - std::string pipe_name = ReadString(ReadPipeHandle()); + std::wstring pipe_name = base::UTF8ToUTF16(ReadString(ReadPipeHandle())); CrashpadClient client; - if (!client.SetHandler(pipe_name)) { + if (!client.SetHandlerIPCPipe(pipe_name)) { ADD_FAILURE(); return EXIT_FAILURE; } diff --git a/util/win/process_info.cc b/util/win/process_info.cc index cdc27b1e..d5316590 100644 --- a/util/win/process_info.cc +++ b/util/win/process_info.cc @@ -514,14 +514,14 @@ bool ProcessInfo::Initialize(HANDLE process) { system_info.wProcessorArchitecture == PROCESSOR_ARCHITECTURE_AMD64; } -#if ARCH_CPU_32_BITS +#if defined(ARCH_CPU_32_BITS) if (is_64_bit_) { LOG(ERROR) << "Reading x64 process from x86 process not supported"; return false; } -#endif +#endif // ARCH_CPU_32_BITS -#if ARCH_CPU_64_BITS +#if defined(ARCH_CPU_64_BITS) bool result = GetProcessBasicInformation( process, is_wow64_, this, &peb_address_, &peb_size_); #else diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc index c997d499..0fc52a58 100644 --- a/util/win/process_info_test.cc +++ b/util/win/process_info_test.cc @@ -31,6 +31,7 @@ #include "test/win/child_launcher.h" #include "util/file/file_io.h" #include "util/misc/uuid.h" +#include "util/win/command_line.h" #include "util/win/get_function.h" #include "util/win/scoped_handle.h"