1091 Commits

Author SHA1 Message Date
Mark Mentovai
b42854dfe1 Fix Doxygen after 8c802aace407
Change-Id: I9fe34c0a0322f327e7a69c831b11daa1cf835324
Reviewed-on: https://chromium-review.googlesource.com/541057
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-06-20 15:53:31 +00:00
Mark Mentovai
c4f6ca3c6a mac: Provide a larger thread state buffer for AVX-512 on 10.13
Crashpad doesn’t use AVX-512, but when receiving replies to exceptions
forwarded to ReportCrash, may see buffers large enough to contain
AVX-512 thread state. This can result in messages like
“UniversalExceptionRaise: (ipc/rcv) msg too large (0x10004004)”.

I386_THREAD_STATE_MAX has increased from 224 to 614 in the 10.13 SDK,
meaning that the maximum supported size for old_state and new_state in
[mach_]exception_raise_state[_identity]() has increased from 896 to
2,456 bytes. This constant defines the size of the buffer that these
MIG-generated routines will work with. By providing this definition in
compat, the buffer size is increased when building with older SDKs.

Note that on the “send” side, the size of the message given to
mach_msg() will be trimmed to include only the valid part of the state
area based on the stateCnt field, so increasing the value to 614 here
won’t result Crashpad sending messages this large. That would be a
potential interoperability concern with older OS versions.

Bug: crashpad:185, crashpad:190
Change-Id: Ia46091ae46fd6227a17f59eb4bc00914be471aa7
Reviewed-on: https://chromium-review.googlesource.com/541515
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-06-20 14:31:38 +00:00
Joshua Peraza
8c802aace4 Add ReinterpretBytes which does a checked, variable size bit cast
This renames and improves the VariableSizeBitCast helper from
util/linux/auxiliary_vector.* and moves it to misc.

Change-Id: I4bf46f4cfc0e60c900ff9bde467a21ad43c684cd
Reviewed-on: https://chromium-review.googlesource.com/534174
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-06-19 23:15:43 +00:00
Joshua Peraza
d3e4f09742 linux: Collect fxsave instead of fsave in ThreadInfo
Bug: crashpad:30
Change-Id: Ib4abf0ad60b792c8241b28e6b5e47970fdfcf451
Reviewed-on: https://chromium-review.googlesource.com/537532
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
2017-06-16 16:16:00 +00:00
Mark Mentovai
63ccbd0e4c Remove compiler_specific.h #include from aligned_allocator.h
This was missed in Crashpad 8c35d92ae403. It syncs with Chromium
16289b3ef759.

Change-Id: I7e92e71fc940e25e751e7487d100b5684bdbf667
Reviewed-on: https://chromium-review.googlesource.com/535577
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-06-14 20:37:08 +00:00
Mark Mentovai
890ad441b3 mac: Accept modules in 10.13’s dyld shared cache
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>
2017-06-14 19:49:44 +00:00
Mark Mentovai
6108d25232 mac: Update the process_types version of dyld_all_image_infos for 10.13
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>
2017-06-14 18:44:17 +00:00
Mark Mentovai
107fb76317 mac: Handle _dyld_get_all_image_infos() not being available on 10.13
_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>
2017-06-14 15:08:05 +00:00
Mark Mentovai
2851e5cfc8 mac: Update cl_kernels workaround for macOS 10.13 (and later)
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>
2017-06-13 19:47:59 +00:00
Mark Mentovai
f845797732 mac: 10.13 SDK compatibility, adapt to x86_state_hdr changes
In the 10.12 SDK, x86_state_hdr from <mach/i386/thread_status.h> was
defined as:

struct x86_state_hdr {
  int flavor;
  int count;
};

This has changed in the 10.13 SDK to:

struct x86_state_hdr {
  uint32_t flavor;
  uint32_t count;
};

This triggers signedness mismatch errors where these values are used
with CHECK/DCHECK macros and gtest EXPECT/ASSERT macros.

