linux: re-order first-chance-handlers, and disabled signal handlers

Both running first chance handlers and checking for disabled signal
handlers should no longer interact with DumpWithoutCrashing().

First-chance-handlers should also run even with disabled crashpad
signal handlers or else those signals would be reported by the next
chained signal handlers as crashes.

Change-Id: I64b3da42c400a1c431c6228d4da181ed56bfda89
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3403413
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>

client/crashpad_client.h

@@ -421,7 +421,7 @@ class CrashpadClient {
   //!     CaptureContext() or similar.
   static void DumpWithoutCrash(NativeCPUContext* context);
 
-  //! \brief Disables any installed crash handler, including any
+  //! \brief Disables any installed crash handler, not including any
   //!     FirstChanceHandler and crashes the current process.
   //!
   //! \param[in] message A message to be logged before crashing.

client/crashpad_client_linux.cc

@@ -148,17 +148,7 @@ class SignalHandler {
 
   // The base implementation for all signal handlers, suitable for calling
   // directly to simulate signal delivery.
-  bool HandleCrash(int signo, siginfo_t* siginfo, void* context) {
-    if (disabled_for_thread_) {
-      return false;
-    }
-
-    if (first_chance_handler_ &&
-        first_chance_handler_(
-            signo, siginfo, static_cast<ucontext_t*>(context))) {
-      return true;
-    }
-
+  void HandleCrash(int signo, siginfo_t* siginfo, void* context) {
     exception_information_.siginfo_address =
         FromPointerCast<decltype(exception_information_.siginfo_address)>(
             siginfo);
@@ -169,7 +159,6 @@ class SignalHandler {
 
     ScopedPrSetDumpable set_dumpable(false);
     HandleCrashImpl();
-    return false;
   }
 
  protected:
@@ -198,9 +187,16 @@ class SignalHandler {
   static void HandleOrReraiseSignal(int signo,
                                     siginfo_t* siginfo,
                                     void* context) {
-    if (handler_->HandleCrash(signo, siginfo, context)) {
+    if (handler_->first_chance_handler_ &&
+        handler_->first_chance_handler_(
+            signo, siginfo, static_cast<ucontext_t*>(context))) {
       return;
     }
+
+    if (!handler_->disabled_for_thread_) {
+      handler_->HandleCrash(signo, siginfo, context);
+    }
+
     Signals::RestoreHandlerAndReraiseSignalOnReturn(
         siginfo, handler_->old_actions_.ActionForSignal(signo));
   }

client/crashpad_client_linux_test.cc

@@ -15,6 +15,7 @@
 #include "client/crashpad_client.h"
 
 #include <dlfcn.h>
+#include <setjmp.h>
 #include <signal.h>
 #include <stdlib.h>
 #include <sys/syscall.h>
@@ -108,10 +109,6 @@ class StartHandlerForSelfTest
   StartHandlerForSelfTestOptions options_;
 };
 
-bool HandleCrashSuccessfully(int, siginfo_t*, ucontext_t*) {
-  return true;
-}
-
 bool InstallHandler(CrashpadClient* client,
                     bool start_at_crash,
                     const base::FilePath& handler_path,
@@ -222,13 +219,21 @@ int RecurseInfinitely(int* ptr) {
 }
 #pragma clang diagnostic pop
 
+sigjmp_buf do_crash_sigjmp_env;
+
+bool HandleCrashSuccessfully(int, siginfo_t*, ucontext_t*) {
+  siglongjmp(do_crash_sigjmp_env, 1);
+  return true;
+}
+
 void DoCrash(const StartHandlerForSelfTestOptions& options,
              CrashpadClient* client) {
+  if (sigsetjmp(do_crash_sigjmp_env, 1) != 0) {
+    return;
+  }
+
   switch (options.crash_type) {
     case CrashType::kSimulated:
-      if (options.set_first_chance_handler) {
-        client->SetFirstChanceExceptionHandler(HandleCrashSuccessfully);
-      }
       CRASHPAD_SIMULATE_CRASH();
       break;
 
@@ -364,6 +369,10 @@ CRASHPAD_CHILD_TEST_MAIN(StartHandlerForSelfTestChild) {
     return EXIT_FAILURE;
   }
 
+  if (options.set_first_chance_handler) {
+    client.SetFirstChanceExceptionHandler(HandleCrashSuccessfully);
+  }
+
 #if BUILDFLAG(IS_ANDROID)
   if (android_set_abort_message) {
     android_set_abort_message(kTestAbortMessage);
@@ -386,17 +395,19 @@ class StartHandlerForSelfInChildTest : public MultiprocessExec {
   StartHandlerForSelfInChildTest(const StartHandlerForSelfTestOptions& options)
       : MultiprocessExec(), options_(options) {
     SetChildTestMainFunction("StartHandlerForSelfTestChild");
-    switch (options.crash_type) {
-      case CrashType::kSimulated:
-        // kTerminationNormal, EXIT_SUCCESS
-        break;
-      case CrashType::kBuiltinTrap:
-        SetExpectedChildTerminationBuiltinTrap();
-        break;
-      case CrashType::kInfiniteRecursion:
-        SetExpectedChildTermination(TerminationReason::kTerminationSignal,
-                                    SIGSEGV);
-        break;
+    if (!options.set_first_chance_handler) {
+      switch (options.crash_type) {
+        case CrashType::kSimulated:
+          // kTerminationNormal, EXIT_SUCCESS
+          break;
+        case CrashType::kBuiltinTrap:
+          SetExpectedChildTerminationBuiltinTrap();
+          break;
+        case CrashType::kInfiniteRecursion:
+          SetExpectedChildTermination(TerminationReason::kTerminationSignal,
+                                      SIGSEGV);
+          break;
+      }
     }
   }
 
@@ -447,9 +458,12 @@ class StartHandlerForSelfInChildTest : public MultiprocessExec {
     reports.clear();
     ASSERT_EQ(database->GetPendingReports(&reports),
               CrashReportDatabase::kNoError);
-    ASSERT_EQ(reports.size(), options_.set_first_chance_handler ? 0u : 1u);
 
-    if (options_.set_first_chance_handler) {
+    bool report_expected = !options_.set_first_chance_handler ||
+                           options_.crash_type == CrashType::kSimulated;
+    ASSERT_EQ(reports.size(), report_expected ? 1u : 0u);
+
+    if (!report_expected) {
       return;
     }
 
@@ -463,11 +477,6 @@ class StartHandlerForSelfInChildTest : public MultiprocessExec {
 };
 
 TEST_P(StartHandlerForSelfTest, StartHandlerInChild) {
-  if (Options().set_first_chance_handler &&
-      Options().crash_type != CrashType::kSimulated) {
-    // TODO(jperaza): test first chance handlers with real crashes.
-    return;
-  }
 #if defined(ADDRESS_SANITIZER) || defined(MEMORY_SANITIZER) || \
     defined(UNDEFINED_SANITIZER)
   if (Options().crash_type == CrashType::kInfiniteRecursion) {

snapshot/linux/exception_snapshot_linux.cc

@@ -24,6 +24,7 @@
 #include "util/linux/traits.h"
 #include "util/misc/reinterpret_bytes.h"
 #include "util/numeric/safe_assignment.h"
+#include "util/posix/signals.h"
 
 namespace crashpad {
 namespace internal {
@@ -445,6 +446,9 @@ bool ExceptionSnapshotLinux::ReadSiginfo(ProcessReaderLinux* reader,
       PUSH_CODE(siginfo.sigval.sigval);
       break;
 
+    case Signals::kSimulatedSigno:
+      break;
+
     default:
       LOG(WARNING) << "Unhandled signal " << siginfo.signo;
   }