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
