24 Commits

Author SHA1 Message Date
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