diff --git a/client/annotation.h b/client/annotation.h index 34d0d566..35bdced4 100644 --- a/client/annotation.h +++ b/client/annotation.h @@ -213,13 +213,13 @@ class StringAnnotation : public Annotation { //! \brief Sets the Annotation's string value. //! - //! \param[in] value The string value. + //! \param[in] string The string value. void Set(base::StringPiece string) { Annotation::ValueSizeType size = std::min(MaxSize, base::saturated_cast(string.size())); memcpy(value_, string.data(), size); // Check for no embedded `NUL` characters. - DCHECK(!memchr(value_, '\0', size)); + DCHECK(!memchr(value_, '\0', size)) << "embedded NUL"; SetSize(size); } diff --git a/client/annotation_test.cc b/client/annotation_test.cc index 083b309b..656ade0c 100644 --- a/client/annotation_test.cc +++ b/client/annotation_test.cc @@ -19,7 +19,7 @@ #include "client/annotation_list.h" #include "client/crashpad_info.h" #include "gtest/gtest.h" -#include "test/gtest_death_check.h" +#include "test/gtest_death.h" namespace crashpad { namespace test { @@ -105,12 +105,17 @@ TEST_F(Annotation, StringType) { EXPECT_EQ(5u, annotation.size()); EXPECT_EQ("loooo", annotation.value()); +} #if DCHECK_IS_ON() - EXPECT_DEATH_CHECK(annotation.Set(std::string("te\0st", 5)), ""); -#endif + +TEST(AnnotationDeathTest, EmbeddedNUL) { + crashpad::StringAnnotation<5> annotation("name"); + EXPECT_DEATH_CHECK(annotation.Set(std::string("te\0st", 5)), "embedded NUL"); } +#endif + } // namespace } // namespace test } // namespace crashpad diff --git a/client/crashpad_client_mac.cc b/client/crashpad_client_mac.cc index a9e4aca4..a16f8d28 100644 --- a/client/crashpad_client_mac.cc +++ b/client/crashpad_client_mac.cc @@ -18,15 +18,12 @@ #include #include #include -#include -#include #include #include #include "base/logging.h" #include "base/mac/mach_logging.h" -#include "base/posix/eintr_wrapper.h" #include "base/strings/stringprintf.h" #include "util/mac/mac_util.h" #include "util/mach/child_port_handshake.h" @@ -36,7 +33,7 @@ #include "util/mach/notify_server.h" #include "util/misc/clock.h" #include "util/misc/implicit_cast.h" -#include "util/posix/close_multiple.h" +#include "util/posix/double_fork_and_exec.h" namespace crashpad { @@ -305,9 +302,6 @@ class HandlerStarter final : public NotifyServer::DefaultInterface { handler_restarter->last_start_time_ = ClockMonotonicNanoseconds(); } - // Set up the arguments for execve() first. These aren’t needed until - // execve() is called, but it’s dangerous to do this in a child process - // after fork(). ChildPortHandshake child_port_handshake; base::ScopedFD server_write_fd = child_port_handshake.ServerWriteFD(); @@ -337,8 +331,6 @@ class HandlerStarter final : public NotifyServer::DefaultInterface { } argv.push_back(FormatArgumentInt("handshake-fd", server_write_fd.get())); - const char* handler_c = handler.value().c_str(); - // argv_c contains const char* pointers and is terminated by nullptr. argv // is required because the pointers in argv_c need to point somewhere, and // they can’t point to temporaries such as those returned by @@ -350,97 +342,23 @@ class HandlerStarter final : public NotifyServer::DefaultInterface { } argv_c.push_back(nullptr); - // Double-fork(). The three processes involved are parent, child, and - // grandchild. The grandchild will become the handler process. The child - // exits immediately after spawning the grandchild, so the grandchild - // becomes an orphan and its parent process ID becomes 1. This relieves the - // parent and child of the responsibility for reaping the grandchild with - // waitpid() or similar. The handler process is expected to outlive the - // parent process, so the parent shouldn’t be concerned with reaping it. - // This approach means that accidental early termination of the handler - // process will not result in a zombie process. - pid_t pid = fork(); - if (pid < 0) { - PLOG(ERROR) << "fork"; + // When restarting, reset the system default crash handler first. Otherwise, + // the crash exception port in the handler will have been inherited from + // this parent process, which was probably using the exception server now + // being restarted. The handler can’t monitor itself for its own crashes via + // this interface. + if (!DoubleForkAndExec( + argv, + server_write_fd.get(), + true, + restart ? CrashpadClient::UseSystemDefaultHandler : nullptr)) { return false; } - if (pid == 0) { - // Child process. - - if (restart) { - // When restarting, reset the system default crash handler first. - // Otherwise, the crash exception port here will have been inherited - // from the parent process, which was probably using the exception - // server now being restarted. The handler can’t monitor itself for its - // own crashes via this interface. - CrashpadClient::UseSystemDefaultHandler(); - } - - // Call setsid(), creating a new process group and a new session, both led - // by this process. The new process group has no controlling terminal. - // This disconnects it from signals generated by the parent process’ - // terminal. - // - // setsid() is done in the child instead of the grandchild so that the - // grandchild will not be a session leader. If it were a session leader, - // an accidental open() of a terminal device without O_NOCTTY would make - // that terminal the controlling terminal. - // - // It’s not desirable for the handler to have a controlling terminal. The - // handler monitors clients on its own and manages its own lifetime, - // exiting when it loses all clients and when it deems it appropraite to - // do so. It may serve clients in different process groups or sessions - // than its original client, and receiving signals intended for its - // original client’s process group could be harmful in that case. - PCHECK(setsid() != -1) << "setsid"; - - pid = fork(); - if (pid < 0) { - PLOG(FATAL) << "fork"; - } - - if (pid > 0) { - // Child process. - - // _exit() instead of exit(), because fork() was called. - _exit(EXIT_SUCCESS); - } - - // Grandchild process. - - CloseMultipleNowOrOnExec(STDERR_FILENO + 1, server_write_fd.get()); - - // &argv_c[0] is a pointer to a pointer to const char data, but because of - // how C (not C++) works, execvp() wants a pointer to a const pointer to - // char data. It modifies neither the data nor the pointers, so the - // const_cast is safe. - execvp(handler_c, const_cast(&argv_c[0])); - PLOG(FATAL) << "execvp " << handler_c; - } - - // Parent process. - // Close the write side of the pipe, so that the handler process is the only // process that can write to it. server_write_fd.reset(); - // waitpid() for the child, so that it does not become a zombie process. The - // child normally exits quickly. - int status; - pid_t wait_pid = HANDLE_EINTR(waitpid(pid, &status, 0)); - PCHECK(wait_pid != -1) << "waitpid"; - DCHECK_EQ(wait_pid, pid); - - if (WIFSIGNALED(status)) { - LOG(WARNING) << "intermediate process: signal " << WTERMSIG(status); - } else if (!WIFEXITED(status)) { - DLOG(WARNING) << "intermediate process: unknown termination " << status; - } else if (WEXITSTATUS(status) != EXIT_SUCCESS) { - LOG(WARNING) << "intermediate process: exit status " - << WEXITSTATUS(status); - } - // Rendezvous with the handler running in the grandchild process. if (!child_port_handshake.RunClient(receive_right.get(), MACH_MSG_TYPE_MOVE_RECEIVE)) { diff --git a/client/simple_address_range_bag_test.cc b/client/simple_address_range_bag_test.cc index ac81a1f4..e9a6d9f3 100644 --- a/client/simple_address_range_bag_test.cc +++ b/client/simple_address_range_bag_test.cc @@ -16,7 +16,7 @@ #include "base/logging.h" #include "gtest/gtest.h" -#include "test/gtest_death_check.h" +#include "test/gtest_death.h" namespace crashpad { namespace test { diff --git a/client/simple_string_dictionary_test.cc b/client/simple_string_dictionary_test.cc index 6944934f..6f85e854 100644 --- a/client/simple_string_dictionary_test.cc +++ b/client/simple_string_dictionary_test.cc @@ -16,7 +16,7 @@ #include "base/logging.h" #include "gtest/gtest.h" -#include "test/gtest_death_check.h" +#include "test/gtest_death.h" namespace crashpad { namespace test { diff --git a/minidump/minidump_exception_writer_test.cc b/minidump/minidump_exception_writer_test.cc index a748641b..e4dc5faa 100644 --- a/minidump/minidump_exception_writer_test.cc +++ b/minidump/minidump_exception_writer_test.cc @@ -27,7 +27,7 @@ #include "minidump/test/minidump_writable_test_util.h" #include "snapshot/test/test_cpu_context.h" #include "snapshot/test/test_exception_snapshot.h" -#include "test/gtest_death_check.h" +#include "test/gtest_death.h" #include "util/file/string_file.h" namespace crashpad { diff --git a/minidump/minidump_file_writer_test.cc b/minidump/minidump_file_writer_test.cc index 00f7a7bf..730da26b 100644 --- a/minidump/minidump_file_writer_test.cc +++ b/minidump/minidump_file_writer_test.cc @@ -34,7 +34,7 @@ #include "snapshot/test/test_process_snapshot.h" #include "snapshot/test/test_system_snapshot.h" #include "snapshot/test/test_thread_snapshot.h" -#include "test/gtest_death_check.h" +#include "test/gtest_death.h" #include "util/file/string_file.h" namespace crashpad { diff --git a/minidump/minidump_module_writer_test.cc b/minidump/minidump_module_writer_test.cc index f6a278e9..9a2739b5 100644 --- a/minidump/minidump_module_writer_test.cc +++ b/minidump/minidump_module_writer_test.cc @@ -28,7 +28,7 @@ #include "minidump/test/minidump_string_writer_test_util.h" #include "minidump/test/minidump_writable_test_util.h" #include "snapshot/test/test_module_snapshot.h" -#include "test/gtest_death_check.h" +#include "test/gtest_death.h" #include "util/file/string_file.h" #include "util/misc/implicit_cast.h" #include "util/misc/uuid.h" diff --git a/minidump/minidump_system_info_writer_test.cc b/minidump/minidump_system_info_writer_test.cc index 645f39ed..c3d02472 100644 --- a/minidump/minidump_system_info_writer_test.cc +++ b/minidump/minidump_system_info_writer_test.cc @@ -27,7 +27,7 @@ #include "minidump/test/minidump_string_writer_test_util.h" #include "minidump/test/minidump_writable_test_util.h" #include "snapshot/test/test_system_snapshot.h" -#include "test/gtest_death_check.h" +#include "test/gtest_death.h" #include "util/file/string_file.h" namespace crashpad { diff --git a/minidump/minidump_thread_writer_test.cc b/minidump/minidump_thread_writer_test.cc index 60173da2..d9e85aa6 100644 --- a/minidump/minidump_thread_writer_test.cc +++ b/minidump/minidump_thread_writer_test.cc @@ -31,7 +31,7 @@ #include "snapshot/test/test_cpu_context.h" #include "snapshot/test/test_memory_snapshot.h" #include "snapshot/test/test_thread_snapshot.h" -#include "test/gtest_death_check.h" +#include "test/gtest_death.h" #include "util/file/string_file.h" namespace crashpad { diff --git a/snapshot/mac/mach_o_image_annotations_reader_test.cc b/snapshot/mac/mach_o_image_annotations_reader_test.cc index 3e7864b0..f486c1d5 100644 --- a/snapshot/mac/mach_o_image_annotations_reader_test.cc +++ b/snapshot/mac/mach_o_image_annotations_reader_test.cc @@ -101,6 +101,32 @@ class TestMachOImageAnnotationsReader final : MachMultiprocess(), UniversalMachExcServer::Interface(), test_type_(test_type) { + switch (test_type_) { + case kDontCrash: + // SetExpectedChildTermination(kTerminationNormal, EXIT_SUCCESS) is the + // default. + break; + + case kCrashAbort: + SetExpectedChildTermination(kTerminationSignal, SIGABRT); + break; + + case kCrashModuleInitialization: + // This crash is triggered by __builtin_trap(), which shows up as + // SIGILL. + SetExpectedChildTermination(kTerminationSignal, SIGILL); + break; + + case kCrashDyld: + // Prior to 10.12, dyld fatal errors result in the execution of an + // int3 instruction on x86 and a trap instruction on ARM, both of + // which raise SIGTRAP. 10.9.5 dyld-239.4/src/dyldStartup.s + // _dyld_fatal_error. This changed in 10.12 to use + // abort_with_payload(), which appears as SIGABRT to a waiting parent. + SetExpectedChildTermination( + kTerminationSignal, MacOSXMinorVersion() < 12 ? SIGTRAP : SIGABRT); + break; + } } ~TestMachOImageAnnotationsReader() {} @@ -322,33 +348,6 @@ class TestMachOImageAnnotationsReader final kMachMessageTimeoutWaitIndefinitely); EXPECT_EQ(mr, MACH_MSG_SUCCESS) << MachErrorMessage(mr, "MachMessageServer::Run"); - - switch (test_type_) { - case kCrashAbort: - SetExpectedChildTermination(kTerminationSignal, SIGABRT); - break; - - case kCrashModuleInitialization: - // This crash is triggered by __builtin_trap(), which shows up as - // SIGILL. - SetExpectedChildTermination(kTerminationSignal, SIGILL); - break; - - case kCrashDyld: - // Prior to 10.12, dyld fatal errors result in the execution of an - // int3 instruction on x86 and a trap instruction on ARM, both of - // which raise SIGTRAP. 10.9.5 dyld-239.4/src/dyldStartup.s - // _dyld_fatal_error. This changed in 10.12 to use - // abort_with_payload(), which appears as SIGABRT to a waiting parent. - SetExpectedChildTermination( - kTerminationSignal, - MacOSXMinorVersion() < 12 ? SIGTRAP : SIGABRT); - break; - - default: - FAIL(); - break; - } } } diff --git a/test/BUILD.gn b/test/BUILD.gn index 5b9b745f..9337fa99 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -24,13 +24,15 @@ static_library("test") { "file.h", "filesystem.cc", "filesystem.h", - "gtest_death_check.h", + "gtest_death.h", "gtest_disabled.cc", "gtest_disabled.h", "hex_string.cc", "hex_string.h", "mac/dyld.cc", "mac/dyld.h", + "mac/exception_swallower.cc", + "mac/exception_swallower.h", "mac/mach_errors.cc", "mac/mach_errors.h", "mac/mach_multiprocess.cc", @@ -78,6 +80,9 @@ static_library("test") { if (is_mac) { libs = [ "bsm" ] + data_deps = [ + ":crashpad_exception_swallower", + ] } } @@ -153,3 +158,19 @@ static_library("gtest_main") { "//testing/gtest", ] } + +if (is_mac) { + executable("crashpad_exception_swallower") { + testonly = true + sources = [ + "mac/exception_swallower_exe.cc", + ] + deps = [ + "//base", + "//third_party/crashpad/crashpad/compat", + "//third_party/crashpad/crashpad/handler", + "//third_party/crashpad/crashpad/tools:tool_support", + "//third_party/crashpad/crashpad/util", + ] + } +} diff --git a/test/gtest_death.h b/test/gtest_death.h new file mode 100644 index 00000000..1c7e5f6a --- /dev/null +++ b/test/gtest_death.h @@ -0,0 +1,115 @@ +// 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_TEST_GTEST_DEATH_H_ +#define CRASHPAD_TEST_GTEST_DEATH_H_ + +#include "base/logging.h" +#include "build/build_config.h" +#include "gtest/gtest.h" + +#if defined(OS_MACOSX) +#include "test/mac/exception_swallower.h" +#endif + +//! \file + +#if defined(OS_MACOSX) || DOXYGEN + +//! \brief Wraps the gtest `ASSERT_DEATH()` macro to make assertions about death +//! caused by crashes. +//! +//! On macOS, this macro prevents the system’s crash reporter from handling +//! crashes that occur in \a statement. Crashes are normally visible to the +//! system’s crash reporter, but it is undesirable for intentional +//! ASSERT_DEATH_CRASH() crashes to be handled by any crash reporter. +//! +//! \sa ASSERT_DEATH_CHECK() +//! \sa EXPECT_DEATH_CRASH() +#define ASSERT_DEATH_CRASH(statement, regex) \ + crashpad::test::ExceptionSwallower::Parent_PrepareForGtestDeathTest(); \ + ASSERT_DEATH(crashpad::test::ExceptionSwallower::Child_SwallowExceptions(); \ + { statement; }, regex) + +//! \brief Wraps the gtest `EXPECT_DEATH()` macro to make assertions about death +//! caused by crashes. +//! +//! On macOS, this macro prevents the system’s crash reporter from handling +//! crashes that occur in \a statement. Crashes are normally visible to the +//! system’s crash reporter, but it is undesirable for intentional +//! EXPECT_DEATH_CRASH() crashes to be handled by any crash reporter. +//! +//! \sa EXPECT_DEATH_CHECK() +//! \sa ASSERT_DEATH_CRASH() +#define EXPECT_DEATH_CRASH(statement, regex) \ + crashpad::test::ExceptionSwallower::Parent_PrepareForGtestDeathTest(); \ + EXPECT_DEATH(crashpad::test::ExceptionSwallower::Child_SwallowExceptions(); \ + { statement; }, regex) + +#else // OS_MACOSX + +#define ASSERT_DEATH_CRASH(statement, regex) ASSERT_DEATH(statement, regex) +#define EXPECT_DEATH_CRASH(statement, regex) EXPECT_DEATH(statement, regex) + +#endif // OS_MACOSX + +#if !(!defined(MINI_CHROMIUM_BASE_LOGGING_H_) && \ + defined(OFFICIAL_BUILD) && \ + defined(NDEBUG)) || \ + DOXYGEN + +//! \brief Wraps the ASSERT_DEATH_CRASH() macro to make assertions about death +//! caused by `CHECK()` failures. +//! +//! In an in-Chromium build in the official configuration, `CHECK()` does not +//! print its condition or streamed messages. In that case, this macro uses an +//! empty \a regex pattern when calling ASSERT_DEATH_CRASH() to avoid looking +//! for any particular output on the standard error stream. In other build +//! configurations, the \a regex pattern is left intact. +//! +//! `CHECK()` failures normally show up as crashes to the system’s crash +//! reporter, but it is undesirable for intentional ASSERT_DEATH_CHECK() crashes +//! to be handled by any crash reporter, so this is implemented in terms of +//! ASSERT_DEATH_CRASH() instead of `ASSERT_DEATH()`. +//! +//! \sa EXPECT_DEATH_CHECK() +#define ASSERT_DEATH_CHECK(statement, regex) \ + ASSERT_DEATH_CRASH(statement, regex) + +//! \brief Wraps the EXPECT_DEATH_CRASH() macro to make assertions about death +//! caused by `CHECK()` failures. +//! +//! In an in-Chromium build in the official configuration, `CHECK()` does not +//! print its condition or streamed messages. In that case, this macro uses an +//! empty \a regex pattern when calling EXPECT_DEATH_CRASH() to avoid looking +//! for any particular output on the standard error stream. In other build +//! configurations, the \a regex pattern is left intact. +//! +//! `CHECK()` failures normally show up as crashes to the system’s crash +//! reporter, but it is undesirable for intentional EXPECT_DEATH_CHECK() crashes +//! to be handled by any crash reporter, so this is implemented in terms of +//! EXPECT_DEATH_CRASH() instead of `EXPECT_DEATH()`. +//! +//! \sa ASSERT_DEATH_CHECK() +#define EXPECT_DEATH_CHECK(statement, regex) \ + EXPECT_DEATH_CRASH(statement, regex) + +#else // !(!MINI_CHROMIUM_BASE_LOGGING_H_ && OFFICIAL_BUILD && NDEBUG) + +#define ASSERT_DEATH_CHECK(statement, regex) ASSERT_DEATH_CRASH(statement, "") +#define EXPECT_DEATH_CHECK(statement, regex) EXPECT_DEATH_CRASH(statement, "") + +#endif // !(!MINI_CHROMIUM_BASE_LOGGING_H_ && OFFICIAL_BUILD && NDEBUG) + +#endif // CRASHPAD_TEST_GTEST_DEATH_H_ diff --git a/test/gtest_death_check.h b/test/gtest_death_check.h deleted file mode 100644 index 8af50aac..00000000 --- a/test/gtest_death_check.h +++ /dev/null @@ -1,55 +0,0 @@ -// 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_TEST_GTEST_DEATH_CHECK_H_ -#define CRASHPAD_TEST_GTEST_DEATH_CHECK_H_ - -#include "base/logging.h" -#include "gtest/gtest.h" - -//! \file - -#if !(!defined(MINI_CHROMIUM_BASE_LOGGING_H_) && \ - defined(OFFICIAL_BUILD) && \ - defined(NDEBUG)) || \ - DOXYGEN - -//! \brief Wraps the gtest `ASSERT_DEATH()` macro to make assertions about death -//! caused by `CHECK()` failures. -//! -//! In an in-Chromium build in the official configuration in Release mode, -//! `CHECK()` does not print its condition or streamed messages. In that case, -//! this macro uses an empty \a regex pattern when calling `ASSERT_DEATH()` to -//! avoid looking for any particular output on the standard error stream. In -//! other build configurations, the \a regex pattern is left intact. -#define ASSERT_DEATH_CHECK(statement, regex) ASSERT_DEATH(statement, regex) - -//! \brief Wraps the gtest `EXPECT_DEATH()` macro to make assertions about death -//! caused by `CHECK()` failures. -//! -//! In an in-Chromium build in the official configuration in Release mode, -//! `CHECK()` does not print its condition or streamed messages. In that case, -//! this macro uses an empty \a regex pattern when calling `EXPECT_DEATH()` to -//! avoid looking for any particular output on the standard error stream. In -//! other build configurations, the \a regex pattern is left intact. -#define EXPECT_DEATH_CHECK(statement, regex) EXPECT_DEATH(statement, regex) - -#else - -#define ASSERT_DEATH_CHECK(statement, regex) ASSERT_DEATH(statement, "") -#define EXPECT_DEATH_CHECK(statement, regex) EXPECT_DEATH(statement, "") - -#endif - -#endif // CRASHPAD_TEST_GTEST_DEATH_CHECK_H_ diff --git a/test/mac/exception_swallower.cc b/test/mac/exception_swallower.cc new file mode 100644 index 00000000..307680c0 --- /dev/null +++ b/test/mac/exception_swallower.cc @@ -0,0 +1,155 @@ +// Copyright 2017 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 "test/mac/exception_swallower.h" + +#include +#include + +#include +#include + +#include "base/logging.h" +#include "base/mac/scoped_mach_port.h" +#include "base/strings/stringprintf.h" +#include "gtest/gtest.h" +#include "test/test_paths.h" +#include "util/file/file_io.h" +#include "util/mach/exception_ports.h" +#include "util/mach/mach_extensions.h" +#include "util/posix/double_fork_and_exec.h" + +namespace crashpad { +namespace test { + +// static +void ExceptionSwallower::Parent_PrepareForCrashingChild() { + Get()->SetParent(); +} + +// static +void ExceptionSwallower::Parent_PrepareForGtestDeathTest() { + if (testing::FLAGS_gtest_death_test_style == "fast") { + Parent_PrepareForCrashingChild(); + } else { + // This is the only other death test style that’s known to gtest. + DCHECK_EQ(testing::FLAGS_gtest_death_test_style, "threadsafe"); + } +} + +// static +void ExceptionSwallower::Child_SwallowExceptions() { + Get()->SwallowExceptions(); +} + +ExceptionSwallower::ExceptionSwallower() + : service_name_(), fd_(), parent_pid_(0) { + base::FilePath exception_swallower_server_path = + TestPaths::Executable().DirName().Append("crashpad_exception_swallower"); + + // Use socketpair() as a full-duplex pipe(). + int socket_fds[2]; + PCHECK(socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, socket_fds) == 0) + << "socketpair"; + + fd_.reset(socket_fds[0]); + base::ScopedFD exception_swallower_fd(socket_fds[1]); + + // fd_ is long-lived. Make sure that nobody accidentaly inherits it. + PCHECK(fcntl(fd_.get(), F_SETFD, FD_CLOEXEC) != -1) << "fcntl"; + + // SIGPIPE is undesirable when writing to this socket. Allow broken-pipe + // writes to fail with EPIPE instead. + for (size_t index = 0; index < arraysize(socket_fds); ++index) { + constexpr int value = 1; + PCHECK(setsockopt(socket_fds[index], + SOL_SOCKET, + SO_NOSIGPIPE, + &value, + sizeof(value)) == 0) + << "setsockopt"; + } + + std::vector argv; + argv.reserve(2); + argv.push_back(exception_swallower_server_path.value()); + argv.push_back( + base::StringPrintf("--socket-fd=%d", exception_swallower_fd.get())); + + CHECK(DoubleForkAndExec(argv, exception_swallower_fd.get(), false, nullptr)); + + // Close the exception swallower server’s side of the socket, so that it’s the + // only process that can use it. + exception_swallower_fd.reset(); + + // When the exception swallower server provides its registered service name, + // it’s ready to go. + uint8_t service_name_size; + CheckedReadFileExactly( + fd_.get(), &service_name_size, sizeof(service_name_size)); + service_name_.resize(service_name_size); + if (!service_name_.empty()) { + CheckedReadFileExactly(fd_.get(), &service_name_[0], service_name_.size()); + } + + // Verify that everything’s set up. + base::mac::ScopedMachSendRight exception_swallower_port( + BootstrapLookUp(service_name_)); + CHECK(exception_swallower_port.is_valid()); +} + +ExceptionSwallower::~ExceptionSwallower() {} + +// static +ExceptionSwallower* ExceptionSwallower::Get() { + static ExceptionSwallower* const instance = new ExceptionSwallower(); + return instance; +} + +void ExceptionSwallower::SetParent() { + parent_pid_ = getpid(); +} + +void ExceptionSwallower::SwallowExceptions() { + CHECK_NE(getpid(), parent_pid_); + + base::mac::ScopedMachSendRight exception_swallower_port( + BootstrapLookUp(service_name_)); + CHECK(exception_swallower_port.is_valid()); + + ExceptionPorts task_exception_ports(ExceptionPorts::kTargetTypeTask, + TASK_NULL); + + // The mask is similar to the one used by CrashpadClient::UseHandler(), but + // EXC_CORPSE_NOTIFY is added. This is done for the benefit of tests that + // crash intentionally with their own custom exception port set for EXC_CRASH. + // In that case, depending on the actions taken by the EXC_CRASH handler, the + // exception may be transformed by the kernel into an EXC_CORPSE_NOTIFY, which + // would be sent to an EXC_CORPSE_NOTIFY handler, normally the system’s crash + // reporter at the task or host level. See 10.13.0 + // xnu-4570.1.46/bsd/kern/kern_exit.c proc_prepareexit(). Swallowing + // EXC_CORPSE_NOTIFY at the task level prevents such exceptions from reaching + // the system’s crash reporter. + CHECK(task_exception_ports.SetExceptionPort( + (EXC_MASK_CRASH | + EXC_MASK_RESOURCE | + EXC_MASK_GUARD | + EXC_MASK_CORPSE_NOTIFY) & ExcMaskValid(), + exception_swallower_port.get(), + EXCEPTION_DEFAULT, + THREAD_STATE_NONE)); +} + +} // namespace test +} // namespace crashpad diff --git a/test/mac/exception_swallower.h b/test/mac/exception_swallower.h new file mode 100644 index 00000000..ddf49b46 --- /dev/null +++ b/test/mac/exception_swallower.h @@ -0,0 +1,157 @@ +// Copyright 2017 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_TEST_MAC_EXCEPTION_SWALLOWER_H_ +#define CRASHPAD_TEST_MAC_EXCEPTION_SWALLOWER_H_ + +#include + +#include + +#include "base/files/scoped_file.h" +#include "base/macros.h" + +namespace crashpad { +namespace test { + +//! \brief Swallows `EXC_CRASH` and `EXC_CORPSE_NOTIFY` exceptions in test child +//! processes. +//! +//! This class is intended to be used by test code that crashes intentionally. +//! +//! On macOS, the system’s crash reporter normally saves crash reports for all +//! crashes in test code, by virtue of being set as the `EXC_CRASH` or +//! `EXC_CORPSE_NOTIFY` handler. This litters the user’s +//! `~/Library/Logs/DiagnosticReports` directory and can be time-consuming. +//! Reports generated for code that crashes intentionally have no value, and +//! many Crashpad tests do crash intentionally. +//! +//! In order to prevent the system’s crash reporter from handling intentional +//! crashes, use this class. First, ensure that the exception swallower server +//! process, `crashpad_exception_swallower`, is running by calling +//! Parent_PrepareForCrashingChild() or Parent_PrepareForGtestDeathTest() from +//! the parent test process. Then, call Child_SwallowExceptions() from the test +//! child process that crashes intentionally. +//! +//! Don’t call Child_SwallowExceptions() except in test child processes that are +//! expected to crash. It is invalid to call Child_SwallowExceptions() in the +//! parent test process. +//! +//! An exception swallower server process started by this interface will live as +//! long as the process that created it, and will then exit. +//! +//! Crashpad’s ASSERT_DEATH_CRASH(), EXPECT_DEATH_CRASH(), ASSERT_DEATH_CHECK(), +//! and EXPECT_DEATH_CHECK() macros make use of this class on macOS, as does the +//! Multiprocess test interface. +class ExceptionSwallower { + public: + //! \brief In a parent test process, prepares for a crashing child process + //! whose exceptions are to be swallowed. + //! + //! Calling this in a parent test process starts an exception swallower server + //! process if none has been started yet. Subsequently, a forked child process + //! expecting to crash can call Child_SwallowExceptions() to direct exceptions + //! to the exception swallower server process. Multiple children can share a + //! single exception swallower server process. + //! + //! This function establishes the exception swallower server unconditionally. + //! This is not appropriate for gtest death tests, which should use + //! Parent_PrepareForGtestDeathTest() instead. + static void Parent_PrepareForCrashingChild(); + + //! \brief In a parent test process, prepares for a gtest death test whose + //! exceptions are to be swallowed. + //! + //! This is similar to Parent_PrepareForCrashingChild(), except it only starts + //! an exception swallower server process if the gtest + //! death test style is “fast”. With the “fast” style, the death test is + //! run directly from a forked child. The alternative, “threadsafe”, + //! reexecutes the test executable to run the death test. Since the death test + //! does not run directly forked from the parent test process, the parent’s + //! ExceptionSwallower object would not be available to the child, rendering + //! any exception swallower server process started by a parent test process + //! unavailable to the child. Since such an exception swallower server process + //! would go unused, this function will not start one when running under the + //! “threadsafe” style. In that case, each child death test is responsible for + //! starting its own exception swallower server process, and this will occur + //! when child death tests call Child_SwallowExceptions(). + //! + //! This function establishes the exception swallower server conditionally + //! based on the gtest death test style. This is not appropriate for tests + //! that unconditionally fork a child that intentionally crashes without an + //! intervening execv(). For such tests, use Parent_PrepareForCrashingChild() + //! instead. + static void Parent_PrepareForGtestDeathTest(); + + //! \brief In a test child process, arranges to swallow `EXC_CRASH` and + //! `EXC_CORPSE_NOTIFY` exceptions. + //! + //! This must be called in a test child process. It must not be called from a + //! parent test process directly. + //! + //! It is not an error to call this in a child process without having first + //! called Parent_PrepareForCrashingChild() or + //! Parent_PrepareForGtestDeathTest() in the parent process, but failing to do + //! so precludes the possibility of multiple qualified child processes sharing + //! a single exception swallower server process. In this context, children + //! running directly from a forked parent are qualified. gtest death tests + //! under the “threadsafe” gtest + //! death test style are not qualified. + static void Child_SwallowExceptions(); + + private: + ExceptionSwallower(); + ~ExceptionSwallower(); + + //! \brief Returns the ExceptionSwallower singleton. + //! + //! If the object does not yet exist, it will be created, and the exception + //! swallower server process, `crashpad_exception_swallower`, will be started. + static ExceptionSwallower* Get(); + + //! \brief Identifies the calling process as the test parent. + //! + //! This is used to check for interface abuses. Its use is optional, but if + //! it’s called, SwallowExceptions() must not be called from the same process. + void SetParent(); + + //! \brief In a test child process, arranges to swallow `EXC_CRASH` and + //! `EXC_CORPSE_NOTIFY` exceptions. + //! + //! This must be called in a test child process. It must not be called from a + //! parent test process directly. + void SwallowExceptions(); + + std::string service_name_; + + // fd_ is half of a socketpair() that serves a dual purpose. The exception + // swallower server process writes its service name to the socket once + // registered, allowing the parent test process to obtain a reference to the + // service. The socket remains open in the parent test process so that the + // exception swallower server process can observe, based on reading + // end-of-file, when the parent test process has died. The exception swallower + // server process uses this as a signal that it’s safe to exit. + base::ScopedFD fd_; + + pid_t parent_pid_; + + DISALLOW_COPY_AND_ASSIGN(ExceptionSwallower); +}; + +} // namespace test +} // namespace crashpad + +#endif // CRASHPAD_TEST_MAC_EXCEPTION_SWALLOWER_H_ diff --git a/test/mac/exception_swallower_exe.cc b/test/mac/exception_swallower_exe.cc new file mode 100644 index 00000000..b5850418 --- /dev/null +++ b/test/mac/exception_swallower_exe.cc @@ -0,0 +1,229 @@ +// Copyright 2017 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 +#include +#include + +#include +#include + +#include "base/logging.h" +#include "base/numerics/safe_conversions.h" +#include "base/strings/stringprintf.h" +#include "handler/mac/exception_handler_server.h" +#include "tools/tool_support.h" +#include "util/file/file_io.h" +#include "util/mach/exc_server_variants.h" +#include "util/mach/mach_extensions.h" +#include "util/misc/random_string.h" +#include "util/posix/close_stdio.h" +#include "util/stdlib/string_number_conversion.h" +#include "util/thread/thread.h" + +namespace crashpad { +namespace { + +// A Mach exception handler that accepts all exceptions but doesn’t do anything +// with any of them. +class SwallowingExceptionHandler : public UniversalMachExcServer::Interface { + public: + SwallowingExceptionHandler() {} + ~SwallowingExceptionHandler() {} + + kern_return_t CatchMachException(exception_behavior_t behavior, + exception_handler_t exception_port, + thread_t thread, + task_t task, + exception_type_t exception, + const mach_exception_data_type_t* code, + mach_msg_type_number_t code_count, + thread_state_flavor_t* flavor, + ConstThreadState old_state, + mach_msg_type_number_t old_state_count, + thread_state_t new_state, + mach_msg_type_number_t* new_state_count, + const mach_msg_trailer_t* trailer, + bool* destroy_complex_request) override { + *destroy_complex_request = true; + + // Swallow. + + ExcServerCopyState( + behavior, old_state, old_state_count, new_state, new_state_count); + return ExcServerSuccessfulReturnValue(exception, behavior, false); + } + + private: + DISALLOW_COPY_AND_ASSIGN(SwallowingExceptionHandler); +}; + +// Watches a file descriptor, and when it reaches end-of-file, asks the +// ExceptionHandlerServer to stop. Because the file descriptor is one end of a +// socketpair(), and the other end is kept open in the client process, +// end-of-file will be reached when the client process terminates. +class EOFWatcherThread : public Thread { + public: + EOFWatcherThread(int fd, ExceptionHandlerServer* exception_handler_server) + : Thread(), + exception_handler_server_(exception_handler_server), + fd_(fd) {} + + private: + void ThreadMain() override { + char c; + ssize_t rv = ReadFile(fd_.get(), &c, 1); + PCHECK(rv >= 0) << internal::kNativeReadFunctionName; + CHECK(rv == 0); + + exception_handler_server_->Stop(); + } + + ExceptionHandlerServer* exception_handler_server_; // weak + base::ScopedFD fd_; + + DISALLOW_COPY_AND_ASSIGN(EOFWatcherThread); +}; + +void Usage(const std::string& me) { + fprintf(stderr, +"Usage: %s [OPTION]...\n" +"Crashpad's exception swallower.\n" +"\n" +" --socket-fd=FD synchronize with the client over FD\n" +" --help display this help and exit\n" +" --version output version information and exit\n", + me.c_str()); + ToolSupport::UsageTail(me); +} + +int ExceptionSwallowerMain(int argc, char* argv[]) { + const std::string me(basename(argv[0])); + + enum OptionFlags { + // Long options without short equivalents. + kOptionLastChar = 255, + kOptionSocketFD, + + // Standard options. + kOptionHelp = -2, + kOptionVersion = -3, + }; + + struct { + int socket_fd; + } options = {}; + options.socket_fd = -1; + + const option long_options[] = { + {"socket-fd", required_argument, nullptr, kOptionSocketFD}, + {"help", no_argument, nullptr, kOptionHelp}, + {"version", no_argument, nullptr, kOptionVersion}, + {nullptr, 0, nullptr, 0}, + }; + + int opt; + while ((opt = getopt_long(argc, argv, "", long_options, nullptr)) != -1) { + switch (opt) { + case kOptionSocketFD: + if (!StringToNumber(optarg, &options.socket_fd) || + options.socket_fd <= STDERR_FILENO) { + ToolSupport::UsageHint(me, "--socket-fd requires a file descriptor"); + return EXIT_FAILURE; + } + break; + case kOptionHelp: { + Usage(me); + return EXIT_SUCCESS; + } + case kOptionVersion: { + ToolSupport::Version(me); + return EXIT_SUCCESS; + } + default: { + ToolSupport::UsageHint(me, nullptr); + return EXIT_FAILURE; + } + } + } + argc -= optind; + argv += optind; + + if (options.socket_fd < 0) { + ToolSupport::UsageHint(me, "--socket-fd is required"); + return EXIT_FAILURE; + } + + if (argc) { + ToolSupport::UsageHint(me, nullptr); + return EXIT_FAILURE; + } + + CloseStdinAndStdout(); + + // Build a service name. Include the PID of the client at the other end of the + // socket, so that the service name has a meaningful relation back to the + // client that started this server process. A simple getppid() won’t do + // because the client started this process with a double-fork(). + pid_t peer_pid; + socklen_t peer_pid_size = base::checked_cast(sizeof(peer_pid)); + PCHECK(getsockopt(options.socket_fd, + SOL_LOCAL, + LOCAL_PEERPID, + &peer_pid, + &peer_pid_size) == 0) + << "getsockopt"; + CHECK_EQ(peer_pid_size, sizeof(peer_pid)); + + std::string service_name = + base::StringPrintf("org.chromium.crashpad.test.exception_swallower.%d.%s", + peer_pid, + RandomString().c_str()); + + base::mac::ScopedMachReceiveRight receive_right( + BootstrapCheckIn(service_name)); + CHECK(receive_right.is_valid()); + + // Tell the client that the service has been checked in, providing the + // service name. + uint8_t service_name_size = base::checked_cast(service_name.size()); + CheckedWriteFile( + options.socket_fd, &service_name_size, sizeof(service_name_size)); + CheckedWriteFile( + options.socket_fd, service_name.c_str(), service_name.size()); + + ExceptionHandlerServer exception_handler_server(std::move(receive_right), + true); + + EOFWatcherThread eof_watcher_thread(options.socket_fd, + &exception_handler_server); + eof_watcher_thread.Start(); + + // This runs until stopped by eof_watcher_thread. + SwallowingExceptionHandler swallowing_exception_handler; + exception_handler_server.Run(&swallowing_exception_handler); + + eof_watcher_thread.Join(); + + return EXIT_SUCCESS; +} + +} // namespace +} // namespace crashpad + +int main(int argc, char* argv[]) { + return crashpad::ExceptionSwallowerMain(argc, argv); +} diff --git a/test/multiprocess.h b/test/multiprocess.h index a7ef6898..b0ff9870 100644 --- a/test/multiprocess.h +++ b/test/multiprocess.h @@ -78,6 +78,10 @@ class Multiprocess { //! TerminationReason::kTerminationNormal, and the default expected //! termination code is `EXIT_SUCCESS` (`0`). //! + //! This method does not need to be called if the default termination + //! expectation is appropriate, but if this method is called, it must be + //! called before Run(). + //! //! \param[in] reason Whether to expect the child to terminate normally or //! as a result of a signal. //! \param[in] code If \a reason is TerminationReason::kTerminationNormal, diff --git a/test/multiprocess_posix.cc b/test/multiprocess_posix.cc index d6796dda..2a8a15cc 100644 --- a/test/multiprocess_posix.cc +++ b/test/multiprocess_posix.cc @@ -27,9 +27,15 @@ #include "base/logging.h" #include "base/posix/eintr_wrapper.h" #include "base/strings/stringprintf.h" +#include "build/build_config.h" #include "gtest/gtest.h" #include "test/errors.h" #include "util/misc/scoped_forbid_return.h" +#include "util/posix/signals.h" + +#if defined(OS_MACOSX) +#include "test/mac/exception_swallower.h" +#endif namespace crashpad { namespace test { @@ -67,6 +73,17 @@ void Multiprocess::Run() { ASSERT_NO_FATAL_FAILURE(PreFork()); +#if defined(OS_MACOSX) + // If the child is expected to crash, set up an exception swallower process + // to swallow the exception instead of allowing it to be seen by the system’s + // crash reporter. + const bool swallow_exceptions = + reason_ == kTerminationSignal && Signals::IsCrashSignal(code_); + if (swallow_exceptions) { + ExceptionSwallower::Parent_PrepareForCrashingChild(); + } +#endif // OS_MACOSX + pid_t pid = fork(); ASSERT_GE(pid, 0) << ErrnoMessage("fork"); @@ -108,7 +125,7 @@ void Multiprocess::Run() { strsignal(code), WCOREDUMP(status) ? " (core dumped)" : ""); } else { - FAIL() << "Unknown termination reason"; + FAIL() << base::StringPrintf("Unknown termination reason 0x%x", status); } if (reason_ == kTerminationNormal) { @@ -123,12 +140,20 @@ void Multiprocess::Run() { ADD_FAILURE() << message; } } else { +#if defined(OS_MACOSX) + if (swallow_exceptions) { + ExceptionSwallower::Child_SwallowExceptions(); + } +#endif // OS_MACOSX + RunChild(); } } void Multiprocess::SetExpectedChildTermination(TerminationReason reason, int code) { + EXPECT_EQ(info_, nullptr) + << "SetExpectedChildTermination() must be called before Run()"; reason_ = reason; code_ = code; } diff --git a/test/multiprocess_posix_test.cc b/test/multiprocess_posix_test.cc index 44894e8c..94891477 100644 --- a/test/multiprocess_posix_test.cc +++ b/test/multiprocess_posix_test.cc @@ -20,7 +20,7 @@ #include "base/macros.h" #include "gtest/gtest.h" -#include "test/gtest_death_check.h" +#include "test/gtest_death.h" #include "util/file/file_io.h" namespace crashpad { diff --git a/test/test.gyp b/test/test.gyp index 4b389334..f6c1f251 100644 --- a/test/test.gyp +++ b/test/test.gyp @@ -37,7 +37,7 @@ 'file.h', 'filesystem.cc', 'filesystem.h', - 'gtest_death_check.h', + 'gtest_death.h', 'gtest_disabled.cc', 'gtest_disabled.h', 'hex_string.cc', @@ -46,6 +46,8 @@ 'linux/fake_ptrace_connection.h', 'mac/dyld.cc', 'mac/dyld.h', + 'mac/exception_swallower.cc', + 'mac/exception_swallower.h', 'mac/mach_errors.cc', 'mac/mach_errors.h', 'mac/mach_multiprocess.cc', @@ -81,6 +83,9 @@ }, 'conditions': [ ['OS=="mac"', { + 'dependencies': [ + 'crashpad_exception_swallower', + ], 'link_settings': { 'libraries': [ '$(SDKROOT)/usr/lib/libbsm.dylib', @@ -141,4 +146,27 @@ ], }, ], + 'conditions': [ + ['OS=="mac"', { + 'targets': [ + { + 'target_name': 'crashpad_exception_swallower', + 'type': 'executable', + 'dependencies': [ + '../compat/compat.gyp:crashpad_compat', + '../handler/handler.gyp:crashpad_handler_lib', + '../third_party/mini_chromium/mini_chromium.gyp:base', + '../tools/tools.gyp:crashpad_tool_support', + '../util/util.gyp:crashpad_util', + ], + 'include_dirs': [ + '..', + ], + 'sources': [ + 'mac/exception_swallower_exe.cc', + ], + }, + ], + }], + ], } diff --git a/util/BUILD.gn b/util/BUILD.gn index b61a76e3..1175de3e 100644 --- a/util/BUILD.gn +++ b/util/BUILD.gn @@ -149,6 +149,8 @@ static_library("util") { "posix/close_multiple.h", "posix/close_stdio.cc", "posix/close_stdio.h", + "posix/double_fork_and_exec.cc", + "posix/double_fork_and_exec.h", "posix/drop_privileges.cc", "posix/drop_privileges.h", "posix/process_info.h", diff --git a/util/mach/composite_mach_message_server_test.cc b/util/mach/composite_mach_message_server_test.cc index 74e1707a..d45eca0b 100644 --- a/util/mach/composite_mach_message_server_test.cc +++ b/util/mach/composite_mach_message_server_test.cc @@ -18,7 +18,7 @@ #include "base/strings/stringprintf.h" #include "gtest/gtest.h" -#include "test/gtest_death_check.h" +#include "test/gtest_death.h" #include "util/mach/mach_message.h" namespace crashpad { diff --git a/util/mach/exc_server_variants_test.cc b/util/mach/exc_server_variants_test.cc index 358cd1ec..5e67bfe3 100644 --- a/util/mach/exc_server_variants_test.cc +++ b/util/mach/exc_server_variants_test.cc @@ -968,7 +968,10 @@ class TestExcServerVariants : public MachMultiprocess, behavior_(behavior), flavor_(flavor), state_count_(state_count), - handled_(false) {} + handled_(false) { + // This is how the __builtin_trap() in MachMultiprocessChild() appears. + SetExpectedChildTermination(kTerminationSignal, SIGILL); + } // UniversalMachExcServer::Interface: @@ -1013,7 +1016,6 @@ class TestExcServerVariants : public MachMultiprocess, if (exception == EXC_CRASH && code_count >= 1) { int signal; ExcCrashRecoverOriginalException(code[0], nullptr, &signal); - SetExpectedChildTermination(kTerminationSignal, signal); } const bool has_state = ExceptionBehaviorHasState(behavior); diff --git a/util/mach/exception_ports_test.cc b/util/mach/exception_ports_test.cc index 224217bf..af9cc726 100644 --- a/util/mach/exception_ports_test.cc +++ b/util/mach/exception_ports_test.cc @@ -132,7 +132,12 @@ class TestExceptionPorts : public MachMultiprocess, set_on_(set_on), set_type_(set_type), who_crashes_(who_crashes), - handled_(false) {} + handled_(false) { + if (who_crashes_ != kNobodyCrashes) { + // This is how the __builtin_trap() in Child::Crash() appears. + SetExpectedChildTermination(kTerminationSignal, SIGILL); + } + } SetOn set_on() const { return set_on_; } SetType set_type() const { return set_type_; } @@ -190,8 +195,6 @@ class TestExceptionPorts : public MachMultiprocess, // The child crashed with __builtin_trap(), which shows up as SIGILL. EXPECT_EQ(signal, SIGILL); - - SetExpectedChildTermination(kTerminationSignal, signal); } EXPECT_EQ(AuditPIDFromMachMessageTrailer(trailer), 0); diff --git a/util/misc/from_pointer_cast_test.cc b/util/misc/from_pointer_cast_test.cc index b961abba..1a6850ff 100644 --- a/util/misc/from_pointer_cast_test.cc +++ b/util/misc/from_pointer_cast_test.cc @@ -21,7 +21,7 @@ #include "build/build_config.h" #include "gtest/gtest.h" -#include "test/gtest_death_check.h" +#include "test/gtest_death.h" namespace crashpad { namespace test { @@ -233,27 +233,28 @@ TEST(FromPointerCast, ToNarrowInteger) { TEST(FromPointerCastDeathTest, ToNarrowInteger) { if (sizeof(int) < sizeof(void*)) { - EXPECT_DEATH(FromPointerCast( - reinterpret_cast(static_cast( - std::numeric_limits::max() + 1ull))), - ""); - EXPECT_DEATH(FromPointerCast( - reinterpret_cast(static_cast( - std::numeric_limits::max() + 1ull))), - ""); + EXPECT_DEATH_CHECK( + FromPointerCast(reinterpret_cast(static_cast( + std::numeric_limits::max() + 1ull))), + ""); + EXPECT_DEATH_CHECK( + FromPointerCast( + reinterpret_cast(static_cast( + std::numeric_limits::max() + 1ull))), + ""); } // int and unsigned int may not be narrower than a pointer, so also test short // and unsigned short. - EXPECT_DEATH(FromPointerCast( - reinterpret_cast(static_cast( - std::numeric_limits::max() + 1u))), - ""); - EXPECT_DEATH(FromPointerCast( - reinterpret_cast(static_cast( - std::numeric_limits::max() + 1u))), - ""); + EXPECT_DEATH_CHECK( + FromPointerCast(reinterpret_cast(static_cast( + std::numeric_limits::max() + 1u))), + ""); + EXPECT_DEATH_CHECK(FromPointerCast( + reinterpret_cast(static_cast( + std::numeric_limits::max() + 1u))), + ""); } } // namespace diff --git a/util/misc/initialization_state_dcheck_test.cc b/util/misc/initialization_state_dcheck_test.cc index 2c407675..f954d004 100644 --- a/util/misc/initialization_state_dcheck_test.cc +++ b/util/misc/initialization_state_dcheck_test.cc @@ -21,7 +21,7 @@ #include "base/logging.h" #include "base/memory/free_deleter.h" #include "gtest/gtest.h" -#include "test/gtest_death_check.h" +#include "test/gtest_death.h" namespace crashpad { namespace test { diff --git a/util/misc/scoped_forbid_return_test.cc b/util/misc/scoped_forbid_return_test.cc index 4f73ea58..06cb5494 100644 --- a/util/misc/scoped_forbid_return_test.cc +++ b/util/misc/scoped_forbid_return_test.cc @@ -16,7 +16,7 @@ #include "base/compiler_specific.h" #include "gtest/gtest.h" -#include "test/gtest_death_check.h" +#include "test/gtest_death.h" namespace crashpad { namespace test { diff --git a/util/net/http_multipart_builder_test.cc b/util/net/http_multipart_builder_test.cc index 08806c37..dab9e7e7 100644 --- a/util/net/http_multipart_builder_test.cc +++ b/util/net/http_multipart_builder_test.cc @@ -19,7 +19,7 @@ #include #include "gtest/gtest.h" -#include "test/gtest_death_check.h" +#include "test/gtest_death.h" #include "test/test_paths.h" #include "util/net/http_body.h" #include "util/net/http_body_test_util.h" diff --git a/util/posix/double_fork_and_exec.cc b/util/posix/double_fork_and_exec.cc new file mode 100644 index 00000000..df74f709 --- /dev/null +++ b/util/posix/double_fork_and_exec.cc @@ -0,0 +1,146 @@ +// Copyright 2017 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/posix/double_fork_and_exec.h" + +#include +#include +#include +#include + +#include "base/logging.h" +#include "base/posix/eintr_wrapper.h" +#include "base/strings/stringprintf.h" +#include "util/posix/close_multiple.h" + +namespace crashpad { + +bool DoubleForkAndExec(const std::vector& argv, + int preserve_fd, + bool use_path, + void (*child_function)()) { + // argv_c contains const char* pointers and is terminated by nullptr. This is + // suitable for passing to execv(). Although argv_c is not used in the parent + // process, it must be built in the parent process because it’s unsafe to do + // so in the child or grandchild process. + std::vector argv_c; + argv_c.reserve(argv.size() + 1); + for (const std::string& argument : argv) { + argv_c.push_back(argument.c_str()); + } + argv_c.push_back(nullptr); + + // Double-fork(). The three processes involved are parent, child, and + // grandchild. The grandchild will call execv(). The child exits immediately + // after spawning the grandchild, so the grandchild becomes an orphan and its + // parent process ID becomes 1. This relieves the parent and child of the + // responsibility to reap the grandchild with waitpid() or similar. The + // grandchild is expected to outlive the parent process, so the parent + // shouldn’t be concerned with reaping it. This approach means that accidental + // early termination of the handler process will not result in a zombie + // process. + pid_t pid = fork(); + if (pid < 0) { + PLOG(ERROR) << "fork"; + return false; + } + + if (pid == 0) { + // Child process. + + if (child_function) { + child_function(); + } + + // Call setsid(), creating a new process group and a new session, both led + // by this process. The new process group has no controlling terminal. This + // disconnects it from signals generated by the parent process’ terminal. + // + // setsid() is done in the child instead of the grandchild so that the + // grandchild will not be a session leader. If it were a session leader, an + // accidental open() of a terminal device without O_NOCTTY would make that + // terminal the controlling terminal. + // + // It’s not desirable for the grandchild to have a controlling terminal. The + // grandchild manages its own lifetime, such as by monitoring clients on its + // own and exiting when it loses all clients and when it deems it + // appropraite to do so. It may serve clients in different process groups or + // sessions than its original client, and receiving signals intended for its + // original client’s process group could be harmful in that case. + PCHECK(setsid() != -1) << "setsid"; + + pid = fork(); + if (pid < 0) { + PLOG(FATAL) << "fork"; + } + + if (pid > 0) { + // Child process. + + // _exit() instead of exit(), because fork() was called. + _exit(EXIT_SUCCESS); + } + + // Grandchild process. + + CloseMultipleNowOrOnExec(STDERR_FILENO + 1, preserve_fd); + + // &argv_c[0] is a pointer to a pointer to const char data, but because of + // how C (not C++) works, execvp() wants a pointer to a const pointer to + // char data. It modifies neither the data nor the pointers, so the + // const_cast is safe. + char* const* argv_for_execv = const_cast(&argv_c[0]); + + if (use_path) { + execvp(argv_for_execv[0], argv_for_execv); + PLOG(FATAL) << "execvp " << argv_for_execv[0]; + } + + execv(argv_for_execv[0], argv_for_execv); + PLOG(FATAL) << "execv " << argv_for_execv[0]; + } + + // waitpid() for the child, so that it does not become a zombie process. The + // child normally exits quickly. + // + // Failures from this point on may result in the accumulation of a zombie, but + // should not be considered fatal. Log only warnings, but don’t treat these + // failures as a failure of the function overall. + int status; + pid_t wait_pid = HANDLE_EINTR(waitpid(pid, &status, 0)); + if (wait_pid == -1) { + PLOG(WARNING) << "waitpid"; + return true; + } + DCHECK_EQ(wait_pid, pid); + + if (WIFSIGNALED(status)) { + int sig = WTERMSIG(status); + LOG(WARNING) << base::StringPrintf( + "intermediate process terminated by signal %d (%s)%s", + sig, + strsignal(sig), + WCOREDUMP(status) ? " (core dumped)" : ""); + } else if (!WIFEXITED(status)) { + LOG(WARNING) << base::StringPrintf( + "intermediate process: unknown termination 0x%x", status); + } else if (WEXITSTATUS(status) != EXIT_SUCCESS) { + LOG(WARNING) << "intermediate process exited with code " + << WEXITSTATUS(status); + } + + return true; +} + +} // namespace crashpad diff --git a/util/posix/double_fork_and_exec.h b/util/posix/double_fork_and_exec.h new file mode 100644 index 00000000..df340d07 --- /dev/null +++ b/util/posix/double_fork_and_exec.h @@ -0,0 +1,67 @@ +// Copyright 2017 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_POSIX_DOUBLE_FORK_AND_EXEC_H_ +#define CRASHPAD_UTIL_POSIX_DOUBLE_FORK_AND_EXEC_H_ + +#include +#include + +namespace crashpad { + +//! \brief Executes a (grand-)child process. +//! +//! The grandchild process will be started through the +//! double-`fork()`-and-`execv()` pattern. This allows the grandchild to fully +//! disassociate from the parent. The grandchild will not be a member of the +//! parent’s process group or session and will not have a controlling terminal, +//! providing isolation from signals not intended for it. The grandchild’s +//! parent process, in terms of the process tree hierarchy, will be the process +//! with process ID 1, relieving any other process of the responsibility to reap +//! it via `waitpid()`. Aside from the three file descriptors associated with +//! the standard input/output streams and any file descriptor passed in \a +//! preserve_fd, the grandchild will not inherit any file descriptors from the +//! parent process. +//! +//! \param[in] argv The argument vector to start the grandchild process with. +//! `argv[0]` is used as the path to the executable. +//! \param[in] preserve_fd A file descriptor to be inherited by the grandchild +//! process. This file descriptor is inherited in addition to the three file +//! descriptors associated with the standard input/output streams. Use `-1` +//! if no additional file descriptors are to be inherited. +//! \param[in] use_path Whether to consult the `PATH` environment variable when +//! requested to start an executable at a non-absolute path. If `false`, +//! `execv()`, which does not consult `PATH`, will be used. If `true`, +//! `execvp()`, which does consult `PATH`, will be used. +//! \param[in] child_function If not `nullptr`, this function will be called in +//! the intermediate child process, prior to the second `fork()`. Take note +//! that this function will run in the context of a forked process, and must +//! be safe for that purpose. +//! +//! \return `true` on success, and `false` on failure with a message logged. +//! Only failures that occur in the parent process that indicate a definite +//! failure to start the the grandchild are reported in the return value. +//! Failures in the intermediate child or grandchild processes cannot be +//! reported in the return value, and are addressed by logging a message and +//! terminating. The caller assumes the responsibility for detecting such +//! failures, for example, by observing a failure to perform a successful +//! handshake with the grandchild process. +bool DoubleForkAndExec(const std::vector& argv, + int preserve_fd, + bool use_path, + void (*child_function)()); + +} // namespace crashpad + +#endif // CRASHPAD_UTIL_POSIX_DOUBLE_FORK_AND_EXEC_H_ diff --git a/util/posix/scoped_mmap_test.cc b/util/posix/scoped_mmap_test.cc index 33807295..7312b849 100644 --- a/util/posix/scoped_mmap_test.cc +++ b/util/posix/scoped_mmap_test.cc @@ -22,6 +22,7 @@ #include "base/rand_util.h" #include "base/strings/stringprintf.h" #include "gtest/gtest.h" +#include "test/gtest_death.h" namespace crashpad { namespace test { @@ -118,7 +119,7 @@ TEST(ScopedMmapDeathTest, Destructor) { cookie.SetUp(mapping.addr_as()); } - EXPECT_DEATH(cookie.Check(), ""); + EXPECT_DEATH_CRASH(cookie.Check(), ""); } TEST(ScopedMmapDeathTest, Reset) { @@ -135,7 +136,7 @@ TEST(ScopedMmapDeathTest, Reset) { ASSERT_TRUE(mapping.Reset()); - EXPECT_DEATH(cookie.Check(), ""); + EXPECT_DEATH_CRASH(cookie.Check(), ""); } TEST(ScopedMmapDeathTest, ResetAddrLen_Shrink) { @@ -164,8 +165,8 @@ TEST(ScopedMmapDeathTest, ResetAddrLen_Shrink) { EXPECT_EQ(cookies[1].Observed(), cookies[1].Expected()); - EXPECT_DEATH(cookies[0].Check(), ""); - EXPECT_DEATH(cookies[2].Check(), ""); + EXPECT_DEATH_CRASH(cookies[0].Check(), ""); + EXPECT_DEATH_CRASH(cookies[2].Check(), ""); } TEST(ScopedMmap, ResetAddrLen_Grow) { @@ -230,7 +231,7 @@ TEST(ScopedMmapDeathTest, ResetAddrLen_MoveDownAndGrow) { EXPECT_EQ(cookies[0].Observed(), cookies[0].Expected()); EXPECT_EQ(cookies[1].Observed(), cookies[1].Expected()); - EXPECT_DEATH(cookies[2].Check(), ""); + EXPECT_DEATH_CRASH(cookies[2].Check(), ""); } TEST(ScopedMmapDeathTest, ResetAddrLen_MoveUpAndShrink) { @@ -262,8 +263,8 @@ TEST(ScopedMmapDeathTest, ResetAddrLen_MoveUpAndShrink) { EXPECT_EQ(cookies[2].Observed(), cookies[2].Expected()); - EXPECT_DEATH(cookies[0].Check(), ""); - EXPECT_DEATH(cookies[1].Check(), ""); + EXPECT_DEATH_CRASH(cookies[0].Check(), ""); + EXPECT_DEATH_CRASH(cookies[1].Check(), ""); } TEST(ScopedMmapDeathTest, ResetMmap) { @@ -289,7 +290,7 @@ TEST(ScopedMmapDeathTest, ResetMmap) { EXPECT_NE(mapping.addr(), MAP_FAILED); EXPECT_EQ(mapping.len(), kPageSize); - EXPECT_DEATH(cookie.Check(), ""); + EXPECT_DEATH_CRASH(cookie.Check(), ""); } TEST(ScopedMmapDeathTest, Mprotect) { @@ -306,7 +307,7 @@ TEST(ScopedMmapDeathTest, Mprotect) { ASSERT_TRUE(mapping.Mprotect(PROT_READ)); - EXPECT_DEATH(*addr = 0, ""); + EXPECT_DEATH_CRASH(*addr = 0, ""); ASSERT_TRUE(mapping.Mprotect(PROT_READ | PROT_WRITE)); EXPECT_EQ(*addr, 1); diff --git a/util/stdlib/aligned_allocator_test.cc b/util/stdlib/aligned_allocator_test.cc index d52dcfa2..1c16dcc5 100644 --- a/util/stdlib/aligned_allocator_test.cc +++ b/util/stdlib/aligned_allocator_test.cc @@ -18,6 +18,7 @@ #include "base/compiler_specific.h" #include "gtest/gtest.h" +#include "test/gtest_death.h" #if defined(OS_WIN) #include @@ -110,7 +111,7 @@ void BadAlignmentTest() { } TEST(AlignedAllocatorDeathTest, BadAlignment) { - ASSERT_DEATH(BadAlignmentTest(), ""); + ASSERT_DEATH_CRASH(BadAlignmentTest(), ""); } } // namespace diff --git a/util/util.gyp b/util/util.gyp index 63314834..5aeb4503 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -179,6 +179,8 @@ 'posix/close_stdio.h', 'posix/drop_privileges.cc', 'posix/drop_privileges.h', + 'posix/double_fork_and_exec.cc', + 'posix/double_fork_and_exec.h', 'posix/process_info.h', 'posix/process_info_linux.cc', 'posix/process_info_mac.cc',