439 Commits

Author SHA1 Message Date
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
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
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
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
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
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
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
Joshua Peraza
9c6d190b95 linux: Add ScopedPtraceAttach to manage ptrace attachments
Bug: crashpad:30
Change-Id: Ic5fb5adaaea88e31068b65a3c0dfff65a2a94743
Reviewed-on: https://chromium-review.googlesource.com/470331
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-04-07 19:14:36 +00:00
Joshua Peraza
46f4033773 posix: Add ScopedDIR for managing open directories
Change-Id: I9f1453db5e33e714c12ebeaaab25813a2b099de8
Reviewed-on: https://chromium-review.googlesource.com/468271
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-04-05 17:00:24 +00:00
Mark Mentovai
4b450c8137 test: Use (actual, [un]expected) in gtest {ASSERT,EXPECT}_{EQ,NE}
gtest used to require (expected, actual) ordering for arguments to
EXPECT_EQ and ASSERT_EQ, and in failed test assertions would identify
each side as “expected” or “actual.” Tests in Crashpad adhered to this
traditional ordering. After a gtest change in February 2016, it is now
agnostic with respect to the order of these arguments.

This change mechanically updates all uses of these macros to (actual,
expected) by reversing them. This provides consistency with our use of
the logging CHECK_EQ and DCHECK_EQ macros, and makes for better
readability by ordinary native speakers. The rough (but working!)
conversion tool is
https://chromium-review.googlesource.com/c/466727/1/rewrite_expectassert_eq.py,
and “git cl format” cleaned up its output.

EXPECT_NE and ASSERT_NE never had a preferred ordering. gtest never made
a judgment that one side or the other needed to provide an “unexpected”
value. Consequently, some code used (unexpected, actual) while other
code used (actual, unexpected). For consistency with the new EXPECT_EQ
and ASSERT_EQ usage, as well as consistency with CHECK_NE and DCHECK_NE,
this change also updates these use sites to (actual, unexpected) where
one side can be called “unexpected” as, for example, std::string::npos
can be. Unfortunately, this portion was a manual conversion.

References:

https://github.com/google/googletest/blob/master/googletest/docs/Primer.md#binary-comparison
77d6b17338
https://github.com/google/googletest/pull/713

Change-Id: I978fef7c94183b8b1ef63f12f5ab4d6693626be3
Reviewed-on: https://chromium-review.googlesource.com/466727
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-04 12:34:24 +00:00
Joshua Peraza
fa8ef92dc7 linux: Add ProcessMemory which reads another process' memory
Provides Read, ReadCString, and ReadCStringSizeLimited. Does not provide
ReadMapped because Linux does not support mmap on /proc/pid/mem.

Bug: crashpad:30
Change-Id: Ia319c0107b1f138aeb8e5d0ee480c77310df7202
Reviewed-on: https://chromium-review.googlesource.com/459700
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
2017-04-03 21:41:51 +00:00
Mark Mentovai
4688351623 “Promote” test::Paths::Executable() to Paths::Executable()
This supports the “double handler” or “double handler with low
probability” models from https://crashpad.chromium.org/bug/143.

For crashpad_handler to be become its own client, it needs access to its
own executable path to pass to CrashpadClient::StartHandler(). This was
formerly available in the test-only test::Paths::Executable(). Bring
that function’s implementation to the non-test Paths::Executable() in
util/misc, and rename test::Paths to test::TestPaths to avoid future
confusion.

test::TestPaths must still be used to access TestDataRoot(), which does
not make any sense to non-test code.

test::TestPaths::Executable() is retained for use by tests, which most
likely prefer the fatal semantics of that function. Paths::Executable()
is not fatal because for the purposes of implementing the double
handler, a failure to locate the executable path (which may happen on
some systems in deeply-nested directory hierarchies) shouldn’t cause the
initial crashpad_handler to abort, even if it does prevent a second
crashpad_handler from being started.

Bug: crashpad:143
Test: crashpad_util_test Paths.*, crashpad_test_test TestPaths.*
Change-Id: I9f75bf61839ce51e33c9f7c0d7031cebead6a156
Reviewed-on: https://chromium-review.googlesource.com/466346
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-04-03 18:58:01 +00:00
Mark Mentovai
c39e4dc976 mac: Remove obsolete comment about mach_msg_header_t::msgh_reserved
mig-generated server dispatch routines used to not clear this field in
reply messages prepared from request messages. This oversight was
corrected in the migcom in bootstrap_cmds-96 (macOS 10.12 and Xcode
8.0). Maybe someone at Apple saw the admonishing comment that we had
left here. This comment can now be removed.

