176 Commits

Author SHA1 Message Date
Mark Mentovai
6950a552bf doc: Fix Doxygen-generated documentation after 34699d378b82
Bug: crashpad:192
Change-Id: Ia8b699ec3abe7491d30277d71f74e31f2fcc8343
Reviewed-on: https://chromium-review.googlesource.com/749311
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-11-01 17:17:04 +00:00
Mark Mentovai
ef262d1ee3 #include "build/build_config.h" where needed
Change-Id: I45c1afe73e8570dfcedde6da01375a4533bb355a
Reviewed-on: https://chromium-review.googlesource.com/741891
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-10-27 18:22:29 +00:00
Mark Mentovai
1dae919b7e #include "base/logging.h" in client/annotation.h for DCHECK()
Bug: crashpad:192
Change-Id: I0da7d1721202794a7fb052731f4457bd5aa53b9f
Reviewed-on: https://chromium-review.googlesource.com/741887
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-10-27 18:01:07 +00:00
Robert Sesek
34699d378b Create client data structures for typed Annotations.
This introduces the Annotation object, used to declare typed
annotations, and the AnnotationList object, used to reference these. The
AnnotationList is referenced by the CrashpadInfo structure. Currently
nothing reads these.

The AnnotationList implements a lock-free linked list, into which
Annotation objects are added exactly once, when they are first set.
Clearing an Annotation merely marks it internally as such, rather than
removing it from the list.

Bug: crashpad:192
Change-Id: I72414b1f83d624c4ae323e09ecea8cfb69a68c5e
Reviewed-on: https://chromium-review.googlesource.com/547135
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
2017-10-25 21:56:20 +00:00
Mark Mentovai
a327c86a52 C++14 is required, don’t pretend to support pre-C++11 or pre-MSVS 2015
Change-Id: Ide835421599480acc63e8e88ce2217433c0d376e
Reviewed-on: https://chromium-review.googlesource.com/719036
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-10-13 15:49:59 +00:00
Mark Mentovai
7a849482ea Switch the language standard to C++14 and use std::make_unique
Update mini_chromium to 7d6697ceb5cb5ca02fde3813496f48b9b1d76d0c

47ff9691450e Switch the language standard to C++14
7d6697ceb5cb Remove base/memory/ptr_util.h and base::WrapUnique

base::WrapUnique and std::make_unique are similar, but the latter is
standardized and preferred.

Most of the mechanical changes were made with this sed:

for f in $(git grep -l base::WrapUnique | uniq); do
  sed -E \
      -e 's%base::WrapUnique\(new ([^(]+)\((.*)\)\);%std::make_unique<\1>(\2);%g' \
      -e 's%base::WrapUnique\(new ([^(]+)\);%std::make_unique<\1>();%g' \
      -e 's%^#include "base/memory/ptr_util.h"$%#include <memory>%' \
      -i '' "${f}"
done

Several uses of base::WrapUnique that did not fit on a single line and
were not matched by this sed were adjusted manually. All #include
changes were audited manually, to at least move <memory> into the
correct section. Where <memory> was already #included by a file (or its
corresponding header), the extra #include was removed. Where <memory>
should have been #included by a header, it was added. Other similar
adjustments to other #includes were also made.

Change-Id: Id4e0baad8b3652646bede4c3f30f41fcabfdbd4f
Reviewed-on: https://chromium-review.googlesource.com/714658
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-10-12 19:07:13 +00:00
Mark Mentovai
c6adcc2482 win: Make CrashpadClient::DumpAndCrashTargetProcess() less chatty
CrashpadClient::DumpAndCrashTargetProcess() suspends the target process
and injects a thread to raise an exception. The injected thread is not
suspended, and may proceed to the point that the system recognizes the
process as terminating by the time the overall process suspension is
lifted. Previously, if this happened, an extraneous error was logged for
the attempt to resume a terminating process.

This introduces “termination tolerance” to ScopedProcessSuspend, which
allows an object to be configured to ignore this error and not log any
messages when this condition is expected.

This resolves log messages such as this one, produced frequently during
calls to CrashpadClient::DumpAndCrashTargetProcess() (including in
end_to_end_test.py):

