652 Commits

Author SHA1 Message Date
Robert Sesek
5e24471188 Fix build break after 6d829e9af7d7630422d8095747905015d347b3be.
../../handler/crash_report_upload_thread.cc:142:7: error: field 'database_' will be initialized after field 'thread_' [-Werror,-Wreorder]
      database_(database),
      ^
1 error generated.

And:

..\util\thread\worker_thread_test.cc(25) : warning C4244: 'initializing' : conversion from 'double' to 'const uint64_t', possible loss of data

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

Review URL: https://codereview.chromium.org/1556043005 .
2016-01-04 17:42:04 -05:00
Robert Sesek
6d829e9af7 Create WorkerThread, an abstraction to perform some work on an interval.
This was extracted from CrashReportUploadThread and will be re-used for the
database pruning thread.

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

Review URL: https://codereview.chromium.org/1526563003 .
2016-01-04 17:10:58 -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
Mark Mentovai
8f3b0af215 client: Depend on compat
The upcoming https://codereview.chromium.org/1513573005/ requires that
std::move() in compat be available to callers in client.

crashpad_client_mac.cc uses macros not available in all SDK versions.

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1510243002 .
2015-12-09 17:31:10 -05:00
Mark Mentovai
a33736dd0d Fix AlignedAllocator for pre-C++11 libraries
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1511233002 .
2015-12-09 17:25:05 -05:00
Scott Graham
e9a843c626 win: Set shutdown order to make the handler shutdown as late as possible
At the default level, we might be terminated before the process we're
watching, which would mean missing shutdown crashes.

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

Review URL: https://codereview.chromium.org/1505423002 .
2015-12-08 16:55:02 -08: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
f55d18ade6 Add AlignedVector and use it for vector<MEMORY_BASIC_INFORMATION64>
MEMORY_BASIC_INFORMATION64 specifies an alignment of 16, but the
standard allocator used by containers doesn't honor this. Although 16
is the default alignment size used on Windows for x86_64, it's not for
32-bit x86. clang assumed that the alignment of the structure was as
declared, and used an SSE load sequence that required this alignment.

AlignedAllocator is a replacement for std::allocator that allows the
alignment to be specified. AlignedVector is an std::vector<> that uses
AlignedAllocator instead of std::allocator.

BUG=chromium:564691
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1498133002 .
2015-12-08 15:38:17 -05:00
Scott Graham
fabcd79057 Add missing comma in compat.gyp
Causes a warning in the gyp-msvs generator.

R=mark@chromium.org

Review URL: https://codereview.chromium.org/1490073005 .
2015-12-02 15:31:44 -08: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
Scott Graham
894a06070e win: Fix some warnings reported by /analyze
Reported by brucedawson@chromium.org.

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

Review URL: https://codereview.chromium.org/1489293004 .
2015-12-02 13:52:03 -08: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
Mark Mentovai
d81d83661d package.h: Update version number to 0.8.0
Per https://codereview.chromium.org/998653002/, we were going to call
the Windows client milestone 0.8.0. Now that the Crashpad client has
successfully shipped in a Chrome Windows canary, I think we can say that
we’re there. Or here.

Congratulations!

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1484013004 .
2015-12-01 15:08:27 -05:00
Scott Graham
72d6be5cb9 win: Fix debug iterator failure on empty database read/write
R=mark@chromium.org, rsesek@chromium.org
BUG=crashpad:80

Review URL: https://codereview.chromium.org/1492503002 .
2015-12-01 10:33:24 -08:00
Mark Mentovai
7764fa1144 Remove errant double-semicolons
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1489063002 .
2015-12-01 12:56:03 -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
Scott Graham
9b92d2fb71 win: Don't try to inherit console handles in StartHandler()
FILE_TYPE_CHAR handles can't be inherited via
PROC_THREAD_ATTRIBUTE_HANDLE_LIST, or CreateProcess() fails with
GetLastError() == 1450 on Windows 7.

I confirmed that an fprintf(stderr, ...) in HandlerMain() does make it
to the console when running tests even after this.

See bug for more discussion.

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