Change-Id: I73d965705a2ff5788afb59dd8ecdf4afe58ee47e
Reviewed-on: https://chromium-review.googlesource.com/465687
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-04-03 15:55:30 +00:00
Mark Mentovai
385fe6615f posix: DCHECK for (addr + len) overflow in ScopedMmap::ResetAddrLen()
This also enhances ScopedMmapDeathTest.Mprotect to better ensure that
ScopedMmap::Mprotect() works properly.

Bug: crashpad:30
Test: crashpad_util_test ScopedMmap*.*
Change-Id: Iff35dba9fa993086f3f4cd8f4a862d802e637bb1
Reviewed-on: https://chromium-review.googlesource.com/464547
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-03-31 16:23:21 +00:00
Mark Mentovai
aa2bc55777 posix: Allow ScopedMmap::ResetAddrLen() to deal with overlap
It should be possible to shrink a region already supervised by
ScopedMmap, or in rare cases when ScopedMmap is supervising only a
smaller portion of an overall larger region, increase the size of the
region it supervises. This is now equivalent to the operation of
base::mac::ScopedMachVM::reset().

The Reset() and ResetAddrLen() methods are upgraded from a void return
to a bool return to indicate their success.

Bug: crashpad:30
Test: crashpad_util_test ScopedMmap*.ResetAddrLen_*
Change-Id: I564e154cd2387e8df3f83b416ecc1c83c9bcf71d
Reviewed-on: https://chromium-review.googlesource.com/464286
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-03-31 02:54:23 +00:00
Mark Mentovai
1a6ae8ce0b posix: Don’t accumulate zombies in ProcessInfo.Forked test
Use test::Multiprocess, which ensures that waitpid() is called to reap
child processes.

Previously, after a several thousand iterations (using --gtest_repeat),
fork() would begin failing with EAGAIN:

[ RUN      ] ProcessInfo.Forked
../../util/posix/process_info_test.cc:165: Failure
Expected: (pid) >= (0), actual: -1 vs 0
fork: Resource temporarily unavailable (35)
[  FAILED  ] ProcessInfo.Forked (0 ms)

Bug: crashpad:30
Test: crashpad_util_test ProcessInfo.Forked
Change-Id: Ia95c9297d5eeb02894f58844ced1b50981870cbc
Reviewed-on: https://chromium-review.googlesource.com/461482
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-03-28 22:18:38 +00:00
Mark Mentovai
96dc950eaf posix: Add ScopedMmap for managing memory-mapped regions
This wraps mmap(), munmap(), and mprotect().

Bug: crashpad:30
Test: crashpad_util_test ScopedMmap.*
Change-Id: If14363dfd00e314482cc91e53c7f4e3df737b0d3
Reviewed-on: https://chromium-review.googlesource.com/461361
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-03-28 18:30:14 +00:00
Mark Mentovai
270490ff79 win: Be more careful about child process exit codes
Checking child process’ exit codes would have helped catch bug
crashpad:160 sooner. Instead, we had a flaky hang that was difficult to
reproduce locally.

Bug: crashpad:160
Test: crashpad_snapshot_test ExceptionSnapshotWinTest.ChildCrash*:ProcessSnapshotTest.CrashpadInfoChild*:SimulateCrash.ChildDumpWithoutCrashing*, crashpad_util_test ProcessInfo.OtherProcess
Change-Id: I73bd2be1437d05f0501a146dcb9efbe3b8e0f8b7
Reviewed-on: https://chromium-review.googlesource.com/459039
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-03-24 20:12:31 +00:00
Mark Mentovai
8e37886d41 linux: Don’t make assumptions about the CPU revision
This code that works out the name of the CPU being built for is most
likely going to move out to be used more generally and for Android. It
should nail down the CPU name correctly when possible. Previously,
32-bit x86 always showed up as “i686” and 32-bit ARM always showed up as
“armv7l”.

