130 Commits

Author SHA1 Message Date
Robert Sesek
b6a3d91342 Read annotation objects from the client when producing snapshots.
This wires up the annotation objects system of the client to the
snapshot production and minidump writing facilities.

Bug: crashpad:192
Change-Id: If7bb7625b140d71a15b84729372cbd0fd4bc63ef
Reviewed-on: https://chromium-review.googlesource.com/749870
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
2017-11-02 16:40:48 +00:00
Mark Mentovai
2db30e035a win: Fix Clang -Wsign-compare warnings in new test code
This test code appeared in 9609b7471676, and was missed by the similar
warning cleanup of a51e912004a6, which was developed in parallel.

Bug: crashpad:192, chromium:779790
Change-Id: I4ed88ed025e4be4410c98ceaca395218f00007be
Reviewed-on: https://chromium-review.googlesource.com/750024
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-11-02 15:16:59 +00:00
Robert Sesek
f9e587b036 Remove NOTREACHED() in ModuleSnapshot::AnnotationObjects() impls.
This is causing crashpad_handler_test to fail in Debug on Windows.

Bug: crashpad:192
Change-Id: Icf3ff387050ee2becf471f4e7c3a75394b1dd436
Reviewed-on: https://chromium-review.googlesource.com/749792
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-11-01 22:16:43 +00:00
Mark Mentovai
43b798b492 test: Fix paths for crashpad_tests monolith in Chromium
Instead of individual per-directory test executables like
crashpad_util_test, all Crashpad tests in Chromium will be run from a
single crashpad_tests executable.

Test: crashpad_util_test Paths.Executable, ProcessInfo.Self; crashpad_snapshot_test PEImageReader.DebugDirectory
Bug: chromium:779790
Change-Id: If95272fd641734fbdb8e231fbcdc4e7ccb2cb822
Reviewed-on: https://chromium-review.googlesource.com/749303
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-11-01 17:00:30 +00:00
Mark Mentovai
1669ca2bac test: Rework TestPaths interface for obtaining 32-bit build artifacts
The design for running all Crashpad unit tests on Chromium’s try- and
buildbots involves pulling all tests into a single monolithic
crashpad_tests executable. Many Crashpad tests base the name of their
child executables or modules on the name of the main test executable.
Since the main test executable will have a different name in the
in-Chromium build, knowledge of the test executable name (referred to as
“module” here) needs to be added to the tests themselves.

This introduces TestPaths::BuildArtifact(), which allows the module name
to be specified. For Crashpad’s standalone build, the module name is
verified against the main test executable’s name.
TestPaths::BuildArtifact() can also locate paths in the alternate 32-bit
output directory for 64-bit Windows tests, taking on the responsibility
for what the new (5e9ed4cb9f69) TestPaths::Output32BitDirectory(), now
obsolete, did.

Bug: chromium:779790
Change-Id: I64c4a2190b6319e487c999812a7cfc512a75a700
Reviewed-on: https://chromium-review.googlesource.com/747536
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-11-01 16:44:45 +00:00
Mark Mentovai
a51e912004 Fix warnings produced by trunk clang in test code
These are mostly -Wsign-compare warnings, with a -Wconstant-conversion
and a -Wunguarded-availability thrown in.

Bug: chromium:779790
Change-Id: Ic2103f3332ce57378db83eca7fa2569efec1a7b6
Reviewed-on: https://chromium-review.googlesource.com/746081
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-11-01 16:35:49 +00:00
Robert Sesek
9609b74716 Expand the PEImageAnnotationsReader to read Annotation objects.
Nothing currently directs the handler to read these Annotation objects
from the target process, so they will not be read by Crashpad nor appear
in the minidump.

Bug: crashpad:192
Change-Id: I1eb1e9f42282c07e37d335631f0cc6083ef28a89
Reviewed-on: https://chromium-review.googlesource.com/726501
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-10-31 22:40:38 +00:00
Robert Sesek
b88fde0b56 Add the AnnotationSnapshot object and attach it to ModuleSnapshot.
The AnnotationSnapshot is the handler-side of the Annotation object,
which will store the annotation data when read by a ProcessReader.

