The implementations for the exc and mach_exc subsystems were nearly
identical, and were a good target for templatization. The existing
split between exc and mach_exc was a good candidate for unification
based on CompositeMachMessageServer instead of the custom unification
previously done in UniversalMachMessageServer.
TEST=util_test ExcServerVariants.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/766193006
MachMessageServer::Run()’s distinct |nonblocking| parameter is removed.
The information it formerly conveyed is now implied by the |timeout_ms|
parameter, which can accept two special values,
kMachMessageTimeoutNonblocking and kMachMessageTimeoutWaitIndefinitely.
TEST=client_test, snapshot_test, util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/777993002
UniversalMachExcServer provided both an interface and an implementation,
contrary to the other classes in the exc_server_variants family. This
was mostly done for reasons of economy in an already-large class family.
Unfortunately, this decision meant that it was impossible for other code
to use UniversalMachExcServer, which required that CatchMachException()
be implemented, and also extend another class without violating the
style guide’s prohibition of multiple implementation inheritance. This
became a problem in a lot of test code, which extended MachMultiprocess
and UniversalMachExcServer.
UniversalMachExcServer is now given its own nested Interface class,
which is a pure interface. All users of UniversalMachExcServer are
changed from “is-a” UniversalMachExcServer to “has-a”
UniversalMachExcServer and “is-a” UniversalMachExcServer::Interface.
TEST=client_test, snapshot_test, util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/775943005
This exposed a bug in the ExcClientVariants test, which was expecting
the memory used for new_state to be initialized with zeroes. In reality,
no guarantee of initialization is made. MIG “out” parameters are
strictly “out” and may contain garbage at function entry.
TEST=util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/779633004
The F_SETNOSIGPIPE fcntl() command is not available on 10.6. Use
socketpair() instead of pipe(), so that the SO_NOSIGPIPE socket option
can be used.
TEST=util_test ChildPortHandshake.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/777573002
MachMessageWithDeadline() is a mach_msg() wrapper that deals with
deadlines instead of timeouts. It is a slight simplification of the
mach_msg() interface because the deadline parameter implies the timeout
option bits, and because the caller does not need to specify send_size
during sends as the message itself already carries this information.
TEST=util_test MachMessage.MachMessageDeadlineFromTimeout
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/773943002
A subsequent change will add MachMessageWithDeadline(), a mach_msg()
wrapper. Conceptually, it makes sense to include that function in this
file family. Since this file family now contains a mach_msg() wrapper,
it makes sense to rename it mach_message and lose the _util suffix.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/772133004
MachMessageServer was wasteful with allocations for request and reply
messages. It allocated new memory for each request receive and for each
reply send, and if it needed to resize an allocation for a request, it
would maintain two request allocations simultaneously. The new behavior
allocates memory for a new request only if it needs a different size
than for the previous request, and it never maintains two request
allocations simultaneously. Memory for a reply is allocated once per
method invocation and maintained, since this never needs to be resized.
One pass of the loop is now guaranteed, even if a caller specifies a
very small timeout that expires before attempting to receive a message.
An infinite looping bug that could occur when ignoring large messages
has also been fixed.
TEST=util_test MachMessageServer.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/759023004
The buffer sizing logic was correct to start with. I don’t know why I
misread it. It should say “if this would resize to receive a large
message, use the entire allocation rounded up to full page size,
otherwise, only use the space expected for a message.”
TEST=util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/760573003
These port dispositions were naïvely taken from excUser.c and
mach_excUser.c, but the local and remote portions were not swapped as
they would be upon receipt in a server. This swaps them to match how
they’d be visible in a server, and uses the port disposition name
aliases expected to be used in servers: MACH_MSG_TYPE_PORT_* instead of
MACH_MSG_TYPE_{MAKE,COPY,MOVE}_*.
TEST=util_test ExcServerVariants.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/755323002
The existing implementation used the same logic as is found in
mach_msg_server(), but that logic seems incorrect. When the caller wants
to retry a mach_msg() receive of a too-large message that returns
MACH_RCV_TOO_LARGE, there’s no harm in attempting the receive with a
larger buffer initially. On the other hand, if the caller does not want
to retry such mach_msg() receive attempts, it’s an indication that the
caller is expecting to be intolerant of too-large messages, and there’s
no need to attempt the receive with a buffer any larger than requested.
TEST=util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/753363003
As documented, MACH_SEND_TRAILER would allow a sender to provide its own
message trailer instead of having the kernel append its own
kernel-generated trailer. This is a Mach feature that supports a network
of multiple Mach hosts, but even in that environment, the option is
restricted to use by privileged callers. In reality, MACH_SEND_TRAILER
has never been implemented in OS X.
The system’s mach_msg_server() family does consider the value of
MACH_SEND_TRAILER, but this is pointless. Any purported trailer set by a
server function would be ignored.
Maintaining this code gives the illusion that it’s functional, so it’s
being removed.
TEST=util_test MachMessageServer.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/736493007
ChildPortHandshake is the most generic system yet to allow child
processes to provide their parents with Mach rights. These are
ordinarily expected to be send rights to the children’s own task ports,
or send rights to servers that the children hold receive rights to.
This updates DEPS to pull mini_chromium 1d3523dbda93, which includes
base::mac::ScopedMachPortSet.
TEST=util_test ChildPortHandshake.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/756603003
Previously, MachMessageServer::Run() only provided two strategies for
dealing with large messages, indicated by mach_msg() returning
MACH_RCV_TOO_LARGE: the receive buffer could be reallocated and the
message received, or the entire function could return MACH_RCV_TOO_LARGE
to the caller. There are situations where an intermediate behavior might
be desirable. This intermediate behavior would allow the function to
continue waiting for another message without returning an error to the
caller or attempting to receive the large message. This is desirable
when dealing with fixed-sized messages and a receiver that might be sent
messages by unknown, possibly-malicious callers. This can happen when
the corresponding send right is published with the bootstrap server, for
example.
Existing users continue to request their existing behavior, typically
receiving an error when encountering a large message.
catch_exception_tool will use the new “ignore” behavior when running in
persistent mode.
TEST=util_test MachMessageServer.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/756803002
the child_port subsystem.
Common routines shared with the ExcServer family of classes have been
moved to a new file, where they can be shared between different
MachMessageServer::Interface implementations.
TEST=util_test ChildPortServer.*:MachMessageUtil.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/754123002
DropPrivileges() is used in exception_port_tool, so that when it is
installed as a setuid executable, it only uses elevated privileges to
obtain a task port for its -p option, and then relinquishes those
privileges.
It is difficult to provide a test for this function, because it must be
running setuid or setgid in order to do anything interesting. However,
the function contains its own CHECKs to verify that it behaves properly.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/727053002
This also transitions exception_port_tool to use TaskForPID(), so that
it can be safely used as a setuid executable without giving permission
to operate on any process on the system.
It is difficult to provide a test for this function, because it must be
running setuid root in order to do anything interesting.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/728973002
Also, move ProcessArgumentsForPID() into ProcessInfo.
This change prepares for a TaskForPID() implementation that’s capable of
operating correctly in a setuid root executable. TaskForPID() belongs in
util/mach, but for its permission checks, it must access some process
properties that were previously fetched by ProcessReader in snapshot.
util can’t depend on snapshot. The generic util-safe process information
bits (Is64Bit(), ProcessID(), ParentProcessID(), and StartTime()) are
moved from ProcessReader to ProcessInfo (in util), where the current
ProcessReader can use it (as it’s OK for snapshot to depend on util),
and the future TaskForPID() in util can also use it. ProcessInfo also
contains other methods that TaskForPID() will use, providing access to
the credentials that the target process holds. ProcessArgumentsForPID()
is related, and is also now a part of ProcessInfo.
TEST=snapshot_test, util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/727973002
implicit_cast<> only performs a cast in cases where an implicit
conversion would be possible. It’s even safer than static_cast<> It’s an
“explicit implicit” cast, which is not normally necsesary, but is
frequently required when working with the ?: operator, functions like
std::min() and std::max(), and logging and testing macros.
The public style guide does not mention implicit_cast<> only because it
is not part of the standard library, but would otherwise require it in
these situations. Since base does provide implicit_cast<>, it should be
used whenever possible.
The only uses of static_cast<> not converted to implicit_cast<> are
those that require static_cast<>, such as those that assign an integer
constant to a variable of an enum type.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/700383007
This only came up in one location, PointerVector.
A template alias is superior to inheritance, which doesn’t provide full
type equivalence and doesn’t automatically inherit non-default
constructors.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/683753005
Not doing this causes gtest to issue these warnings:
[WARNING] ../../third_party/gtest/gtest/src/gtest-death-test.cc:825::
Death tests use fork(), which is unsafe particularly in a threaded
context. For this test, Google Test detected 4 threads.
The gtest documentation recommends giving the test case a name ending in
DeathTest. Test cases named according to this convention run before all
other tests. Other death tests in Crashpad also follow this convention.
https://code.google.com/p/googletest/wiki/AdvancedGuide#Death_Tests_And_Threads
TEST=util_test HTTPMultipartBuilderDeathTest.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/694963002
This change was generated mechanically by running:
find . \( -name \*.cc -or -name \*.mm -or -name \*.h \) \
-and -not -path ./third_party/\* -and -not -path ./out/\* \
-exec sed -i '' -E -e 's/(^|[^_])NULL/\1nullptr/g' {} +
Further manual fix-ups were applied to remove casts of nullptr to other
pointer types where possible, to preserve the intentional use of NULL
(as a short form of MACH_PORT_NULL) in exception_port_tool, and to fix
80-column violations.
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/4mijeJHzxLg/discussion
TEST=*_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/656703002
This uses THREAD_NULL, TASK_NULL, and HOST_NULL in preference to
MACH_PORT_NULL and kMachPortNull. These constants are correctly-typed
(thread_t, task_t, and host_t) and result in more readable source code,
especially where thread and task parameters appear together as they do
in exc_*_variants.
TEST=util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/649713002
amount of time spent sleeping.
Even with the slop, this wound up being flaky, specifically on virtual
machines. And guess what our automated test infrastructure runs on?
TEST=util_test Clock.SleepNanoseconds
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/640373003
ExpectProcessIsRunning() goes to a lot of trouble to deal with the flaky
{CTL_KERN, KERN_PROCARGS2} API exposed by ProcessArgumentsForPID(). It
retries that call inside an inner loop until it succeeds, because it may
fail while a process is mid-exec(). Once it succeeds, it should use the
argv it obtained. Waiting for success and then attempting to call
ProcessArgumentsForPID() again to get argv can be flaky when the first
attempt succeeds because it’s pre-exec(), and the second one catches the
process in the middle of the exec() and fails.
The existence of the inner loop was intended to protect against this
problem, and the subsequent call outside of the inner loop appears to be
left behind from before the inner loop existed. It should have been
removed when the inner loop was added.
TEST=util_test ServiceManagement.SubmitRemoveJob
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/644473005
7dda7b322881 accidentally moved the libbsm.dylib dependency from
util_test_lib to util_test. It is only used by
test/mac/mach_multiprocess.cc in util_test_lib.
Meanwhile, util_test should link against Foundation.framework, which is
used by three tests in util/mac: launchd_test.mm, mac_util_test.mm, and
service_management_test.mm.
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/612923004
This includes ClockMonotonicNanoseconds() and SleepNanoseconds().
SleepNanoseconds() is like base::PlatformThread::Sleep(), but
PlatformThread is not in mini_chromium and I’m not keen on adding it
because I’m not sold on the interface. I’m not convinced Sleep() belongs
there, and I don’t want to have to bring all of base::Time* along for
the ride.
TEST=util_test Clock.*:MachMessageServer.*:ServiceManagement.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/597533002
This will be used in the upcoming Minidump context structures.
This also updates mini_chromium to ba9b15f1b6a7 to pick up:
ba9b15f1b6a7 Make bit_cast available in C++11 mode.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/583283003
The queue length of a new receive port appears to be 2 on Mac OS X 10.10
DP8 14A361c. The value of MACH_PORT_QLIMIT_DEFAULT in the 10.10 SDK is
still 5, so a read of the kernel source should be interesting, if we
ever get to see it.
In the meantime, mach_port_set_attributes() can be used to set a
traditional queue length.
TEST=util_test MachMessageServer.PersistentNonblockingFourMessages
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/584293003
This reverts commit 63fd3ae47d2513ef4390c3d8c0dde3fa95152480.
SMJobRemove() still performs a 1-second sleep() when its |wait|
parameter is true. Radar 18398683 is filed. In the mean time, the
launchd-based implementation of these functions does not have this
undesirable property.
The comments have been revised to not reflect operating systems before
Mac OS X 10.6. Comments describing bugs in earlier 10.10 seeds that have
since been fixed have been removed.
TEST=util_test Launchd.*:ServiceManagement.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/579083003
On 10.6, the main executable does not show up at index 0, but appears
elsewhere in the list. Modules are now scanned to ensure that the
MH_EXECUTE one is first in the list. This means that ProcessReader is
now responsible for creating a MachOImageReader object for each module,
rather than having its callers perform that task.
TEST=util_test MachOImageReader.*:ProcessReader.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/586123002
elements whose handler port would be MACH_PORT_NULL.
For most exception targets, *_get_exception_ports() will normally return
an exception port of MACH_PORT_NULL when no handler is registered.
However, as of Mac OS X 10.9, thread_get_exception_ports() will return
an empty list when no handler is registered for any exception type on a
thread.
Consequently, a caller would have to do additional processing to
determine whether a specific exception port is registered: an
unregistered port will either appear but have a handler port of
MACH_PORT_NULL, or it will not appear at all. This is confusing for
callers. The behaviors are unified, and when a handler port of
MACH_PORT_NULL is found, it will not be returned to the caller. This is
expected to be the simpler of the two possible behaviors for callers to
make use of.
The change in the kernel can be seen by comparing 10.8.5
xnu-2050.48.11/osfmk/kern/ipc_tt.c thread_get_exception_ports() to the
same function in 10.9.4 xnu-2422.110.17. The 10.9 version has a special
check for thread->exc_actions being NULL, which short-circuits the rest
of the function without returning any exception ports. In 10.8.5,
thread->exc_actions can never be NULL. This new check is only present
for thread targets, presumably because it’s very common for threads to
not have any exception ports set, and not having to initialize this data
is an optimization. Typical user-level tasks in Mac OS X always have at
least some exception ports set at the task level.
TEST=util_test ExceptionPorts.TaskAndThreadExceptionPorts
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/584223002
exception handler ports.
task_t, thread_t, and exception_handler_t are typedefs for mach_port_t.
They are more descriptive than mach_port_t, and are already in use in
exc_server_variants.
TEST=util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/577293002
mach_exception_data_type_t is more generic and is used for any element
of a codes[] array. For individual elements, the typedefs
mach_exception_code_t and mach_exception_subcode_t are available. Using
mach_exception_code_t when possible gives slightly more descriptive
code.
No functional change.
TEST=util_test ExcServerVariants.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/585473003
This includes the functions ExceptionBehaviorHasState(),
ExceptionBehaviorHasIdentity(),
ExceptionBehaviorHasMachExceptionCodes(), and ExceptionBehaviorBasic().
TEST=util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/565793005
This also puts kMachExceptionCodes into mach_extensions.h, because a
form of MACH_EXCEPTION_CODES that’s the right type
(exception_behavior_t) has now shown its use in more than one file.
TEST=util_test ExcClientVariants.UniversalExceptionRaise
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/567283002
until the server is done handling its request.
In most tests, this is handled automatically because the client expects
a reply from the server, and waits for this reply. This test is
different because the server does handle the message, but does not send
a reply, so the client does not wait for one. When the child process
exits before the parent has had a chance to inspect the message it
received, the parent will find a dead name where it expects the remote
port to be, causing this test to be flaky based on the outcome of this
race.
The potential for a similar race existed in the
MachMessageServer.ComplexNotDestroyedNoReply test because it also
involved a request without a reply. In that test, the situation was
handled by not allowing the child to exit until signaled to do so by the
parent. The same logic should be used for both no-reply tests.
TEST=util_test MachMessageServer.ReturnCodeNoReply
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/569953002
Kernel bugs prevent debug thread state flavors from being delivered to
exception handlers properly. Through luck, things work out for
x86_DEBUG_STATE and x86_DEBUG_STATE64, but a handler registered to
receive the x86_DEBUG_STATE32 thread state flavor will never be called.
For the x86 family, the only thread state flavors that can be delivered
to exception handlers are the thread, float, and exception flavors.
Radar 18312067 is filed.
TEST=util_test ExcServerVariants.ThreadStates
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/558503006
MachOImageSymbolTableReader.
This results in a speed boost for MachOImageSymbolTableReader because
it’s able to read the entire string table in one operation, rather than
reading each string from the remote process individually. Copying is
also reduced. In a debug-mode build on my laptop, util_test
MachOImageReader.* has improved from ~1400ms to ~1000ms.
TEST=util_test TaskMemory.*:MachOImageReader.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/558313002
There’s no reason that “in” or “request” messages should be non-const.
This makes them const, bridges the const gap left by the MIG-generated
“check” functions with wrappers, and uses non-const fields in
“out” messages instead of const fields in “in” messages for in-out
parameters.
TEST=util_test ExcServerVariants.*:MachMessageServer.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/564533002
This includes the StringToNumber() function, both int and unsigned int variants.
Similar functionality is available in base, but it is unsuitable for
applications where a number’s base may be determined based on an "0x" or
"0X" prefix (hexadecimal) or an "0" prefix (octal).
TEST=util_test StringNumberConversion.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/557033002
An expectation of the child’s termination reason can now be specified.
It is now possible to test with children that do not exit cleanly, if
they are not expected to exit cleanly.
TEST=util_test Multiprocess.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/553153002
(MACH_RCV_LARGE).
This also adds a COMPILE_ASSERT to check that the
PersistentNonblockingFourMessages test case is not exceeding the value
of MACH_PORT_QLIMIT_DEFAULT.
TEST=util_test MachMessageServer.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/557793002
MachMessageServer is much like mach_msg_server() and
mach_msg_server_once(), but with a C++ interface and with a number of
deficiencies corrected.
TEST=util_test MachMessageServer.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/544393002
mach_exc is necessary because libSystem doesn’t contain this at all. exc
is necessary too, however: the copy in libSystem relies on the server
callbacks being externally defined symbols, which is cheesey.
Additionally, some Crashpad code wants to call internal validation
(“check”) routines that are not normally accessible to outside callers
via the copy of exc in libSystem, but they are made accessible here by
processing mig’s output in this Python script.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/541213002
