Previously, the mac version was under client/ and win under util/win/.
This cl brings them all together under util/misc/ and combines common
test code.
Bug: crashpad:30
Change-Id: Idf0d0158b969d5aa9802dfc8c21f73041b2bcc6c
Reviewed-on: https://chromium-review.googlesource.com/907755
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
In setting up the gn build, slightly different optimization settings
were applied for release builds. This caused a couple things to happen,
1) the sketchy noinline declspec was ignored, and 2) the distance
between reading the IP and the actual crash exceeded the tolerance of 64
bytes in the parent.
To make the test more robust to this, use CaptureContext() (I think our
improved version didn't exist at the time the tests was originally
written). Also, switch from crashpad::CheckedWriteFile to Windows'
WriteFile(), which avoids inlining a whole lot of code at that point.
The return value is not checked, but the next thing that happens is that
the function crashes unconditionally, so this does not seem like a huge
problem.
Bug: crashpad:79
Change-Id: I8193d8ce8b01e1533c16b207813c36d6d6113d89
Reviewed-on: https://chromium-review.googlesource.com/902693
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
kDoesNotObserveDaylightSavingTime can indicate only that the
standard/daylight transition is not automatic, as opposed to it not
existing at all.
Bug: crashpad:214
Change-Id: Ib7016806e79465a6dde605dd667b75a802e1b6c5
Reviewed-on: https://chromium-review.googlesource.com/904767
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Follows https://chromium-review.googlesource.com/c/374019/.
Causes MinidumpMemoryListWriter to merge all overlapping ranges before
writing the MINIDUMP_MEMORY_LIST. This is:
1) Necessary for the Google internal crash processor, which in some
cases attempts to read the raw memory (displaying ASAN red zones),
and aborts if there are any overlapping ranges in the minidump on
load;
2) Necessary for new-ish versions of windbg (see bug 216 below). It is
believed that this is a change in behavior in the tool that made
dumps with overlapping ranges unreadable;
3) More efficient. The .dmp for crashy_program goes from 306K to 140K
with this enabled. In Chrome minidumps where
set_gather_indirectly_referenced_memory() is used (in practice this
means Chrome Windows Beta, Dev, and Canary), the savings are expected
to be substantial.
Bug: crashpad:61, chromium:638370, crashpad:216
Change-Id: I969e1a52da555ceba59a727d933bfeef6787c7a5
Reviewed-on: https://chromium-review.googlesource.com/374539
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
It’s better to be prepared for the future than…to not be.
This is mostly the result of running 2to3 on all .py files, with some
small shims to maintain compatibility with Python 2.
http_transport_test_server.py was slightly more involved, requiring many
objects to change from “str” to “bytes”.
The #! lines and invokers still haven’t changed, so these scripts will
still normally be interpreted by Python 2.
Change-Id: Idda3c5650f967401a5942c4d8abee86151642a2e
Reviewed-on: https://chromium-review.googlesource.com/797434
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
I ran the thing below (piped to “grep -v namespace”), fixed things up,
and rewrapped comments in the affected file.
import re
import sys
LAST_WORD_RE = re.compile('^.*[\s]+([\w]+)$')
FIRST_WORD_RE = re.compile('^[^\w]+([\w]+).*$')
for path in sys.argv[1:]:
with open(path) as file:
line_number = 0
last_word = None
for line in file:
line_number += 1
first_word = FIRST_WORD_RE.match(line)
if first_word and first_word.group(1) == last_word:
print('%s:%u: %s' % (path, line_number - 1, last_word))
last_word = LAST_WORD_RE.match(line)
if last_word:
last_word = last_word.group(1)
Change-Id: Iea9f2a6453d9d9ec17e2f238e09252535d7408bd
Reviewed-on: https://chromium-review.googlesource.com/780284
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
Change-Id: I4b247d7fae1a212350f8ffcf2bf5ba1fa730f5c1
Reviewed-on: https://chromium-review.googlesource.com/780339
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
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>
When this test examines a module that doesn’t have a CodeView PDB link,
it will fail. Such a link may be missing when linking with Lexan
ld-link.exe without /DEBUG. The test had been examining the executable
as its module. Since it’s easier to provide a single small module linked
with /DEBUG than it is to require that the test executable always be
linked with /DEBUG, the test is revised to always load a module and
operate on it. The module used is the existing
crashpad_snapshot_test_image_reader_module.dll. It was chosen because
it’s also used by PEImageReader.DebugDirectory, which also requires a
CodeView PDB link.
It’s the build system’s responsibility to ensure that
crashpad_snapshot_test_image_reader_module.dll is linked appropriately.
Crashpad’s own GYP-based build always links with /DEBUG. Chrome’s
GN-based Crashpad build will require additional attention at
symbol_level = 0.
Bug: chromium:782781
Change-Id: I0dda8cd13278b82842263e76bcc46362bd3998df
Reviewed-on: https://chromium-review.googlesource.com/761501
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
crashpad_snapshot_test PEImageReader.DebugDirectory was hanging when
crashpad_snapshot_test_image_reader.exe did not have a CodeView PDB
link. This occurred when linked by Lexan ld-link.exe without /DEBUG.
Bug: chromium:782781
Change-Id: I8fbc4d8decf6ac5e19f7ffeb230fd15d7c40fd51
Reviewed-on: https://chromium-review.googlesource.com/761320
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
Change-Id: Ibecedd195224ea53ff36f376897a6ff3c4e773d2
Reviewed-on: https://chromium-review.googlesource.com/757085
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
This was previously proposed at
https://chromium-review.googlesource.com/c/crashpad/crashpad/+/339103/2/util/win/pe_image_reader_test.cc#84.
It didn’t land because the change was abandoned for other reasons, but
the fix was valid. nsi.dll is not VFT_APP or VFT_DLL, and if it’s
loaded, crashpad_snapshot_test PEImageReader.VSFixedFileInfo_AllModules
fails.
Although I can’t reproduce nsi.dll being loaded spontaneously in local
testing or on trybots, it occurred in the monolithic crashpad_tests at
https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/64492:
[ RUN ] PEImageReader.VSFixedFileInfo_AllModules
../../third_party/crashpad/crashpad/snapshot/win/pe_image_reader_test.cc(90): error: Value of: observed.dwFileType == VFT_APP || observed.dwFileType == VFT_DLL
Actual: false
Expected: true
Google Test trace:
../../third_party/crashpad/crashpad/snapshot/win/pe_image_reader_test.cc(164): C:\Windows\syswow64\NSI.dll
[ FAILED ] PEImageReader.VSFixedFileInfo_AllModules (11 ms)
I can also reproduce locally by calling LoadLibrary(L"nsi.dll").
Bug: chromium:779790, chromium:782011
Test: crashpad_snapshot_test PEImageReader.VSFixedFileInfo_AllModules
Change-Id: I361c7d6521645913277a441ce38779aaa4a182c2
Reviewed-on: https://chromium-review.googlesource.com/757077
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
This CL pulls together similar time conversion functions and adds
conversions between `FILETIME`s and `timespec`s.
Bug: crashpad:206
Change-Id: I1d9b1560884ffde2364af0092114f82e1534ad1c
Reviewed-on: https://chromium-review.googlesource.com/752574
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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: 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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
