From 94a5a72efaf29e431436ba39996a5e62d3be4c17 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Mon, 20 Nov 2017 13:32:26 -0500 Subject: [PATCH] =?UTF-8?q?mac:=20Tests=20that=20crash=20intentionally=20s?= =?UTF-8?q?houldn=E2=80=99t=20go=20to=20ReportCrash?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Crashpad has many tests that crash intentionally. Some of these are gtest death tests, and others arrange for intentional crashes to test Crashpad’s own crash-catching logic. On macOS, all of the gtest death tests and some of the other intentional crashes were being logged by ReportCrash, the system’s crash reporter. Since these reports corresponded to intentional crashes, they were never useful, and served only to clutter ~/Library/Logs/DiagnosticReports. Since Crashpad is adept at handling exceptions on its own, this introduces the “exception swallowing server”, crashpad_exception_swallower, which is a Mach exception server that implements a no-op exception handler routine for all exceptions received. The exception swallowing server is established as the task handler for EXC_CRASH and EXC_CORPSE_NOTIFY exceptions during gtest death tests invoked by {ASSERT,EXPECT}_DEATH_{CHECK,CRASH}, and for all child processes invoked by the Multiprocess test infrastructure. The exception swallowing server is not in effect at other times, so unexpected crashes in test code can still be handled by ReportCrash or another crash reporter. With this change in place, no new reports are generated in the user-level ~/Library/Logs/DiagnosticReports or the system’s /Library/Logs/DiagnosticReports during a run of Crashpad’s full test suite on macOS. Bug: crashpad:33 Change-Id: I13891853a7e25accc30da21fa7ea8bd7d1f3bd2f Reviewed-on: https://chromium-review.googlesource.com/777859 Commit-Queue: Mark Mentovai Reviewed-by: Robert Sesek --- client/annotation.h | 4 +- client/annotation_test.cc | 11 +- client/crashpad_client_mac.cc | 104 +------- client/simple_address_range_bag_test.cc | 2 +- client/simple_string_dictionary_test.cc | 2 +- minidump/minidump_exception_writer_test.cc | 2 +- minidump/minidump_file_writer_test.cc | 2 +- minidump/minidump_module_writer_test.cc | 2 +- minidump/minidump_system_info_writer_test.cc | 2 +- minidump/minidump_thread_writer_test.cc | 2 +- .../mach_o_image_annotations_reader_test.cc | 53 ++-- test/BUILD.gn | 23 +- test/gtest_death.h | 115 +++++++++ test/gtest_death_check.h | 55 ----- test/mac/exception_swallower.cc | 155 ++++++++++++ test/mac/exception_swallower.h | 157 ++++++++++++ test/mac/exception_swallower_exe.cc | 229 ++++++++++++++++++ test/multiprocess.h | 4 + test/multiprocess_posix.cc | 27 ++- test/multiprocess_posix_test.cc | 2 +- test/test.gyp | 30 ++- util/BUILD.gn | 2 + .../composite_mach_message_server_test.cc | 2 +- util/mach/exc_server_variants_test.cc | 6 +- util/mach/exception_ports_test.cc | 9 +- util/misc/from_pointer_cast_test.cc | 35 +-- util/misc/initialization_state_dcheck_test.cc | 2 +- util/misc/scoped_forbid_return_test.cc | 2 +- util/net/http_multipart_builder_test.cc | 2 +- util/posix/double_fork_and_exec.cc | 146 +++++++++++ util/posix/double_fork_and_exec.h | 67 +++++ util/posix/scoped_mmap_test.cc | 19 +- util/stdlib/aligned_allocator_test.cc | 3 +- util/util.gyp | 2 + 34 files changed, 1053 insertions(+), 227 deletions(-) create mode 100644 test/gtest_death.h delete mode 100644 test/gtest_death_check.h create mode 100644 test/mac/exception_swallower.cc create mode 100644 test/mac/exception_swallower.h create mode 100644 test/mac/exception_swallower_exe.cc create mode 100644 util/posix/double_fork_and_exec.cc create mode 100644 util/posix/double_fork_and_exec.h 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',