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

Rebase on top of upsteam #3

Closed
wants to merge 249 commits into from

Conversation

MarijnS95
Copy link

Per #2 this rebases all the (picked) patches on top of https://github.com/getsentry/breakpad/tree/handler

Direct diff until this repo's main branch is reset to the handler branch of Sentry's repo, allowing this PR to be merged: getsentry/breakpad@handler...MarijnS95:rebase-upstream

Swatinem and others added 30 commits April 12, 2021 16:19
meta: Update client/handler code to 2021-06-14
Bug: chromium:1066980
Change-Id: Ie95754402ce30bbd4bfcfc0c0150f07d2e3008f6
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3055796
Reviewed-by: Nelson Billing <nbilling@google.com>
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 <jperaza@chromium.org>
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 <saugustine@google.com>
Change-Id: I35d7a5e50537bd6f20bcb5a91d386ffee9325b18
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3098093
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
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 <jperaza@chromium.org>
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 <jperaza@chromium.org>
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 <jperaza@chromium.org>
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 <jperaza@chromium.org>
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 <jperaza@chromium.org>
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 <rsesek@chromium.org>
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 <jperaza@chromium.org>
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 <saugustine@google.com>
Reviewed-by: Nelson Billing <nbilling@google.com>
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 <jperaza@chromium.org>
Change-Id: I83a2d026f1cef1771d28b420d76de17f0cf296ec
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3166678
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
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 <jperaza@chromium.org>
Change-Id: I3904d52e946158439899f4c5aaa92d1d15160745
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3183519
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
- 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 <jperaza@chromium.org>
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 <primiano@chromium.org>
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 <jperaza@chromium.org>
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 <mark@chromium.org>
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 <mark@chromium.org>
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 <mark@chromium.org>
Change-Id: I4c6a6fb353cacb09710c579e59332d70d1e801a8
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3093129
Reviewed-by: Mark Mentovai <mark@chromium.org>
Change-Id: I468f19048f6b48b230913e911d0da7a20d96cae8
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3222826
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Nelson Billing <nbilling@google.com>
bungeman and others added 26 commits May 3, 2023 19:25
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 <jperaza@chromium.org>
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 <jperaza@chromium.org>
…ompatibility_statically

fix: statically check arch API compatibility
Some minidumps contain invalid thread memory descriptors pointing to memory
regions other than the ones referred to by the RSP. This is the case in UE4, for
example. This causes the stackwalker to terminate early as it cannot load the
actual stack memory.

Other Minidump processors such as WinDbg or the Visual Studio debugger can still
process such minidumps correctly. They do not load the stack memory region based
on the information obtained from the TEB and instead just follow the stack
walker.

This patch adds the same behavior to the minidump processor. If the thread
memory region does not cover the stack pointer, the region is replaced with the
one covering it, if any.
…os#3)

When we are reading a minidump of different endianness we need to swap
the bytes of the integer fields of the GUID to build the correct
debug_id.  This because the debug_id is the hex representation of the
big-endian GUID.

When reading a little-endian minidump this does not swap, but since
the bytes are then implicitly swapped by reading them as little-endian
we end up converting their big-endian representation to hex.
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 <jperaza@chromium.org>
Change 4505156 changed the RISCV register names, this change adjusts
the unittest to match the new names.

Bug: 1432426
Change-Id: I0887d8fc11eec63ab6953ea1a136873591e49286
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4507066
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
This adds a new flag `enable_objdump_for_exploitability_` to the
MinidumpProcessor, which allows enabling objdump separately for crash
address fixups and for exploitability analysis, as the performance cost
of the exploitability analysis is significantly higher.

Change-Id: I667ffdce7cc0a970793f91413c3d2e3af93f4247
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4507067
Reviewed-by: Ivan Penkov <ivanpe@google.com>
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
- Replace DISALLOW_COPY_AND_ASSIGN with =delete.
- Replace some NULLs with nullptrs;
- Use the override keyword when appropriate.
- Use =default when appropriate.

Change-Id: I99e1d7f349dd4c32aa5d05e2ebdce7a86e47f551
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4527718
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
macOS caps filenames at 255 characters. When upload_system_symbols runs
`dump_syms`, the resulting filename is based on a mangled version of
the file's full path. In some circumstances (for example, the dumped
file itself lives in a temp directory), this name can exceed the max.

This change replaces the current mangling by mapping each path component but the last to its first initial, greatly shortening
the resulting filename.

Bug: 1400770
Change-Id: I68203a98eda2912893c5d8f7c676faee17e39e91
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4519231
Reviewed-by: Robert Sesek <rsesek@chromium.org>
It fixes following two problems:
1. When we have skeleton compilation unit (DW_TAG_skeleton_unit) in a
binary file refers to the complete unit in a split dwarf file
(.dwo/.dwp file), we should use the split dwarf file's path in warning
reporting. Right now, it uses the original file (binary file) path in
warning report, which is incorrect.

For example, if we have chrome.debug which is the binary with skeleton
debug info and chrome.dwp which is the complete debug info and the debug
info in chrome.dwp has some incorrect reference, it will warn on
chrome.debug rather than chrome.dwp

2. When split dwarf is enabled, the global inline_origin_map will likely
encounter key collision because the offsets as keys are now relative to
each CU's offset which is relative to .debug_info section. Also
offsets from different files might collide.

This change makes a inline_origin_map for each debug file and use
offsets only relative to .debug_info section as keys.

Bug: b/280290608
Change-Id: If70e2e1bfcbeeeef2d425c918796d351a0e9ab3b
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4544694
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Test: arm softfp build, crashed program intentionally with kill -4 and
observed successful minidump generation.
Bug: b/283473162
Change-Id: Id71f92653ced04575ffbb87e309d4139ca34d843
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4545508
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
@github-actions
Copy link

Thanks for your contribution! Unfortunately, we don't use GitHub pull requests to manage code contributions to this repository. Instead, please see README.md which provides full instructions on how to get involved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet