commit 8c70c504b241f918e41f28a3102f5ca03475082b
author Mark Mentovai <mark@chromium.org> Fri Apr 19 17:08:56 2019 -0400
committer Mark Mentovai <mark@chromium.org> Fri Apr 19 21:42:27 2019 +0000
tree b50206b34e6fa0498ff25a6971af6159d2c90f9c
parent ebab5970b027c8c41eccf67a83883b4bfb736505

Make breakpad_unittests work with Chrome's test runner instead of gtest's

Chrome's test runner on Linux installs its own StackDumpSignalHandler
which swallows signals and doesn't re-raise them. This is sloppy, but
apparently there are reasons (https://crbug.com/551681). For
breakpad_unittests, it causes problems where a test process expects (via
waitpid()) to observe a child crash.  Deal with those cases by
explicitly restoring the default signal handler.

In another case, Chrome's test runner seems to have been arriving at the
conclusion that it was to expect output from a child. Transitioning from
exit() to _exit() fixes this problem, and it's not necessarily a bad
idea to do this in post-fork() children without an execve() anyway.

Bug: chromium:949098
Change-Id: I5a6af0c2a09cd8eac9998358f6d5ea665288236f
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1575670
Reviewed-by: Mark Mentovai <mark@chromium.org>

src/client/linux/handler/exception_handler_unittest.cc

diff --git a/src/client/linux/handler/exception_handler_unittest.cc b/src/client/linux/handler/exception_handler_unittest.cc
index 3040e0f..2a08060 100644
--- a/src/client/linux/handler/exception_handler_unittest.cc
+++ b/src/client/linux/handler/exception_handler_unittest.cc
@@ -421,6 +421,10 @@
 
   const pid_t child = fork();
   if (child == 0) {
+    // Custom signal handlers, which may have been installed by a test launcher,
+    // are undesirable in this child.
+    signal(SIGSEGV, SIG_DFL);
+
     CrashWithCallbacks(FilterCallbackReturnFalse, NULL, temp_dir.path());
   }
 

src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc

diff --git a/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc b/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc
index a4a0fd9..3060327 100644
--- a/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc
+++ b/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc
@@ -144,8 +144,8 @@
     if (child_pid_ == 0) {
       // child process
       RealTestBody();
-      exit(HasFatalFailure() ? kFatalFailure :
-           (HasNonfatalFailure() ? kNonFatalFailure : 0));
+      _exit(HasFatalFailure() ? kFatalFailure :
+            (HasNonfatalFailure() ? kNonFatalFailure : 0));
     }
 
     ASSERT_TRUE(child_pid_ > 0);
@@ -250,7 +250,7 @@
   helper_path_ = GetHelperBinary();
   if (helper_path_.empty()) {
     FAIL() << "Couldn't find helper binary";
-    exit(1);
+    _exit(1);
   }
 
   // mmap two segments out of the helper binary, one

src/common/linux/tests/crash_generator.cc

diff --git a/src/common/linux/tests/crash_generator.cc b/src/common/linux/tests/crash_generator.cc
index dd68113..3631967 100644
--- a/src/common/linux/tests/crash_generator.cc
+++ b/src/common/linux/tests/crash_generator.cc
@@ -184,6 +184,12 @@
 
   pid_t pid = fork();
   if (pid == 0) {
+    // Custom signal handlers, which may have been installed by a test launcher,
+    // are undesirable in this child.
+    if (signal(crash_signal, SIG_DFL) == SIG_ERR) {
+      perror("CrashGenerator: signal");
+      exit(1);
+    }
     if (chdir(temp_dir_.path().c_str()) == -1) {
       perror("CrashGenerator: Failed to change directory");
       exit(1);