122 Commits

Author SHA1 Message Date
Mark Mentovai
e9f40ae176 Remove double double words
I ran the thing below (piped to “grep -v namespace”), fixed things up,
and rewrapped comments in the affected file.

import re
import sys

LAST_WORD_RE = re.compile('^.*[\s]+([\w]+)$')
FIRST_WORD_RE = re.compile('^[^\w]+([\w]+).*$')

for path in sys.argv[1:]:
  with open(path) as file:
    line_number = 0
    last_word = None
    for line in file:
      line_number += 1
      first_word = FIRST_WORD_RE.match(line)
      if first_word and first_word.group(1) == last_word:
        print('%s:%u: %s' % (path, line_number - 1, last_word))
      last_word = LAST_WORD_RE.match(line)
      if last_word:
        last_word = last_word.group(1)

Change-Id: Iea9f2a6453d9d9ec17e2f238e09252535d7408bd
Reviewed-on: https://chromium-review.googlesource.com/780284
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-11-20 23:38:48 +00:00
Mark Mentovai
94a5a72efa mac: Tests that crash intentionally shouldn’t go to ReportCrash
Crashpad has many tests that crash intentionally. Some of these are
gtest death tests, and others arrange for intentional crashes to test
Crashpad’s own crash-catching logic. On macOS, all of the gtest death
tests and some of the other intentional crashes were being logged by
ReportCrash, the system’s crash reporter. Since these reports
corresponded to intentional crashes, they were never useful, and served
only to clutter ~/Library/Logs/DiagnosticReports.

Since Crashpad is adept at handling exceptions on its own, this
introduces the “exception swallowing server”,
crashpad_exception_swallower, which is a Mach exception server that
implements a no-op exception handler routine for all exceptions
received. The exception swallowing server is established as the task
handler for EXC_CRASH and EXC_CORPSE_NOTIFY exceptions during gtest
death tests invoked by {ASSERT,EXPECT}_DEATH_{CHECK,CRASH}, and for all
child processes invoked by the Multiprocess test infrastructure. The
exception swallowing server is not in effect at other times, so
unexpected crashes in test code can still be handled by ReportCrash or
another crash reporter.

With this change in place, no new reports are generated in the
user-level ~/Library/Logs/DiagnosticReports or the system’s
/Library/Logs/DiagnosticReports during a run of Crashpad’s full test
suite on macOS.

Bug: crashpad:33
Change-Id: I13891853a7e25accc30da21fa7ea8bd7d1f3bd2f
Reviewed-on: https://chromium-review.googlesource.com/777859
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-11-20 18:58:34 +00:00
Robert Sesek
f09257b17c Remove more unneeded MIG server stubs.
These were missed with the mig.py change in
6dd2be7c44a9c8bbea5df918e7ebe46d76da97df.

Change-Id: I7ad066cd9425cab26e56a8b3dfb90f5f54a6648d
Reviewed-on: https://chromium-review.googlesource.com/774999
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
2017-11-16 17:37:23 +00:00
Robert Sesek
6dd2be7c44 Stop providing unimplemented stubs for util/mach/exc_server_variants.cc.
Rather than providing these stubs to make the linker happy, use the
mig.py script to modify the _Xserver_routine functions to not even call
server_routine.

Change-Id: I5a2f5cd228462e38dddbf899d0ad8033a6f817bd
Reviewed-on: https://chromium-review.googlesource.com/773359
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-11-16 00:09:23 +00:00
Joshua Peraza
59c5d848e5 linux: Refactor ptrace usage.
1) Add PtraceConnection which serves as the base class for specific
types of connections Crashpad uses to trace processes.
2) Add DirectPtraceConnection which is used when the handler process
has `ptrace` capabilities for the target process.
3) Move `ptrace` logic into Ptracer. This class isolates `ptrace` call
logic for use by various PtraceConnection implementations.

Bug: crashpad:30
Change-Id: I98083134a9f7d9f085e4cc816d2b85ffd6d73162
Reviewed-on: https://chromium-review.googlesource.com/671659
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-09-22 16:25:32 +00:00
Mark Mentovai
8f0636288a Use constexpr at namespace scope
This is essentially based on a search for “^const .*=”.

