184 Commits

Author SHA1 Message Date
Mark Mentovai
acabe35928 doc: Fix all Doxygen warnings, cleaning up some generated documentation
This makes Doxygen’s output more actionable by setting QUIET = YES to
suppress verbose progress spew, and WARN_IF_UNDOCUMENTED = NO to prevent
warnings for undocumented classes and members from being generated. The
latter is too noisy, producing 721 warnings in the current codebase.

The remaining warnings produced by Doxygen were useful and actionable.
They fell into two categories: abuses of Doxygen’s markup syntax, and
missing (or misspelled) parameter documentation. In a small number of
cases, pass-through parameters had intentionally been left undocumented.
In these cases, they are now given blank \param descriptions. This is
not optimal, but there doesn’t appear to be any other way to tell
Doxygen to allow a single parameter to be undocumented.

Some tricky Doxygen errors were resolved by asking it to not enter
directiores that we do not provide documentation in (such as the
“on-platform” compat directories, compat/mac and compat/win, as well as
compat/non_cxx11_lib) while allowing it to enter the
“off-platform” directories that we do document (compat/non_mac and
compat/non_win).

A Doxygen run (doc/support/generate_doxygen.sh) now produces no output
at all. It would produce warnings if any were triggered.

Not directly related, but still relevant to documentation,
doc/support/generate.sh is updated to remove temporary removals of
now-extinct files and directories. doc/appengine/README is updated so
that a consistent path to “goapp” is used throughout the file.

Change-Id: I300730c04de4d3340551ea3086ca70cc5ff862d1
Reviewed-on: https://chromium-review.googlesource.com/408812
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-11-08 19:24:05 +00:00
Mark Mentovai
952f787f4a doc: Standardize on “macOS” in comments
Use “macOS” as the generic unversioned name of the operating system in
comments. For version-specific references, use Mac OS X through 10.6, OS
X from 10.7 through 10.11, and macOS for 10.12.

