182 Commits

Author SHA1 Message Date
Nico Weber
2fd16e3392 Reformat all gn files
Port of chromium-side
https://chromium-review.googlesource.com/c/chromium/src/+/1997899

Bug: chromium:1041419
Change-Id: Ic7afefa0dea024da37fe4bb0f965840a160e2166
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2015428
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2020-01-23 13:59:00 +00:00
Tim Zheng
726ab2a655 Integrate Crashpad with Chrome OS
This CL adds modification to Crashpad to integrate Crashpad reporting
for Chrome on Chrome OS.

Design doc: go/cros-crashpad
BUG=chromium:944123

Change-Id: I22e2f2a93f32c2dc149c9c011fa8134cf6d5b74f
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1707369
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2019-08-19 23:28:00 +00:00
Casey Dahlin
5f77cf41b6 Add CodeViewRecordBuildId
Until now we've been stuffing ELF debug symbol link information into a
CodeViewPDB70. This has reached the limits of its usefulness. We now add
a CodeViewRecord that can contain a proper ELF build ID.

Change-Id: Ice52cb2a958a1b9031943f280d9054da02d2f17d
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1574107
Commit-Queue: Casey Dahlin <sadmac@google.com>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-04-22 23:16:22 +00:00
Mark Mentovai
03850fb6cf Remove the last two extraneous semicolons
Patch by Nico Weber <thakis@chromium.org>, originally
https://crrev.com/c/1476986.

Bug: chromium:926235
Change-Id: I6fd9aed5c57ffe0fec8f9adcd4cf5211d7b795c3
Reviewed-on: https://chromium-review.googlesource.com/c/1475992
Commit-Queue: Mark Mentovai <mark@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
2019-02-19 15:59:10 +00:00
Mark Mentovai
ff5a25e11f Remove a few unnecessary semicolons.
Patch by Nico Weber <thakis@chromium.org>, originally
https://crrev.com/c/1463405.

Bug: chromium:926235
Change-Id: I7e0ba822aa8dd104768d7ad6e603539576ae96a9
Reviewed-on: https://chromium-review.googlesource.com/c/1463744
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
2019-02-11 16:21:20 +00:00
Mark Mentovai
cc166d71f4 Use base::size where appropriate, and ArraySize elsewhere
This is a follow-up to c8a016b99d97, following the post-landing
discussion at
https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1393921/5#message-2058541d8c4505d20a990ab7734cd758e437a5f7

base::size, and std::size that will eventually replace it when C++17 is
assured, does not allow the size of non-static data members to be taken
in constant expression context. The remaining uses of ArraySize are in:

minidump/minidump_exception_writer.cc (×1)
minidump/minidump_system_info_writer.cc (×2, also uses base::size)
snapshot/cpu_context.cc (×4, also uses base::size)
util/misc/arraysize_test.cc (×10, of course)

The first of these occurs when initializing a constexpr variable. All
others are in expressions used with static_assert.

Includes:
Update mini_chromium to 737433ebade4d446643c6c07daae02a67e8deccao

f701716d9546 Add Windows ARM64 build target to mini_chromium
87a95a3d6ac2 Remove the arraysize macro
1f7255ead1f7 Placate MSVC in areas of base::size usage
737433ebade4 Add cast

Bug: chromium:837308
Change-Id: I6a5162654461b1bdd9b7b6864d0d71a734bcde19
Reviewed-on: https://chromium-review.googlesource.com/c/1396108
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-01-04 22:42:57 +00:00
Avi Drissman
c8a016b99d Remove base's arraysize from Crashpad.
BUG=837308
R=mark@chromium.org

Change-Id: Ibecbfc7bc2d61ee54bc1114e4b20978adbc77db2
Reviewed-on: https://chromium-review.googlesource.com/c/1393921
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
2019-01-03 19:44:15 +00:00
Vlad Tsyrklevich
f42a579b8c Add ProcessSnapshotMinidump::CustomMinidumpStreams()
Add a method to ProcessSnapshotMinidump to expose a similar interface
to ModuleSnapshot::CustomMinidumpStreams(). It's implemented on the
process snapshot here because there is no way to map custom minidump
streams back to a specific module. This allows implementing tests that
inspect custom user streams in minidumps.

Bug: 896019

Change-Id: I1673c342753e13d64bddcc0083ca29fa356deac7
Reviewed-on: https://chromium-review.googlesource.com/c/1271405
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2018-11-17 05:01:50 +00:00
Francois Rousseau
a4754a9ae9 [snapshot][arm64] rename CPU context pstate to spsr
* upon exception, the process state ("pstate") is stored in the saved
process status register ("spsr") so the register we are manipulating is
really just the SPSR
* https://developer.arm.com/products/architecture/cpu-architecture/a-profile/docs/100878/latest/the-saved-process-status-register

Change-Id: I9ce612c00b7a56a0f6d778d974ff9e0e5402ca5e
Reviewed-on: https://chromium-review.googlesource.com/c/1312193
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Francois Rousseau <frousseau@google.com>
2018-11-01 18:01:40 +00:00
Scott Graham
2dee96b841 Remove use of MSVC_SUPPRESS_WARNING
Upstreaming of https://chromium-review.googlesource.com/c/chromium/src/+/1290574.

Change-Id: I4e37879f34b6db7ee93b04b999bbae1ac4d645ae
Reviewed-on: https://chromium-review.googlesource.com/c/1291054
Reviewed-by: Scott Graham <scottmg@chromium.org>
2018-10-19 23:50:09 +00:00
Francois Rousseau
0ad7e1fbbe [fuchsia] disable MinidumpStringWriter.ConvertInvalidUTF8ToUTF16
re-enable once LUCI supports invalid UTF8 characters in test logs.

Bug: fuchsia:DX-487
Bug: chromium:872892
Bug: chromium:889582
Tested: /system/test/crashpad_tests on Fuchsia device
Change-Id: I3d6564423fb20554fdc39ffb7bd8e8bf7b1d3d48
Reviewed-on: https://chromium-review.googlesource.com/1251382
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2018-09-28 22:14:34 +00:00
Joshua Peraza
9cd2bae5ab Use Microsoft's ARM64 context layout
Change-Id: Ic92447e99474f9b24197375acfc324cca4899222
Reviewed-on: https://chromium-review.googlesource.com/1157286
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
2018-08-01 17:45:48 +00:00
Djordje Golubovic
cb41ba7471 Added MIPS support to crashpad.
Modified gyp/gn files to support MIPS targets.

Implemented thread_info, cpu_context, signal context classes for MIPS target.

Addressed MIPS specific signal ordering.

Added "MIPS Technologies, Inc." to AUTHORS file.

Bug: crashpad:232
Change-Id: Ibfc221ba54088e95f984b9dc6be5fd52f86abcc2
Reviewed-on: https://chromium-review.googlesource.com/1064594
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2018-07-10 15:29:34 +00:00
Scott Graham
7c6d2334a9 Don't fail minidump write if memory snapshot read fails
On Windows (and probably elsewhere) it's possible that something else on
the system changes the memory map between when a memory snapshot range
is added to the minidump, and when the process's memory is actually read
from the target and written to the .dmp file. As a result, failing the
Read() should not result in aborting the minidump's write, which it
previously would have.

Bug: crashpad:234
Change-Id: Ib24e255a34fa2e1758621d3955ebc7a0f96166e2
Reviewed-on: https://chromium-review.googlesource.com/1096452
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
2018-06-12 17:46:22 +00:00
Scott Graham
f5f0aa4a8e fuchsia: Implementation of system snapshot
Mostly sensible implementation for x64 via cpuid. It's too early for
Fuchsia to have a version number, so nothing is reported for those
fields. ARM64 isn't implemented at all and would hit a lot of
NOTREACHED()s.

Bug: crashpad:196
Change-Id: I6ca8b12e16fe0cf773a17c88ca9d407b028a501c
Reviewed-on: https://chromium-review.googlesource.com/1005906
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2018-04-10 22:40:45 +00:00
Joshua Peraza
7d5487fc44 minidump: add switch cases to handle linux/android and ARM
Bug: crashpad:30
Change-Id: Ib2ff936451a68415f39f6a19f561d2f536daf8d3
Reviewed-on: https://chromium-review.googlesource.com/920786
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
2018-02-15 01:58:38 +00:00
Scott Graham
a8ecdbc973 Updates to support -Wimplicit-fallthrough
https://chromium-review.googlesource.com/c/chromium/mini_chromium/+/899847
turns the warning on. This adds one annotation, and fixes one bug.

Includes mini_chromium roll:

.../mini_chromium$ git log 5fcfa43c1587b94132e24782579350cb8266b990..3b953302848580cdf23b50402befc0ae09d03ff9 --oneline
3b95330 (HEAD, origin/master, origin/HEAD) Add -Wimplicit-fallthrough when building on clang

Bug: chromium:807632
Change-Id: I2f3ddca0228e52013844cb8d78d10cb359e851d0
Reviewed-on: https://chromium-review.googlesource.com/900317
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2018-02-03 03:05:45 +00:00
Scott Graham
9b6c69cbb5 Coalesce memory ranges
Follows https://chromium-review.googlesource.com/c/374019/.

Causes MinidumpMemoryListWriter to merge all overlapping ranges before
writing the MINIDUMP_MEMORY_LIST. This is:

1) Necessary for the Google internal crash processor, which in some
   cases attempts to read the raw memory (displaying ASAN red zones),
   and aborts if there are any overlapping ranges in the minidump on
   load;

2) Necessary for new-ish versions of windbg (see bug 216 below). It is
   believed that this is a change in behavior in the tool that made
   dumps with overlapping ranges unreadable;

3) More efficient. The .dmp for crashy_program goes from 306K to 140K
   with this enabled. In Chrome minidumps where
   set_gather_indirectly_referenced_memory() is used (in practice this
   means Chrome Windows Beta, Dev, and Canary), the savings are expected
   to be substantial.

Bug: crashpad:61, chromium:638370, crashpad:216

Change-Id: I969e1a52da555ceba59a727d933bfeef6787c7a5
Reviewed-on: https://chromium-review.googlesource.com/374539
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2018-02-02 00:04:20 +00:00
Joshua Peraza
c9244d58df Add ARM family minidump support
Bug: crashpad:30
Change-Id: I6784d42ba6c525c4e0b16dfdbbb4949c83e32fea
Reviewed-on: https://chromium-review.googlesource.com/888541
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2018-01-30 20:47:28 +00:00
Scott Graham
9fa60710f9 fuchsia: Don't use {0} for initialization
Fuchsia builds with -Wmissing-field-initializers. Remove these {0}s. It
all seems a bit awful, but as far as I can tell from reading
http://en.cppreference.com/w/cpp/language/aggregate_initialization, the
0 is unnecessary.

../../third_party/crashpad/minidump/minidump_system_info_writer_test.cc:42:27: error: missing field 'Buffer' initializer [-Werror,-Wmissing-field-initializers]
  MINIDUMP_STRING tmp = {0};
                            ^
			    1 error generated.


Bug: crashpad:196
Change-Id: I21f48eb24238a607475b0e92ffe5fd88386b40b6
Reviewed-on: https://chromium-review.googlesource.com/833454
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2018-01-10 22:26:59 +00:00
Scott Graham
ab153f7e1b gn: Avoid depending on BUILDCONFIG.gn globals
Goes with https://chromium-review.googlesource.com/c/chromium/mini_chromium/+/834648.

Includes mini_chromium DEPS roll to pull in edfe51ce81

Bug: crashpad:79, crashpad:196
Change-Id: Ib45cc738aecf9ae727f8faeff81f3b71e2dc9de8
Reviewed-on: https://chromium-review.googlesource.com/834543
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-12-19 23:38:36 +00:00
Scott Graham
457cc6a34f gn: Refactor build files to avoid build/secondary
In doing standalone bringup of Crashpad targeting Fuchsia, it seemed
tidy to keep the same literal paths to the dependencies that Chromium
needed and add stubs/forwarding to build/secondary in the Crashpad tree
as required to make those work.

However, when trying to build Crashpad in the Fuchsia tree itself, that
would require adding forwarding files to the Fuchsia tree to match the
Chromium directory structure, which would be awkward. Instead, have
explicit dependencies in the Crashpad tree that select the locations
for various dependencies.

Bug: crashpad:79, crashpad:196
Change-Id: Ib506839f9c97d8ef823663cdc733cbdcfa126139
Reviewed-on: https://chromium-review.googlesource.com/826025
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-12-18 22:53:58 +00:00
Hans Wennborg
914b0d6755 Fix enum vs unsigned -Wsign-compare warnings
Recent Clang versions started taking into account that enums are signed
on Windows when emitting these warnings.

Bug: chromium:792519
Change-Id: I08767fa1f5c8211e663769c7e76b13a1b7146f4f
Reviewed-on: https://chromium-review.googlesource.com/813497
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-12-07 04:44:47 +00:00
Scott Graham
2bb56fafe3 Rework GN files to start to support building standalone, and also in Chromium
- Adds a .gn and a build/BUILDCONFIG.gn that uses mini_chromium's
  build/BUILD.gn.
- Adds some stub BUILD.gn files in locations where Chromium expects them
  (in //build, //testing, //third_party) containing empty targets/configs.
  These are no-ops in standalone builds, but add functionality when
  building in Chromium.  This is in preference to having a global bool
  that conditionally does Chromium-y things in the Crashpad build files.
  These stub files are all contained in a secondary source root in
  build/chromium_compatibility, referred to by //.gn.
- Adds //base/BUILD.gn which forwards to mini_chromium/base. This is
  only used when building standalone so that both Chromium and Crashpad
  can refer to it as "//base".
- Changes references to other Crashpad targets to be relatively
  specified so that they work when the root of the project is //, and also
  when it's //third_party/crashpad/crashpad as it is in Chromium.
- Moves any error-causing Mac/Win-specific files into explicit if (is_mac)
  or if (is_win) blocks as part of removing the dependency on
  set_sources_assignment_filter().

As yet unresolved:
- CRASHPAD_IN_CHROMIUM needs to be removed when standalone; to be tackled
  in a follow up.
- Not sure what to do with zlib yet, the build file currently assumes
  "in Chromium" too, and similarly having Crashpad //third_party/zlib:zlib
  pointing at itself doesn't work.

Bug: crashpad:79
Change-Id: I6a7dda214e4b3b14a60c1ed285267ab97432a1a8
Reviewed-on: https://chromium-review.googlesource.com/777410
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
2017-11-28 20:29:35 +00:00
Scott Graham
ee84ce22bb fuchsia: Set kOS in MinidumpMiscInfoDebugBuildString
Bug: crashpad:196
Change-Id: I80c979967d95383e0f703a336a494f30ff583f1b
Reviewed-on: https://chromium-review.googlesource.com/786448
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-11-27 21:15:44 +00:00
Mark Mentovai
e9f40ae176 Remove double double words
I ran the thing below (piped to “grep -v namespace”), fixed things up,
and rewrapped comments in the affected file.

import re
import sys

LAST_WORD_RE = re.compile('^.*[\s]+([\w]+)$')
FIRST_WORD_RE = re.compile('^[^\w]+([\w]+).*$')

for path in sys.argv[1:]:
  with open(path) as file:
    line_number = 0
    last_word = None
    for line in file:
      line_number += 1
      first_word = FIRST_WORD_RE.match(line)
      if first_word and first_word.group(1) == last_word:
        print('%s:%u: %s' % (path, line_number - 1, last_word))
      last_word = LAST_WORD_RE.match(line)
      if last_word:
        last_word = last_word.group(1)

Change-Id: Iea9f2a6453d9d9ec17e2f238e09252535d7408bd
Reviewed-on: https://chromium-review.googlesource.com/780284
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-11-20 23:38:48 +00:00
Mark Mentovai
20e5aba1af URL cleanups: switch to HTTPS, fix dead ones, use canonical ones
Change-Id: I4b247d7fae1a212350f8ffcf2bf5ba1fa730f5c1
Reviewed-on: https://chromium-review.googlesource.com/780339
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-11-20 22:23:39 +00:00
Mark Mentovai
94a5a72efa mac: Tests that crash intentionally shouldn’t go to ReportCrash
Crashpad has many tests that crash intentionally. Some of these are
gtest death tests, and others arrange for intentional crashes to test
Crashpad’s own crash-catching logic. On macOS, all of the gtest death
tests and some of the other intentional crashes were being logged by
ReportCrash, the system’s crash reporter. Since these reports
corresponded to intentional crashes, they were never useful, and served
only to clutter ~/Library/Logs/DiagnosticReports.

Since Crashpad is adept at handling exceptions on its own, this
introduces the “exception swallowing server”,
crashpad_exception_swallower, which is a Mach exception server that
implements a no-op exception handler routine for all exceptions
received. The exception swallowing server is established as the task
handler for EXC_CRASH and EXC_CORPSE_NOTIFY exceptions during gtest
death tests invoked by {ASSERT,EXPECT}_DEATH_{CHECK,CRASH}, and for all
child processes invoked by the Multiprocess test infrastructure. The
exception swallowing server is not in effect at other times, so
unexpected crashes in test code can still be handled by ReportCrash or
another crash reporter.

With this change in place, no new reports are generated in the
user-level ~/Library/Logs/DiagnosticReports or the system’s
/Library/Logs/DiagnosticReports during a run of Crashpad’s full test
suite on macOS.

Bug: crashpad:33
Change-Id: I13891853a7e25accc30da21fa7ea8bd7d1f3bd2f
Reviewed-on: https://chromium-review.googlesource.com/777859
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-11-20 18:58:34 +00:00
Scott Graham
d5ead4d70f Upstream lightly modified Chromium BUILD.gn files
Unreferenced, and not working at all in Crashpad-standalone.

Copied from Chromium at 52a9831d81f2099ef9f50fcdaca5853019262c35 to have
a point where a roll back into Chromium should be a no-op (with Chromium's
build/secondary/third_party/crashpad/... removed).

I'm not sure what we want to do about the various gni references into
Chromium (e.g. //build/config/sanitizers/sanitizers.gni, //testing/test.gni,
etc.) but I guess the sooner they live in Crashpad rather than in Chromium
the sooner we can figure out the sort of knobs and dials we need.

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

Change-Id: I50341bfe81ee4799b3f6278d2e31ec31741952ac
Reviewed-on: https://chromium-review.googlesource.com/755654
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-11-07 00:25:09 +00:00
Robert Sesek
4d7d4dd56c Flip gtest _EQ parameter order in minidump_annotation_writer_test.cc
Bug: crashpad:192
Change-Id: I82dddda5ba3d4fe5bf843572e6a793131cb6fa40
Reviewed-on: https://chromium-review.googlesource.com/751441
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
2017-11-02 16:17:06 +00:00
Robert Sesek
e38c57a9c6 Expose the annotation_objects on MinidumpModuleCrashpadInfo.
This writes any set annotation objects list for a module into a
minidump, though no crash handler currently sets annotation objects for
a crashing process.

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

Bug: crashpad:192
Change-Id: Ic51536157177921640ca15ae14e5e01ca875ae12
Reviewed-on: https://chromium-review.googlesource.com/731309
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-11-01 21:04:47 +00:00
Mark Mentovai
af594c8deb Heap-allocate MinidumpContextAMD64Writer objects with proper alignment
While making crashpad_minidump_test run in Chromium’s try- and buildbots
(https://crbug.com/779790), crashes in the
MinidumpThreadWriter.OneThread_AMD64_Stack test were observed in 32-bit
x86 Windows builds produced by Clang in the release configuration. These
crashes occurred in crashpad::test::InitializeMinidumpContextAMD64,
which heap-allocates a MinidumpContextAMD64Writer object. These objects
have an alignment requirement of 16, based on the alignment requirement
of their MinidumpContextAMD64 member.

Although this problem was never observed with MSVC, Clang was making use
of the known strict alignment and producing code that depended on it.
This code crashed if the requirement was not met. MSVC had raised a
warning about this usage (C4316), but the warning was disabled as it did
not appear to have any ill effect on code produced by that compiler.

The problem surfaced in test code, but heap-allocated
MinidumpContextAMD64Writer objects are created in non-test code as well.
The impact is limited, because a 32-bit Windows Crashpad handler would
not have a need to allocate one of these objects.

As a fix, MinidumpContextAMD64Writer is given a custom allocation
function (a static “operator new()” member and matching “operator
delete()”) that returns properly aligned memory.

Change-Id: I0cb924da91716eb01b88ec2ae952a69262cc2de6
Reviewed-on: https://chromium-review.googlesource.com/746539
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-11-01 16:40:58 +00:00
Mark Mentovai
a51e912004 Fix warnings produced by trunk clang in test code
These are mostly -Wsign-compare warnings, with a -Wconstant-conversion
and a -Wunguarded-availability thrown in.

Bug: chromium:779790
Change-Id: Ic2103f3332ce57378db83eca7fa2569efec1a7b6
Reviewed-on: https://chromium-review.googlesource.com/746081
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-11-01 16:35:49 +00:00
Robert Sesek
38c8f51ae8 Add MinidumpByteArray extension and its Writer.
The MinidumpByteArray can be used to carry arbitrary blob payloads in a
minidump file.

Bug: crashpad:192
Change-Id: I1a0710b856375213cdd97eafa9247830aa9a9291
Reviewed-on: https://chromium-review.googlesource.com/716462
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
2017-10-31 21:03:47 +00:00
Mark Mentovai
419f25eac8 Remove PointerVector<> and replace with std::vector<std::unique_ptr<>>
As mentioned at
https://chromium-review.googlesource.com/c/crashpad/crashpad/+/721978/13/tools/crashpad_http_upload.cc#90
Change-Id: I4820346cc0b0bf26633e1de598c884af8af19983
Reviewed-on: https://chromium-review.googlesource.com/724744
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-10-19 04:53:36 +00:00
Mark Mentovai
7a849482ea Switch the language standard to C++14 and use std::make_unique
Update mini_chromium to 7d6697ceb5cb5ca02fde3813496f48b9b1d76d0c

47ff9691450e Switch the language standard to C++14
7d6697ceb5cb Remove base/memory/ptr_util.h and base::WrapUnique

base::WrapUnique and std::make_unique are similar, but the latter is
standardized and preferred.

Most of the mechanical changes were made with this sed:

for f in $(git grep -l base::WrapUnique | uniq); do
  sed -E \
      -e 's%base::WrapUnique\(new ([^(]+)\((.*)\)\);%std::make_unique<\1>(\2);%g' \
      -e 's%base::WrapUnique\(new ([^(]+)\);%std::make_unique<\1>();%g' \
      -e 's%^#include "base/memory/ptr_util.h"$%#include <memory>%' \
      -i '' "${f}"
done

Several uses of base::WrapUnique that did not fit on a single line and
were not matched by this sed were adjusted manually. All #include
changes were audited manually, to at least move <memory> into the
correct section. Where <memory> was already #included by a file (or its
corresponding header), the extra #include was removed. Where <memory>
should have been #included by a header, it was added. Other similar
adjustments to other #includes were also made.

Change-Id: Id4e0baad8b3652646bede4c3f30f41fcabfdbd4f
Reviewed-on: https://chromium-review.googlesource.com/714658
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-10-12 19:07:13 +00:00
Mark Mentovai
b653f86153 Fix Windows build after 8f0636288a0e
Clang, GCC, and MSVS 2017 were fine with a “constexpr” definition
corresponding to a class-scope “static const” declaration, but MSVS 2015
is not.

Change-Id: I8c80c6e62d1a312bad161db98e584be225b70bbf
Reviewed-on: https://chromium-review.googlesource.com/592644
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-07-29 01:25:34 +00:00
Mark Mentovai
8f0636288a Use constexpr at namespace scope
This is essentially based on a search for “^const .*=”.

Change-Id: I9332c1f0cf7c891ba1ae373dc537f700f9a1d956
Reviewed-on: https://chromium-review.googlesource.com/585452
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-07-29 01:06:52 +00:00
Mark Mentovai
6dac7ecdf5 Use constexpr at function scope
This is essentially based on a search for “^ *const [^*&]*=[^(]*$”

Change-Id: Id571119d0b9a64c6f387eccd51cea7c9eb530e13
Reviewed-on: https://chromium-review.googlesource.com/585555
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-07-29 00:50:40 +00:00
Mark Mentovai
281be63d00 Standardize on static constexpr for arrays when possible
This uses “static” at function scope to avoid making local copies, even
in cases where the compiler can’t see that the local copy is
unnecessary. “constexpr” adds additional safety in that it prevents
global state from being initialized from any runtime dependencies, which
would be undesirable.

At namespace scope, “constexpr” is also used where appropriate.

For the most part, this was a mechanical transformation for things
matching '(^| )const [^=]*\['.

Similar transformations could be applied to non-arrays in some cases,
but there’s limited practical impact in most non-array cases relative to
arrays, there are far more use sites, and much more manual intervention
would be required.

Change-Id: I3513b739ee8b0be026f8285475cddc5f9cc81152
Reviewed-on: https://chromium-review.googlesource.com/583997
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-07-25 17:40:51 +00:00
Mark Mentovai
8c35d92ae4 Use the C++11-standardized alignof instead of ALIGNOF
Use the standard alignas instead of ALIGNAS in cases where this is
possible too. It’s not currently possible where ALIGNAS may be mixed
with other attributes, although the not-landed
https://codereview.chromium.org/2670873002/ suggests that where ALIGNAS
is mixed with __attribute__((packed)), it’s viable to write “struct
alignas(4) S { /* … */ } __attribute__((packed));”.

This includes an update of mini_chromium to
723e840a2f100a525f7feaad2e93df31d701780a, picking up:

723e840a2f10 Remove ALIGNOF

This tracks upstream https://codereview.chromium.org/2932053002/.

Change-Id: I7ddaf829020ef3be0512f803cecbb7c543294f07
Reviewed-on: https://chromium-review.googlesource.com/533356
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-06-13 18:33:35 +00:00
Sigurdur Asgeirsson
30385d4e47 handler: Add user extensibility stream call-out.
Bug: crashpad:167
Test: Add crashpad_handler_test.
Change-Id: I79b0b71dc4f61e6dce6bc10083e2f924dc83c940
Reviewed-on: https://chromium-review.googlesource.com/463746
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-04-11 19:06:00 +00:00
Mark Mentovai
4b450c8137 test: Use (actual, [un]expected) in gtest {ASSERT,EXPECT}_{EQ,NE}
gtest used to require (expected, actual) ordering for arguments to
EXPECT_EQ and ASSERT_EQ, and in failed test assertions would identify
each side as “expected” or “actual.” Tests in Crashpad adhered to this
traditional ordering. After a gtest change in February 2016, it is now
agnostic with respect to the order of these arguments.

This change mechanically updates all uses of these macros to (actual,
expected) by reversing them. This provides consistency with our use of
the logging CHECK_EQ and DCHECK_EQ macros, and makes for better
readability by ordinary native speakers. The rough (but working!)
conversion tool is
https://chromium-review.googlesource.com/c/466727/1/rewrite_expectassert_eq.py,
and “git cl format” cleaned up its output.

EXPECT_NE and ASSERT_NE never had a preferred ordering. gtest never made
a judgment that one side or the other needed to provide an “unexpected”
value. Consequently, some code used (unexpected, actual) while other
code used (actual, unexpected). For consistency with the new EXPECT_EQ
and ASSERT_EQ usage, as well as consistency with CHECK_NE and DCHECK_NE,
this change also updates these use sites to (actual, unexpected) where
one side can be called “unexpected” as, for example, std::string::npos
can be. Unfortunately, this portion was a manual conversion.

References:

https://github.com/google/googletest/blob/master/googletest/docs/Primer.md#binary-comparison
77d6b17338
https://github.com/google/googletest/pull/713

Change-Id: I978fef7c94183b8b1ef63f12f5ab4d6693626be3
Reviewed-on: https://chromium-review.googlesource.com/466727
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-04 12:34:24 +00:00
Sigurdur Asgeirsson
810d4815df minidump: Allow for user extension streams computed at crash time
Bug: crashpad:167
Change-Id: I94c143353482f100a31d6afb0685b38757a662d6
Reviewed-on: https://chromium-review.googlesource.com/455976
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-03-23 13:10:57 +00:00
Mark Mentovai
fa3413e14a GCC fix for -Wunused-but-set-variable
While compiling minidump_module_writer_test.cc:

minidump/minidump_module_writer_test.cc: In member function 'virtual void crashpad::test::{anonymous}::MinidumpModuleWriter_InitializeFromSnapshot_Test::TestBody()':
minidump/minidump_module_writer_test.cc:656:15: error: variable 'module_names' set but not used [-Werror=unused-but-set-variable]
   const char* module_names[arraysize(expect_modules)] = {};
               ^

Bug: crashpad:30
Change-Id: Ie6bcbced67c947ba6cca32a7057a8ac6de4d0e5a
Reviewed-on: https://chromium-review.googlesource.com/457958
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-03-23 02:52:26 +00:00
Mark Mentovai
7a7815506b GCC fix: Don’t use arraysize() on packed structs
These were previously fixed in f83530bf9a0b for some targets, but not
crashpad_minidump_test.

While compiling minidump_misc_info_writer_test.cc:

In file included from minidump/minidump_misc_info_writer.h:26:0,
                 from minidump/minidump_misc_info_writer_test.cc:15:
minidump/minidump_misc_info_writer_test.cc: In member function ‘virtual void crashpad::test::{anonymous}::MinidumpMiscInfoWriter_TimeZone_Test::TestBody()’:
minidump/minidump_misc_info_writer_test.cc:401:39: error: cannot bind packed field ‘expected.MINIDUMP_MISC_INFO_3::TimeZone.TIME_ZONE_INFORMATION::StandardName’ to ‘short unsigned int (&)[32]’
           arraysize(expected.TimeZone.StandardName));
                     ~~~~~~~~~~~~~~~~~~^
third_party/mini_chromium/mini_chromium/base/macros.h:41:50: note: in definition of macro ‘arraysize’
 #define arraysize(array) (sizeof(ArraySizeHelper(array)))
                                                  ^~~~~

Bug: crashpad:30
Change-Id: I2a1c3b356c0064e8161ec70a9ac156053fc28df7
Reviewed-on: https://chromium-review.googlesource.com/457881
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-03-23 02:40:32 +00:00
Mark Mentovai
013d5e14a3 #include <stddef.h> where offsetof() is used
Bug: crashpad:30
Change-Id: If23ca9ea3141d3d34dc494aa29a1bd1dc8f83130
Reviewed-on: https://chromium-review.googlesource.com/458079
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-03-23 02:15:32 +00:00
Mark Mentovai
542306626d minidump: Make the MemoryListStream the caboose once again
After b10d9118dea4, the MemoryListStream was moved from its preferred
position as the last stream in the file to precede user minidump
streams, in an effort to prevent it from being preempted by a user
minidump stream that specified the memory list stream’s type. A better
solution, which keeps all streams where they want to be, is to put the
MemoryListStream at the end, put user streams before it, and omit user
streams that purport to be a MemoryListStream.

Bug: crashpad:171
Change-Id: I6974fbd4c9ec67284f86c593c553af7adf73601b
Reviewed-on: https://chromium-review.googlesource.com/456823
Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org>
2017-03-20 17:32:01 +00:00