The handler will now be less strict about checking CrashpadInfo struct
sizes. Assuming the signature and version fields match:
- If the handler sees a struct smaller than it’s expecting, the module
was likely built with an earlier version of the client library, and
it’s safe to treat the unknown fields as though they were zero or
other suitable default values.
- If the handler sees a struct larger than it’s expecting, the module
was likely built with a later version of the client library. In that
case, actions desired by the client will not be performed, but this
is not otherwise an error condition.
The CrashpadInfo struct must always be at least large enough to contain
at least the size field. The signature and version fields are always
checked.
The section size must be at least as large as the size carried within
the struct. To account for possible section padding, strict equality is
not required.
Bug: chromium:784427
Test: crashpad_snapshot_test CrashpadInfoSizes_ClientOptions/*.*
Change-Id: Ibb0690ca6ed5e7619d1278a68ba7e893d55f19fb
Reviewed-on: https://chromium-review.googlesource.com/767709
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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-comparison77d6b17338https://github.com/google/googletest/pull/713
Change-Id: I978fef7c94183b8b1ef63f12f5ab4d6693626be3
Reviewed-on: https://chromium-review.googlesource.com/466727
Reviewed-by: Scott Graham <scottmg@chromium.org>
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>
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>
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>
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>
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>
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>
__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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Forgot some u number suffixes.
BUG=crashpad:94
Change-Id: I2a3b47b4eab07bf5b9ced3859f5a8b388a840b35
Reviewed-on: https://chromium-review.googlesource.com/329760
Reviewed-by: Robert Sesek <rsesek@chromium.org>
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 .
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 .
FILE_TYPE_CHAR handles can't be inherited via
PROC_THREAD_ATTRIBUTE_HANDLE_LIST, or CreateProcess() fails with
GetLastError() == 1450 on Windows 7.
I confirmed that an fprintf(stderr, ...) in HandlerMain() does make it
to the console when running tests even after this.
See bug for more discussion.
R=mark@chromium.org
BUG=crashpad:77
Review URL: https://codereview.chromium.org/1473793002 .
This requires Windows NT 6.0 (Vista and Server 2008). On earlier
operating system versions, the existing behavior of inheriting all
inheritable handles is retained.
BUG=crashpad:69
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1427273003 .
This consolidates all of the twisted casts and comments that discuss how
HANDLEs are really only 32 bits wide even in 64-bit processes on 64-bit
operating systems into a single location.
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1422503015 .
Allowing the client to create its own pipe name string caused a race
between client and server. Instead, in this mode, the server now creates
the pipe name along with a pipe, and returns it to its client via a
--handshake-handle. This guarantees that by the time the client gets the
pipe name, the server has already created it.
Ephemeral mode is now implied by --handshake-handle. The --persistent
option is gone. --persistent mode is enabled when using --pipe-name.
BUG=crashpad:69
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1432563003 .
By invoking crashpad_handler with --mach-service instead of
--handshake-fd, the handler will run as a well-behaved launchd job. The
launchd job may be as a launch agent or launch daemon, or be submitted
to launchd by on_demand_service_tool.
BUG=crashpad:25
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/1414533006 .
For multiprocess architectures, this method allows the pipe used for
registration to be obtained from CrashpadHandler, even when
CrashpadHandler chooses its own name. This may happen if the handler is
not running on a well-known pipe name but was instead started by
CrashpadHandler::StartHandler(). If Chrome uses this interface, for
example, the browser process will need to call
CrashpadClient::GetHandlerIPCPipe() and pass the pipe name to its child
processes.
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1427163004 .
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 .
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 .
Fixes two incorrect usages of ssize_t/off_t being implicitly converted
to bool. As such, I think it's worth the cost of the additional !! on
BOOL returning Win32 functions.
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1408123006 .
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 .
I thought I had confirmed that this still allocated and ignored the flag
on older OSs, but I must have not had the PLOG active yet? I'm not sure
what I did. (I might try to blame VMware as it has an annoying habit of
caching old binaries when you use it's "Shared Folders" feature to point
at the dev machine's build dir.)
I confirmed that it does work on Win8 and Win10 but doesn't on Win XP
and Win 7.
R=mark@chromium.org
BUG=crashpad:52
Review URL: https://codereview.chromium.org/1405243002 .
Capture the memory for the loader lock (can be inspected by !cs), as
well as all locks that were created with .DebugInfo which can be viewed
with !locks.
e.g.
0:000> !cs ntdll!LdrpLoaderLock
-----------------------------------------
Critical section = 0x778d6410 (ntdll!LdrpLoaderLock+0x0)
DebugInfo = 0x778d6b6c
NOT LOCKED
LockSemaphore = 0x0
SpinCount = 0x04000000
0:000> !locks -v
CritSec ntdll!RtlpProcessHeapsListLock+0 at 778d7620
LockCount NOT LOCKED
RecursionCount 0
OwningThread 0
EntryCount 0
ContentionCount 0
CritSec +7a0248 at 007a0248
LockCount NOT LOCKED
RecursionCount 0
OwningThread 0
EntryCount 0
ContentionCount 0
CritSec crashy_program!g_critical_section_with_debug_info+0 at 01342c48
LockCount NOT LOCKED
RecursionCount 0
OwningThread 0
EntryCount 0
ContentionCount 0
CritSec crashy_program!crashpad::`anonymous namespace'::g_test_critical_section+0 at 01342be0
WaiterWoken No
LockCount 0
RecursionCount 1
OwningThread 34b8
EntryCount 0
ContentionCount 0
*** Locked
Scanned 4 critical sections
R=mark@chromium.org
BUG=crashpad:52
Review URL: https://codereview.chromium.org/1392093003 .
When not building against the C++11 library headers, the compiler cannot
treat the lambda as lvalue. When building against the C++11 library headers, it
is converted to an rvalue.
BUG=chromium:542321
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1406733003 .
I’ve accidentally created Crashpad databases when running
crashpad_database_util by mistyping the argument to --database. Typical
users of crashpad_database_util probably don’t want the database to be
created.
This adds a new --create option to crashpad_database_util that is
required to get it to create a database. If not present, a database will
not be created if it does not already exist.
TEST=crashpad_client_test CrashReportDatabaseTest.*
R=rsesek@chromium.org, scottmg@chromium.org
Review URL: https://codereview.chromium.org/1395653002 .
Previously, any attempt to create a new crash report database would
result in this message being logged:
[p:t:yyyymmdd,hhmmss.uuuuuu:ERROR file_io.cc:30] read: expected 40,
observed 0
This would be the first thing that a developer embedding Crashpad into
their application would see after getting everything right. It doesn’t
exactly seem like everything’s right with that being logged. It would
also be the first thing that a user would see on stderr or in logs upon
launching a Crashpad-enabled application, which also seems kind of
dodgy.
The crash report database settings creation logic is restructured to
avoid logging this error when definitely creating a new database, while
retaining all other error logging.
BUG=crashpad:63
TEST=crashpad_database_util --database $new_db --show-client-id
(should not show any errors)
R=rsesek@chromium.org, scottmg@chromium.org
Review URL: https://codereview.chromium.org/1392953002 .
This resolves some left-behind TODOs referring to a closed bug. It looks
like this should have worked since dfaa25af4929.
BUG=crashpad:13
TEST=crashpad_snapshot_test CrashReportDatabaseTest.*
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1391993002 .
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 .
If the task’s exception handler for EXC_CRASH, EXC_RESOURCE, and
EXC_GUARD exceptions cannot be set, clear the handler instead.
Nothing considered this function’s return value, and the only viable
fallback action on failure would have been to do what the function now
does, so its return type is changed to void.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/1386943002 .
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 .
Windows requires the connection to the handler to do anything, so it
can't really be implemented or tested without CrashpadClient and the
connection machinery.
R=mark@chromium.org
BUG=crashpad:53
Review URL: https://codereview.chromium.org/1356383002 .
