Add ExcServerCopyState().

ExcServerCopyState() properly sets the new_state and new_state_count
out-parameters for exception handler routines that may deal with
state-carrying exceptions.

This used to exist inline in catch_exception_tool, but that
implementation had a bug caught by the new test.

TEST=crashpad_util_test ExcServerVariants.ExcServerCopyState and others
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1049023003

client/simulate_crash_mac_test.cc

@@ -225,6 +225,9 @@ class TestSimulateCrashMac final : public MachMultiprocess,
       return KERN_ABORTED;
     }
 
+    ExcServerCopyState(
+        behavior, old_state, old_state_count, new_state, new_state_count);
+
     return ExcServerSuccessfulReturnValue(behavior, true);
   }
 

handler/mac/crash_report_exception_handler.cc

@@ -193,6 +193,7 @@ kern_return_t CrashReportExceptionHandler::CatchMachException(
     upload_thread_->ReportPending();
   }
 
+  bool forwarded = false;
   if (client_options.system_crash_reporter_forwarding != TriState::kDisabled &&
       (exception == EXC_CRASH ||
        exception == EXC_RESOURCE ||
@@ -243,10 +244,19 @@ kern_return_t CrashReportExceptionHandler::CatchMachException(
           old_state_count,
           new_state_forward_count ? &new_state_forward[0] : nullptr,
           &new_state_forward_count);
-      MACH_LOG_IF(WARNING, kr != KERN_SUCCESS, kr)
+      if (kr == KERN_SUCCESS) {
+        forwarded = true;
+      } else {
+        MACH_LOG(WARNING, kr)
             << "UniversalExceptionRaise " << kSystemCrashReporterServiceName;
       }
     }
+  }
+
+  if (!forwarded) {
+    ExcServerCopyState(
+        behavior, old_state, old_state_count, new_state, new_state_count);
+  }
 
   return ExcServerSuccessfulReturnValue(behavior, false);
 }

tools/mac/catch_exception_tool.cc

@@ -133,13 +133,6 @@ class ExceptionServer : public UniversalMachExcServer::Interface {
     }
 
     if (ExceptionBehaviorHasState(behavior)) {
-      // If this is a state-carrying exception, make new_state something valid.
-      memcpy(
-          new_state,
-          old_state,
-          std::min(old_state_count, *new_state_count) * sizeof(old_state[0]));
-      *new_state_count = old_state_count;
-
       std::string flavor_string =
           ThreadStateFlavorToString(*flavor, kUseFullName | kUnknownIsNumeric);
       fprintf(options_.file,
@@ -156,6 +149,9 @@ class ExceptionServer : public UniversalMachExcServer::Interface {
       return KERN_FAILURE;
     }
 
+    ExcServerCopyState(
+        behavior, old_state, old_state_count, new_state, new_state_count);
+
     return ExcServerSuccessfulReturnValue(behavior, false);
   }
 

util/mach/exc_server_variants.cc

@@ -14,6 +14,8 @@
 
 #include "util/mach/exc_server_variants.h"
 
+#include <string.h>
+
 #include <algorithm>
 #include <vector>
 
@@ -801,4 +803,15 @@ kern_return_t ExcServerSuccessfulReturnValue(exception_behavior_t behavior,
   return KERN_SUCCESS;
 }
 
+void ExcServerCopyState(exception_behavior_t behavior,
+                        const natural_t* old_state,
+                        mach_msg_type_number_t old_state_count,
+                        thread_state_t new_state,
+                        mach_msg_type_number_t* new_state_count) {
+  if (ExceptionBehaviorHasState(behavior)) {
+    *new_state_count = std::min(old_state_count, *new_state_count);
+    memcpy(new_state, old_state, *new_state_count * sizeof(old_state[0]));
+  }
+}
+
 }  // namespace crashpad

util/mach/exc_server_variants.h

@@ -199,6 +199,43 @@ exception_type_t ExcCrashRecoverOriginalException(
 kern_return_t ExcServerSuccessfulReturnValue(exception_behavior_t behavior,
                                              bool set_thread_state);
 
+//! \brief Copies the old state to the new state for state-carrying exceptions.
+//!
+//! When the kernel sends a state-carrying exception request and the response is
+//! successful (`MACH_MSG_SUCCESS`, a synonym for `KERN_SUCCESS`), it will set
+//! a new thread state based on \a new_state and \a new_state_count. To ease
+//! initialization of the new state, this function copies \a old_state and
+//! \a old_state_count. This is only done if \a behavior indicates a
+//! state-carrying exception.
+//!
+//! \param[in] behavior The behavior of the exception handler as invoked. This
+//!     may be taken directly from the \a behavior parameter of
+//!     internal::SimplifiedExcServer::Interface::CatchException(), for example.
+//! \param[in] old_state The original state value. This may be taken directly
+//!     from the \a old_state parameter of
+//!     internal::SimplifiedExcServer::Interface::CatchException(), for example.
+//! \param[in] old_state_count The number of significant `natural_t` words in \a
+//!     old_state. This may be taken directly from the \a old_state_count
+//!     parameter of internal::SimplifiedExcServer::Interface::CatchException(),
+//!     for example.
+//! \param[out] new_state The state value to be set. This may be taken directly
+//!     from the \a new_state parameter of
+//!     internal::SimplifiedExcServer::Interface::CatchException(), for example.
+//!     This parameter is untouched if \a behavior is not state-carrying.
+//! \param[inout] new_state_count On entry, the number of `natural_t` words
+//!     available to be written to in \a new_state. On return, the number of
+//!     significant `natural_t` words in \a new_state. This may be taken
+//!     directly from the \a new_state_count parameter of
+//!     internal::SimplifiedExcServer::Interface::CatchException(), for example.
+//!     This parameter is untouched if \a behavior is not state-carrying. If \a
+//!     \a behavior is state-carrying, this parameter should be at least as
+//!     large as \a old_state_count.
+void ExcServerCopyState(exception_behavior_t behavior,
+                        const natural_t* old_state,
+                        mach_msg_type_number_t old_state_count,
+                        thread_state_t new_state,
+                        mach_msg_type_number_t* new_state_count);
+
 }  // namespace crashpad
 
 #endif  // CRASHPAD_UTIL_MACH_EXC_SERVER_VARIANTS_H_

