meta: update handler 2023-05-03 #35
Merged
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
If symupload client failed to connect the backend, we need this error message to be exposed. This could help the failure we are facing in official staging builders. BUG=chromium:1401761 TEST=NA Change-Id: Ic720aff9cb523c38553d6c02bf72aa5b95e862a7 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4240299 Reviewed-by: Nelson Billing <[email protected]>
Updates minidump_dump to print out any Crashpad annotation objects that are in a minidump. If an annotation contains a string value, it will be printed out as such, otherwise it will be printed out as hex bytes. Bug: crashpad:329 Change-Id: Ieecd6381c623f9011b16357742f7145a118dbc3c Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4261631 Reviewed-by: Joshua Peraza <[email protected]>
Changes a recent introduction of sprintf to snprintf since sprintf is deprecated in Chromium. Bug: crashpad:329 Change-Id: Icd346da4c86bd8e867266dfebaf617991dd90113 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4261633 Reviewed-by: Joshua Peraza <[email protected]>
Added
#ifdef HAVE_CONFIG_H
#include <config.h>
#endif
to the beginning of all source files that didn't have it.
This ensures that configuration options are respected in all source
files. In particular, it ensures that the defines needed to fix Large
File System issues are set before including system headers.
More generally, it ensures consistency between the source files, and
avoids the possibility of ODR violations between source files that were
including config.h and source files that were not.
Process:
Ran
find . \( -name third_party -prune \) -o \( -name '.git*' -prune \) -o \( \( -name '*.cc' -o -name '*.c' \) -exec sed -i '0,/^#include/ s/^#include/#ifdef HAVE_CONFIG_H\n#include <config.h> \/\/ Must come first\n#endif\n\n#include/' {} + \)
and then manually fixed up src/common/linux/guid_creator.cc,
src/tools/solaris/dump_syms/testdata/dump_syms_regtest.cc,
src/tools/windows/dump_syms/testdata/dump_syms_regtest.cc,
src/common/stabs_reader.h, and src/common/linux/breakpad_getcontext.h.
BUG=google-breakpad:877
Fixed: google-breakpad:877
TEST=./configure && make && make check
TEST=Did the find/sed in ChromeOS's copy, ensured emerge-hana google-breakpad
worked and had fewer LFS violations.
TEST=Did the find/sed in Chrome's copy, ensured compiling hana, windows, linux, and
eve still worked (since Chrome doesn't used config.h)
Change-Id: I16cededbba0ea0c28e919b13243e35300999e799
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4289676
Reviewed-by: Mike Frysinger <[email protected]>
It's deprecated in macOS 13/iOS 16, so this is an incremental step towards using newly introduced APIs for those OSes. Since the description field is no longer available in the new mach-o/util.h API, stop using it, especially since architecture name is sufficiently informative. Bug: chromium:1420654 Change-Id: If2cec4f1fc88d13a71f011822bff61f173486b68 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4322265 Reviewed-by: Mark Mentovai <[email protected]>
The only references to this are in derelict Xcode projects. Bug: chromium:1420654 Change-Id: If0d7064f32bab23630f79f459bb1dc429a203b88 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4329733 Reviewed-by: Mark Mentovai <[email protected]>
The added flag will print only one line per frame for the requesting thread (This is mostly the crashing thread). Refactor the code for printing the frame so it can be reused. Bug: 1374075 Change-Id: I8a1c8b1a09740fcaa23c3cc642468622ee64ea73 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4339771 Reviewed-by: Joshua Peraza <[email protected]>
Bug: 1374075 Change-Id: I1fb0f73b286625f3c99735e51418393af891a2b8 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4345752 Reviewed-by: Joshua Peraza <[email protected]>
Previously, dump_syms always used the basename of the on-disk file as the Breakpad module name and required that the on-disk filename of the dSYM and binary file match, or it would exit with an error. Build automation often uses filenames unrelated to the Breakpad module name, so this CL adds a new optional "-n MODULE" argument to Mac dump_syms that allows passing in the Breakpad module name from outside. In this case, the basename of the on-disk file(s) is ignored and no longer required to match. Change-Id: Ic38e8cf762c79bce61d289b397293eff6c0039ce Bug: b/273531493 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4338857 Reviewed-by: Robert Sesek <[email protected]>
`NXFindBestFatArch` is deprecated in macOS 13. We use this when an architecture is passed in via the `-a` flag. Unfortunately, neither of the potential replacements can help with this use case: - `macho_for_each_slice` as suggested in a reply to FB11955188 just enumerates slices, without the logic for inexact matches (for example, x86_64h -> x86_64 or arm64e -> arm64). - `macho_best_slice` as recommended by the deprecation notice only supports finding a suitable slice to run on the local machine. We could adapt the logic in `NXFindBestFatArch` but it gets quite complex for some architectures. Instead, this change adapts the `NXFindBestFatArch` polyfill used in `dump_syms_mac` for Linux, which returns an exact match if possible, and the first slice that matches the requested CPU type otherwise. I think this is probably Good Enough for most cases; if not, we can try porting the x86_64 and ARM logic and falling back to this for the rest. Change-Id: I3b269dab7246eced768cecd994e915debd95721a Bug: chromium:14206541420654 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4335477 Reviewed-by: Mark Mentovai <[email protected]>
`dump_syms` fails to write symbol file without knowing how to convert the ELF `e_machine` field to a string. Use "riscv" as the value because ELF `e_machine` does not distinguish between 32 bit and 64 bit RISC-V. Test: run `dump_syms` on the libc++ that's shipped with the Clang toolchain, or any other riscv binary: `./dump_syms -r -n libc++.so -o Fuchsia <clang_path>/lib/riscv64-unknown-fuchsia/libc++.so.2.0` Bug: fuchsia:124084 Change-Id: Ic04db96ec3d3d484350bdd0b90c9dfb70d7f7eb2 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4376828 Reviewed-by: Mike Frysinger <[email protected]>
RISC-V register names are needed in order to load DWARF call frame information. Bug: fuchsia:124084 Change-Id: I2791b3a38ea35ddc2bb293f60f75dcc86338e354 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4376827 Reviewed-by: Mike Frysinger <[email protected]>
Breakpad skips the xstate area in x64 contexts but allowed this area to be of unconstrained size. This hits problems if the size is greater than Chrome's maximum allocation size, so we change to skipping a maximum size. The maximum is chosen to allow the full set of states today, plus some slack for the future: Based on Intel x64 manual 13.5 XSAVE-MANAGED STATE * => further bytes might be reserved | Size | Region | | 576 | Legacy + header | | 384 | AVX State | | 80 | MPX State | | 1600 | AVX-512 State | | 72*| PT State | | 8 | pkru state | | 8 | pasid state | | 16 | CET state | | 8 | HDC State | | 96?| uintr state | | 808*| lbr state | | 8 | hwp state | | 16 | amx state | == 3680 so jump up a bit for the future to 2**12. Bug:1425631 Change-Id: Ie08555651977cdbfa1c351c661118f13238213c4 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4379497 Reviewed-by: Ivan Penkov <[email protected]>
Printing the register values as part of the stack trace relies on the CPU architecture being "riscv" or "riscv64" rather than the numeric identifiers (0x8005 and 0x8006, respectively). Fixed: 1432306 Test: Run `minidump_stackwalk` on a RISC-V minidump Change-Id: I0009da687438d51047e2ee39ffa1c50d78798caa Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4416399 Reviewed-by: Joshua Peraza <[email protected]>
…ble. Bug: 277976345 Change-Id: Iddf55d8e172f98c76ae7167f609fb53c4c60fa48 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4437089 Reviewed-by: Joshua Peraza <[email protected]>
Bug: 1435239 Change-Id: I4ea6cbe89d5ef0907f7e07c454e4533995996521 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4459351 Reviewed-by: Joshua Peraza <[email protected]>
…ismatch When built with -gmlt, .dSYMs are (by design) missing the `DW_AT_linkage_name` which Breakpad uses to fill out the (name-mangled) function names. Thankfully, the .dSYM contains both the old-school LC_SYMTAB command containing the STABS-format symbols (which include the fully-qualified C++ symbol names we want, but no actual compilation unit data), as well as the LC_SEGMENT_64 containing the __DWARF segment with the minimal -gmlt debug information (which excludes the name-mangled C++ symbols). Unfortunately, since the .dSYM's STABS does not define compilation units, the usual path in `StabsReader` ignores all the fully-qualified C++ symbol names for the functions: https://chromium.googlesource.com/breakpad/breakpad/+/bd9d94c70843620adeebcd73c243001237c6d426/src/common/stabs_reader.cc#100 Fortunately, when built for macOS platforms (`HAVE_MACH_O_NLIST_H`), `StabsReader` supports storing all the STABS-format symbols as `Extern`s, regardless of whether or not they're in a compilation unit: https://chromium.googlesource.com/breakpad/breakpad/+/bd9d94c70843620adeebcd73c243001237c6d426/src/common/stabs_reader.cc#119 Currently, when there's both a `Function` and an `Extern` with the same address, `Module` discards the `Extern`: https://chromium.googlesource.com/breakpad/breakpad/+/bd9d94c70843620adeebcd73c243001237c6d426/src/common/module.cc#161 This CL adds a new `-x` option to the Mac `dump_syms` which prefers the Extern function name if there's a mismatch. Bug: https://bugs.chromium.org/p/google-breakpad/issues/detail?id=883 Change-Id: I0d32adc64fbf567600b0a5ca63c71c422b7f0f8c Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4453650 Reviewed-by: Joshua Peraza <[email protected]>
Previously, the logic to mark a symbol as "multiple" would always fire for C++ symbols for Apple `.dSYM`s built with `-gmlt`. This was because for a C++ symbol like `void foo::bar::Baz()`, the DWARF data would contain the truncated function name `Baz`, but the STABS would contain the fully-qualified name `void foo::bar::Baz()`. This CL relaxes the name matching to not mark as multiple: 1) Symbols which were missing names entirely in the DWARF (e.g, "<name omitted">)` 2) Symbols whose fully-qualified name includes the truncated name as a substring Bug: https://bugs.chromium.org/p/google-breakpad/issues/detail?id=883 Change-Id: I26ded7ca84d964aa4a73da19e4bdd7e686e2c998 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4470047 Reviewed-by: Joshua Peraza <[email protected]>
This is a speculative fix for a memory bug where our symbol files are looking like they've grown enough that serializing them will outgrow UINT_MAX. Before this change a size_t is implicitly cast to a size_t in unsigned int, allocate a buffer of that size and then continue to write module data out of bounds. I have not been able to reproduce the OOB write locally as the original uploaded symbol data is gone, but I have been able to reproduce builds where, if we enable inline frames and CFI dumping, the size grows to 3.6GB when serializing it, which is close enough to 4.2GB that the wrapping theory seems reasonable on another board or build. No effort is made here to prevent wrapping behavior on 32-bit systems. Bug: b/237242489, chromium:1410232 Change-Id: I3d7ec03c51c298f10df3d5b1e5306433875c7919 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4477821 Reviewed-by: Leonard Grey <[email protected]> Reviewed-by: Mark Mentovai <[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]>
Swatinem
approved these changes
May 4, 2023
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.
No description provided.