Compatibility with existing and new SDKs must be maintained, so more
casts must be used.

Bug: crashpad:185, crashpad:188
Change-Id: I8844d6a78520430a8b5b90a35403896c3c6cfa37
Reviewed-on: https://chromium-review.googlesource.com/533375
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-06-13 18:46:25 +00:00
Mark Mentovai
8c35d92ae4 Use the C++11-standardized alignof instead of ALIGNOF
Use the standard alignas instead of ALIGNAS in cases where this is
possible too. It’s not currently possible where ALIGNAS may be mixed
with other attributes, although the not-landed
https://codereview.chromium.org/2670873002/ suggests that where ALIGNAS
is mixed with __attribute__((packed)), it’s viable to write “struct
alignas(4) S { /* … */ } __attribute__((packed));”.

This includes an update of mini_chromium to
723e840a2f100a525f7feaad2e93df31d701780a, picking up:

723e840a2f10 Remove ALIGNOF

This tracks upstream https://codereview.chromium.org/2932053002/.

Change-Id: I7ddaf829020ef3be0512f803cecbb7c543294f07
Reviewed-on: https://chromium-review.googlesource.com/533356
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-06-13 18:33:35 +00:00
Joshua Peraza
1c0c305bc9 linux: Add FindMappingWithName to MemoryMap
Bug: crashpad:30
Change-Id: I5e03dc14e3cd1e09ac45cba97922499ec48ea389
Reviewed-on: https://chromium-review.googlesource.com/532753
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-06-13 16:00:01 +00:00
Joshua Peraza
8e2e805fa5 linux: Add AuxiliaryVector for reading other process' aux vectors
Bug: crashpad:30
Change-Id: Ief19be7d60decb17f159b3d740ac9d15a034b807
Reviewed-on: https://chromium-review.googlesource.com/526533
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-06-12 20:20:42 +00:00
Scott Graham
dbc229a2d7 Update mini_chromium to 606ff8a3
> git log --oneline ef0ded87..606ff8a3
606ff8a Remove base/memory/aligned_memory.*

Change-Id: Id3b1b75f2e18437543dc4703f6b2dc578ac7fa75
Reviewed-on: https://chromium-review.googlesource.com/530071
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
2017-06-09 21:50:59 +00:00
Joshua Peraza
8fb23f2acc linux: Provide ThreadInfo to collect register sets with ptrace
ThreadInfo provides a uniform interface to collect register sets or
the thread-local storage address across bitness for x86 and ARM family
architectures. Additionally, ThreadInfo.h defines context structs which
mirror those provided in sys/user.h. This allows tracing across bitness
as the structs in sys/user.h are only provided for a single target
architecture.

Bug: crashpad:30
Change-Id: I91d0d788927bdac5fb630a6ad3c6ea6d3645ef8a
Reviewed-on: https://chromium-review.googlesource.com/494075
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-06-01 19:25:06 +00:00
Mark Mentovai
5ebd24e96e Upload to the production Chromium Gerrit instance, not the canary
Change-Id: Iad3bf52ba5f7babb1c6b3508fc034ab78949967d
Reviewed-on: https://chromium-review.googlesource.com/509933
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-05-19 20:56:57 +00:00
Mark Mentovai
f53f2c84cc Fix comments identifying the source of module TimeDateStamp information
Change-Id: I164f0208db103410c3133a67a994a4f603ce1b27
Reviewed-on: https://chromium-review.googlesource.com/494827
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-05-03 21:13:05 +00:00
Pierre-Antoine Manzagol
7d56fd2386 Rely on winsock2.h for timeval
Bug: crashpad:
Change-Id: Iee8ebfaf7c4a1e8e87fcfcbc6ee8a4529a2f7c52
Reviewed-on: https://chromium-review.googlesource.com/493893
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Pierre-Antoine Manzagol <manzagop@chromium.org>
2017-05-03 17:53:45 +00:00
Mark Mentovai
1969a5d758 Document who has access to the try server and commit queue
Drop the text recommending the PolyGerrit UI, since it is now the
default Gerrit UI.