Bug: crashpad:30
Change-Id: Ifd4b91f30062f5ef621a166f77a732dd8a88a58e
Reviewed-on: https://chromium-review.googlesource.com/458118
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-03-23 20:25:18 +00:00
Mark Mentovai
cedfd7b9cd android: Don’t use OPEN_MAX
NDK r13 and earlier provided a bogus definition of OPEN_MAX, but it was
removed from NDK r14 effective in a future API level. It is also not
available when using a standalone toolchain with unified headers.

ff5f17bc8a

Bug: crashpad:30
Change-Id: Ic89d6879cb1a4e5b9d20e9cb06bedd5176df0f2a
Reviewed-on: https://chromium-review.googlesource.com/458121
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-03-23 18:45:17 +00:00
Mark Mentovai
013d5e14a3 #include <stddef.h> where offsetof() is used
Bug: crashpad:30
Change-Id: If23ca9ea3141d3d34dc494aa29a1bd1dc8f83130
Reviewed-on: https://chromium-review.googlesource.com/458079
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-03-23 02:15:32 +00:00
Mark Mentovai
3983b80ca2 util/file: Handle oversized reads and writes gracefully
file_io and the FileReader family had a few loose ends regarding big
reads and writes. It’s not likely that we’ve experienced these
conditions yet, but they’d be likely to appear in a potential future
involving full memory dumps. This specifies the behavior with large
reads and writes, consolidates some logic, and improves some interfaces.

ReadFile() should always return without retrying after a short read, and
in fact does return after short reads since 00b64427523b. It is
straightforward to limit the maximum read size based on a parameter
limitation of the underlying operation, or a limitation of the type used
for FileOperationResult.

In contrast, WriteFile() should always retry after a short write,
including a write shortened because of a parameter limitation of the
underlying operation, or a limitation of the type used for
FileOperationResult. This allows its return value to be simplified to a
“bool”.

The platform-specific WriteFile() code has been moved to
internal::NativeWriteFile(), and the platform-independent loop that
retries following a short write has been refactored into
internal::WriteAllInternal so that it can be used by a new test.

The platform-agnostic ReadFileExactlyInternal() implementation has been
refactored into internal::ReadExactlyInternal so that it can be used by
a new test and by FileReaderInterface::ReadExactly(), which had a nearly
identical implementation.

Test: crashpad_util_test FileIO.ReadExactly_*:FileIO.WriteAll_*:FileReader.ReadExactly_*
Change-Id: I487450322ab049c6f2acd4061ea814037cc9a864
Reviewed-on: https://chromium-review.googlesource.com/456824
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-03-22 02:34:41 +00:00
Mark Mentovai
bc5b7b06db Fix racy WorkerThread test
WorkDelegate::DoWork() can be called more times than the value set by
WorkDelegate::SetDesiredWorkCount(). The main test thread may not be
able to “squeeze” its call to WorkerThread::Stop() in after its
WorkDelegate::WaitForWorkCount() returns. If the worker thread cannot be
stopped in time, one or more additional iterations of
WorkDelegate::DoWork() can run. WorkDelegate::DoWork() should take care
to not increment work_count_ beyond the desired value.

Bug: crashpad:169
Test: crashpad_util_test WorkerThread.*
Change-Id: I9e261a2a8a57420e12c0f1c9abd0ee6304dacd53
Reviewed-on: https://chromium-review.googlesource.com/456821
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-03-20 17:16:50 +00:00
Mark Mentovai
14138936b5 test: Compare ProcessInfo::Arguments() to main()’s argc/argv on POSIX
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>
2017-03-20 17:05:30 +00:00
Mark Mentovai
88bc09fb86 posix: Fix StdioFileHandle() for GCC
With GCC 6.3:

util/file/file_io_posix.cc: In function ‘crashpad::FileHandle crashpad::StdioFileHandle(crashpad::StdioStream)’:
util/file/file_io_posix.cc:193:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^

Bug: crashpad:30
Change-Id: I03111b672ab7f796103ef61ea3d126fc25571390
Reviewed-on: https://chromium-review.googlesource.com/456820
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-03-20 16:39:45 +00:00
Mark Mentovai
4f90f15146 Remove WeakStdioFileReader and WeakStdioFileWriter
These classes were a bit of a hack, and one of the the reasons that
WeakStdioFileReader was introduced, accurate detection of EOF when stdin
is a terminal, will be obsolete once
https://chromium-review.googlesource.com/456676/ lands. In fact,
WeakStdioFileReader didn’t even work properly for this purpose on
Windows.

Use WeakFile{Reader,Writer} in place of these classes (there were only
two use sites). Provide a StdioFileHandle() function to access the
proper values to use as a FileHandle for native file I/O given each OS’
own interface.

Change-Id: I35e8d49982162bb9813855f41739cc77597ea74d
Reviewed-on: https://chromium-review.googlesource.com/456358
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-03-16 20:21:19 +00:00
Mark Mentovai
00b6442752 Make file_io reads more rational and predictable
ReadFile() attempted to continue reading after a short read. In most
cases, this is fine. However, ReadFile() would keep trying to fill a
partially-filled buffer until experiencing a 0-length read(), signaling
end-of-file. For certain weird file descriptors like terminal input, EOF
is an ephemeral condition, and attempting to read beyond EOF doesn’t
actually return 0 (EOF) provided that they remain open, it will block
waiting for more input. Consequently, ReadFile() and anything based on
ReadFile() had an undocumented and quirky interface, which was that any
short read that it returned (not an underlying short read) actually
indicated EOF.

This facet of ReadFile() was unexpected, so it’s being removed. The new
behavior is that ReadFile() will return an underlying short read. The
behavior of FileReaderInterface::Read() is updated in accordance with
this change.

Upon experiencing a short read, the caller can determine the best
action. Most callers were already prepared for this behavior. Outside of
util/file, only crashpad_database_util properly implemented EOF
detection according to previous semantics, and adapting it to new
semantics is trivial.

Callers who require an exact-length read can use the new
ReadFileExactly(), or the newly renamed LoggingReadFileExactly() or
CheckedReadFileExactly(). These functions will retry following a short
read. The renamed functions were previously called LoggingReadFile() and
CheckedReadFile(), but those names implied that they were simply
wrapping ReadFile(), which is not the case. They wrapped ReadFile() and
further, insisted on a full read. Since ReadFile()’s semantics are now
changing but these functions’ are not, they’re now even more distinct
from ReadFile(), and must be renamed to avoid confusion.

Test: *
Change-Id: I06b77e0d6ad8719bd2eb67dab93a8740542dd908
Reviewed-on: https://chromium-review.googlesource.com/456676
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-03-16 20:07:43 +00:00
Mark Mentovai
51b21d8874 Add DelimitedFileReader and use it in Linux/Android’s ProcessInfo
This implements a non-stdio-based getline() equivalent. getline() is not
in the Android NDK until API 21 (Android 5.0.0), while Chrome builds for
32-bit platforms with API 16 (Android 4.1.0). Although a getline()
declaration could be provided in compat for use with older NDK headers,
it’s desirable to move away from stdio entirely. The C++
DelimitedFileReader interface is also a bit more comfortable to use than
getline().

A getdelim() equivalent is also provided, and is also used in the
Linux/Android ProcessInfo implementation.

Bug: crashpad:30
Test: crashpad_util_test FileLineReader.*:ProcessInfo.*
Change-Id: Ic1664758a87cfe4953ab22bd3ae190761404b22c
Reviewed-on: https://chromium-review.googlesource.com/455998
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-03-16 17:42:09 +00:00
Mark Mentovai
5938c6e993 linux: Support ProcessInfo::Is64Bit() for ARM on pre-3.5.0 Linux
The PTRACE_GETREGSET ptrace() request is not supported on ARM before
Linux 3.5.0. This request was only used to determine the bitness of the
target process. Since 64-bit ARM is only supported as of Linux 3.7.0,
when this request is not supported on 32-bit ARM, 64-bit is also not
supported, and the target process must be a 32-bit process.

Bug: crashpad:30
Test: crashpad_util_test ProcessInfo.*
Change-Id: Ib004d24858f146df898dfa6796926d97e2510541
Reviewed-on: https://chromium-review.googlesource.com/455398
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-03-15 17:36:52 +00:00
Mark Mentovai
d7467ba7e4 linux: Use user_regs instead of pt_regs for 32-bit ARM in ProcessInfo
Not all libc implementations reliably expose pt_regs from
<sys/ptrace.h>. glibc-2.25/sysdeps/generic/sys/ptrace.h, for example,
does not #include <asm/ptrace.h> (which defines the structure) or
anything else that would #include that file such as <linux/ptrace.h>. On
the other hand, Android 7.1.1 bionic/libc/include/sys/ptrace.h does
#include <linux/ptrace.h>.

