Crash Handler #13
Draft
Crash Handler #13
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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]>
meta: update handler 2023-05-03
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]>
…ity_statically fix: statically check arch API compatibility
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 <[email protected]>
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 <[email protected]> Reviewed-by: Ivan Penkov <[email protected]>
- 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 <[email protected]>
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 <[email protected]>
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 <[email protected]> Reviewed-by: Mark Mentovai <[email protected]>
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 <[email protected]>
- RISCV32 will only include support for 32 bit floating point registers - RISCV64 will only include support for 64 bit floating point registers - RISCV 32/64 context will include a "version" field to account for future extensions Fixed: 1447862 Tested: `make check` on x86 host Tested: `minidump_stackwalk` for RISCV64 minidump on x86 host Change-Id: I605d5b2c35e627a5dc986aaf818a9c9898f6ae0b Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4553281 Reviewed-by: Joshua Peraza <[email protected]>
Change-Id: I30fab42e2a1e7d0abf970b825e66a0db8b6a0fd5 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4557444 Reviewed-by: Joshua Peraza <[email protected]>
Bug: b/280290608, chromium:1448979 Change-Id: I3f9e4c3d62b4c858238ccbbda0366926c306e27f Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4568824 Reviewed-by: Joshua Peraza <[email protected]>
The hex formatting in MinidumpCrashpadInfo::Print() was missing the leading 0, so byte values < 128 were not possible to decode. Change-Id: Ib355bcdaf86e91d644045df645fb4fa75332aa4b Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4571100 Reviewed-by: Joshua Peraza <[email protected]>
…file. The debug info in the dwp file needs to refer to the .debug_line and .debug_line_str sections in the main binary. This fixes dump_syms not generating LINE records for dwp in split dwarf. Bug: chromium:1448979 Change-Id: I71923f12cea72caae081c1406e2cbca55e95859e Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4576346 Reviewed-by: Joshua Peraza <[email protected]>
Bug: chromium:1448979 Change-Id: Ib174ab1592d189e0f05e6baa6a96af2742d00eda Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4580929 Reviewed-by: Joshua Peraza <[email protected]>
Fix warnings on Ubuntu 22.04 Change-Id: I2f64988706e72838b4e2cec50d0bde9eb90929ad Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4668734 Reviewed-by: Mike Frysinger <[email protected]>
Support for zstd must be enabled by passing --enable-zstd to configure. Change-Id: I57d0196552284de86575d979d673ac20a3fc4d64 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4722191 Reviewed-by: Joshua Peraza <[email protected]>
The _tmp buffer used in STRNCATF is too small for several callers,
which might lead to truncated output in some situations.
For example, GCC 11 warns:
src/third_party/libdisasm/x86_format.c:899:40: warning: ‘%s’ directive output may be truncated writing up to 63 bytes into a region of size 32 [-Wformat-truncation=]
899 | STRNCATF( buf, "%s:", str, len );
| ^~~~~ ~~~
src/third_party/libdisasm/x86_format.c:34:38: note: in definition of macro ‘STRNCATF’
34 | snprintf( _tmp, sizeof _tmp, fmt, data ); \
| ^~~
src/third_party/libdisasm/x86_format.c:899:41: note: format string is defined here
899 | STRNCATF( buf, "%s:", str, len );
| ^~
In file included from /usr/include/stdio.h:894,
from src/third_party/libdisasm/x86_format.c:1:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:71:10: note: ‘__builtin___snprintf_chk’ output between 2 and 65 bytes into a destination of size 32
71 | return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
72 | __glibc_objsize (__s), __fmt,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
73 | __va_arg_pack ());
| ~~~~~~~~~~~~~~~~~
Change-Id: Ia876e288bf9629f2c72db3faf2287c7940924ea0
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4668735
Reviewed-by: Mike Frysinger <[email protected]>
Functions such as FindElfSection and FindElfSegments that inspect the ELF header expect a pointer to the first byte of the file. IsValidElf() checks for the ELF magic number at offset 0. Thus, we must map ELF object files from offset 0. Change-Id: Icebfb46229a04019f57a7ec07844257b98ceb278 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4674337 Reviewed-by: Mike Frysinger <[email protected]>
Bug: chromium:1137393 Change-Id: I1a6a5f2013e6a08e189958b89415183ffb6fe345 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4722972 Reviewed-by: Mark Mentovai <[email protected]>
Python 2 is deprecated and have now been removed from CI builders. Change-Id: Ic838714502e16136bd8ed345a47a00b71ff889aa Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4754416 Reviewed-by: Mike Frysinger <[email protected]>
Change-Id: I400130d67acea47158b9ba4f5703c7e9bc2cbb79 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4811217 Reviewed-by: Joshua Peraza <[email protected]>
… (StackFrame) to derived-class (StackFrameARM64). Inline frames are always of the base-class type (StackFrame). Treating them as derived-class and accessing members is causing buffer overflows. Change-Id: Ib41b74256e6162e7d2b14ca3905dfaf5591b9c86 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4847317 Reviewed-by: Joshua Peraza <[email protected]>
meta: update handler 2023-10-02
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To better separate our patches to the processor (in the
masterbranch) and the client used insentry-native.This branch contains patches for the client / handler only.