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

Sentry Patches #1

Open
wants to merge 547 commits into
base: master
Choose a base branch
from
Open

Sentry Patches #1

wants to merge 547 commits into from

Conversation

jan-auer
Copy link
Member

This PR contains patches for Sentry. Do not merge.

Swatinem pushed a commit that referenced this pull request Sep 10, 2020
__builtin_trap uses ud2 on x86_64, producing a SIGILL. On arm64, it uses
brk #1, producing a SIGTRAP. Test expectations must be adjusted
accordingly.

Bug: crashpad:345
Test: crashpad_snapshot_test MachOImageAnnotationsReader.CrashModuleInitialization, crashpad_util_test ExcServerVariants.*,ExceptionPorts.*
Change-Id: I22e75b7b48b8887031b1d95f1cea8a09733daf49
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2386464
Commit-Queue: Mark Mentovai <[email protected]>
Reviewed-by: Robert Sesek <[email protected]>
supervacuus pushed a commit to supervacuus/crashpad that referenced this pull request Dec 12, 2022
…Body

This bug was found when trying to upgrading the MSAN bots from Ubuntu
18.04 (where this codepath was not hit) to 20.04.  The following MSAN
error is produced when running HTTPTransport/HTTPTransport.*

==3496553==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x5616c540ad7d in __is_long buildtools/third_party/libc++/trunk/include/string:1674:33
    getsentry#1 0x5616c540ad7d in size buildtools/third_party/libc++/trunk/include/string:1069:17
    getsentry#2 0x5616c540ad7d in crashpad::(anonymous namespace)::HTTPTransportLibcurl::WriteResponseBody(char*, unsigned long, unsigned long, void*) third_party/crashpad/crashpad/util/net/http_transport_libcurl.cc:528:50
    ...
    SUMMARY: MemorySanitizer: use-of-uninitialized-value buildtools/third_party/libc++/trunk/include/string:1674:33 in __is_long
  ORIGIN: invalid (0). Might be a bug in MemorySanitizer origin tracking.

The memory is initialized in http_transport_test.cc:293, but MSAN gets
confused.  Given the message output by MSAN (ORIGIN: invalid (0).
Might be a bug in MemorySanitizer origin tracking), this appears
to be a bug in MSAN, not crashpad, so this CL suppresses the error.

Bug: chromium: 1260217
Change-Id: I2d6a46e3489816270cc1fee776793ffafe0147e4
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4015160
Reviewed-by: Joshua Peraza <[email protected]>
Commit-Queue: Thomas Anderson <[email protected]>
Swatinem and others added 28 commits December 14, 2022 10:39
To support a new crashpad::RingBufferAnnotation type which can be safely
written to and read from simultaneously by different threads/processes,
this CL introduces a new class ScopedSpinGuard, which is a simple RAII
wrapper around an atomic boolean.

Change-Id: I5bafe6927a8dc2a3e25734cb941fd9fce9a8d139
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4031729
Commit-Queue: Ben Hamilton <[email protected]>
Reviewed-by: Robert Sesek <[email protected]>
This CL implements LengthDelimitedRingBuffer, a general-purpose
ringbuffer suitable for use as a Crashpad Annotation.

This ringbuffer supports writing variably-sized data delimited by a Base
128 varint-encoded length separator.

LengthDelimitedRingBuffer is backed by a std::array, so it has a fixed
maximum size. It supports reading via RingBufferReader as well as
writing via RingBufferWriter.

Change-Id: I23ecb4a85ee8e846e1efc6937a5cb089a494d50a
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4023618
Reviewed-by: Robert Sesek <[email protected]>
Commit-Queue: Ben Hamilton <[email protected]>
GN now does more iOS xcode project generation natively.

Change-Id: I0a133e648a9426cfc822e40af36c2626ddb58f68
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4111089
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Justin Cohen <[email protected]>
Change-Id: I5511911110b58b7accd0f78cc1094924bfbda71e
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4114702
Reviewed-by: Justin Cohen <[email protected]>
Commit-Queue: Ben Hamilton <[email protected]>
Change-Id: I477919feec68d317ca3cb8a0d07022e9405156dd
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4118347
Reviewed-by: Justin Cohen <[email protected]>
Commit-Queue: Ben Hamilton <[email protected]>
... as we move crashpad from //third_party/crashpad to
//third_party/crashpad/src

