Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

meta: Update crashpad handler to 2022-07-18 #69

Merged
merged 21 commits into from
Jul 18, 2022
Merged

Conversation

Swatinem
Copy link
Member

This also adds CMake definitions for the new WER handler

backes and others added 21 commits June 14, 2022 14:25
Sanitizers can prevent the installation of signal handlers, but
sigaction would still return 0 (for success). Detect this by checking
the installed signal handler via a second call to sigaction.

R=mark@chromium.org

Bug: chromium:1328749
Change-Id: I62a5777379ec5c6b1ca2d5a62e7cd3fb8ed1437b
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3702302
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Fuchsia's Crashpad roller was broken due to uninitialized fields in
structs.

Bug: fxbug.dev/101498
Change-Id: I1283afea9c5ac4eddb432590f9a5ec5cb1856a7c
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3704517
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Alex Pankhurst <pankhurst@google.com>
The Chromium presubmits flagged a missing #include in
process_reader_win_test.cc.  This adds the missing #include.

Change-Id: I68aed4328f976bba547a0cb7a9ea833fdf71873b
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3703312
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
Importing Crashpad into Chromium revealed a few build failures:

1) The MSVC compiler needed assistance constructing SleepingThreads

2) scoped_set_thread_name_posix.cc did not build on Android, where
   BUILDFLAG(IS_LINUX) is not defined and __ANDROID_API__ must be
   set to 24 or higher to use pthread_getname_np()

This fixes the build failures, which I tested with a Chromium CQ
dry-run:

https://crrev.com/c/3703491

Change-Id: Ibde7cacaa45d384272890ea9b1ee2d707048ab03
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3703446
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
clang-format doesn’t work after week’s buildtools update to 0a14d52
without separately checking out buildtools/clang_format/script.

Change-Id: I8330aacb85d1ba96318e5f2cd4563b6d32615963
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3707851
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
The DoubleForkAndExec() function was taking over 622 milliseconds to run
on macOS 11 (BigSur) on Intel i5-1038NG7. I did some debugging by adding
some custom traces and found that the fork() syscall is the bottleneck
here, i.e., the first fork() takes around 359 milliseconds and the
nested fork() takes around 263 milliseconds. Replacing the nested fork()
and exec() with posix_spawn() reduces the time consumption to 257
milliseconds!

See libuv/libuv#3064 to know why fork() is so
slow on macOS and why posix_spawn() is a better replacement.

Another point to note is that even base::LaunchProcess() from Chromium
calls posix_spawnp() on macOS -
https://source.chromium.org/chromium/chromium/src/+/8f8d82dea0fa8f11f57c74dbb65126f8daba58f7:base/process/launch_mac.cc;l=295-296

Change-Id: I25c6ee9629a1ae5d0c32b361b56a1ce0b4b0fd26
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3641386
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
Now that we have llvm-ml, we no longer need the workaround for this.

This upstreams
https://chromium-review.googlesource.com/c/chromium/src/+/3708412

Bug: chromium:762167
Change-Id: Iadc8ba9753bb7dd079415ee744f3b176b7e2f629
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3707748
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
This fixes a test case that accesses settings for the first time in
multiple threads simultaneously.

Fixed: crashpad:417
Change-Id: I6539682f171563f8ff5a1203fdd550ab92afc276
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3711807
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Fixes error: invalid operands to binary expression ('std::ostream'
(aka 'basic_ostream<char>') and 'const char[19]') << "pthread_setname_np";

in test/scoped_set_thread_name_posix.cc

Change-Id: I77eeeee9c828d563aaa15331733001e522a04642
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3714964
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
This reverts commit 460943d.

Reason for revert: This fails to compile in Chromium Android.
posix_spawn and posix_spawnp are available in Android NDK 28, but
Chromium is building with version 23.

https://ci.chromium.org/ui/p/chromium/builders/try/android_compile_dbg/1179765/overview

Original change's description:
> posix: Replace DoubleForkAndExec() with ForkAndSpawn()
>
> The DoubleForkAndExec() function was taking over 622 milliseconds to run
> on macOS 11 (BigSur) on Intel i5-1038NG7. I did some debugging by adding
> some custom traces and found that the fork() syscall is the bottleneck
> here, i.e., the first fork() takes around 359 milliseconds and the
> nested fork() takes around 263 milliseconds. Replacing the nested fork()
> and exec() with posix_spawn() reduces the time consumption to 257
> milliseconds!
>
> See libuv/libuv#3064 to know why fork() is so
> slow on macOS and why posix_spawn() is a better replacement.
>
> Another point to note is that even base::LaunchProcess() from Chromium
> calls posix_spawnp() on macOS -
> https://source.chromium.org/chromium/chromium/src/+/8f8d82dea0fa8f11f57c74dbb65126f8daba58f7:base/process/launch_mac.cc;l=295-296
>
> Change-Id: I25c6ee9629a1ae5d0c32b361b56a1ce0b4b0fd26
> Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3641386
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Commit-Queue: Mark Mentovai <mark@chromium.org>

Change-Id: I7f6161bc4734c50308438cdde1e193023ee9bfb8
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3719439
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
This is a reland of 460943d

