1066 Commits

Author SHA1 Message Date
Joshua Peraza
6f6f8a144d Add FileModificationTime
FileModificationTime gets the last write time for files, directories,
or symbolic links. Symbolic links may point to files, directories, or
be dangling.

Bug: crashpad:206
Change-Id: Ic83b5a7d318502ad5db5c01731d06c8624925e15
Reviewed-on: https://chromium-review.googlesource.com/744298
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-11-09 06:27:46 +00:00
Mark Mentovai
2d077a2c57 win: Use “long” format modifier for DWORD
Change-Id: Ibecedd195224ea53ff36f376897a6ff3c4e773d2
Reviewed-on: https://chromium-review.googlesource.com/757085
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-11-07 21:46:36 +00:00
Mark Mentovai
d3b7463c7a win: Recognize nsi.dll presenting as VFT_DRV/VFT2_DRV_NETWORK
This was previously proposed at
https://chromium-review.googlesource.com/c/crashpad/crashpad/+/339103/2/util/win/pe_image_reader_test.cc#84.
It didn’t land because the change was abandoned for other reasons, but
the fix was valid. nsi.dll is not VFT_APP or VFT_DLL, and if it’s
loaded, crashpad_snapshot_test PEImageReader.VSFixedFileInfo_AllModules
fails.

Although I can’t reproduce nsi.dll being loaded spontaneously in local
testing or on trybots, it occurred in the monolithic crashpad_tests at
https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/64492:

[ RUN      ] PEImageReader.VSFixedFileInfo_AllModules
../../third_party/crashpad/crashpad/snapshot/win/pe_image_reader_test.cc(90): error: Value of: observed.dwFileType == VFT_APP || observed.dwFileType == VFT_DLL
  Actual: false
Expected: true
Google Test trace:
../../third_party/crashpad/crashpad/snapshot/win/pe_image_reader_test.cc(164): C:\Windows\syswow64\NSI.dll
[  FAILED  ] PEImageReader.VSFixedFileInfo_AllModules (11 ms)

I can also reproduce locally by calling LoadLibrary(L"nsi.dll").

Bug: chromium:779790, chromium:782011
Test: crashpad_snapshot_test PEImageReader.VSFixedFileInfo_AllModules
Change-Id: I361c7d6521645913277a441ce38779aaa4a182c2
Reviewed-on: https://chromium-review.googlesource.com/757077
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-11-07 17:42:18 +00:00
Mark Mentovai
34f5e8d513 Only disable -Wconstant-conversion for Clang, not GCC
The warning suppression was recently added in a51e912004a6, and I don’t
know what I was thinking. I went out of my way to make it apply to both
Clang and GCC, but GCC doesn’t recognize this warning at all, nor does
it need any other warning suppressed.

Change-Id: I50341bfe81ee4799b3f6278d2e31ec31741952ac
Reviewed-on: https://chromium-review.googlesource.com/755654
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-11-07 00:25:09 +00:00
Joshua Peraza
18726100ed Move win/time to misc/time and add more conversion functions
This CL pulls together similar time conversion functions and adds
conversions between `FILETIME`s and `timespec`s.

Bug: crashpad:206
Change-Id: I1d9b1560884ffde2364af0092114f82e1534ad1c
Reviewed-on: https://chromium-review.googlesource.com/752574
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-11-06 22:37:27 +00:00
Mark Mentovai
b851b2590b Update mini_chromium to dd0c3e9680ae3c4c22f2221a2a75e48dd4a562ec
7f523b111c8c win: Don’t define c16*() functions in
             base/strings/string16.{cc,h}
dd0c3e9680ae Use ICU 60.1 as the basis for base/third_party/icu

