This is essentially based on a search for “^const .*=”.
Change-Id: I9332c1f0cf7c891ba1ae373dc537f700f9a1d956
Reviewed-on: https://chromium-review.googlesource.com/585452
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
This is essentially based on a search for “^ *const [^*&]*=[^(]*$”
Change-Id: Id571119d0b9a64c6f387eccd51cea7c9eb530e13
Reviewed-on: https://chromium-review.googlesource.com/585555
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
This uses “static” at function scope to avoid making local copies, even
in cases where the compiler can’t see that the local copy is
unnecessary. “constexpr” adds additional safety in that it prevents
global state from being initialized from any runtime dependencies, which
would be undesirable.
At namespace scope, “constexpr” is also used where appropriate.
For the most part, this was a mechanical transformation for things
matching '(^| )const [^=]*\['.
Similar transformations could be applied to non-arrays in some cases,
but there’s limited practical impact in most non-array cases relative to
arrays, there are far more use sites, and much more manual intervention
would be required.
Change-Id: I3513b739ee8b0be026f8285475cddc5f9cc81152
Reviewed-on: https://chromium-review.googlesource.com/583997
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
Debug registers are currently initialized to 0 until methods are added
to ThreadInfo to collect them.
Bug: crashpad:30
Change-Id: Ic1aab1151dcd4bed48eca8a60b76fb0d8d613418
Reviewed-on: https://chromium-review.googlesource.com/579889
Reviewed-by: Mark Mentovai <mark@chromium.org>
ProcessReader is responsible for collecting information needed to build
a snapshot of the target process, independent of the Snapshot
interface. This CL includes implementation and tests for collecting
thread information, but does not yet collect module information.
Bug: crashpad:30
Change-Id: I911f155c953129a5fa8c031e923c0de2bd740ce0
Reviewed-on: https://chromium-review.googlesource.com/488162
Reviewed-by: Mark Mentovai <mark@chromium.org>
Dynamic linkers use `struct r_debug` and `struct link_map` (defined in
`<link.h>`) to communicate lists of loaded modules to debuggers.
Bug: crashpad:30
Change-Id: Id903a1c199288dd85c34e38710cdb4c6b5fedb5b
Reviewed-on: https://chromium-review.googlesource.com/534853
Reviewed-by: Mark Mentovai <mark@chromium.org>
ELF executables and libraries may be loaded into memory in several
mappings, possibly with holes containing anonymous mappings
or mappings of other files. This method takes an input mapping and
attempts to find the mapping for file offset 0 of the same file.
Bug: crashpad:30
Change-Id: I79abf060b015d58ef0eba54a399a74315d7d2d77
Reviewed-on: https://chromium-review.googlesource.com/565223
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
107fb7631788 added the snapshot library as a dependency of
crashpad_util_test. Most of snapshot has not yet been ported to Linux or
Android. snapshot/capture_memory.cc only supports x86 and x86_64, and
will #error on other CPUs. We don’t build for other CPUs on Mac or
Windows, but we do for Android.
To make it easy to run crashpad_util_test on non-x86 again,
conditionally remove capture_memory.cc on Linux and Android.
crashpad_snapshot_test can be enabled for Linux and Android too by
disabling the CrashpadInfoClientOptions tests which require OS support.
There’s not much left in crashpad_snapshot_test currently for Linux
except for CPUContextX86 and ProcessSnapshotMinidump.EmptyFile, but both
pass.
Bug: crashpad:30
Change-Id: Ic19a79932072710c69a296bc0156cbe5656b8cb3
Reviewed-on: https://chromium-review.googlesource.com/549116
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
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>