Bug: chromium:717982
Change-Id: I7041ee51670a7a18b510ed7a55045cc2eb09983e
Reviewed-on: https://chromium-review.googlesource.com/494726
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-05-03 17:49:24 +00:00
Mark Mentovai
d9ca2ad21f Give group project-crashpad-tryjob-access access to the commit queue
Bug: chromium:717982
Change-Id: I826f7520409656f5f549a110895e46de111d17f4
Reviewed-on: https://chromium-review.googlesource.com/494666
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-05-03 16:00:30 +00:00
Mark Mentovai
dc60e106f3 linux: Make fewer (but still a lot of) regions in MemoryMap’s test
The lots-of-regions tests in the MemoryMap test case were very
time-consuming, particularly in debug mode. MemoryMap.MapRunningChild
took as long as 15 seconds on-device (Nexus 5X), and the best result was
in the neighborhood of 7 seconds.

The bulk of the time spent in these tests was in ExpectMappings(), which
calls MemoryMap::FindMapping() in a loop to verify each region. Each
call to FindMapping() traverses the MemoryMap (internally, currently
just a std::vector<>) from the beginning. With the need to verify 4,096
regions, a single call to ExpectMappings() had to perform over 8,000,000
checks to find the regions it needed. In turn, ExpectMappings() is
called once by the SelfLargeMapFile test, and eight times by
MapRunningChild. By reducing the number of regions to 1,024, each call
to ExpectMappings() needs to perform “only” fewer than 600,000 checks.

After this change, MemoryMap.MapRunningChild completes in about a half a
second on-device.

https://crashpad.chromium.org/bug/181 is concerned with implementing a
RangeMap to serve MemoryMap and other similar code. After that’s done,
it, it should be feasible to raise the number of regions used for these
tests again.

Bug: crashpad:30, crashpad:181
Test: crashpad_util_test MemoryMap.SelfLargeMapFile:MemoryMap.MapRunningChild
Change-Id: I8ff88dac72a63c97ac937304b578fbe3b4ebf316
Reviewed-on: https://chromium-review.googlesource.com/494128
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-05-02 21:18:53 +00:00
Mark Mentovai
abbeffead9 Fix file descriptor/handle leak in LoggingReadEntireFile()
8af3203d811c introduced a resource leak.

Change-Id: Ia909eef39b6b772d8808dd6f5770c06add6467bc
Reviewed-on: https://chromium-review.googlesource.com/493946
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-05-02 19:50:28 +00:00
Joshua Peraza
51779ce639 linux: Make MemoryMap retry when duplicates are detected
When the /proc/pid/maps file is not read atomically and the target
process is actively mapping memory, entries can be read multiple times
or missed entirely. This change makes MemoryMap read the whole contents
of the maps file before attempting to parse it as well as check for
duplication/overlap errors, retrying on failure. This change also adds
ptrace attachements to unit tests to reflect actual intended usage.

Bug: crashpad:30
Change-Id: Ie8549548e25c47baa418ee7439d82743f84ff41e
Reviewed-on: https://chromium-review.googlesource.com/491950
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
2017-05-02 17:28:31 +00:00
Joshua Peraza
8af3203d81 Add LoggingReadEntireFile for reading a file into a string
Change-Id: Ie07ef12131ef1d995aa78749091f3adacde75160
Reviewed-on: https://chromium-review.googlesource.com/492446
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-05-02 04:01:48 +00:00
Mark Mentovai
f03c7b2d8f mac: Trigger a real SIGSYS on 32-bit x86 during tests
syscall(0) results in SIGSYS on x86_64, but not 32-bit x86. Choose a
high number as a nonexistent syscall number. As of 10.12.4, the highest
known system call number is 521.

Test: crashpad_util_test Signals.Cause*
Change-Id: I82dbd210f0c90fe933898ea0d360b431b10d090e
Reviewed-on: https://chromium-review.googlesource.com/489826
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-05-01 15:54:51 +00:00
Mark Mentovai
15103742e0 Use FromPointerCast<>() in many places where it makes sense
I opted to leave casts to types that were definitely the same size
alone. reinterpret_cast<uintptr_t>(pointer) and
reinterpret_cast<intptr_t>(pointer) should always be safe, for example.
Casts to other integral types have been replaced with
FromPointerCast<>(), which does zero-extension or sign-extension based
on the target type.

To make it possible to use FromPointerCast<>() with some use sites that
were already using checked_cast<>(), FromPointerCast<>() now uses
check_cast<>() when converting to a narrower type.

Test: crashpad_util_test FromPointerCast*, others
Change-Id: I4a71b4aa2d87f545c75524290a702f5f3138d675
Reviewed-on: https://chromium-review.googlesource.com/489701
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-05-01 15:54:00 +00:00
Mark Mentovai
984749479f Introduce FromPointerCast<>(), with defined sign/zero-extension behavior
Some of the new Linux/Android tests were failing in 32-bit code where
pointers were being casted via reinterpret_cast<>() to LinuxVMAddress,
an unsigned 64-bit type. The behavior of such casts is
implementation-defined, and in this case, sign-extension was being used
to convert the 32-bit pointers to 64 bits, resulting in very large
(unsigned) LinuxVMAddress values that could not possibly refer to proper
addresses in a 32-bit process’ address space.

The offending reinterpret_cast<>() conversions have been replaced with
the new FromPointerCast<>(), which is careful to do sign-extension when
converting to a signed type, and zero-extension when converting to an
unsigned type like LinuxVMAddress.

Bug: crashpad:30
Test: crashpad_util_test FromPointerCast*:MemoryMap.*:ProcessMemory.*
Change-Id: I6f1408dc63369a8740ecd6015d657e4407a7c271
Reviewed-on: https://chromium-review.googlesource.com/488264
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-04-27 19:42:25 +00:00
Mark Mentovai
ed8e637817 linux: Fill a test file with zeroes instead of garbage in MemoryMapTest
Bug: crashapd:30
Test: MemoryMap.MapChild
Change-Id: I40cd1c3a1f37e7a9d0c344c50b79b15ae3842182
Reviewed-on: https://chromium-review.googlesource.com/486602
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-04-25 20:05:14 +00:00
Mark Mentovai
44e32fe123 Tweak InitializationState tests that rely on undefined behavior
These tests:
 - InitializationState.InitializationState
 - InitializationStateDcheckDeathTest.Destroyed_NotUninitialized
 - InitializationStateDcheckDeathTest.Destroyed_NotValid
rely on certain behavior from destroyed objects. This is undefined
behavior and we know it, but the whole point of the of
InitializationState and InitializationStateDcheck destructors is to try
to help catch other parts of the program making use of undefined
behavior.

To make it impossible for the memory that formerly hosted these objects
to be repurposed during tests after the objects are destroyed, these
tests that attempt to work with destroyed objects are changed to use
placement new, so that the lifetimes of the objects can be decoupled
from the lifetimes of the buffers.

Test: crashpad_util_test InitializationState*
Change-Id: Ie972a54116c8b90a21a502d3ba13623583dfac06
Reviewed-on: https://chromium-review.googlesource.com/486383
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-04-25 18:09:49 +00:00
Mark Mentovai
f31459b266 Update GYP to ffd524cefaad for MSVS 2017 (15)/C++ 14.1/C 19.10 support
aae1e3efb507 CQ config: add gerrit CQAbility verifier.
95da7665b1a3 [win-test] loosen win-driver-target-type test
eb296f67da07 [win] Add support for MS VS2017 (via Registry)
19495aa28282 Update test/no-cpp/gyptest-no-cpp.
a94b02ec68fb Disable a bunch of tests on Mac
ae76d9198630 Clean up gyptest.py
b62d04ff85e6 win,ninja: ninja generator better on windows
8dc77241251e Disable flaky test/copies/gyptest-all under msvs
e8850240a433 Fix MSVC++ 32-on-32 builds after b62d04ff85e6
ffd524cefaad win ninja/make: Always use a native compiler executable
             with MSVS 2017

