diff --git a/snapshot/mac/mach_o_image_annotations_reader_test.cc b/snapshot/mac/mach_o_image_annotations_reader_test.cc index 01ecc7a0..1978d386 100644 --- a/snapshot/mac/mach_o_image_annotations_reader_test.cc +++ b/snapshot/mac/mach_o_image_annotations_reader_test.cc @@ -193,7 +193,7 @@ class TestMachOImageAnnotationsReader final // Wait for the child process to indicate that it’s done setting up its // annotations via the CrashpadInfo interface. char c; - CheckedReadFile(ReadPipeFD(), &c, sizeof(c)); + CheckedReadFile(ReadPipeHandle(), &c, sizeof(c)); // Verify the “simple map” annotations set via the CrashpadInfo interface. const std::vector& modules = @@ -216,7 +216,7 @@ class TestMachOImageAnnotationsReader final EXPECT_EQ("", all_annotations_simple_map["#TEST# empty_value"]); // Tell the child process that it’s permitted to crash. - CheckedWriteFile(WritePipeFD(), &c, sizeof(c)); + CheckedWriteFile(WritePipeHandle(), &c, sizeof(c)); if (test_type_ != kDontCrash) { // Handle the child’s crash. Further validation will be done in @@ -268,10 +268,10 @@ class TestMachOImageAnnotationsReader final // Tell the parent that the environment has been set up. char c = '\0'; - CheckedWriteFile(WritePipeFD(), &c, sizeof(c)); + CheckedWriteFile(WritePipeHandle(), &c, sizeof(c)); // Wait for the parent to indicate that it’s safe to crash. - CheckedReadFile(ReadPipeFD(), &c, sizeof(c)); + CheckedReadFile(ReadPipeHandle(), &c, sizeof(c)); // Direct an exception message to the exception server running in the // parent. diff --git a/snapshot/mac/process_reader_test.cc b/snapshot/mac/process_reader_test.cc index 51034882..f5b8f9c0 100644 --- a/snapshot/mac/process_reader_test.cc +++ b/snapshot/mac/process_reader_test.cc @@ -88,10 +88,10 @@ class ProcessReaderChild final : public MachMultiprocess { EXPECT_EQ(getpid(), process_reader.ParentProcessID()); EXPECT_EQ(ChildPID(), process_reader.ProcessID()); - int read_fd = ReadPipeFD(); + FileHandle read_handle = ReadPipeHandle(); mach_vm_address_t address; - CheckedReadFile(read_fd, &address, sizeof(address)); + CheckedReadFile(read_handle, &address, sizeof(address)); std::string read_string; ASSERT_TRUE(process_reader.Memory()->ReadCString(address, &read_string)); @@ -99,15 +99,15 @@ class ProcessReaderChild final : public MachMultiprocess { } void MachMultiprocessChild() override { - int write_fd = WritePipeFD(); + FileHandle write_handle = WritePipeHandle(); mach_vm_address_t address = reinterpret_cast(kTestMemory); - CheckedWriteFile(write_fd, &address, sizeof(address)); + CheckedWriteFile(write_handle, &address, sizeof(address)); // Wait for the parent to signal that it’s OK to exit by closing its end of // the pipe. - CheckedReadFileAtEOF(ReadPipeFD()); + CheckedReadFileAtEOF(ReadPipeHandle()); } DISALLOW_COPY_AND_ASSIGN(ProcessReaderChild); @@ -424,7 +424,7 @@ class ProcessReaderThreadedChild final : public MachMultiprocess { ProcessReader process_reader; ASSERT_TRUE(process_reader.Initialize(ChildTask())); - int read_fd = ReadPipeFD(); + FileHandle read_handle = ReadPipeHandle(); // Build a map of all expected threads, keyed by each thread’s ID, and with // addresses that should lie somewhere within each thread’s stack as values. @@ -434,13 +434,13 @@ class ProcessReaderThreadedChild final : public MachMultiprocess { thread_index < thread_count_ + 1; ++thread_index) { uint64_t thread_id; - CheckedReadFile(read_fd, &thread_id, sizeof(thread_id)); + CheckedReadFile(read_handle, &thread_id, sizeof(thread_id)); TestThreadPool::ThreadExpectation expectation; - CheckedReadFile(read_fd, + CheckedReadFile(read_handle, &expectation.stack_address, sizeof(expectation.stack_address)); - CheckedReadFile(read_fd, + CheckedReadFile(read_handle, &expectation.suspend_count, sizeof(expectation.suspend_count)); @@ -461,22 +461,22 @@ class ProcessReaderThreadedChild final : public MachMultiprocess { TestThreadPool thread_pool; ASSERT_NO_FATAL_FAILURE(thread_pool.StartThreads(thread_count_)); - int write_fd = WritePipeFD(); + FileHandle write_handle = WritePipeHandle(); // This thread isn’t part of the thread pool, but the parent will be able // to inspect it. Write an entry for it. uint64_t thread_id = PthreadToThreadID(pthread_self()); - CheckedWriteFile(write_fd, &thread_id, sizeof(thread_id)); + CheckedWriteFile(write_handle, &thread_id, sizeof(thread_id)); TestThreadPool::ThreadExpectation expectation; expectation.stack_address = reinterpret_cast(&thread_id); expectation.suspend_count = 0; - CheckedWriteFile(write_fd, + CheckedWriteFile(write_handle, &expectation.stack_address, sizeof(expectation.stack_address)); - CheckedWriteFile(write_fd, + CheckedWriteFile(write_handle, &expectation.suspend_count, sizeof(expectation.suspend_count)); @@ -487,18 +487,18 @@ class ProcessReaderThreadedChild final : public MachMultiprocess { uint64_t thread_id = thread_pool.GetThreadInfo(thread_index, &expectation); - CheckedWriteFile(write_fd, &thread_id, sizeof(thread_id)); - CheckedWriteFile(write_fd, + CheckedWriteFile(write_handle, &thread_id, sizeof(thread_id)); + CheckedWriteFile(write_handle, &expectation.stack_address, sizeof(expectation.stack_address)); - CheckedWriteFile(write_fd, + CheckedWriteFile(write_handle, &expectation.suspend_count, sizeof(expectation.suspend_count)); } // Wait for the parent to signal that it’s OK to exit by closing its end of // the pipe. - CheckedReadFileAtEOF(ReadPipeFD()); + CheckedReadFileAtEOF(ReadPipeHandle()); } size_t thread_count_; @@ -594,10 +594,10 @@ class ProcessReaderModulesChild final : public MachMultiprocess { // and for dyld. ASSERT_GE(modules.size(), 3u); - int read_fd = ReadPipeFD(); + FileHandle read_handle = ReadPipeHandle(); uint32_t expect_modules; - CheckedReadFile(read_fd, &expect_modules, sizeof(expect_modules)); + CheckedReadFile(read_handle, &expect_modules, sizeof(expect_modules)); ASSERT_EQ(expect_modules, modules.size()); @@ -607,15 +607,15 @@ class ProcessReaderModulesChild final : public MachMultiprocess { uint32_t expect_name_length; CheckedReadFile( - read_fd, &expect_name_length, sizeof(expect_name_length)); + read_handle, &expect_name_length, sizeof(expect_name_length)); // The NUL terminator is not read. std::string expect_name(expect_name_length, '\0'); - CheckedReadFile(read_fd, &expect_name[0], expect_name_length); + CheckedReadFile(read_handle, &expect_name[0], expect_name_length); EXPECT_EQ(expect_name, modules[index].name); mach_vm_address_t expect_address; - CheckedReadFile(read_fd, &expect_address, sizeof(expect_address)); + CheckedReadFile(read_handle, &expect_address, sizeof(expect_address)); EXPECT_EQ(expect_address, modules[index].reader->Address()); if (index == 0 || index == modules.size() - 1) { @@ -635,7 +635,7 @@ class ProcessReaderModulesChild final : public MachMultiprocess { } void MachMultiprocessChild() override { - int write_fd = WritePipeFD(); + FileHandle write_handle = WritePipeHandle(); uint32_t dyld_image_count = _dyld_image_count(); const struct dyld_all_image_infos* dyld_image_infos = @@ -648,7 +648,8 @@ class ProcessReaderModulesChild final : public MachMultiprocess { ++write_image_count; } - CheckedWriteFile(write_fd, &write_image_count, sizeof(write_image_count)); + CheckedWriteFile( + write_handle, &write_image_count, sizeof(write_image_count)); for (size_t index = 0; index < write_image_count; ++index) { const char* dyld_image_name; @@ -665,19 +666,20 @@ class ProcessReaderModulesChild final : public MachMultiprocess { } uint32_t dyld_image_name_length = strlen(dyld_image_name); - CheckedWriteFile( - write_fd, &dyld_image_name_length, sizeof(dyld_image_name_length)); + CheckedWriteFile(write_handle, + &dyld_image_name_length, + sizeof(dyld_image_name_length)); // The NUL terminator is not written. - CheckedWriteFile(write_fd, dyld_image_name, dyld_image_name_length); + CheckedWriteFile(write_handle, dyld_image_name, dyld_image_name_length); CheckedWriteFile( - write_fd, &dyld_image_address, sizeof(dyld_image_address)); + write_handle, &dyld_image_address, sizeof(dyld_image_address)); } // Wait for the parent to signal that it’s OK to exit by closing its end of // the pipe. - CheckedReadFileAtEOF(ReadPipeFD()); + CheckedReadFileAtEOF(ReadPipeHandle()); } DISALLOW_COPY_AND_ASSIGN(ProcessReaderModulesChild); diff --git a/util/file/file_io_win.cc b/util/file/file_io_win.cc index 861f15ac..adbe7a99 100644 --- a/util/file/file_io_win.cc +++ b/util/file/file_io_win.cc @@ -44,10 +44,16 @@ ssize_t ReadFile(FileHandle file, void* buffer, size_t size) { while (size_dword > 0) { DWORD bytes_read; BOOL success = ::ReadFile(file, buffer_c, size_dword, &bytes_read, nullptr); - if (!success && GetLastError() != ERROR_MORE_DATA) { - return -1; - } else if (success && bytes_read == 0 && - GetFileType(file) != FILE_TYPE_PIPE) { + if (!success) { + if (GetLastError() == ERROR_BROKEN_PIPE) { + // When reading a pipe and the write handle has been closed, ReadFile + // fails with ERROR_BROKEN_PIPE, but only once all pending data has been + // read. + break; + } else if (GetLastError() != ERROR_MORE_DATA) { + return -1; + } + } else if (bytes_read == 0 && GetFileType(file) != FILE_TYPE_PIPE) { // Zero bytes read for a file indicates reaching EOF. Zero bytes read from // a pipe indicates only that there was a zero byte WriteFile issued on // the other end, so continue reading. diff --git a/util/mach/exception_ports_test.cc b/util/mach/exception_ports_test.cc index abba9bfe..43fb0257 100644 --- a/util/mach/exception_ports_test.cc +++ b/util/mach/exception_ports_test.cc @@ -242,10 +242,10 @@ class TestExceptionPorts : public MachMultiprocess, // Tell the parent process that everything is set up. char c = '\0'; - CheckedWriteFile(test_exception_ports_->WritePipeFD(), &c, 1); + CheckedWriteFile(test_exception_ports_->WritePipeHandle(), &c, 1); // Wait for the parent process to say that its end is set up. - CheckedReadFile(test_exception_ports_->ReadPipeFD(), &c, 1); + CheckedReadFile(test_exception_ports_->ReadPipeHandle(), &c, 1); EXPECT_EQ('\0', c); // Regardless of where ExceptionPorts::SetExceptionPort() ran, @@ -359,7 +359,7 @@ class TestExceptionPorts : public MachMultiprocess, // Wait for the child process to be ready. It needs to have all of its // threads set up before proceeding if in kSetOutOfProcess mode. char c; - CheckedReadFile(ReadPipeFD(), &c, 1); + CheckedReadFile(ReadPipeHandle(), &c, 1); EXPECT_EQ('\0', c); mach_port_t local_port = LocalPort(); @@ -442,7 +442,7 @@ class TestExceptionPorts : public MachMultiprocess, // Let the child process know that everything in the parent process is set // up. c = '\0'; - CheckedWriteFile(WritePipeFD(), &c, 1); + CheckedWriteFile(WritePipeHandle(), &c, 1); if (who_crashes_ != kNobodyCrashes) { UniversalMachExcServer universal_mach_exc_server(this); @@ -463,7 +463,7 @@ class TestExceptionPorts : public MachMultiprocess, // Wait for the child process to exit or terminate, as indicated by it // closing its pipe. This keeps LocalPort() alive in the child as // RemotePort(), for the child’s use in its TestGetExceptionPorts(). - CheckedReadFileAtEOF(ReadPipeFD()); + CheckedReadFileAtEOF(ReadPipeHandle()); } void MachMultiprocessChild() override { diff --git a/util/mach/mach_message_server_test.cc b/util/mach/mach_message_server_test.cc index 0ad7efc8..a3a2a364 100644 --- a/util/mach/mach_message_server_test.cc +++ b/util/mach/mach_message_server_test.cc @@ -335,13 +335,13 @@ class TestMachMessageServer : public MachMessageServer::Interface, if (options_.child_wait_for_parent_pipe_early) { // Tell the child to begin sending messages. char c = '\0'; - CheckedWriteFile(WritePipeFD(), &c, 1); + CheckedWriteFile(WritePipeHandle(), &c, 1); } if (options_.parent_wait_for_child_pipe) { // Wait until the child is done sending what it’s going to send. char c; - CheckedReadFile(ReadPipeFD(), &c, 1); + CheckedReadFile(ReadPipeHandle(), &c, 1); EXPECT_EQ('\0', c); } @@ -386,7 +386,7 @@ class TestMachMessageServer : public MachMessageServer::Interface, if (options_.child_wait_for_parent_pipe_late) { // Let the child know it’s safe to exit. char c = '\0'; - CheckedWriteFile(WritePipeFD(), &c, 1); + CheckedWriteFile(WritePipeHandle(), &c, 1); } } @@ -394,7 +394,7 @@ class TestMachMessageServer : public MachMessageServer::Interface, if (options_.child_wait_for_parent_pipe_early) { // Wait until the parent is done setting things up on its end. char c; - CheckedReadFile(ReadPipeFD(), &c, 1); + CheckedReadFile(ReadPipeHandle(), &c, 1); EXPECT_EQ('\0', c); } @@ -428,7 +428,7 @@ class TestMachMessageServer : public MachMessageServer::Interface, if (options_.child_wait_for_parent_pipe_late) { char c; - CheckedReadFile(ReadPipeFD(), &c, 1); + CheckedReadFile(ReadPipeHandle(), &c, 1); ASSERT_EQ('\0', c); } } @@ -550,7 +550,7 @@ class TestMachMessageServer : public MachMessageServer::Interface, // running MachMessageServer() once it’s received. void ChildNotifyParentViaPipe() { char c = '\0'; - CheckedWriteFile(WritePipeFD(), &c, 1); + CheckedWriteFile(WritePipeHandle(), &c, 1); } // In the child process, sends a request message to the server and then diff --git a/util/net/http_transport_test.cc b/util/net/http_transport_test.cc index 0a91fa56..8fb8a57d 100644 --- a/util/net/http_transport_test.cc +++ b/util/net/http_transport_test.cc @@ -69,18 +69,18 @@ class HTTPTransportTestFixture : public MultiprocessExec { private: void MultiprocessParent() override { - // Use Logging*FD() instead of Checked*FD() so that the test can fail + // Use Logging*File() instead of Checked*File() so that the test can fail // gracefully with a gtest assertion if the child does not execute properly. // The child will write the HTTP server port number as a packed unsigned // short to stdout. uint16_t port; - ASSERT_TRUE(LoggingReadFile(ReadPipeFD(), &port, sizeof(port))); + ASSERT_TRUE(LoggingReadFile(ReadPipeHandle(), &port, sizeof(port))); // Then the parent will tell the web server what response code to send // for the HTTP request. ASSERT_TRUE(LoggingWriteFile( - WritePipeFD(), &response_code_, sizeof(response_code_))); + WritePipeHandle(), &response_code_, sizeof(response_code_))); // Now execute the HTTP request. scoped_ptr transport(HTTPTransport::Create()); @@ -97,7 +97,7 @@ class HTTPTransportTestFixture : public MultiprocessExec { std::string request; char buf[32]; ssize_t bytes_read; - while ((bytes_read = ReadFile(ReadPipeFD(), buf, sizeof(buf))) != 0) { + while ((bytes_read = ReadFile(ReadPipeHandle(), buf, sizeof(buf))) != 0) { ASSERT_GE(bytes_read, 0); request.append(buf, bytes_read); } diff --git a/util/test/mac/mach_multiprocess.cc b/util/test/mac/mach_multiprocess.cc index ca4e6350..9660ee0e 100644 --- a/util/test/mac/mach_multiprocess.cc +++ b/util/test/mac/mach_multiprocess.cc @@ -262,7 +262,7 @@ void MachMultiprocess::MultiprocessChild() { // Wait for the parent process to close its end of the pipe. The child process // needs to remain alive until then because the parent process will attempt to // verify it using the task port it has access to via ChildTask(). - CheckedReadFileAtEOF(ReadPipeFD()); + CheckedReadFileAtEOF(ReadPipeHandle()); if (testing::Test::HasFailure()) { // Trigger the ScopedForbidReturn destructor. diff --git a/util/test/multiprocess.h b/util/test/multiprocess.h index c3a7949e..be706947 100644 --- a/util/test/multiprocess.h +++ b/util/test/multiprocess.h @@ -15,9 +15,11 @@ #ifndef CRASHPAD_UTIL_TEST_MULTIPROCESS_H_ #define CRASHPAD_UTIL_TEST_MULTIPROCESS_H_ -#include +#include #include "base/basictypes.h" +#include "build/build_config.h" +#include "util/file/file_io.h" namespace crashpad { namespace test { @@ -33,6 +35,9 @@ struct MultiprocessInfo; //! //! Subclasses are expected to implement the parent and child by overriding the //! appropriate methods. +//! +//! On Windows, this class is only an internal implementation detail of +//! MultiprocessExec and all tests must use that class. class Multiprocess { public: //! \brief The termination type for a child process. @@ -103,47 +108,52 @@ class Multiprocess { //! gtest assertions. virtual void PreFork(); +#if !defined(OS_WIN) //! \brief Returns the child process’ process ID. //! //! This method may only be called by the parent process. pid_t ChildPID() const; +#endif // !OS_WIN - //! \brief Returns the read pipe’s file descriptor. + //! \brief Returns the read pipe’s file handle. //! //! This method may be called by either the parent or the child process. //! Anything written to the write pipe in the partner process will appear - //! on the this file descriptor in this process. + //! on this file handle in this process. //! //! It is an error to call this after CloseReadPipe() has been called. //! - //! \return The read pipe’s file descriptor. - int ReadPipeFD() const; + //! \return The read pipe’s file handle. + FileHandle ReadPipeHandle() const; - //! \brief Returns the write pipe’s file descriptor. + //! \brief Returns the write pipe’s file handle. //! //! This method may be called by either the parent or the child process. - //! Anything written to this file descriptor in this process will appear on + //! Anything written to this file handle in this process will appear on //! the read pipe in the partner process. //! //! It is an error to call this after CloseWritePipe() has been called. //! - //! \return The write pipe’s file descriptor. - int WritePipeFD() const; + //! \return The write pipe’s file handle. + FileHandle WritePipeHandle() const; //! \brief Closes the read pipe. //! //! This method may be called by either the parent or the child process. An //! attempt to write to the write pipe in the partner process will fail with - //! `EPIPE` or `SIGPIPE`. ReadPipeFD() must not be called after this. + //! `EPIPE` or `SIGPIPE`. ReadPipeHandle() must not be called after this. void CloseReadPipe(); //! \brief Closes the write pipe. //! //! This method may be called by either the parent or the child process. An //! attempt to read from the read pipe in the partner process will indicate - //! end-of-file. WritePipeFD() must not be called after this. + //! end-of-file. WritePipeHandle() must not be called after this. void CloseWritePipe(); + void set_info(internal::MultiprocessInfo* info) { info_ = info; } + internal::MultiprocessInfo* info() const { return info_; } + private: //! \brief Runs the parent side of the test. //! diff --git a/util/test/multiprocess_exec.h b/util/test/multiprocess_exec.h index 5b5f7a29..4f60f814 100644 --- a/util/test/multiprocess_exec.h +++ b/util/test/multiprocess_exec.h @@ -19,6 +19,7 @@ #include #include "base/basictypes.h" +#include "build/build_config.h" #include "util/test/multiprocess.h" namespace crashpad { @@ -62,7 +63,11 @@ class MultiprocessExec : public Multiprocess { std::string command_; std::vector arguments_; +#if defined(OS_POSIX) std::vector argv_; +#elif defined(OS_WIN) + std::wstring command_line_; +#endif // OS_POSIX DISALLOW_COPY_AND_ASSIGN(MultiprocessExec); }; diff --git a/util/test/multiprocess_exec.cc b/util/test/multiprocess_exec_posix.cc similarity index 92% rename from util/test/multiprocess_exec.cc rename to util/test/multiprocess_exec_posix.cc index 2e725dc9..48a91ffb 100644 --- a/util/test/multiprocess_exec.cc +++ b/util/test/multiprocess_exec_posix.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include "base/posix/eintr_wrapper.h" #include "gtest/gtest.h" @@ -72,21 +73,21 @@ void MultiprocessExec::MultiprocessChild() { static_assert(STDERR_FILENO == 2, "stderr must be fd 2"); // Move the read pipe to stdin. - int read_fd = ReadPipeFD(); - ASSERT_NE(read_fd, STDIN_FILENO); - ASSERT_NE(read_fd, STDOUT_FILENO); + FileHandle read_handle = ReadPipeHandle(); + ASSERT_NE(read_handle, STDIN_FILENO); + ASSERT_NE(read_handle, STDOUT_FILENO); ASSERT_EQ(STDIN_FILENO, fileno(stdin)); int rv = fpurge(stdin); ASSERT_EQ(0, rv) << ErrnoMessage("fpurge"); - rv = HANDLE_EINTR(dup2(read_fd, STDIN_FILENO)); + rv = HANDLE_EINTR(dup2(read_handle, STDIN_FILENO)); ASSERT_EQ(STDIN_FILENO, rv) << ErrnoMessage("dup2"); // Move the write pipe to stdout. - int write_fd = WritePipeFD(); - ASSERT_NE(write_fd, STDIN_FILENO); - ASSERT_NE(write_fd, STDOUT_FILENO); + FileHandle write_handle = WritePipeHandle(); + ASSERT_NE(write_handle, STDIN_FILENO); + ASSERT_NE(write_handle, STDOUT_FILENO); ASSERT_EQ(STDOUT_FILENO, fileno(stdout)); // Make a copy of the original stdout file descriptor so that in case there’s @@ -104,7 +105,7 @@ void MultiprocessExec::MultiprocessChild() { rv = HANDLE_EINTR(fflush(stdout)); ASSERT_EQ(0, rv) << ErrnoMessage("fflush"); - rv = HANDLE_EINTR(dup2(write_fd, STDOUT_FILENO)); + rv = HANDLE_EINTR(dup2(write_handle, STDOUT_FILENO)); ASSERT_EQ(STDOUT_FILENO, rv) << ErrnoMessage("dup2"); CloseMultipleNowOrOnExec(STDERR_FILENO + 1, dup_orig_stdout_fd); diff --git a/util/test/multiprocess_exec_test.cc b/util/test/multiprocess_exec_test.cc index 8fb1548f..d151d2b1 100644 --- a/util/test/multiprocess_exec_test.cc +++ b/util/test/multiprocess_exec_test.cc @@ -15,6 +15,8 @@ #include "util/test/multiprocess_exec.h" #include "base/basictypes.h" +#include "base/strings/utf_string_conversions.h" +#include "build/build_config.h" #include "gtest/gtest.h" #include "util/file/file_io.h" #include "util/test/executable_path.h" @@ -31,13 +33,13 @@ class TestMultiprocessExec final : public MultiprocessExec { private: void MultiprocessParent() override { - // Use Logging*FD() instead of Checked*FD() so that the test can fail + // Use Logging*File() instead of Checked*File() so that the test can fail // gracefully with a gtest assertion if the child does not execute properly. char c = 'z'; - ASSERT_TRUE(LoggingWriteFile(WritePipeFD(), &c, 1)); + ASSERT_TRUE(LoggingWriteFile(WritePipeHandle(), &c, 1)); - ASSERT_TRUE(LoggingReadFile(ReadPipeFD(), &c, 1)); + ASSERT_TRUE(LoggingReadFile(ReadPipeHandle(), &c, 1)); EXPECT_EQ('Z', c); } @@ -47,8 +49,16 @@ class TestMultiprocessExec final : public MultiprocessExec { TEST(MultiprocessExec, MultiprocessExec) { TestMultiprocessExec multiprocess_exec; base::FilePath test_executable = ExecutablePath(); +#if defined(OS_POSIX) + std::string child_test_executable = test_executable.value(); +#elif defined(OS_WIN) std::string child_test_executable = - test_executable.value() + "_multiprocess_exec_test_child"; + base::UTF16ToUTF8(test_executable.RemoveFinalExtension().value()); +#endif // OS_POSIX + child_test_executable += "_multiprocess_exec_test_child"; +#if defined(OS_WIN) + child_test_executable += ".exe"; +#endif multiprocess_exec.SetChildCommand(child_test_executable, nullptr); multiprocess_exec.Run(); } diff --git a/util/test/multiprocess_exec_test_child.cc b/util/test/multiprocess_exec_test_child.cc index 8ad358a5..c796e4a3 100644 --- a/util/test/multiprocess_exec_test_child.cc +++ b/util/test/multiprocess_exec_test_child.cc @@ -32,102 +32,6 @@ #include #endif -#if defined(OS_WIN) - -namespace { - -// Various semi-documented NT internals to retrieve open handles. - -typedef enum _SYSTEM_INFORMATION_CLASS { - SystemHandleInformation = 16 -} SYSTEM_INFORMATION_CLASS; - -typedef struct _SYSTEM_HANDLE_INFORMATION { - USHORT ProcessId; - USHORT CreatorBackTraceIndex; - UCHAR ObjectTypeNumber; - UCHAR Flags; - USHORT Handle; - PVOID Object; - ACCESS_MASK GrantedAccess; -} SYSTEM_HANDLE_INFORMATION, *PSYSTEM_HANDLE_INFORMATION; - -typedef struct _SYSTEM_HANDLE_INFORMATION_EX { - ULONG NumberOfHandles; - SYSTEM_HANDLE_INFORMATION Information[1]; -} SYSTEM_HANDLE_INFORMATION_EX, *PSYSTEM_HANDLE_INFORMATION_EX; - -typedef NTSTATUS(WINAPI* NTQUERYSYSTEMINFORMATION)( - SYSTEM_INFORMATION_CLASS SystemInformationClass, - PVOID SystemInformation, - ULONG SystemInformationLength, - PULONG ReturnLength); - -void EnsureOnlyStdioHandlesOpen() { - // Initialize the NTAPI functions we need. - HMODULE ntdll_handle = GetModuleHandle(L"ntdll.dll"); - if (!ntdll_handle) { - fprintf(stderr, "GetModuleHandle ntdll.dll failed.\n"); - abort(); - } - - NTQUERYSYSTEMINFORMATION NtQuerySystemInformation; - NtQuerySystemInformation = reinterpret_cast( - GetProcAddress(ntdll_handle, "NtQuerySystemInformation")); - if (!NtQuerySystemInformation) { - fprintf(stderr, "GetProcAddress NtQuerySystemInformation failed.\n"); - abort(); - } - - // Get the number of handles on the system. - DWORD buffer_size = 0; - SYSTEM_HANDLE_INFORMATION_EX temp_info; - NTSTATUS status = NtQuerySystemInformation( - SystemHandleInformation, &temp_info, sizeof(temp_info), &buffer_size); - if (!buffer_size) { - fprintf(stderr, - "NtQuerySystemInformation for number of handles failed: 0x%lX\n", - status); - abort(); - } - - SYSTEM_HANDLE_INFORMATION_EX *system_handles = - reinterpret_cast(new BYTE[buffer_size]); - - // This is likely flaky as we're racing with other handles being created on - // the system between the size query above, and the actual retrieval here. - status = NtQuerySystemInformation(SystemHandleInformation, system_handles, - buffer_size, &buffer_size); - if (status != 0) { - fprintf(stderr, "Failed to get the handle list: 0x%lX\n", status); - delete[] system_handles; - abort(); - } - - for (ULONG i = 0; i < system_handles->NumberOfHandles; ++i) { - USHORT h = system_handles->Information[i].Handle; - if (system_handles->Information[i].ProcessId != GetCurrentProcessId()) - continue; - - // TODO(scottmg): This is probably insufficient, we'll need to allow having - // a few other standard handles open (for example, to the window station), - // or only check for handles of certain types. - HANDLE handle = reinterpret_cast(h); - if (handle != GetStdHandle(STD_INPUT_HANDLE) && - handle != GetStdHandle(STD_OUTPUT_HANDLE) && - handle != GetStdHandle(STD_ERROR_HANDLE)) { - fprintf(stderr, "Handle 0x%lX is not stdio handle\n", handle); - abort(); - } - } - - delete [] system_handles; -} - -} // namespace - -#endif // OS_WIN - int main(int argc, char* argv[]) { #if defined(OS_POSIX) // Make sure that there’s nothing open at any FD higher than 3. All FDs other @@ -155,13 +59,13 @@ int main(int argc, char* argv[]) { abort(); } #elif defined(OS_WIN) - // Make sure there's nothing open other than stdin, stdout, and stderr. - EnsureOnlyStdioHandlesOpen(); + // TODO(scottmg): Verify that only the handles we expect to be open, are. // Read a byte from stdin, expecting it to be a specific value. char c; DWORD bytes_read; - if (!ReadFile(GetStdHandle(STD_INPUT_HANDLE), &c, 1, &bytes_read, nullptr) || + HANDLE stdin_handle = GetStdHandle(STD_INPUT_HANDLE); + if (!ReadFile(stdin_handle, &c, 1, &bytes_read, nullptr) || bytes_read != 1 || c != 'z') { abort(); } diff --git a/util/test/multiprocess_exec_win.cc b/util/test/multiprocess_exec_win.cc new file mode 100644 index 00000000..afd79b60 --- /dev/null +++ b/util/test/multiprocess_exec_win.cc @@ -0,0 +1,206 @@ +// 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/test/multiprocess_exec.h" + +#include "base/logging.h" +#include "base/strings/utf_string_conversions.h" +#include "gtest/gtest.h" + +namespace crashpad { +namespace test { + +namespace { + +// 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 + +namespace internal { + +struct MultiprocessInfo { + MultiprocessInfo() {} + ScopedFileHANDLE pipe_c2p_read; + ScopedFileHANDLE pipe_c2p_write; + ScopedFileHANDLE pipe_p2c_read; + ScopedFileHANDLE pipe_p2c_write; + PROCESS_INFORMATION process_info; +}; + +} // namespace internal + +Multiprocess::Multiprocess() + : info_(nullptr), + code_(EXIT_SUCCESS), + reason_(kTerminationNormal) { +} + +void Multiprocess::Run() { + // Set up and spawn the child process. + ASSERT_NO_FATAL_FAILURE(PreFork()); + RunChild(); + + // And then run the parent actions in this process. + RunParent(); + + // Reap the child. + WaitForSingleObject(info_->process_info.hProcess, INFINITE); + CloseHandle(info_->process_info.hThread); + CloseHandle(info_->process_info.hProcess); +} + +Multiprocess::~Multiprocess() { + delete info_; +} + +void Multiprocess::PreFork() { + NOTREACHED(); +} + +FileHandle Multiprocess::ReadPipeHandle() const { + // This is the parent case, it's stdin in the child. + return info_->pipe_c2p_read.get(); +} + +FileHandle Multiprocess::WritePipeHandle() const { + // This is the parent case, it's stdout in the child. + return info_->pipe_p2c_write.get(); +} + +void Multiprocess::CloseReadPipe() { + info_->pipe_c2p_read.reset(); +} + +void Multiprocess::CloseWritePipe() { + info_->pipe_p2c_write.reset(); +} + +void Multiprocess::RunParent() { + MultiprocessParent(); + + info_->pipe_c2p_read.reset(); + info_->pipe_p2c_write.reset(); +} + +void Multiprocess::RunChild() { + MultiprocessChild(); + + info_->pipe_c2p_write.reset(); + info_->pipe_p2c_read.reset(); +} + +MultiprocessExec::MultiprocessExec() + : Multiprocess(), command_(), arguments_(), command_line_() { +} + +void MultiprocessExec::SetChildCommand( + const std::string& command, + const std::vector* arguments) { + command_ = command; + if (arguments) { + arguments_ = *arguments; + } else { + arguments_.clear(); + } +} + +MultiprocessExec::~MultiprocessExec() { +} + +void MultiprocessExec::PreFork() { + ASSERT_FALSE(command_.empty()); + + 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_); + } + + // Make pipes for child-to-parent and parent-to-child communication. Mark them + // as inheritable via the SECURITY_ATTRIBUTES, but use SetHandleInformation to + // ensure that the parent sides are not inherited. + ASSERT_EQ(nullptr, info()); + set_info(new internal::MultiprocessInfo()); + + SECURITY_ATTRIBUTES security_attributes = {0}; + security_attributes.nLength = sizeof(SECURITY_ATTRIBUTES); + security_attributes.bInheritHandle = TRUE; + + HANDLE c2p_read, c2p_write; + PCHECK(CreatePipe(&c2p_read, &c2p_write, &security_attributes, 0)); + PCHECK(SetHandleInformation(c2p_read, HANDLE_FLAG_INHERIT, 0)); + info()->pipe_c2p_read.reset(c2p_read); + info()->pipe_c2p_write.reset(c2p_write); + + HANDLE p2c_read, p2c_write; + PCHECK(CreatePipe(&p2c_read, &p2c_write, &security_attributes, 0)); + PCHECK(SetHandleInformation(p2c_write, HANDLE_FLAG_INHERIT, 0)); + info()->pipe_p2c_read.reset(p2c_read); + info()->pipe_p2c_write.reset(p2c_write); +} + +void MultiprocessExec::MultiprocessChild() { + STARTUPINFO startup_info = {0}; + startup_info.cb = sizeof(startup_info); + startup_info.hStdInput = info()->pipe_p2c_read.get(); + startup_info.hStdOutput = info()->pipe_c2p_write.get(); + startup_info.hStdError = GetStdHandle(STD_ERROR_HANDLE); + startup_info.dwFlags = STARTF_USESTDHANDLES; + PCHECK(CreateProcess(base::UTF8ToUTF16(command_).c_str(), + &command_line_[0], // This cannot be constant, per MSDN. + nullptr, + nullptr, + TRUE, + 0, + nullptr, + nullptr, + &startup_info, + &info()->process_info)); +} + +} // namespace test +} // namespace crashpad diff --git a/util/test/multiprocess.cc b/util/test/multiprocess_posix.cc similarity index 98% rename from util/test/multiprocess.cc rename to util/test/multiprocess_posix.cc index c6b648b6..f50446b1 100644 --- a/util/test/multiprocess.cc +++ b/util/test/multiprocess_posix.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include @@ -153,14 +154,14 @@ pid_t Multiprocess::ChildPID() const { return info_->child_pid; } -int Multiprocess::ReadPipeFD() const { +FileHandle Multiprocess::ReadPipeHandle() const { int fd = info_->child_pid ? info_->pipe_c2p_read.get() : info_->pipe_p2c_read.get(); CHECK_NE(fd, -1); return fd; } -int Multiprocess::WritePipeFD() const { +FileHandle Multiprocess::WritePipeHandle() const { int fd = info_->child_pid ? info_->pipe_p2c_write.get() : info_->pipe_c2p_write.get(); CHECK_NE(fd, -1); diff --git a/util/test/multiprocess_test.cc b/util/test/multiprocess_posix_test.cc similarity index 87% rename from util/test/multiprocess_test.cc rename to util/test/multiprocess_posix_test.cc index 4d978e04..a8c7d9da 100644 --- a/util/test/multiprocess_test.cc +++ b/util/test/multiprocess_posix_test.cc @@ -36,33 +36,33 @@ class TestMultiprocess final : public Multiprocess { // Multiprocess: void MultiprocessParent() override { - int read_fd = ReadPipeFD(); + FileHandle read_handle = ReadPipeHandle(); char c; - CheckedReadFile(read_fd, &c, 1); + CheckedReadFile(read_handle, &c, 1); EXPECT_EQ('M', c); pid_t pid; - CheckedReadFile(read_fd, &pid, sizeof(pid)); + CheckedReadFile(read_handle, &pid, sizeof(pid)); EXPECT_EQ(pid, ChildPID()); c = 'm'; - CheckedWriteFile(WritePipeFD(), &c, 1); + CheckedWriteFile(WritePipeHandle(), &c, 1); // The child will close its end of the pipe and exit. Make sure that the // parent sees EOF. - CheckedReadFileAtEOF(read_fd); + CheckedReadFileAtEOF(read_handle); } void MultiprocessChild() override { - int write_fd = WritePipeFD(); + FileHandle write_handle = WritePipeHandle(); char c = 'M'; - CheckedWriteFile(write_fd, &c, 1); + CheckedWriteFile(write_handle, &c, 1); pid_t pid = getpid(); - CheckedWriteFile(write_fd, &pid, sizeof(pid)); + CheckedWriteFile(write_handle, &pid, sizeof(pid)); - CheckedReadFile(ReadPipeFD(), &c, 1); + CheckedReadFile(ReadPipeHandle(), &c, 1); EXPECT_EQ('m', c); } @@ -160,8 +160,8 @@ class TestMultiprocessClosePipe final : public Multiprocess { private: void VerifyInitial() { - ASSERT_NE(-1, ReadPipeFD()); - ASSERT_NE(-1, WritePipeFD()); + ASSERT_NE(-1, ReadPipeHandle()); + ASSERT_NE(-1, WritePipeHandle()); } // Verifies that the partner process did what it was supposed to do. This must @@ -178,9 +178,9 @@ class TestMultiprocessClosePipe final : public Multiprocess { // the read pipe in this process to show end-of-file. void VerifyPartner() { if (what_closes_ == kWriteCloses) { - CheckedReadFileAtEOF(ReadPipeFD()); + CheckedReadFileAtEOF(ReadPipeHandle()); } else if (what_closes_ == kReadAndWriteClose) { - CheckedReadFileAtEOF(ReadPipeFD()); + CheckedReadFileAtEOF(ReadPipeHandle()); char c = '\0'; // This will raise SIGPIPE. If fatal (the normal case), that will cause @@ -189,7 +189,7 @@ class TestMultiprocessClosePipe final : public Multiprocess { // abort execution. Regardless of how SIGPIPE is handled, the process will // be terminated. Because the actual termination mechanism is not known, // no regex can be specified. - EXPECT_DEATH(CheckedWriteFile(WritePipeFD(), &c, 1), ""); + EXPECT_DEATH(CheckedWriteFile(WritePipeHandle(), &c, 1), ""); } } @@ -197,19 +197,19 @@ class TestMultiprocessClosePipe final : public Multiprocess { switch (what_closes_) { case kReadCloses: CloseReadPipe(); - EXPECT_NE(-1, WritePipeFD()); - EXPECT_DEATH(ReadPipeFD(), "fd"); + EXPECT_NE(-1, WritePipeHandle()); + EXPECT_DEATH(ReadPipeHandle(), "fd"); break; case kWriteCloses: CloseWritePipe(); - EXPECT_NE(-1, ReadPipeFD()); - EXPECT_DEATH(WritePipeFD(), "fd"); + EXPECT_NE(-1, ReadPipeHandle()); + EXPECT_DEATH(WritePipeHandle(), "fd"); break; case kReadAndWriteClose: CloseReadPipe(); CloseWritePipe(); - EXPECT_DEATH(ReadPipeFD(), "fd"); - EXPECT_DEATH(WritePipeFD(), "fd"); + EXPECT_DEATH(ReadPipeHandle(), "fd"); + EXPECT_DEATH(WritePipeHandle(), "fd"); break; } } diff --git a/util/util.gyp b/util/util.gyp index 15a45055..2d3d848c 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -219,10 +219,11 @@ 'test/mac/mach_errors.h', 'test/mac/mach_multiprocess.cc', 'test/mac/mach_multiprocess.h', - 'test/multiprocess.cc', 'test/multiprocess.h', - 'test/multiprocess_exec.cc', 'test/multiprocess_exec.h', + 'test/multiprocess_exec_posix.cc', + 'test/multiprocess_exec_win.cc', + 'test/multiprocess_posix.cc', 'test/scoped_temp_dir.cc', 'test/scoped_temp_dir.h', 'test/scoped_temp_dir_posix.cc', @@ -297,7 +298,7 @@ 'test/executable_path_test.cc', 'test/mac/mach_multiprocess_test.cc', 'test/multiprocess_exec_test.cc', - 'test/multiprocess_test.cc', + 'test/multiprocess_posix_test.cc', 'test/scoped_temp_dir_test.cc', ], 'conditions': [