Change-Id: I21b921c36a3ee1f989fa9786f60d980724577f64
Reviewed-on: https://chromium-review.googlesource.com/755215
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-11-06 20:16:15 +00:00
Joshua Peraza
d768538e39 Add ProcessSnapshotLinux
Bug: crashpad:30
Change-Id: Ie03592aeb91741d957b98716e4d4bb19695a42cf
Reviewed-on: https://chromium-review.googlesource.com/604627
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
2017-11-03 16:49:28 +00:00
Robert Sesek
b6a3d91342 Read annotation objects from the client when producing snapshots.
This wires up the annotation objects system of the client to the
snapshot production and minidump writing facilities.

Bug: crashpad:192
Change-Id: If7bb7625b140d71a15b84729372cbd0fd4bc63ef
Reviewed-on: https://chromium-review.googlesource.com/749870
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
2017-11-02 16:40:48 +00:00
Robert Sesek
79e2dd843e Include string annotation objects when uploading crash reports.
This extracts string annotation objects from the minidumps and includes
them as form POST key-value pairs.

This change also starts building a crashpad_handler_test binary on Mac.

Bug: crashpad:192
Change-Id: I68cbf6fda6f1e57c1e621d5e3de8717cfaea65bf
Reviewed-on: https://chromium-review.googlesource.com/749793
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-11-02 16:39:06 +00:00
Robert Sesek
4d7d4dd56c Flip gtest _EQ parameter order in minidump_annotation_writer_test.cc
Bug: crashpad:192
Change-Id: I82dddda5ba3d4fe5bf843572e6a793131cb6fa40
Reviewed-on: https://chromium-review.googlesource.com/751441
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
2017-11-02 16:17:06 +00:00
Mark Mentovai
2db30e035a win: Fix Clang -Wsign-compare warnings in new test code
This test code appeared in 9609b7471676, and was missed by the similar
warning cleanup of a51e912004a6, which was developed in parallel.

Bug: crashpad:192, chromium:779790
Change-Id: I4ed88ed025e4be4410c98ceaca395218f00007be
Reviewed-on: https://chromium-review.googlesource.com/750024
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-11-02 15:16:59 +00:00
Robert Sesek
620a29dbf3 Add support for reading annotation objects in ModuleSnapshotMinidump.
This will be used to include the annotations as form-post data when
uploading reports.

Bug: crashpad:192
Change-Id: I85ba9afd3cae7c96c0f8fe4f31a2460c97ed42d3
Reviewed-on: https://chromium-review.googlesource.com/747514
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-11-01 23:08:55 +00:00
Robert Sesek
e38c57a9c6 Expose the annotation_objects on MinidumpModuleCrashpadInfo.
This writes any set annotation objects list for a module into a
minidump, though no crash handler currently sets annotation objects for
a crashing process.

Bug: crashpad:192
Change-Id: Ib6d92edecb8d40061eaee08cbbc5c20dd1f048ef
Reviewed-on: https://chromium-review.googlesource.com/744942
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-11-01 22:44:16 +00:00
Robert Sesek
f9e587b036 Remove NOTREACHED() in ModuleSnapshot::AnnotationObjects() impls.
This is causing crashpad_handler_test to fail in Debug on Windows.

Bug: crashpad:192
Change-Id: Icf3ff387050ee2becf471f4e7c3a75394b1dd436
Reviewed-on: https://chromium-review.googlesource.com/749792
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-11-01 22:16:43 +00:00
Robert Sesek
3231a80e8b Add the minidump extension and writer for Annotation objects.
This adds extensions for MinidumpAnnotation and MinidumpAnnotationList
as well as their writer classes. Nothing currently connects the client-
side annotations to the writer, so annotations are not yet written into
minidumps.

