d:\src\crashpad\crashpad\util\file\string_file_writer_test.cc(367) : warning C4244: 'initializing' : conversion from 'const uint64_t' to 'const off_t', possible loss of data
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/836413004
This adds an Info.plist to these two tools that use task_for_pid().
Since they’re flat unbundled executables, the Info.plist must be placed
as a __TEXT,__info_plist section.
By setting SecTaskAccess to allowed and signing these tools with a
certificate trusted by the system, they will be allowed to use
task_for_pid() without requiring the invoking user to be root. This
provides an alternative to installing these tools as setuid root.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/822533002
run_with_crashpad runs a program with a Crashpad exception handler. It
allows a Crashpad exception-handling server to be started and to direct
crashes to it from programs that are not themselves clients of the
Crashpad client library.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/810423004
CrashpadClient is the primary interface for an application to have
Crashpad monitor it for crashes. It contains StartHandler(), which
starts a Crashpad handler process, and UseHandler(), which configures
the process to direct its crashes to a handler process.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/785233011
crashpad_handler is Crashpad’s exception handler server.
Currently, it runs a loop to receive exceptions, and exits when it no
longer has any clients. In the future, this will be extended to write
and potentially upload dumps.
The handler is expected to be started by its initial client via the
CrashpadClient interface.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/789693005
This function will be useful in upcoming non-test code. Because the
first Crashpad client that wants a Crashpad handler will now be
responsible for starting the handler process, this will prevent file
descriptors from leaking to the handler process.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/819483002
There will be no reason to leave the handler process connected to its
invoker’s stdin or stdout.
On the other hand, I’m currently leaving it connected to the original
and stderr, as these may be useful for diagnostics.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/818573002
I started (https://codereview.chromium.org/812403002/) emulating oflag values
on Windows in FileWriter, but it seemed awkward. On the assumption that we're
only likely to need "read a file" and "write a file" this seemed simpler, and
sufficient (but I don't know if that's necessarily true).
Users of open are not yet switched.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/818433002
Otherwise, when assigning to a smaller type, MSVC warns e.g.
d:\src\crashpad\crashpad\util\numeric\safe_assignment.h(38) : error C2220: warning treated as error - no 'object' file generated
d:\src\crashpad\crashpad\util\file\string_file_writer.cc(127) : see reference to function template instantiation 'bool crashpad::AssignIfInRange<size_t,FileOffset>(Destination *,Source)' being compiled
with
[
Destination=size_t
, Source=FileOffset
]
d:\src\crashpad\crashpad\util\numeric\safe_assignment.h(38) : warning C4244: '=' : conversion from 'FileOffset' to 'size_t', possible loss of data
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/809303003
Rename fd_io to file_io, and ReadFD to ReadFile, etc.
file_io.cc is the higher level versions that call the basic ReadFile/WriteFile
and then file_io_posix.cc and file_io_win.cc are the implementations of
those functions.
The Windows path is as yet untested, lacking the ability to link the test binary.
R=cpu@chromium.org, mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/811823003
This picks up:
222feaf3d302 Update README
e38cddb83571 Lock: don’t require NativeThreadType to be a pointer type
8c599712d10c Include What You Use: <string.h> for memmove() and memcpy()
ef991f44ed63 Linux: always build in C++11 mode
BUG=crashpad:6
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/812133002
There were many call sites that wasted a few lines on
mach_port_allocate() and sticking the result into a scoper. I was about
to add three more, so I took the opportunity to simplify things.
TEST=util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/809103002
Else,
d:\src\crashpad\crashpad\util\net\http_multipart_builder.cc(50) : warning C4244: 'initializing' : conversion from 'uint64_t' to 'int', possible loss of data
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/796643006
This DCHECK fails for me locally as
[----------] 3 tests from ProcessInfo
[ RUN ] ProcessInfo.Self
[70989:10546846:20141216,112509.948519:FATAL process_info_mac.cc:114] Check failed: static_cast<size_t>(ngroups) < (sizeof(ArraySizeHelper(kern_proc_info_.kp_eproc.e_ucred.cr_groups))) (16 vs. 16).
Abort trap: 6
It doesn't seem to happen on the waterfall, so maybe I'm building against
an incorrect header? I don't particularly understand the code, but assuming
it's normal 0-based array, perhaps it should be a DCHECK_LE in any case.
R=mark@chromium.org
Review URL: https://codereview.chromium.org/813473002
CXX_LIBRARY_VERSION is irrelevant, because the only C++11 library
feature of any concern is whether numeric_limits’ min() and max() are
declared constexpr.
Crashpad is C++11-only as far as the language is concerned, and the
comment doesn’t need to call it out explicitly because static_assert()
is always available.
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/809833002
Just avoid the Mac-specific __attribute__ tagging for now. There will need
to be some Windows-specific pragmas added here once the reader has been
written.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/808623002
e.g.
FAILED: ninja -t msvc -e environment.x86 -- "C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\bin\amd64_x86\cl.exe" /nologo /showIncludes /FC @obj\util\misc\util_test.clock_test.obj.rsp /c ..\..\util\misc\clock_test.cc /Foobj\util\misc\util_test.clock_test.obj /Fdobj\util\util_test.cc.pdb
d:\src\crashpad\crashpad\third_party\mini_chromium\mini_chromium\base\basictypes.h(49) : error C2220: warning treated as error - no 'object' file generated
d:\src\crashpad\crashpad\util\misc\clock_test.cc(72) : see reference to function template instantiation 'To implicit_cast<uint64_t,double>(const From &)' being compiled
with
[
To=uint64_t
, From=double
]
d:\src\crashpad\crashpad\third_party\mini_chromium\mini_chromium\base\basictypes.h(49) : warning C4244: 'return' : conversion from 'const double' to 'uint64_t', possible loss of data
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/807653002
e.g.
d:\src\crashpad\crashpad\util\numeric\in_range_cast.h(35) : warning C4244: 'return' : conversion from 'unsigned __int64' to 'uint32_t', possible loss of data
d:\src\crashpad\crashpad\util\numeric\in_range_cast.h(35) : warning C4244: 'return' : conversion from '__int64' to 'int32_t', possible loss of data
d:\src\crashpad\crashpad\util\numeric\in_range_cast_test.cc(54) : see reference to function template instantiation 'Destination crashpad::InRangeCast<int32_t,__int64>(Source,Destination)' being compiled
with
[
Destination=int32_t
, Source=__int64
]
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/800073003
I could also add COMPILER_CLANG to build_config.h, but that doesn't
appear in Chromium, apparently in preference to using __clang__
directly. I'm not sure if there's any good reason for that.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/803283002
Unfortunately VS2013's support of C++11 is partial. It supports the
extended union definition, but does not fully support constexpr.
So, update some locations where CXX_LIBRARY_VERSION is used where
toolchain support is lacking. It works correctly in the locations
where std::is_standard_layout is used.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/803273002
When run from the wrong location and test data or other test programs
can’t be found, the tests should fail with gtest assertions. The test
executable should not crash.
BUG=crashpad:4
TEST=util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/799083003
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
Option names like --mach-service are easier to type than --mach_service.
Command-line tools don’t necessarily have the most ergonomic interfaces
anyway, but this is an improvement.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/774763006
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
In b6a0183ccec7, gtest and gmock were held back at older versions with
limited C++11 support, because the then-current heads of each did not
work well in environments with C++11 language support but pre-C++11
library support. This is the environment that Chrome code for Mac builds
in. All necessary patches for this have been merged upstream, and this
has been corrected in the current released versions of gtest and gmock.
TEST=*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/731473007
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
MinidumpLocationDescriptorListWriter and MinidumpRVAListWriter are
implementation details and should be in the crashpad::internal
namespace.
MinidumpUTF16StringListWriter and MinidumpUTF8StringListWriter are
accessible to outside code and should not be in this namespace.
TEST=minidump_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/699313007