Bug: crashpad:192
Change-Id: Ic65c95022c452522678c1070c27c429dd631fb64
Reviewed-on: https://chromium-review.googlesource.com/717197
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-10-31 18:24:41 +00:00
Mark Mentovai
5e9ed4cb9f win: Dynamically disable WoW64 tests absent explicit 32-bit build output
Rather than having the 64-bit build assume that it lives in
out\{Debug,Release}_x64 and that it can find 32-bit build output in
out\{Debug,Release}, require the location of 32-bit build output to be
provided explicitly via the CRASHPAD_TEST_32_BIT_OUTPUT environment
variable. If this variable is not set, 64-bit tests that require 32-bit
test build output will dynamically disable themselves at runtime.

In order for this to work, a new DISABLED_TEST() macro is added to
support dynamically disabled tests. gtest does not have its own
first-class support for this
(https://groups.google.com/d/topic/googletestframework/Nwh3u7YFuN4,
https://github.com/google/googletest/issues/490) so this local solution
is used instead.

For tests via Crashpad’s own build\run_tests.py, which is how Crashpad’s
own buildbots and trybots invoke tests, CRASHPAD_TEST_32_BIT_OUTPUT is
set to a locaton compatible with the paths expected for the GYP-based
build. No test coverage is lost on Crashpad’s own buildbots and trybots.

For Crashpad tests in Chromium’s buildbots and trybots, this environment
variable will not be set, causing these tests to be dynamically
disabled.

Bug: crashpad:203, chromium:743139, chromium:777924
Change-Id: I3c0de2bf4f835e13ed5a4adda5760d6fed508126
Reviewed-on: https://chromium-review.googlesource.com/739795
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-10-26 18:31:57 +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
419f25eac8 Remove PointerVector<> and replace with std::vector<std::unique_ptr<>>
As mentioned at
https://chromium-review.googlesource.com/c/crashpad/crashpad/+/721978/13/tools/crashpad_http_upload.cc#90
Change-Id: I4820346cc0b0bf26633e1de598c884af8af19983
Reviewed-on: https://chromium-review.googlesource.com/724744
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-10-19 04:53:36 +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
93c88d87f0 win: Don’t appear to be a client connecting in end_to_end_test.py
end_to_end_test.py was producing these error messages 6 times (32-bit
x86) and 7 times (x86_64) per run:

[pid:tid:yyyymmdd,hhmmss.mmm:ERROR file_io.cc:89] ReadExactly: expected
36, observed 0

These messages were being produced by crashpad_handler, in the
LoggingReadFileExactly() call in
ExceptionHandlerServer::ServiceClientConnection().
sizeof(ClientToServerMessage) is 36. crashpad_handler believed that a
client was connecting, but the client sent no data.

This was tracked down to the use of os.path.exists() in
end_to_end_test.py to wait for crashpad_handler’s named pipe to be
created. Checking named pipe existence in this way appeared to be a
client connecting to the the pipe server in crashpad_handler, although
of course no real client was connecting and no message was forthcoming.
I found that running “dir” on the named pipe’s path produced the same
result.

Using WaitNamedPipe() is an alternative that can be used to signal when
the named pipe’s path exists. Furthermore, it tests more than mere
creation, it indicates that the pipe server has become ready to service
clients. That’s not necessary in this case as proper clients already
need to deal with this on their own, but checking it in
end_to_end_test.py should be harmless.

Test: end_to_end_test.py
Change-Id: Ida29a3d2325368f58930cdf8fb053449f621ea52
Reviewed-on: https://chromium-review.googlesource.com/703276
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-10-05 20:58:59 +00:00
Mark Mentovai
e5896de993 win: Fix process_structs.h definition of RTL_USER_PROCESS_PARAMETERS
In the 64-bit version of the structure, padding is needed between
ShowWindowFlags and WindowTitle.

The CurrentDirectores (yes, that’s how it’s spelled) members would have
been interpreted incorrectly because STRING was defined incorrectly. The
length fields are USHORT, not DWORD. In the 64-bit version of the
structure, a padding member ensured that the structure was at least the
correct size. In the 32-bit version of the structure, this caused the
structure size to be inflated, so all but the first CurrentDirectores
element and any struct member that followed would appear at incorrect
offsets, and the overall struct size being read was larger than
appropriate.

This resolves crashpad_handler logging (usually) three errors while
handling a 64-bit process crash, such as:

[pid:tid:yyyymmdd,hhmmss.mmm:ERROR process_info.cc:632] range at
0x780f24de00000000, size 0x275 fully unreadable
[pid:tid:yyyymmdd,hhmmss.mmm:ERROR process_info.cc:632] range at
0x780f24fe00000000, size 0x275 fully unreadable
[pid:tid:yyyymmdd,hhmmss.mmm:ERROR process_info.cc:632] range at 0x0,
size 0x275 fully unreadable

Bug: crashpad:198
Test: end_to_end_test.py
Change-Id: I1655101de01cf46b4b50eda45a11f8d0f3bca8b3
Reviewed-on: https://chromium-review.googlesource.com/701736
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-10-05 20:49:08 +00:00
Mark Mentovai
90054edf62 win: De-flake hanging_program.exe
hanging_program.exe is used by crash_other_program.exe, which is in turn
used by end_to_end_test.py. It hangs by loading loader_lock_dll.dll,
which squats in its entry point function while the loader lock is held.

hanging_program.exe needs to do some work in its Thread1() before the
loader lock is taken (a SetThreadPriority() call), and needs to do some
work in its main thread once the loader lock is held (it needs to signal
crash_other_program.exe that it’s successfully wedged itself).
Previously, proper synchronization was not provided. A 1-second Sleep()
was used to wait for the loader lock to be taken. Thread1() pre-work was
only achieved before the loader lock was taken by sheer luck. Things
didn’t always work out so nicely.

This uses an event handle to provide synchronization. An environment
variable is used to pass the handle to loader_lock_dll.dll, because
there aren’t many better options available. This eliminates both flake
and the unnecessary 1-second delay in hanging_program.exe, and since
this program runs twice during end_to_end_test.py, it improves that
test’s runtime by 2 seconds.

Bug: crashpad:197
Test: end_to_end_test.py
Change-Id: Ib9883215ef96bed7571464cc68e09b6ab6310ae6
Reviewed-on: https://chromium-review.googlesource.com/700076
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-10-04 19:58:56 +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
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
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
74fddc3fed win: Wrap test::ChildLauncher::Start() in ASSERT_NO_FATAL_FAILURE()
Test: crashpad_snapshot_test, crashpad_util_test, end_to_end_test
Change-Id: I09581521678fe3b083c409f308eeab2e583b3c9f
Reviewed-on: https://chromium-review.googlesource.com/481245
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-19 17:47:23 +00:00
Mark Mentovai
79425e4d97 win: Free an old buffer before attempting to allocate a resized one
When GetProcessInformation() obtains SystemProcessInformation, it
resizes its buffer as directed by NtQuerySystemInformation(). Nothing of
value resides in the old buffer if a resize is attempted, so it can be
freed before attempting to allocate a resized one.

This may help crashes like go/crash/f385e94c80000000, which experience
out-of-memory while attempting to allocate a resized buffer. It also may
not help, because the required buffer size may just be too large to fit
in memory. See https://crashpad.chromium.org/bug/143#c19.

Change-Id: I63b9b8c1efda22d2fdbf05ef2b74975b92556bbd
Reviewed-on: https://chromium-review.googlesource.com/473792
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-11 21:49:46 +00:00
Joshua Peraza
45305395ad win: Determine length of unloaded module names
Change-Id: I802b2a8a505cf53009c0c5648acdad7a44e9f0e7
Reviewed-on: https://chromium-review.googlesource.com/466598
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-04-04 17:15:39 +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
Mark Mentovai
f14eda221f win: Be more careful about exit codes in end_to_end_test.py
This is like 270490ff79df, but for things run by end_to_end_test.py, and
things run for it by crash_other_program.exe.

Change-Id: Iabf3c762c50f41eb61ab31f714c646364196e745
Reviewed-on: https://chromium-review.googlesource.com/458822
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-03-24 22:34:48 +00:00
Mark Mentovai
6c8a3f7007 win: Use a semaphore for synchronization in ProcessSnapshotTest
Bug: crashpad:160
Test: crashpad_snapshot_test ProcessSnapshotTest.CrashpadInfoChild*
Change-Id: Id9bd1ee6ec71c57ecab2ccba6106d41446d66902
Reviewed-on: https://chromium-review.googlesource.com/458879
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-03-24 21:54:17 +00:00
Mark Mentovai
270490ff79 win: Be more careful about child process exit codes
Checking child process’ exit codes would have helped catch bug
crashpad:160 sooner. Instead, we had a flaky hang that was difficult to
reproduce locally.

Bug: crashpad:160
Test: crashpad_snapshot_test ExceptionSnapshotWinTest.ChildCrash*:ProcessSnapshotTest.CrashpadInfoChild*:SimulateCrash.ChildDumpWithoutCrashing*, crashpad_util_test ProcessInfo.OtherProcess
Change-Id: I73bd2be1437d05f0501a146dcb9efbe3b8e0f8b7
Reviewed-on: https://chromium-review.googlesource.com/459039
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-03-24 20:12:31 +00:00
Scott Graham
9104d53d1c Fix thread startup in snapshot reader test
The use of InterlockedCompareExchange() was very wrong. Improved error
checking coming in another CL from mark@, see linked bug for discussion.

Bug: crashpad:160
Change-Id: Id230af6f37c6cdce807dd4d8aba9d33e9bdeffd0
Reviewed-on: https://chromium-review.googlesource.com/459230
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-03-24 19:32:04 +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
Mark Mentovai
60be5a66a0 net: Provide better HTTP User-Agent strings
Previously, macOS used “User-Agent: crashpad_util_test (unknown version)
CFNetwork/807.2.14 Darwin/16.4.0 (x86_64)” and Windows gave results like
“User-Agent: Crashpad/0.8.0”.

Now, macOS uses “User-Agent: Crashpad/0.8.0 CFNetwork/807.2.14
Darwin/16.4.0 (x86_64)” and Windows uses “User-Agent: Crashpad/0.8.0
WinHTTP/10.0.14393.351 Windows_NT/10.0.14393.0 (x64)”

Change-Id: I578b44734cf59d79e3d9b6136b4b92f05acefe71
Reviewed-on: https://chromium-review.googlesource.com/447796
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-03-01 17:44:08 +00:00
Mark Mentovai
546e64cd0b Centrally define CPUContextX86::Fsave and fsave↔︎fxsave conversions
As I was finishing d98a4de718d9, it became evident that fsave
proliferation was becoming a problem. Especially considering tests,
there was much duplicated conversion code. This ties everything up
together in a central location.

test::BytesToHexString() is a new function to ease testing of byte
arrays like x87 registers, without having to loop over each byte.

Some static_asserts are added to verify that complex structures that
need to maintain interoperability don’t grow or shrink. This is used
to check the size of the fxsave and fsave structures, as well as the
MinidumpCPUContext* structures.

BUG=crashpad:162

Change-Id: I1a1be18096ee9be250cbfb2e006adfd08eba8753
Reviewed-on: https://chromium-review.googlesource.com/444004
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-02-16 18:26:13 +00:00
Mark Mentovai
d98a4de718 win: support native x86 CONTEXT structures with x87 but no SSE context
When no SSE (fxsave) context is available but x87 (fsave) context is, use the
x87 context.

This also embeds the x87 FPU opcode from the fxsave fop field in bits 16-26 of
the fsave error_selector field, true to the layout of the fsave structure. See
Intel SDM volume 1 (253665-061) 8.1.10 and figure 8-9.

BUG=crashpad:161
TEST=crashpad_snapshot_test CPUContextX86.*:CPUContextWin.*

Change-Id: I0bf7ed995c152f124166eaa20104d228d3468f76
Reviewed-on: https://chromium-review.googlesource.com/442144
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-02-15 17:39:50 +00:00
Mark Mentovai
88442dd578 Merge Chromium 294442c0ce05 upstream to Crashpad
Remove stl_util from Crashpad. This also updates mini_chromium to
4f3cfc8e7c2b7d77f94f41a32c3ec84a6920f05d to remove stl_util from there
as well.

4f3cfc8e7c2b Remove stl_util from mini_chromium

BUG=chromium:555865

Change-Id: I8ecb1639a258dd233d524834ed205a4fcc641bac
Reviewed-on: https://chromium-review.googlesource.com/438865
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-02-07 21:04:42 +00:00
Scott Graham
0567536f86 win: Attempt to fix unloaded modules list by using RtlGetUnloadEventTraceEx
I haven't been able to reproduce this locally, but we see errors in
crash dumps where the unloaded module list consists of a number of
modules with invalid names and implausible addresses. My assumption is
that RTL_UNLOAD_EVENT_TRACE isn't correct for some OS levels. Instead of
trying to finesse and test that, use RtlGetUnloadEventTraceEx() instead
of RtlGetUnloadEventTrace(), which returns an element size. (This
function is Vista+ which is why it wasn't used the first time around.)

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

Change-Id: I4d7080a03623276f9c1c038d6e7329af70e4a64c
Reviewed-on: https://chromium-review.googlesource.com/421564
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-12-16 20:32:25 +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
57b2210ed7 GCC fix: Make UUID POD
This eliminates all constructors, but nearly all points of use were
using the default constructor to initialize a UUID member variable as in
uuid_(). This syntax will still produce a zeroed-out UUID.

While compiling, for example, minidump_rva_list_writer.cc:

In file included from ../../minidump/minidump_rva_list_writer.h:25:0,
                 from ../../minidump/minidump_rva_list_writer.cc:15:
../../minidump/minidump_extensions.h:412:8: error: ignoring packed attribute because of unpacked non-POD field ‘crashpad::UUID crashpad::MinidumpCrashpadInfo::report_id’ [-Werror]
   UUID report_id;
        ^~~~~~~~~
../../minidump/minidump_extensions.h:424:8: error: ignoring packed attribute because of unpacked non-POD field ‘crashpad::UUID crashpad::MinidumpCrashpadInfo::client_id’ [-Werror]
   UUID client_id;
        ^~~~~~~~~

Tested with:
 - GCC 4.9 from NDK r13 targeting arm with SDK 16
 - GCC 4.9 from NDK r13 targeting arm64 with SDK 21
 - GCC 6.2 targeting x86_64

BUG=crashpad:30

Change-Id: Iec6b1557441b69d75246f2f75c59c4158fb7ca29
Reviewed-on: https://chromium-review.googlesource.com/409641
Reviewed-by: Scott Graham <scottmg@chromium.org>
2016-11-11 17:37:14 +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
Scott Graham
b47bf6c250 Fix tests when running on Win10
The Windows 10 loader starts a few extra threads before main(). In a few
of the test cases, the tests were relying on thread ordering (generally,
the test thread being at index #1). Instead, use other signals to find
the correct thread to verify.

R=mark@chromium.org

Change-Id: Icb1f5a8fdf3a0ea6d82ab65960dbcb650965f269
Reviewed-on: https://chromium-review.googlesource.com/407073
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-11-03 18:51:01 +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
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
Mark Mentovai
9807cba2f4 end_to_end_test: accept mangled anonymous namespaces from cdb
When crashy_test_program's SomeCrashyFunction is inlined into
CrashyMain, cdb doesn't demangle the decorated form of an anonymous
namespace (?A0x12345678) into the expected `anonymous namespace' string.

I experienced this in Release_x64 and Release modes using MSVS 2015
update 3 (14.0.25420.1, cl 19.00.24213.1) and cdb versions 10.0.10240.9
and 10.0.14321.1024.

BUG=crashpad:129

Change-Id: I0a665b88891c271253adccd9b2b414fcaac26c8f
Reviewed-on: https://chromium-review.googlesource.com/368730
Reviewed-by: Scott Graham <scottmg@chromium.org>
2016-08-12 17:33:28 +00:00
Scott Graham
f497e54ccb win: Fix indirectly gathered memory cap
The limit was being reset per-thread, which isn't how it was intended or
documented. Add test to confirm the limit is more-or-less respected.

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

Change-Id: Ifae9f1ce2afcc2d6c6832db46f9b5c36adb35b42
Reviewed-on: https://chromium-review.googlesource.com/346131
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-05-20 15:47:28 +00:00
Scott Graham
b22cca6c3b win: Avoid variable shadowing warning for thread_id on VS2015
R=mark@chromium.org

Change-Id: I0bf09c96715161827bdad70bb375ad8193456d28
Reviewed-on: https://chromium-review.googlesource.com/342634
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-05-05 17:25:21 +00:00
Scott Graham
ab01df1ffe win: Adjust thread suspend count for DumpAndCrashTargetProcess() case
Because DumpAndCrashTargetProcess() suspends the process, the thread
suspend count is one too high for all threads other than the injection
one in the thread snapshots. Compensate for this when we detect this
type of exception.

BUG=crashpad:103

Change-Id: Ib77112fddf5324fc0e43f598604e56f77d67ff54
Reviewed-on: https://chromium-review.googlesource.com/340372
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-05-02 18:39:29 +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