developing.md is updated to call out supported toolchain versions, and
to explain the CDB requirement for end_to_end_tests.py.

Change-Id: Iace68009aa22acec7303ea02a2ded755645ea96c
Reviewed-on: https://chromium-review.googlesource.com/486539
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-25 16:43:10 +00:00
Joshua Peraza
4036e2c9d9 linux: Add MemoryMap to collect information about mapped memory regions
Bug: crashpad:30
Change-Id: Id11d549829bd1a956d31991d4b829a43ce5696aa
Reviewed-on: https://chromium-review.googlesource.com/477597
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-04-25 15:33:52 +00:00
Mark Mentovai
fd8e2de0c5 win: MSVS 2017 (15)/C++ 14.1/C 19.10 compatibility
Includes mini_chromium ef0ded8717340c9fe48e8e0f34f3e0e74d10a392.

1d2a024fdb1d android: Use _FILE_OFFSET_BITS after all (undo
             dc3d480305b2)
ef0ded871734 win: MSVS 2017 (15)/C++ 14.1/C 19.10 compatibility

Change-Id: I5c814669a0ef8577872bddff9112ce28ec628ba3
Reviewed-on: https://chromium-review.googlesource.com/482639
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-20 00:28:35 +00:00
Joshua Peraza
2815dbdf8e linux: Add CheckedLinuxAddressRange and make CheckedAddressRanges copyable
Bug: crashpad:30
Change-Id: Ied2b8659315c09c77054c0a5a82ac37284f27334
Reviewed-on: https://chromium-review.googlesource.com/481036
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-04-19 20:46:54 +00:00
Mark Mentovai
f487da4ff2 win handler: Move test targets to handler_test.gyp
Test: end_to_end_test
Change-Id: I1fb01e0a6e701c8ec3958b68e2665cd4348a2242
Reviewed-on: https://chromium-review.googlesource.com/481083
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-19 18:39:52 +00:00
Mark Mentovai
74fddc3fed win: Wrap test::ChildLauncher::Start() in ASSERT_NO_FATAL_FAILURE()
Test: crashpad_snapshot_test, crashpad_util_test, end_to_end_test
Change-Id: I09581521678fe3b083c409f308eeab2e583b3c9f
Reviewed-on: https://chromium-review.googlesource.com/481245
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-19 17:47:23 +00:00
Mark Mentovai
e04194afd9 win: Wrap TerminateProcess() to accept cdecl patches on x86
TerminateProcess(), like most of the Windows API, is declared WINAPI,
which is __stdcall on 32-bit x86. That means that the callee,
TerminateProcess() itself, is responsible for cleaning up parameters on
the stack on return. In https://crashpad.chromium.org/bug/179, crashes
in ExceptionHandlerServer::OnNonCrashDumpEvent() were observed in ways
that make it evident that TerminateProcess() has been patched with a
__cdecl routine. The crucial difference between __stdcall and __cdecl is
that the caller is responsible for stack parameter cleanup in __cdecl.
The mismatch means that nobody cleans parameters from the stack, and the
stack pointer has an unexpected value, which in the case of the Crashpad
handler crash, results in TerminateProcess()’s second argument
erroneously being used as the lock address in the call to
ReleaseSRWLockExclusive() or LeaveCriticalSection().

As a workaround, on 32-bit x86, call through SafeTerminateProcess(), a
custom assembly routine that’s compatible with either __stdcall or
__cdecl implementations of TerminateProcess() by not trusting the value
of the stack pointer on return from that function. Instead, the stack
pointer is restored directly from the frame pointer.

Bug: crashpad:179
Test: crashpad_util_test SafeTerminateProcess.*, others
Change-Id: If9508f4eb7631020ea69ddbbe4a22eb335cdb325
Reviewed-on: https://chromium-review.googlesource.com/481180
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-19 17:45:32 +00:00
Mark Mentovai
ffe4c1018c net: Update Blink source code references
The references to RFC 2388 §3 and RFC 2047 are removed. RFC 7578 has
replaced RFC 2388, and RFC 7578 acknowledges that the area of RFC 2388
called into question by the previous comment in this code was not widely
adopted. The code does not violate RFC 7578, so the TODO is removed.

Change-Id: Ie68cba49f9fbc95a4ae3a156783a6db3b406950c
Reviewed-on: https://chromium-review.googlesource.com/481244
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-04-19 16:08:53 +00:00
Mark Mentovai
b8aaa22905 mac handler: Record a file-limits annotation (temporarily)
The "file-limit" annotation will be used to confirm the theory that
certain crashes are caused by systems at or near their file descriptor
table size limits.

The annotation records the system-wide kern.num_files and kern.maxfiles
values, and the process-specific current and maximum file descriptor
limits.

The annotation will be set on crashpad_handler startup, and will be
refreshed every time an exception is handled and every time the upload
thread processes a pending report.

It’s expected that this annotation will be removed after enough data has
been collected to confirm the theory. However, the principle is useful
enough that we may want to provide this feature more generally under
bugs 19 or 21.

Bug: crashpad:180
Change-Id: I3bb78fae60e0567bc4ac2625716e0abe0ddae08c
Reviewed-on: https://chromium-review.googlesource.com/479914
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-04-18 17:27:31 +00:00
Mark Mentovai
ddcc74f08f mac: Tolerate dead names for reply ports in the exception handler server
Self-monitoring revealed this CHECK was being hit in the wild:

base::debug::BreakDebugger()                debugger_posix.cc:260
logging::LogMessage::~LogMessage()          logging.cc:759
logging::MachLogMessage::~MachLogMessage()  mach_logging.cc:45
crashpad::ExceptionHandlerServer::Run()     exception_handler_server.cc:108
crashpad::HandlerMain()                     handler_main.cc:744

The MACH_CHECK() was:

108        MACH_CHECK(mr == MACH_MSG_SUCCESS, mr) << "MachMessageServer::Run";

Crash reports captured the full message, including the value of mr:

[0418/015158.777231:FATAL:exception_handler_server.cc(108)] Check failed: mr == MACH_MSG_SUCCESS. MachMessageServer::Run: (ipc/send) invalid destination port (0x10000003)

0x10000003 = MACH_SEND_INVALID_DEST.

This can happen when attempting to send a Mach message to a dead name.
Send (and send-once) rights become dead names when the corresponding
receive right dies. This would not normally happen for exception
requests originating in the kernel. It can happen for requests
originating from a user task: when the user task dies, the receive right
dies with it. All it takes to trigger this CHECK() in crashpad_handler
is for a Crashpad client to die (or be killed) while the handler is
processing a SimulateCrash() that the client originated.

Accept MACH_SEND_INVALID_DEST as a valid return value for
MachMessageServer::Run().

Note that MachMessageServer’s test coverage was already aware of this
behavior. MachMessageServer::Run()’s documentation is updated to reflect
it too.

Change-Id: I483c065d3c5f9a7da410ef3ad54db45ee53aa3c2
Reviewed-on: https://chromium-review.googlesource.com/479093
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-04-17 21:20:40 +00:00
Mark Mentovai
c64fd3f9b4 Update mini_chromium to dc3d480305b27a5a1fb57f51a997529e00fed00b
2c1b54080cb2 android: Recognize Bionic’s semantics for strerror_r()
9a7a549b414d Terminate from CHECK(), LOG(FATAL) consistently
dc3d480305b2 android: Don’t set _FILE_OFFSET_BITS, it never worked