Change-Id: I9332c1f0cf7c891ba1ae373dc537f700f9a1d956
Reviewed-on: https://chromium-review.googlesource.com/585452
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-07-29 01:06:52 +00:00
Mark Mentovai
6dac7ecdf5 Use constexpr at function scope
This is essentially based on a search for “^ *const [^*&]*=[^(]*$”

Change-Id: Id571119d0b9a64c6f387eccd51cea7c9eb530e13
Reviewed-on: https://chromium-review.googlesource.com/585555
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-07-29 00:50:40 +00:00
Mark Mentovai
281be63d00 Standardize on static constexpr for arrays when possible
This uses “static” at function scope to avoid making local copies, even
in cases where the compiler can’t see that the local copy is
unnecessary. “constexpr” adds additional safety in that it prevents
global state from being initialized from any runtime dependencies, which
would be undesirable.

At namespace scope, “constexpr” is also used where appropriate.

For the most part, this was a mechanical transformation for things
matching '(^| )const [^=]*\['.

Similar transformations could be applied to non-arrays in some cases,
but there’s limited practical impact in most non-array cases relative to
arrays, there are far more use sites, and much more manual intervention
would be required.

Change-Id: I3513b739ee8b0be026f8285475cddc5f9cc81152
Reviewed-on: https://chromium-review.googlesource.com/583997
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-07-25 17:40:51 +00:00
Mark Mentovai
c4f6ca3c6a mac: Provide a larger thread state buffer for AVX-512 on 10.13
Crashpad doesn’t use AVX-512, but when receiving replies to exceptions
forwarded to ReportCrash, may see buffers large enough to contain
AVX-512 thread state. This can result in messages like
“UniversalExceptionRaise: (ipc/rcv) msg too large (0x10004004)”.

I386_THREAD_STATE_MAX has increased from 224 to 614 in the 10.13 SDK,
meaning that the maximum supported size for old_state and new_state in
[mach_]exception_raise_state[_identity]() has increased from 896 to
2,456 bytes. This constant defines the size of the buffer that these
MIG-generated routines will work with. By providing this definition in
compat, the buffer size is increased when building with older SDKs.

Note that on the “send” side, the size of the message given to
mach_msg() will be trimmed to include only the valid part of the state
area based on the stateCnt field, so increasing the value to 614 here
won’t result Crashpad sending messages this large. That would be a
potential interoperability concern with older OS versions.

Bug: crashpad:185, crashpad:190
Change-Id: Ia46091ae46fd6227a17f59eb4bc00914be471aa7
Reviewed-on: https://chromium-review.googlesource.com/541515
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-06-20 14:31:38 +00:00
Mark Mentovai
15103742e0 Use FromPointerCast<>() in many places where it makes sense
I opted to leave casts to types that were definitely the same size
alone. reinterpret_cast<uintptr_t>(pointer) and
reinterpret_cast<intptr_t>(pointer) should always be safe, for example.
Casts to other integral types have been replaced with
FromPointerCast<>(), which does zero-extension or sign-extension based
on the target type.

To make it possible to use FromPointerCast<>() with some use sites that
were already using checked_cast<>(), FromPointerCast<>() now uses
check_cast<>() when converting to a narrower type.

Test: crashpad_util_test FromPointerCast*, others
Change-Id: I4a71b4aa2d87f545c75524290a702f5f3138d675
Reviewed-on: https://chromium-review.googlesource.com/489701
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-05-01 15:54:00 +00:00
Mark Mentovai
ddcc74f08f mac: Tolerate dead names for reply ports in the exception handler server
Self-monitoring revealed this CHECK was being hit in the wild:

base::debug::BreakDebugger()                debugger_posix.cc:260
logging::LogMessage::~LogMessage()          logging.cc:759
logging::MachLogMessage::~MachLogMessage()  mach_logging.cc:45
crashpad::ExceptionHandlerServer::Run()     exception_handler_server.cc:108
crashpad::HandlerMain()                     handler_main.cc:744

The MACH_CHECK() was:

108        MACH_CHECK(mr == MACH_MSG_SUCCESS, mr) << "MachMessageServer::Run";

Crash reports captured the full message, including the value of mr:

[0418/015158.777231:FATAL:exception_handler_server.cc(108)] Check failed: mr == MACH_MSG_SUCCESS. MachMessageServer::Run: (ipc/send) invalid destination port (0x10000003)

0x10000003 = MACH_SEND_INVALID_DEST.

This can happen when attempting to send a Mach message to a dead name.
Send (and send-once) rights become dead names when the corresponding
receive right dies. This would not normally happen for exception
requests originating in the kernel. It can happen for requests
originating from a user task: when the user task dies, the receive right
dies with it. All it takes to trigger this CHECK() in crashpad_handler
is for a Crashpad client to die (or be killed) while the handler is
processing a SimulateCrash() that the client originated.

Accept MACH_SEND_INVALID_DEST as a valid return value for
MachMessageServer::Run().

Note that MachMessageServer’s test coverage was already aware of this
behavior. MachMessageServer::Run()’s documentation is updated to reflect
it too.

Change-Id: I483c065d3c5f9a7da410ef3ad54db45ee53aa3c2
Reviewed-on: https://chromium-review.googlesource.com/479093
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-04-17 21:20:40 +00:00
Mark Mentovai
4b450c8137 test: Use (actual, [un]expected) in gtest {ASSERT,EXPECT}_{EQ,NE}
gtest used to require (expected, actual) ordering for arguments to
EXPECT_EQ and ASSERT_EQ, and in failed test assertions would identify
each side as “expected” or “actual.” Tests in Crashpad adhered to this
traditional ordering. After a gtest change in February 2016, it is now
agnostic with respect to the order of these arguments.

This change mechanically updates all uses of these macros to (actual,
expected) by reversing them. This provides consistency with our use of
the logging CHECK_EQ and DCHECK_EQ macros, and makes for better
readability by ordinary native speakers. The rough (but working!)
conversion tool is
https://chromium-review.googlesource.com/c/466727/1/rewrite_expectassert_eq.py,
and “git cl format” cleaned up its output.

EXPECT_NE and ASSERT_NE never had a preferred ordering. gtest never made
a judgment that one side or the other needed to provide an “unexpected”
value. Consequently, some code used (unexpected, actual) while other
code used (actual, unexpected). For consistency with the new EXPECT_EQ
and ASSERT_EQ usage, as well as consistency with CHECK_NE and DCHECK_NE,
this change also updates these use sites to (actual, unexpected) where
one side can be called “unexpected” as, for example, std::string::npos
can be. Unfortunately, this portion was a manual conversion.

References:

https://github.com/google/googletest/blob/master/googletest/docs/Primer.md#binary-comparison
77d6b17338
https://github.com/google/googletest/pull/713

Change-Id: I978fef7c94183b8b1ef63f12f5ab4d6693626be3
Reviewed-on: https://chromium-review.googlesource.com/466727
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-04 12:34:24 +00:00
Mark Mentovai
c39e4dc976 mac: Remove obsolete comment about mach_msg_header_t::msgh_reserved
mig-generated server dispatch routines used to not clear this field in
reply messages prepared from request messages. This oversight was
corrected in the migcom in bootstrap_cmds-96 (macOS 10.12 and Xcode
8.0). Maybe someone at Apple saw the admonishing comment that we had
left here. This comment can now be removed.

Change-Id: I73d965705a2ff5788afb59dd8ecdf4afe58ee47e
Reviewed-on: https://chromium-review.googlesource.com/465687
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-04-03 15:55:30 +00:00
Mark Mentovai
013d5e14a3 #include <stddef.h> where offsetof() is used
Bug: crashpad:30
Change-Id: If23ca9ea3141d3d34dc494aa29a1bd1dc8f83130
Reviewed-on: https://chromium-review.googlesource.com/458079
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-03-23 02:15:32 +00:00
Mark Mentovai
00b6442752 Make file_io reads more rational and predictable
ReadFile() attempted to continue reading after a short read. In most
cases, this is fine. However, ReadFile() would keep trying to fill a
partially-filled buffer until experiencing a 0-length read(), signaling
end-of-file. For certain weird file descriptors like terminal input, EOF
is an ephemeral condition, and attempting to read beyond EOF doesn’t
actually return 0 (EOF) provided that they remain open, it will block
waiting for more input. Consequently, ReadFile() and anything based on
ReadFile() had an undocumented and quirky interface, which was that any
short read that it returned (not an underlying short read) actually
indicated EOF.

This facet of ReadFile() was unexpected, so it’s being removed. The new
behavior is that ReadFile() will return an underlying short read. The
behavior of FileReaderInterface::Read() is updated in accordance with
this change.

Upon experiencing a short read, the caller can determine the best
action. Most callers were already prepared for this behavior. Outside of
util/file, only crashpad_database_util properly implemented EOF
detection according to previous semantics, and adapting it to new
semantics is trivial.

Callers who require an exact-length read can use the new
ReadFileExactly(), or the newly renamed LoggingReadFileExactly() or
CheckedReadFileExactly(). These functions will retry following a short
read. The renamed functions were previously called LoggingReadFile() and
CheckedReadFile(), but those names implied that they were simply
wrapping ReadFile(), which is not the case. They wrapped ReadFile() and
further, insisted on a full read. Since ReadFile()’s semantics are now
changing but these functions’ are not, they’re now even more distinct
from ReadFile(), and must be renamed to avoid confusion.

Test: *
Change-Id: I06b77e0d6ad8719bd2eb67dab93a8740542dd908
Reviewed-on: https://chromium-review.googlesource.com/456676
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-03-16 20:07:43 +00:00
Erik Chen
c1b305244a Update mig.py to take an explicit sdk argument.
BUG=chromium:690734

> Review-Url: https://codereview.chromium.org/2685233002
> Cr-Commit-Position: refs/heads/master@{#449550}
> Message-Id: Merged from chromium 53f2146935506b4f382705b605dffec41b5519eb

Change-Id: I1b3176a4a62078f1e27184ad589c9c3f4b548674
Reviewed-on: https://chromium-review.googlesource.com/440847
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-02-10 18:14:10 +00:00
Mark Mentovai
56020daea9 ExceptionTypes test: test “naked” signals
Since it’s possible to receive an EXC_CRASH for any signal that
generates a core by default even if the signal did not originate from a
Mach exception, update the tests to ensure that all such signals can be
unwrapped from an exception properly. This happens when a signal such as
SIGSEGV is sent with kill(), for example.

Change-Id: I1ee32cc6943f21ae349fa6788430d074acff9ed8
Reviewed-on: https://chromium-review.googlesource.com/434717
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-01-30 16:13:53 +00:00
Mark Mentovai
3e5ae2dc87 Update comments in IsExceptionNonfatalResource() given 10.12 source
With reference to 10.12 source, commentary regarding RESOURCE_TYPE_IO
can be authoritative.

Cursory examination of 10.12 source reveals that RESOURCE_TYPE_MEMORY
can now be fatal, although deeper examination reveals that this is
impossible on macOS. State this authoritatively as well.

BUG=crashpad:124

Change-Id: I52124c68fe017015983ab46e54006ba97ecd0142
Reviewed-on: https://chromium-review.googlesource.com/434297
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-01-30 14:39:27 +00:00
Mark Mentovai
1e4be91918 mac: Faster bit testing for EXC_GUARD exception “flavors”
After e7630628e9c9, I thought “isn’t there a standard library function
for that?” There is!

Change-Id: I284c7fdf8535c4fc53100e80fceb363bf2afee93
Reviewed-on: https://chromium-review.googlesource.com/431856
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-01-24 19:00:34 +00:00
Mark Mentovai
e7630628e9 mac: Report richer exception codes via metrics
Previously, only the top-level exception code was reported via the
Crashpad.ExceptionCode.Mac histogram. Making this histogram work
(https://crbug.com/678720) has revealed that Chrome is triggering
EXC_RESOURCE exceptions at a rate in excess of 4x that of ordinary
crashes. These exceptions were not previously visible because they are
not uploaded unless the system treats them as fatal, which it does not
normally do absent an explicit request.

In order to learn more about the problem, this change augments the data
reported via the Crashpad.ExceptionCode.Mac histogram to report (at
least) second-level exception data. This means that we will no longer
see just EXC_RESOURCE, but potentially more useful information such as
EXC_RESOURCE / RESOURCE_TYPE_IO / FLAVOR_IO_PHYSICAL_WRITES. This also
applies to other exception types, so that the majority of crashes
currently falling into the EXC_CRASH bucket will now have additional
information decoded and will be reported as, for example, EXC_BAD_ACCESS
/ KERN_INVALID_ADDRESS, EXC_BAD_INSTRUCTION / EXC_I386_INVOP, and
EXC_CRASH / SIGABRT.

Because the old mechanism was only live (in an “it works” sense) for
several days, and the new mechanism does not overlap with histogram
values used by the old one, there’s no need to invent a new histogram
name.

BUG=chromium:684051

Change-Id: Ia0a372b4127f7b3b2e7dbbaac9304cce3b5aadfe
Reviewed-on: https://chromium-review.googlesource.com/430933
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-01-24 15:59:30 +00:00
Mark Mentovai
741c9cc51e mac: Deal with bootstrap_look_up() race encountered on 10.12.1
bootstrap_look_up() “successfully” returns MACH_PORT_DEAD about half of
the time on 10.12.1 16B2657 (xnu-3789.21.4). Replace that with
MACH_PORT_NULL in the BootstrapLookUp() wrapper that all callers are
already routed through.

BUG=crashpad:139
TEST=crashpad_util_test MachExtensions.BootstrapCheckInAndLookUp

Change-Id: I9a39b709add5ca7e64bb5b970ed6ba3fdfd1d47a
Reviewed-on: https://chromium-review.googlesource.com/409671
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-11-09 22:06:23 +00:00
Mark Mentovai
acabe35928 doc: Fix all Doxygen warnings, cleaning up some generated documentation
This makes Doxygen’s output more actionable by setting QUIET = YES to
suppress verbose progress spew, and WARN_IF_UNDOCUMENTED = NO to prevent
warnings for undocumented classes and members from being generated. The
latter is too noisy, producing 721 warnings in the current codebase.

The remaining warnings produced by Doxygen were useful and actionable.
They fell into two categories: abuses of Doxygen’s markup syntax, and
missing (or misspelled) parameter documentation. In a small number of
cases, pass-through parameters had intentionally been left undocumented.
In these cases, they are now given blank \param descriptions. This is
not optimal, but there doesn’t appear to be any other way to tell
Doxygen to allow a single parameter to be undocumented.

Some tricky Doxygen errors were resolved by asking it to not enter
directiores that we do not provide documentation in (such as the
“on-platform” compat directories, compat/mac and compat/win, as well as
compat/non_cxx11_lib) while allowing it to enter the
“off-platform” directories that we do document (compat/non_mac and
compat/non_win).

A Doxygen run (doc/support/generate_doxygen.sh) now produces no output
at all. It would produce warnings if any were triggered.

Not directly related, but still relevant to documentation,
doc/support/generate.sh is updated to remove temporary removals of
now-extinct files and directories. doc/appengine/README is updated so
that a consistent path to “goapp” is used throughout the file.

Change-Id: I300730c04de4d3340551ea3086ca70cc5ff862d1
Reviewed-on: https://chromium-review.googlesource.com/408812
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-11-08 19:24:05 +00:00
Mark Mentovai
952f787f4a doc: Standardize on “macOS” in comments
Use “macOS” as the generic unversioned name of the operating system in
comments. For version-specific references, use Mac OS X through 10.6, OS
X from 10.7 through 10.11, and macOS for 10.12.

Change-Id: I1ebee64fbf79200bc799d4a351725dd73257b54d
Reviewed-on: https://chromium-review.googlesource.com/408269
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-11-08 19:21:44 +00:00
Erik Chen
1e6dbcb300 Support passing DEVELOPER_DIR to mig.py
BUG=chromium:651267

Change-Id: If02f9bac603237677d348869d05d7b4d0b31909e
Reviewed-on: https://chromium-review.googlesource.com/392486
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-10-04 14:50:45 +00:00
Mark Mentovai
3887d99e48 mac: Handle EXC_RESOURCE RESOURCE_TYPE_IO
RESOURCE_TYPE_IO always appears to be non-fatal based on disassembly of
the function responsible for sending it in xnu 3705.0.0.1.10 (10.12dp1
16A201w).

BUG=crashpad:120,crashpad:124

Change-Id: I9dcc6673f922cbd7af910b76991825a9d9c96fe6
Reviewed-on: https://chromium-review.googlesource.com/355250
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-06-23 15:11:55 +00:00
Scott Graham
a02ba24006 Convert from scoped_ptr to std::unique_ptr
Follows https://codereview.chromium.org/1911823002/ but fixes includes
that were messed up there.

Change-Id: Ic4bad7d095ee6f5a1c9f8ca2d11ac9e67d55a626
Reviewed-on: https://chromium-review.googlesource.com/340497
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-04-25 19:16:26 +00:00
Mark Mentovai
6d2d31d2d1 Use base/macros.h instead of base/basictypes.h
This was done in Chromium’s local copy of Crashpad in 562827afb599. This
change is similar to that one, except more care was taken to avoid
including headers from a .cc or _test.cc when already included by the
associated .h. Rather than using <stddef.h> for size_t, Crashpad has
always used <sys/types.h>, so that’s used here as well.

This updates mini_chromium to 8a2363f486e3a0dc562a68884832d06d28d38dcc,
which removes base/basictypes.h.

e128dcf10122 Remove base/move.h; use std::move() instead of Pass()
8a2363f486e3 Move basictypes.h to macros.h

R=avi@chromium.org

Review URL: https://codereview.chromium.org/1566713002 .
2016-01-06 12:22:50 -05:00
Mark Mentovai
583d1dc3ef Provide std::move() in compat instead of using crashpad::move()
This more-natural spelling doesn’t require Crashpad developers to have
to remember anything special when writing code in Crashpad. It’s easier
to grep for and it’s easier to remove the “compat” part when pre-C++11
libraries are no longer relevant.

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1513573005 .
2015-12-09 17:36:32 -05:00
Mark Mentovai
7764fa1144 Remove errant double-semicolons
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1489063002 .
2015-12-01 12:56:03 -05:00
Dana Jansens
6bebb10829 Replace use of .Pass() with crashpad::move().
Since C++11 library support isn't available everywhere crashpad is
compiled, add our own move() method in the crashpad namespace to replace
std::move() for now. Replace uses of .Pass() with this method.

R=mark@chromium.org, scottmg@chromium.org
BUG=chromium:557422

Review URL: https://codereview.chromium.org/1483073004 .
2015-11-30 14:20:54 -08:00
Mark Mentovai
4f09b58d1f Add RandomString() and its test, and use it everywhere it makes sense
This unifies several things that used a 16-character random string, and
a few other users of random identifiers where it also made sense to use
a 16-character random string.

TEST=crashpad_util_test RandomString.RandomString
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1451793002 .
2015-11-16 13:39:01 -05:00
Mark Mentovai
c1b841442f mac: Add NotifyServer::DefaultInterface, a default no-op implementation
Each routine in this implementation returns MIG_BAD_ID. These routines
may be overridden.

Most things that implement NotifyServer::Interface will only need to
implement one of the interface routines. Since another user of
NotifyServer will be added soon, it makes sense to provide a default
no-op implementation rather than forcing everyone to write the same
no-op boilerplate repeatedly.

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1414413006 .
2015-10-30 15:44:40 -04:00
Mark Mentovai
cd0e25f1ba Update all URLs to point to https://crashpad.chromium.org/
All other links to code.google.com and googlecode.com are fixed to point
to their proper new homes as well.

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1414243005 .
2015-10-29 18:31:20 -04:00
Mark Mentovai
fc7d8b3a27 mac: Make crashpad_handler get its receive right from its client
Previously, crashpad_handler made its own receive right, and transferred
a corresponding send right to its client. There are two advantages to
making the receive right in the client:

 - It is possible to monitor the receive right for a port-destroyed
   notificaiton in the client, allowing the handler to be restarted if
   it dies.
 - For the future run-from-launchd mode (bug crashpad:25), the handler
   will obtain its receive right from the bootstrap server instead of
   making its own. Having the handler get its receive right from
   different sources allows more code to be shared than if it were to
   sometimes get a receive right and sometimes make a receive right and
   transfer a send right.

This includes a restructuring in crashpad_client_mac.cc that will make
it easier to give it an option to restart crashpad_handler if it dies.
The handler starting logic should all behave the same as before.

BUG=crashpad:68
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1409073013 .
2015-10-29 18:09:03 -04:00
Mark Mentovai
062138106c mac: ChildPortHandshake: allow receive rights to be sent
The intended use is to flip the client-server relationship in
CrashpadClient so that the initial client (parent process) furnishes the
handler process with a receive right. The parent can optionally receive
a port-destroyed notification allowing it to restart the handler if it
exits prematurely.

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1408473002 .
2015-10-29 14:14:15 -04:00
Mark Mentovai
6c0d42ce9d Mach port scopers should use get() instead of type conversion operators
In https://codereview.chromium.org/1411523006, the Mach port scopers are
becoming better ScopedGenerics and are losing the type conversion
operators in the process. This is needed to adapt to that change. get()
is ugly, but being explicit about conversion isn’t a bad thing, and
these scopers will gain functionality such as Pass() as part of the
switch.

As a bonus, some would-be uses of get() to check for valid port rights
are becoming a more descriptive is_valid().

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1405273002 .
2015-10-20 11:03:25 -04:00
Mark Mentovai
5f7eda87a6 mac: Don’t leak send rights from ExceptionPorts::GetExceptionPorts()
ExceptionPorts::GetExceptionPorts() returned a
std::vector<ExceptionPorts::ExceptionHandler>, which contained send
rights to Mach ports. The interface required callers to assume ownership
of each send right contained within the vector. This was cumbersome and
error-prone, and despite the care taken in Crashpad, port right leaks
did occur:

 - SimulateCrash() didn’t make any attempt to release these resources at
   all.
 - Neither did crashpad_util_test ExceptionPorts.HostExceptionPorts,
   which also reused a vector.

This replaces the vector with the interface-compatible (as far as
necessary) ExceptionPorts::ExceptionHandlerVector, which deallocates
collected port rights on destruction or clear().

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1381023007 .
2015-10-06 16:14:29 -04:00
Mark Mentovai
bb13efbda7 Add and use scoped-right-returning wrappers for Mach bootstrap routines
This wraps bootstrap_check_in() in BootstrapCheckIn(), and
bootstrap_look_up() in BootstrapLookUp(). The wrappers make it more
difficult to accidentally leak a returned right. They’re easier to use,
encapsulating common error checking and logging, simplifying all call
sites.

TEST=crashpad_util_test MachExtensions.BootstrapCheckInAndLookUp
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1383283003 .
2015-10-05 17:07:15 -04:00
Mark Mentovai
cd85c9f700 mac: Add CrashpadClient::UseSystemDefaultHandler()
Chrome’s relauncher process needs a way to sever ties with the
crashpad_handler instance running from the disk image in order to cause
that instance to exit so that the disk image may be unmounted. This new
function is otherwise not thought to be interesting, and its use is not
recommended.

This comes with a small refactoring to create a
SystemCrashReporterHandler() function, and a fix for a minor port leak
in CrashReportExceptionHandler::CatchMachException().

BUG=chromium:538373
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1375573005 .
2015-10-02 14:40:38 -04:00
Mark Mentovai
4ff6c2d71f Remove #include "base/basictypes.h" as appropriate
These files were only using basictypes.h for implicit_cast, which moved
to util/misc/implicit_cast.h in 0b022d72a2a4.

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1352873002 .
2015-09-17 12:32:38 -04:00
Scott Graham
0b022d72a2 Include implicit_cast.h at all users of it.
The implicit_cast in base will be no more, make sure we have a reference
to the crashpad version at all callsites.

BUG=529769, 472900, crashpad:51
R=mark@chromium.org, scottmg@chromium.org

Review URL: https://codereview.chromium.org/1344683002 .
2015-09-14 14:51:05 -07:00
Mark Mentovai
9086d25ce8 Don’t trigger EXC_CORPSE_NOTIFY on OS X 10.11
CrashReportExceptionHandler::CatchMachException() must always set a
valid new_state. Failing to do so appears to trigger corpse generation
on OS X 10.11. This is addressed by calling ExcServerCopyState().
Previously, this was not done for exceptions forwarded to the user
ReportCrash, under the apparent mistaken assumption that ReportCrash
would do it. However, ReportCrash is given copies of out-parameters like
new_state to explicitly prevent it from influencing Crashpad’s returned
state.

ExcServerSuccessfulReturnValue() must not return MACH_RCV_PORT_DIED for
an EXC_CRASH handler on OS X 10.11. This appears to trigger corpse
generation. This is addressed by always returning KERN_SUCCESS from
EXC_CRASH handlers on OS X 10.11.

This also adds generic EXC_CORPSE_NOTIFY support throughout Crashpad.
The crashpad_handler does not listen for this exception type, but it is
now possible to work with this exception type using tools like
exception_port_tool and catch_exception_tool.

BUG=crashpad:48
TEST=Crashes handled by crashpad_handler do not result in the generation
     of reports in the root /Library/Logs/DiagnosticReports.

R=kerrnel@chromium.org, rsesek@chromium.org

Review URL: https://codereview.chromium.org/1305893010 .
2015-09-04 14:29:12 -04:00
Mark Mentovai
34aef02cc7 ubsan: Don’t call v[0] on empty vectors
Calling std::vector<>::operator[]() with an out-of-range index argument
is undefined behavior. In two cases, Crashpad used &v[0] in situations
where it was known that the address would not be used. These calls were
wrapped in conditions guarding against vector emptiness.

While s[0] is valid on an empty string, in two cases, Crashpad used
&s[0] as an argument to a system call that would be a no-op. These calls
were wrapped in similar conditions to avoid the system call.

The two uses of vector with undefined behavior were caught by the
following tests in crashpad_snapshot_test with
UndefinedBehaviorSanitizer:

[ RUN      ] CrashpadInfoClientOptions.OneModule
/Users/mark/compilatorium/llvm.build/bin/../include/c++/v1/vector:1493:12:
runtime error: reference binding to null pointer of type
'crashpad::process_types::section'
[       OK ] CrashpadInfoClientOptions.OneModule (72 ms)

[ RUN      ] ProcessSnapshotMinidump.Empty
/Users/mark/compilatorium/llvm.build/bin/../include/c++/v1/vector:1493:12:
runtime error: reference binding to null pointer of type
'MINIDUMP_DIRECTORY'
[       OK ] ProcessSnapshotMinidump.Empty (1 ms)

The Crashpad codebase was audited by searching for resize() calls and
analyzing how resized strings and vectors are used.

TEST=*
BUG=
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1283243004 .
2015-08-20 11:50:19 -04:00
Mark Mentovai
b1d7833600 Use EXPECT_STREQ(a, b) when a and b are both const char*
While not strictly asan-related, this bug was found while running tests
under asan. Evidently, strings are pooled differently in that build
configuration.

TEST=crashpad_util_test ExceptionPorts.*
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1291573004 .
2015-08-19 18:47:51 -04:00
Mark Mentovai
6645a69240 asan: Fix invalid memory access in UniversalExceptionRaise()
TEST=crashpad_util_test ExcClientVariants.UniversalExceptionRaise
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1283323010 .
2015-08-19 18:47:02 -04:00
Erik Chen
3d216f5516 mac: Suppress partial availability warnings.
The warnings are emitted when a translation unit attempts to reference
a function whose availability is newer than the deployment target.

BUG=471823
R=mark@chromium.org

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

Patch from Erik Chen <erikchen@chromium.org>.
2015-05-01 14:16:58 -04:00
Mark Mentovai
1baff4ff92 Accept non-fatal resource exceptions without generating crash reports.
This adds IsExceptionNonfatalResource() and its test, and uses it in
crashpad_handler. When non-fatal resource exceptions are encountered, no
crash report is generated. crashpad_handler swallows these exceptions.
Alternatively, it could allow them to be sent to the system’s host-level
resource exception handler, normally com.apple.ReportCrash.root, which
would allow them to be processed in the same way as when Crashpad is not
in use. I’m not sure which option is better. I chose to swallow them
because there doesn’t appear to be much value in letting
com.apple.ReportCrash.root and spindump look at them.

This also moves ExcCrashRecoverOriginalException() to the new file as a
sibling of IsExceptionNonfatalResource(). This provides better
organization.

BUG=crashpad:35, chromium:474163, chromium:474326
TEST=crashpad_util_test ExceptionTypes.IsExceptionNonfatalResource
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1066243002
2015-04-08 17:46:09 -04:00
Mark Mentovai
40b1d7cb1d Add ConstThreadState to mach_extensions.h and use it everywhere.
TEST=everything
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1058523002
2015-04-02 15:28:28 -04:00
Mark Mentovai
352160906b 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
2015-04-01 12:16:22 -04:00
Mark Mentovai
809ea8158d test: Move util/test to its own top-level directory, test.
After 9e79ea1da719, it no longer makes sense for crashpad_util_test_lib
to “hide” in util/util_test.gyp. All of util/test is moved to its own
top-level directory, test, which all other test code is allowed to
depend on. test, too, is allowed to depend on all other non-test code.

In a future change, when crashpad_util_test_lib gains a dependency on
crashpad_client, it won’t look so weird for something in util (even
though it’s in util/test) to depend on something in client, because the
thing that needs to depend on client will live in test, not util.

BUG=crashpad:33
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1051533002
2015-03-31 17:44:14 -04:00