diff --git a/README.md b/README.md index 57e81695..914af349 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,7 @@ limitations under the License. code, building, testing, and contributing to the project. * [Crashpad interface documentation](https://crashpad.chromium.org/doxygen/) * [Crashpad tool man pages](doc/man.md) + * [Crashpad overview design](doc/overview_design.md) ## Source Code diff --git a/build/gyp_crashpad.py b/build/gyp_crashpad.py index fe9da840..dd5fa69c 100755 --- a/build/gyp_crashpad.py +++ b/build/gyp_crashpad.py @@ -75,11 +75,24 @@ def main(args): return result if sys.platform == 'win32': - # Also generate the x86 build. - result = gyp.main(args + ['-D', 'target_arch=ia32', '-G', 'config=Debug']) - if result != 0: - return result - result = gyp.main(args + ['-D', 'target_arch=ia32', '-G', 'config=Release']) + # Check to make sure that no target_arch was specified. target_arch may be + # set during a cross build, such as a cross build for Android. + has_target_arch = False + for arg_index in xrange(0, len(args)): + arg = args[arg_index] + if (arg.startswith('-Dtarget_arch=') or + (arg == '-D' and arg_index + 1 < len(args) and + args[arg_index + 1].startswith('target_arch='))): + has_target_arch = True + break + + if not has_target_arch: + # Also generate the x86 build. + result = gyp.main(args + ['-D', 'target_arch=ia32', '-G', 'config=Debug']) + if result != 0: + return result + result = gyp.main( + args + ['-D', 'target_arch=ia32', '-G', 'config=Release']) return result diff --git a/build/gyp_crashpad_android.py b/build/gyp_crashpad_android.py new file mode 100755 index 00000000..852839aa --- /dev/null +++ b/build/gyp_crashpad_android.py @@ -0,0 +1,79 @@ +#!/usr/bin/env python + +# Copyright 2017 The Crashpad Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import argparse +import glob +import gyp_crashpad +import os +import subprocess +import sys + + +def main(args): + parser = argparse.ArgumentParser( + description='Set up an Android cross build', + epilog='Additional arguments will be passed to gyp_crashpad.py.') + parser.add_argument('--ndk', required=True, help='Standalone NDK toolchain') + parser.add_argument('--compiler', + default='clang', + choices=('clang', 'gcc'), + help='The compiler to use, clang by default') + (parsed, extra_args) = parser.parse_known_args(args) + + NDK_ERROR=( + 'NDK must be a valid standalone NDK toolchain.\n' + + 'See https://developer.android.com/ndk/guides/standalone_toolchain.html') + arch_dirs = glob.glob(os.path.join(parsed.ndk, '*-linux-android*')) + if len(arch_dirs) != 1: + parser.error(NDK_ERROR) + + arch_triplet = os.path.basename(arch_dirs[0]) + ARCH_TRIPLET_TO_ARCH = { + 'arm-linux-androideabi': 'arm', + 'aarch64-linux-android': 'arm64', + 'i686-linux-android': 'x86', + 'x86_64-linux-android': 'x86_64', + 'mipsel-linux-android': 'mips', + 'mips64el-linux-android': 'mips64', + } + if arch_triplet not in ARCH_TRIPLET_TO_ARCH: + parser.error(NDK_ERROR) + arch = ARCH_TRIPLET_TO_ARCH[arch_triplet] + + ndk_bin_dir = os.path.join(parsed.ndk, 'bin') + + if parsed.compiler == 'clang': + os.environ['CC_target'] = os.path.join(ndk_bin_dir, 'clang') + os.environ['CXX_target'] = os.path.join(ndk_bin_dir, 'clang++') + elif parsed.compiler == 'gcc': + os.environ['CC_target'] = os.path.join(ndk_bin_dir, + '%s-gcc' % arch_triplet) + os.environ['CXX_target'] = os.path.join(ndk_bin_dir, + '%s-g++' % arch_triplet) + + for tool in ('ar', 'nm', 'readelf'): + os.environ['%s_target' % tool.upper()] = ( + os.path.join(ndk_bin_dir, '%s-%s' % (arch_triplet, tool))) + + return gyp_crashpad.main( + ['-D', 'OS=android', + '-D', 'target_arch=%s' % arch, + '-D', 'clang=%d' % (1 if parsed.compiler == 'clang' else 0), + '-f', 'ninja-android'] + extra_args) + + +if __name__ == '__main__': + sys.exit(main(sys.argv[1:])) diff --git a/doc/developing.md b/doc/developing.md index 9beec52c..65d62356 100644 --- a/doc/developing.md +++ b/doc/developing.md @@ -137,46 +137,35 @@ Note that Chrome uses Android API level 21 for 64-bit platforms and 16 for [`build/config/android/config.gni`](https://chromium.googlesource.com/chromium/src/+/master/build/config/android/config.gni) which sets `_android_api_level` and `_android64_api_level`. -To configure a Crashpad build for Android using this standalone toolchain, set -several environment variables directing the build to the standalone toolchain, -along with GYP options to identify an Android build. This must be done after any -`gclient sync`, or instead of any `gclient runhooks` operation. The environment -variables only need to be set for this `gyp_crashpad.py` invocation, and need -not be permanent. +To configure a Crashpad build for Android using the standalone toolchain +assembled above, use `gyp_crashpad_android.py`. This script is a wrapper for +`gyp_crashpad.py` that sets several environment variables directing the build to +the standalone toolchain, and several GYP options to identify an Android build. +This must be done after any `gclient sync`, or instead of any `gclient runhooks` +operation. ``` $ cd ~/crashpad/crashpad -$ CC_target=~/android-ndk-r14_arm64_api21/bin/clang \ - CXX_target=~/android-ndk-r14_arm64_api21/bin/clang++ \ - AR_target=~/android-ndk-r14_arm64_api21/bin/aarch64-linux-android-ar \ - NM_target=~/android-ndk-r14_arm64_api21/bin/aarch64-linux-android-nm \ - READELF_target=~/android-ndk-r14_arm64_api21/bin/aarch64-linux-android-readelf \ - python build/gyp_crashpad.py \ - -DOS=android -Dtarget_arch=arm64 -Dclang=1 \ - --generator-output=out/android_arm64_api21 -f ninja-android +$ python build/gyp_crashpad_android.py \ + --ndk ~/android-ndk-r14_arm64_api21 \ + --generator-output out/android_arm64_api21 ``` -It is also possible to use GCC instead of Clang by making the appropriate -substitutions: `aarch64-linux-android-gcc` for `CC_target`; -`aarch64-linux-android-g++` for `CXX_target`; and `-Dclang=0` as an argument to -`gyp_crashpad.py`. +`gyp_crashpad_android.py` detects the build type based on the characteristics of +the standalone toolchain given in its `--ndk` argument. -Target “triplets” to use for `ar`, `nm`, `readelf`, `gcc`, and `g++` are: +`gyp_crashpad_android.py` sets the build up to use Clang by default. It’s also +possible to use GCC by providing the `--compiler=gcc` argument to +`gyp_crashpad_android.py`. -| Architecture | Target “triplet” | -|:-------------|:------------------------| -| `arm` | `arm-linux-androideabi` | -| `arm64` | `aarch64-linux-android` | -| `x86` | `i686-linux-android` | -| `x86_64` | `x86_64-linux-android` | - -The port is incomplete, but targets known to be working include `crashpad_util`, -`crashpad_test`, and `crashpad_test_test`. This list will grow over time. To -build, direct `ninja` to the specific `out` directory chosen by -`--generator-output` above. +The Android port is incomplete, but targets known to be working include +`crashpad_test`, `crashpad_util`, and their tests. This list will grow over +time. To build, direct `ninja` to the specific `out` directory chosen by the +`--generator-output` argument to `gyp_crashpad_android.py`. ``` -$ ninja -C out/android_arm64_api21/out/Debug crashpad_test_test +$ ninja -C out/android_arm64_api21/out/Debug \ + crashpad_test_test crashpad_util_test ``` ## Testing diff --git a/doc/layering.png b/doc/layering.png new file mode 100644 index 00000000..791bea53 Binary files /dev/null and b/doc/layering.png differ diff --git a/doc/overview.png b/doc/overview.png new file mode 100644 index 00000000..78f849ef Binary files /dev/null and b/doc/overview.png differ diff --git a/doc/overview_design.md b/doc/overview_design.md new file mode 100644 index 00000000..ad871c18 --- /dev/null +++ b/doc/overview_design.md @@ -0,0 +1,505 @@ + + +# Crashpad Overview Design + +[TOC] + +## Objective + +Crashpad is a library for capturing, storing and transmitting postmortem crash +reports from a client to an upstream collection server. Crashpad aims to make it +possible for clients to capture process state at the time of crash with the best +possible fidelity and coverage, with the minimum of fuss. + +Crashpad also provides a facility for clients to capture dumps of process state +on-demand for diagnostic purposes. + +Crashpad additionally provides minimal facilities for clients to adorn their +crashes with application-specific metadata in the form of per-process key/value +pairs. More sophisticated clients are able to adorn crash reports further +through extensibility points that allow the embedder to augment the crash report +with application-specific metadata. + +## Background + +It’s an unfortunate truth that any large piece of software will contain bugs +that will cause it to occasionally crash. Even in the absence of bugs, software +incompatibilities can cause program instability. + +Fixing bugs and incompatibilities in client software that ships to millions of +users around the world is a daunting task. User reports and manual reproduction +of crashes can work, but even given a user report, often times the problem is +not readily reproducible. This is for various reasons, such as e.g. system +version or third-party software incompatibility, or the problem can happen due +to a race of some sort. Users are also unlikely to report problems they +encounter, and user reports are often of poor quality, as unfortunately most +users don’t have experience with making good bug reports. + +Automatic crash telemetry has been the best solution to the problem so far, as +this relieves the burden of manual reporting from users, while capturing the +hardware and software state at the time of crash. + +TODO(siggi): examples of this? + +Crash telemetry involves capturing postmortem crash dumps and transmitting them +to a backend collection server. On the server they can be stackwalked and +symbolized, and evaluated and aggregated in various ways. Stackwalking and +symbolizing the reports on an upstream server has several benefits over +performing these tasks on the client. High-fidelity stackwalking requires access +to bulky unwind data, and it may be desirable to not ship this to end users out +of concern for the application size. The process of symbolization requires +access to debugging symbols, which can be quite large, and the symbolization +process can consume considerable other resources. Transmitting un-stackwalked +and un-symbolized postmortem dumps to the collection server also allows deep +analysis of individual dumps, which is often necessary to resolve the bug +causing the crash. + +Transmitting reports to the collection server allows aggregating crashes by +cause, which in turn allows assessing the importance of different crashes in +terms of the occurrence rate and e.g. the potential security impact. + +A postmortem crash dump must contain the program state at the time of crash +with sufficient fidelity to allow diagnosing and fixing the problem. As the full +program state is usually too large to transmit to an upstream server, the +postmortem dump captures a heuristic subset of the full state. + +The crashed program is in an indeterminate state and, in fact, has often crashed +because of corrupt global state - such as heap. It’s therefore important to +generate crash reports with as little execution in the crashed process as +possible. Different operating systems vary in the facilities they provide for +this. + +## Overview + +Crashpad is a client-side library that focuses on capturing machine and program +state in a postmortem crash report, and transmitting this report to a backend +server - a “collection server”. The Crashpad library is embedded by the client +application. Conceptually, Crashpad breaks down into the handler and the client. +The handler runs in a separate process from the client or clients. It is +responsible for snapshotting the crashing client process’ state on a crash, +saving it to a crash dump, and transmitting the crash dump to an upstream +server. Clients register with the handler to allow it to capture and upload +their crashes. + +### The Crashpad handler + +The Crashpad handler is instantiated in a process supplied by the embedding +application. It provides means for clients to register themselves by some means +of IPC, or where operating system support is available, by taking advantage of +such support to cause crash notifications to be delivered to the handler. On +crash, the handler snapshots the crashed client process’ state, writes it to a +postmortem dump in a database, and may also transmit the dump to an upstream +server if so configured. + +The Crashpad handler is able to handle cross-bitted requests and generate crash +dumps across bitness, where e.g. the handler is a 64-bit process while the +client is a 32-bit process or vice versa. In the case of Windows, this is +limited by the OS such that a 32-bit handler can only generate crash dumps for +32-bit clients, but a 64-bit handler can acquire nearly all of the detail for a +32-bit process. + +### The Crashpad client + +The Crashpad client provides two main facilities. +1. Registration with the Crashpad handler. +2. Metadata communication to the Crashpad handler on crash. + +A Crashpad embedder links the Crashpad client library into one or more +executables, whether a loadable library or a program file. The client process +then registers with the Crashpad handler through some mode of IPC or other +operating system-specific support. + +On crash, metadata is communicated to the Crashpad handler via the CrashpadInfo +structure. Each client executable module linking the Crashpad client library +embeds a CrashpadInfo structure, which can be updated by the client with +whatever state the client wishes to record with a crash. + +![Overview image](overview.png) + +Here is an overview picture of the conceptual relationships between embedder (in +light blue), client modules (darker blue), and Crashpad (in green). Note that +multiple client modules can contain a CrashpadInfo structure, but only one +registration is necessary. + +## Detailed Design + +### Requirements + +The purpose of Crashpad is to capture machine, OS and application state in +sufficient detail and fidelity to allow developers to diagnose and, where +possible, fix the issue causing the crash. + +Each distinct crash report is assigned a globally unique ID, in order to allow +users to associate them with a user report, report in bug reports and so on. + +It’s critical to safeguard the user’s privacy by ensuring that no crash report +is ever uploaded without user consent. Likewise it’s important to ensure that +Crashpad never captures or uploads reports from non-client processes. + +### Concepts + +* **Client ID**. A UUID tied to a single instance of a Crashpad database. When + creating a crash report, the Crashpad handler includes the client ID stored + in the database. This provides a means to determine how many individual end + users are affected by a specific crash signature. + +* **Crash ID**. A UUID representing a single crash report. Uploaded crash + reports also receive a “server ID.” The Crashpad database indexes both the + locally-generated and server-generated IDs. + +* **Collection Server**. See [crash server documentation.]( + https://goto.google.com/crash-server-overview) + +* **Client Process**. Any process that has registered with a Crashpad handler. + +* **Handler process**. A process hosting the Crashpad handler library. This may + be a dedicated executable, or it may be hosted within a client executable + with control passed to it based on special signaling under the client’s + control, such as a command-line parameter. + +* **CrashpadInfo**. A structure used by client modules to provide information to + the handler. + +* **Annotations**. Each CrashpadInfo structure points to a dictionary of + {string, string} annotations that the client can use to communicate + application state in the case of crash. + +* **Database**. The Crashpad database contains persistent client settings as + well as crash dumps pending upload. + +TODO(siggi): moar concepts? + +### Overview Picture + +Here is a rough overview picture of the various Crashpad constructs, their +layering and intended use by clients. + +![Layering image](layering.png) + +Dark blue boxes are interfaces, light blue boxes are implementation. Gray is the +embedding client application. Note that wherever possible, implementation that +necessarily has to be OS-specific, exposes OS-agnostic interfaces to the rest of +Crashpad and the client. + +### Registration + +The particulars of how a client registers with the handler varies across +operating systems. + +#### macOS + +At registration time, the client designates a Mach port monitored by the +Crashpad handler as the EXC_CRASH exception port for the client. The port may be +acquired by launching a new handler process or by retrieving service already +registered with the system. The registration is maintained by the kernel and is +inherited by subprocesses at creation time by default, so only the topmost +process of a process tree need register. + +Crashpad provides a facility for a process to disassociate (unregister) with an +existing crash handler, which can be necessary when an older client spawns an +updated version. + +#### Windows + +There are two modes of registration on Windows. In both cases the handler is +advised of the address of a set of structures in the client process’ address +space. These structures include a pair of ExceptionInformation structs, one for +generating a postmortem dump for a crashing process, and another one for +generating a dump for a non- crashing process. + +##### Normal registration + +In the normal registration mode, the client connects to a named pipe by a +pre-arranged name. A registration request is written to the pipe. During +registration, the handler creates a set of events, duplicates them to the +registering client, then returns the handle values in the registration response. +This is a blocking process. + +##### Initial Handler Creation + +In order to avoid blocking client startup for the creation and initialization of +the handler, a different mode of registration can be used for the handler +creation. In this mode, the client creates a set of event handles and inherits +them into the newly created handler process. The handler process is advised of +the handle values and the location of the ExceptionInformation structures by way +of command line arguments in this mode. + +#### Linux/Android + +TODO(mmentovai): describe this. See this preliminary doc. + +### Capturing Exceptions + +The details of how Crashpad captures the exceptions leading to crashes varies +between operating systems. + +#### macOS + +On macOS, the operating system will notify the handler of client crashes via the +Mach port set as the client process’ exception port. As exceptions are +dispatched to the Mach port by the kernel, on macOS, exceptions can be handled +entirely from the Crashpad handler without the need to run any code in the crash +process at the time of the exception. + +#### Windows + +On Windows, the OS dispatches exceptions in the context of the crashing thread. +To notify the handler of exceptions, the Crashpad client registers an +UnhandledExceptionFilter (UEF) in the client process. When an exception trickles +up to the UEF, it stores the exception information and the crashing thread’s ID +in the ExceptionInformation structure registered with the handler. It then sets +an event handle to signal the handler to go ahead and process the exception. + +##### Caveats + +* If the crashing thread’s stack is smashed when an exception occurs, the + exception cannot be dispatched. In this case the OS will summarily terminate + the process, without the handler having an opportunity to generate a crash + report. +* If an exception is handled in the crashing thread, it will never propagate + to the UEF, and thus a crash report won’t be generated. This happens a fair + bit in Windows as system libraries will often dispatch callbacks under a + structured exception handler. This occurs during Window message dispatching + on some system configurations, as well as during e.g. DLL entry point + notifications. +* A growing number of conditions in the system and runtime exist where + detected corruption or illegal calls result in summary termination of the + process, in which case no crash report will be generated. + +###### Out-Of-Process Exception Handling + +There exists a mechanism in Windows Error Reporting (WER) that allows a client +process to register for handling client exceptions out of the crashing process. +Unfortunately this mechanism is difficult to use, and doesn’t provide coverage +for many of the caveats above. [Details +here.](https://crashpad.chromium.org/bug/133) + +#### Linux/Android + +TODO(mmentovai): describe this. See [this preliminary +doc.](https://goto.google.com/crashpad-android-dd) + +### The CrashpadInfo structure + +The CrashpadInfo structure is used to communicate information from the client to +the handler. Each executable module in a client process can contain a +CrashpadInfo structure. On a crash, the handler crawls all modules in the +crashing process to locate all CrashpadInfo structures present. The CrashpadInfo +structures are linked into a special, named section of the executable, where the +handler can readily find them. + +The CrashpadInfo structure has a magic signature, and contains a size and a +version field. The intent is to allow backwards compatibility from older client +modules to newer handler. It may also be necessary to provide forwards +compatibility from newer clients to older handler, though this hasn’t occurred +yet. + +The CrashpadInfo structure contains such properties as the cap for how much +memory to include in the crash dump, some tristate flags for controlling the +handler’s behavior, a pointer to an annotation dictionary and so on. + +### Snapshot + +Snapshot is a layer of interfaces that represent the machine and OS entities +that Crashpad cares about. Different concrete implementations of snapshot can +then be backed different ways, such as e.g. from the in-memory representation of +a crashed process, or e.g. from the contents of a minidump. + +### Crash Dump Creation + +To create a crash dump, a subset of the machine, OS and application state is +grabbed from the crashed process into an in-memory snapshot structure in the +handler process. Since the full application state is typically too large for +capturing to disk and transmitting to an upstream server, the snapshot contains +a heuristically selected subset of the full state. + +The precise details of what’s captured varies between operating systems, but +generally includes the following +* The set of modules (executable, shared libraries) that are loaded into the + crashing process. +* An enumeration of the threads running in the crashing process, including the + register contents and the contents of stack memory of each thread. +* A selection of the OS-related state of the process, such as e.g. the command + line, environment and so on. +* A selection of memory potentially referenced from registers and from stack. + +To capture a crash dump, the crashing process is first suspended, then a +snapshot is created in the handler process. The snapshot includes the +CrashpadInfo structures of the modules loaded into the process, and the contents +of those is used to control the level of detail captured for the crash dump. + +Once the snapshot has been constructed, it is then written to a minidump file, +which is added to the database. The process is un-suspended after the minidump +file has been written. In the case of a crash (as opposed to a client request to +produce a dump without crashing), it is then either killed by the operating +system or the Crashpad handler. + +In general the snapshotting process has to be very intimate with the operating +system it’s working with, so there will be a set of concrete implementation +classes, many deriving from the snapshot interfaces, doing this for each +operating system. + +### Minidump + +The minidump implementation is responsible for writing a snapshot to a +serialized on-disk file in the minidump format. The minidump implementation is +OS-agnostic, as it works on an OS-agnostic Snapshot interface. + +TODO(siggi): Talk about two-phase writes and contents ordering here. + +### Database + +The Crashpad database contains persistent client settings, including a unique +crash client identifier and the upload-enabled bit. Note that the crash client +identifier is assigned by Crashpad, and is distinct from any identifiers the +client application uses to identify users, installs, machines or such - if any. +The expectation is that the client application will manage the user’s upload +consent, and inform Crashpad of changes in consent. + +The unique client identifier is set at the time of database creation. It is then +recorded into every crash report collected by the handler and communicated to +the upstream server. + +The database stores a configurable number of recorded crash dumps to a +configurable maximum aggregate size. For each crash dump it stores annotations +relating to whether the crash dumps have been uploaded. For successfully +uploaded crash dumps it also stores their server-assigned ID. + +The database consists of a settings file, named "settings.dat" with binary +contents (see crashpad::Settings::Data for the file format), as well as +directory containing the crash dumps. Additionally each crash dump is adorned +with properties relating to the state of the dump for upload and such. The +details of how these properties are stored vary between platforms. + +#### macOS + +The macOS implementation simply stores database properties on the minidump files +in filesystem extended attributes. + +#### Windows + +The Windows implementation stores database properties in a binary file named +“metadata” at the top level of the database directory. + +### Report Format + +Crash reports are recorded in the Windows minidump format with +extensions to support Crashpad additions, such as e.g. Annotations. + +### Upload to collection server + +#### Wire Format + +For the time being, Crashpad uses the Breakpad wire protocol, which is +essentially a MIME multipart message communicated over HTTP(S). To support this, +the annotations from all the CrashpadInfo structures found in the crashing +process are merged to create the Breakpad “crash keys” as form data. The +postmortem minidump is then attached as an “application/octet- stream” +attachment with the name “upload_file_minidump”. The entirety of the request +body, including the minidump, can be gzip-compressed to reduce transmission time +and increase transmission reliability. Note that by convention there is a set of +“crash keys” that are used to communicate the product, version, client ID and +other relevant data about the client, to the server. Crashpad normally stores +these values in the minidump file itself, but retrieves them from the minidump +and supplies them as form data for compatibility with the Breakpad-style server. + +This is a temporary compatibility measure to allow the current Breakpad-based +upstream server to handle Crashpad reports. In the fullness of time, the wire +protocol is expected to change to remove this redundant transmission and +processing of the Annotations. + +#### Transport + +The embedding client controls the URL of the collection server by the command +line passed to the handler. The handler can upload crashes with HTTP or HTTPS, +depending on client’s preference. It’s strongly suggested use HTTPS transport +for crash uploads to protect the user’s privacy against man-in-the-middle +snoopers. + +TODO(mmentovai): Certificate pinning. + +#### Throttling & Retry Strategy + +To protect both the collection server from DDoS as well as to protect the +clients from unreasonable data transfer demands, the handler implements a +client-side throttling strategy. At the moment, the strategy is very simplistic, +it simply limits uploads to one upload per hour, and failed uploads are aborted. + +An experiment has been conducted to lift all throttling. Analysis on the +aggregate data this produced shows that multiple crashes within a short timespan +on the same client are nearly always due to the same cause. Therefore there is +very little loss of signal due to the throttling, though the ability to +reconstruct at least the full crash count is highly desirable. + +The lack of retry is expected to [change +soon](https://crashpad.chromium.org/bug/23), as this creates blind spots for +client crashes that exclusively occur on e.g. network down events, during +suspend and resume and such. + +### Extensibility + +Clients are able to extend the generated crash reports in two ways, by +manipulating their CrashpadInfo structure. +The two extensibility points are: +1. Nominating a set of address ranges for inclusion in the crash report. +2. Adding user-defined minidump streams for inclusion in the crash report. + +In both cases the CrashpadInfo structure has to be updated before a crash +occurs. + +### Dependencies + +Aside from system headers and APIs, when used outside of Chromium, Crashpad has +a dependency on “mini_chromium”, which is a subset of the Chromium base library. +This is to allow non-Chromium clients to use Crashpad, without taking a direct +dependency on the Chromium base, while allowing Chromium projects to use +Crashpad with minimum code duplication or hassle. When using Crashpad as part of +Chromium, Chromium’s own copy of the base library is used instead of +mini_chromium. + +The downside to this is that mini_chromium must be kept up to date with +interface and implementation changes in Chromium base, for the subset of +functionality used by Crashpad. + +## Caveats + +TODO(anyone): You may need to describe what you did not do or why simpler +approaches don't work. Mention other things to watch out for (if any). + +## Security Considerations + +Crashpad may be used to capture the state of sandboxed processes and it writes +minidumps to disk. It may therefore straddle security boundaries, so it’s +important that Crashpad handle all data it reads out of the crashed process with +extreme care. The Crashpad handler takes care to access client address spaces +through specially-designed accessors that check pointer validity and enforce +accesses within prescribed bounds. The flow of information into the Crashpad +handler is exclusively one-way: Crashpad never communicates anything back to +its clients, aside from providing single-bit indications of completion. + +## Privacy Considerations + +Crashpad may capture arbitrary contents from crashed process’ memory, including +user IDs and passwords, credit card information, URLs and whatever other content +users have trusted the crashing program with. The client program must acquire +and honor the user’s consent to upload crash reports, and appropriately manage +the upload state in Crashpad’s database. + +Crashpad must also be careful not to upload crashes for arbitrary processes on +the user’s system. To this end, Crashpad will never upload a process that hasn’t +registered with the handler, but note that registrations are inherited by child +processes on some operating systems. diff --git a/minidump/minidump.gyp b/minidump/minidump.gyp index 35528060..3135de3d 100644 --- a/minidump/minidump.gyp +++ b/minidump/minidump.gyp @@ -72,6 +72,8 @@ 'minidump_thread_writer.h', 'minidump_unloaded_module_writer.cc', 'minidump_unloaded_module_writer.h', + 'minidump_user_extension_stream_data_source.cc', + 'minidump_user_extension_stream_data_source.h', 'minidump_user_stream_writer.cc', 'minidump_user_stream_writer.h', 'minidump_writable.cc', diff --git a/minidump/minidump_file_writer.cc b/minidump/minidump_file_writer.cc index ebff2bab..3e99d2a3 100644 --- a/minidump/minidump_file_writer.cc +++ b/minidump/minidump_file_writer.cc @@ -29,6 +29,7 @@ #include "minidump/minidump_thread_id_map.h" #include "minidump/minidump_thread_writer.h" #include "minidump/minidump_unloaded_module_writer.h" +#include "minidump/minidump_user_extension_stream_data_source.h" #include "minidump/minidump_user_stream_writer.h" #include "minidump/minidump_writer_util.h" #include "snapshot/exception_snapshot.h" @@ -199,6 +200,19 @@ bool MinidumpFileWriter::AddStream( return true; } +bool MinidumpFileWriter::AddUserExtensionStream( + std::unique_ptr + user_extension_stream_data) { + DCHECK_EQ(state(), kStateMutable); + + auto user_stream = base::WrapUnique(new MinidumpUserStreamWriter()); + user_stream->InitializeFromBuffer(user_extension_stream_data->stream_type(), + user_extension_stream_data->buffer(), + user_extension_stream_data->buffer_size()); + + return AddStream(std::move(user_stream)); +} + bool MinidumpFileWriter::WriteEverything(FileWriterInterface* file_writer) { DCHECK_EQ(state(), kStateMutable); diff --git a/minidump/minidump_file_writer.h b/minidump/minidump_file_writer.h index 05cecb14..d5e2d1a5 100644 --- a/minidump/minidump_file_writer.h +++ b/minidump/minidump_file_writer.h @@ -33,6 +33,7 @@ namespace crashpad { class ProcessSnapshot; +class MinidumpUserExtensionStreamDataSource; //! \brief The root-level object in a minidump file. //! @@ -61,7 +62,11 @@ class MinidumpFileWriter final : public internal::MinidumpWritable { //! - kMinidumpStreamTypeThreadList //! - kMinidumpStreamTypeException (if present) //! - kMinidumpStreamTypeModuleList + //! - kMinidumpStreamTypeUnloadedModuleList (if present) //! - kMinidumpStreamTypeCrashpadInfo (if present) + //! - kMinidumpStreamTypeMemoryInfoList (if present) + //! - kMinidumpStreamTypeHandleData (if present) + //! - User streams (if present) //! - kMinidumpStreamTypeMemoryList //! //! \param[in] process_snapshot The process snapshot to use as source data. @@ -95,6 +100,30 @@ class MinidumpFileWriter final : public internal::MinidumpWritable { //! with a message logged. bool AddStream(std::unique_ptr stream); + //! \brief Adds a user extension stream to the minidump file and arranges for + //! a MINIDUMP_DIRECTORY entry to point to it. + //! + //! This object takes ownership of \a user_extension_stream_data. + //! + //! At most one object of each stream type (as obtained from + //! internal::MinidumpStreamWriter::StreamType()) may be added to a + //! MinidumpFileWriter object. If an attempt is made to add a stream whose + //! type matches an existing stream’s type, this method discards the new + //! stream. + //! + //! \param[in] user_extension_stream_data The stream data to add to the + //! minidump file. Note that the buffer this object points to must be valid + //! through WriteEverything(). + //! + //! \note Valid in #kStateMutable. + //! + //! \return `true` on success. `false` on failure, as occurs when an attempt + //! is made to add a stream whose type matches an existing stream’s type, + //! with a message logged. + bool AddUserExtensionStream( + std::unique_ptr + user_extension_stream_data); + // MinidumpWritable: //! \copydoc internal::MinidumpWritable::WriteEverything() diff --git a/minidump/minidump_file_writer_test.cc b/minidump/minidump_file_writer_test.cc index d8d0f394..0d300603 100644 --- a/minidump/minidump_file_writer_test.cc +++ b/minidump/minidump_file_writer_test.cc @@ -25,6 +25,7 @@ #include "base/memory/ptr_util.h" #include "gtest/gtest.h" #include "minidump/minidump_stream_writer.h" +#include "minidump/minidump_user_extension_stream_data_source.h" #include "minidump/minidump_writable.h" #include "minidump/test/minidump_file_writer_test_util.h" #include "minidump/test/minidump_writable_test_util.h" @@ -127,6 +128,50 @@ TEST(MinidumpFileWriter, OneStream) { EXPECT_EQ(0, memcmp(stream_data, expected_stream.c_str(), kStreamSize)); } +TEST(MinidumpFileWriter, AddUserExtensionStream) { + MinidumpFileWriter minidump_file; + const time_t kTimestamp = 0x155d2fb8; + minidump_file.SetTimestamp(kTimestamp); + + static const uint8_t kStreamData[] = "Hello World!"; + const size_t kStreamSize = arraysize(kStreamData); + const MinidumpStreamType kStreamType = static_cast(0x4d); + + auto stream = base::WrapUnique(new MinidumpUserExtensionStreamDataSource( + kStreamType, kStreamData, kStreamSize)); + ASSERT_TRUE(minidump_file.AddUserExtensionStream(std::move(stream))); + + // Adding the same stream type a second time should fail. + stream = base::WrapUnique(new MinidumpUserExtensionStreamDataSource( + kStreamType, kStreamData, kStreamSize)); + ASSERT_FALSE(minidump_file.AddUserExtensionStream(std::move(stream))); + + StringFile string_file; + ASSERT_TRUE(minidump_file.WriteEverything(&string_file)); + + const size_t kDirectoryOffset = sizeof(MINIDUMP_HEADER); + const size_t kStreamOffset = kDirectoryOffset + sizeof(MINIDUMP_DIRECTORY); + const size_t kFileSize = kStreamOffset + kStreamSize; + + ASSERT_EQ(kFileSize, string_file.string().size()); + + const MINIDUMP_DIRECTORY* directory; + const MINIDUMP_HEADER* header = + MinidumpHeaderAtStart(string_file.string(), &directory); + ASSERT_NO_FATAL_FAILURE(VerifyMinidumpHeader(header, 1, kTimestamp)); + ASSERT_TRUE(directory); + + EXPECT_EQ(kStreamType, directory[0].StreamType); + EXPECT_EQ(kStreamSize, directory[0].Location.DataSize); + EXPECT_EQ(kStreamOffset, directory[0].Location.Rva); + + const uint8_t* stream_data = MinidumpWritableAtLocationDescriptor( + string_file.string(), directory[0].Location); + ASSERT_TRUE(stream_data); + + EXPECT_EQ(0, memcmp(stream_data, kStreamData, kStreamSize)); +} + TEST(MinidumpFileWriter, ThreeStreams) { MinidumpFileWriter minidump_file; const time_t kTimestamp = 0x155d2fb8; diff --git a/minidump/minidump_misc_info_writer_test.cc b/minidump/minidump_misc_info_writer_test.cc index 8f2b5517..71fdbfbd 100644 --- a/minidump/minidump_misc_info_writer_test.cc +++ b/minidump/minidump_misc_info_writer_test.cc @@ -34,6 +34,7 @@ #include "snapshot/test/test_process_snapshot.h" #include "snapshot/test/test_system_snapshot.h" #include "util/file/string_file.h" +#include "util/misc/arraysize_unsafe.h" #include "util/stdlib/strlcpy.h" namespace crashpad { @@ -398,7 +399,7 @@ TEST(MinidumpMiscInfoWriter, TimeZone) { base::string16 standard_name_utf16 = base::UTF8ToUTF16(kStandardName); c16lcpy(expected.TimeZone.StandardName, standard_name_utf16.c_str(), - arraysize(expected.TimeZone.StandardName)); + ARRAYSIZE_UNSAFE(expected.TimeZone.StandardName)); memcpy(&expected.TimeZone.StandardDate, &kStandardDate, sizeof(expected.TimeZone.StandardDate)); @@ -406,7 +407,7 @@ TEST(MinidumpMiscInfoWriter, TimeZone) { base::string16 daylight_name_utf16 = base::UTF8ToUTF16(kDaylightName); c16lcpy(expected.TimeZone.DaylightName, daylight_name_utf16.c_str(), - arraysize(expected.TimeZone.DaylightName)); + ARRAYSIZE_UNSAFE(expected.TimeZone.DaylightName)); memcpy(&expected.TimeZone.DaylightDate, &kDaylightDate, sizeof(expected.TimeZone.DaylightDate)); @@ -426,9 +427,10 @@ TEST(MinidumpMiscInfoWriter, TimeZoneStringsOverflow) { const int32_t kBias = 300; MINIDUMP_MISC_INFO_N tmp; ALLOW_UNUSED_LOCAL(tmp); - std::string standard_name(arraysize(tmp.TimeZone.StandardName) + 1, 's'); + std::string standard_name(ARRAYSIZE_UNSAFE(tmp.TimeZone.StandardName) + 1, + 's'); const int32_t kStandardBias = 0; - std::string daylight_name(arraysize(tmp.TimeZone.DaylightName), 'd'); + std::string daylight_name(ARRAYSIZE_UNSAFE(tmp.TimeZone.DaylightName), 'd'); const int32_t kDaylightBias = -60; // Test using kSystemTimeZero, because not all platforms will be able to @@ -459,7 +461,7 @@ TEST(MinidumpMiscInfoWriter, TimeZoneStringsOverflow) { base::string16 standard_name_utf16 = base::UTF8ToUTF16(standard_name); c16lcpy(expected.TimeZone.StandardName, standard_name_utf16.c_str(), - arraysize(expected.TimeZone.StandardName)); + ARRAYSIZE_UNSAFE(expected.TimeZone.StandardName)); memcpy(&expected.TimeZone.StandardDate, &kSystemTimeZero, sizeof(expected.TimeZone.StandardDate)); @@ -467,7 +469,7 @@ TEST(MinidumpMiscInfoWriter, TimeZoneStringsOverflow) { base::string16 daylight_name_utf16 = base::UTF8ToUTF16(daylight_name); c16lcpy(expected.TimeZone.DaylightName, daylight_name_utf16.c_str(), - arraysize(expected.TimeZone.DaylightName)); + ARRAYSIZE_UNSAFE(expected.TimeZone.DaylightName)); memcpy(&expected.TimeZone.DaylightDate, &kSystemTimeZero, sizeof(expected.TimeZone.DaylightDate)); @@ -498,12 +500,12 @@ TEST(MinidumpMiscInfoWriter, BuildStrings) { base::string16 build_string_utf16 = base::UTF8ToUTF16(kBuildString); c16lcpy(expected.BuildString, build_string_utf16.c_str(), - arraysize(expected.BuildString)); + ARRAYSIZE_UNSAFE(expected.BuildString)); base::string16 debug_build_string_utf16 = base::UTF8ToUTF16(kDebugBuildString); c16lcpy(expected.DbgBldStr, debug_build_string_utf16.c_str(), - arraysize(expected.DbgBldStr)); + ARRAYSIZE_UNSAFE(expected.DbgBldStr)); ExpectMiscInfoEqual(&expected, observed); } @@ -517,8 +519,8 @@ TEST(MinidumpMiscInfoWriter, BuildStringsOverflow) { MINIDUMP_MISC_INFO_N tmp; ALLOW_UNUSED_LOCAL(tmp); - std::string build_string(arraysize(tmp.BuildString) + 1, 'B'); - std::string debug_build_string(arraysize(tmp.DbgBldStr), 'D'); + std::string build_string(ARRAYSIZE_UNSAFE(tmp.BuildString) + 1, 'B'); + std::string debug_build_string(ARRAYSIZE_UNSAFE(tmp.DbgBldStr), 'D'); misc_info_writer->SetBuildString(build_string, debug_build_string); @@ -535,12 +537,12 @@ TEST(MinidumpMiscInfoWriter, BuildStringsOverflow) { base::string16 build_string_utf16 = base::UTF8ToUTF16(build_string); c16lcpy(expected.BuildString, build_string_utf16.c_str(), - arraysize(expected.BuildString)); + ARRAYSIZE_UNSAFE(expected.BuildString)); base::string16 debug_build_string_utf16 = base::UTF8ToUTF16(debug_build_string); c16lcpy(expected.DbgBldStr, debug_build_string_utf16.c_str(), - arraysize(expected.DbgBldStr)); + ARRAYSIZE_UNSAFE(expected.DbgBldStr)); ExpectMiscInfoEqual(&expected, observed); } @@ -680,7 +682,7 @@ TEST(MinidumpMiscInfoWriter, Everything) { base::string16 standard_name_utf16 = base::UTF8ToUTF16(kStandardName); c16lcpy(expected.TimeZone.StandardName, standard_name_utf16.c_str(), - arraysize(expected.TimeZone.StandardName)); + ARRAYSIZE_UNSAFE(expected.TimeZone.StandardName)); memcpy(&expected.TimeZone.StandardDate, &kSystemTimeZero, sizeof(expected.TimeZone.StandardDate)); @@ -688,7 +690,7 @@ TEST(MinidumpMiscInfoWriter, Everything) { base::string16 daylight_name_utf16 = base::UTF8ToUTF16(kDaylightName); c16lcpy(expected.TimeZone.DaylightName, daylight_name_utf16.c_str(), - arraysize(expected.TimeZone.DaylightName)); + ARRAYSIZE_UNSAFE(expected.TimeZone.DaylightName)); memcpy(&expected.TimeZone.DaylightDate, &kSystemTimeZero, sizeof(expected.TimeZone.DaylightDate)); @@ -696,12 +698,12 @@ TEST(MinidumpMiscInfoWriter, Everything) { base::string16 build_string_utf16 = base::UTF8ToUTF16(kBuildString); c16lcpy(expected.BuildString, build_string_utf16.c_str(), - arraysize(expected.BuildString)); + ARRAYSIZE_UNSAFE(expected.BuildString)); base::string16 debug_build_string_utf16 = base::UTF8ToUTF16(kDebugBuildString); c16lcpy(expected.DbgBldStr, debug_build_string_utf16.c_str(), - arraysize(expected.DbgBldStr)); + ARRAYSIZE_UNSAFE(expected.DbgBldStr)); ExpectMiscInfoEqual(&expected, observed); } @@ -744,18 +746,18 @@ TEST(MinidumpMiscInfoWriter, InitializeFromSnapshot) { expect_misc_info.TimeZone.Bias = 300; c16lcpy(expect_misc_info.TimeZone.StandardName, standard_time_name_utf16.c_str(), - arraysize(expect_misc_info.TimeZone.StandardName)); + ARRAYSIZE_UNSAFE(expect_misc_info.TimeZone.StandardName)); expect_misc_info.TimeZone.StandardBias = 0; c16lcpy(expect_misc_info.TimeZone.DaylightName, daylight_time_name_utf16.c_str(), - arraysize(expect_misc_info.TimeZone.DaylightName)); + ARRAYSIZE_UNSAFE(expect_misc_info.TimeZone.DaylightName)); expect_misc_info.TimeZone.DaylightBias = -60; c16lcpy(expect_misc_info.BuildString, build_string_utf16.c_str(), - arraysize(expect_misc_info.BuildString)); + ARRAYSIZE_UNSAFE(expect_misc_info.BuildString)); c16lcpy(expect_misc_info.DbgBldStr, debug_build_string_utf16.c_str(), - arraysize(expect_misc_info.DbgBldStr)); + ARRAYSIZE_UNSAFE(expect_misc_info.DbgBldStr)); const timeval kStartTime = { static_cast(expect_misc_info.ProcessCreateTime), 0 }; diff --git a/minidump/minidump_module_writer.cc b/minidump/minidump_module_writer.cc index f74a2a70..8fdda897 100644 --- a/minidump/minidump_module_writer.cc +++ b/minidump/minidump_module_writer.cc @@ -14,6 +14,8 @@ #include "minidump/minidump_module_writer.h" +#include + #include #include diff --git a/minidump/minidump_module_writer_test.cc b/minidump/minidump_module_writer_test.cc index f5484744..6a0464cf 100644 --- a/minidump/minidump_module_writer_test.cc +++ b/minidump/minidump_module_writer_test.cc @@ -14,6 +14,7 @@ #include "minidump/minidump_module_writer.h" +#include #include #include @@ -653,7 +654,6 @@ void InitializeTestModuleSnapshotFromMinidumpModule( TEST(MinidumpModuleWriter, InitializeFromSnapshot) { MINIDUMP_MODULE expect_modules[3] = {}; const char* module_paths[arraysize(expect_modules)] = {}; - const char* module_names[arraysize(expect_modules)] = {}; const char* module_pdbs[arraysize(expect_modules)] = {}; UUID uuids[arraysize(expect_modules)] = {}; uint32_t ages[arraysize(expect_modules)] = {}; @@ -667,7 +667,6 @@ TEST(MinidumpModuleWriter, InitializeFromSnapshot) { expect_modules[0].VersionInfo.dwProductVersionLS = 0x00070008; expect_modules[0].VersionInfo.dwFileType = VFT_APP; module_paths[0] = "/usr/bin/true"; - module_names[0] = "true"; module_pdbs[0] = "true"; const uint8_t kUUIDBytes0[16] = {0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, @@ -684,7 +683,6 @@ TEST(MinidumpModuleWriter, InitializeFromSnapshot) { expect_modules[1].VersionInfo.dwProductVersionLS = 0x000f0000; expect_modules[1].VersionInfo.dwFileType = VFT_DLL; module_paths[1] = "/usr/lib/libSystem.B.dylib"; - module_names[1] = "libSystem.B.dylib"; module_pdbs[1] = "libSystem.B.dylib.pdb"; const uint8_t kUUIDBytes1[16] = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, @@ -701,7 +699,6 @@ TEST(MinidumpModuleWriter, InitializeFromSnapshot) { expect_modules[2].VersionInfo.dwProductVersionLS = 0xbbbbcccc; expect_modules[2].VersionInfo.dwFileType = VFT_UNKNOWN; module_paths[2] = "/usr/lib/dyld"; - module_names[2] = "dyld"; module_pdbs[2] = "/usr/lib/dyld.pdb"; const uint8_t kUUIDBytes2[16] = {0xff, 0xfe, 0xfd, 0xfc, 0xfb, 0xfa, 0xf9, 0xf8, diff --git a/minidump/minidump_user_extension_stream_data_source.cc b/minidump/minidump_user_extension_stream_data_source.cc new file mode 100644 index 00000000..8056011c --- /dev/null +++ b/minidump/minidump_user_extension_stream_data_source.cc @@ -0,0 +1,30 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "minidump/minidump_user_extension_stream_data_source.h" + +namespace crashpad { + +MinidumpUserExtensionStreamDataSource::MinidumpUserExtensionStreamDataSource( + uint32_t stream_type, + const void* buffer, + size_t buffer_size) + : stream_type_(static_cast(stream_type)), + buffer_(buffer), + buffer_size_(buffer_size) {} + +MinidumpUserExtensionStreamDataSource:: + ~MinidumpUserExtensionStreamDataSource() {} + +} // namespace crashpad diff --git a/minidump/minidump_user_extension_stream_data_source.h b/minidump/minidump_user_extension_stream_data_source.h new file mode 100644 index 00000000..1afb1661 --- /dev/null +++ b/minidump/minidump_user_extension_stream_data_source.h @@ -0,0 +1,55 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_MINIDUMP_MINIDUMP_USER_EXTENSION_STREAM_DATA_SOURCE_H_ +#define CRASHPAD_MINIDUMP_MINIDUMP_USER_EXTENSION_STREAM_DATA_SOURCE_H_ + +#include +#include + +#include "base/macros.h" + +#include "minidump/minidump_extensions.h" + +namespace crashpad { + +//! \brief Describes a user extension data stream in a minidump. +class MinidumpUserExtensionStreamDataSource { + public: + //! \brief Constructs a MinidumpUserExtensionStreamDataSource. + //! + //! \param[in] stream_type The type of the user extension stream. + //! \param[in] buffer Points to the data for this stream. \a buffer is not + //! owned, and must outlive the use of this object. + //! \param[in] buffer_size The length of data in \a buffer. + MinidumpUserExtensionStreamDataSource(uint32_t stream_type, + const void* buffer, + size_t buffer_size); + ~MinidumpUserExtensionStreamDataSource(); + + MinidumpStreamType stream_type() const { return stream_type_; } + const void* buffer() const { return buffer_; } + size_t buffer_size() const { return buffer_size_; } + + private: + MinidumpStreamType stream_type_; + const void* buffer_; // weak + size_t buffer_size_; + + DISALLOW_COPY_AND_ASSIGN(MinidumpUserExtensionStreamDataSource); +}; + +} // namespace crashpad + +#endif // CRASHPAD_MINIDUMP_MINIDUMP_USER_EXTENSION_STREAM_DATA_SOURCE_H_ diff --git a/minidump/minidump_user_stream_writer.cc b/minidump/minidump_user_stream_writer.cc index 7332cd01..6520f0f4 100644 --- a/minidump/minidump_user_stream_writer.cc +++ b/minidump/minidump_user_stream_writer.cc @@ -14,13 +14,67 @@ #include "minidump/minidump_user_stream_writer.h" +#include "base/memory/ptr_util.h" #include "util/file/file_writer.h" namespace crashpad { -MinidumpUserStreamWriter::MinidumpUserStreamWriter() - : stream_type_(0), reader_() { -} +class MinidumpUserStreamWriter::ContentsWriter { + public: + virtual ~ContentsWriter() {} + virtual bool WriteContents(FileWriterInterface* writer) = 0; + virtual size_t GetSize() const = 0; +}; + +class MinidumpUserStreamWriter::SnapshotContentsWriter final + : public MinidumpUserStreamWriter::ContentsWriter, + public MemorySnapshot::Delegate { + public: + explicit SnapshotContentsWriter(const MemorySnapshot* snapshot) + : snapshot_(snapshot), writer_(nullptr) {} + + bool WriteContents(FileWriterInterface* writer) override { + DCHECK(!writer_); + + writer_ = writer; + if (!snapshot_) + return true; + + return snapshot_->Read(this); + } + + size_t GetSize() const override { return snapshot_ ? snapshot_->Size() : 0; }; + + bool MemorySnapshotDelegateRead(void* data, size_t size) override { + return writer_->Write(data, size); + } + + private: + const MemorySnapshot* snapshot_; + FileWriterInterface* writer_; + + DISALLOW_COPY_AND_ASSIGN(SnapshotContentsWriter); +}; + +class MinidumpUserStreamWriter::BufferContentsWriter final + : public MinidumpUserStreamWriter::ContentsWriter { + public: + BufferContentsWriter(const void* buffer, size_t buffer_size) + : buffer_(buffer), buffer_size_(buffer_size) {} + + bool WriteContents(FileWriterInterface* writer) override { + return writer->Write(buffer_, buffer_size_); + } + size_t GetSize() const override { return buffer_size_; } + + private: + const void* buffer_; + size_t buffer_size_; + + DISALLOW_COPY_AND_ASSIGN(BufferContentsWriter); +}; + +MinidumpUserStreamWriter::MinidumpUserStreamWriter() : stream_type_() {} MinidumpUserStreamWriter::~MinidumpUserStreamWriter() { } @@ -28,10 +82,23 @@ MinidumpUserStreamWriter::~MinidumpUserStreamWriter() { void MinidumpUserStreamWriter::InitializeFromSnapshot( const UserMinidumpStream* stream) { DCHECK_EQ(state(), kStateMutable); + DCHECK(!contents_writer_.get()); - stream_type_ = stream->stream_type(); - if (stream->memory()) - stream->memory()->Read(&reader_); + stream_type_ = static_cast(stream->stream_type()); + contents_writer_ = + base::WrapUnique(new SnapshotContentsWriter(stream->memory())); +} + +void MinidumpUserStreamWriter::InitializeFromBuffer( + MinidumpStreamType stream_type, + const void* buffer, + size_t buffer_size) { + DCHECK_EQ(state(), kStateMutable); + DCHECK(!contents_writer_.get()); + + stream_type_ = stream_type; + contents_writer_ = + base::WrapUnique(new BufferContentsWriter(buffer, buffer_size)); } bool MinidumpUserStreamWriter::Freeze() { @@ -42,7 +109,8 @@ bool MinidumpUserStreamWriter::Freeze() { size_t MinidumpUserStreamWriter::SizeOfObject() { DCHECK_GE(state(), kStateFrozen); - return reader_.size(); + + return contents_writer_->GetSize(); } std::vector @@ -53,21 +121,12 @@ MinidumpUserStreamWriter::Children() { bool MinidumpUserStreamWriter::WriteObject(FileWriterInterface* file_writer) { DCHECK_EQ(state(), kStateWritable); - return file_writer->Write(reader_.data(), reader_.size()); + + return contents_writer_->WriteContents(file_writer); } MinidumpStreamType MinidumpUserStreamWriter::StreamType() const { return static_cast(stream_type_); } -MinidumpUserStreamWriter::MemoryReader::~MemoryReader() {} - -bool MinidumpUserStreamWriter::MemoryReader::MemorySnapshotDelegateRead( - void* data, - size_t size) { - data_.resize(size); - memcpy(&data_[0], data, size); - return true; -} - } // namespace crashpad diff --git a/minidump/minidump_user_stream_writer.h b/minidump/minidump_user_stream_writer.h index b31247a3..838ed0de 100644 --- a/minidump/minidump_user_stream_writer.h +++ b/minidump/minidump_user_stream_writer.h @@ -22,6 +22,7 @@ #include #include "base/macros.h" +#include "minidump/minidump_extensions.h" #include "minidump/minidump_stream_writer.h" #include "minidump/minidump_writable.h" #include "snapshot/module_snapshot.h" @@ -41,6 +42,18 @@ class MinidumpUserStreamWriter final : public internal::MinidumpStreamWriter { //! \note Valid in #kStateMutable. void InitializeFromSnapshot(const UserMinidumpStream* stream); + //! \brief Initializes a MINIDUMP_USER_STREAM based on \a stream_type, + //! \a buffer and \a buffer_size. + //! + //! \param[in] stream_type The type of the stream. + //! \param[in] buffer The data for the stream. + //! \param[in] buffer_size The length of \a buffer, and the resulting stream. + //! + //! \note Valid in #kStateMutable. + void InitializeFromBuffer(MinidumpStreamType stream_type, + const void* buffer, + size_t buffer_size); + protected: // MinidumpWritable: bool Freeze() override; @@ -52,22 +65,13 @@ class MinidumpUserStreamWriter final : public internal::MinidumpStreamWriter { MinidumpStreamType StreamType() const override; private: - class MemoryReader : public MemorySnapshot::Delegate { - public: - ~MemoryReader() override; - bool MemorySnapshotDelegateRead(void* data, size_t size) override; + class ContentsWriter; + class SnapshotContentsWriter; + class BufferContentsWriter; - const void* data() const { - return reinterpret_cast(data_.data()); - } - size_t size() const { return data_.size(); } + std::unique_ptr contents_writer_; - private: - std::vector data_; - }; - - uint32_t stream_type_; - MemoryReader reader_; + MinidumpStreamType stream_type_; DISALLOW_COPY_AND_ASSIGN(MinidumpUserStreamWriter); }; diff --git a/minidump/minidump_user_stream_writer_test.cc b/minidump/minidump_user_stream_writer_test.cc index e4e8cccb..3942e0e6 100644 --- a/minidump/minidump_user_stream_writer_test.cc +++ b/minidump/minidump_user_stream_writer_test.cc @@ -32,7 +32,8 @@ namespace { // The user stream is expected to be the only stream. void GetUserStream(const std::string& file_contents, MINIDUMP_LOCATION_DESCRIPTOR* user_stream_location, - uint32_t stream_type) { + uint32_t stream_type, + size_t stream_size) { const size_t kDirectoryOffset = sizeof(MINIDUMP_HEADER); const size_t kUserStreamOffset = kDirectoryOffset + sizeof(MINIDUMP_DIRECTORY); @@ -47,13 +48,16 @@ void GetUserStream(const std::string& file_contents, ASSERT_EQ(stream_type, directory[kDirectoryIndex].StreamType); EXPECT_EQ(kUserStreamOffset, directory[kDirectoryIndex].Location.Rva); + EXPECT_EQ(stream_size, directory[kDirectoryIndex].Location.DataSize); *user_stream_location = directory[kDirectoryIndex].Location; } -TEST(MinidumpUserStreamWriter, NoData) { +constexpr MinidumpStreamType kTestStreamId = + static_cast(0x123456); + +TEST(MinidumpUserStreamWriter, InitializeFromSnapshotNoData) { MinidumpFileWriter minidump_file_writer; auto user_stream_writer = base::WrapUnique(new MinidumpUserStreamWriter()); - const uint32_t kTestStreamId = 0x123456; auto stream = base::WrapUnique(new UserMinidumpStream(kTestStreamId, nullptr)); user_stream_writer->InitializeFromSnapshot(stream.get()); @@ -67,14 +71,29 @@ TEST(MinidumpUserStreamWriter, NoData) { MINIDUMP_LOCATION_DESCRIPTOR user_stream_location; ASSERT_NO_FATAL_FAILURE(GetUserStream( - string_file.string(), &user_stream_location, kTestStreamId)); - EXPECT_EQ(0u, user_stream_location.DataSize); + string_file.string(), &user_stream_location, kTestStreamId, 0u)); } -TEST(MinidumpUserStreamWriter, OneStream) { +TEST(MinidumpUserStreamWriter, InitializeFromBufferNoData) { + MinidumpFileWriter minidump_file_writer; + auto user_stream_writer = base::WrapUnique(new MinidumpUserStreamWriter()); + user_stream_writer->InitializeFromBuffer(kTestStreamId, nullptr, 0); + minidump_file_writer.AddStream(std::move(user_stream_writer)); + + StringFile string_file; + ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); + + ASSERT_EQ(sizeof(MINIDUMP_HEADER) + sizeof(MINIDUMP_DIRECTORY), + string_file.string().size()); + + MINIDUMP_LOCATION_DESCRIPTOR user_stream_location; + ASSERT_NO_FATAL_FAILURE(GetUserStream( + string_file.string(), &user_stream_location, kTestStreamId, 0u)); +} + +TEST(MinidumpUserStreamWriter, InitializeFromSnapshotOneStream) { MinidumpFileWriter minidump_file_writer; auto user_stream_writer = base::WrapUnique(new MinidumpUserStreamWriter()); - const uint32_t kTestStreamId = 0x123456; TestMemorySnapshot* test_data = new TestMemorySnapshot(); test_data->SetAddress(97865); @@ -92,10 +111,33 @@ TEST(MinidumpUserStreamWriter, OneStream) { ASSERT_EQ(sizeof(MINIDUMP_HEADER) + sizeof(MINIDUMP_DIRECTORY) + kStreamSize, string_file.string().size()); - MINIDUMP_LOCATION_DESCRIPTOR user_stream_location; + MINIDUMP_LOCATION_DESCRIPTOR user_stream_location = {}; ASSERT_NO_FATAL_FAILURE(GetUserStream( - string_file.string(), &user_stream_location, kTestStreamId)); - EXPECT_EQ(kStreamSize, user_stream_location.DataSize); + string_file.string(), &user_stream_location, kTestStreamId, kStreamSize)); + const std::string stream_data = string_file.string().substr( + user_stream_location.Rva, user_stream_location.DataSize); + EXPECT_EQ(std::string(kStreamSize, 'c'), stream_data); +} + +TEST(MinidumpUserStreamWriter, InitializeFromBufferOneStream) { + MinidumpFileWriter minidump_file_writer; + auto user_stream_writer = base::WrapUnique(new MinidumpUserStreamWriter()); + + const size_t kStreamSize = 128; + std::vector data(kStreamSize, 'c'); + user_stream_writer->InitializeFromBuffer( + kTestStreamId, &data[0], data.size()); + minidump_file_writer.AddStream(std::move(user_stream_writer)); + + StringFile string_file; + ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); + + ASSERT_EQ(sizeof(MINIDUMP_HEADER) + sizeof(MINIDUMP_DIRECTORY) + kStreamSize, + string_file.string().size()); + + MINIDUMP_LOCATION_DESCRIPTOR user_stream_location = {}; + ASSERT_NO_FATAL_FAILURE(GetUserStream( + string_file.string(), &user_stream_location, kTestStreamId, kStreamSize)); const std::string stream_data = string_file.string().substr( user_stream_location.Rva, user_stream_location.DataSize); EXPECT_EQ(std::string(kStreamSize, 'c'), stream_data); diff --git a/minidump/test/minidump_writable_test_util.cc b/minidump/test/minidump_writable_test_util.cc index 66c0591d..49ff1720 100644 --- a/minidump/test/minidump_writable_test_util.cc +++ b/minidump/test/minidump_writable_test_util.cc @@ -14,6 +14,8 @@ #include "minidump/test/minidump_writable_test_util.h" +#include + #include #include "base/strings/string16.h" diff --git a/snapshot/cpu_context.cc b/snapshot/cpu_context.cc index c3887a2c..5c964480 100644 --- a/snapshot/cpu_context.cc +++ b/snapshot/cpu_context.cc @@ -14,6 +14,7 @@ #include "snapshot/cpu_context.h" +#include #include #include "base/logging.h" diff --git a/snapshot/cpu_context_test.cc b/snapshot/cpu_context_test.cc index ffc18cfe..6b94871b 100644 --- a/snapshot/cpu_context_test.cc +++ b/snapshot/cpu_context_test.cc @@ -14,6 +14,7 @@ #include "snapshot/cpu_context.h" +#include #include #include diff --git a/snapshot/mac/cpu_context_mac.cc b/snapshot/mac/cpu_context_mac.cc index 8dca246c..c91e3318 100644 --- a/snapshot/mac/cpu_context_mac.cc +++ b/snapshot/mac/cpu_context_mac.cc @@ -14,6 +14,7 @@ #include "snapshot/mac/cpu_context_mac.h" +#include #include #include "base/logging.h" diff --git a/snapshot/mac/module_snapshot_mac.h b/snapshot/mac/module_snapshot_mac.h index 23904a5e..16ad7e16 100644 --- a/snapshot/mac/module_snapshot_mac.h +++ b/snapshot/mac/module_snapshot_mac.h @@ -37,7 +37,7 @@ struct UUID; namespace internal { //! \brief A ModuleSnapshot of a code module (binary image) loaded into a -//! running (or crashed) process on a Mac OS X system. +//! running (or crashed) process on a macOS system. class ModuleSnapshotMac final : public ModuleSnapshot { public: ModuleSnapshotMac(); diff --git a/snapshot/mac/process_types/custom.cc b/snapshot/mac/process_types/custom.cc index 33e3a7ec..c896ebf6 100644 --- a/snapshot/mac/process_types/custom.cc +++ b/snapshot/mac/process_types/custom.cc @@ -14,6 +14,7 @@ #include "snapshot/mac/process_types.h" +#include #include #include diff --git a/snapshot/mac/system_snapshot_mac.cc b/snapshot/mac/system_snapshot_mac.cc index b65a9e08..135fe98e 100644 --- a/snapshot/mac/system_snapshot_mac.cc +++ b/snapshot/mac/system_snapshot_mac.cc @@ -14,6 +14,7 @@ #include "snapshot/mac/system_snapshot_mac.h" +#include #include #include #include diff --git a/snapshot/mac/system_snapshot_mac.h b/snapshot/mac/system_snapshot_mac.h index 0f18a48d..2ac2ef90 100644 --- a/snapshot/mac/system_snapshot_mac.h +++ b/snapshot/mac/system_snapshot_mac.h @@ -29,8 +29,7 @@ class ProcessReader; namespace internal { -//! \brief A SystemSnapshot of the running system, when the system runs Mac OS -//! X. +//! \brief A SystemSnapshot of the running system, when the system runs macOS. class SystemSnapshotMac final : public SystemSnapshot { public: SystemSnapshotMac(); diff --git a/snapshot/win/cpu_context_win.cc b/snapshot/win/cpu_context_win.cc index cc66e1cb..1a76c6d9 100644 --- a/snapshot/win/cpu_context_win.cc +++ b/snapshot/win/cpu_context_win.cc @@ -14,6 +14,7 @@ #include "snapshot/win/cpu_context_win.h" +#include #include #include diff --git a/snapshot/win/pe_image_reader.cc b/snapshot/win/pe_image_reader.cc index f7e8e51d..342021a6 100644 --- a/snapshot/win/pe_image_reader.cc +++ b/snapshot/win/pe_image_reader.cc @@ -14,6 +14,7 @@ #include "snapshot/win/pe_image_reader.h" +#include #include #include diff --git a/snapshot/win/process_snapshot_win.cc b/snapshot/win/process_snapshot_win.cc index 11e83145..7a18dbef 100644 --- a/snapshot/win/process_snapshot_win.cc +++ b/snapshot/win/process_snapshot_win.cc @@ -14,6 +14,8 @@ #include "snapshot/win/process_snapshot_win.h" +#include + #include #include "base/logging.h" diff --git a/third_party/gtest/gmock.gyp b/third_party/gtest/gmock.gyp index 466de6a9..a6600708 100644 --- a/third_party/gtest/gmock.gyp +++ b/third_party/gtest/gmock.gyp @@ -104,7 +104,7 @@ ], }, }], - ['OS=="linux"', { + ['OS=="linux" or OS=="android"', { 'cflags': [ '-Wno-inconsistent-missing-override', '-Wno-unknown-warning-option', diff --git a/util/file/file_writer.cc b/util/file/file_writer.cc index a8d92566..13ac6cf5 100644 --- a/util/file/file_writer.cc +++ b/util/file/file_writer.cc @@ -15,6 +15,7 @@ #include "util/file/file_writer.h" #include +#include #include #include diff --git a/util/mach/notify_server_test.cc b/util/mach/notify_server_test.cc index 445e35bf..b0ef4a35 100644 --- a/util/mach/notify_server_test.cc +++ b/util/mach/notify_server_test.cc @@ -14,6 +14,8 @@ #include "util/mach/notify_server.h" +#include + #include "base/compiler_specific.h" #include "base/mac/scoped_mach_port.h" #include "gmock/gmock.h" diff --git a/util/misc/uuid.cc b/util/misc/uuid.cc index d4345a8a..ac2468c6 100644 --- a/util/misc/uuid.cc +++ b/util/misc/uuid.cc @@ -19,6 +19,7 @@ #include "util/misc/uuid.h" #include +#include #include #include diff --git a/util/win/process_info.cc b/util/win/process_info.cc index 2e285b8b..cb2051c0 100644 --- a/util/win/process_info.cc +++ b/util/win/process_info.cc @@ -14,6 +14,7 @@ #include "util/win/process_info.h" +#include #include #include diff --git a/util/win/registration_protocol_win.cc b/util/win/registration_protocol_win.cc index e5cbd8d0..87c45748 100644 --- a/util/win/registration_protocol_win.cc +++ b/util/win/registration_protocol_win.cc @@ -14,6 +14,7 @@ #include "util/win/registration_protocol_win.h" +#include #include #include "base/logging.h" diff --git a/util/win/scoped_process_suspend.cc b/util/win/scoped_process_suspend.cc index fc75ce02..f07acc3c 100644 --- a/util/win/scoped_process_suspend.cc +++ b/util/win/scoped_process_suspend.cc @@ -14,6 +14,7 @@ #include "util/win/scoped_process_suspend.h" +#include #include #include "util/win/nt_internals.h" diff --git a/util/win/scoped_process_suspend_test.cc b/util/win/scoped_process_suspend_test.cc index adddfe27..f770456e 100644 --- a/util/win/scoped_process_suspend_test.cc +++ b/util/win/scoped_process_suspend_test.cc @@ -14,6 +14,7 @@ #include "util/win/scoped_process_suspend.h" +#include #include #include