30 Commits

Author SHA1 Message Date
Jan Wilken Dörrie
a98ee20e57 [crashpad] Prepare crashpad for base::string16 switch
This change prepares crashpad for the upcoming switch of base::string16
to std::u16string on all platforms. It does so by replacing Windows-only
instances of base::string16 with std::wstring, and using appropriate
string utility functions.

Bug: chromium:911896
Change-Id: Ibb0b8a4e4dc7fae1d24d18823f8dbb6da31f8239
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2332402
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2020-09-12 07:36:53 +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
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
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
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
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
b10d9118de minidump: Ignore attempts to add user streams with type collisions
The unconditional CHECK() in MinidumpFileWriter::AddStream() made sense
when all streams were under the Minidump class family’s control, but
became hazardous upon the introduction of user streams with arbitrary
types under the crashy process’ control.

Bug: crashpad:171
Test: crashpad_minidump_test MinidumpFileWriter.SameStreamType
Change-Id: Iba5be08b330261286d11d22d8e9a2fef5fcc1070
Reviewed-on: https://chromium-review.googlesource.com/456056
Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org>
2017-03-15 19:48:38 +00:00
Mark Mentovai
f848952e05 Support MINIDUMP_MISC_INFO_5 defined in Windows 10
MINIDUMP_MISC_INFO_5 can carry information about extended XSTATE state
components and the process cookie value.

I made some informed guesses about the precise meanings of some of the
attributes of the XSTATE stuff.

I don’t know what “process cookie” refers to yet. My guess is that it’s
the stack canary value, or something similar. But since this isn’t an
informed guess, I haven’t written it into the documentation.

Crashpad does not yet use either of these features.

BUG=crashpad:58

Change-Id: I614568287a01fec99d6cd60e378a6d6e20b4f48c
Reviewed-on: https://chromium-review.googlesource.com/409630
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-03-02 19:27:21 +00:00
Scott Graham
a02ba24006 Convert from scoped_ptr to std::unique_ptr
Follows https://codereview.chromium.org/1911823002/ but fixes includes
that were messed up there.

Change-Id: Ic4bad7d095ee6f5a1c9f8ca2d11ac9e67d55a626
Reviewed-on: https://chromium-review.googlesource.com/340497
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-04-25 19:16:26 +00:00
Mark Mentovai
6d2d31d2d1 Use base/macros.h instead of base/basictypes.h
This was done in Chromium’s local copy of Crashpad in 562827afb599. This
change is similar to that one, except more care was taken to avoid
including headers from a .cc or _test.cc when already included by the
associated .h. Rather than using <stddef.h> for size_t, Crashpad has
always used <sys/types.h>, so that’s used here as well.

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

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

R=avi@chromium.org

Review URL: https://codereview.chromium.org/1566713002 .
2016-01-06 12:22:50 -05:00
Mark Mentovai
583d1dc3ef Provide std::move() in compat instead of using crashpad::move()
This more-natural spelling doesn’t require Crashpad developers to have
to remember anything special when writing code in Crashpad. It’s easier
to grep for and it’s easier to remove the “compat” part when pre-C++11
libraries are no longer relevant.

R=scottmg@chromium.org

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

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

Review URL: https://codereview.chromium.org/1483073004 .
2015-11-30 14:20:54 -08:00
Mark Mentovai
583233cf78 Add FileReaderInterface. Move StringFileWriter to StringFile and
implement the new interface.

The upcoming minidump reader will get minidump data from a
FileReaderInterface. For ease of testing, a string-based implementation
is provided. There wasn’t a good reason to have a separate
StringFileReader and StringFileWriter, so I combined them into a single
StringFile.

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

Review URL: https://codereview.chromium.org/936153002
2015-02-18 14:15:38 -05:00
Scott Graham
2d2ad633ea win: Fixes for minidump_misc_info_writer_test.cc
MINIDUMP_MISC_INFO and MINIDUMP_MISC_INFO_2, etc. are not
derived from each other in Windows' dbghelp.h, so need
a reinterpret_cast.

arraysize fails on Struct::Member with a big mess (below)
but works ok on a local stack instance.

d:\src\crashpad\crashpad\minidump\minidump_misc_info_writer_test.cc(405) : error C2664: 'std::basic_string<char,std::char_traits<char>,std::allocator<char>>::basic_string(std::initializer_list<_Elem>,const std::allocator<char> &)' : cannot convert argument 1 from 'char' to 'const std::basic_string<char,std::char_traits<char>,std::allocator<char>> &'
        with
        [
            _Elem=char
        ]
        Reason: cannot convert from 'char' to 'const std::basic_string<char,std::char_traits<char>,std::allocator<char>>'
        No constructor could take the source type, or constructor overload resolution was ambiguous
d:\src\crashpad\crashpad\minidump\minidump_misc_info_writer_test.cc(408) : error C2784: 'char (&ArraySizeHelper(const T (&)[N]))[N]' : could not deduce template argument for 'const T (&)[N]' from 'unknown'
        d:\src\crashpad\crashpad\third_party\mini_chromium\mini_chromium\base\basictypes.h(39) : see declaration of 'ArraySizeHelper'

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

Review URL: https://codereview.chromium.org/899163004
2015-02-05 09:35:54 -08:00
Scott Graham
a813dd2e1b win: Fixes for minidump_misc_info_writer_test.cc
test_process_snapshot.h apparently requires the full inclusion,
not a forward declaration otherwise it claims to be deleting
undefined types.

And, some more potentially uninitialized variables.

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

Review URL: https://codereview.chromium.org/902803002
2015-02-05 09:25:14 -08:00
Scott Graham
429a3368d4 win: Work towards getting 'minidump' to compile
- dbghelp.h requires windows.h to be included before it (ick!).
  Add a stub one for non_win to make this work.
- convert __attribute__ -> macro that can work work with MSVC;
- a handful of narrowing casts.

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

Review URL: https://codereview.chromium.org/883773005
2015-02-04 17:30:03 -08:00
Scott Graham
af07f4022b Move string16 and char16 in to base::
Needs to include roll with https://codereview.chromium.org/803593002/ included.

R=mark@chromium.org

Review URL: https://codereview.chromium.org/804593002
2014-12-12 11:06:09 -08:00
Mark Mentovai
408262d58f Fix MinidumpMiscInfoWriter test compilation with 32-bit time_t.
This produced errors such as:
minidump_misc_info_writer_test.cc:665:32: error: non-constant-expression
cannot be narrowed from type 'uint32_t' (aka 'unsigned int') to
'__darwin_time_t' (aka 'long') in initializer list [-Wc++11-narrowing]
  const timeval kStartTime = { expect_misc_info.ProcessCreateTime, 0 };
                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
minidump_misc_info_writer_test.cc:665:32: note: override this message by
inserting an explicit cast
  const timeval kStartTime = { expect_misc_info.ProcessCreateTime, 0 };
                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                               static_cast<__darwin_time_t>(     )

TEST=minidump_test MinidumpMiscInfoWriter.InitializeFromSnapshot
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/711603002
2014-11-07 10:02:21 -05:00
Mark Mentovai
bdfd147a47 Add MinidumpMiscInfoWriter::InitializeFromSnapshot() and its test.
TEST=minidump_test MinidumpMiscInfoWriter.InitializeFromSnapshot
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/701783004
2014-11-05 18:15:19 -05:00
Mark Mentovai
0a4ea0b52d minidump: Change the ownership model.
All minidump objects now own their all of their children, rather than
having them maintain weak pointers and requiring callers to maintain
ownership.

The only weak object in the entire tree now is the “extra memory” added
to a MinidumpMemoryListWriter by its AddExtraMemory() method. Extra
memory aliases objects owned elsewhere in the tree, typically by a
MinidumpThreadWriter as stack memory. Non-“extra” memory added to a
MinidumpMemoryListWriter by its AddMemory() method is strongly owned.

Many objects are now deleted through base pointers, and in those cases,
the base classes now have public virtual destructors. The ultimate base,
MinidumpWritable, is still protected to guard against direct
instantiation and deletion, and thus its destructor does not need to be
virtual.

This updates mini_chromium to eeb3b6a4f020 specifically for that
revision, which includes necessary updates to scoped_ptr. It also picks
up:

eeb3b6a4f020 Update base/move.h and base/memory/scoped_ptr.h to match
67ad2efafaba More porting to Windows
be27a006421e AUTHORS: Fix link post-git migration flag day.
05f5b1503230 Add codereview.settings to mini_chromium.
a32c2b199811 Beginnings of Windows support in mini_chromium

TEST=minidump_test
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/674153002
2014-10-27 15:01:39 -04:00
Mark Mentovai
20f95a2f5b minidump: Migrate the rest of the tests to
MinidumpWritableAtLocationDescriptor<>().

TEST=minidump_test
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/670853002
2014-10-22 18:35:18 -04:00
Mark Mentovai
8a6a4c68e4 Refactor minidump test utilities for MinidumpWritable,
MinidumpFileWriter, and Minidump*StringWriter.

TEST=minidump_test
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/664283002
2014-10-21 14:15:07 -04:00
Mark Mentovai
01c535b001 Move minidump/*_test_util.* to minidump/test.
Suggested at
https://codereview.chromium.org/654573003/diff/1/minidump/minidump_string_writer_test_util.h#newcode22

TEST=minidump_test
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/663093003
2014-10-20 12:11:14 -04:00
Mark Mentovai
c182be95a2 Rename minidump_test_util.* to minidump_file_writer_test_util.*.
This is in line with the other new minidump_*_test_util files, namely,
the “context” and “memory” ones. minidump_file_writer_test_util.*
provides VerifyMinidumpHeader(), which is used to validate the
MINIDUMP_HEADER at the beginning of a minidump file, as written by
MinidumpFileWriter.

TEST=minidump_test
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/643643003
2014-10-09 17:07:59 -04:00
Mark Mentovai
22350bd676 In tests, use ASSERT_NO_FATAL_FAILURE() instead of checking
testing::Test::HasFatalFailure() after calling functions that could fail
fatally.

Inspired by
https://codereview.chromium.org/637503006/diff/20001/minidump/minidump_thread_writer_test.cc#newcode437

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

Review URL: https://codereview.chromium.org/640383002
2014-10-09 15:08:54 -04:00
Mark Mentovai
6d1af6922f Don’t use using directives (“using namespace”) in tests.
The contents of tests are moved into the namespace
crashpad::test::(anonymous namespace).

https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Namespaces

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/635883002
2014-10-07 17:28:50 -04:00
Mark Mentovai
3b2c6d32f4 Add MinidumpMemoryWriter, MinidumpMemoryListWriter, and their test.
TEST=minidump_test MinidumpMemoryWriter
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/459973002
2014-08-12 10:26:40 -07:00
Mark Mentovai
c73a97d4e3 Add MinidumpMiscInfoWriter and its test.
TEST=minidump_test MinidumpMiscInfoWriter
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/454073002
2014-08-11 11:42:12 -07:00