177 Commits

Author SHA1 Message Date
Mark Mentovai
809ea8158d test: Move util/test to its own top-level directory, test.
After 9e79ea1da719, it no longer makes sense for crashpad_util_test_lib
to “hide” in util/util_test.gyp. All of util/test is moved to its own
top-level directory, test, which all other test code is allowed to
depend on. test, too, is allowed to depend on all other non-test code.

In a future change, when crashpad_util_test_lib gains a dependency on
crashpad_client, it won’t look so weird for something in util (even
though it’s in util/test) to depend on something in client, because the
thing that needs to depend on client will live in test, not util.

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

Review URL: https://codereview.chromium.org/1051533002
2015-03-31 17:44:14 -04:00
Mark Mentovai
d5ddd14ee1 Improve map insertion operations.
Add MapInsertOrReplace<>() to insert a key-value pair into a map if the
key is not already present, or replace the existing value for key if the
key is present. The original value can optionally be returned to the
caller in this case.

Map insertions now use either MapInsertOrReplace<>() or
std::map<>::insert() directly.

Use MapInsertOrReplace<>() when the map should be updated to contain a
mapping from a key to a value regardless of whether the key is already
present.

Use std::map<>::insert() to insert a mapping from a key to a value
without replacing any existing mapping from a key, if present. If it is
important to know whether an existing mapping from a key was present,
use the returned std::pair<>.second. If it is important to know the
existing value, use the returned std::pair<>.first->second.

This change has a slight positive impact on performance.

TEST=crashpad_util_test MapInsert.MapInsertOrReplace and others
BUG=
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1044273002
2015-03-31 14:29:32 -04:00
Mark Mentovai
5d0a133ecd Tolerate weird cl_kernels modules.
cl_kernels modules (OpenCL kernels) are not structured as correct Mach-O
images on Mac OS X 10.10, but they’re present frequently enough that
it’s worth detecting and tolerating their quirks.

As discussed in
https://groups.google.com/a/chromium.org/d/msg/crashpad-dev/NaB7PrfW04g/FanqNJkVBfUJ

Apple bug: https://openradar.appspot.com/20239912

TEST=crashpad_snapshot_test ProcessReader.*
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1019243006
2015-03-23 16:27:42 -04:00
Mark Mentovai
6bf80c3e48 Add MinidumpCrashpadInfo::report_id.
Now that Chrome’s about:crashes displays the crash report UUID, I wanted
to add it to the minidump. In the future, we may be able to index these
on the server. This will also help identify dumps that correspond to the
same event once we’re equipped to convert between different formats.

Ideally, this new field is populated with the same UUID used locally in
the crash report database. To make this work,
CrashReportDatabase::NewReport must carry the UUID. This was actually
part of CrashReportDatabaseWin’s private extension to NewReport, so that
extension subclass can now be cleaned up.

TEST=crashpad_minidump_test MinidumpCrashpadInfoWriter.*,
     crashpad_client_test CrashReportDatabaseTest.NewCrashReport

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1000263003
2015-03-13 13:00:56 -04:00
Mark Mentovai
359bdd8622 Carry the client ID from the database all the way through upload.
The client ID is added to a new field, MinidumpCrashpadInfo::client_id,
in each minidump file that is written. The ProcessSnapshot::ClientID()
gives access to value at the snapshot level. In the upload thread,
client IDs are retrieved from minidump files and used to populate the
“guid” HTTP form parameter.

The Breakpad client supplies these values at upload without hyphens and
with all capital letters. Currently, the Crashpad client uses hyphens
and lowercase letters when communicating with a Breakpad server.

TEST=crashpad_minidump_test MinidumpCrashpadInfoWriter.*,
     crashpad_snapshot_test ProcessSnapshotMinidump.*,
     run_with_crashpad --handler crashpad_handler \
         -a --database=/tmp/crashpad_db \
         -a --url=https://clients2.google.com/cr/staging_report \
         -a --annotation=prod=crashpad \
         -a --annotation=ver=0.7.0 \
         crashy_program

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/998033002
2015-03-11 17:10:50 -04:00
Mark Mentovai
9b7ff0ea5a Allow exception forwarding to the system’s native crash reporter to be
disabled.