Review URL: https://codereview.chromium.org/1473793002 .
2015-11-24 16:36:27 -08:00
Mark Mentovai
116e1087f0 win: Don't warn when no VERSIONINFO resource is present in a module
Modules will work without a VERSIONINFO resource. This happens in some
of Crashpad's tests.

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1467993003 .
2015-11-23 16:17:50 -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
Mark Mentovai
c60ec9beeb Update gtest to 786564fa4a3c8e0f908acca32cce481de5481b9f
8b085f0d2168 Fix warnings encountered in MSVC build of gtest/gmock tests
322a4914a309 Better use of character constants
cbce23fb8670 Leave decltype(max_ulps_) alone and cast, not sure this is
             better
cfe466a0a753 Use a templated helper to wrap the cast
4a8e54401e49 Name the helper AsBits()
786564fa4a3c Merge pull request #627 from mark-chromium/tests_msvs

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1467283002 .
2015-11-23 13:17:11 -05:00
Mark Mentovai
e1dc5d6dbe Update mini_chromium to 43f04e7299621f708801bf475c84d9f294826ff9
43f04e729962 win: Compile with /bigobj

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1465083003 .
2015-11-23 12:52:06 -05:00
Scott Graham
74e69ed555 win: Avoid some more clang complaints
R=mark@chromium.org
BUG=chromium:82385

Review URL: https://codereview.chromium.org/1463633002 .
2015-11-19 15:09:59 -08:00
Scott Graham
0a717f0d27 win: class VM_COUNTERS -> struct
The fields aren't accessed anyway, but clang warns that they're
completely inaccessible. Also, matches all the other definitions in this
file.

R=mark@chromium.org

Review URL: https://codereview.chromium.org/1462203002 .
2015-11-19 14:26:36 -08:00
Scott Graham
3562fe4ccd win: Don't duplicate handles in handle restriction list
R=mark@chromium.org
BUG=crashpad:77

Review URL: https://codereview.chromium.org/1464473003 .
2015-11-19 11:25:52 -08:00
Mark Mentovai
4f09b58d1f Add RandomString() and its test, and use it everywhere it makes sense
This unifies several things that used a 16-character random string, and
a few other users of random identifiers where it also made sense to use
a 16-character random string.

TEST=crashpad_util_test RandomString.RandomString
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1451793002 .
2015-11-16 13:39:01 -05:00
Scott Graham
7a7d66ca89 win: Compile getopt as C++ and wrap in crashpad namespace
All our Main()s are in namespace crashpad already, so this just works.

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

Review URL: https://codereview.chromium.org/1439113002 .
2015-11-12 14:10:24 -08:00
Mark Mentovai
6e89d45d7d Switch googletest and googlemock to pull from the GitHub git repository
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1404033002 .
2015-11-11 11:13:37 -05:00
Mark Mentovai
97b0f86d0c Update mini_chromium to ea8d8de53f9407d57cab716fef2a36b6bf1b78b2
985372e5da14 win: Send log messages to a debugger via
             OutputDebugString()
06e0fa1309a0 win: Use FormatMessage[W]() instead of FormatMessageA() for
             logging
ea8d8de53f94 mac: Log messages to the system log via ASL

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1417803006 .
2015-11-10 17:03:09 -05:00
Mark Mentovai
7413569ea6 win: Explain the CreateFile() client-side pipe-opening loop
The bug and linked code review has more of the history, but we’ve been
tempted to remove the loop outright a couple of times already before
realizing that it serves an important purpose. Hopefully this comment
will protect our future selves from going on the same fool’s errand.

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

Review URL: https://codereview.chromium.org/1427643010 .
2015-11-10 16:43:13 -05:00
Mark Mentovai
4a7d599b64 mac: Scope crashpad_handler’s SIGTERM handler more broadly
Previously, there was a window after starting the upload thread but
before the SIGTERM handler was installed, where receipt of SIGTERM
could have interrupted an in-progress upload. There was also the
possibility that a second SIGTERM sent after the exception handler
stopped running would interrupt an in-progress upload. By pulling the
signal handler out of ExceptionHandlerServer and into the main
function, these races are avoided.

BUG=crashpad:25
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1429353002 .
2015-11-09 16:29:25 -05:00
Mark Mentovai
ee58effe14 mac: Log a message when restarting crashpad_handler
This is unexpected and important enough that the restart shouldn’t
happen silently.

BUG=crashpad:68
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1411313007 .
2015-11-09 15:57:46 -05:00
Mark Mentovai
1c1ce825bd win: generate_dump: PLOG() when OpenProcess() fails
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1408353011 .
2015-11-09 15:50:33 -05:00
Mark Mentovai
81ece89d6a Make ScopedGeneric uses slightly more consistent
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1426533008 .
2015-11-09 15:33:36 -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
Scott Graham
9e4cd8f07b win: Add DumpAndCrash to client
Something like this is required to implement something like
https://code.google.com/p/chromium/codesearch#chromium/src/components/crash/content/app/breakpad_win.cc&l=397
in Chrome (used by Syzygy and V8 in x64 it looks like).

I didn't want to expose UnhandledExceptionFilter() directly as it's
__stdcall so adding a forwarder to CrashpadClient seemed tidier, but the
functionality matches what is needed.

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

Review URL: https://codereview.chromium.org/1416603010 .
2015-11-06 14:08:13 -08:00
Mark Mentovai
d3825afb25 win: Make StartHandler() restrict HANDLES inherited by crashpad_handler
This requires Windows NT 6.0 (Vista and Server 2008). On earlier
operating system versions, the existing behavior of inheriting all
inheritable handles is retained.

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

Review URL: https://codereview.chromium.org/1427273003 .
2015-11-06 16:55:31 -05:00
Mark Mentovai
b666bcbe98 win: Use signed int as the integer representation of HANDLEs
HandleToInt() and IntToHandle() use int, a signed type, for the 32-bit
integer representation of HANDLE values. For opaque values, an unsigned
type would normally be used, but in this case, signed was chosen for
sign extension to work correctly. INVALID_HANDLE_VALUE is defined as
((HANDLE)(LONG_PTR)-1), and this needs to round-trip through the chosen
integer representation back to the same HANDLE value. Sign extension is
also recommended by
https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203.

As suggested in
https://codereview.chromium.org/1422503015/diff/1/util/win/handle.cc#newcode24

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1422023010 .
2015-11-06 15:03:13 -05:00
Scott Graham
6c1bd97df0 Fix compile after 20869d9
TBR=mark@chromium.org
BUG=chromium:546288

Review URL: https://codereview.chromium.org/1431813003 .
2015-11-06 10:59:33 -08:00
Scott Graham
20869d9468 Break crashpad_handler into lib and exe for Windows
I've heard/lived enough horror stories about AV, outbound-blocking
firewalls, etc. on Windows, that I think the best approach is to have
chrome.exe embed the majority of crashpad_handler and jump to it as
early as possible when running in that mode.

So, move most of crashpad_handler into a static_library with just main()
in the executable target.

R=mark@chromium.org
BUG=chromium:546288, crashpad:27

Review URL: https://codereview.chromium.org/1416873016 .
2015-11-06 10:52:09 -08:00
Scott Graham
e75e8c800f win: Lower integrity level of connection pipe
This is necessary to be able to connect to crashpad_handler from a
Chrome renderer.

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

Review URL: https://codereview.chromium.org/1405093013 .
2015-11-06 10:43:39 -08:00
Mark Mentovai
9a9076656f win: Fix ClockMonotonicNanoseconds()
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1419533014 .
2015-11-06 10:15:58 -05:00
Mark Mentovai
82ffeaa0f0 win: crashpad_util_test ProcessInfo.Handles doesn't work with CONOUT$
In 2eeaa3ac549a, I added a check to make sure that the expected CONOUT$
handle was found. Its omission seemed to be unintentional. The tests
passed for me on Windows 10, but failed on the bots. I can reproduce
the failures locally on Windows 7.

Doing the inheritance test with a file other than CONOUT$ fixes the
immediate problem, but we should find out why this CONOUT$ handle isn't
showing up in the handles list on Windows 7, fix it, and add back a
test.

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1428753009 .
2015-11-05 15:08:28 -05:00
Mark Mentovai
827e045279 win: Fix build after 2eeaa3ac549a
Review URL: https://codereview.chromium.org/1424713008 .
2015-11-05 14:06:18 -05:00