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 2023-05-03 #82

Merged
merged 27 commits into from
May 4, 2023
Merged

Conversation

supervacuus
Copy link
Collaborator

No description provided.

Justin Cohen and others added 25 commits February 15, 2023 18:13
Bug: 1415371
Change-Id: I9e1bd902494a664d4f07829e686803712fa8e7a8
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4255568
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Justin Cohen <[email protected]>
The test BaseAnnotationShouldNotSupportSpinGuard assumed NDEBUG builds
always disabled DCHECK()s, but DCHECK_ALWAYS_ON overrides this.

This CL fixes the test for NDEDBUG + DCHECK_ALWAYS_ON builds by using
the DCHECK_IS_ON() macro to skip the test when DCHECKs are enabled.

Change-Id: I7b64729568c5d3139ca777e27462d81eba931834
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4255429
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Ben Hamilton <[email protected]>
This is the only change needed to build crashpad against musl, yay! The
reason this change is needed is that user_vfp is bionic-specific, and
does not exist in glibc, dietlibc, uclibc, or musl.

I have not (yet) tried running the tests against another libc.

Bug: chromium:1380656
Change-Id: I2247352e1611a300dff995156d393508c8257039
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4255370
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Elly Fong-Jones <[email protected]>
Change-Id: I5a29f690a4512252d0d5730492f7fd4cec16ffaa
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4262547
Commit-Queue: Justin Cohen <[email protected]>
Reviewed-by: Mark Mentovai <[email protected]>
…RECORD

The EXCEPTION_RECORD contains a NumberParameters field, which could
store a value that exceeds the amount of space allocated for the
ExceptionInformation array.

Bug: chromium:1412658
Change-Id: Ibfed8eb6317e28d3addf9215cda7fffc32e1030d
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4284559
Reviewed-by: Alex Gough <[email protected]>
Commit-Queue: Robert Sesek <[email protected]>
Adds the dump_minidump_annotations tool (modified from jperaza's WIP
code). This works similarly to Breakpad's minidump_dump tool, but:

1. Is available on Windows
2. Only dumps simple/vectored annotations and annotation objects instead
   of the entire minidump contents.

Current use case for this is to be able to get a minidump's process
type on Windows without having to go through symbolization, but there
may be other use cases in the future.

Bug: chromium:1006331
Change-Id: I392024e230c10ea18673b3cf0d0ad4793d21f5eb
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4287994
Reviewed-by: Joshua Peraza <[email protected]>
Commit-Queue: Brian Sheedy <[email protected]>
In order to determine in crash_reporter whether a crash was fatal, we
need the exception number (-1 is not an actual crash).

BUG=b:269159625
TEST=deploy to DUT; chrome://crashdump; verify metadata present.

Change-Id: I83d3c9cc839a685af2f50d143d627cf9fcfaf3ac
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4265253
Reviewed-by: Joshua Peraza <[email protected]>
Commit-Queue: Miriam Zimmerman <[email protected]>
It's not required that LC_SOURCE_VERSION be present in every module, and
common for it to be missing. Suppress recording its absence.

Fixed: crashpad:443
Change-Id: Iae10c38c78514e78af6c3176cc809d95a3ae3811
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4294861
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Justin Cohen <[email protected]>
StringToInt(string_piece) works because base::StringPiece is in
namespace base, but when it is switched to std::string_view, this won't
work anymore. Use the idiomatic spelling.

Bug: chromium:691162
Change-Id: Ic45e0d2729fa5fc7c3e7a56fe159957b1bdcdf94
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4298113
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Robert Sesek <[email protected]>
Lacros can be up to 2 milestones ahead of ash (and consequently the
platform code), so until the crash_reporter change has been in for 2
milestones, we need to manually check version compatibility.

BUG=chromium:1420445
TEST=Build, deploy, check that flag is set only on right version

Change-Id: Ic99d5ac58840814f7eeecd47c628ea0e8107f675
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4308129
Commit-Queue: Mark Mentovai <[email protected]>
Reviewed-by: Mark Mentovai <[email protected]>
The windows property of UIApplication is unavailable in iOS15.

Bug: 1406561 crashpad:
Change-Id: I19642067a13801142cd3f24586bab6958a81635d
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4304398
Reviewed-by: Justin Cohen <[email protected]>
Commit-Queue: Joemer Ramos <[email protected]>
This also significantly simplifies the implementation, since we don't
really need the ThreadLogMessagesMaster class at all.

Bug: chromium:1416710
Change-Id: I85849230015f901dfbf084d140e639f14cb872a7
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4313281
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Peter Kasting <[email protected]>
ObjcExceptionPreprocessor is a 'reasonable effort' attempt to catch an
NSException minidump at time the exception is thrown as opposed to when the application terminates due to the exception. If multiple
exceptions are thrown at the same time, Crashpad should correctly
report the final uncaught exception, but the minidump may not
represent the full `caught-at-thrown` minidump.

 - Don't assume ObjcExceptionPreprocessor throws an NSException.
 - Don't retain/release the exception. Instead of calling isEqual,
   just use a simple pointer comparison.
 - Make last_exception atomic.

Bug: crashpad: 445, 446
Change-Id: I9f2f2041e96aa9818c63937025e507487ae9d03d
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4317110
Reviewed-by: Ben Hamilton <[email protected]>
Commit-Queue: Justin Cohen <[email protected]>
Reviewed-by: Robert Sesek <[email protected]>
Missed this the first time around because it was Windows-only.

Bug: chromium:691162
Change-Id: Ic98a5943957f77fbf17d92a93409eaa35910ae0e
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4297482
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Robert Sesek <[email protected]>
The extra_memory cap in ProcessSnapshotTest.CrashpadInfoChild is not
high enough to avoid test failures on all machines. The actual amount
recorded has been seen to vary between 726,556 and 1,152,803. This
change rases the limit from 1,000,000 to 1,200,000 to avoid the
failures.

The highest amount was seen on a 64-GB gWindows ThinkPad laptop.

Instrumentation shows that the low and high cases both have 104 threads.
The low case has 304 ExtraMemory() blocks, whereas the high case has
409. In both cases the sizes range from 384 to 6,024.

Change-Id: I8873921fa913c31445384db34d4aa90200401a4a
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4348802
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Bruce Dawson <[email protected]>
Several tests in filesystem_test.cc create symbol links. The privilege
needed to do this is not enabled on all Windows systems so several of
the tests check for the privilege and are skipped if it is not
available.

However, two tests that created symbol links were not doing this check
and therefore failed on some Windows machines. This corrects those
failures by adding the checks.

Bug: chromium:1418165
Change-Id: I6621796b462b8db02271ad5a05e0c29ee047f648
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4348801
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Bruce Dawson <[email protected]>
After https://reviews.llvm.org/D141222 exceptions call into
__libcpp_verbose_abort, which Chromium sets to `brk 0` in release.

Bug: 1425429
Change-Id: Ie00d1317bb03fcb1f15fb5c41ab69640dfb564b7
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4347775
Reviewed-by: Robert Sesek <[email protected]>
Commit-Queue: Justin Cohen <[email protected]>
Pointer Authentication works by adding a signature to the top bits of
an instruction or data pointer (only instruction pointers on the stack
are currently signed in Chromium). This can confuse range checks,
because they need to strip the top bits. Masking these bits during sanitization range checks prevents confusion.


Test: Testing was done manually on a device with pointer authentication enabled.
Bug: crashpad:364
Bug: 919548
Change-Id: I2e739cadb2844cfaf73a75596d664135aeb5faac
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4387271
Commit-Queue: Adam Walls <[email protected]>
Reviewed-by: Joshua Peraza <[email protected]>
Reviewed-by: Ben Hamilton <[email protected]>
Always reset the file descriptor to -1, even if FlushWriteBuffer or
RawLoggingCloseFile fails.

Bug: 1431760
Change-Id: I193f526d65f477bba002dd9faf68996020e48a3b
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4406657
Reviewed-by: Ben Hamilton <[email protected]>
Commit-Queue: Justin Cohen <[email protected]>
Reviewed-by: Robert Sesek <[email protected]>
__has_feature is a clang extension. GCC errors out on the test.
Define a helper macro to make the code working with other compilers.

Bug: chromium:819294
Change-Id: I359150acd4700e65b4faf5f297b29664c18000d3
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4418706
Reviewed-by: Joshua Peraza <[email protected]>
Commit-Queue: Joshua Peraza <[email protected]>
Reviewed-by: Ben Hamilton <[email protected]>
In the recent llvm upstream change, https://reviews.llvm.org/D148269,
clang becomes smarter and will remove the infinite recursion function.
Use the clang attribute __attribute__((optnone)) to disable optimization
for it.

Bug: chromium:1435016
Change-Id: I74e823bf64d0b03d81c0bda7a8338e2fa67033aa
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4456156
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Zequan Wu <[email protected]>
Remove the reference to `base/cxx17_backports.h` from the code.

Bug: chromium:1373621
Change-Id: I84dd5fc1b069b168e4558316344c1f1c5377a68b
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4471860
Commit-Queue: Mark Mentovai <[email protected]>
Reviewed-by: Mark Mentovai <[email protected]>
Also: clean up and fix the failing tool-build on macOS.
@supervacuus
Copy link
Collaborator Author

I added all of you as reviewers because it makes sense, but also because it is not 100% clear whether we are all yet in owners-native (and it seems that group cannot be assigned in this repo) or if this will be the way forward (@ashwoods?).

Just in case this hasn't been communicated yet, the idea was that we four would be owners-native (I don't know who is now) or something similar if that group has another meaning, so it is easier to address y'all on GitHub, but also for @Swatinem to drop out of the group whenever it makes the most sense.

@ashwoods ashwoods requested a review from a team May 4, 2023 08:30
@ashwoods
Copy link

ashwoods commented May 4, 2023

@supervacuus I checked that owners-native has Markus, Stefan, Sebastian, and Arpad as members, and I could set it in the PR.

@supervacuus
Copy link
Collaborator Author

@supervacuus I checked that owners-native has Markus, Stefan, Sebastian, and Arpad as members, and I could set it in the PR.

I guess it is a permission issue then, because I can neither select nor deselect any groups.

@Swatinem Swatinem merged commit 1904ae1 into getsentry:getsentry May 4, 2023
@supervacuus supervacuus deleted the meta/update branch February 13, 2024 16:51
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.