> [pid:tid:yyyymmdd,hhmmss.mmm:ERROR scoped_process_suspend.cc:39]
> NtResumeProcess: An attempt was made to access an exiting process.
> (0xc000010a)

0xc000010a = STATUS_PROCESS_IS_TERMINATING

Test: end_to_end_test.py
Change-Id: Iab4c50fb21adce5502080ad25a6f734ec566d65c
Reviewed-on: https://chromium-review.googlesource.com/700715
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-10-04 19:04:47 +00:00
Joshua Peraza
cea0671011 win: Add crashpad_handler_console as a dependency of crashpad_client_test
The binary crashpad_handler.com is used by crashpad_client_win_test.cc,
but is not currently built when building crashpad_client_test.

Bug: crashpad:
Change-Id: I7a440774e49be9e821bca57c154a67b968a4bfbd
Reviewed-on: https://chromium-review.googlesource.com/695832
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
2017-10-02 22:08:54 +00:00
Sigurdur Asgeirsson
20ed4146d3 Use StringPiece for key and value in SimpleStringDictionary interface.
Bug: crashpad:193
Change-Id: I22ffad0f76f5aec0397bf9ab797641ea0889af24
Reviewed-on: https://chromium-review.googlesource.com/638910
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
2017-09-06 13:01:04 +00:00
Xi Cheng
01110c0a3b win: Fix %u, %d, %x/DWORD printf mismatches
To enable clang-cl's printf format string mismatch checking, a few
mismatch errors need to be fixed where DWORD (unsigned long) is printed
with %u, %d or %x (an 'l' is needed).

Change-Id: I2cbfafe823a186bfe3a555aec3a7ca03e85466f8
Reviewed-on: https://chromium-review.googlesource.com/598651
Commit-Queue: Xi Cheng <chengx@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-08-02 22:04:13 +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
f845797732 mac: 10.13 SDK compatibility, adapt to x86_state_hdr changes
In the 10.12 SDK, x86_state_hdr from <mach/i386/thread_status.h> was
defined as:

struct x86_state_hdr {
  int flavor;
  int count;
};

This has changed in the 10.13 SDK to:

struct x86_state_hdr {
  uint32_t flavor;
  uint32_t count;
};

This triggers signedness mismatch errors where these values are used
with CHECK/DCHECK macros and gtest EXPECT/ASSERT macros.

Compatibility with existing and new SDKs must be maintained, so more
casts must be used.

Bug: crashpad:185, crashpad:188
Change-Id: I8844d6a78520430a8b5b90a35403896c3c6cfa37
Reviewed-on: https://chromium-review.googlesource.com/533375
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-06-13 18:46:25 +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
fd8e2de0c5 win: MSVS 2017 (15)/C++ 14.1/C 19.10 compatibility
Includes mini_chromium ef0ded8717340c9fe48e8e0f34f3e0e74d10a392.

1d2a024fdb1d android: Use _FILE_OFFSET_BITS after all (undo
             dc3d480305b2)
ef0ded871734 win: MSVS 2017 (15)/C++ 14.1/C 19.10 compatibility

Change-Id: I5c814669a0ef8577872bddff9112ce28ec628ba3
Reviewed-on: https://chromium-review.googlesource.com/482639
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-20 00:28:35 +00:00
Mark Mentovai
e04194afd9 win: Wrap TerminateProcess() to accept cdecl patches on x86
TerminateProcess(), like most of the Windows API, is declared WINAPI,
which is __stdcall on 32-bit x86. That means that the callee,
TerminateProcess() itself, is responsible for cleaning up parameters on
the stack on return. In https://crashpad.chromium.org/bug/179, crashes
in ExceptionHandlerServer::OnNonCrashDumpEvent() were observed in ways
that make it evident that TerminateProcess() has been patched with a
__cdecl routine. The crucial difference between __stdcall and __cdecl is
that the caller is responsible for stack parameter cleanup in __cdecl.
The mismatch means that nobody cleans parameters from the stack, and the
stack pointer has an unexpected value, which in the case of the Crashpad
handler crash, results in TerminateProcess()’s second argument
erroneously being used as the lock address in the call to
ReleaseSRWLockExclusive() or LeaveCriticalSection().