It is not viable to #include <asm/ptrace.h> or <linux/ptrace.h>
directly: it would be natural to #include them, sorted, before
<sys/ptrace.h> but this causes problems for glibc’s <sys/ptrace.h>.
Constants like PTRACE_GETREGS and PTRACE_TRACEME are simple macros in
<asm/ptrace.h> and <linux/ptrace.h>, respectively, but are defined in
enums in glibc’s <sys/ptrace.h>, and this doesn’t mix well. It is
possible to #include <asm/ptrace.h> (but not <linux/ptrace.h>) after
<sys/ptrace.h>, but because this involves same-value macro redefinitions
and because it reaches into internal headers, it’s not preferred.

The alternative approach taken here is to use the user_regs structure
from <sys/user.h>, which is reliably defined by both Bionic and glibc,
and has the same layout as the kernel’s pt_regs structure. (All that
matters in this code is the size of the structure.) See Android 7.1.1
bionic/libc/include/sys/user.h,
glibc-2.25/sysdeps/unix/sysv/linux/arm/sys/user.h, and
linux-4.9.15/arch/arm/include/asm/ptrace.h for the various equivalent
definitions.

Take the same approach for 64-bit ARM: use user_regs_struct from
<sys/user.h> in preference to hoping for a C library’s <sys/ptrace.h> to
somehow provide the kernel’s user_pt_regs.

This mirrors the approach already being used for x86 and x86_64, which
use the C library’s <sys/user.h> user_regs_struct.

Bug: crashpad:30
Test: crashpad_util_test ProcessInfo.*
Change-Id: I3067e32c7fa4d6c8f4f2d5b63df141a0f490cd13
Reviewed-on: https://chromium-review.googlesource.com/455558
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-03-15 17:11:48 +00:00
Mark Mentovai
9be4745be0 linux: Lazily initialize ProcessInfo’s Is64Bit() and StartTime()
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>
2017-03-15 16:01:27 +00:00
Mark Mentovai
48781dc182 linux: Fix process start time computation
The process start time in ticks was being converted to an integer from a
temporary string that had gone out of scope by the time the conversion
was performed.

It was possible for a format error in /proc/pid/stat to go undetected
and result in a buffer overflow.

Bug: crashpad:30
Change-Id: I03566dda797bc1f23543bfffcfdb2c5ffe1eca66
Reviewed-on: https://chromium-review.googlesource.com/455378
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-03-15 02:08:12 +00:00
Mark Mentovai
bad4fd0011 linux: Fix ProcessInfo for x86[_64]
This configuration uses user_regs_struct, which is declared in
<sys/user.h>.

Bug: crashpad:30
Change-Id: Ibdcc60c6719fc2bad9fbeef116efbe764229e14b
Reviewed-on: https://chromium-review.googlesource.com/455197
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-03-14 22:20:33 +00:00
Joshua Peraza
87c75552ad Implement ProcessInfo for Linux/Android
Bug: crashpad:30
Change-Id: I45853a96cdbe94a2dbf3fa265b015170badb1bbb
Reviewed-on: https://chromium-review.googlesource.com/446903
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-03-14 20:10:17 +00:00
Mark Mentovai
6a5695967f Introduce the crashpad_http_upload tool
crashpad_http_upload sends HTTP POST multipart/form-data requests and
receives responses in exactly the same manner that crashpad_handler does
for crash report uploads, but separates it out for more general testing
and debugging.

Change-Id: I5c5919f9b1dc1e6be1e43b15a35b31f51add8a46
2017-03-03 15:11:50 -05:00
Mark Mentovai
777e36014f linux: Add HTTPTransportLibcurl for Linux (but not Android)
BUG=crashpad:30
TEST=crashpad_util_test HTTPTransport.*

Change-Id: Ifef812830fe2d778f400467d93771dc166cef390
2017-03-03 14:47:03 -05:00
Mark Mentovai
9660a1b66e win: Fix build after 60be5a66a050
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>
2017-03-01 19:32:52 +00:00
Mark Mentovai
60be5a66a0 net: Provide better HTTP User-Agent strings
Previously, macOS used “User-Agent: crashpad_util_test (unknown version)
CFNetwork/807.2.14 Darwin/16.4.0 (x86_64)” and Windows gave results like
“User-Agent: Crashpad/0.8.0”.

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

Change-Id: I578b44734cf59d79e3d9b6136b4b92f05acefe71
Reviewed-on: https://chromium-review.googlesource.com/447796
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-03-01 17:44:08 +00:00
Mark Mentovai
bf2c5155d2 Add Signals, utilities for signal handling
Use these utilities for signal handling in crashpad_handler

BUG=crashpad:30
TEST=crashpad_util_test Signals.*

Change-Id: I6c9a1de35c4a81b58d77768c4753bdba5ebea4df
Reviewed-on: https://chromium-review.googlesource.com/446917
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-03-01 17:25:54 +00:00
Mark Mentovai
58aac1bd87 Set FD_CLOEXEC on file descriptors obtained from open() and fopen()
Includes an update of mini_chromium to 3a2d52d74c9a:

3a2d52d74c9a Use O_CLOEXEC (and O_NOCTTY) when calling open()

BUG=chromium:688362

Change-Id: I2bdf86efe4e6559ecb77492ac5bdc728aa035889
Reviewed-on: https://chromium-review.googlesource.com/447999
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-02-28 21:16:41 +00:00
Mark Mentovai
1dcd45ab75 win: Pass a query component, if present, to an HTTP server
BUG=crashpad:163

Change-Id: I046dcef56a788ae04848383b0601e35d172c8183
Reviewed-on: https://chromium-review.googlesource.com/447916
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-02-28 19:01:34 +00:00
Mark Mentovai
6da9708e7c doc: Fix Doxygen errors
Change-Id: I5d5abf7b7eabe269a7c7b4d305a67fe910c887fd
Reviewed-on: https://chromium-review.googlesource.com/446480
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-02-23 01:43:40 +00:00
Mark Mentovai
c1af20f1aa metrics: Consistently comment about enums used for metrics
BUG=crashpad:100

Change-Id: I9ed0f260b4c92e7a706418f58c3db1ae027a04ab
Reviewed-on: https://chromium-review.googlesource.com/446557
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-02-23 00:13:04 +00:00
Mark Mentovai
f34ed66b93 metrics: Record handler lifetime milestone events
It could be useful to put our existing Crashpad.HandlerCrashed metrics
into context by getting a sense of handler starts, clean exits, and
other types of exits.

BUG=crashpad:100

Change-Id: I8982075158ea6d210eb2ddad678302e339a42192
Reviewed-on: https://chromium-review.googlesource.com/444124
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-02-22 18:48:12 +00:00
Mark Mentovai
0c322ecc3f Use zlib to gzip-compress uploads
This adds zlib to Crashpad. By default in standalone Crashpad builds,
the system zlib will be used where available. A copy of Chromium’s zlib
(currently a slightly patched 1.2.11) is checked out via DEPS into
third_party for use on Windows, which does not have a system zlib.

zlib is used to produce gzip streams for HTTP upload request bodies sent
by crashpad_handler by default. The Content-Encoding: gzip header is set
for these compressed request bodies. Compression can be disabled for
upload to servers without corresponding decompression support by
starting crashpad_handler with the --no-upload-gzip option.

Most minidumps compress quite well with zlib. A size reduction of 90% is
not uncommon.

BUG=crashpad:157
TEST=crashpad_util_test GzipHTTPBodyStream.*:HTTPTransport.*

Change-Id: I99b86db3952c3685cd78f5dc858a60b54399c513
Reviewed-on: https://chromium-review.googlesource.com/438585
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-02-16 16:26:19 +00:00
Erik Chen
c1b305244a Update mig.py to take an explicit sdk argument.
BUG=chromium:690734

> Review-Url: https://codereview.chromium.org/2685233002
> Cr-Commit-Position: refs/heads/master@{#449550}
> Message-Id: Merged from chromium 53f2146935506b4f382705b605dffec41b5519eb

Change-Id: I1b3176a4a62078f1e27184ad589c9c3f4b548674
Reviewed-on: https://chromium-review.googlesource.com/440847
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-02-10 18:14:10 +00:00