From b4fe6dfae04961633e2ffd16fafcdddd8df3ee32 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Wed, 17 Sep 2014 17:59:35 -0400 Subject: [PATCH] Refactor EXC_CRASH code[0] processing into a new function, ExcCrashRecoverOriginalException(), and use it where sensible. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TEST=util_test ExcVariantsTest.ExcCrashRecoverOriginalException … R=rsesek@chromium.org Review URL: https://codereview.chromium.org/566693003 --- tools/catch_exception_tool.cc | 20 +++----- util/mach/exc_server_variants.cc | 27 ++++++++++ util/mach/exc_server_variants.h | 34 +++++++++++++ util/mach/exc_server_variants_test.cc | 72 +++++++++++++++++++++++---- util/mach/exception_ports_test.cc | 18 +++---- 5 files changed, 140 insertions(+), 31 deletions(-) diff --git a/tools/catch_exception_tool.cc b/tools/catch_exception_tool.cc index 2cc547c7..69447f75 100644 --- a/tools/catch_exception_tool.cc +++ b/tools/catch_exception_tool.cc @@ -117,20 +117,16 @@ class ExceptionServer : public UniversalMachExcServer { } if (exception == EXC_CRASH) { - // 10.9.4 xnu-2422.110.17/bsd/kern/kern_exit.c proc_prepareexit() sets - // code[0] based on the signal value, original exception type, and low 20 - // bits of the original code[0] before calling - // xnu-2422.110.17/osfmk/kern/exception.c task_exception_notify() to raise - // an EXC_CRASH. - int sig = (code[0] >> 24) & 0xff; - int orig_exception = (code[0] >> 20) & 0xf; - int orig_code_0 = code[0] & 0xfffff; + mach_exception_data_type_t original_code_0; + int signal; + exception_type_t original_exception = + ExcCrashRecoverOriginalException(code[0], &original_code_0, &signal); fprintf(options_.file, - ", original exception %s, original code0 %d, signal %s", - ExceptionToString(orig_exception, + ", original exception %s, original code[0] %lld, signal %s", + ExceptionToString(original_exception, kUseFullName | kUnknownIsNumeric).c_str(), - orig_code_0, - SignalToString(sig, kUseFullName | kUnknownIsNumeric).c_str()); + original_code_0, + SignalToString(signal, kUseFullName | kUnknownIsNumeric).c_str()); } fprintf(options_.file, "\n"); diff --git a/util/mach/exc_server_variants.cc b/util/mach/exc_server_variants.cc index 46e85882..fd653b42 100644 --- a/util/mach/exc_server_variants.cc +++ b/util/mach/exc_server_variants.cc @@ -723,6 +723,33 @@ kern_return_t UniversalMachExcServer::CatchException( destroy_complex_request); } +exception_type_t ExcCrashRecoverOriginalException( + mach_exception_data_type_t code_0, + mach_exception_data_type_t* original_code_0, + int* signal) { + // 10.9.4 xnu-2422.110.17/bsd/kern/kern_exit.c proc_prepareexit() sets code[0] + // based on the signal value, original exception type, and low 20 bits of the + // original code[0] before calling xnu-2422.110.17/osfmk/kern/exception.c + // task_exception_notify() to raise an EXC_CRASH. + // + // The list of core-generating signals (as used in proc_prepareexit()’s call + // to hassigprop()) is in 10.9.4 xnu-2422.110.17/bsd/sys/signalvar.h sigprop: + // entires with SA_CORE are in the set. These signals are SIGQUIT, SIGILL, + // SIGTRAP, SIGABRT, SIGEMT, SIGFPE, SIGBUS, SIGSEGV, and SIGSYS. Processes + // killed for code-signing reasons will be killed by SIGKILL and are also + // eligible for EXC_CRASH handling, but processes killed by SIGKILL for other + // reasons are not. + if (signal) { + *signal = (code_0 >> 24) & 0xff; + } + + if (original_code_0) { + *original_code_0 = code_0 & 0xfffff; + } + + return (code_0 >> 20) & 0xf; +} + kern_return_t ExcServerSuccessfulReturnValue(exception_behavior_t behavior, bool set_thread_state) { if (!set_thread_state && ExceptionBehaviorHasState(behavior)) { diff --git a/util/mach/exc_server_variants.h b/util/mach/exc_server_variants.h index d605869a..3719b525 100644 --- a/util/mach/exc_server_variants.h +++ b/util/mach/exc_server_variants.h @@ -429,6 +429,40 @@ class UniversalMachExcServer internal::SimplifiedMachExcServer mach_exc_server_; }; +//! \brief Recovers the original exception, first exception code, and signal +//! from the encoded form of the first exception code delivered with +//! `EXC_CRASH` exceptions. +//! +//! `EXC_CRASH` exceptions are generated when the kernel has committed to +//! terminating a process as a result of a core-generating POSIX signal and, for +//! hardware exceptions, an earlier Mach exception. Information about this +//! earlier exception and signal is made available to the `EXC_CRASH` handler +//! via its `code[0]` parameter. This function recovers the original exception, +//! the value of `code[0]` from the original exception, and the value of the +//! signal responsible for process termination. +//! +//! \param[in] code_0 The first exception code (`code[0]`) passed to a Mach +//! exception handler in an `EXC_CRASH` exception. It is invalid to call +//! this function with an exception code from any exception other than +//! `EXC_CRASH`. +//! \param[out] original_code_0 The first exception code (`code[0]`) passed to +//! the Mach exception handler for a hardware exception that resulted in the +//! generation of a POSIX signal that caused process termination. If the +//! signal that caused termination was not sent as a result of a hardware +//! exception, this will be `0`. Callers that do not need this value may +//! pass `NULL`. +//! \param[out] signal The POSIX signal that caused process termination. Callers +//! that do not need this value may pass `NULL`. +//! +//! \return The original exception for a hardware exception that resulted in the +//! generation of a POSIX signal that caused process termination. If the +//! signal that caused termination was not sent as a result of a hardware +//! exception, this will be `0`. +exception_type_t ExcCrashRecoverOriginalException( + mach_exception_data_type_t code_0, + mach_exception_data_type_t* original_code_0, + int* signal); + //! \brief Computes an approriate successful return value for an exception //! handler function. //! diff --git a/util/mach/exc_server_variants_test.cc b/util/mach/exc_server_variants_test.cc index 8ca041eb..36439f48 100644 --- a/util/mach/exc_server_variants_test.cc +++ b/util/mach/exc_server_variants_test.cc @@ -15,6 +15,7 @@ #include "util/mach/exc_server_variants.h" #include +#include #include #include "base/basictypes.h" @@ -904,15 +905,13 @@ class TestExcServerVariants : public UniversalMachExcServer, EXPECT_EQ(EXC_CRASH, exception); EXPECT_EQ(2u, code_count); - // The code_count check above would ideally use ASSERT_EQ so that the next - // conditional would not be necessary, but ASSERT_* requires a function - // returning type void, and the interface dictates otherwise here. - if (code_count >= 1) { - // The signal that terminated the process is stored in code[0] along with - // some other data. See 10.9.4 xnu-2422.110.17/bsd/kern/kern_exit.c - // proc_prepareexit(). - int sig = (code[0] >> 24) & 0xff; - SetExpectedChildTermination(kTerminationSignal, sig); + // The exception and code_count checks above would ideally use ASSERT_EQ so + // that the next conditional would not be necessary, but ASSERT_* requires a + // function returning type void, and the interface dictates otherwise here. + if (exception == EXC_CRASH && code_count >= 1) { + int signal; + ExcCrashRecoverOriginalException(code[0], NULL, &signal); + SetExpectedChildTermination(kTerminationSignal, signal); } const bool has_state = ExceptionBehaviorHasState(behavior); @@ -1075,6 +1074,61 @@ TEST(ExcServerVariants, ThreadStates) { } } +TEST(ExcServerVariants, ExcCrashRecoverOriginalException) { + struct TestData { + mach_exception_data_type_t code_0; + exception_type_t exception; + mach_exception_data_type_t original_code_0; + int signal; + }; + const TestData kTestData[] = { + {0xb100001, EXC_BAD_ACCESS, KERN_INVALID_ADDRESS, SIGSEGV}, + {0xb100002, EXC_BAD_ACCESS, KERN_PROTECTION_FAILURE, SIGSEGV}, + {0xa100002, EXC_BAD_ACCESS, KERN_PROTECTION_FAILURE, SIGBUS}, + {0x4200001, EXC_BAD_INSTRUCTION, 1, SIGILL}, + {0x8300001, EXC_ARITHMETIC, 1, SIGFPE}, + {0x5600002, EXC_BREAKPOINT, 2, SIGTRAP}, + {0x6000000, 0, 0, SIGABRT}, + {0, 0, 0, 0}, + }; + + for (size_t index = 0; index < arraysize(kTestData); ++index) { + const TestData& test_data = kTestData[index]; + SCOPED_TRACE(base::StringPrintf("index %zu, code_0 0x%llx", + index, + test_data.code_0)); + + mach_exception_data_type_t original_code_0; + int signal; + exception_type_t exception = + ExcCrashRecoverOriginalException(test_data.code_0, + &original_code_0, + &signal); + + EXPECT_EQ(test_data.exception, exception); + EXPECT_EQ(test_data.original_code_0, original_code_0); + EXPECT_EQ(test_data.signal, signal); + } + + // Now make sure that ExcCrashRecoverOriginalException() properly ignores + // optional arguments. + COMPILE_ASSERT(arraysize(kTestData) >= 1, must_have_something_to_test); + const TestData& test_data = kTestData[0]; + EXPECT_EQ(test_data.exception, + ExcCrashRecoverOriginalException(test_data.code_0, NULL, NULL)); + + mach_exception_data_type_t original_code_0; + EXPECT_EQ(test_data.exception, + ExcCrashRecoverOriginalException( + test_data.code_0, &original_code_0, NULL)); + EXPECT_EQ(test_data.original_code_0, original_code_0); + + int signal; + EXPECT_EQ(test_data.exception, + ExcCrashRecoverOriginalException(test_data.code_0, NULL, &signal)); + EXPECT_EQ(test_data.signal, signal); +} + TEST(ExcServerVariants, ExcServerSuccessfulReturnValue) { struct TestData { exception_behavior_t behavior; diff --git a/util/mach/exception_ports_test.cc b/util/mach/exception_ports_test.cc index d549651c..7da1d56d 100644 --- a/util/mach/exception_ports_test.cc +++ b/util/mach/exception_ports_test.cc @@ -183,21 +183,19 @@ class TestExceptionPorts : public UniversalMachExcServer, EXPECT_EQ(EXC_CRASH, exception); EXPECT_EQ(2u, code_count); - // The code_count check above would ideally use ASSERT_EQ so that the next - // conditional would not be necessary, but ASSERT_* requires a function - // returning type void, and the interface dictates otherwise here. - if (code_count >= 1) { - // The signal that terminated the process is stored in code[0] along with - // some other data. See 10.9.4 xnu-2422.110.17/bsd/kern/kern_exit.c - // proc_prepareexit(). - int sig = (code[0] >> 24) & 0xff; + // The exception and code_count checks above would ideally use ASSERT_EQ so + // that the next conditional would not be necessary, but ASSERT_* requires a + // function returning type void, and the interface dictates otherwise here. + if (exception == EXC_CRASH && code_count >= 1) { + int signal; + ExcCrashRecoverOriginalException(code[0], NULL, &signal); // The child crashed with a division by zero, which shows up as SIGFPE. // This was chosen because it’s unlikely to be generated by testing or // assertion failures. - EXPECT_EQ(SIGFPE, sig); + EXPECT_EQ(SIGFPE, signal); - SetExpectedChildTermination(kTerminationSignal, sig); + SetExpectedChildTermination(kTerminationSignal, signal); } // Even for an EXC_CRASH handler, returning KERN_SUCCESS with a