Bug: crashpad:192
Change-Id: Ic51536157177921640ca15ae14e5e01ca875ae12
Reviewed-on: https://chromium-review.googlesource.com/731309
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-11-01 21:04:47 +00:00
Mark Mentovai
6950a552bf doc: Fix Doxygen-generated documentation after 34699d378b82
Bug: crashpad:192
Change-Id: Ia8b699ec3abe7491d30277d71f74e31f2fcc8343
Reviewed-on: https://chromium-review.googlesource.com/749311
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-11-01 17:17:04 +00:00
Mark Mentovai
4f5524fc68 Fix Windows build after 81eced5192d9
81eced5192d9 added a a dependency on
crashpad_snapshot_test_simple_annotations to crashpad_snapshot_test, but
9609b7471676 renamed this target to crashpad_snapshot_test_annotations.

I should have rebased onto HEAD, rebuilt, and retested before landing.
Bad developer! No candy. 🎃

Change-Id: I8fcd1020d8bd4ee163afa555ae6e815325485024
Reviewed-on: https://chromium-review.googlesource.com/748814
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-11-01 17:02:27 +00:00
Mark Mentovai
43b798b492 test: Fix paths for crashpad_tests monolith in Chromium
Instead of individual per-directory test executables like
crashpad_util_test, all Crashpad tests in Chromium will be run from a
single crashpad_tests executable.

Test: crashpad_util_test Paths.Executable, ProcessInfo.Self; crashpad_snapshot_test PEImageReader.DebugDirectory
Bug: chromium:779790
Change-Id: If95272fd641734fbdb8e231fbcdc4e7ccb2cb822
Reviewed-on: https://chromium-review.googlesource.com/749303
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-11-01 17:00:30 +00:00
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
Mark Mentovai
a51e912004 Fix warnings produced by trunk clang in test code
These are mostly -Wsign-compare warnings, with a -Wconstant-conversion
and a -Wunguarded-availability thrown in.

Bug: chromium:779790
Change-Id: Ic2103f3332ce57378db83eca7fa2569efec1a7b6
Reviewed-on: https://chromium-review.googlesource.com/746081
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-11-01 16:35:49 +00:00
Mark Mentovai
a0f4f294b1 win handler: Provide a wmain() entry point
Crashpad’s own build always uses wWinMain(), the default entry point for
/subsystem:windows, producing crashpad_handler.exe. crashpad_handler.com
is a /subsystem:console version produced by running editbin on a copy of
crashpad_handler.exe. This leaves the entry point intact, so both copies
use wWinMain(). crashpad_handler.com does not use wmain() as
traditionally used by /subsystem:console programs.

For the in-Chromium build’s tests, it is conveient to produce the
/subsystem:console version, crashpad_handler.com, directly as linker
output, as opposed to using editbin to transform a /subsystem:windows
version. This /subsystem:console version uses the normal wmain() entry
point.

By providing both wWinMain() and wmain(), both build types can be
accommodated.

Bug: chromium:779790
Change-Id: Ieb784db0cc245c6e4c12fb1dd83b8b95e159bdec
Reviewed-on: https://chromium-review.googlesource.com/746161
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-11-01 16:33:34 +00:00
Mark Mentovai
81eced5192 snapshot: Specify test dependencies properly
Two dependency targets were missing from crashpad_snapshot_test.

Change-Id: I9efba73639e529313d4aa49df5e68bb5117cf95a
Reviewed-on: https://chromium-review.googlesource.com/746121
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-11-01 16:27:55 +00:00
Mark Mentovai
8db7e7dc39 test: Don’t hide --gtest_* arguments from Chromium’s test runner
testing::InitGoogleMock() and testing::InitGoogleTest() modify argc and
argv, removing --gtest_* arguments that are processed. When building as
a part of Chromium, this prevents these arguments from being visible to
Chromium’s base::LaunchUnitTests() test runner.

Only call these initialization functions when using gtest’s native
RUN_ALL_TESTS() test runner.

Change-Id: I8242e1047f90d1cd923518a5cb9bd2527201ad25
Reviewed-on: https://chromium-review.googlesource.com/746082
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-11-01 16:26:51 +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
Robert Sesek
9609b74716 Expand the PEImageAnnotationsReader to read Annotation objects.
Nothing currently directs the handler to read these Annotation objects
from the target process, so they will not be read by Crashpad nor appear
in the minidump.