As a workaround, on 32-bit x86, call through SafeTerminateProcess(), a
custom assembly routine that’s compatible with either __stdcall or
__cdecl implementations of TerminateProcess() by not trusting the value
of the stack pointer on return from that function. Instead, the stack
pointer is restored directly from the frame pointer.

Bug: crashpad:179
Test: crashpad_util_test SafeTerminateProcess.*, others
Change-Id: If9508f4eb7631020ea69ddbbe4a22eb335cdb325
Reviewed-on: https://chromium-review.googlesource.com/481180
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-19 17:45:32 +00:00
Mark Mentovai
bc7c6e235d mac: Prevent the same report from being uploaded multiple times
With multiple crashpad_handlers running out of the same database, it was
possible for more than one to attempt to upload the same report. Nothing
ensured that the reports remained pending between the calls to
CrashReportDatabaseMac::GetPendingReports() and
CrashReportDatabaseMac::GetReportForUploading().

The Windows equivalent did not share this bug, but it would return
kBusyError. kReportNotFound is a better code.

Test: crashpad_client_test CrashReportDatabaseTest.*
Change-Id: Ieaee7f94ca8e6f2606d000bd2ba508d3cfa2fe07
Reviewed-on: https://chromium-review.googlesource.com/473928
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-04-13 14:12:56 +00:00
Sigurdur Asgeirsson
ab9c03f882 win: Promote WinMultiProcessWithTempDir to test/win for reuse.
Bug: crashpad:167
Change-Id: I80a4a58246d479bceb7154f270f34380a65ebf6d
Reviewed-on: https://chromium-review.googlesource.com/470110
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-04-07 15:13:16 +00:00
Mark Mentovai
b409540163 handler: Reuse existing annotations SimpleStringDictionary if present
Bug: crashpad:143
Change-Id: I75a77adacd83febb7c363598bbc6d19c184b773d
Reviewed-on: https://chromium-review.googlesource.com/468167
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-05 14:09:17 +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
4688351623 “Promote” test::Paths::Executable() to Paths::Executable()
This supports the “double handler” or “double handler with low
probability” models from https://crashpad.chromium.org/bug/143.

For crashpad_handler to be become its own client, it needs access to its
own executable path to pass to CrashpadClient::StartHandler(). This was
formerly available in the test-only test::Paths::Executable(). Bring
that function’s implementation to the non-test Paths::Executable() in
util/misc, and rename test::Paths to test::TestPaths to avoid future
confusion.

test::TestPaths must still be used to access TestDataRoot(), which does
not make any sense to non-test code.

test::TestPaths::Executable() is retained for use by tests, which most
likely prefer the fatal semantics of that function. Paths::Executable()
is not fatal because for the purposes of implementing the double
handler, a failure to locate the executable path (which may happen on
some systems in deeply-nested directory hierarchies) shouldn’t cause the
initial crashpad_handler to abort, even if it does prevent a second
crashpad_handler from being started.

Bug: crashpad:143
Test: crashpad_util_test Paths.*, crashpad_test_test TestPaths.*
Change-Id: I9f75bf61839ce51e33c9f7c0d7031cebead6a156
Reviewed-on: https://chromium-review.googlesource.com/466346
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-04-03 18:58:01 +00:00
Sigurdur Asgeirsson
542a91e20e Fix the race causing flaky CrashpadClient tests.
Bug: crashpad:81
Change-Id: I3cb115440638df909d1c0cdfd01c824ac0d0b073
Reviewed-on: https://chromium-review.googlesource.com/458592
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
2017-03-24 19:46:24 +00:00
Mark Mentovai
14138936b5 test: Compare ProcessInfo::Arguments() to main()’s argc/argv on POSIX
Previously on macOS, the test used an OS-specific library function to
recover the original argc and argv. On Linux/Android, it essentially
reimplemented the very code it was testing, which didn’t make for a very
good test. The new approach is to save argc and argv in main() and base
the comparison on that.

