-
Notifications
You must be signed in to change notification settings - Fork 2
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
Merge breakpad upstream #2
Conversation
Bug: chromium:1066980 Change-Id: Ie95754402ce30bbd4bfcfc0c0150f07d2e3008f6 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3055796 Reviewed-by: Nelson Billing <[email protected]>
The size of symbol file for chrome binary increased from 577 MB to 1205 MB. There are 7,453,748 INLINE records and 1,268,493 INLINE_ORIGIN records. Bug: 1190878 Change-Id: I802ec1b4574c14f74ff80d0f69daf3c81085778a Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2915828 Reviewed-by: Joshua Peraza <[email protected]>
The header is not present in earlier versions of split dwarf. Change-Id: I8fde233268230cea157b2b3276f3cf05190962f2 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3083253 Reviewed-by: Sterling Augustine <[email protected]>
Change-Id: I35d7a5e50537bd6f20bcb5a91d386ffee9325b18 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3098093 Reviewed-by: Joshua Peraza <[email protected]>
This is a follow-up to 3c70e01 to make -d work. Bug: chromium:1190878,chromium:1238693 Change-Id: Ie0c6c663c98491462fca1aa992503037f19cefa9 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3103526 Reviewed-by: Joshua Peraza <[email protected]>
Building fails for some people because configure requires c++11 but make_unique is a c++14 feature. Change-Id: I23ce689fc92e9e90a95e7643ff29602f6b32ccbb Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3107784 Reviewed-by: Joshua Peraza <[email protected]>
Dwarf generated by Clang -g1 will not have DW_AT_inline attribute for some DW_TAG_subprograms even if they are inlined. This warning recently increased a lot (~ 3 million) due to DW_TAG_inlined_subroutine also complains about unknown abstract origin. It caused infra failure in building bots. Bug: 1241579 Change-Id: I9b5135925b71aa915760c140bcf73fc603bb77d3 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3111782 Reviewed-by: Joshua Peraza <[email protected]>
Use range-based for-loops where appropriate. Change-Id: I2fffd270d434c90850e8151ee40e5adf0736ce55 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3120666 Reviewed-by: Joshua Peraza <[email protected]>
This allows INLINE_ORIGIN records appears in after FUNC records. Change-Id: I69b8b5948ed91453e15c7f4c3888dfbe38e7bc5c Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3132381 Reviewed-by: Joshua Peraza <[email protected]>
Break statements immediately following returns are unreachable. Bug: chromium:1246232 Change-Id: I0892a66617f7b27b5e317a7d9741f5fcd19249f2 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3140192 Reviewed-by: Robert Sesek <[email protected]>
Temporarily works around an issue on Mac where the system version of NXGetLocalArchInfo is returning x86 information on x86_64 devices, which results in dump_syms failing on said devices. Instead, the Breakpad implementation of NXGetLocalArchInfo, which is meant for dump_syms_mac on Linux, will be used until the system version is fixed. Bug: 1242776 Change-Id: Id398338e580eb9c67c61f9f01670d2e7dbe86bea Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3143524 Reviewed-by: Joshua Peraza <[email protected]>
The app will check if process_architecture is ARM64_OLD which is 0x8003 but newman is a new arch which is ARM64 (0x12) We can fix the issue by checking both values Test: "/google/src/cloud/zyanwu/latest/google3/blaze-bin/chrome/dongle/platform/tools/minidump --crash_report_id=49ed111b84c0736e --crash_server=crash --build_number=265669 --build_branch=1.56 --product=newman-user --eureka_root=/usr/local/google/home/zyanwu/eureka --symbol_cache_dir=/usr/local/google/home/zyanwu/android/debug/symbols --debug" can work and it can convert the minidump to core dump then load gdb. Bug: 199144156 Change-Id: I1590a5b617e55ae8347aad426ba5b636ff6dcdfb Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3146740 Reviewed-by: Sterling Augustine <[email protected]> Reviewed-by: Nelson Billing <[email protected]>
This change makes sure dump_syms process DW_TAG_inlined_subroutine only when -d flag is given, which save memory and time when -d is not given. Before this, it always processes DW_TAG_inlined_subroutine and -d determines whether or not to emit INLINE records. Bug: chromium:1250351, chromium:1246974 Change-Id: I54725ba1e513cafe17268ca389ff8acc9c11b25e Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3166674 Reviewed-by: Joshua Peraza <[email protected]>
Change-Id: I83a2d026f1cef1771d28b420d76de17f0cf296ec Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3166678 Reviewed-by: Joshua Peraza <[email protected]>
It moves InlineOriginMap to module.h. Let Module keeps the global InlineOriginMap to easily get all referenced InlineOrigin when emitting. And release allocated memory inside its destructor. Verified that the symbol file with inline records for chrome is the same before and after this change. Change-Id: I7541aa05d3d2df0b9d52d670cab58241baecf20d Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3171638 Reviewed-by: Joshua Peraza <[email protected]>
Change-Id: I3904d52e946158439899f4c5aaa92d1d15160745 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3183519 Reviewed-by: Ivan Penkov <[email protected]>
- Added StringView which is used as a reference to a string, but doesn't own the string. - Removed the old string pool in DwarfCUToModule::FilePrivate, since it's doing string copy. - Added a string pool in Module to store functions/inline origins' names (mangled and demangled). - The peak memory usage drops from 20.6 GB to 12.5 GB when disabling inline records and drops from 36 GB to 20.3 GB when enabling inline records. Bug: chromium:1246974, chromium:1250351 Change-Id: Ie7e9740ea10c1930a0fc58c6becaae2d718b83b8 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3189410 Reviewed-by: Joshua Peraza <[email protected]>
The context arguments are of type DWORD_PTR which is actually a integer type, not a pointer, so using NULL here causes a type missmatch warning: error: passing NULL to non-pointer argument 8 [...] Change-Id: Ia52f51fd0cd33af3b139f0427dec6c59c2455d0a Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3168663 Reviewed-by: Primiano Tucci <[email protected]>
After ff5892c added the new StringView, building fails with GCC 6 due to it apparently failing to properly find the type for nullptr_t resulting in the following error: In file included from ../src/common/module.h:49:0, from ../src/common/dwarf_cfi_to_module.h:49, from ../src/common/linux/dump_symbols.cc:59: ../src/common/string_view.h:55:27: error: field 'nullptr_t' has incomplete type 'google_breakpad::StringView' StringView(nullptr_t) = delete; ^~~~~~ ../src/common/string_view.h:42:7: note: definition of 'class google_breakpad::StringView' is not complete until the closing brace class StringView { ^~~~~~~~~~ This can be fixed by adding the std:: namespace to nullptr_t. Change-Id: I00a090d307ebe21d1143eac4a605ff319ce27048 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3201997 Reviewed-by: Joshua Peraza <[email protected]>
The probot app we were using has been shutdown, so switch over to the new GH actions flow. Change-Id: Ifa8c2835e1ac1a4df53a5c4f0aa851fbacbd4096 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3217681 Reviewed-by: Mark Mentovai <[email protected]>
With Travis shutdown, convert our flows over to GH actions. Change-Id: Ia4d358dbbf3d8a73c347f4b9e4cd4637ce44e594 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3216116 Reviewed-by: Mark Mentovai <[email protected]>
Keeps us in sync with Chromium a bit better. Change-Id: I4cb80f28fc3aa2e3d0cd8637dd2a5b1ff4ae633d Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3223799 Reviewed-by: Mark Mentovai <[email protected]>
Change-Id: I4c6a6fb353cacb09710c579e59332d70d1e801a8 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3093129 Reviewed-by: Mark Mentovai <[email protected]>
Change-Id: I468f19048f6b48b230913e911d0da7a20d96cae8 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3222826 Reviewed-by: Mark Mentovai <[email protected]> Reviewed-by: Nelson Billing <[email protected]>
…ew at https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3189410 Change-Id: I258863e5de6201bc24b53dbe50b4d2515d29e338 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3221513 Reviewed-by: Mike Frysinger <[email protected]>
Bug: chromium:794619 Change-Id: I7edb70a915ffb3c6f945dce77b0bd913e32e85eb Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3229392 Reviewed-by: Mark Mentovai <[email protected]>
Processor shows incorrect source file name if a frame have an inlined frame and their source files are different. Consider this example: FILE 0 /tmp/a.h FILE 1 /tmp/a.cpp INLINE_ORIGIN 0 0 foo() FUNC 1110 a 0 main INLINE 0 22 0 1110 7 1110 7 3 0 1117 3 23 1 When querying the address 0x1110, we know this line 0x1110 corresponds to /tmp/a.h line 3 and it's inside a inlined function foo() which is defined at /tmp/a.h and called at line 22. But we don't know at which file it's being called at line 22. So, we will get stacks like this: void foo() /tmp/a.h:3 int main() /tmp/a.h:22 The correct stacks should be this: void foo() /tmp/a.h:3 int main() /tmp/a.cpp:22 In this change: 1. Remove file_id field for INLINE_ORIGIN record. 2. Add call_site_file_id for INLINE record to represents the file where this call being inlined. After adding call_site_file_id to it (as third field), it looks like this: FILE 0 /tmp/a.h FILE 1 /tmp/a.cpp INLINE_ORIGIN 0 foo() FUNC 1110 a 0 main INLINE 0 22 1 0 1110 7 1110 7 3 0 1117 3 23 1 Bug: 1190878 Change-Id: Ibbb697d2f7e1b6ac3208cac6fae4353c8743198d Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3232838 Reviewed-by: Joshua Peraza <[email protected]>
Introduces Arm's Pointer Authentication and Branch Target Identification to breakpad. The changes are similar to changes for PA/BTI to Marl, see google/marl#204 Bug: 1145581 Change-Id: I6a770316ad333bfcfad2ce7f3c1ff78afb35c010 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3226471 Reviewed-by: Primiano Tucci <[email protected]>
This reverts commit 54d878a. 54d878a changed the dump_syms format incompatibly. This must be redone in a multi-step process: the processor must be made to understand the old and new formats simultaneously and the processor service must be rebuilt and run with that update before dump_syms output can change to use the new format. Bug: chromium:1263390 Change-Id: I5b6f8aff8ea2916b2c07ac6a74b569fa27db51b9 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3244775 Reviewed-by: Joshua Peraza <[email protected]>
…ORIGIN This is similar to the processor part of https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3232838/, but added compatibility to process both old and new format of INLINE/INLINE_ORIGIN records in symbol file. Old INLINE format: INLINE <inline_nest_level> <call_site_line> <origin_id> [<address> <size>]+ New INLINE format: INLINE <inline_nest_level> <call_site_line> <call_site_file_id> <origin_id> [<address> <size>]+ Old INLINE_ORIGIN format: INLINE_ORIGIN <origin_id> <file_id> <name> New INLINE_ORIGIN format: INLINE_ORIGIN <origin_id> <name> Change-Id: I555d9747bfd44a1a95113b9946dcd509b7710876 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3248433 Reviewed-by: Joshua Peraza <[email protected]>
These are reimported from Apple's Github source drops, see exact provenance in README. Most were imported as is, some were edited to match previous versions, and as noted below - Added arm headers where needed - Removed (now) unused `/mach/i386/vm_param.h` - Removed availability annotations - Removed `__kernel_ptr_semantics` - Added `defined(__aarch64__)` to all arm64 define guards Bug: chromium:1420654, google-breakpad:880, b/257505171 Change-Id: I17bd03fa871a8f1dc4285daafa3d7b26c2186e2b Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4482294 Reviewed-by: Mark Mentovai <[email protected]>
The NXArch* family is deprecated in macOS 13. This change: - Uses the replacements where available - Silences deprecation warnings otherwise - Removes the Linux cross-compile shims in favor of having completely separate implementations for Mac and non-Mac. The logic of the Linux versions uses the same prepopulated data as before, but they no longer use NXArchInfo. clang diagnostic disables are necessary due to https://crbug.com/1406057 Bug: chromium:1420654, google-breakpad:880, b/257505171 Change-Id: Iad777915a5a058551cfb3a7d3cf681cce180dfea Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4437109 Reviewed-by: Mark Mentovai <[email protected]>
Use MD_CONTEXT_AMD64_DEBUG_REGISTERS instead of MD_CONTEXT_AMD64_DEBUG_REGISTERS in the definition of MD_CONTEXT_AMD64_ALL. This previously happened to work because the two flags happened to have the same values and every includer of minidump_cpu_amd64.h also happened to previously include minidump_cpu_x86.h. Change-Id: If8b422d3623936f4a0b57a4cf6dac4f348daa024 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4480251 Reviewed-by: Joshua Peraza <[email protected]>
Use the exception record's context for the crashed thread instead of the thread's own context. For the crashed thread the thread's own context is the state inside the exception handler. Using it would not result in the expected stack trace from the time of the crash. This change aligns the behavior of minidump-2-core with the behavior of minidump_stackwalk. Bug: google-breakpad:885 Change-Id: I5cd3e9d39807308491b64fcd335f5f85b1dcd084 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4473128 Reviewed-by: Joshua Peraza <[email protected]> Reviewed-by: Joshua Peraza <[email protected]>
Bug: b/257505171 Change-Id: I210b6689683ff2cf561997584924fd9b568943cb Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4494631 Reviewed-by: Mark Mentovai <[email protected]>
Bug: None Change-Id: I0409a0c2ab8e60b1f84f72b50a1fd400b5a41cbd Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4500379 Reviewed-by: Mark Mentovai <[email protected]>
Bug: chromium:1420654 Change-Id: Id0281089962147040b6332223bf4593bf4fc60cd Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4500259 Reviewed-by: Mark Mentovai <[email protected]>
MDRawModuleCrashpadInfoList::modules is a flexible array of MDRawModuleCrashpadInfoLink and not MDLocationDescriptor. Breakpad does not currently use the MDRawModuleCrashpadInfoList type, but its definition should be updated to reflect the correct type to avoid confusion. Change-Id: If97f490db8d41529b59a225a275a37116746c2b7 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4504150 Reviewed-by: Joshua Peraza <[email protected]>
MDRawCrashpadAnnotationList::objects is a flexible array of MDRawCrashpadAnnotation and not MDLocationDescriptor. Breakpad does not currently use the MDRawCrashpadAnnotationList type, but its definition should be updated to reflect the correct type to avoid confusion. Change-Id: I58b5b0e4f7f95bc003b103e2750e3759c3e31292 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4503630 Reviewed-by: Joshua Peraza <[email protected]>
dump_syms was using x0...x31 notation, while the rest of Breakpad was using the ABI names. This mismatch was causing stackwalking to not fully succeed. Fixed: 1432426 Change-Id: I0713e76e65ff6dad492b51bc3607e94e25dc2c3a Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4505156 Reviewed-by: Joshua Peraza <[email protected]>
case MD_CONTEXT_RISCV: | ||
{ | ||
const MDRawContextRISCV* context_riscv = context->GetContextRISCV(); | ||
// memory = GetActualStackMemory(context_riscv->__gregs[MD_CONTEXT_RISCV_REG_SP], | ||
// memory, memory_list); | ||
cpu_stackwalker = new StackwalkerRISCV(system_info, | ||
context_riscv, | ||
memory, modules, | ||
frame_symbolizer); | ||
break; | ||
} | ||
|
||
case MD_CONTEXT_RISCV64: | ||
{ | ||
const MDRawContextRISCV64* context_riscv64 = context->GetContextRISCV64(); | ||
// memory = GetActualStackMemory(context_riscv64->__gregs[MD_CONTEXT_RISCV_REG_SP], | ||
// memory, memory_list); | ||
cpu_stackwalker = new StackwalkerRISCV64(system_info, | ||
context_riscv64, | ||
memory, modules, | ||
frame_symbolizer); | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only conflict (out of two) I could not resolve: haven't figured out where the stack-pointer register is.
Sentry's |
@Jake-Shadle how do you want to proceed here and in EmbarkStudios/sentry-contrib-rust#24? |
* breakpad-sys: Update `breakpad` submodule with merged upstream EmbarkStudios/breakpad#2 * Update submodule * Fix formatting * Remove mac * Fix mac compilation * Fix mac compilation harder --------- Co-authored-by: Jake Shadle <[email protected]>
It's ever so slightly painful that the squash merge now does not track when we last integrated the (And the "x commits behind upstream" text is also wrong - and how about Sentry's own fork of Google Breakpad? 😰) |
Oops. |
https://github.com/EmbarkStudios/breakpad/compare/main..MarijnS95:breakpad:merge-upstream contains the remaining changes of merging I can help address that if you want :) |
Sure. |
@Jake-Shadle done, the branch is identical to this original branch with the latest https://github.com/MarijnS95/breakpad/compare/rebase-upstream..merge-upstream |
@Jake-Shadle do you still intend to go the rebase-route? |
Yes, need a PR |
@Jake-Shadle sure, if you reset this repo to the upstream branch first, make sure you then rebase-merge instead of squash-merge it 😬 |
Done in #3 :) |
Probably more good reasons than just this, but my Arch install upgraded to GCC 13 yesterday and that triggers:
Upstream already has a solution at getsentry@7ea7ded, so we just merge in main completely.
I'm not sure if we should also go through the sentry fork, or do a clean rebase. Alternatively we can also pick just the patch that fixes the build?