Bug: crashpad:192
Change-Id: I1eb1e9f42282c07e37d335631f0cc6083ef28a89
Reviewed-on: https://chromium-review.googlesource.com/726501
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-10-31 22:40:38 +00:00
Robert Sesek
e4723d524f Expand the MachOImageAnnotationsReader to read Annotation objects.
Nothing currently directs the handler to read these Annotation objects
from the target process, so they will not be read by Crashpad nor appear
in the minidump.

Bug: crashpad:192
Change-Id: I8ebabb4f5c77c5620b0d8e5036c3185eecfa4646
Reviewed-on: https://chromium-review.googlesource.com/717236
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-10-31 22:18:10 +00:00
Robert Sesek
38c8f51ae8 Add MinidumpByteArray extension and its Writer.
The MinidumpByteArray can be used to carry arbitrary blob payloads in a
minidump file.

Bug: crashpad:192
Change-Id: I1a0710b856375213cdd97eafa9247830aa9a9291
Reviewed-on: https://chromium-review.googlesource.com/716462
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
2017-10-31 21:03:47 +00:00
Robert Sesek
b88fde0b56 Add the AnnotationSnapshot object and attach it to ModuleSnapshot.
The AnnotationSnapshot is the handler-side of the Annotation object,
which will store the annotation data when read by a ProcessReader.

Bug: crashpad:192
Change-Id: Ic65c95022c452522678c1070c27c429dd631fb64
Reviewed-on: https://chromium-review.googlesource.com/717197
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-10-31 18:24:41 +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
1dae919b7e #include "base/logging.h" in client/annotation.h for DCHECK()
Bug: crashpad:192
Change-Id: I0da7d1721202794a7fb052731f4457bd5aa53b9f
Reviewed-on: https://chromium-review.googlesource.com/741887
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-10-27 18:01:07 +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
Sigurdur Asgeirsson
cb3aa9c4d8 DumpAndCrash in extended handler test in favor of debug break.
As the crashing function runs inside GoogleTests SEH handler,
I think it, or something in the OS may be interfering with the
exception dispatch somehow. In any case, if this flakes, we have
no one to blame but ourselves.

Bug: crashpad:773569
Change-Id: I2230d02735be4a71b688e1acc94d0ae6f082d9bd
Reviewed-on: https://chromium-review.googlesource.com/739464
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
2017-10-26 18:36:19 +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
Robert Sesek
34699d378b Create client data structures for typed Annotations.
This introduces the Annotation object, used to declare typed
annotations, and the AnnotationList object, used to reference these. The
AnnotationList is referenced by the CrashpadInfo structure. Currently
nothing reads these.

The AnnotationList implements a lock-free linked list, into which
Annotation objects are added exactly once, when they are first set.
Clearing an Annotation merely marks it internally as such, rather than
removing it from the list.

Bug: crashpad:192
Change-Id: I72414b1f83d624c4ae323e09ecea8cfb69a68c5e
Reviewed-on: https://chromium-review.googlesource.com/547135
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
2017-10-25 21:56:20 +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
Mark Mentovai
b96b9f627e win: Use test::ErrorMessage() in test::ChildLauncher
Context:
https://chromium-review.googlesource.com/c/chromium/src/+/735820#message-ea316c39d1418af51ceb165cdfbd0d1724798466

Change-Id: I9ad3731ea4a049fd90e18afb0c210b3928f9f674
Reviewed-on: https://chromium-review.googlesource.com/738533
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-10-25 20:00: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
8b63f1f00a snapshot/linux: Fix #includes
From edf4dde8ae10: one #include was missing, and another was sorted
incorrectly.

Change-Id: I77825f3909ae81ebf965f8c5527b44c95af29945
Reviewed-on: https://chromium-review.googlesource.com/734229
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-10-23 21:07:05 +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