ClientInfo::set_system_crash_reporter_forwarding() can be used to
disable forwarding. The first module that is found with a non-default
value in this field will dictate whether forwarding is enabled or
disabled. It is possible to enable or disable reporting with this call,
as well as reset it to default, which will allow later modules a chance
to influence the behavior.

ClientInfo::set_crashpad_handler_behavior() is also provided, which can
be used to disable Crashpad’s handling of the exception. Most users
should not call this, but should use Settings::SetUploadsEnabled()
instead.

TEST=crashpad_snapshot_test \
         CrashpadInfoClientOptions.*:MachOImageReader.Self_DyldImages; \
     run_with_crashpad --handler crashpad_handler \
         -a --database=/tmp/crashpad_db \
         -a --url=https://clients2.google.com/cr/staging_report \
         -a --annotation=prod=crashpad \
         -a --annotation=ver=0.7.0 \
         crashy_program

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/997713002
2015-03-11 17:07:11 -04:00
Mark Mentovai
56399b2553 snapshot/mac: MachOImageAnnotationsReader test shouldn’t crash with a
nullptr ProcessReader::Module.

Prior to 64b87325b9de, the alignment problem meant that the Module for
dyld was looking at the wrong address instead of dyld’s correct load
address when a 32-bit process attempted to examine a crashing 64-bit
process. This resulted in a crash during the
MachOImageAnnotationsReader.CrashDyld test.

ProcessReader::Module pointers are permitted to be nullptr. This allows
minimal module data (its name) to be preserved even when no sense can be
made of the module based on its load address. The producer,
ProcessReader::InitializeModules(), and the non-test consumer,
ModuleSnapshotMac::Initialize(), both accept this correctly. The
producer’s documentation is updated to call this out. The ProcessReader
test is also updated to tolerate this case without crashing by adding
assertions.

TEST=snapshot_test MachOImageAnnotationsReader.*, ProcessReader.*
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/989713002
2015-03-08 21:02:42 -04:00
Mark Mentovai
64b87325b9 snapshot/mac: align members of 64-bit dyld_all_image_infos properly in
32-bit mode.

TEST=snapshot_test (specifically MachOImageAnnotationsReader.CrashDyld)
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/984023005
2015-03-06 18:44:43 -05:00
Mark Mentovai
445c0eae7c snapshot: Add a minimal ModuleSnapshotMinidump and accessor from
ProcessSnapshotMinidump.

ModuleSnapshotMinidump is currently only capable of reading module
annotations, in both vector and simple-dictionary forms. It does not
read any other module information from minidump files. These annotations
are all that are necessary to be able to upload Crashpad-produced
minidumps to Breakpad crash processor servers, because Breakpad accepts
them as HTTP POST parameters, while Crashpad places them in the minidump
file itself.

TEST=snapshot_test ProcessSnapshotMinidump.Modules
BUG=crashpad:10
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/972383002
2015-03-04 12:29:01 -05:00
Mark Mentovai
40b931bd8e Add ProcessSnapshotMinidump, the beginning of the minidump variant of
the Snapshot family.

For the time being, only ProcessSnapshotMinidump::AnnotationsSimpleMap()
is implemented. In order to complete the initial uploader for Crashpad,
ModuleSnapshotMinidump::AnnotationsSimpleMap() is also needed, to be
accessed by ProcessSnapshotMinidump::Modules().

TEST=snapshot_test ProcessSnapshotMinidump.*
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/932153003
2015-02-26 14:43:10 -05:00
Mark Mentovai
a0f5dc62a4 Add MinidumpCrashpadInfo::simple_annotations.
Some annotations will exist at a broader scope than per-module, which is
the only place that annotations can currently be stored. The product
name and version are not under the control of any module, but are
established when the first Crashpad client establishes a handler. These
annotations will be stored in a minidump’s MinidumpCrashpadInfo
structure, which applies to the entire minidump.

Within the snapshot interface, this data is carried within the
“process” snapshot because it is the top-level structure in that family.
Note that the data may not correspond directly with a process, however.

TEST=minidump_test MinidumpCrashpadInfoWriter.*
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/924673003
2015-02-17 17:38:02 -05:00
Mark Mentovai
878400947f MachOImageSymbolTableReader: permit indirect symbols to exist among
external defined symbols.

Indirect symbols remain unsupported.

Xcode 5.1 ld64-236.3/src/ld/LinkEditClassic.hpp
ld::tool::SymbolTableAtom<>::addGlobal() is responsible for producing
symbols found in the extdef portion of the symbol table. It always sets
the type to N_ABS, N_SECT, or N_INDR, each with the N_EXT bit set. The
N_PEXT bit is never set for non-object files, and we’re not concerned
with reading object files. With this change to tolerate N_INDR, I think
we’re covering all of the symbol types that we might find ld writing to
the extdef portion of the symbol table.

https://groups.google.com/a/chromium.org/forum/#!topic/crashpad-dev/k7QkLwO71Zo

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/901463004
2015-02-05 13:39:20 -05:00
Scott Graham
892c29e8ba Reorganize Multiprocess and implement for Windows
- Various "FD" to "Handle"
- Existing Multiprocess implementation moves to _posix.
- Stub implementation for _win.

At the moment, multiprocess_exec_win.cc contains implementations of both
Multiprocess methods and MultiprocessExec functions. This will need more
work in the future, but reflects the idea that all tests should be in
terms of MultiprocessExec eventually.

Currently, this works sufficiently to have util_test succeed (including
multiprocess_exec_test, and the recently ported HTTPTransport tests.)

R=mark@chromium.org
BUG=crashpad:1, crashpad:7

Review URL: https://codereview.chromium.org/880763002
2015-01-28 14:49:42 -08:00
Scott Graham
10165ce449 Cross platform low level file IO wrappers
Rename fd_io to file_io, and ReadFD to ReadFile, etc.

file_io.cc is the higher level versions that call the basic ReadFile/WriteFile
and then file_io_posix.cc and file_io_win.cc are the implementations of
those functions.

The Windows path is as yet untested, lacking the ability to link the test binary.

R=cpu@chromium.org, mark@chromium.org
BUG=crashpad:1

Review URL: https://codereview.chromium.org/811823003
2014-12-17 14:35:18 -08:00
Mark Mentovai
c874958fd0 MachMessageServer: eliminate argument redundancy.
MachMessageServer::Run()’s distinct |nonblocking| parameter is removed.
The information it formerly conveyed is now implied by the |timeout_ms|
parameter, which can accept two special values,
kMachMessageTimeoutNonblocking and kMachMessageTimeoutWaitIndefinitely.

TEST=client_test, snapshot_test, util_test
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/777993002
2014-12-10 11:11:21 -05:00
Mark Mentovai
821ed8fe0f UniversalMachExcServer: eliminate multiple implementation inheritance.
UniversalMachExcServer provided both an interface and an implementation,
contrary to the other classes in the exc_server_variants family. This
was mostly done for reasons of economy in an already-large class family.
Unfortunately, this decision meant that it was impossible for other code
to use UniversalMachExcServer, which required that CatchMachException()
be implemented, and also extend another class without violating the
style guide’s prohibition of multiple implementation inheritance. This
became a problem in a lot of test code, which extended MachMultiprocess
and UniversalMachExcServer.

UniversalMachExcServer is now given its own nested Interface class,
which is a pure interface. All users of UniversalMachExcServer are
changed from “is-a” UniversalMachExcServer to “has-a”
UniversalMachExcServer and “is-a” UniversalMachExcServer::Interface.