Original change's description:
> The DoubleForkAndExec() function was taking over 622 milliseconds to run
> on macOS 11 (BigSur) on Intel i5-1038NG7. I did some debugging by adding
> some custom traces and found that the fork() syscall is the bottleneck
> here, i.e., the first fork() takes around 359 milliseconds and the
> nested fork() takes around 263 milliseconds. Replacing the nested fork()
> and exec() with posix_spawn() reduces the time consumption to 257
> milliseconds!
>
> See libuv/libuv#3064 to know why fork() is so
> slow on macOS and why posix_spawn() is a better replacement.
>
> Another point to note is that even base::LaunchProcess() from Chromium
> calls posix_spawnp() on macOS -
> https://source.chromium.org/chromium/chromium/src/+/8f8d82dea0fa8f11f57c74dbb65126f8daba58f7:base/process/launch_mac.cc;l=295-296

The reland isolates the change to non-Android POSIX systems because
posix_spawn and posix_spawnp are available in Android NDK 28, but
Chromium is building with version 23.

Change-Id: If44629f5445bb0e3d0a1d3698b85f047d1cbf04f
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3721655
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
Windows 7 doesn't support SetThreadDescription/GetThreadDescription. Add
an IsSupported to ScopedSetThreadName test to wrap unsupported calls.

Change-Id: I70d4e20b94efea03e41c5f7ed8d8e1b886192923
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3722556
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Changes copied verbatim from Chromium with one exception to remove
Chromium specific gn args.

This includes a mini_chromium roll to not codesign within Xcode.

Change-Id: I89b35bee08f9bc9e37f902f2b57e02acb2113ae1
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3726509
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
This adds a runtime exception helper (& test module) for Windows and
plumbing to allow the module to be registered by the crashpad client,
and to trigger the crashpad handler. Embedders can build their own
module to control which exceptions are passed to the handler.

See: go/chrome-windows-runtime-exception-helper for motivation.

When registered (which is the responsibility of the embedding
application), the helper is loaded by WerFault.exe when Windows
Error Reporting receives crashes that are not caught by crashpad's
normal handlers - for instance a control-flow violation when a
module is compiled with /guard:cf.

Registration:

The embedder must arrange for the full path to the helper to
be added in the appropriate Windows Error Reporting\
RuntimeExceptionHelperModules registry key.

Once an embedder's crashpad client is connected to a crashpad
handler (e.g. through SetIpcPipeName()) the embedder calls
RegisterWerModule. Internally, this registration includes handles
used to trigger the crashpad handler, an area reserved to hold an
exception and context, and structures needed by the crashpad handler.

Following a crash:

WerFault.exe handles the crash then validates and loads the helper
module. WER hands the helper module a handle to the crashing target
process and copies of the exception and context for the faulting thread.

The helper then copies out the client's registration data and
duplicates handles to the crashpad handler, then fills back the various structures in the paused client that the crashpad handler will need.

The helper then signals the crashpad handler, which collects a dump then
notifies the helper that it is done.

Support:

WerRegisterExceptionHelperModule has been availble since at least
Windows 7 but WerFault would not pass on the exceptions that crashpad
could not already handle. This changed in Windows 10 20H1 (19041),
which supports HKCU and HKLM registrations, and passes in more types of
crashes. It is harmless to register the module for earlier versions
of Windows as it simply won't be loaded by WerFault.exe.

Tests:

snapshot/win/end_to_end_test.py has been refactored slightly to
group crash generation and output validation in main() by breaking
up RunTests into smaller functions.

As the module works by being loaded in WerFault.exe it is tested
in end_to_end_test.py.

Bug: crashpad:133, 866033, 865632
Change-Id: Id668bd15a510a24c79753e1bb03e9456f41a9780
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3677284
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
GCC does not allow binding a packed field to an address. Assign
to a intermediate variable instead before pushing to map.

Bug: chromium:819294
Change-Id: I806e5f99c2b19e656b91a60f72172b59c961ba5f
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3751392
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
Fixed when running a simulator on arm64 Apple Silicon.

Change-Id: I6a6e917b4d5ff009683214794fe6a6af833be3c0
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3758362
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Adds a new IOSIntermediateDumpWriter::AddPropertyCString method which
takes an address to a cstring of unknown length and page-by-page
searches for a NUL-byte terminator.

This is necessary because currently WriteModuleInfo calls strlen
directly on the dyld and module filePath without first using vm_read.
On iOS14 this occasionally crashes, and is generally unwise. Instead,
use AddPropertyCString.

This patch also removes WriteDyldErrorStringAnnotation, as it's no
longer used going forward with iOS 15.

Bug: 1332862
Change-Id: I3801693bc39259a0127e5175dccf286a1cd97ba7
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3689516
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
On iOS, holding a lock during a slow upload can lead to watchdog kills
if the app is suspended mid-upload. Instead, if the client can obtain
the lock, the database sets a lock-time file attribute and releases the
flock. The file attribute is cleared when the upload is completed. The
lock-time attribute can be used to prevent file access from other
processes, or to discard reports that likely were terminated mid-upload.

Bug:chromium:1342051
Change-Id: Ib878f6ade8eae467ee39acb52288296759c84582
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3739019
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Rather than vm_reading each individual module load_command, load all of
the commands at once. This saves nearly 200ms on an iPhone 12 Pro.

Change-Id: I06f56c3ecbdf74f78759648ea62bcccd027f304c
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3764242
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
@Swatinem Swatinem requested review from a team July 18, 2022 11:05
@Swatinem Swatinem self-assigned this Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants