The build broke because x30 was used instead of r14 in place of LR
which gcc doesn't recognize when building for 64-bit ARM. gcc does
recognize LR for 32-bit ARM, however, so revert to that since it's
more readable.
Also, de-duplicate saving of FP/IP which are synonyms of r11/r12,
saved above.
Change-Id: I8ae28f430cc3c47f4e4cf3679383ed5b94fadd2e
Reviewed-on: https://chromium-review.googlesource.com/1217483
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
When tested with GCC 6, it couldn't to understand LR register.
Thus, use x30 instead.
The error this patch fixes is the following:
Error: operand 1 must be an integer register -- `str LR,[x0,#0x1b8]'
Test: compile for aarch64
Change-Id: Icf1199254c6a29f72b6d2fa7940e1f33259a728b
Reviewed-on: https://chromium-review.googlesource.com/1213125
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
These fixes are mostly related to address sanitizer causing stack
variables to not be stored on the call-stack. Attempting to disable
safe-stack has no effect.
Change-Id: Ib5718bfb74ce91dee560b397ccdbf68d78e4ec6a
Reviewed-on: https://chromium-review.googlesource.com/1140507
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Sanitization is controlled by a SanitizationInformation struct to be
read from the client's memory. The address of this struct is either
passed in a ClientInformation when the client requests a crash dump,
or as a flag to the handler --sanitization_information.
Bug: crashpad:30
Change-Id: I2744f8fb85b4fea7362b2b88faa4bef1da74e36b
Reviewed-on: https://chromium-review.googlesource.com/1083143
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
A ProcessSnapshotSanitized enables filtering possibly sensitive
information from a snapshot.
WebView has different privacy constraints than Chrome and needs to
avoid collecting data in annotations or from stack memory that may
contain PII. This CL enables:
1. Filtering annotations by name using a whitelist.
2. Filtering for crashes which reference a particular module.
3. Redacting non-essential information from stack memory.
This CL does not provide a client interface to enable sanitization.
Bug: crashpad:30
Change-Id: I8944c70fdcca6d6d4b7955d983320909bf871254
Reviewed-on: https://chromium-review.googlesource.com/1070472
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Officially, register X31 does not exist. The code is zeroing out a location
and thus actually needs XZR.
LLVM seems to automatically translate X31 into XZR when compiling the code,
but GCC (tested 7.3.0) refuses to accept the instruction:
../../third_party/crashpad/crashpad/util/misc/capture_context_linux.S: Assembler messages:
../../third_party/crashpad/crashpad/util/misc/capture_context_linux.S:291: Error: operand 1 must be an integer register -- `str x31,[x0,#0xb0]'
Bug: chromium:819294
Change-Id: I85be3923ac56fca6e3ec59d7e22b2223cfc8fa63
Reviewed-on: https://chromium-review.googlesource.com/1078818
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Copied from the _linux implementation, which looks close to what
ucontext on Fuchsia is (though it will probably need to change).
In arm64 debug, CaptureContext.CaptureContext requires slightly longer
slop distance.
Bug: crashpad:196
Change-Id: I2a6f90095e06fe8b468fbfd8add66a73c8a1d92f
Reviewed-on: https://chromium-review.googlesource.com/1031091
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
uc_mcontext.fpregs is a pointer to the floating point context, but
CaptureContext() doesn't yet capture floating point context.
This error manages to slip by unit tests when run all together, but
fails when CrashpadClient.SimulateCrash is run by itself.
Bug: crashpad:30
Change-Id: I7adc30648642912d66a7ba8cf9973c9bc0fbd8bc
Reviewed-on: https://chromium-review.googlesource.com/1011504
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Fuchsia enables safe-stack by default in the compiler. Disable it for
the test function so that a candidate RSP value can be found by using
the value of locals on the stack.
(This also reduces the function prolog size sufficiently for the PC
comparison to work, otherwise it required 75 bytes for the delta
comparison.)
Bug: crashpad:196
Change-Id: I2adbcee93c90dbc415309b79e3d16e9c4635f989
Reviewed-on: https://chromium-review.googlesource.com/1000140
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
glibc 2.26 defines ucontext_t from a struct ucontext_t while Bionic
and older versions of glibc use a struct ucontext.
Bug: crashpad:30
Change-Id: I473c317dbdbbedfad601c7594cfa7df7f7c01cb9
Reviewed-on: https://chromium-review.googlesource.com/972613
Reviewed-by: Mark Mentovai <mark@chromium.org>
Embeds the address of g_crashpad_info into a .note section (which is
readable by the generic code to read notes in ElfImageReader).
Unfortunately because the note section is in libclient.a, it would
normally be dropped at link time. To avoid that, GetCrashpadInfo() has
a reference *back* to that section, which in turn forces the linker to
include it, allowing the note reader to find it at runtime.
Previously, it was necessary to have the embedder of "client" figure out
how to cause `g_crashpad_info` to appear in the final module's dynamic
symbol table. With this new approach, there's no manual configuration
necessary, as it's not necessary for the symbol to be exported.
This is currently only implemented in the Linux module reader (and I
believe the current set of enabled tests aren't exercising it?) but it
will also be done this way for the Fuchsia implementation of
ModuleSnapshot.
Bug: crashpad:196
Change-Id: I599db5903bc98303130d11ad850ba9ceed3b801a
Reviewed-on: https://chromium-review.googlesource.com/912284
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
This change updates CrashReportDatbase::NewReport objects to own the
file handle associated with the new report, now accessible via a
FileWriter. NewReport's destructor closes its file handle and removes
its new report unless disarmed with FinishedWritingCrashReport,
eliminating the need for CallErrorWritingCrashReport.
Bug: crashpad:206
Change-Id: Iccb5bbc0ebadb07a237ff8eb938389afcfeae2a5
Reviewed-on: https://chromium-review.googlesource.com/916941
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Split out of crrev.com/c/689745 by jperaza, with a simple test added.
It is useful for this to be an overload instead of a separate signature
so that code that extracts a UUID string out of a filename can treat it
generically between Windows and non-Windows.
Bug: crashpad:196, crashpad:206
Change-Id: I0d7d84a93d9526d1aae8839179dfe903acca091b
Reviewed-on: https://chromium-review.googlesource.com/916885
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Previously, the mac version was under client/ and win under util/win/.
This cl brings them all together under util/misc/ and combines common
test code.
Bug: crashpad:30
Change-Id: Idf0d0158b969d5aa9802dfc8c21f73041b2bcc6c
Reviewed-on: https://chromium-review.googlesource.com/907755
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
This supports multiprocess tests of the non-fork() variety.
Also, improve directory finding so that the
crashpad_test_test_multiprocess_exec_test_child binary can be located
correctly on Fuchsia.
Doc ref for launchpad:
https://fuchsia.googlesource.com/zircon/+/master/system/ulib/launchpad/include/launchpad/launchpad.h#23
Also, roll mini_chromium to pick up ScopedZxHandle addition. Includes:
a19ef08 Merge ScopedZxHandle from Chromium base
f21c900 fuchsia: Move zircon libs dep to base, rather than global
Bug: crashpad:196
Change-Id: Id01dee43f2d04e682e70c12777aff41f8dd848d6
Reviewed-on: https://chromium-review.googlesource.com/868967
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
- Implement build/run_tests.py to run on Fuchsia device
- Implement paths_fuchsia.cc using standard Fuchsia namespace layout
- Exclude multiprocess tests, currently unimplemented
- Don't use unnecessary O_ flags on Fuchsia in open() call.
Bug: crashpad:196, chromium:726124, ZX-797
Change-Id: Ie59dce685b4c3fe54f3e36f357c1101d402ee8b7
Reviewed-on: https://chromium-review.googlesource.com/802180
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
There's no particular UUID generator on Fuchsia, so use the RandBytes()
version. (That won't work either yet, but will once RandBytes() is
implemented.)
Bug: crashpad:196
Change-Id: Id740bbfc80e170d7ab19995ac88db5eed474c119
Reviewed-on: https://chromium-review.googlesource.com/786822
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Bug: crashpad:196
Change-Id: I4f01c4f04c94a745b4c30bc41f66d2ae010e883a
Reviewed-on: https://chromium-review.googlesource.com/786817
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
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>
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>
This CL pulls together similar time conversion functions and adds
conversions between `FILETIME`s and `timespec`s.
Bug: crashpad:206
Change-Id: I1d9b1560884ffde2364af0092114f82e1534ad1c
Reviewed-on: https://chromium-review.googlesource.com/752574
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Instead of individual per-directory test executables like
crashpad_util_test, all Crashpad tests in Chromium will be run from a
single crashpad_tests executable.
Test: crashpad_util_test Paths.Executable, ProcessInfo.Self; crashpad_snapshot_test PEImageReader.DebugDirectory
Bug: chromium:779790
Change-Id: If95272fd641734fbdb8e231fbcdc4e7ccb2cb822
Reviewed-on: https://chromium-review.googlesource.com/749303
Reviewed-by: Scott Graham <scottmg@chromium.org>
P0012R1, accepted into C++17, makes a function’s “noexcept” (or
“throw()”) specification part of its signature. GCC 7.2 provides a
warning, -Wnoexcept-type, that is triggered when a function pointer type
with an exception specification is used in pre-C++17 code in such a way
as to pose an ABI incompatibility with C++17 code.
https://gcc.gnu.org/onlinedocs/gcc-7.2.0/gcc/C_002b_002b-Dialect-Options.html#index-Wnoexcept-type
Warnings are of the form:
In file included from ../../util/misc/from_pointer_cast_test.cc:15:0:
../../util/misc/from_pointer_cast.h:64:1: error: mangled name for ‘typename std::enable_if<(std::is_pointer<From>::value && std::is_pointer<_Tp>::value), To>::type crashpad::FromPointerCast(From) [with To = const volatile void*; From = void* (*)(long unsigned int) throw ()]’ will change in C++17 because the exception specification is part of a function type [-Werror=noexcept-type]
FromPointerCast(From from) {
^~~~~~~~~~~~~~~
../../util/misc/from_pointer_cast.h:64:1: error: mangled name for ‘typename std::enable_if<(std::is_pointer<From>::value && std::is_pointer<_Tp>::value), To>::type crashpad::FromPointerCast(From) [with To = volatile void*; From = void* (*)(long unsigned int) throw ()]’ will change in C++17 because the exception specification is part of a function type [-Werror=noexcept-type]
In Crashpad, this warning is triggered by the two FromPointerCast<>()
variants that accept function pointer “From” arguments. This occurs when
using glibc as the standard C library, since glibc declares its
functions as “throw()”. FromPointerCast<>() is used with pointers to
glibc functions such as malloc() and getpid().
The warning is disabled for the FromPointerCast<>() variants that would
trigger it. The warning is not useful or actionable in this internal
Crashpad code where ABI changes due to language version (including
mangling changes) are not a concern.
Clang 4.0 has the similar -Wc++1z-compat-mangling option (also available
as -Wc++17-compat-mangling and the GCC-compatible -Wnoexcept-type in
Clang 5.0) but it is not triggered by this pattern.
Change-Id: Id293db3954be415f67a55476ca72bfb7d399aa3b
Reviewed-on: https://chromium-review.googlesource.com/738292
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
This change also adds functions to create directories, remove files and
directories, and check for the existence of files and directories.
Change-Id: I62b78219ae2b277d6976d2d90ec86fcabd0ef073
Reviewed-on: https://chromium-review.googlesource.com/696132
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
OS_LINUX is not defined on Android. Chromium made this call and we can’t
revisit it here and now.
Change-Id: I70fd6ac35ba9731e2fd06792bf8cae332e2b360c
Reviewed-on: https://chromium-review.googlesource.com/700655
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
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>
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>
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>
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>
This renames and improves the VariableSizeBitCast helper from
util/linux/auxiliary_vector.* and moves it to misc.
Change-Id: I4bf46f4cfc0e60c900ff9bde467a21ad43c684cd
Reviewed-on: https://chromium-review.googlesource.com/534174
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
I opted to leave casts to types that were definitely the same size
alone. reinterpret_cast<uintptr_t>(pointer) and
reinterpret_cast<intptr_t>(pointer) should always be safe, for example.
Casts to other integral types have been replaced with
FromPointerCast<>(), which does zero-extension or sign-extension based
on the target type.
To make it possible to use FromPointerCast<>() with some use sites that
were already using checked_cast<>(), FromPointerCast<>() now uses
check_cast<>() when converting to a narrower type.
Test: crashpad_util_test FromPointerCast*, others
Change-Id: I4a71b4aa2d87f545c75524290a702f5f3138d675
Reviewed-on: https://chromium-review.googlesource.com/489701
Reviewed-by: Scott Graham <scottmg@chromium.org>
Some of the new Linux/Android tests were failing in 32-bit code where
pointers were being casted via reinterpret_cast<>() to LinuxVMAddress,
an unsigned 64-bit type. The behavior of such casts is
implementation-defined, and in this case, sign-extension was being used
to convert the 32-bit pointers to 64 bits, resulting in very large
(unsigned) LinuxVMAddress values that could not possibly refer to proper
addresses in a 32-bit process’ address space.
The offending reinterpret_cast<>() conversions have been replaced with
the new FromPointerCast<>(), which is careful to do sign-extension when
converting to a signed type, and zero-extension when converting to an
unsigned type like LinuxVMAddress.
Bug: crashpad:30
Test: crashpad_util_test FromPointerCast*:MemoryMap.*:ProcessMemory.*
Change-Id: I6f1408dc63369a8740ecd6015d657e4407a7c271
Reviewed-on: https://chromium-review.googlesource.com/488264
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
These tests:
- InitializationState.InitializationState
- InitializationStateDcheckDeathTest.Destroyed_NotUninitialized
- InitializationStateDcheckDeathTest.Destroyed_NotValid
rely on certain behavior from destroyed objects. This is undefined
behavior and we know it, but the whole point of the of
InitializationState and InitializationStateDcheck destructors is to try
to help catch other parts of the program making use of undefined
behavior.
To make it impossible for the memory that formerly hosted these objects
to be repurposed during tests after the objects are destroyed, these
tests that attempt to work with destroyed objects are changed to use
placement new, so that the lifetimes of the objects can be decoupled
from the lifetimes of the buffers.
Test: crashpad_util_test InitializationState*
Change-Id: Ie972a54116c8b90a21a502d3ba13623583dfac06
Reviewed-on: https://chromium-review.googlesource.com/486383
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
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-comparison77d6b17338https://github.com/google/googletest/pull/713
Change-Id: I978fef7c94183b8b1ef63f12f5ab4d6693626be3
Reviewed-on: https://chromium-review.googlesource.com/466727
Reviewed-by: Scott Graham <scottmg@chromium.org>
This supports the “double handler” or “double handler with low
probability” models from https://crashpad.chromium.org/bug/143.
For crashpad_handler to be become its own client, it needs access to its
own executable path to pass to CrashpadClient::StartHandler(). This was
formerly available in the test-only test::Paths::Executable(). Bring
that function’s implementation to the non-test Paths::Executable() in
util/misc, and rename test::Paths to test::TestPaths to avoid future
confusion.
test::TestPaths must still be used to access TestDataRoot(), which does
not make any sense to non-test code.
test::TestPaths::Executable() is retained for use by tests, which most
likely prefer the fatal semantics of that function. Paths::Executable()
is not fatal because for the purposes of implementing the double
handler, a failure to locate the executable path (which may happen on
some systems in deeply-nested directory hierarchies) shouldn’t cause the
initial crashpad_handler to abort, even if it does prevent a second
crashpad_handler from being started.
Bug: crashpad:143
Test: crashpad_util_test Paths.*, crashpad_test_test TestPaths.*
Change-Id: I9f75bf61839ce51e33c9f7c0d7031cebead6a156
Reviewed-on: https://chromium-review.googlesource.com/466346
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
It could be useful to put our existing Crashpad.HandlerCrashed metrics
into context by getting a sense of handler starts, clean exits, and
other types of exits.
BUG=crashpad:100
Change-Id: I8982075158ea6d210eb2ddad678302e339a42192
Reviewed-on: https://chromium-review.googlesource.com/444124
Reviewed-by: Scott Graham <scottmg@chromium.org>
This adds zlib to Crashpad. By default in standalone Crashpad builds,
the system zlib will be used where available. A copy of Chromium’s zlib
(currently a slightly patched 1.2.11) is checked out via DEPS into
third_party for use on Windows, which does not have a system zlib.
zlib is used to produce gzip streams for HTTP upload request bodies sent
by crashpad_handler by default. The Content-Encoding: gzip header is set
for these compressed request bodies. Compression can be disabled for
upload to servers without corresponding decompression support by
starting crashpad_handler with the --no-upload-gzip option.
Most minidumps compress quite well with zlib. A size reduction of 90% is
not uncommon.
BUG=crashpad:157
TEST=crashpad_util_test GzipHTTPBodyStream.*:HTTPTransport.*
Change-Id: I99b86db3952c3685cd78f5dc858a60b54399c513
Reviewed-on: https://chromium-review.googlesource.com/438585
Reviewed-by: Robert Sesek <rsesek@chromium.org>
ConvertStringSecurityDescriptorToSecurityDescriptor() is used when
creating the initial connection pipe. Because this is done from inside
DllMain(), we cannot use advapi32 (where this function is). Instead,
save the binary representation of the self-relative SECURITY_DESCRIPTOR.
It is conceivable that this could change, but unlikely as this is the
same blob that would be stored on a file in NTFS.
Another potential approach would be to not make the pipe available to
all integrity levels here, and instead modify the Chromium sandbox code
to allow a specific pipe name prefix that would have to correspond with
the pipe name that Crashpad creates.
Similarly, UuidCreate() (used when initializing the database) is in a
DLL that can't be loaded early, so use the Linux/Android implementation
on Windows too.
R=mark@chromium.org
BUG=chromium:655788,chromium:656800
Change-Id: I434f8e96fc275fc30d0a31208b025bfc08595ff9
Reviewed-on: https://chromium-review.googlesource.com/417223
Reviewed-by: Mark Mentovai <mark@chromium.org>
__has_feature() is a Clang-ism not implemented by GCC.
base/compiler_specific.h provides a HAS_FEATURE() macro that always
returns 0 when __has_feature() is not implemented. Use this macro for
compatibility with GCC and other compilers that do not implement this
Clang extension.
http://clang.llvm.org/docs/LanguageExtensions.html#has-feature-and-has-extension
For GCC’s Address Sanitizer implementation, test the
__SANITIZE_ADDRESS__ macro that it provides as an alternative to
__has_feature(address_sanitizer).
Note that in Chrome builds, ADDRESS_SANITIZER is pushed in by the build
system. The definition of ADDRESS_SANITIZER provides another way for
that macro to be set. It’s supplementary, not exclusive.
cb33b24372/build/config/BUILD.gn (118)
BUG=crashpad:30
Change-Id: I5c3145d29bbc966925369c03a37b1ecb5622a004
Reviewed-on: https://chromium-review.googlesource.com/413109
Reviewed-by: Robert Sesek <rsesek@chromium.org>
After f83530bf9a0b and 72fbc56e58d3, while compiling
arraysize_unsafe_test.cc:
…\crashpad\util\misc\arraysize_unsafe_test.cc(58) : error C2220: warning treated as error - no 'object' file generated
…\crashpad\util\misc\arraysize_unsafe_test.cc(58) : warning C4101: 's10' : unreferenced local variable
…\crashpad\util\misc\arraysize_unsafe_test.cc(33) : warning C4101: 'i1' : unreferenced local variable
…\crashpad\util\misc\arraysize_unsafe_test.cc(24) : warning C4101: 'c1' : unreferenced local variable
…\crashpad\util\misc\arraysize_unsafe_test.cc(27) : warning C4101: 'c2' : unreferenced local variable
…\crashpad\util\misc\arraysize_unsafe_test.cc(55) : warning C4101: 's1' : unreferenced local variable
…\crashpad\util\misc\arraysize_unsafe_test.cc(39) : warning C4101: 'i4' : unreferenced local variable
…\crashpad\util\misc\arraysize_unsafe_test.cc(45) : warning C4101: 'l9' : unreferenced local variable
…\crashpad\util\misc\arraysize_unsafe_test.cc(30) : warning C4101: 'c4' : unreferenced local variable
…\crashpad\util\misc\arraysize_unsafe_test.cc(42) : warning C4101: 'l8' : unreferenced local variable
…\crashpad\util\misc\arraysize_unsafe_test.cc(36) : warning C4101: 'i2' : unreferenced local variable
The line numbers are totally out of order!
I think that my error was not actually ever running “gclient runhooks”,
so I never tested this locally on Windows as I thought I had.
https://build.chromium.org/p/client.crashpad/builders/crashpad_win_x64_dbg/builds/266/steps/compile%20with%20ninja/logs/stdioTBR=scottmg@chromium.org (holiday)
Change-Id: I00414b54c04b5b7e3aa564b0c6fd49d20a47b6ea
Reviewed-on: https://chromium-review.googlesource.com/410129
Reviewed-by: Mark Mentovai <mark@chromium.org>
While compiling, for example, minidump_exception_writer.cc:
In file included from ../../minidump/minidump_exception_writer.h:26:0,
from ../../minidump/minidump_exception_writer.cc:15:
../../minidump/minidump_exception_writer.cc: In member function ‘void crashpad::MinidumpExceptionWriter::SetExceptionInformation(const std::vector<long unsigned int>&)’:
../../minidump/minidump_exception_writer.cc:67:44: error: cannot bind packed field ‘((crashpad::MinidumpExceptionWriter*)this)->crashpad::MinidumpExceptionWriter::exception_.MINIDUMP_EXCEPTION_STREAM::ExceptionRecord.MINIDUMP_EXCEPTION::ExceptionInformation’ to ‘long unsigned int (&)[15]’
arraysize(exception_.ExceptionRecord.ExceptionInformation);
~~~~~~~~~~~~~~~~~~~~~~~~~~~^
../../third_party/mini_chromium/mini_chromium/base/macros.h:41:50: note: in definition of macro ‘arraysize’
#define arraysize(array) (sizeof(ArraySizeHelper(array)))
Tested with:
- GCC 4.9 from NDK r13 targeting arm with SDK 16
- GCC 4.9 from NDK r13 targeting arm64 with SDK 21
- GCC 6.2 targeting x86_64
BUG=crashpad:30
Change-Id: I63963b277a309b4715148215f51902c33ba13b5a
Reviewed-on: https://chromium-review.googlesource.com/409694
Reviewed-by: Scott Graham <scottmg@chromium.org>
This eliminates all constructors, but nearly all points of use were
using the default constructor to initialize a UUID member variable as in
uuid_(). This syntax will still produce a zeroed-out UUID.
While compiling, for example, minidump_rva_list_writer.cc:
In file included from ../../minidump/minidump_rva_list_writer.h:25:0,
from ../../minidump/minidump_rva_list_writer.cc:15:
../../minidump/minidump_extensions.h:412:8: error: ignoring packed attribute because of unpacked non-POD field ‘crashpad::UUID crashpad::MinidumpCrashpadInfo::report_id’ [-Werror]
UUID report_id;
^~~~~~~~~
../../minidump/minidump_extensions.h:424:8: error: ignoring packed attribute because of unpacked non-POD field ‘crashpad::UUID crashpad::MinidumpCrashpadInfo::client_id’ [-Werror]
UUID client_id;
^~~~~~~~~
Tested with:
- GCC 4.9 from NDK r13 targeting arm with SDK 16
- GCC 4.9 from NDK r13 targeting arm64 with SDK 21
- GCC 6.2 targeting x86_64
BUG=crashpad:30
Change-Id: Iec6b1557441b69d75246f2f75c59c4158fb7ca29
Reviewed-on: https://chromium-review.googlesource.com/409641
Reviewed-by: Scott Graham <scottmg@chromium.org>
This makes Doxygen’s output more actionable by setting QUIET = YES to
suppress verbose progress spew, and WARN_IF_UNDOCUMENTED = NO to prevent
warnings for undocumented classes and members from being generated. The
latter is too noisy, producing 721 warnings in the current codebase.
The remaining warnings produced by Doxygen were useful and actionable.
They fell into two categories: abuses of Doxygen’s markup syntax, and
missing (or misspelled) parameter documentation. In a small number of
cases, pass-through parameters had intentionally been left undocumented.
In these cases, they are now given blank \param descriptions. This is
not optimal, but there doesn’t appear to be any other way to tell
Doxygen to allow a single parameter to be undocumented.
Some tricky Doxygen errors were resolved by asking it to not enter
directiores that we do not provide documentation in (such as the
“on-platform” compat directories, compat/mac and compat/win, as well as
compat/non_cxx11_lib) while allowing it to enter the
“off-platform” directories that we do document (compat/non_mac and
compat/non_win).
A Doxygen run (doc/support/generate_doxygen.sh) now produces no output
at all. It would produce warnings if any were triggered.
Not directly related, but still relevant to documentation,
doc/support/generate.sh is updated to remove temporary removals of
now-extinct files and directories. doc/appengine/README is updated so
that a consistent path to “goapp” is used throughout the file.
Change-Id: I300730c04de4d3340551ea3086ca70cc5ff862d1
Reviewed-on: https://chromium-review.googlesource.com/408812
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Use “macOS” as the generic unversioned name of the operating system in
comments. For version-specific references, use Mac OS X through 10.6, OS
X from 10.7 through 10.11, and macOS for 10.12.
Change-Id: I1ebee64fbf79200bc799d4a351725dd73257b54d
Reviewed-on: https://chromium-review.googlesource.com/408269
Reviewed-by: Robert Sesek <rsesek@chromium.org>
- 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>
With this change, it is possible to build crashpad_util for Android with
clang. I built with NDK 13b (clang 3.8) at API 24 (current), API 21
(used by Chrome in 64-bit builds), and API 16 (used by Chrome in 32-bit
builds).
- In WeakFileHandleFileWriter::WriteIoVec(): Android does not expose
the IOV_MAX macro, but its value can be obtained by calling
sysconf(_SC_IOV_MAX).
- In CloseMultipleNowOrOnExec(): API 21 removes getdtablesize(). Skip
it, because it returned the same thing as sysconf(_SC_OPEN_MAX),
which is already consulted.
- Throughout: Various #ifdefs checking for OS_LINUX have been extended
to also check for OS_ANDROID. In Chrome’s build_config.h (and thus
mini_chromium’s), OS_LINUX is not defined when OS_ANDROID is.
This has not been tested beyond building the crashpad_util target.
BUG=crashpad:30
Change-Id: Ieb0bed736029d2d776c534e30e534f186e6fb663
Reviewed-on: https://chromium-review.googlesource.com/405267
Reviewed-by: Robert Sesek <rsesek@chromium.org>
With this change, it is possible to build crashpad_util on Linux. I
built with clang 3.8.1 and GCC 6.2.0.
- For per-OS “exception code” metrics, Android and Linux are broken out
distinctly.
- Because Linux provides no standard UUID generator, base::RandBytes()
is used to generate random UUIDs for the InitializeWithNew() form.
- Multiple fixes for CloseMultipleNowOrOnExec():
- readdir_r() is deprecated in glibc 2.24. Use readdir() on Linux.
- Linux does not have OPEN_MAX. Use the fs.nr_open sysctl (via
/proc/sys) to determine the maximum (currently-configured)
possible number of file descriptors per process.
- Use the {CTL_KERN, KERN_MAXFILESPERPROC} sysctl on Mac to
determine the maximum (currently-configured) possible number of
file descriptors per process. This is an improvement over using
OPEN_MAX, which is still consulted.
- ThreadLogMessages’ use of DCHECK_EQ() needs an address-of operator on
function pointers to avoid confusing GCC.
One problem remains:
- util/misc/pdb_structures.h produces -Wmultichar errors. -Wmultichar
is enabled by default with GCC (but not clang). It is impossible to
disable this warning with #pragma GCC diagnostic ignored. See
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431
This has not been tested beyond building the crashpad_util target.
BUG=crashpad:30
Change-Id: I02e7a05da512ca312806d825b3fc9b2c5bf1a990
Reviewed-on: https://chromium-review.googlesource.com/404009
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Three new metrics:
- counting upload success/failure;
- enum tracking the reason upload was skipped;
- enum describing how an upload got to the pending state.
R=mark@chromium.org, asvitkine@chromium.org
BUG=crashpad:100
Change-Id: I5e0cbc1ac3424e974f3a51560e5cdad484ffc038
Reviewed-on: https://chromium-review.googlesource.com/388855
Reviewed-by: Mark Mentovai <mark@chromium.org>
Otherwise, the Chromium expansions complain about not being able to add
and needing explicit conversions.
R=mark@chromium.org
BUG=crashpad:100
Change-Id: I0540a8dabff61f2189d9532422adae5c2885ae03
Reviewed-on: https://chromium-review.googlesource.com/387166
Reviewed-by: Mark Mentovai <mark@chromium.org>
Includes mini_chromium DEPS roll for:
88e0a3e Add stub of sparse_histogram.h
R=mark@chromium.org
BUG=crashpad:100
Change-Id: I4c541a33be0f7f47e972af638d4765bd06682acf
Reviewed-on: https://chromium-review.googlesource.com/386385
Reviewed-by: Mark Mentovai <mark@chromium.org>
Solves two problems with having the macros inline:
1. Deduplicates some of the logic (in this case, the name of the
histogram, and whether it should be divided by 1024);
2. More useful check for compilation. As the macros are no-ops in
Crashpad, it was easy to use the wrong name for a variable in the
arguments to the macros (see .mm!)
This way, we have some better chance of at least having code that
compiles when built in Chromium if all the arguments are passed to
Metrics::Something() in a standalone build.
Also rolls mini_chromium DEPS to include:
99213eb Mark histogram arguments as unused to avoid warnings
R=mark@chromium.org
BUG=crashpad:100
Change-Id: I9f7fc3b85854fd61c1ebdf0084d728a7b690c2f1
Reviewed-on: https://chromium-review.googlesource.com/380445
Reviewed-by: Mark Mentovai <mark@chromium.org>
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 .
This unifies several things that used a 16-character random string, and
a few other users of random identifiers where it also made sense to use
a 16-character random string.
TEST=crashpad_util_test RandomString.RandomString
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1451793002 .
After 9e79ea1da719, it no longer makes sense for crashpad_util_test_lib
to “hide” in util/util_test.gyp. All of util/test is moved to its own
top-level directory, test, which all other test code is allowed to
depend on. test, too, is allowed to depend on all other non-test code.
In a future change, when crashpad_util_test_lib gains a dependency on
crashpad_client, it won’t look so weird for something in util (even
though it’s in util/test) to depend on something in client, because the
thing that needs to depend on client will live in test, not util.
BUG=crashpad:33
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1051533002
The new call is also used in
CrashReportDatabaseWin::PrepareNewCrashReport(). Previously, that method
used the UUID::InitializeFromBytes() constructor. That actually caused
various fields of the UUID to be byte-swapped so that the ::UUID and
crashpad::UUID would be different UUIDs. Although a UUID is mostly
random, the version field in data_3 is used as a namespace and should be
4 for random UUIDs, and this was not the case under swapping.
TEST=crashpad_util_test UUID.FromSystem
BUG=crashpad:1
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1004913004
disabled.
ClientInfo::set_system_crash_reporter_forwarding() can be used to
disable forwarding. The first module that is found with a non-default
value in this field will dictate whether forwarding is enabled or
disabled. It is possible to enable or disable reporting with this call,
as well as reset it to default, which will allow later modules a chance
to influence the behavior.
ClientInfo::set_crashpad_handler_behavior() is also provided, which can
be used to disable Crashpad’s handling of the exception. Most users
should not call this, but should use Settings::SetUploadsEnabled()
instead.
TEST=crashpad_snapshot_test \
CrashpadInfoClientOptions.*:MachOImageReader.Self_DyldImages; \
run_with_crashpad --handler crashpad_handler \
-a --database=/tmp/crashpad_db \
-a --url=https://clients2.google.com/cr/staging_report \
-a --annotation=prod=crashpad \
-a --annotation=ver=0.7.0 \
crashy_program
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/997713002
Likewise for EXPECT_DEATH_CHECK() and EXPECT_DEATH().
In the in-Chromium build configured for official builds in Release mode,
CHECK() throws away its condition string and stream parameters without
ever printing them, although it still evaluates the condition and
triggers death appropriately. {ASSERT,EXPECT}_DEATH(statement, regex)
will not work correctly for any regex that attempts to match what
CHECK() prints. In these build configurations,
{ASSERT,EXPECT}_DEATH_CHECK() use a match-all regex (""). In other build
configurations, they transparently wrap {ASSERT,EXPECT}_DEATH().
BUG=crashpad:12
R=rsesek@chromium.org, scottmg@chromium.org
Review URL: https://codereview.chromium.org/992693003
A couple of the problems related to not having a C++11 library:
- You can’t put const elements into a std::vector<>, so
CrashReportDatabase::GetPendingReports() and
CrashReportDatabase::GetCompletedReports() need to change. There was
no data-safety benefit to const elements.
- std::string::pop_back() does not exist, another mechanism must be
used to trim strings in BreakpadHTTPFormParametersFromMinidump().
One relates to a feature that does not exist in 10.6:
- The O_CLOEXEC flag to open() was introduced in 10.7. Although it
would be possible to use fcntl(..., F_SETFD, FD_CLOEXEC) on 10.6, the
O_CLOEXEC behavior is just removed from
CrashReportDatabaseMac::ObtainReportLock(), in line with other open()
calls in Crashpad.
And one was a real bug:
- #define __STDC_FORMAT_MACROS before #including <inttypes.h> to get
format macros like SCNx32, used in UUID::InitializeFromString().
TEST=* (gyp_crashpad.py -Dmac_sdk=10.6 -Dmac_deployment_target=10.6)
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/987693004
As there are no extended file attributes available on all Windows file
systems (NTFS supports alternate data streams, but Chrome still supports
running on FAT), instead of using metadata attached to the file, metadata
is stored separately in a simple record-based file and keyed by UUID.
Initially, I attempted a metadata file beside each report, each locked
separately more closely mirroring the Mac implementation. But given the
expected number of of active reports (in the 10s to 100s range?) and the
size of the metadata for each, simply storing it all in one file is much
less complicated when considering error situations.
If the serialization/deserialization becomes a measurable problem, it
could be optimized at some complexity by reading/writing only as
necessary, or optimizing the storage.
R=mark@chromium.org, rsesek@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/867363003
e.g.
FAILED: ninja -t msvc -e environment.x86 -- "C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\bin\amd64_x86\cl.exe" /nologo /showIncludes /FC @obj\util\misc\util_test.clock_test.obj.rsp /c ..\..\util\misc\clock_test.cc /Foobj\util\misc\util_test.clock_test.obj /Fdobj\util\util_test.cc.pdb
d:\src\crashpad\crashpad\third_party\mini_chromium\mini_chromium\base\basictypes.h(49) : error C2220: warning treated as error - no 'object' file generated
d:\src\crashpad\crashpad\util\misc\clock_test.cc(72) : see reference to function template instantiation 'To implicit_cast<uint64_t,double>(const From &)' being compiled
with
[
To=uint64_t
, From=double
]
d:\src\crashpad\crashpad\third_party\mini_chromium\mini_chromium\base\basictypes.h(49) : warning C4244: 'return' : conversion from 'const double' to 'uint64_t', possible loss of data
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/807653002
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
amount of time spent sleeping.
Even with the slop, this wound up being flaky, specifically on virtual
machines. And guess what our automated test infrastructure runs on?
TEST=util_test Clock.SleepNanoseconds
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/640373003
This includes ClockMonotonicNanoseconds() and SleepNanoseconds().
SleepNanoseconds() is like base::PlatformThread::Sleep(), but
PlatformThread is not in mini_chromium and I’m not keen on adding it
because I’m not sold on the interface. I’m not convinced Sleep() belongs
there, and I don’t want to have to bring all of base::Time* along for
the ride.
TEST=util_test Clock.*:MachMessageServer.*:ServiceManagement.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/597533002
