519 Commits

Author SHA1 Message Date
Mark Mentovai
c542a5ae03 net: Remove extra spaces from WinHttp log messages
I recently (90054edf6202) removed the extra spaces from ntdll log
messages. The winhttp messages have the same problem.

Context:
https://groups.google.com/a/chromium.org/d/topic/crashpad-dev/kD3a9Jekzx0

Change-Id: I49682ba99c3dfe1cd4c49507806cf8adf154bc9f
Reviewed-on: https://chromium-review.googlesource.com/749305
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-11-01 16:48:29 +00:00
Mark Mentovai
1669ca2bac test: Rework TestPaths interface for obtaining 32-bit build artifacts
The design for running all Crashpad unit tests on Chromium’s try- and
buildbots involves pulling all tests into a single monolithic
crashpad_tests executable. Many Crashpad tests base the name of their
child executables or modules on the name of the main test executable.
Since the main test executable will have a different name in the
in-Chromium build, knowledge of the test executable name (referred to as
“module” here) needs to be added to the tests themselves.

This introduces TestPaths::BuildArtifact(), which allows the module name
to be specified. For Crashpad’s standalone build, the module name is
verified against the main test executable’s name.
TestPaths::BuildArtifact() can also locate paths in the alternate 32-bit
output directory for 64-bit Windows tests, taking on the responsibility
for what the new (5e9ed4cb9f69) TestPaths::Output32BitDirectory(), now
obsolete, did.

Bug: chromium:779790
Change-Id: I64c4a2190b6319e487c999812a7cfc512a75a700
Reviewed-on: https://chromium-review.googlesource.com/747536
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-11-01 16:44:45 +00:00
Mark Mentovai
af594c8deb Heap-allocate MinidumpContextAMD64Writer objects with proper alignment
While making crashpad_minidump_test run in Chromium’s try- and buildbots
(https://crbug.com/779790), crashes in the
MinidumpThreadWriter.OneThread_AMD64_Stack test were observed in 32-bit
x86 Windows builds produced by Clang in the release configuration. These
crashes occurred in crashpad::test::InitializeMinidumpContextAMD64,
which heap-allocates a MinidumpContextAMD64Writer object. These objects
have an alignment requirement of 16, based on the alignment requirement
of their MinidumpContextAMD64 member.

Although this problem was never observed with MSVC, Clang was making use
of the known strict alignment and producing code that depended on it.
This code crashed if the requirement was not met. MSVC had raised a
warning about this usage (C4316), but the warning was disabled as it did
not appear to have any ill effect on code produced by that compiler.

The problem surfaced in test code, but heap-allocated
MinidumpContextAMD64Writer objects are created in non-test code as well.
The impact is limited, because a 32-bit Windows Crashpad handler would
not have a need to allocate one of these objects.

As a fix, MinidumpContextAMD64Writer is given a custom allocation
function (a static “operator new()” member and matching “operator
delete()”) that returns properly aligned memory.

Change-Id: I0cb924da91716eb01b88ec2ae952a69262cc2de6
Reviewed-on: https://chromium-review.googlesource.com/746539
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-11-01 16:40:58 +00:00
Joshua Peraza
7e82179d43 Move filesystem test utilities to test/
These utilities will be useful for database tests.

Bug: crashpad:206
Change-Id: Iae0d831934ea7f020f167dbbcba901a72472937b
Reviewed-on: https://chromium-review.googlesource.com/747885
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-11-01 01:08:11 +00:00
Mark Mentovai
3fae8ff07c win: Fix -Wsign-compare warnings produced by clang
../../third_party/crashpad/crashpad/util/file/filesystem_test_util.cc(79,27):  error: comparison of integers of different signs: 'DWORD' (aka 'unsigned long') and 'int' [-Werror,-Wsign-compare]
  if (symbolic_link_flags == -1) {
      ~~~~~~~~~~~~~~~~~~~ ^  ~~
In file included from ../../third_party/crashpad/crashpad/util/file/filesystem_test_util.cc:23:
../../third_party/googletest/src/googletest/include\gtest/gtest.h(1392,11):  error: comparison of integers of different signs: 'const unsigned long' and 'const long' [-Werror,-Wsign-compare]
  if (lhs == rhs) {
      ~~~ ^  ~~~
../../third_party/googletest/src/googletest/include\gtest/gtest.h(1421,12):  note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<unsigned long, long>' requested here
    return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
           ^
../../third_party/crashpad/crashpad/util/file/filesystem_test_util.cc(73,5):  note: in instantiation of function template specialization 'testing::internal::EqHelper<false>::Compare<unsigned long, long>' requested here
    EXPECT_EQ(error, ERROR_PRIVILEGE_NOT_HELD)
    ^
../../third_party/googletest/src/googletest/include\gtest/gtest.h(1924,63):  note: expanded from macro 'EXPECT_EQ'
                      EqHelper<GTEST_IS_NULL_LITERAL_(val1)>::Compare, \
                                                              ^
2 errors generated.

and

../../third_party/crashpad/crashpad/util/file/filesystem_test_util.cc(111,5):  note: in instantiation of function template specialization 'testing::internal::EqHelper<false>::Compare<unsigned long, long>' requested here
    EXPECT_EQ(GetLastError(), ERROR_FILE_NOT_FOUND)
    ^

Change-Id: I55b33b39c271d765376ff9c416e737d0608eb781
Reviewed-on: https://chromium-review.googlesource.com/742561
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-10-28 12:19:57 +00:00
Mark Mentovai
692488a254 Un-disable WinMultiprocess-based tests in Chromium
As of
00a0654929,
crashpad_util_test is able to run in Chromium. It uses Chromium’s own
base::TestLauncher rather than gtest’s RUN_ALL_TESTS() for proper
integration with Swarming.

Launching WinMultiprocess test children out of the same test executable
via WinChildProcess is not compatible with Chromium’s parallel, shardy,
Swarmy test launcher. When running these children, the standard gtest
RUN_ALL_TESTS() launcher will now be used, even in Chromium.

Two tests disabled in Chromium are now enabled:
ExceptionHandlerServerTest.MultipleConnections and
ScopedProcessSuspend.ScopedProcessSuspend.

As part of this work, I discovered that disabled tests chosen to run via
--gtest_also_run_disabled_tests did not actually work for
WinMultiprocess-based tests, because gtest’s test launcher would refuse
to run the child side of the test, believing it was disabled. This is
fixed by always supplying --gtest_also_run_disabled_tests to
WinChildProcess children, on the basis that if the parent is managing to
run and it’s disabled, disabled tests must actually be enabled.

Bug: crashpad:205
Change-Id: Ied22f16b9329ee13b6b07fd29de704f6fe2a058e
Reviewed-on: https://chromium-review.googlesource.com/742462
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-10-27 21:42:10 +00:00
Mark Mentovai
672c872589 Upstream crashpad_util_test-in-Chromium changes
This upstreams part of
00a0654929.

The gmock_main and gtest_main test launchers detect via a
CRASHPAD_IN_CHROMIUM macro that they are building as part of Chromium,
and use Chromium’s custom test launcher rather than gtest’s
RUN_ALL_TESTS(). This enables parallelism, sharding, and integration
with Swarming.

WinMultiprocess-based tests are not compatible with this test launcher
or with the Swarming test design, and must be disabled when
CRASHPAD_IN_CHROMIUM is set. This is covered by
https://crashpad.chromium.org/bug/205.

CRASHPAD_IN_CHROMIUM is never defined during Crashpad’s own standalone
build, it’s only defined when building in Chromium.

Change-Id: I969c5d376f86ab4b3f4cc85c97d4452b53b35063
Reviewed-on: https://chromium-review.googlesource.com/740988
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-10-27 20:20:35 +00:00
Mark Mentovai
ef262d1ee3 #include "build/build_config.h" where needed
Change-Id: I45c1afe73e8570dfcedde6da01375a4533bb355a
Reviewed-on: https://chromium-review.googlesource.com/741891
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-10-27 18:22:29 +00:00
Mark Mentovai
c49da9caef win: Expect uneven symbolic link support
As mentioned at
https://chromium-review.googlesource.com/c/chromium/src/+/735820#message-e8b199498d8b850f2612c46648069d819dd47517,
the typical Windows behavior for symbolic links requires administrative
privileges.

Symbolic links are available to non-administrators in Windows 10.0.15063
and later (1703, Creators Update), provided that developer mode has been
enabled and SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE is passed to
CreateSymbolicLink(). See
https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/.

This adds SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE to uses of
CreateSymbolicLink(), and creates test::CanCreateSymbolicLinks() to
determine whether symbolic link creation is possible. Tests that
exercise symbolic links are adapted to gate all symbolic link operations
on this test.

Test: crashpad_util_test DirectoryReader.*:Filesystem.*
Change-Id: I8250cadd974ffcc7abe32701a0d5bc487061baf0
Bug: crashpad:
Reviewed-on: https://chromium-review.googlesource.com/739472
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-10-26 23:01:14 +00:00
Mark Mentovai
5e9ed4cb9f win: Dynamically disable WoW64 tests absent explicit 32-bit build output
Rather than having the 64-bit build assume that it lives in
out\{Debug,Release}_x64 and that it can find 32-bit build output in
out\{Debug,Release}, require the location of 32-bit build output to be
provided explicitly via the CRASHPAD_TEST_32_BIT_OUTPUT environment
variable. If this variable is not set, 64-bit tests that require 32-bit
test build output will dynamically disable themselves at runtime.

In order for this to work, a new DISABLED_TEST() macro is added to
support dynamically disabled tests. gtest does not have its own
first-class support for this
(https://groups.google.com/d/topic/googletestframework/Nwh3u7YFuN4,
https://github.com/google/googletest/issues/490) so this local solution
is used instead.

For tests via Crashpad’s own build\run_tests.py, which is how Crashpad’s
own buildbots and trybots invoke tests, CRASHPAD_TEST_32_BIT_OUTPUT is
set to a locaton compatible with the paths expected for the GYP-based
build. No test coverage is lost on Crashpad’s own buildbots and trybots.

For Crashpad tests in Chromium’s buildbots and trybots, this environment
variable will not be set, causing these tests to be dynamically
disabled.

Bug: crashpad:203, chromium:743139, chromium:777924
Change-Id: I3c0de2bf4f835e13ed5a4adda5760d6fed508126
Reviewed-on: https://chromium-review.googlesource.com/739795
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-10-26 18:31:57 +00:00
Nico Weber
9bc5989125 crashpad_util_test warning fixes for clang-cl, 64-bit edition.
This upstreams https://chromium-review.googlesource.com/c/chromium/src/+/738402

Bug: chromium:777924
Change-Id: Ib3c8f4f77631da45a2911029e8925c1afad1c244
Reviewed-on: https://chromium-review.googlesource.com/738553
Commit-Queue: Nico Weber <thakis@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-10-25 20:53:51 +00:00
Mark Mentovai
52d766400d linux: ProcessReader can own ProcessMemoryLinux without unique_ptr
There’s no reason for ProcessReader to own its ProcessMemoryLinux via
std::unique_ptr<>.

This was discovered in a trunk Clang build, during which a
-Wdelete-non-virtual-dtor warning was produced (since Clang r312167).
The warning is not produced by earlier Clang versions or by GCC because
the “delete” happens in a system header, <memory>, when performed by
std::unique_ptr<>. Although ownership via std::unique_ptr<> is no longer
used, ProcessMemoryLinux is marked “final” because it ought to be.

In file included from ../../snapshot/linux/process_reader.cc:15:
In file included from ../../snapshot/linux/process_reader.h:21:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/7.2.0/../../include/c++/7.2.0/memory:80:
/usr/bin/../lib/gcc/x86_64-linux-gnu/7.2.0/../../include/c++/7.2.0/bits/unique_ptr.h:78:2: error: delete called on non-final 'crashpad::ProcessMemoryLinux' that has virtual functions but non-virtual destructor [-Werror,-Wdelete-non-virtual-dtor]
        delete __ptr;
        ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/7.2.0/../../include/c++/7.2.0/bits/unique_ptr.h:268:4: note: in instantiation of member function 'std::default_delete<crashpad::ProcessMemoryLinux>::operator()' requested here
          get_deleter()(__ptr);
          ^
../../snapshot/linux/process_reader.cc:169:16: note: in instantiation of member function 'std::unique_ptr<crashpad::ProcessMemoryLinux, std::default_delete<crashpad::ProcessMemoryLinux> >::~unique_ptr' requested here
ProcessReader::ProcessReader()
               ^
1 error generated.
Change-Id: Ibe9671db429262aca12bbfdf457c8f72cad2f358
Reviewed-on: https://chromium-review.googlesource.com/738530
Reviewed-by: Dave Bort <dbort@google.com>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-10-25 20:07:29 +00:00
Nico Weber
2f48159011 Make crashpad_util_test build without warnings with clang-cl on Windows.
This upstreams https://chromium-review.googlesource.com/c/chromium/src/+/735820/

Bug: chromium:777924
Change-Id: I9fe76b839442d73a6c2836ccfe6cbe41acd67fad
Reviewed-on: https://chromium-review.googlesource.com/738394
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
2017-10-25 19:02:47 +00:00
Mark Mentovai
fbc365fa9e GCC 7.2 support (-Wnoexcept-type)
P0012R1, accepted into C++17, makes a function’s “noexcept” (or
“throw()”) specification part of its signature. GCC 7.2 provides a
warning, -Wnoexcept-type, that is triggered when a function pointer type
with an exception specification is used in pre-C++17 code in such a way
as to pose an ABI incompatibility with C++17 code.

https://gcc.gnu.org/onlinedocs/gcc-7.2.0/gcc/C_002b_002b-Dialect-Options.html#index-Wnoexcept-type

Warnings are of the form:

In file included from ../../util/misc/from_pointer_cast_test.cc:15:0:
../../util/misc/from_pointer_cast.h:64:1: error: mangled name for ‘typename std::enable_if<(std::is_pointer<From>::value && std::is_pointer<_Tp>::value), To>::type crashpad::FromPointerCast(From) [with To = const volatile void*; From = void* (*)(long unsigned int) throw ()]’ will change in C++17 because the exception specification is part of a function type [-Werror=noexcept-type]
 FromPointerCast(From from) {
 ^~~~~~~~~~~~~~~
../../util/misc/from_pointer_cast.h:64:1: error: mangled name for ‘typename std::enable_if<(std::is_pointer<From>::value && std::is_pointer<_Tp>::value), To>::type crashpad::FromPointerCast(From) [with To = volatile void*; From = void* (*)(long unsigned int) throw ()]’ will change in C++17 because the exception specification is part of a function type [-Werror=noexcept-type]

In Crashpad, this warning is triggered by the two FromPointerCast<>()
variants that accept function pointer “From” arguments. This occurs when
using glibc as the standard C library, since glibc declares its
functions as “throw()”. FromPointerCast<>() is used with pointers to
glibc functions such as malloc() and getpid().

The warning is disabled for the FromPointerCast<>() variants that would
trigger it. The warning is not useful or actionable in this internal
Crashpad code where ABI changes due to language version (including
mangling changes) are not a concern.

Clang 4.0 has the similar -Wc++1z-compat-mangling option (also available
as -Wc++17-compat-mangling and the GCC-compatible -Wnoexcept-type in
Clang 5.0) but it is not triggered by this pattern.

Change-Id: Id293db3954be415f67a55476ca72bfb7d399aa3b
Reviewed-on: https://chromium-review.googlesource.com/738292
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-10-25 18:29:58 +00:00
Mark Mentovai
6d5bd1d04d win: Go back to using ml.exe for SafeTerminateProcess()
This reverts 55133d332b6c and adds a broken dummy SafeTerminateProcess()
for cross builds instead. It’s similar to 2f4516f93838, which was for
CaptureContext().

This upstreams
af5f31ed61
(slightly modified).

The dummy implementation in the “broken” file affords no protection
against third-party code patching TerminateProcess() badly. The “broken”
file is not used by Crashpad anywhere at all, and is only used by
Crashpad in Chromium during a cross build targeting Windows without the
benefit of Microsoft’s ml.exe assembler. Strictly speaking, this file
does not need to be checked in to the Crashpad repository, but since
Chromium needs it to unblock its not-production-ready cross build for
Windows, it’s being landed here to avoid Chromium’s copy of Crashpad
appearing as modified or “dirty” relative to this upstream copy.

Bug: chromium:762167, chromium:777924
Change-Id: Iba68c0cab142fbe9541ea254a9a856b8263e4c70
Reviewed-on: https://chromium-review.googlesource.com/735078
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-10-24 19:07:38 +00:00
Mark Mentovai
025455e77a Remove one more vestige of pre-C++11 library support
a327c86a52c3 missed this one.

Change-Id: Icbfc897b2f379641080dddc273b6279fc742f452
Reviewed-on: https://chromium-review.googlesource.com/727719
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-10-24 18:29:19 +00:00
Mark Mentovai
c2cc76dc26 util/linux: #include what you use
An #include was missing from 59c5d848e5c5.

Change-Id: Ib0074aefbc8dc231a097c2edd3ef3047f5cff32e
Reviewed-on: https://chromium-review.googlesource.com/734232
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-10-23 21:32:04 +00:00
Mark Mentovai
aff8d906b6 linux: Fix interpretation of device numbers in /proc/pid/maps
While the kernel formats device major and minor numbers as %02x:%02x,
they are not restricted to 8 bits apiece. Crashpad was requiring that
the hexadecimal representations be exactly two characters, rather than
at least two characters.

The proper way to reconstruct a dev_t from major and minor numbers in
user space is to use makedev() from <sys/sysmacros.h>. MKDEV() from
<linux/kdev_t.h> interfaces with an older (pre-Linux 2.6) format which
actually did use 8-bit major and minor numbers. makedev() places the
major number at bits 8-19, and splits the minor number into two groups
at bits 0-7 and 20-31. This is the correct user space view of device
numbers. (Note that this is distinct from the kernel’s view: the kernel
uses MKDEV() from a distinct internal <linux/kdev_t.h> which places the
minor number at bits 0-19 and the major number at bits 20-31.)

Bionic for 32-bit platforms uses a 32-bit user space dev_t while a
64-bit version is used elsewhere, and a comment in Bionic’s
<sys/types.h> calls this a “historical accident”. However, due to the
kernel’s use of only 32 bits for device numbers, this accident does not
have any ill effect.

Bug: crashpad:30
Test: crashpad_util_test, crashpad_snapshot_test
Change-Id: Ic343454393d7399f598f9eba169a9e5f5630e601
Reviewed-on: https://chromium-review.googlesource.com/733863
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-10-23 20:11:37 +00:00
Mark Mentovai
2f4516f938 win: Provide broken CHECK()ing dummy CaptureContext() for cross builds
This upstreams
fc1ac734b0
(slightly modified).

This dummy implementation is not used by Crashpad anywhere at all, and
is only used by Crashpad in Chromium during a cross build targeting
Windows without the benefit of Microsoft’s ml.exe/ml64.exe assembler.
Strictly speaking, this file does not need to be checked in to the
Crashpad repository, but since Chromium needs it to unblock its
not-production-ready cross build for Windows, it’s being landed here to
avoid Chromium’s copy of Crashpad appearing as modified or “dirty”
relative to this upstream copy. (Even though this file is really dirty.)

Bug: chromium:762167
Change-Id: Ibfdc316c1f5fe81d4b3a1d86f4032adccac467e5
Reviewed-on: https://chromium-review.googlesource.com/734102
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-10-23 19:56:35 +00:00
Mark Mentovai
55133d332b win: Use inline asm instead of ml.exe for SafeTerminateProcess()
This upstreams
912c9907d5
(slightly modified).

Bug: chromium:762167
Change-Id: I69c605f693da8691d32222b5617f62637c1c2dcd
Reviewed-on: https://chromium-review.googlesource.com/734100
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-10-23 19:14:05 +00:00
Joshua Peraza
ce084d37c8 Add MoveFileOrDirectory to move files, directories, or symbolic links
Change-Id: I6eaeef0dc3ec4300b361c1a96d14209aec736ff0
Reviewed-on: https://chromium-review.googlesource.com/727567
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-10-20 17:41:09 +00:00
Mark Mentovai
419f25eac8 Remove PointerVector<> and replace with std::vector<std::unique_ptr<>>
As mentioned at
https://chromium-review.googlesource.com/c/crashpad/crashpad/+/721978/13/tools/crashpad_http_upload.cc#90
Change-Id: I4820346cc0b0bf26633e1de598c884af8af19983
Reviewed-on: https://chromium-review.googlesource.com/724744
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-10-19 04:53:36 +00:00
Joshua Peraza
68a0e736c6 Use a FileReaderInterface for file attachments instead of a FilePath
This is a step towards a database which gives out FileReaders in Report
objects instead of FilePaths.

Change-Id: I59704da65fc5521e5d47019416bf962c215d13bc
Reviewed-on: https://chromium-review.googlesource.com/721978
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-10-17 16:32:08 +00:00
Joshua Peraza
4d7a07f684 Add ScopedRemoveFile to call LoggingRemoveFile for a FilePath
Change-Id: Iea3c6d54f35fb67811732af9e17c03b24b189d7b
Reviewed-on: https://chromium-review.googlesource.com/721076
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-10-17 00:47:07 +00:00
Joshua Peraza
474c7331a6 Add DirectoryReader to iterate over files in a directory
This change also adds functions to create directories, remove files and
directories, and check for the existence of files and directories.

Change-Id: I62b78219ae2b277d6976d2d90ec86fcabd0ef073
Reviewed-on: https://chromium-review.googlesource.com/696132
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-10-16 19:56:54 +00:00
Dave Bort
906fce1d01 Make ProcessMemory an abstract interface
Only a Linux implementation for now, but similar code for other
OSes can move behind it in the future.

Bug: crashpad:196
Change-Id: I05966db1599a9cac3146d2a3d964e7ad8629d616
Reviewed-on: https://chromium-review.googlesource.com/685408
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Dave Bort <dbort@google.com>
2017-10-13 21:45:14 +00:00
Nikhil Marathe
dabe8477da win: Fix TEB.TlsSlots offset
The Crashpad representation of the TEB struct had an incorrect PVOID
reserved of len 397. This should be 402 once we calculate that the other
members occupy 40/80 (32 vs 64) bytes.

Wine has a well documented copy
4df0162caf/include/winternl.h (L309)
that shows the offsets TlsSlots should be at. This patch makes that
change. TlsSlots is now at offset 3600 on 32-bit and offset 5248 on
64-bit.

Change-Id: I4ea4c44b1e49d3ea02d433f386f164703a373dab
Reviewed-on: https://chromium-review.googlesource.com/717040
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-10-13 19:03:07 +00:00
Joshua Peraza
c958c16491 Declare overriding method with override
Change-Id: I49407ecac4ae5956fdf6a7f845b0e7f3649dc75c
Reviewed-on: https://chromium-review.googlesource.com/717546
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
2017-10-13 16:34:46 +00:00
Mark Mentovai
a327c86a52 C++14 is required, don’t pretend to support pre-C++11 or pre-MSVS 2015
Change-Id: Ide835421599480acc63e8e88ce2217433c0d376e
Reviewed-on: https://chromium-review.googlesource.com/719036
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-10-13 15:49:59 +00:00
Mark Mentovai
4c4e67952c win: 10.0.16299.0 SDK compatibility
This corresponds to Windows 10 version 1709 (Fall Creators Update,
“Redstone 3”).

While compiling util/win/nt_internals.cc:

…\crashpad\crashpad\util\win\nt_internals.cc(22): error C2371: 'CLIENT_ID': redefinition; different basic types
c:\program files (x86)\windows kits\10\include\10.0.16299.0\um\winternl.h(83): note: see declaration of 'CLIENT_ID'

The CLIENT_ID structure, which should have been part of the SDK to begin
with, has been added. Provide a compatible definition in <winternl.h>.

Bug: chromium:773476
Change-Id: Iafc77f8cffd06d1194fc909bad587f1ffd1687a2
Reviewed-on: https://chromium-review.googlesource.com/711415
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-10-11 22:39:00 +00:00
Dave Bort
a99c84b8b4 Use generic VM types in util/process
A step towards making these files usable by non-Linux systems.

Bug: crashpad:196
Change-Id: Iaa8bfae1c325735c320e502698a61e4851777649
Reviewed-on: https://chromium-review.googlesource.com/685407
Commit-Queue: Dave Bort <dbort@google.com>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-10-10 19:02:39 +00:00
Dave Bort
fe4b16fe88 Move linux/process files to util/process
A step towards making these files usable by non-Linux systems.

Bug: crashpad:196
Change-Id: I71323b29e46208b3992055722e4622d79409c44c
Reviewed-on: https://chromium-review.googlesource.com/685406
Commit-Queue: Dave Bort <dbort@google.com>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-10-10 18:25:07 +00:00
Mark Mentovai
1abaf22e28 Use readdir() instead of readdir_r() on all (POSIX) platforms
readdir_r() is a thread-safe version of readdir(), although readdir() is
not particularly thread-unsafe with most usage. The dirent* returned by
readdir() can only be invalidated by a subsequent readdir() or
closedir() on the same DIR*. In typical usage, where a returned dirent*
is used exclusively within a loop around readdir() and is not expected
to outlive that loop, there are no lifetime or thread-safety issues with
the use of readdir().

readdir_r() may be harmful in certain situations because its buffer is
not explicitly sized, and attempts to provide a suitably sized buffer
dynamically (which, incidentally, our code did not do) are subject to a
race condition.

https://elliotth.blogspot.com/2012/10/how-not-to-use-readdirr3.html
https://womble.decadent.org.uk/readdir_r-advisory.html

glibc has already deprecated readdir_r(), and all Linux (including
Android) code was already using readdir(). This change eliminates
variant codepaths. It delegates buffer sizing (which we weren’t doing
correctly) to the C library, which also has more options at its disposal
to avoid races in sizing that buffer.

Change-Id: I4fca8948454116360180ad0017f226d06727ef81
Reviewed-on: https://chromium-review.googlesource.com/705756
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-10-06 21:08:50 +00:00
Mark Mentovai
31df2acb12 win: Fix messages in ProcessInfo::LoggingRangeIsFullyReadable()
|ranges| is a coalesced list of committed and accessible memory ranges
trimmed to reflect only those that overlap |range|. |range| is only
fully unreadable if |ranges| is empty. If |ranges| contains more than
one element, it indicates that |range| is sparse (since |ranges| is
coalesced, there must be a “hole”). This should be treated as partially
unreadable, the same as when |ranges[0]| doesn’t begin or end where
|range| does.

Test: self_destroying_test_program.exe (via end_to_end_test.py)
Change-Id: I55fc2b201089113f2b07395e352704b99d212801
Reviewed-on: https://chromium-review.googlesource.com/702535
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-10-05 20:55:32 +00:00
Mark Mentovai
e5896de993 win: Fix process_structs.h definition of RTL_USER_PROCESS_PARAMETERS
In the 64-bit version of the structure, padding is needed between
ShowWindowFlags and WindowTitle.

The CurrentDirectores (yes, that’s how it’s spelled) members would have
been interpreted incorrectly because STRING was defined incorrectly. The
length fields are USHORT, not DWORD. In the 64-bit version of the
structure, a padding member ensured that the structure was at least the
correct size. In the 32-bit version of the structure, this caused the
structure size to be inflated, so all but the first CurrentDirectores
element and any struct member that followed would appear at incorrect
offsets, and the overall struct size being read was larger than
appropriate.

This resolves crashpad_handler logging (usually) three errors while
handling a 64-bit process crash, such as:

[pid:tid:yyyymmdd,hhmmss.mmm:ERROR process_info.cc:632] range at
0x780f24de00000000, size 0x275 fully unreadable
[pid:tid:yyyymmdd,hhmmss.mmm:ERROR process_info.cc:632] range at
0x780f24fe00000000, size 0x275 fully unreadable
[pid:tid:yyyymmdd,hhmmss.mmm:ERROR process_info.cc:632] range at 0x0,
size 0x275 fully unreadable

Bug: crashpad:198
Test: end_to_end_test.py
Change-Id: I1655101de01cf46b4b50eda45a11f8d0f3bca8b3
Reviewed-on: https://chromium-review.googlesource.com/701736
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-10-05 20:49:08 +00:00
Mark Mentovai
90054edf62 win: De-flake hanging_program.exe
hanging_program.exe is used by crash_other_program.exe, which is in turn
used by end_to_end_test.py. It hangs by loading loader_lock_dll.dll,
which squats in its entry point function while the loader lock is held.

hanging_program.exe needs to do some work in its Thread1() before the
loader lock is taken (a SetThreadPriority() call), and needs to do some
work in its main thread once the loader lock is held (it needs to signal
crash_other_program.exe that it’s successfully wedged itself).
Previously, proper synchronization was not provided. A 1-second Sleep()
was used to wait for the loader lock to be taken. Thread1() pre-work was
only achieved before the loader lock was taken by sheer luck. Things
didn’t always work out so nicely.

This uses an event handle to provide synchronization. An environment
variable is used to pass the handle to loader_lock_dll.dll, because
there aren’t many better options available. This eliminates both flake
and the unnecessary 1-second delay in hanging_program.exe, and since
this program runs twice during end_to_end_test.py, it improves that
test’s runtime by 2 seconds.

Bug: crashpad:197
Test: end_to_end_test.py
Change-Id: Ib9883215ef96bed7571464cc68e09b6ab6310ae6
Reviewed-on: https://chromium-review.googlesource.com/700076
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-10-04 19:58:56 +00:00
Mark Mentovai
c6adcc2482 win: Make CrashpadClient::DumpAndCrashTargetProcess() less chatty
CrashpadClient::DumpAndCrashTargetProcess() suspends the target process
and injects a thread to raise an exception. The injected thread is not
suspended, and may proceed to the point that the system recognizes the
process as terminating by the time the overall process suspension is
lifted. Previously, if this happened, an extraneous error was logged for
the attempt to resume a terminating process.

This introduces “termination tolerance” to ScopedProcessSuspend, which
allows an object to be configured to ignore this error and not log any
messages when this condition is expected.

This resolves log messages such as this one, produced frequently during
calls to CrashpadClient::DumpAndCrashTargetProcess() (including in
end_to_end_test.py):

> [pid:tid:yyyymmdd,hhmmss.mmm:ERROR scoped_process_suspend.cc:39]
> NtResumeProcess: An attempt was made to access an exiting process.
> (0xc000010a)

0xc000010a = STATUS_PROCESS_IS_TERMINATING

Test: end_to_end_test.py
Change-Id: Iab4c50fb21adce5502080ad25a6f734ec566d65c
Reviewed-on: https://chromium-review.googlesource.com/700715
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-10-04 19:04:47 +00:00
Mark Mentovai
f6aebd8baf android: Fix build after 45de8bf76e32
OS_LINUX is not defined on Android. Chromium made this call and we can’t
revisit it here and now.

Change-Id: I70fd6ac35ba9731e2fd06792bf8cae332e2b360c
Reviewed-on: https://chromium-review.googlesource.com/700655
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-10-04 18:42:35 +00:00
Roman Margold
f3a8dbd671 net: Identify clients via URL parameters during report upload
During crash report upload, the client now provides the product
name, version, and client id via URL parameters to the crash
reporting service.
Also added percent-encoding function and a test.

Change-Id: I62f3a646d4ab6029543bd80938b79de28b1f20e4
Test: crashpad_util_test URLEncode.Empty
Test: crashpad_util_test URLEncode.ReservedCharacters
Test: crashpad_util_test URLEncode.UnreservedCharacters
Test: crashpad_util_test URLEncode.SimpleAddress
Reviewed-on: https://chromium-review.googlesource.com/493917
Commit-Queue: Roman Margold <rmargold@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-09-28 17:15:40 +00:00
Dave Bort
45de8bf76e Add generic VMAddress-related types
This will allow sharing code that is currently hard-coded to use (e.g.)
LinuxVMAddress or mach_vm_size_t.

Change-Id: I7bf20600c73d4ec7d2a029754f9043a236a38e5a
Reviewed-on: https://chromium-review.googlesource.com/677142
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Dave Bort <dbort@google.com>
2017-09-26 17:45:38 +00:00
Joshua Peraza
59c5d848e5 linux: Refactor ptrace usage.
1) Add PtraceConnection which serves as the base class for specific
types of connections Crashpad uses to trace processes.
2) Add DirectPtraceConnection which is used when the handler process
has `ptrace` capabilities for the target process.
3) Move `ptrace` logic into Ptracer. This class isolates `ptrace` call
logic for use by various PtraceConnection implementations.

Bug: crashpad:30
Change-Id: I98083134a9f7d9f085e4cc816d2b85ffd6d73162
Reviewed-on: https://chromium-review.googlesource.com/671659
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-09-22 16:25:32 +00:00
Robert Sesek
f16e4eb9ff Implement SleepNanoseconds() on Windows.
This uses the naïve implementation originally written
https://codereview.chromium.org/807973002/#ps180001.

Bug: crashpad:192
Change-Id: Id00908dafb8886d6163a8b17213d3b7c33b81963
Reviewed-on: https://chromium-review.googlesource.com/606998
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-08-08 22:11:42 +00:00
Xi Cheng
01110c0a3b win: Fix %u, %d, %x/DWORD printf mismatches
To enable clang-cl's printf format string mismatch checking, a few
mismatch errors need to be fixed where DWORD (unsigned long) is printed
with %u, %d or %x (an 'l' is needed).

Change-Id: I2cbfafe823a186bfe3a555aec3a7ca03e85466f8
Reviewed-on: https://chromium-review.googlesource.com/598651
Commit-Queue: Xi Cheng <chengx@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-08-02 22:04:13 +00:00
Joshua Peraza
edf4dde8ae linux: Add ExceptionSnapshotLinux
Bug: crashpad:30
Change-Id: I450d53a89af2995c0fd13b31821360e781fe015a
Reviewed-on: https://chromium-review.googlesource.com/589747
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-08-02 02:29:51 +00:00
Mark Mentovai
c332e7ffda Fix Doxygen usage in util/misc/lexing.h
Change-Id: Ifdef347426655df2ab54aed0eec0cfbe4bbd7cb1
Reviewed-on: https://chromium-review.googlesource.com/592696
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-07-29 21:11:33 +00:00
Mark Mentovai
8f0636288a Use constexpr at namespace scope
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>
2017-07-29 01:06:52 +00:00
Mark Mentovai
6dac7ecdf5 Use constexpr at function scope
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>
2017-07-29 00:50:40 +00:00
Mark Mentovai
281be63d00 Standardize on static constexpr for arrays when possible
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>
2017-07-25 17:40:51 +00:00
Joshua Peraza
01b347732e linux: Collect CPU times in ProcStatReader and use in ProcessReader
Bug: crashpad:30
Change-Id: I6d4020220031670937acad12d0b7878c1ae0fae7
Reviewed-on: https://chromium-review.googlesource.com/583952
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-07-25 04:15:32 +00:00
Mark Mentovai
90e4649f0d linux: Sort alphabetically in util .gyp files
Bug: crashpad:30
Change-Id: Iea992cd9eef1029c046cb354f7c1c0173b6f0675
Reviewed-on: https://chromium-review.googlesource.com/583767
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-07-24 18:51:31 +00:00