Bug: crashpad:30
Change-Id: I93f810efa17047b797491a9b1089ea8c80f81e41
Reviewed-on: https://chromium-review.googlesource.com/478252
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-04-14 21:28:58 +00:00
Mark Mentovai
8297b19a5e Don’t attempt to do periodic tasks in a secondary crashpad_handler
76a67a37b1d0 adds crashpad_handler’s --monitor-self argument, which
results in a second crashpad_handler instance running out of the same
database as the initial crashpad_handler instance that it monitors. The
two handlers start at nearly the same time, and will initially be on
precisely the same schedule for periodic tasks such as scanning for new
reports to upload and pruning the database. This is an unnecessary
duplication of effort.

This adds a new --no-periodic-tasks argument to crashpad_handler. When
the first instance of crashpad_handler starts a second to monitor it, it
will use this argument, which prevents the second instance from
performing these tasks.

When --no-periodic-tasks is in effect, crashpad_handler will still be
able to upload crash reports that it knows about by virtue of having
written them itself, but it will not scan the database for other pending
reports to upload.

Bug: crashpad:143
Test: crashpad_util_test ThreadSafeVector.ThreadSafeVector
Change-Id: I7b249dd7b6d5782448d8071855818f986b98ab5a
Reviewed-on: https://chromium-review.googlesource.com/473827
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-04-14 19:52:14 +00:00
Mark Mentovai
2ec34e32c2 linux: Support 4.10 format for empty Groups: lines in /proc/pid/status
The Groups: line unfortunately always had a trailing space, but Linux
4.10 takes this to a new level by including a trailing space even when
no groups are present. See commit f7a5f132b447,
linux-4.10.10/fs/proc/array.c task_state().

Bug: crashpad:30
Test: crashpad_util_test ProcessInfo.Pid1
Change-Id: If498abd929b27c7f28b69144e7c4928b1626acdb
Reviewed-on: https://chromium-review.googlesource.com/477070
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-04-13 17:53:36 +00:00
Mark Mentovai
5d07d81458 Fix Doxygen warnings after 30385d4e4772
Bug: crashpad:167
Change-Id: Ia12abd5298e4a2a3822d6641ef9d19eb05c41f38
Reviewed-on: https://chromium-review.googlesource.com/477012
Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org>
2017-04-13 17:52:43 +00:00
Mark Mentovai
bc7c6e235d mac: Prevent the same report from being uploaded multiple times
With multiple crashpad_handlers running out of the same database, it was
possible for more than one to attempt to upload the same report. Nothing
ensured that the reports remained pending between the calls to
CrashReportDatabaseMac::GetPendingReports() and
CrashReportDatabaseMac::GetReportForUploading().

The Windows equivalent did not share this bug, but it would return
kBusyError. kReportNotFound is a better code.

Test: crashpad_client_test CrashReportDatabaseTest.*
Change-Id: Ieaee7f94ca8e6f2606d000bd2ba508d3cfa2fe07
Reviewed-on: https://chromium-review.googlesource.com/473928
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-04-13 14:12:56 +00:00
Sigurdur Asgeirsson
1f28a123a4 designdoc: Add description of user data sources.
Bug: crashpad:167
Change-Id: I8c5032d6710ea828e354103743d34772fd083958
Reviewed-on: https://chromium-review.googlesource.com/472947
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-04-12 13:07:52 +00:00
Mark Mentovai
79425e4d97 win: Free an old buffer before attempting to allocate a resized one
When GetProcessInformation() obtains SystemProcessInformation, it
resizes its buffer as directed by NtQuerySystemInformation(). Nothing of
value resides in the old buffer if a resize is attempted, so it can be
freed before attempting to allocate a resized one.

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

Change-Id: I63b9b8c1efda22d2fdbf05ef2b74975b92556bbd
Reviewed-on: https://chromium-review.googlesource.com/473792
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-11 21:49:46 +00:00
Sigurdur Asgeirsson
30385d4e47 handler: Add user extensibility stream call-out.
Bug: crashpad:167
Test: Add crashpad_handler_test.
Change-Id: I79b0b71dc4f61e6dce6bc10083e2f924dc83c940
Reviewed-on: https://chromium-review.googlesource.com/463746
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-04-11 19:06:00 +00:00
Mark Mentovai
a5d81370be linux: Use pread64() instead of pread() in ProcessMemory
This fixes ProcessMemory for 32-bit processes. All ProcessMemory tests
were failing on 32-bit ARM on Android like this:

[ RUN      ] ProcessMemory.ReadSelf
[17345:17345:20170407,172222.579687:ERROR process_memory.cc:55] pread: Invalid argument (22)
../../../../util/linux/process_memory_test.cc:73: Failure
Value of: memory.Read(address, region_size_, result.get())
  Actual: false
Expected: true
[  FAILED  ] ProcessMemory.ReadSelf (5 ms)

Contemporary Linux doesn’t provide a pread() system call, it provides
pread64(), which operates on off64_t. pread() is a user-space wrapper
that accepts off_t. See Android 7.1.1
bionic/libc/bionic/legacy_32_bit_support.cpp pread().

Note that off_t is a signed type. With a 32-bit off_t, when the
“offset” parameter to pread() has its high bit set, it will be
sign-extended into the 64-bit off64_t, and when interpreted as a memory
address by virtue of being used as an offset into /proc/pid/mem, the
value will take on an incorrect meaning. In fact, the kernel will reject
it outright for its negativity. See linux-4.9.20/fs/read_write.c
[sys_]pread64().

Since ProcessMemory accepts its address parameter as a LinuxVMAddress,
which is wisely a uint64_t, it converts to off64_t properly, retaining
its original value.

Note, however, that the pread64() mechanism evidently cannot read memory
in the high half of a process’ address space even when pread64() is used
throughout. Most importantly, the (pos < 0) check in the kernel will be
tripped. Less importantly, the conversion of our unsigned LinuxVMAddress
to pread64’s signed off64_t, with the high bit set, is not defined. This
is not an immediate practical problem. With the exception of possible
shared pages mapped from kernel space (I only see this for the vsyscall
page on x86_64), Linux restricts 64-bit user process’ address space to
at least the lower half of the addressable range, with the high bit
clear. (The limit of the user address space is
linux-4.9.20/arch/x86/include/asm/processor.h TASK_SIZE_MAX =
0x7ffffffff000 for x86_64 and
linux-4.9.20/arch/arm64/include/asm/memory.h TASK_SIZE_64 =
0x1000000000000 at maximum for arm64.)

The 32-bit off_t may be a surprise, because
third_party/mini_chromium/mini_chromium/build/common.gypi sets
_FILE_OFFSET_BITS=64. Altough this macro is considered in the NDK’s
“unified headers”, in the classic NDK, this macro is never consulted.
Instead, off_t is always “long”, and pread() always gets the
compatibility shim in Bionic.

Bug: crashpad:30
Change-Id: Id00c882a3d521a46ef3fc0060d03ea0ab9493175
Reviewed-on: https://chromium-review.googlesource.com/472048
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-04-08 02:41:15 +00:00
Mark Mentovai
e142aa87d6 linux: Fix crashpad_util_test ScopedPtraceAttach.* with the Yama LSM
When Yama is enabled and /proc/sys/kernel/yama/ptrace_scope is set to 1
(YAMA_SCOPE_RELATIONAL), for a child to ptrace() its parent, the parent
must first call prctl(PR_SET_PTRACER, child_pid, ...).

Bug: crashpad:30
Test: crashpad_util_test ScopedPtraceAttach.*
Change-Id: Ic85e8551259f17f372b2362887e7701b833b4cb4
Reviewed-on: https://chromium-review.googlesource.com/472006
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-04-07 21:28:59 +00:00
Mark Mentovai
fd9f952393 Fix crashpad_util_test build with GCC after 4b450c813795
Change-Id: I968005ccb518f80c572d11d3443646cdb5de813e
Reviewed-on: https://chromium-review.googlesource.com/471946
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-04-07 20:06:42 +00:00