Change-Id: I081520ad44334cc83397234e5d16535d0db4806d
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4132465
Reviewed-by: Francois Rousseau <[email protected]>
Commit-Queue: Jay Zhuang <[email protected]>
The try and catch macros were conditionally defined by libstdc++ pre-gcc
4.4 (2009-04-21), fixed in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25191. Surely none of this
code would build with such an old libstdc++ any more, since Crashpas has
adopted modern C++ (C++11 and later). Remove this obsolete nod to
history.

Change-Id: Ie3cea1ecc1cfd358f27ea48f8111791e7f08bfa5
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4136890
Reviewed-by: Robert Sesek <[email protected]>
Commit-Queue: Mark Mentovai <[email protected]>
... when compile target is Fuchsia

Change-Id: Id77babbd3dffff26c75a52296ac91101780e4c33
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4132464
Reviewed-by: Francois Rousseau <[email protected]>
Commit-Queue: Jay Zhuang <[email protected]>
... when build is targeting Fuchsia

Change-Id: I03fab9368bd5e8687899a2efeb51736707bce81e
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4137733
Commit-Queue: Jay Zhuang <[email protected]>
Reviewed-by: Francois Rousseau <[email protected]>
bind.h, callback.h, callback_forward.h, and callback_helpers.h
moved into /base/functional/. Update the include paths to
directly include them in their new location.

Bug: chromium:1364441
Change-Id: I23aaa16644c10ac5b607ea29d25799fbb08dfaca
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4163072
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Avi Drissman <[email protected]>
Co-authored-by: Mischan Toosarani-Hausberger <[email protected]>
Much of crashpad's unittests propagate tagged pointers to fuchsia
syscalls which do not accept tagged values. Rather than fixing them all
right now, just ensure that the tests do not build with the hwasan
variant if enabled.

Bug: fxbug.dev/108368
Change-Id: Ib32eb95ba671a6b55694075b68c7fbbb733cf501
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4175438
Reviewed-by: Francois Rousseau <[email protected]>
Commit-Queue: Francois Rousseau <[email protected]>
Bug: chromium:1373664
Change-Id: I948eac3a46e8ad0d6fe70413111641e77c7ae57c
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4196074
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Leonard Grey <[email protected]>
On both mingw and MSVC (when targeting < Windows 10)
It looks like macOS 13 only *sometimes* puts __crash_info in
__DATA_DIRTY. Instead of splitting by version check, let's just look
in __DATA_DIRTY if we can't find it in __DATA.

Bug: chromium:1372165
Change-Id: I99d2e759c66841d982039449e83f8658259d7ed1
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4197706
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Leonard Grey <[email protected]>
ARM64 supports storing pointer authentication codes in the upper bits of
a pointer. This mask can be used by LLDB to mimic ptrauth_strip and
strip the pointer authentication codes. To recover an address from
pointer with an authentication code, `AND` this mask with the pointer.

If the platform does not support pointer authentication, or the range of
valid addressees for a pointer was unaccessible, this field will be 0
and should be ignored.

Change-Id: Ie5cef90802dd1e892d456195ab8874223eac6a1b
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2773358
Commit-Queue: Justin Cohen <[email protected]>
Reviewed-by: Mark Mentovai <[email protected]>
This CL cleans up types and code style comments from post-submit code
review comments on https://crrev.com/c/4023618 .

I also added fixes for potential overflows in varint length decoding
and included new tests.

Bug: crashpad:437
Change-Id: I0a3585036028d81f42d0d36e87cce4264f4ed9ad
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4199705
Commit-Queue: Justin Cohen <[email protected]>
Reviewed-by: Justin Cohen <[email protected]>
Reviewed-by: Mark Mentovai <[email protected]>
This CL introduces a new class ScopedVMMap, a fork of ScopedVMRead
which maps the memory using vm_remap() instead of reading it.

This is useful for Annotations which use ScopedSpinGuard to
protect reads from simultaneous writes; the in-process intermediate
dump handler can try to take the spin guard when reading such
an Annotation and skip reading it if it the spin guard could not
be obtained.

Change-Id: I60d7a48d1ba4e5d2dfdb44307b78b4d9ffb73560
Bug: crashpad:437
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4114550
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Ben Hamilton <[email protected]>
from documentation of `ProcessException` in `crashpad_wer.h`:

```
//! \param[in] handled_exceptions is an array of exception codes that the helper
//!     should pass on to crashpad handler (if possible). Pass nullptr and set
//!     num_handled_exceptions to 0 to pass every exception on to the crashpad
//!     handler.
```

fix the check to handle `num_handled_exceptions == 0` case to not filter
out any exceptions.

Bug: crashpad:439
Change-Id: Ic4559a730a26e37c7a8f13e6bcae7595d743924a
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4206503
Commit-Queue: Alex Gough <[email protected]>
Reviewed-by: Alex Gough <[email protected]>
This CL optionally integrates ScopedSpinGuard (an atomic boolean) with
crashpad::Annotation.

Subclasses of Annotation can choose to integrate ScopedSpinGuard into
their Set(...) methods to ensure reads and writes are serialized.

I didn't integrate this into StringAnnotation in this CL, but it'd be
pretty trivial to do in a follow-up.

Change-Id: I1c5b8982576b03f9780a57acb7627c9194f8f0ff
Bug: crashpad:437
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4022484
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Ben Hamilton <[email protected]>
Reviewed-by: Robert Sesek <[email protected]>
This CL integrates the new ScopedSpinGuard with the new
LengthDelimitedRingBuffer into a new class, RingBufferAnnotation.

RingBufferAnnotation is thread-safe both for reading and writing, and is
suitable for streaming logs, trace events, and other high-throughput
data streams.

I included a load test (ring_buffer_annotation_load_test) which launches
two threads which simultaneously write to and read from the
RingBufferAnnotation.

By default, reads and writes are serialized using ScopedSpinGuard, but
passing the flag "--disable_spin_guard" to the test disables the spin
guard on the reading side (which is expected to make the test fail).

Change-Id: Ic8e28866d085d57e778c4f86bcb7492ef0638ab9
Bug: crashpad:437
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4023619
Reviewed-by: Robert Sesek <[email protected]>
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Ben Hamilton <[email protected]>
danakj and others added 25 commits May 7, 2024 18:02
[email protected]

Bug: 40284755
Change-Id: I2f491cc3f630767e607ea1df7d21791be522b4de
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5518909
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: danakj <[email protected]>
The memory region data saved in the intermediate dump is an address
offset by 128 bytes. However, the MemorySnapshot generated was using the
original address and not the offset address.  The same data is being
captured in the minidump.

Change-Id: Ia34912c035319e79cf446a130e662084f4ab51ea
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5529059
Commit-Queue: Justin Cohen <[email protected]>
Reviewed-by: Joshua Peraza <[email protected]>
Reviewed-by: Rohit Rao <[email protected]>
Bug: crashpad: 326459659,326458942,326459376,326459390,326459417,326458979,326459333,326459016,326458338,326458738,326459156,326459512,326458694
Change-Id: I04724530cbef50a8d3c18f306d16c0bbf3b0815b
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5512394
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Arthur Wang <[email protected]>
Change-Id: Ia328c6defc59fc06fcf69a2875a0eed335c8dd78
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5532142
Commit-Queue: Arthur Wang <[email protected]>
Reviewed-by: Mark Mentovai <[email protected]>
Change-Id: I486e7bcdb205de5b24846a8e782a6163c2ce8829
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5531439
Commit-Queue: Arthur Wang <[email protected]>
Reviewed-by: Mark Mentovai <[email protected]>
Bug: crashpad:326459219
Change-Id: Ic5e252285ed1fb1769eef67ddaea00a830d0ee6a
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5535006
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Arthur Wang <[email protected]>
Change-Id: I9756fa76f94cfcb9edc9f8df8ecfa5ff33918c22
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5542412
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Arthur Wang <[email protected]>
Bug: b:340659332
Change-Id: I4c2c82a1868d7a4f4a062a4e7a64258deedfb794
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5542248
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Joshua Peraza <[email protected]>
…lock

Previously, ScopedSpinGuard used std::atomic::compare_exchange_weak()
in a loop to implement a spinlock. After looping for the specified
number of nanoseconds, it would give up and return an error.

