29 Commits

Author SHA1 Message Date
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
Scott Graham
bfeb194b01 fuchsia: Get crashpad_util_test building
Links, but various tests fail.

Also adds support to run_tests.py to run a single binary, likely only
useful on Fuchsia.

Bug: crashpad:196
Change-Id: Ie82ef26ec214ff4262194e877469953aa8fb367e
Reviewed-on: https://chromium-review.googlesource.com/809467
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-12-05 23:23:30 +00:00
Dave Bort
45de8bf76e Add generic VMAddress-related types
This will allow sharing code that is currently hard-coded to use (e.g.)
LinuxVMAddress or mach_vm_size_t.

Change-Id: I7bf20600c73d4ec7d2a029754f9043a236a38e5a
Reviewed-on: https://chromium-review.googlesource.com/677142
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Dave Bort <dbort@google.com>
2017-09-26 17:45:38 +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
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
Joshua Peraza
2815dbdf8e linux: Add CheckedLinuxAddressRange and make CheckedAddressRanges copyable
Bug: crashpad:30
Change-Id: Ied2b8659315c09c77054c0a5a82ac37284f27334
Reviewed-on: https://chromium-review.googlesource.com/481036
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-04-19 20:46:54 +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
b978b03fa1 Port most of crashpad_util_test to Linux/Android
- In the ProcessInfo test, port the global argc/argv getter to Linux by
   reading /proc/self/cmdline.
 - Use <inttypes.h> format macros for 64-bit types.
 - Only #include <sys/sysctl.h> on macOS.
 - #include <signal.h> instead of <sys/signal.h>.

In order to test on Linux/Android, the following changes to the
crashpad_util_test target must be made until more porting is complete:

 - Remove the dependency on crashpad_client because that library has not
   been ported yet.
 - Remove process_info_test.cc because it depends on crashpad_client and
   there is no implementation of ProcessInfo for Linux yet.
 - Remove http_transport_test.cc because there is no HTTPTransport
   implementation for Linux or Android yet.
 - Remove checked_address_range_test.cc because checked_address_range.cc
   does not yet expose a cross-bit usable type for addresses and sizes
   on Linux.

BUG=crashpad:30
TEST=crashpad_util_test

Change-Id: Ic17cf26bdf19b3eff3915bb1acdaa701f28222cd
Reviewed-on: https://chromium-review.googlesource.com/405647
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-10-31 21:07:06 +00:00
Scott Graham
d6d726a0eb Update to use base/bit_cast.h to follow to follow https://codereview.chromium.org/399313006
Change-Id: Idaa31e287d709d12dee5fe743869c368d465b3af
Reviewed-on: https://chromium-review.googlesource.com/340496
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2016-04-25 18:55:22 +00:00
Scott Graham
7217cc0a8f Support client-specified extra memory ranges
Change-Id: I378e2513a4894fb1548445b660bb3db86e281572
Reviewed-on: https://chromium-review.googlesource.com/329564
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-02-29 22:16:13 +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
5be8ce4ea0 Get module versions and types from in-memory images
Don't call GetFileVersionInfo(), which calls LoadLibrary() to be able to
access the module's resources. Loading modules from the crashy process
into the handler process can cause trouble. The Crashpad handler
definitely doesn't want to run arbitrary modules' module initializer
code.

Since the VS_FIXEDFILEINFO needed is already in memory in the remote
process' address space, just access it from there.

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

Review URL: https://codereview.chromium.org/1475023004 .
2015-12-01 17:06:37 -05:00
Scott Graham
4df538f283 win: Add memory map range intersection helper
To be used for improved version of ReadMemory() that is memory-map
aware, in particular for reading the environment block in
https://codereview.chromium.org/1360863006/.

R=mark@chromium.org
BUG=crashpad:20, crashpad:46

Review URL: https://codereview.chromium.org/1372183002 .
2015-10-01 11:47:32 -07:00
Scott Graham
0b022d72a2 Include implicit_cast.h at all users of it.
The implicit_cast in base will be no more, make sure we have a reference
to the crashpad version at all callsites.

BUG=529769, 472900, crashpad:51
R=mark@chromium.org, scottmg@chromium.org

Review URL: https://codereview.chromium.org/1344683002 .
2015-09-14 14:51:05 -07:00
Mark Mentovai
b70d302987 Add a few more expectations to crashpad_util_test InRangeCast
TEST=crashpad_util_test InRangeCast.*
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1300403002 .
2015-08-20 12:13:46 -04:00
Scott Graham
69d135acda win: make CrashpadInfo retrievable
The main goal was to get the beginnings of module iteration and retrieval
of CrashpadInfo in snapshot. The main change for that is to move
crashpad_info_client_options[_test] down out of mac/.

This also requires adding some of the supporting code of snapshot in
ProcessReaderWin, ProcessSnapshotWin, and ModuleSnapshotWin. These are
partially copied from Mac or stubbed out with lots of TODO annotations.
This is a bit unfortunate, but seemed like the most productive way to
make progress incrementally. That is, it's mostly placeholder at the
moment, but hopefully has the right shape for things to come.

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

Review URL: https://codereview.chromium.org/1052813002
2015-05-01 13:48:23 -07:00
Scott Graham
cb8c01f410 win: Some %zu to PRIuS
%zu aborts in system printf functions on Windows, so use PRIuS instead.

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

Review URL: https://codereview.chromium.org/849193002
2015-01-15 10:00:43 -08:00
Scott Graham
ec38bf152d win: Add static_cast to AssignIfInRange
Otherwise, when assigning to a smaller type, MSVC warns e.g.

d:\src\crashpad\crashpad\util\numeric\safe_assignment.h(38) : error C2220: warning treated as error - no 'object' file generated
        d:\src\crashpad\crashpad\util\file\string_file_writer.cc(127) : see reference to function template instantiation 'bool crashpad::AssignIfInRange<size_t,FileOffset>(Destination *,Source)' being compiled
        with
        [
            Destination=size_t
,            Source=FileOffset
        ]
d:\src\crashpad\crashpad\util\numeric\safe_assignment.h(38) : warning C4244: '=' : conversion from 'FileOffset' to 'size_t', possible loss of data

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

Review URL: https://codereview.chromium.org/809303003
2014-12-18 08:42:26 -08:00
Scott Graham
1a17e7e643 win: Avoid warnings about conversion to smaller integer types
e.g.

d:\src\crashpad\crashpad\util\numeric\in_range_cast.h(35) : warning C4244: 'return' : conversion from 'unsigned __int64' to 'uint32_t', possible loss of data
d:\src\crashpad\crashpad\util\numeric\in_range_cast.h(35) : warning C4244: 'return' : conversion from '__int64' to 'int32_t', possible loss of data
        d:\src\crashpad\crashpad\util\numeric\in_range_cast_test.cc(54) : see reference to function template instantiation 'Destination crashpad::InRangeCast<int32_t,__int64>(Source,Destination)' being compiled
        with
        [
            Destination=int32_t
,            Source=__int64
        ]

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

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

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

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

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/700383007
2014-11-06 16:44:38 -05:00
Mark Mentovai
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
ae7ad05090 int128_test.cc: Add missing #include after d198c50abe9b.
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/620793003
2014-10-01 16:56:56 -04:00
Scott Graham
d198c50abe Convert COMPILE_ASSERT to static_assert
(Perhaps I should have just left it in mini_chromium, but anyway.)

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

Review URL: https://codereview.chromium.org/615923004
2014-10-01 12:29:01 -07:00
Mark Mentovai
fec869992f uint128_struct: address misunderstood review feedback.
TEST=util_test Int128.UInt128
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/590283002
2014-09-22 17:16:03 -04:00
Mark Mentovai
fee5d8aea9 Add uint128_struct and its test.
This will be used in the upcoming Minidump context structures.

This also updates mini_chromium to ba9b15f1b6a7 to pick up:

ba9b15f1b6a7 Make bit_cast available in C++11 mode.

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/583283003
2014-09-22 15:46:00 -04:00
Mark Mentovai
d05166c26c Add CheckedRange<> and its test.
TEST=util_test CheckedRange
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/467113002
2014-08-15 10:22:37 -07:00
Mark Mentovai
6997418cd4 Add InRangeCast<> and its test.
InRangeCast<> is similar to base::saturated_cast<>, but it allows the
caller to specify the value to be returned in the event that the source
value can’t be converted to the destination data type without data loss.

TEST=util_test InRangeCast
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/467103002
2014-08-14 16:00:44 -07:00
Mark Mentovai
995012534e Introduce MinidumpWritable, its dependencies, and their tests.
MinidumpWritable is the base class for all minidump-writing operations.

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/432003005
2014-08-01 12:48:28 -04:00