util/mach/exc_server_variants_test.cc

@@ -1032,6 +1032,9 @@ class TestExcServerVariants : public MachMultiprocess,
     EXPECT_EQ(REQUESTED_TRAILER_SIZE(kMachMessageOptions),
               trailer->msgh_trailer_size);
 
+    ExcServerCopyState(
+        behavior, old_state, old_state_count, new_state, new_state_count);
+
     return ExcServerSuccessfulReturnValue(behavior, false);
   }
 
@@ -1275,6 +1278,80 @@ TEST(ExcServerVariants, ExcServerSuccessfulReturnValue) {
   }
 }
 
+TEST(ExcServerVariants, ExcServerCopyState) {
+  const natural_t old_state[] = {1, 2, 3, 4, 5};
+  natural_t new_state[10] = {};
+
+  const mach_msg_type_number_t old_state_count = arraysize(old_state);
+  mach_msg_type_number_t new_state_count = arraysize(new_state);
+
+  // EXCEPTION_DEFAULT (with or without MACH_EXCEPTION_CODES) is not
+  // state-carrying. new_state and new_state_count should be untouched.
+  ExcServerCopyState(EXCEPTION_DEFAULT,
+                     old_state,
+                     old_state_count,
+                     new_state,
+                     &new_state_count);
+  EXPECT_EQ(arraysize(new_state), new_state_count);
+  for (size_t i = 0; i < arraysize(new_state); ++i) {
+    EXPECT_EQ(0u, new_state[i]) << "i " << i;
+  }
+
+  ExcServerCopyState(MACH_EXCEPTION_CODES | EXCEPTION_DEFAULT,
+                     old_state,
+                     old_state_count,
+                     new_state,
+                     &new_state_count);
+  EXPECT_EQ(arraysize(new_state), new_state_count);
+  for (size_t i = 0; i < arraysize(new_state); ++i) {
+    EXPECT_EQ(0u, new_state[i]) << "i " << i;
+  }
+
+  // This is a state-carrying exception where old_state_count is small.
+  mach_msg_type_number_t copy_limit = 2;
+  ExcServerCopyState(
+      EXCEPTION_STATE, old_state, copy_limit, new_state, &new_state_count);
+  EXPECT_EQ(copy_limit, new_state_count);
+  for (size_t i = 0; i < copy_limit; ++i) {
+    EXPECT_EQ(old_state[i], new_state[i]) << "i " << i;
+  }
+  for (size_t i = copy_limit; i < arraysize(new_state); ++i) {
+    EXPECT_EQ(0u, new_state[i]) << "i " << i;
+  }
+
+  // This is a state-carrying exception where new_state_count is small.
+  copy_limit = 3;
+  new_state_count = copy_limit;
+  ExcServerCopyState(EXCEPTION_STATE_IDENTITY,
+                     old_state,
+                     old_state_count,
+                     new_state,
+                     &new_state_count);
+  EXPECT_EQ(copy_limit, new_state_count);
+  for (size_t i = 0; i < copy_limit; ++i) {
+    EXPECT_EQ(old_state[i], new_state[i]) << "i " << i;
+  }
+  for (size_t i = copy_limit; i < arraysize(new_state); ++i) {
+    EXPECT_EQ(0u, new_state[i]) << "i " << i;
+  }
+
+  // This is a state-carrying exception where all of old_state is copied to
+  // new_state, which is large enough to receive it and then some.
+  new_state_count = arraysize(new_state);
+  ExcServerCopyState(MACH_EXCEPTION_CODES | EXCEPTION_STATE_IDENTITY,
+                     old_state,
+                     old_state_count,
+                     new_state,
+                     &new_state_count);
+  EXPECT_EQ(old_state_count, new_state_count);
+  for (size_t i = 0; i < arraysize(old_state); ++i) {
+    EXPECT_EQ(old_state[i], new_state[i]) << "i " << i;
+  }
+  for (size_t i = arraysize(old_state); i < arraysize(new_state); ++i) {
+    EXPECT_EQ(0u, new_state[i]) << "i " << i;
+  }
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace crashpad