Change-Id: I1ebee64fbf79200bc799d4a351725dd73257b54d
Reviewed-on: https://chromium-review.googlesource.com/408269
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-11-08 19:21:44 +00:00
Scott Graham
b47bf6c250 Fix tests when running on Win10
The Windows 10 loader starts a few extra threads before main(). In a few
of the test cases, the tests were relying on thread ordering (generally,
the test thread being at index #1). Instead, use other signals to find
the correct thread to verify.

R=mark@chromium.org

Change-Id: Icb1f5a8fdf3a0ea6d82ab65960dbcb650965f269
Reviewed-on: https://chromium-review.googlesource.com/407073
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-11-03 18:51:01 +00:00
Mark Mentovai
375082098d mac: Fix tests on 10.12.1
crashpad_snapshot_test MachOImageAnnotationsReader.CrashDyld was failing
on 10.12.1. In 10.12, dyld’s intentional crashes come through
abort_with_payload(). In 10.12.1, it appears that the task port sent
along with abort_with_payload() crashes is now a corpse port, which has
a different port name than the task port that it originated from.

https://openradar.appspot.com/29079442

TEST=crashpad_snapshot_test MachOImageAnnotationsReader.CrashDyld
BUG=crashpad:137

Change-Id: I43f89c0f595dd5614fc910fa1f19f21ddf0a7c4d
Reviewed-on: https://chromium-review.googlesource.com/407087
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-11-03 14:44:42 +00:00
Mark Mentovai
bb7d249d65 Partially port the crashpad_client library to Linux/Android
This defines the global (per-module) CrashpadInfo structure properly on
Linux/Android, located via the “crashpad_info” section name.

Per the ELF specification, section names with a leading dot are reserved
for the system. Reading that, I realized that the same is true of Mach-O
sections with leading underscores, so this renames the section as used
on Mach-O from __DATA,__crashpad_info to __DATA,crashpad_info.

This change is sufficient to successfully build crashpad_client as a
static library on Linux/Android, but the library is incomplete. There’s
no platform-specific database implementation, no CaptureContext() or
CRASHPAD_SIMULATE_CRASH() implementation, and most notably, no
CrashpadClient implementation.

BUG=crashpad:30

Change-Id: I29df7b0f8ee1c79bf8a19502812f59d4b1577b85
Reviewed-on: https://chromium-review.googlesource.com/406427
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-11-02 23:19:50 +00:00
Scott Graham
2d87606bb5 win: Start crashpad_handler by inheriting connection data to it
Previously, StartHandler() launched the handler process, then connected
over a pipe to register for crash handling. Instead, the initial client
can create and inherit handles to the handler and pass those handle
values and other data (addresses, etc.) on the command line.

This should improve startup time as there's no need to synchronize with
the process at startup, and allows avoiding a call to CreateProcess()
directly in StartHandler(), which is important for registration for
crash reporting from DllMain().

Incidentally adds new utility functions for string/number conversion and
string splitting.

Note: API change; UseHandler() is removed for all platforms.

BUG=chromium:567850,chromium:656800

Change-Id: I1602724183cb107f805f109674c53e95841b24fd
Reviewed-on: https://chromium-review.googlesource.com/400015
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-10-21 20:35:58 +00:00
Scott Graham
660a5e69d6 win: switch crashpad_handler.exe to /subsystem:windows and add .com
This switches the default behaviour of crashpad_handler.exe to be a
/subsystem:windows app, so that normal usage won't cause a console to be
popped up. At the same time, creates a copy of crashpad_handler.exe in
the output dir named crashpad_handler.com. The .com doesn't affect
normal operation, as the way StartHandler() uses CreateProcess()
requires a real path to a file. However, when run from a command prompt,
.com are found before .exe, so editbin the .com to be to a console app,
which will be run in preference to the exe when run as just
"crashpad_handler", as one tends to do from a command prompt when
debugging. That is:

  d:\src\crashpad\crashpad\out\Debug>where crashpad_handler
  d:\src\crashpad\crashpad\out\Debug\crashpad_handler.com
  d:\src\crashpad\crashpad\out\Debug\crashpad_handler.exe

  d:\src\crashpad\crashpad\out\Debug>crashpad_handler --help
  Usage: crashpad_handler [OPTION]...
  ...

  d:\src\crashpad\crashpad\out\Debug>crashpad_handler.exe --help
  <no output>

  d:\src\crashpad\crashpad\out\Debug>crashpad_handler.com --help
  Usage: crashpad_handler.com [OPTION]...
  ...

We also use the .com file in test invocations so that output streams
will be visible.

R=mark@chromium.org

Change-Id: I1a27f88472d491b2a1d76e63c45e6415d9f679c0
Reviewed-on: https://chromium-review.googlesource.com/371578
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-08-17 20:50:47 +00:00
Mark Mentovai
9807cba2f4 end_to_end_test: accept mangled anonymous namespaces from cdb
When crashy_test_program's SomeCrashyFunction is inlined into
CrashyMain, cdb doesn't demangle the decorated form of an anonymous
namespace (?A0x12345678) into the expected `anonymous namespace' string.

I experienced this in Release_x64 and Release modes using MSVS 2015
update 3 (14.0.25420.1, cl 19.00.24213.1) and cdb versions 10.0.10240.9
and 10.0.14321.1024.

BUG=crashpad:129

Change-Id: I0a665b88891c271253adccd9b2b414fcaac26c8f
Reviewed-on: https://chromium-review.googlesource.com/368730
Reviewed-by: Scott Graham <scottmg@chromium.org>
2016-08-12 17:33:28 +00:00
Mark Mentovai
93485b2ccb Fix grammar in ModuleSnapshot::DebugFileName() documentation
Change-Id: Id3a9b5c1167e0d2a734af07f1f04e8c76f183c98
Reviewed-on: https://chromium-review.googlesource.com/368040
Reviewed-by: Scott Graham <scottmg@chromium.org>
2016-08-11 03:23:05 +00:00
Mark Mentovai
073ce275e0 De-tab and reindent after 7c807242e0b1
Change-Id: Ia68aa8294aa57d713066fbadd2200089e50e315b
Reviewed-on: https://chromium-review.googlesource.com/368030
Reviewed-by: Scott Graham <scottmg@chromium.org>
2016-08-11 02:17:59 +00:00
Mark Mentovai
7c807242e0 mac: dyld fatal errors appear as abort() on 10.12
In 10.12, dyld calls abort_with_payload() on fatal error from
dyld::halt(). In previous 10.12 betas, abort_with_payload() caused the
process to appear to terminate as exit(1). This was weird, so I filed
https://openradar.appspot.com/26894758. In 10.12db4 16A270f, Apple seems
to have fixed this bug. abort_with_payload() as used by dyld now causes
the process to appear to terminate as abort() as I had requested.

A Crashpad test that assures Crashpad’s ability to catch dyld crashes
needs its expectations updated with each change to a process’ apparent
termination code. It’s updated to expect SIGABRT on 10.12 or later. No
concessions are made for previous 10.12 betas or their buggy exit(1)
behavior. Nobody should be running any 10.12 beta prior to 10.12db4
16A270f now or at any point in the future.

This undoes (redoes) 335ef494677f.

BUG=crashpad:120
TEST=crashpad_snapshot_test MachOImageAnnotationsReader.CrashDyld

Change-Id: I13b330ac83fc9b33907ac172d35983974b8910f0
Reviewed-on: https://chromium-review.googlesource.com/365920
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-08-03 20:55:52 +00:00
Mark Mentovai
7b8de8a404 Adapt dyld_images.proctype to running changes in 10.12
The layout of dyld_all_image_infos changed slightly in 10.12db3 16A254g
and Xcode 8b3 8S174q.

BUG=crashpad:120
TEST=crashpad_snapshot_test ProcessTypes.DyldImagesSelf

Change-Id: I66fb60c80b26f465913f5100a8c40564723b0021
Reviewed-on: https://chromium-review.googlesource.com/361800
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-07-19 20:09:26 +00:00
Mark Mentovai
335ef49467 mac: dyld fatal errors appear as exit(1) on 10.12
exit(1) is a weird code for this, so I filed
https://openradar.appspot.com/26894758.

This doesn’t completely fix bug crashpad:121 unless both
crashpad_snapshot_test and crashpad_snapshot_test_no_op are signed with
the same Developer ID certificate. I’m hoping to get some action on
https://openradar.appspot.com/26902656, which will enable a complete fix
for this bug in unsigned developer builds. It would be unusual to have
to sign test executables.

BUG=crashpad:120,crashpad:121
TEST=crashpad_snapshot_test MachOImageAnnotationsReader.CrashDyld

Change-Id: I54fdfaa9178029b91ea3cbc12f2760dfa5124858
Reviewed-on: https://chromium-review.googlesource.com/355260
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-06-23 15:16:36 +00:00
Mark Mentovai
c281e30f93 mac: Update the dyld_all_image_infos structure for 10.12
BUG=crashpad:120

Change-Id: I7b2df5f2de13517b2586569ce267bcb0ae845101
Reviewed-on: https://chromium-review.googlesource.com/353830
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-06-20 16:14:24 +00:00
Mark Mentovai
495a64fcdb mac: Don’t check file offset values in Mach-O images
The Mach-O reader validated segment and section file offsets by checking
that they were relative to the same base, insisting that a section’s
file offset be the same distance from a segment’s file offset as the
section’s preferred load address was from the segment’s preferred load
address. Notably, these file offsets already could not be validated
against the Mach-O image’s start because in the dyld shared cache, for
all segments other than __TEXT, these offsets were relative to the dyld
shared cache’s start.

In 10.12dp1 16A201w, file offsets for sections in the __TEXT segment are
also relative to the dyld shared cache’s start, but the file offset for
the __TEXT segment itself is relative to the Mach-O image’s start. Being
relative to different positions breaks Crashpad’s sanity check of the
module data. https://openradar.appspot.com/26864860 is filed for the use
of distinct bases in what should be related file offset fields.

While it would be possible with a bit of work to identify modules within
the dyld shared cache and adjust expectations accordingly, in reality,
these file offset values were only used to verify that the Mach-O
module.

In addition, the file offsets stored within the Mach-O file for sections
are 32-bit quantities, even in 64-bit images. It is possible to create a
large image whose section offset values have overflowed, and in these
cases, the offset value verification would also fail.

For these reasons, all file offset value validation is removed from the
Mach-O image reader.

BUG=crashpad:118, crashpad:120

Change-Id: I9c4bcc5fd0aeceef3bc8a43e5a8651735852d87b
Reviewed-on: https://chromium-review.googlesource.com/353631
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-06-17 20:01:30 +00:00
Mark Mentovai
6fe7c5414e mac: Update cl_kernels workaround for 10.12
BUG=crashpad:120

Change-Id: If863a181cb0671a90752070a818efaa7eea89ff9
Reviewed-on: https://chromium-review.googlesource.com/353630
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-06-17 17:33:31 +00:00
Scott Graham
f497e54ccb win: Fix indirectly gathered memory cap
The limit was being reset per-thread, which isn't how it was intended or
documented. Add test to confirm the limit is more-or-less respected.

R=mark@chromium.org
BUG=crashpad:111

Change-Id: Ifae9f1ce2afcc2d6c6832db46f9b5c36adb35b42
Reviewed-on: https://chromium-review.googlesource.com/346131
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-05-20 15:47:28 +00:00
Scott Graham
b22cca6c3b win: Avoid variable shadowing warning for thread_id on VS2015
R=mark@chromium.org

Change-Id: I0bf09c96715161827bdad70bb375ad8193456d28
Reviewed-on: https://chromium-review.googlesource.com/342634
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-05-05 17:25:21 +00:00
Scott Graham
ab01df1ffe win: Adjust thread suspend count for DumpAndCrashTargetProcess() case
Because DumpAndCrashTargetProcess() suspends the process, the thread
suspend count is one too high for all threads other than the injection
one in the thread snapshots. Compensate for this when we detect this
type of exception.

BUG=crashpad:103

Change-Id: Ib77112fddf5324fc0e43f598604e56f77d67ff54
Reviewed-on: https://chromium-review.googlesource.com/340372
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-05-02 18:39:29 +00:00
Scott Graham
a02ba24006 Convert from scoped_ptr to std::unique_ptr
Follows https://codereview.chromium.org/1911823002/ but fixes includes
that were messed up there.

Change-Id: Ic4bad7d095ee6f5a1c9f8ca2d11ac9e67d55a626
Reviewed-on: https://chromium-review.googlesource.com/340497
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-04-25 19:16:26 +00:00
Scott Graham
cf452d9a86 Fix Mac after dbfcb5d03
Change-Id: I9d889d8de5f7db9be8da99a708d8e6434dc9c93e
Reviewed-on: https://chromium-review.googlesource.com/340361
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-04-25 18:10:20 +00:00
Scott Graham
6a6a0c27ed win: Support dumping another process by causing it to crash
Adds a new client API which allows causing an exception in another
process. This is accomplished by injecting a thread that calls
RaiseException(). A special exception code is used that indicates to the
handler that the exception arguments contain a thread id and exception
code, which are in turn used to fabricate an exception record. This is
so that the API can allow the client to "blame" a particular thread in
the target process.

The target process must also be a registered Crashpad client, as the
normal exception mechanism is used to handle the exception.

The injection of a thread is used instead of DebugBreakProcess() which
does not cause the UnhandledExceptionFilter() to be executed.
NtCreateThreadEx() is used in lieu of CreateRemoteThread() as it allows
passing of a flag which avoids calling DllMain()s. This is necessary to
allow thread creation to succeed even when the target process is
deadlocked on the loader lock.

BUG=crashpad:103

Change-Id: I797007bd2b1e3416afe3f37a6566c0cdb259b106
Reviewed-on: https://chromium-review.googlesource.com/339263
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-04-22 17:27:58 +00:00
Scott Graham
dbfcb5d032 win: Cap indirect memory gathering
Add a user-configurable cap on the amount of memory that is gathered by
dereferencing thread stacks. (SyzyAsan stores a tremendously large
number of pointers on the stack, so the dumps were ending up in the ~25M
range.)

Also reduce the range around pointers somewhat.

Change-Id: I6bce57d86bd2f6a796e1580c530909e089ec00ed
Reviewed-on: https://chromium-review.googlesource.com/338463
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-04-22 17:04:33 +00:00
Scott Graham
0d4438b2a5 win: Don't add zero-sized memory ranges to dump
One possible cause for this would be a register "pointing" to the edge of an
inaccessible range. Having these zero-sized ranges doesn't break the minidump,
but it causes a warning when opening in windbg.

Also drop user-supplied zero-length memory ranges for the same reason.

BUG=crashpad:104

Change-Id: I2c5acc54f04fb617806cecd87ac4ad5db93f3db8
Reviewed-on: https://chromium-review.googlesource.com/339210
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-04-19 19:41:36 +00:00
Scott Graham
71f6724239 Disable end-to-end test of extra memory range removal
In debug builds, the extra memory is sometimes getting captured
(probably by a stale stack pointer), so disable this test for now to
un-red the bots. We can probably fix it by moving this one test to a
separate binary (or perhaps just removing it, I'm not sure it's that
useful anyway above and beyond the unit test.)

R=mark@chromium.org
BUG=crashpad:101

Change-Id: I98a58a467fb4a4d9f84d2e0d020a031a0ea9743c
Reviewed-on: https://chromium-review.googlesource.com/334821
Reviewed-by: Scott Graham <scottmg@chromium.org>
2016-03-24 21:22:39 +00:00
Scott Graham
c307f94f19 Support custom streams in the minidump
BUG=crashpad:95

Change-Id: Iee956906651dfd56e0ae3d2bcec82daabdc97067
Reviewed-on: https://chromium-review.googlesource.com/329733
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-03-02 17:39:34 +00:00
Scott Graham
70ae71fe51 Another Mac fix after 7217cc0a8f26 -- correct bad CrashpadInfo proctype layout
Change-Id: Ieb8a45d8ff0526d970829f6a71915edd5a2c750f
Reviewed-on: https://chromium-review.googlesource.com/329716
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-02-29 22:40:31 +00:00
Scott Graham
7217cc0a8f Support client-specified extra memory ranges
Change-Id: I378e2513a4894fb1548445b660bb3db86e281572
Reviewed-on: https://chromium-review.googlesource.com/329564
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-02-29 22:16:13 +00:00
Scott Graham
badfacccee win: Add support for capturing unloaded modules
R=mark@chromium.org
BUG=crashpad:89

Change-Id: Ib6a67147e538811168d68f14a457fdceab30c02e
Reviewed-on: https://chromium-review.googlesource.com/327231
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-02-18 00:55:38 +00:00
Scott Graham
feb3aa3923 win: Capture memory pointed to by the stack
Change-Id: Ide75475aa9c42edf36c3a709bfc7dfbfed68b0d3
Reviewed-on: https://chromium-review.googlesource.com/322261
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-01-29 18:13:19 +00:00
Scott Graham
83247fda60 Fix Mac after af3dc54f
CrashpadInfo not being initialized/propagated properly on Mac.

Change-Id: I5f33a16e4e18bb1b068e0d4aeb7f2032a6cb6278
Reviewed-on: https://chromium-review.googlesource.com/324500
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-01-28 21:16:16 +00:00
Scott Graham
af3dc54f2a Add a field to CrashpadInfo to control indirect memory capture
Change-Id: I6467aafba5d20f8a199bab0e2476f98a5318f84a
Reviewed-on: https://chromium-review.googlesource.com/322245
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-01-28 20:16:01 +00:00
Patrick Monette
4794225f22 Adding an API to read module annotations in snapshot.gyp
Kasko needs a way to read crash keys from out of process. This API
reuses the functionality of PEImageAnnotationsReader.

Change-Id: I2f3bbc358212e6f50235183e9dbb4e5a2cf989cf

This is a reupload of https://codereview.chromium.org/1586433003/ but
for gerrit.

Change-Id: I2f3bbc358212e6f50235183e9dbb4e5a2cf989cf
Reviewed-on: https://chromium-review.googlesource.com/322550
Reviewed-by: Scott Graham <scottmg@chromium.org>
Tested-by: Scott Graham <scottmg@chromium.org>
Reviewed-by: Scott Graham <scottmg@google.com>
2016-01-18 20:35:42 +00:00
Scott Graham
417097b91f win: Better setting of DI for register capture test
The previous approach was nice for its simplicity, but unfortunately
didn't work when the compiler decided to do some of its confounded
"optimization".

R=mark@chromium.org
BUG=crashpad:86, chromium:571144

Review URL: https://codereview.chromium.org/1563273004 .
2016-01-10 13:32:20 -08:00
Scott Graham
5af9c42638 win: Capture some memory pointed at by context
R=mark@chromium.org
BUG=crashpad:86, chromium:571144

Review URL: https://codereview.chromium.org/1533183002 .
2016-01-08 17:24:04 -08:00
Mark Mentovai
6d2d31d2d1 Use base/macros.h instead of base/basictypes.h
This was done in Chromium’s local copy of Crashpad in 562827afb599. This
change is similar to that one, except more care was taken to avoid
including headers from a .cc or _test.cc when already included by the
associated .h. Rather than using <stddef.h> for size_t, Crashpad has
always used <sys/types.h>, so that’s used here as well.

This updates mini_chromium to 8a2363f486e3a0dc562a68884832d06d28d38dcc,
which removes base/basictypes.h.

e128dcf10122 Remove base/move.h; use std::move() instead of Pass()
8a2363f486e3 Move basictypes.h to macros.h

R=avi@chromium.org

Review URL: https://codereview.chromium.org/1566713002 .
2016-01-06 12:22:50 -05:00
Bruce Dawson
b0394744cc Fix some VS 2015 warnings
Fix some warnings when compiling crashpad with VC++ 2015 Update 1.

Warning 4302 occurs if you convert from a pointer to a <sizeof(void*)
integer in one cast, because this often indicates an accidental pointer
truncation which can be a bug in 64-bit builds.

Warning 4577 warns that noexcept will not be enforced, but we don't want
it to be enforced anyway, so I disabled it. The full warning is:

warning C4577: 'noexcept' used with no exception handling mode specified
termination on exception is not guaranteed. Specify /EHsc

BUG=440500
R=mark@chromium.org

Review URL: https://codereview.chromium.org/1527803002 .

Patch from Bruce Dawson <brucedawson@chromium.org>.
2015-12-14 20:01:05 -05:00
Mark Mentovai
583d1dc3ef Provide std::move() in compat instead of using crashpad::move()
This more-natural spelling doesn’t require Crashpad developers to have
to remember anything special when writing code in Crashpad. It’s easier
to grep for and it’s easier to remove the “compat” part when pre-C++11
libraries are no longer relevant.

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1513573005 .
2015-12-09 17:36:32 -05:00
Scott Graham
b9e732d318 win: Fix a few sign mismatch warnings in crashpad.
BUG=chromium:567877
R=mark@chromium.org, scottmg@chromium.org

Review URL: https://codereview.chromium.org/1503403003 .
2015-12-08 14:21:29 -08:00
Mark Mentovai
7efdc94f59 Fixes for Chromium checkperms.py PRESUBMIT
BUG=chromium:472900
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1508193002 .
2015-12-08 16:24:54 -05:00
Mark Mentovai
47fbac4ae1 win: Placate more OS versions in the PEImageReader test
The BaseName() was added because system DLLs were being reported by
GetFileVersionInfo()/VerQueryValue() as having major file versions of
6.2 instead of 10.0 on Windows 10 when accessed by full path, but not
by BaseName(). The PEImageReader gets the correct version from the
in-memory images, 10.0.

This trick didn't work on Windows XP, where two copies of comctl32.dll
were found loaded into the process, one with a major file version of
5.82 and the other with 6.0. Giving GetFileVersionInfo() the BaseName()
would result in it returning information from one of these, which would
cause the version to not match when the PEImageReader was looking at the
other.

All of these GetFileVersionInfo() quirks make me glad that we're not
using it anymore (outside of the test).

Because of the version numbers involved (NT 6.2 = Windows 8, where
GetVersion()/GetVersionEx() start behaving differently for
non-manifested applications) and the fact that GetFileVersionInfo()
and VerQueryValue() seem to report 10.0 even with full paths on Windows
10 in applications manifested to run on that OS, the BaseName() thing is
restricted to Windows 8 and higher.

TEST=crashpad_snapshot_test PEImageReader.VSFixedFileInfo_AllModules
BUG=crashpad:78
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1493933002 .
2015-12-02 17:48:20 -05:00
Mark Mentovai
384fd93d6d win: Add a VSFixedFileInfo test to check all modules
BUG=crashpad:78
TEST=crashpad_snapshot_test PEImageReader.VSFixedFileInfo_AllModules
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1485333002 .
2015-12-02 11:11:54 -05:00
Mark Mentovai
5be8ce4ea0 Get module versions and types from in-memory images
Don't call GetFileVersionInfo(), which calls LoadLibrary() to be able to
access the module's resources. Loading modules from the crashy process
into the handler process can cause trouble. The Crashpad handler
definitely doesn't want to run arbitrary modules' module initializer
code.

Since the VS_FIXEDFILEINFO needed is already in memory in the remote
process' address space, just access it from there.

BUG=crashpad:78
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1475023004 .
2015-12-01 17:06:37 -05:00
Dana Jansens
6bebb10829 Replace use of .Pass() with crashpad::move().
Since C++11 library support isn't available everywhere crashpad is
compiled, add our own move() method in the crashpad namespace to replace
std::move() for now. Replace uses of .Pass() with this method.

R=mark@chromium.org, scottmg@chromium.org
BUG=chromium:557422

Review URL: https://codereview.chromium.org/1483073004 .
2015-11-30 14:20:54 -08:00
Scott Graham
1f3ced1846 win: Only capture the loader lock for now
On Win 7 in a debug configuration, walking all locks was gathering
hundreds of thousands of locks, causing test timeouts to be exceeded in
debug. On user machines, UnhandledExceptionHandler() could have timed
out too. For now, only grab the loader lock as it's the most interesting
one. Unfortunately, this means that !locks won't work for now.

In the future, we may want to figure out a signalling mechanism so that
the client can note other interesting locks to be grabbed, and just
avoid walking the list entirely.

R=mark@chromium.org
BUG=chromium:546288

Review URL: https://codereview.chromium.org/1475033005 .
2015-11-30 12:29:21 -08:00
Scott Graham
866cffce8a Revert "win: Cap number of locks gathered"
This reverts commit b3464d96f5fc0d82f860651b7918626dfbd80d65.

It was temporarily landed to be able to run as the DEPS version in Chrome.

BUG=

Review URL: https://codereview.chromium.org/1474223002 .
2015-11-26 12:23:12 -08:00
Scott Graham
b3464d96f5 win: Cap number of locks gathered
On Win 7 in a debug configuration, this was gathering hundreds of
thousands of locks, causing test timeouts to be exceeded. On user
machines, UnhandledExceptionHandler() probably would have timed out
also. Arbitrarily cap the number of locks captured, as we don't have a
pressing need for anything other than the LoaderLock anyway.

In the future, we may want to figure out a signalling mechanism so that
the client can note other interesting locks to be grabbed, and just
avoid walking the list entirely.

R=mark@chromium.org
BUG=chromium:546288

Review URL: https://codereview.chromium.org/1475033005 .
2015-11-26 12:19:26 -08:00
Hans Wennborg
33414779e1 Build fixes for Clang on Windows
See e.g. http://build.chromium.org/p/chromium.fyi/builders/CrWinClang/builds/4502/steps/compile/logs/stdio

BUG=chromium:82385
R=mark@chromium.org

Review URL: https://codereview.chromium.org/1473073008 .

Patch from Hans Wennborg <hans@chromium.org>.
2015-11-25 23:50:31 -05:00
Mark Mentovai
f5c4273d4f win: Only call GetFileVersionInfo() once per module
This log spam from end_to_end_test.py indicated that
GetFileVersionInfo() was being called three times per module:

[3076:3424:20151123,102817.290:WARNING module_version.cc:29]
GetFileVersionInfoSize: ...\crashpad\out\Release_x64\crashy_program.exe:
The specified resource type cannot be found in the image file. (1813)
[3076:3424:20151123,102817.291:WARNING module_version.cc:29]
GetFileVersionInfoSize: ...\crashpad\out\Release_x64\crashy_program.exe:
The specified resource type cannot be found in the image file. (1813)
[3076:3424:20151123,102817.291:WARNING module_version.cc:29]
GetFileVersionInfoSize: ...\crashpad\out\Release_x64\crashy_program.exe:
The specified resource type cannot be found in the image file. (1813)

This is unnecessary. It only needs to be called once.

We may want to avoid logging in GetModuleVersionAndType() when
GetLastError() is ERROR_RESOURCE_TYPE_NOT_FOUND.

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1472963002 .
2015-11-23 16:04:25 -05:00
Scott Graham
ff274507dc win: Only retry in UseHandler() loop on ERROR_PIPE_BUSY
This is better because now end_to_end_test.py fails immediately with

[1180:9020:20151106,145204.830:ERROR registration_protocol_win.cc:39] CreateFile: The system cannot find the file specified.  (0x2)

R=mark@chromium.org
BUG=crashpad:75

Review URL: https://codereview.chromium.org/1409693011 .
2015-11-06 15:54:48 -08:00