A few bugs have come in on ARM platforms (https://crbug.com/340980960,
http://b/296082201) which indicate that this can fail even in
single-threaded cases where nothing else has the spinlock.

From https://cbloomrants.blogspot.com/2011/07/07-14-11-compareexchangestrong-vs.html :

> compare_exchange_weak exists for LL-SC (load linked/store
> conditional) type architectures (Power, ARM, basically everything
> except x86), because on them compare_exchange_strong must be
> implemented as a loop, while compare_exchange_weak can be
> non-looping.

and:

https://en.cppreference.com/w/cpp/atomic/atomic/compare_exchange#Notes

> compare_exchange_weak is allowed to fail spuriously, that is, acts
> as if *this != expected even if they are equal. When a
> compare-and-exchange is in a loop, compare_exchange_weak will yield
> better performance on some platforms.
>
> When compare_exchange_weak would require a loop and
> compare_exchange_strong would not, compare_exchange_strong is
> preferable [...]

My conclusion is that this logic needs to use
`compare_exchange_strong` to avoid spurious failures on ARM in the
common case when there's no other thread holding the spinlock.

Change-Id: I2a08031db6b219d7d14a5cd02b3634985f81ab06

Bug: b:340980960
Change-Id: I2a08031db6b219d7d14a5cd02b3634985f81ab06
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5545257
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Ben Hamilton <[email protected]>
…6e1ea6 (2 commits)

https://chromium.googlesource.com/chromium/mini_chromium/+log/a6607b1fd76b..5856e1ea610f

$ git log a6607b1fd..5856e1ea6 --date=short --no-merges --format='%ad %ae %s'
2024-05-16 pbos Add NOTREACHED_IN_MIGRATION()
2024-05-11 pwjworks Remove workaround for missing std::atomic_thread_fence

Created with:
  roll-dep crashpad/third_party/mini_chromium/mini_chromium

Bug: chromium:40580068
Change-Id: I6d1385739042fa5b826bfa91c50b914eebaaf8fd
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5544927
Commit-Queue: Peter Boström <[email protected]>
Reviewed-by: Mark Mentovai <[email protected]>
This was generated by replacing "  NOTREACHED()" with
"  NOTREACHED_IN_MIGRATION()" and running git cl format.

This prepares for making NOTREACHED() [[noreturn]] alongside
NotReachedIsFatal migration of existing inventory.

Bug: chromium:40580068
Change-Id: Idb68e2fc8adba180350b0595fd494cf0f206bded
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5548246
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Peter Boström <[email protected]>
These are the last remaining jobs on the last remaining bionic pools.
Moving these over should let the bionic flex pool finally be decommed.

All of chrome/chromium has been on Ubuntu-22.04 for months now. So
hopefully this is transparent for crashpad.

Bug: chromium:40255350
Change-Id: I82828c0ae0c6efc8868b33779d7ebf5fb9b16116
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5550958
Commit-Queue: Ben Pastene <[email protected]>
Reviewed-by: Mark Mentovai <[email protected]>
Bug: crashpad: 480
Change-Id: Ie37c557d2170f6d96968ec4922ec52bfc6ad8136
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5580854
Reviewed-by: Mark Mentovai <[email protected]>
Co-authored-by: jernejstrasner <[email protected]>
* Add GAS version of capture_context_win_arm64.asm (for llvm-mingw)

* Use capture_context_win_arm64.S on mingw targeting arm64

* ci: tweak build matrix

* ci: add llvm-mingw x64 build

* ci: add llvm-mingw arm64 build

* compat/mingw/werapi.h: Fix compat with recent mingw headers

* ci: Bump actions/checkout to v4

To fix "Node.js 16 actions are deprecated"
The latest build rules have the ffuchsia-api-level.
Need https://crrev.com/c/5586319.

Bug: fuchsia:42085580, fuchsia:327691011
Change-Id: I21383e02f9fff3db9405c0dbe42051122a325003
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5585353
Commit-Queue: Zijie He <[email protected]>
Reviewed-by: Mark Mentovai <[email protected]>
This is a temporary workaround to avoid error like
https://ci.chromium.org/ui/p/crashpad/builders/try/crashpad_fuchsia_arm64_dbg/1299/overview

Bug:  41489832
Change-Id: Ia5404b5d7a2f01f53211fba7b8d602b9236ccbd3
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5599904
Commit-Queue: Takuto Ikuta <[email protected]>
Reviewed-by: Mark Mentovai <[email protected]>
goma is not maintained anymore.

Bug: 41489832
Change-Id: Ic82524205c7593ddba041c253e7d6283f9c89f33
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5599864
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Takuto Ikuta <[email protected]>
This reverts commit a63adda.

Reason for revert:
This is not necessary after https://crrev.com/c/5585353.

Original change's description:
> DEPS: fix version of fuchsia/sdk/core to avoid build error
>
> This is a temporary workaround to avoid error like
> https://ci.chromium.org/ui/p/crashpad/builders/try/crashpad_fuchsia_arm64_dbg/1299/overview
>
> Bug:  41489832
> Change-Id: Ia5404b5d7a2f01f53211fba7b8d602b9236ccbd3
> Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5599904
> Commit-Queue: Takuto Ikuta <[email protected]>
> Reviewed-by: Mark Mentovai <[email protected]>

Bug: 41489832
Change-Id: I2c6d3c3008047a6f79fb3bfddd8240a758ee645a
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5599914
Reviewed-by: Mark Mentovai <[email protected]>
Bot-Commit: Rubber Stamper <[email protected]>
Commit-Queue: Takuto Ikuta <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.