Bug: crashpad:30
Test: crashpad_util_test ProcessInfo.*, crashpad_test_test MainArguments.*
Change-Id: I578abed3b04ae10a22f79a193bbb8b6589276c97
Reviewed-on: https://chromium-review.googlesource.com/456798
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-03-20 17:05:30 +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
Mark Mentovai
b5284cdcba mac: Add CrashpadClient::GetHandlerMachPort()
Bug: chromium:699607
Change-Id: Ib1886550fe81787cb1ffc8d8853f6969cc96831e
Reviewed-on: https://chromium-review.googlesource.com/451127
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-03-09 00:44:58 +00:00
Mark Mentovai
58aac1bd87 Set FD_CLOEXEC on file descriptors obtained from open() and fopen()
Includes an update of mini_chromium to 3a2d52d74c9a:

3a2d52d74c9a Use O_CLOEXEC (and O_NOCTTY) when calling open()

BUG=chromium:688362

Change-Id: I2bdf86efe4e6559ecb77492ac5bdc728aa035889
Reviewed-on: https://chromium-review.googlesource.com/447999
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-02-28 21:16:41 +00:00
Scott Graham
cdbb90ec69 win: Add timeout argument to WaitForHandlerStart()
As brought up in https://codereview.chromium.org/2475863004/, there's
the potential for failed startup if StartHandlerProcess() hangs for
whatever reason. Add a timeout to the wait function so that this case
can attempt to log an error.

R=mark@chromium.org
BUG=655788, 656800, 565063

Change-Id: Ib08cd0641daa6a6cefabb773ffe470227b51958c
Reviewed-on: https://chromium-review.googlesource.com/419060
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-12-13 16:47:34 +00:00
Scott Graham
556c4e4f50 Have crashpad call ASan's crash handler if present
Upstreaming change made downstream in
https://codereview.chromium.org/2504773002. Formatting modified
slightly.

R=mark@chromium.org, rnk@chromium.org
BUG=661209

Change-Id: Iab8c4ffda3af24b7a61ec0a4a10b187966da481f
Reviewed-on: https://chromium-review.googlesource.com/417237
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-12-07 22:03:52 +00:00
Mark Mentovai
777634b1eb Use ADDRESS_SANITIZER instead of __has_feature(address_sanitizer)
__has_feature() is a Clang-ism not implemented by GCC.
base/compiler_specific.h provides a HAS_FEATURE() macro that always
returns 0 when __has_feature() is not implemented. Use this macro for
compatibility with GCC and other compilers that do not implement this
Clang extension.

http://clang.llvm.org/docs/LanguageExtensions.html#has-feature-and-has-extension

For GCC’s Address Sanitizer implementation, test the
__SANITIZE_ADDRESS__ macro that it provides as an alternative to
__has_feature(address_sanitizer).

Note that in Chrome builds, ADDRESS_SANITIZER is pushed in by the build
system. The definition of ADDRESS_SANITIZER provides another way for
that macro to be set. It’s supplementary, not exclusive.

cb33b24372/build/config/BUILD.gn (118)

BUG=crashpad:30

Change-Id: I5c3145d29bbc966925369c03a37b1ecb5622a004
Reviewed-on: https://chromium-review.googlesource.com/413109
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-11-22 19:28:06 +00:00
Scott Graham
68095b6a4e Don't LOG(ERROR) for usage, and fix VS2013 build after 8b3eec8
R=mark@chromium.org
BUG=crashpad:57

Change-Id: I6514a82ae5de38a695422ef86c044ec3b2ce171b
Reviewed-on: https://chromium-review.googlesource.com/412269
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-11-17 22:59:17 +00:00
Scott Graham
8b3eec83e9 win: Add signal handler for SIGABRT to handle abort() calls
R=mark@chromium.org
BUG=crashpad:57

Change-Id: Ib7141f00e74e3db9e5be427cc990847331e09912
Reviewed-on: https://chromium-review.googlesource.com/412058
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-11-17 22:00:38 +00:00
Mark Mentovai
f09d0cde00 Improve time handling and error checking
The database settings object’s last_upload_attempt_time (time_t) field
is switched from uint64_t to int64_t, for better compatibility with
time_t, which is normally a signed type. This change should be
transparent, as there should be no valid high-bit-set 64-bit timestamps
in this field in the wild.

A number of improvements are made to crashpad_database_util’s time
handling. Errors are checked during time conversion.
--set-last-upload-attempt-time=now is a new supported (and documented)
option.

A StringToNumber() overload for int64_t, along with a test, is added to
aid in crashpad_database_util’s time conversions from numeric strings. A
test is also added for the previously-untested uint64_t implementation.

TEST=crashpad_util_test StringNumberConversion.*

Change-Id: I089c4bf7b95f5df0982bdbb3c27b4f6a89db966e
Reviewed-on: https://chromium-review.googlesource.com/410068
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-11-15 18:53:11 +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
Scott Graham
c4cdec3d72 Handle non-crashing cases for server failure to start
Follow up #4!

R=mark@chromium.org
BUG=chromium:567850,chromium:656800
TEST=tests added to crashpad_client_test

Change-Id: I2a53f2168988e620ce240750c6c2d544ba95c8b4
Reviewed-on: https://chromium-review.googlesource.com/406741
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-11-03 16:38:22 +00:00
Mark Mentovai
bb7d249d65 Partially port the crashpad_client library to Linux/Android
This defines the global (per-module) CrashpadInfo structure properly on
Linux/Android, located via the “crashpad_info” section name.

Per the ELF specification, section names with a leading dot are reserved
for the system. Reading that, I realized that the same is true of Mach-O
sections with leading underscores, so this renames the section as used
on Mach-O from __DATA,__crashpad_info to __DATA,crashpad_info.

This change is sufficient to successfully build crashpad_client as a
static library on Linux/Android, but the library is incomplete. There’s
no platform-specific database implementation, no CaptureContext() or
CRASHPAD_SIMULATE_CRASH() implementation, and most notably, no
CrashpadClient implementation.

BUG=crashpad:30

Change-Id: I29df7b0f8ee1c79bf8a19502812f59d4b1577b85
Reviewed-on: https://chromium-review.googlesource.com/406427
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-11-02 23:19:50 +00:00
Scott Graham
76ef9b5c2b win: Address failure-to-start-handler case for async startup
Second follow up to https://chromium-review.googlesource.com/c/400015/

The ideal would be that if we fail to start the handler, then we don't
end up passing through our unhandled exception filter at all.

In the case of the non-initial client (i.e. renderers) we can do this by
not setting our UnhandledExceptionFilter until after we know we've
connected successfully (because those connections are synchronous from
its point of view). We also change WaitForNamedPipe in the connection
message to block forever, so as long as the precreated pipe exists,
they'll wait to connect. After the initial client has passed the server
side of that pipe to the handler, the handler has the only handle to it.
So, if the handler has disappeared for whatever reason, pipe-connecting
clients will fail with FILE_NOT_FOUND, and will not stick around in the
connection loop. This means non-initial clients do not need additional
logic to avoid getting stuck in our UnhandledExceptionFilter.

For the initial client, it would be ideal to avoid passing through our
UEF too, but none of the 3 options are great:
1. Block until we find out if we started, and then install the filter.
   We don't want to do that, because we don't want to wait.
2. Restore the old filter if it turns out we failed to start. We can't
   do that because Chrome disables ::SetUnhandledExceptionFilter()
   immediately after StartHandler/SetHandlerIPCPipe returns.
3. Don't install our filter until we've successfully started. We don't
   want to do that because we'd miss early crashes, negating the benefit
   of deferred startup.

So, we do need to pass through our UnhandledExceptionFilter. I don't
want more Win32 API calls during the vulnerable filter function. So, at
any point during async startup where there's a failure, set a global
atomic that allows the filter function to abort without trying to signal
a handler that's known to not exist.

One further improvement we might want to look at is unexpected
termination of the handler (as opposed to a failure to start) which
would still result in a useless Sleep(60s). This isn't new behaviour,
but now we have a clear thing to do if we detect the handler is gone.

(Also a missing DWORD/size_t cast for the _x64 bots.)

R=mark@chromium.org
BUG=chromium:567850,chromium:656800

Change-Id: I5be831ca39bd8b2e5c962b9647c8bd469e2be878
Reviewed-on: https://chromium-review.googlesource.com/400985
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-11-02 21:39:52 +00:00
Mark Mentovai
88e3b6b022 Omit platform-specific assembler source from builds as needed
The default filename rules do not match .S or .asm, so the
platform-specific assembler implementations of CaptureContext() were not
being affirmatively excluded from other platforms’ builds. This
previously worked without causing problems because the Mac build
environment didn’t know what to do with .asm files, and the Windows
build environment didn’t know what to do with .S files. Now that another
platform that may understand .S files is being added, the rules for when
to build these files must be tailored a bit more tightly.

BUG=crashpad:30

Change-Id: Ib62e619c007320d45279c104b3e229d92698aa72
Reviewed-on: https://chromium-review.googlesource.com/406348
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-11-01 21:17:02 +00:00
Scott Graham
2d87606bb5 win: Start crashpad_handler by inheriting connection data to it
Previously, StartHandler() launched the handler process, then connected
over a pipe to register for crash handling. Instead, the initial client
can create and inherit handles to the handler and pass those handle
values and other data (addresses, etc.) on the command line.

This should improve startup time as there's no need to synchronize with
the process at startup, and allows avoiding a call to CreateProcess()
directly in StartHandler(), which is important for registration for
crash reporting from DllMain().

Incidentally adds new utility functions for string/number conversion and
string splitting.

Note: API change; UseHandler() is removed for all platforms.

BUG=chromium:567850,chromium:656800

Change-Id: I1602724183cb107f805f109674c53e95841b24fd
Reviewed-on: https://chromium-review.googlesource.com/400015
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-10-21 20:35:58 +00:00
Scott Graham
a16a7fd4ef win: Make DumpAndCrash() always terminate, and tidy up special codes
Upstreaming with tidying up of
https://codereview.chromium.org/2377693002/.

R=mark@chromium.org
BUG=chromium:634418

Change-Id: Ie8b3b673c8d2a06c5cc918a034688aa9396cfbf4
Reviewed-on: https://chromium-review.googlesource.com/390436
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-09-29 04:44:20 +00:00
Scott Graham
ac6c01b575 Add metrics for tracking uploads
Three new metrics:
- counting upload success/failure;
- enum tracking the reason upload was skipped;
- enum describing how an upload got to the pending state.

R=mark@chromium.org, asvitkine@chromium.org
BUG=crashpad:100

Change-Id: I5e0cbc1ac3424e974f3a51560e5cdad484ffc038
Reviewed-on: https://chromium-review.googlesource.com/388855
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-09-26 22:09:32 +00:00
Scott Graham
27aeb2c9da Upstream Chromium UMA integration
Unmodified from https://codereview.chromium.org/2308763002/ other than
to add empty arguments to test code (that doesn't build in Chrome).

Requires https://chromium-review.googlesource.com/c/386236/.

Rolls mini_chromium to include:
438bd4f4 Add stub of persistent_histogram_allocator.h

R=mark@chromium.org
BUG=crashpad:100

Change-Id: Ibc88338ae2fd40a5a4ade7ff098be2bc19511543
Reviewed-on: https://chromium-review.googlesource.com/386084
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-09-16 20:23:06 +00:00
Scott Graham
afc177ee21 Pull metrics instrumentation out to central file
Solves two problems with having the macros inline:

1. Deduplicates some of the logic (in this case, the name of the
   histogram, and whether it should be divided by 1024);

2. More useful check for compilation. As the macros are no-ops in
   Crashpad, it was easy to use the wrong name for a variable in the
   arguments to the macros (see .mm!)

This way, we have some better chance of at least having code that
compiles when built in Chromium if all the arguments are passed to
Metrics::Something() in a standalone build.

Also rolls mini_chromium DEPS to include:
99213eb Mark histogram arguments as unused to avoid warnings

R=mark@chromium.org
BUG=crashpad:100

Change-Id: I9f7fc3b85854fd61c1ebdf0084d728a7b690c2f1
Reviewed-on: https://chromium-review.googlesource.com/380445
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-09-02 19:13:00 +00:00
Scott Graham
5f42313ed5 Test first integration of UMA plumbing
Add a first example of a UMA entry to have it available to try to plumb
through to Chromium.

Adds LoggingFileSizeByHandle() to util/file/file_io.* to retrieve the
size of on disk file to report to UMA.

Also rolls DEPS for mini_chromium to include:
b5ec9ce Add stub versions of histogram_macros.h

R=mark@chromium.org
BUG=crashpad:100

Change-Id: Ib8e96ad4b7d715b46d2c71810c95c92965a89821
Reviewed-on: https://chromium-review.googlesource.com/338821
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-09-02 00:04:29 +00:00
Gayane Petrosyan
b35ee1fca1 Adding support for on-demand uploads.
In order to allow on-demand uploads for crash reports, adding a
upload_explicitly_requested bit on 'pending' state and necessary support
for it.

BUG=chromium:620762

Change-Id: Ida38e483fe8d0e48eb5cbe95e8b8bfd96a2f8f00
Reviewed-on: https://chromium-review.googlesource.com/367328
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-08-24 21:57:02 +00:00
Scott Graham
660a5e69d6 win: switch crashpad_handler.exe to /subsystem:windows and add .com
This switches the default behaviour of crashpad_handler.exe to be a
/subsystem:windows app, so that normal usage won't cause a console to be
popped up. At the same time, creates a copy of crashpad_handler.exe in
the output dir named crashpad_handler.com. The .com doesn't affect
normal operation, as the way StartHandler() uses CreateProcess()
requires a real path to a file. However, when run from a command prompt,
.com are found before .exe, so editbin the .com to be to a console app,
which will be run in preference to the exe when run as just
"crashpad_handler", as one tends to do from a command prompt when
debugging. That is:

  d:\src\crashpad\crashpad\out\Debug>where crashpad_handler
  d:\src\crashpad\crashpad\out\Debug\crashpad_handler.com
  d:\src\crashpad\crashpad\out\Debug\crashpad_handler.exe

  d:\src\crashpad\crashpad\out\Debug>crashpad_handler --help
  Usage: crashpad_handler [OPTION]...
  ...

  d:\src\crashpad\crashpad\out\Debug>crashpad_handler.exe --help
  <no output>

  d:\src\crashpad\crashpad\out\Debug>crashpad_handler.com --help
  Usage: crashpad_handler.com [OPTION]...
  ...

We also use the .com file in test invocations so that output streams
will be visible.

R=mark@chromium.org

Change-Id: I1a27f88472d491b2a1d76e63c45e6415d9f679c0
Reviewed-on: https://chromium-review.googlesource.com/371578
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-08-17 20:50:47 +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
Scott Graham
6a6a0c27ed win: Support dumping another process by causing it to crash
Adds a new client API which allows causing an exception in another
process. This is accomplished by injecting a thread that calls
RaiseException(). A special exception code is used that indicates to the
handler that the exception arguments contain a thread id and exception
code, which are in turn used to fabricate an exception record. This is
so that the API can allow the client to "blame" a particular thread in
the target process.

The target process must also be a registered Crashpad client, as the
normal exception mechanism is used to handle the exception.

The injection of a thread is used instead of DebugBreakProcess() which
does not cause the UnhandledExceptionFilter() to be executed.
NtCreateThreadEx() is used in lieu of CreateRemoteThread() as it allows
passing of a flag which avoids calling DllMain()s. This is necessary to
allow thread creation to succeed even when the target process is
deadlocked on the loader lock.

BUG=crashpad:103

Change-Id: I797007bd2b1e3416afe3f37a6566c0cdb259b106
Reviewed-on: https://chromium-review.googlesource.com/339263
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-04-22 17:27:58 +00:00
Scott Graham
dbfcb5d032 win: Cap indirect memory gathering
Add a user-configurable cap on the amount of memory that is gathered by
dereferencing thread stacks. (SyzyAsan stores a tremendously large
number of pointers on the stack, so the dumps were ending up in the ~25M
range.)

Also reduce the range around pointers somewhat.

Change-Id: I6bce57d86bd2f6a796e1580c530909e089ec00ed
Reviewed-on: https://chromium-review.googlesource.com/338463
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-04-22 17:04:33 +00:00