Add ExcServerSuccessfulReturnValue() and its test.

There’s also some light reformatting in here. Should save a few lines.

TEST=util_test ExcServerVariants.ExcServerSuccessfulReturnValue
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/574753003
This commit is contained in:
Mark Mentovai 2014-09-17 12:08:18 -04:00
parent 63fd3ae47d
commit 57eb311528
3 changed files with 200 additions and 117 deletions

View File

@ -20,6 +20,7 @@
#include "base/basictypes.h"
#include "base/logging.h"
#include "util/mach/exc.h"
#include "util/mach/exception_behaviors.h"
#include "util/mach/excServer.h"
#include "util/mach/mach_exc.h"
#include "util/mach/mach_excServer.h"
@ -247,8 +248,8 @@ bool ExcServer::MachMessageServerFunction(const mach_msg_header_t* in_header,
// in_request_1 is used for the portion of the request after the codes,
// which in theory can be variable-length. The check function will set it.
const Request* in_request_1;
kern_return_t kr = MIGCheckRequestExceptionRaiseState(
in_request, &in_request_1);
kern_return_t kr =
MIGCheckRequestExceptionRaiseState(in_request, &in_request_1);
if (kr != MACH_MSG_SUCCESS) {
SetReplyError(out_header, kr);
return true;
@ -287,8 +288,8 @@ bool ExcServer::MachMessageServerFunction(const mach_msg_header_t* in_header,
// in_request_1 is used for the portion of the request after the codes,
// which in theory can be variable-length. The check function will set it.
const Request* in_request_1;
kern_return_t kr = MIGCheckRequestExceptionRaiseStateIdentity(
in_request, &in_request_1);
kern_return_t kr =
MIGCheckRequestExceptionRaiseStateIdentity(in_request, &in_request_1);
if (kr != MACH_MSG_SUCCESS) {
SetReplyError(out_header, kr);
return true;
@ -382,8 +383,8 @@ bool MachExcServer::MachMessageServerFunction(
// in_request_1 is used for the portion of the request after the codes,
// which in theory can be variable-length. The check function will set it.
const Request* in_request_1;
kern_return_t kr = MIGCheckRequestMachExceptionRaiseState(
in_request, &in_request_1);
kern_return_t kr =
MIGCheckRequestMachExceptionRaiseState(in_request, &in_request_1);
if (kr != MACH_MSG_SUCCESS) {
SetReplyError(out_header, kr);
return true;
@ -722,4 +723,13 @@ kern_return_t UniversalMachExcServer::CatchException(
destroy_complex_request);
}
kern_return_t ExcServerSuccessfulReturnValue(exception_behavior_t behavior,
bool set_thread_state) {
if (!set_thread_state && ExceptionBehaviorHasState(behavior)) {
return MACH_RCV_PORT_DIED;
}
return KERN_SUCCESS;
}
} // namespace crashpad

View File

@ -249,14 +249,13 @@ class SimplifiedExcServer : public ExcServer, public ExcServer::Interface {
// ExcServer::Interface:
virtual kern_return_t CatchExceptionRaise(
exception_handler_t exception_port,
thread_t thread,
task_t task,
exception_type_t exception,
const exception_data_type_t* code,
mach_msg_type_number_t code_count,
bool* destroy_request) override;
virtual kern_return_t CatchExceptionRaise(exception_handler_t exception_port,
thread_t thread,
task_t task,
exception_type_t exception,
const exception_data_type_t* code,
mach_msg_type_number_t code_count,
bool* destroy_request) override;
virtual kern_return_t CatchExceptionRaiseState(
exception_handler_t exception_port,
exception_type_t exception,
@ -411,26 +410,75 @@ class UniversalMachExcServer
// internal::SimplifiedExcServer::Interface:
virtual kern_return_t CatchException(
exception_behavior_t behavior,
exception_handler_t exception_port,
thread_t thread,
task_t task,
exception_type_t exception,
const exception_data_type_t* code,
mach_msg_type_number_t code_count,
thread_state_flavor_t* flavor,
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,
bool* destroy_complex_request) override;
virtual kern_return_t CatchException(exception_behavior_t behavior,
exception_handler_t exception_port,
thread_t thread,
task_t task,
exception_type_t exception,
const exception_data_type_t* code,
mach_msg_type_number_t code_count,
thread_state_flavor_t* flavor,
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,
bool* destroy_complex_request) override;
private:
internal::SimplifiedExcServer exc_server_;
internal::SimplifiedMachExcServer mach_exc_server_;
};
//! \brief Computes an approriate successful return value for an exception
//! handler function.
//!
//! For exception handlers that respond to state-carrying behaviors, when the
//! handler is called by the kernel (as it is normally), the kernel will attempt
//! to set a new thread state when the exception handler returns successfully.
//! Other code that mimics the kernels exception-delivery semantics may
//! implement the same or similar behavior. In some situations, it is
//! undesirable to set a new thread state. If the exception handler were to
//! return unsuccessfully, however, the kernel would continue searching for an
//! exception handler at a wider (task or host) scope. This may also be
//! undesirable.
//!
//! If such exception handlers return `MACH_RCV_PORT_DIED`, the kernel will not
//! set a new thread state and will also not search for another exception
//! handler. See 10.9.4 `xnu-2422.110.17/osfmk/kern/exception.c`.
//! `exception_deliver()` will only set a new thread state if the handlers
//! return code was `MACH_MSG_SUCCESS` (a synonym for `KERN_SUCCESS`), and
//! subsequently, `exception_triage()` will not search for a new handler if the
//! handlers return code was `KERN_SUCCESS` or `MACH_RCV_PORT_DIED`.
//!
//! This function allows exception handlers to compute an appropriate return
//! code to influence their caller (the kernel) in the desired way with respect
//! to setting a new thread state while suppressing the callers subsequent
//! search for other exception handlers. An exception handler should return the
//! value returned by this function.
//!
//! This function is useful even for `EXC_CRASH` handlers, where returning
//! `KERN_SUCCESS` and allowing the kernel to set a new thread state has been
//! observed to cause a perceptible and unnecessary waste of time. The victim
//! task in an `EXC_CRASH` handler is already being terminated and is no longer
//! schedulable, so there is no point in setting the states of any of its
//! threads.
//!
//! \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] set_thread_state `true` if the handler would like its caller to
//! set the new thread state using the \a flavor, \a new_state, and \a
//! new_state_count out parameters. This can only happen when \a behavior is
//! a state-carrying behavior.
//!
//! \return `KERN_SUCCESS` or `MACH_RCV_PORT_DIED`. `KERN_SUCCESS` is used when
//! \a behavior is not a state-carrying behavior, or when it is a
//! state-carrying behavior and \a set_thread_state is `true`.
//! `MACH_RCV_PORT_DIED` is used when \a behavior is a state-carrying
//! behavior and \a set_thread_state is `false`.
kern_return_t ExcServerSuccessfulReturnValue(exception_behavior_t behavior,
bool set_thread_state);
} // namespace crashpad
#endif // CRASHPAD_UTIL_MACH_EXC_SERVER_VARIANTS_H_

View File

@ -556,17 +556,16 @@ TEST(ExcServerVariants, MockExceptionRaise) {
const exception_behavior_t kExceptionBehavior = EXCEPTION_DEFAULT;
EXPECT_CALL(server,
MockCatchMachException(
kExceptionBehavior,
kServerLocalPort,
kExceptionThreadPort,
kExceptionTaskPort,
kExceptionType,
AreExceptionCodes(
kTestExceptonCodes[0], kTestExceptonCodes[1]),
Pointee(Eq(THREAD_STATE_NONE)),
IsThreadStateCount(0u),
IsThreadStateCount(0u)))
MockCatchMachException(kExceptionBehavior,
kServerLocalPort,
kExceptionThreadPort,
kExceptionTaskPort,
kExceptionType,
AreExceptionCodes(kTestExceptonCodes[0],
kTestExceptonCodes[1]),
Pointee(Eq(THREAD_STATE_NONE)),
IsThreadStateCount(0u),
IsThreadStateCount(0u)))
.WillOnce(Return(KERN_SUCCESS))
.RetiresOnSaturation();
@ -595,18 +594,18 @@ TEST(ExcServerVariants, MockExceptionRaiseState) {
const exception_behavior_t kExceptionBehavior = EXCEPTION_STATE;
EXPECT_CALL(server,
MockCatchMachException(
kExceptionBehavior,
kServerLocalPort,
MACH_PORT_NULL,
MACH_PORT_NULL,
kExceptionType,
AreExceptionCodes(
kTestExceptonCodes[0], kTestExceptonCodes[1]),
Pointee(Eq(kThreadStateFlavor)),
IsThreadStateCount(kThreadStateFlavorCount),
IsThreadStateCount(arraysize(reply.new_state))))
EXPECT_CALL(
server,
MockCatchMachException(
kExceptionBehavior,
kServerLocalPort,
MACH_PORT_NULL,
MACH_PORT_NULL,
kExceptionType,
AreExceptionCodes(kTestExceptonCodes[0], kTestExceptonCodes[1]),
Pointee(Eq(kThreadStateFlavor)),
IsThreadStateCount(kThreadStateFlavorCount),
IsThreadStateCount(arraysize(reply.new_state))))
.WillOnce(Return(KERN_SUCCESS))
.RetiresOnSaturation();
@ -638,18 +637,18 @@ TEST(ExcServerVariants, MockExceptionRaiseStateIdentity) {
const exception_behavior_t kExceptionBehavior = EXCEPTION_STATE_IDENTITY;
EXPECT_CALL(server,
MockCatchMachException(
kExceptionBehavior,
kServerLocalPort,
kExceptionThreadPort,
kExceptionTaskPort,
kExceptionType,
AreExceptionCodes(
kTestExceptonCodes[0], kTestExceptonCodes[1]),
Pointee(Eq(kThreadStateFlavor)),
IsThreadStateCount(kThreadStateFlavorCount),
IsThreadStateCount(arraysize(reply.new_state))))
EXPECT_CALL(
server,
MockCatchMachException(
kExceptionBehavior,
kServerLocalPort,
kExceptionThreadPort,
kExceptionTaskPort,
kExceptionType,
AreExceptionCodes(kTestExceptonCodes[0], kTestExceptonCodes[1]),
Pointee(Eq(kThreadStateFlavor)),
IsThreadStateCount(kThreadStateFlavorCount),
IsThreadStateCount(arraysize(reply.new_state))))
.WillOnce(Return(KERN_SUCCESS))
.RetiresOnSaturation();
@ -681,17 +680,16 @@ TEST(ExcServerVariants, MockMachExceptionRaise) {
EXPECT_CALL(
server,
MockCatchMachException(
kExceptionBehavior,
kServerLocalPort,
kExceptionThreadPort,
kExceptionTaskPort,
kExceptionType,
AreExceptionCodes(
kTestMachExceptionCodes[0], kTestMachExceptionCodes[1]),
Pointee(Eq(THREAD_STATE_NONE)),
IsThreadStateCount(0u),
IsThreadStateCount(0u)))
MockCatchMachException(kExceptionBehavior,
kServerLocalPort,
kExceptionThreadPort,
kExceptionTaskPort,
kExceptionType,
AreExceptionCodes(kTestMachExceptionCodes[0],
kTestMachExceptionCodes[1]),
Pointee(Eq(THREAD_STATE_NONE)),
IsThreadStateCount(0u),
IsThreadStateCount(0u)))
.WillOnce(Return(KERN_SUCCESS))
.RetiresOnSaturation();
@ -723,17 +721,16 @@ TEST(ExcServerVariants, MockMachExceptionRaiseState) {
EXPECT_CALL(
server,
MockCatchMachException(
kExceptionBehavior,
kServerLocalPort,
MACH_PORT_NULL,
MACH_PORT_NULL,
kExceptionType,
AreExceptionCodes(
kTestMachExceptionCodes[0], kTestMachExceptionCodes[1]),
Pointee(Eq(kThreadStateFlavor)),
IsThreadStateCount(kThreadStateFlavorCount),
IsThreadStateCount(arraysize(reply.new_state))))
MockCatchMachException(kExceptionBehavior,
kServerLocalPort,
MACH_PORT_NULL,
MACH_PORT_NULL,
kExceptionType,
AreExceptionCodes(kTestMachExceptionCodes[0],
kTestMachExceptionCodes[1]),
Pointee(Eq(kThreadStateFlavor)),
IsThreadStateCount(kThreadStateFlavorCount),
IsThreadStateCount(arraysize(reply.new_state))))
.WillOnce(Return(KERN_SUCCESS))
.RetiresOnSaturation();
@ -768,17 +765,16 @@ TEST(ExcServerVariants, MockMachExceptionRaiseStateIdentity) {
EXPECT_CALL(
server,
MockCatchMachException(
kExceptionBehavior,
kServerLocalPort,
kExceptionThreadPort,
kExceptionTaskPort,
kExceptionType,
AreExceptionCodes(
kTestMachExceptionCodes[0], kTestMachExceptionCodes[1]),
Pointee(Eq(kThreadStateFlavor)),
IsThreadStateCount(kThreadStateFlavorCount),
IsThreadStateCount(arraysize(reply.new_state))))
MockCatchMachException(kExceptionBehavior,
kServerLocalPort,
kExceptionThreadPort,
kExceptionTaskPort,
kExceptionType,
AreExceptionCodes(kTestMachExceptionCodes[0],
kTestMachExceptionCodes[1]),
Pointee(Eq(kThreadStateFlavor)),
IsThreadStateCount(kThreadStateFlavorCount),
IsThreadStateCount(arraysize(reply.new_state))))
.WillOnce(Return(KERN_SUCCESS))
.RetiresOnSaturation();
@ -870,8 +866,7 @@ class TestExcServerVariants : public UniversalMachExcServer,
behavior_(behavior),
flavor_(flavor),
state_count_(state_count),
handled_(false) {
}
handled_(false) {}
// UniversalMachExcServer:
@ -936,14 +931,7 @@ class TestExcServerVariants : public UniversalMachExcServer,
EXPECT_EQ(NULL, new_state);
}
// Even for an EXC_CRASH handler, returning KERN_SUCCESS with a
// state-carrying reply will cause the kernel to try to set a new thread
// state, leading to a perceptible waste of time. Returning
// MACH_RCV_PORT_DIED is the only way to suppress this behavior while also
// preventing the kernel from looking for another (host-level) EXC_CRASH
// handler. See 10.9.4 xnu-2422.110.17/osfmk/kern/exception.c
// exception_triage().
return has_state ? MACH_RCV_PORT_DIED : KERN_SUCCESS;
return ExcServerSuccessfulReturnValue(behavior, false);
}
private:
@ -1057,18 +1045,18 @@ TEST(ExcServerVariants, ThreadStates) {
// Additionaly, the AVX state flavors are also not tested because theyre
// not available on all CPUs and OS versions.
#if defined(ARCH_CPU_X86)
{ x86_THREAD_STATE32, x86_THREAD_STATE32_COUNT },
{ x86_FLOAT_STATE32, x86_FLOAT_STATE32_COUNT },
{ x86_EXCEPTION_STATE32, x86_EXCEPTION_STATE32_COUNT },
{x86_THREAD_STATE32, x86_THREAD_STATE32_COUNT},
{x86_FLOAT_STATE32, x86_FLOAT_STATE32_COUNT},
{x86_EXCEPTION_STATE32, x86_EXCEPTION_STATE32_COUNT},
#endif
#if defined(ARCH_CPU_X86_64)
{ x86_THREAD_STATE64, x86_THREAD_STATE64_COUNT },
{ x86_FLOAT_STATE64, x86_FLOAT_STATE64_COUNT },
{ x86_EXCEPTION_STATE64, x86_EXCEPTION_STATE64_COUNT },
{x86_THREAD_STATE64, x86_THREAD_STATE64_COUNT},
{x86_FLOAT_STATE64, x86_FLOAT_STATE64_COUNT},
{x86_EXCEPTION_STATE64, x86_EXCEPTION_STATE64_COUNT},
#endif
{ x86_THREAD_STATE, x86_THREAD_STATE_COUNT },
{ x86_FLOAT_STATE, x86_FLOAT_STATE_COUNT },
{ x86_EXCEPTION_STATE, x86_EXCEPTION_STATE_COUNT },
{x86_THREAD_STATE, x86_THREAD_STATE_COUNT},
{x86_FLOAT_STATE, x86_FLOAT_STATE_COUNT},
{x86_EXCEPTION_STATE, x86_EXCEPTION_STATE_COUNT},
#else
#error Port this test to your CPU architecture.
#endif
@ -1076,8 +1064,8 @@ TEST(ExcServerVariants, ThreadStates) {
for (size_t index = 0; index < arraysize(test_data); ++index) {
const TestData& test = test_data[index];
SCOPED_TRACE(base::StringPrintf(
"index %zu, flavor %d", index, test.flavor));
SCOPED_TRACE(
base::StringPrintf("index %zu, flavor %d", index, test.flavor));
TestExcServerVariants test_exc_server_variants(
MACH_EXCEPTION_CODES | EXCEPTION_STATE_IDENTITY,
@ -1087,4 +1075,41 @@ TEST(ExcServerVariants, ThreadStates) {
}
}
TEST(ExcServerVariants, ExcServerSuccessfulReturnValue) {
struct TestData {
exception_behavior_t behavior;
bool set_thread_state;
kern_return_t kr;
};
const TestData kTestData[] = {
{EXCEPTION_DEFAULT, false, KERN_SUCCESS},
{EXCEPTION_STATE, false, MACH_RCV_PORT_DIED},
{EXCEPTION_STATE_IDENTITY, false, MACH_RCV_PORT_DIED},
{kMachExceptionCodes | EXCEPTION_DEFAULT, false, KERN_SUCCESS},
{kMachExceptionCodes | EXCEPTION_STATE, false, MACH_RCV_PORT_DIED},
{kMachExceptionCodes | EXCEPTION_STATE_IDENTITY,
false,
MACH_RCV_PORT_DIED},
{EXCEPTION_DEFAULT, true, KERN_SUCCESS},
{EXCEPTION_STATE, true, KERN_SUCCESS},
{EXCEPTION_STATE_IDENTITY, true, KERN_SUCCESS},
{kMachExceptionCodes | EXCEPTION_DEFAULT, true, KERN_SUCCESS},
{kMachExceptionCodes | EXCEPTION_STATE, true, KERN_SUCCESS},
{kMachExceptionCodes | EXCEPTION_STATE_IDENTITY, true, KERN_SUCCESS},
};
for (size_t index = 0; index < arraysize(kTestData); ++index) {
const TestData& test_data = kTestData[index];
SCOPED_TRACE(
base::StringPrintf("index %zu, behavior %d, set_thread_state %s",
index,
test_data.behavior,
test_data.set_thread_state ? "true" : "false"));
EXPECT_EQ(test_data.kr,
ExcServerSuccessfulReturnValue(test_data.behavior,
test_data.set_thread_state));
}
}
} // namespace