In 10.13, modules loaded from the dyld shared cache appear with __TEXT
segments that have a nonzero “fileoff” (file offset). Previously, the
fileoff was always 0. Previously, the fileoff for segments in the dyld
shared cache was the actual offset into the shared cache (not 0), but
special consideration was given to __TEXT segments which were forced to
0. See 10.12.4 dyld-433.5/interlinked-dylibs/OptimizerLinkedit.cpp
LinkeditOptimizer<>::updateLoadCommands(). Note the comment there where
the __TEXT segment’s apparent fileoff is set to 0:
// HACK until lldb fixed in: <rdar://problem/20357466>
// DynamicLoaderMacOSXDYLD fixes for Monarch dyld shared cache
Refer also to the lldb commit that references the above,
http://llvm.org/viewvc/llvm-project?view=revision&revision=233714.
Evidently, update_dyld_shared_cache has been revised to no longer apply
this hack in 10.13. Crashpad’s sanity check for __TEXT segments having a
fileoff of 0 is no longer valid, and causes it to reject modules loaded
from the dyld shared cache.
Since this was just a sanity check, remove it entirely.
This caused module information for modules loaded from the dyld shared
cache to be missing from minidumps produced on 10.13, which in turn
prevented symbolization in frames belonging to most system libraries.
For reasons not yet understood, I don’t see this problem in Chrome on
10.13db1 17A264c on a test virtual machine (HFS+ filesystem), although I
do see it on actual hardware (APFS filesystem), and I do see it in
Crashpad’s tests and reduced testcases on both as well.
Bug: crashpad:185, crashpad:189
Test: crashpad_snapshot_test MachOImageReader.Self_DyldImages:ProcessReader.SelfModules:ProcessReader.ChildModules:ProcessTypes.DyldImagesSelf
Change-Id: I8b0a22c55c33ce920804a879f6fab67272f3556e
Reviewed-on: https://chromium-review.googlesource.com/535576
Reviewed-by: Robert Sesek <rsesek@chromium.org>
10.13 introduces two new fields to dyld_all_image_infos. Oddly, it
doesn’t put them in the “reserved” area that was defined in this
structure. This addition made it necessary for the padding problem in
the 32-bit structure previously worked around in Crashpad to be
addressed in the native structure, so Crashpad’s definition is adapted
to match.
This fixes tests on 10.13 that verify that dyld_all_image_infos can be
interpreted correctly.
Note that although the 10.13 SDK includes this structure extension,
numbered version 16, 10.13db1 17A264c continues to use version 15 as
used on 10.12, at least in crashpad_snapshot_test.
Bug: crashpad:185
Test: crashpad_snapshot_test ProcessTypes.DyldImagesSelf
Change-Id: I59a80c85bb234ef698c65a0ac5bbeac5b40fda77
Reviewed-on: https://chromium-review.googlesource.com/535394
Reviewed-by: Robert Sesek <rsesek@chromium.org>
_dyld_get_all_image_infos() was only used in test code in Crashpad.
This addresses two related problems.
When running on 10.13 or later, _dyld_get_all_image_infos() is not
available. It appears to still be implemented in dyld, but its symbol is
now private. This was always known to be an “internal” interface. When
it’s not available, fall back to obtaining the address of the process’
dyld_all_image_infos structure by calling task_info(…, TASK_DYLD_INFO,
…). Note that this is the same thing that the code being tested does,
although the tests are not rendered entirely pointless because the code
being tested consumes dyld_all_image_infos through its own
implementation of an out-of-process reader interface, while the
dyld_all_image_infos data obtained by _dyld_get_all_image_infos() is
handled strictly in-process by ordinary memory reads. This is covered by
bug 187.
When building with the 10.13 SDK, no _dyld_get_all_image_infos symbol is
available to link against. In this case, access the symbol strictly at
runtime via dlopen() if it may be available, or when expecting to only
run on 10.13 and later, don’t even bother looking for this symbol. This
is covered by part of bug 188.
Bug: crashpad:185, crashpad:187, crashpad:188
Change-Id: Ib283e070faf5d1ec35deee420213b53ec24fb1d3
Reviewed-on: https://chromium-review.googlesource.com/534633
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Since Apple closed https://openradar.appspot.com/20239912 without fixing
anything, it looks like we’ll be stuck with these quriky cl_kernels
modules for quite some time. Allow these modules to be tolerated on any
OS version >= 10.10, where they first appeared in a broken state, by
removing the upper bound for the OS version to tolerate with this quirk.
The tolerance was previously expanded to include 10.11 in
cd1f8fa3d2f2c76802952beac71ad85f51bbf771 and 10.12 in
6fe7c5414e46acfa30e8984513bf0896e91b9407. After this third update, this
should hopefully no longer be an annual exercise.
Bug: crashpad:185, crashpad:186
Change-Id: I66d409f2d1638bcf7601b6622f000be245230f34
Reviewed-on: https://chromium-review.googlesource.com/534253
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>
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>
macOS 10.12.4 includes an updated timezone database. Abbreviations for
Australia/Eucla (formerly ACWST, now +0845) and Australia/Lord_Howe
(formerly LHST/LHDT, now +1030/+11) were dropped in IANA TZ 2017a. The
test is updated so that the abbreviations for these two time zones are
no longer checked.
References:
a25d615495https://mm.icann.org/pipermail/tz/2017-February/024837.html
Test: crashpad_snapshot_test SystemSnapshotMacTest.TimeZone
Change-Id: I2845c6aee7b7b6a8fcdc6faa4d5cefe5e0f72e5c
Reviewed-on: https://chromium-review.googlesource.com/461500
Reviewed-by: Scott Graham <scottmg@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>
952f787f4aab missed two occurrences that should have been updated.
Change-Id: I425367689eb19edfd309a2210a79ed400e190673
Reviewed-on: https://chromium-review.googlesource.com/458116
Reviewed-by: Robert Sesek <rsesek@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>
Lazy initialization is particularly beneficial for Is64Bit(), which uses
a different (ptrace()-based) approach than the rest of the class (which
is /proc-based). It is possible for the /proc-based Initialize() to
succeed while ptrace() would fail, as it typically would in the
ProcessInfo.Pid1 test. Because this test does not call Is64Bit(),
permission to ptrace() shouldn’t be necessary, and in fact ptrace()
shouldn’t even be called.
This enables the ProcessInfo.Pid1 test on Android (due to ptrace(), it
was actually failing on any Linux, not just Android). It also enables
the ProcessInfo.Forked test on non-Linux, as the prctl(PR_SET_DUMPABLE)
Linux-ism can be removed from it.
Bug: crashpad:30
Test: crashpad_util_test ProcessInfo.*
Change-Id: Ic883733a6aed7e7de9a0f070a5a3544126c7e976
Reviewed-on: https://chromium-review.googlesource.com/455656
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
Apple has responded to their bug 29079442 with a resolution stating that
these are not corpse ports but task ports that have changed after
execve(), as part of the large task port and execve() strategy rewrite
from 10.12.1. The comments being replaced were written before we had
10.12.1 source code. Now that we can see what’s going on, revise the
comments, and re-enable the task port check for the non-execve() test
variants.
https://openradar.appspot.com/29079442https://googleprojectzero.blogspot.com/2016/10/taskt-considered-harmful.html
Bug: crashpad:137
Test: crashpad_snapshot_test MachOImageAnnotationsReader.CrashDyld
Change-Id: I463637816085f4165b92b85a5b98bfeddcdf4094
Reviewed-on: https://chromium-review.googlesource.com/451120
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
crashpad_util should already have been the target to depend on
version.lib, but this wasn’t caught until something that depends on
crashpad_util but not crashpad_snapshot used that code, as
crashpad_util_test now does.
Change-Id: I1b7ced72c657946b297a328c0f89f51190d7d708
Reviewed-on: https://chromium-review.googlesource.com/448203
Reviewed-by: Scott Graham <scottmg@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>
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>
In locations where daylight saving time was once observed or is expected
to be observed in the future, but where no transitions to or from
daylight saving time occurred or will occur within a year of the current
date, act as though DST is not being observed at all.
Set TZ=America/Phoenix to test for this bug.
BUG=crashpad:130
TEST=crashpad_snapshot_test SystemSnapshotMacTest.TimeZone
Change-Id: Ie466b5906eab3c0cf2e51b962a171acb5b16210b
Reviewed-on: https://chromium-review.googlesource.com/438004
Reviewed-by: Robert Sesek <rsesek@chromium.org>
The 32-bit process_types definition of dyld_all_image_infos winds up
with four extra bytes of tail padding when built into a 64-bit
crashpad_handler compared to a 32-bit one, and compared to the
structure’s native size. This prevents a 64-bit crashpad_handler from
being able to create a module snapshot of a 32-bit process for
dyld_all_image_infos versions 14 (since 10.9) and 15 (since 10.12).
Work around this by placing a zero-length “end” marker and using
offsetof(dyld_all_image_infos, end) in preference to
sizeof(dyld_all_image_infos).
BUG=crashpad:156,crashpad:148,crashpad:120
TEST=crashpad_snapshot_test ProcessTypes.DyldImagesSelf,
run_with_crashpad --handler=crashpad_handler{,32} builtin_trap{,32}
Change-Id: I406ad53851b5bd29ec802b7ad3073836ebe8c34c
Reviewed-on: https://chromium-review.googlesource.com/437924
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Previously, only the top-level exception code was reported via the
Crashpad.ExceptionCode.Mac histogram. Making this histogram work
(https://crbug.com/678720) has revealed that Chrome is triggering
EXC_RESOURCE exceptions at a rate in excess of 4x that of ordinary
crashes. These exceptions were not previously visible because they are
not uploaded unless the system treats them as fatal, which it does not
normally do absent an explicit request.
In order to learn more about the problem, this change augments the data
reported via the Crashpad.ExceptionCode.Mac histogram to report (at
least) second-level exception data. This means that we will no longer
see just EXC_RESOURCE, but potentially more useful information such as
EXC_RESOURCE / RESOURCE_TYPE_IO / FLAVOR_IO_PHYSICAL_WRITES. This also
applies to other exception types, so that the majority of crashes
currently falling into the EXC_CRASH bucket will now have additional
information decoded and will be reported as, for example, EXC_BAD_ACCESS
/ KERN_INVALID_ADDRESS, EXC_BAD_INSTRUCTION / EXC_I386_INVOP, and
EXC_CRASH / SIGABRT.
Because the old mechanism was only live (in an “it works” sense) for
several days, and the new mechanism does not overlap with histogram
values used by the old one, there’s no need to invent a new histogram
name.
BUG=chromium:684051
Change-Id: Ia0a372b4127f7b3b2e7dbbaac9304cce3b5aadfe
Reviewed-on: https://chromium-review.googlesource.com/430933
Reviewed-by: Scott Graham <scottmg@chromium.org>
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>
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>
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>
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>
crashpad_snapshot_test MachOImageAnnotationsReader.CrashDyld was failing
on 10.12.1. In 10.12, dyld’s intentional crashes come through
abort_with_payload(). In 10.12.1, it appears that the task port sent
along with abort_with_payload() crashes is now a corpse port, which has
a different port name than the task port that it originated from.
https://openradar.appspot.com/29079442
TEST=crashpad_snapshot_test MachOImageAnnotationsReader.CrashDyld
BUG=crashpad:137
Change-Id: I43f89c0f595dd5614fc910fa1f19f21ddf0a7c4d
Reviewed-on: https://chromium-review.googlesource.com/407087
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>
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>
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>
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>
In 10.12, dyld calls abort_with_payload() on fatal error from
dyld::halt(). In previous 10.12 betas, abort_with_payload() caused the
process to appear to terminate as exit(1). This was weird, so I filed
https://openradar.appspot.com/26894758. In 10.12db4 16A270f, Apple seems
to have fixed this bug. abort_with_payload() as used by dyld now causes
the process to appear to terminate as abort() as I had requested.
A Crashpad test that assures Crashpad’s ability to catch dyld crashes
needs its expectations updated with each change to a process’ apparent
termination code. It’s updated to expect SIGABRT on 10.12 or later. No
concessions are made for previous 10.12 betas or their buggy exit(1)
behavior. Nobody should be running any 10.12 beta prior to 10.12db4
16A270f now or at any point in the future.
This undoes (redoes) 335ef494677f.
BUG=crashpad:120
TEST=crashpad_snapshot_test MachOImageAnnotationsReader.CrashDyld
Change-Id: I13b330ac83fc9b33907ac172d35983974b8910f0
Reviewed-on: https://chromium-review.googlesource.com/365920
Reviewed-by: Robert Sesek <rsesek@chromium.org>
The layout of dyld_all_image_infos changed slightly in 10.12db3 16A254g
and Xcode 8b3 8S174q.
BUG=crashpad:120
TEST=crashpad_snapshot_test ProcessTypes.DyldImagesSelf
Change-Id: I66fb60c80b26f465913f5100a8c40564723b0021
Reviewed-on: https://chromium-review.googlesource.com/361800
Reviewed-by: Robert Sesek <rsesek@chromium.org>
exit(1) is a weird code for this, so I filed
https://openradar.appspot.com/26894758.
This doesn’t completely fix bug crashpad:121 unless both
crashpad_snapshot_test and crashpad_snapshot_test_no_op are signed with
the same Developer ID certificate. I’m hoping to get some action on
https://openradar.appspot.com/26902656, which will enable a complete fix
for this bug in unsigned developer builds. It would be unusual to have
to sign test executables.
BUG=crashpad:120,crashpad:121
TEST=crashpad_snapshot_test MachOImageAnnotationsReader.CrashDyld
Change-Id: I54fdfaa9178029b91ea3cbc12f2760dfa5124858
Reviewed-on: https://chromium-review.googlesource.com/355260
Reviewed-by: Robert Sesek <rsesek@chromium.org>
The Mach-O reader validated segment and section file offsets by checking
that they were relative to the same base, insisting that a section’s
file offset be the same distance from a segment’s file offset as the
section’s preferred load address was from the segment’s preferred load
address. Notably, these file offsets already could not be validated
against the Mach-O image’s start because in the dyld shared cache, for
all segments other than __TEXT, these offsets were relative to the dyld
shared cache’s start.
In 10.12dp1 16A201w, file offsets for sections in the __TEXT segment are
also relative to the dyld shared cache’s start, but the file offset for
the __TEXT segment itself is relative to the Mach-O image’s start. Being
relative to different positions breaks Crashpad’s sanity check of the
module data. https://openradar.appspot.com/26864860 is filed for the use
of distinct bases in what should be related file offset fields.
While it would be possible with a bit of work to identify modules within
the dyld shared cache and adjust expectations accordingly, in reality,
these file offset values were only used to verify that the Mach-O
module.
In addition, the file offsets stored within the Mach-O file for sections
are 32-bit quantities, even in 64-bit images. It is possible to create a
large image whose section offset values have overflowed, and in these
cases, the offset value verification would also fail.
For these reasons, all file offset value validation is removed from the
Mach-O image reader.
BUG=crashpad:118, crashpad:120
Change-Id: I9c4bcc5fd0aeceef3bc8a43e5a8651735852d87b
Reviewed-on: https://chromium-review.googlesource.com/353631
Reviewed-by: Robert Sesek <rsesek@chromium.org>