TEST=client_test, snapshot_test, util_test
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/775943005
2014-12-04 10:18:24 -05:00
Mark Mentovai
0437bc53b6 Pass Mach message trailers to server handler functions.
TEST=util_test ChildPortServer.*:ExcServerVariants.*:MachMessageUtil.*
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/755313004
2014-12-01 16:06:56 -05:00
Mark Mentovai
79b4434c81 Add a ReceiveLarge parameter to MachMessageServer::Run().
Previously, MachMessageServer::Run() only provided two strategies for
dealing with large messages, indicated by mach_msg() returning
MACH_RCV_TOO_LARGE: the receive buffer could be reallocated and the
message received, or the entire function could return MACH_RCV_TOO_LARGE
to the caller. There are situations where an intermediate behavior might
be desirable. This intermediate behavior would allow the function to
continue waiting for another message without returning an error to the
caller or attempting to receive the large message. This is desirable
when dealing with fixed-sized messages and a receiver that might be sent
messages by unknown, possibly-malicious callers. This can happen when
the corresponding send right is published with the bootstrap server, for
example.

Existing users continue to request their existing behavior, typically
receiving an error when encountering a large message.
catch_exception_tool will use the new “ignore” behavior when running in
persistent mode.

TEST=util_test MachMessageServer.*
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/756803002
2014-11-25 14:48:44 -05:00
Mark Mentovai
6812cec67e Move some parts of ProcessReader (in snapshot) to ProcessInfo (in util).
Also, move ProcessArgumentsForPID() into ProcessInfo.

This change prepares for a TaskForPID() implementation that’s capable of
operating correctly in a setuid root executable. TaskForPID() belongs in
util/mach, but for its permission checks, it must access some process
properties that were previously fetched by ProcessReader in snapshot.
util can’t depend on snapshot. The generic util-safe process information
bits (Is64Bit(), ProcessID(), ParentProcessID(), and StartTime()) are
moved from ProcessReader to ProcessInfo (in util), where the current
ProcessReader can use it (as it’s OK for snapshot to depend on util),
and the future TaskForPID() in util can also use it. ProcessInfo also
contains other methods that TaskForPID() will use, providing access to
the credentials that the target process holds. ProcessArgumentsForPID()
is related, and is also now a part of ProcessInfo.

TEST=snapshot_test, util_test
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/727973002
2014-11-14 17:54:42 -05:00
Mark Mentovai
48b1964d1b Use implicit_cast<> instead of static_cast<> whenever possible.
implicit_cast<> only performs a cast in cases where an implicit
conversion would be possible. It’s even safer than static_cast<> It’s an
“explicit implicit” cast, which is not normally necsesary, but is
frequently required when working with the ?: operator, functions like
std::min() and std::max(), and logging and testing macros.

The public style guide does not mention implicit_cast<> only because it
is not part of the standard library, but would otherwise require it in
these situations. Since base does provide implicit_cast<>, it should be
used whenever possible.

The only uses of static_cast<> not converted to implicit_cast<> are
those that require static_cast<>, such as those that assign an integer
constant to a variable of an enum type.

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/700383007
2014-11-06 16:44:38 -05:00
Mark Mentovai
de0979b930 C++11: Use type aliases instead of typedefs.
This replaces all occurrences of “typedef Y X;” with “using X = Y;”.

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/700143004
2014-11-05 14:09:01 -05:00
Mark Mentovai
4299ab3c4c Add MinidumpMemoryListWriter::AddFromSnapshot(), everything downstream,
and its test.

TEST=minidump_test MinidumpMemoryWriter.AddFromSnapshot
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/641603006
2014-10-30 17:15:49 -04:00
Mark Mentovai
4ee00568b7 snapshot: Make destructors of interface classes public and virtual.
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/689483003
2014-10-29 17:31:23 -04:00
Mark Mentovai
279ca87e68 Add ProcessSnapshotMac.
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/650223007
2014-10-17 14:00:58 -04:00
Mark Mentovai
6584361343 Add ModuleSnapshotMac.
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/649633004
2014-10-17 13:56:31 -04:00
Mark Mentovai
b43f510a52 Add crashpad_info, MachOImageAnnotationsReader, and its test.
TEST=snapshot_test MachOImageAnnotationsReader.*
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/651283003
2014-10-17 13:47:02 -04:00
Mark Mentovai
bcae4d94d5 Create snapshot/mac and move some files from snapshot and util to there.
TEST=snapshot_test, util_test CheckedMachAddressRange.*
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/666483002
2014-10-17 13:41:45 -04:00