aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAgeFilesLines
* Write field indicating multiple symbols at an address in dump_symsMike Wittman2017-12-118-31/+12
| | | | | | | | | | | Updates dump_syms to write the optional 'm' first field in FUNCTION and PUBLIC records to indicate that the address corresponds to more than one symbol. Bug: google-breakpad:751 Change-Id: I850b0122324ed5f9ec747aa92ba354a3126a7ef9 Reviewed-on: https://chromium-review.googlesource.com/820711 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Remove duplicate import.Adam Harrison2017-12-051-1/+0
| | | | | | | The mac exception_handler is included in a conditional below. Change-Id: I505fad7ef6731706a39b7aaacc9a948800fc3069 Reviewed-on: https://chromium-review.googlesource.com/809306 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Make iterator string types match map container string typesMike Wittman2017-12-011-2/+2
| | | | | | | | | Fixes a compilation error when ::string != std::string. Bug: Change-Id: Ifa782da65dd08973de1fc4215f658c798ae5160b Reviewed-on: https://chromium-review.googlesource.com/802324 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Add optional field indicating multiple symbols at an addressMike Wittman2017-11-2911-144/+310
| | | | | | | | | | | | | | | | | | Adds an optional 'm' as the first field in FUNCTION and PUBLIC records to indicate that the address corresponds to more than one symbol. Controls this by a command line flag for now to give symbol file users a chance to update. Also reduces the number of IDiaSymbols retained in memory to one per address. This reduces memory consumption by 8% when processing chrome.dll.pdb. Updates the processor to parse the new optional field. Bug: google-breakpad:751 Change-Id: I6503edaf057312d21a1d63d9c84e5a4fa019dc46 Reviewed-on: https://chromium-review.googlesource.com/773418 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Update test data for identical-code-folded symbol changesMike Wittman2017-11-286-6/+6
| | | | | | | Bug: google-breakpad:749 Change-Id: I2e56c8414c98c95372bd73811581cf1e98efe88e Reviewed-on: https://chromium-review.googlesource.com/791914 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Make identical-code-folded symbol output more consistent between runsMike Wittman2017-11-141-37/+43
| | | | | | | | | | | | | | Consistently output the "least" symbol by decorated name when multiple symbols share an address. Testing with chrome.dll.pdb the diffs between the new and old output look sensible, and this is actually ~20% faster than the existing implementation. Bug: 749 Change-Id: Ie638559b63f0eb2dcb80b1ebb579228d62c63bb2 Reviewed-on: https://chromium-review.googlesource.com/758885 Reviewed-by: Mark Mentovai <mark@chromium.org>
* List missing 64-bit arches in the bundled curlTomas Popela2017-11-131-1/+2
| | | | | | | | | | | Currently the bundled curl fails to build on ppc64/ppc64le or s390x, because it has an incomplete list of 64-bit arches (where long is 64-bit). Similar version is currently used as a downstream patch in Fedora https://src.fedoraproject.org/rpms/firefox/blob/master/f/build-ppc64-s390x-curl.patch Change-Id: Id27bfe1ca048340c45926f5435336941c080f132 Reviewed-on: https://chromium-review.googlesource.com/765453 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Add index-based set functionality to NonAllocatingMap.Robert Sesek2017-11-072-31/+79
| | | | | | | | | | This enables repeatedly setting a value based on index, which avoids a linear scan of the entry table after the first SetKeyValue(). Bug: chromium:598854 Change-Id: I9964670a09dcd8ff76180d031a373f20990bf4d8 Reviewed-on: https://chromium-review.googlesource.com/757579 Reviewed-by: Mark Mentovai <mark@chromium.org>
* dump_symbols: Stop rejecting files with Android packed relocation sections.Peter Collingbourne2017-11-071-31/+0
| | | | | | | | | | | | The lld linker has native support for creating packed relocation sections, and as a result we can expect files with these sections to have symbols. Bug: chromium:742655 Change-Id: I48a50bff041146f51b3a8b730d7a778f832787f6 Reviewed-on: https://chromium-review.googlesource.com/754239 Reviewed-by: Mike Frysinger <vapier@chromium.org> Reviewed-by: Mark Mentovai <mark@chromium.org>
* Create LongStringDictionary and replace SimpleStringDictionary on iOSYi Wang2017-11-037-21/+602
| | | | | | | | | | | This relands fd0a0d2b7ae9dd3d8a02b6a12e7941f7189fbb6c which was reverted in 5dad29423e62292c6ff468cabfee4422ba55b18b, with a fix for guarding kMaxSuffixLength which only used in assert()s with macros which breaks chromium.mac/ios-device. Change-Id: I5ee21b7f290517d6e7a0ef90b693b97f92392549 Reviewed-on: https://chromium-review.googlesource.com/751922 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Don’t set exit_after_write to false for tvOS.Adam Harrison2017-11-031-0/+2
| | | | | | | | | On tvOS, the app fails to shutdown after write. Allow exit_after_write to be false for tvOS in order to force an exit() after write. Change-Id: Ib2e1e1d03264a2972f5607b3070f4a6287aa0a98 Reviewed-on: https://chromium-review.googlesource.com/752071 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Revert "Create LongStringDictionary and replace SimpleStringDictionary ↵Mark Mentovai2017-11-027-599/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | usages in client/ios/Breakpad.mm." This reverts commit fd0a0d2b7ae9dd3d8a02b6a12e7941f7189fbb6c. Reason for revert: Build failures reported at https://chromium-review.googlesource.com/c/chromium/src/+/750591#message-cc4f7dd486fa1da7373ad5d83d56f550d607d429 Failed build on chromium.mac/ios-device: https://build.chromium.org/p/chromium.mac/builders/ios-device/builds/73163, https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.mac%2Fios-device%2F73163%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout [637/3593] CXX obj/third_party/breakpad/client/long_string_dictionary.o FAILED: obj/third_party/breakpad/client/long_string_dictionary.o […] ../../third_party/breakpad/breakpad/src/common/long_string_dictionary.cc:46:16: error: unused variable 'kMaxSuffixLength' [-Werror,-Wunused-const-variable] const size_t kMaxSuffixLength = 4; ^ 1 error generated. […] [641/3593] CXX ios_clang_arm64/obj/third_party/breakpad/client/long_string_dictionary.o FAILED: ios_clang_arm64/obj/third_party/breakpad/client/long_string_dictionary.o ../../third_party/breakpad/breakpad/src/common/long_string_dictionary.cc:46:16: error: unused variable 'kMaxSuffixLength' [-Werror,-Wunused-const-variable] const size_t kMaxSuffixLength = 4; ^ 1 error generated. Change-Id: I285eaac6abfcb7d173a0d1e4998b92d5c8dd6ecb Reviewed-on: https://chromium-review.googlesource.com/751723 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Rename an argument named "register" to "reg".Peter Collingbourne2017-11-021-5/+5
| | | | | | | | | | This silences a warning in newer versions of clang that complains about "register" being a deprecated keyword. Bug: chromium:780692 Change-Id: If354b9b18421e3e910849b385c44207e0ce02590 Reviewed-on: https://chromium-review.googlesource.com/750362 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Fix double declaration of tgkill when using Android NDK Headers.Nicholas Baldwin2017-10-301-7/+1
| | | | | | | | | | | | | As of Android API level 16 tgkill is declared in the NDK version of signal.h, which conflicts with the static definition found in src/client/linux/handler/exception_handler.cc. This change removes the static tgkill definition and replaces its use with sys_tgkill from the linux syscall support library. Bug: Change-Id: Ic70addd8a064cfa36345d86b7e36409e2089e909 Reviewed-on: https://chromium-review.googlesource.com/738912 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Create LongStringDictionary and replace SimpleStringDictionary usages in ↵Yi Wang2017-10-277-21/+599
| | | | | | | | | client/ios/Breakpad.mm. Bug: Change-Id: I401028f5d90417d79fb109b510aaa9660a039b44 Reviewed-on: https://chromium-review.googlesource.com/688301 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Avoid skipping an initializer with a gotoBruce Dawson2017-10-261-6/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | C++ doesn't allow skipping initialization with a goto. This means that this code is illegal: void func(bool b) { if(b) goto END; int value = 0; //error C2362 with /permissive- //... value used here END: return; } Adding an extra scope makes the code legal. This problem is only detected with /permissive- but now that compiling with this switch is practical we might as well stay /permissive- clean: https://blogs.msdn.microsoft.com/vcblog/2016/11/16/permissive-switch/ Note that compiling /permissive- clean only works with the 10.0.16299.0 SDK which currently has other issues... Bug: 773476 Change-Id: I54e64aaef46d70a817cf7da272f76d9ae5f6a6f7 Reviewed-on: https://chromium-review.googlesource.com/740287 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Provide helper wrappers for basename(3) and dirname(3)Tobias Sargeant2017-10-188-23/+113
| | | | | | | | | | This hides the need to provide mutable C strings, and unifies existing basename calls and variations in a single location. Change-Id: Idfb449c47b1421f1a751efc3d7404f15f8b369ca Reviewed-on: https://chromium-review.googlesource.com/725731 Reviewed-by: Mark Mentovai <mark@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Move main executable handling out of procmaps parser loop.Peter Collingbourne2017-10-171-19/+21
| | | | | | | | | | | | | | | | | | | | | | | | If the mapping for the main executable needed to be merged (for example, if it was linked with lld and therefore contains an r mapping followed by an r/x mapping), we would never reach the code that makes it the first module. Handle that situation by moving that code into a separate loop. This fixes an issue where breakpad_unittests fails on Android devices when linked with lld. It appears that the glibc dynamic loader happens to always load executables (or at least the executables that we create) at a lower address than DSOs, so we never hit this bug on desktop Linux. Testing: "make check" with both gold and lld as linker. Also breakpad_unittests when patched into Chromium on Linux (lld) and Android (gold and lld). Bug: chromium:469376 Change-Id: I6329e4afd2f1bf44c25a6c3e684495e21dba83a6 Reviewed-on: https://chromium-review.googlesource.com/722286 Reviewed-by: Mike Frysinger <vapier@chromium.org> Reviewed-by: Mark Mentovai <mark@chromium.org>
* Rename src/common/memory.h to memory_allocator.h.Ted Mielczarek2017-10-1713-16/+16
| | | | | | | | | | | | memory.h shadows a system header which normally isn't a problem because of the include paths in Breakpad, but the Firefox build system winds up with src/common in the include path so we've had a workaround for this for years. Renaming the file lets us get rid of that workaround and shouldn't hurt anything. Change-Id: I3b7c4239dc77f3b2b7cf2b572a0cad88cd7e8522 Reviewed-on: https://chromium-review.googlesource.com/723261 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Add -s flag to microdump_stackwalk for dumping stack contents.Tobias Sargeant2017-10-176-30/+32
| | | | | | | | | | | | | | | Note that the current MicrodumpProcessor::Process implementation has a bug due to the fact that it creates a local Microdump instance, and then holds onto a pointer to the object returned by microdump.GetMemory() which is destroyed when microdump goes out of scope. This CL fixes the crash by making Microdump outlive MicrodumpProcessor, which is the same pattern that Minidump/MinidumpProcessor uses. Bug: google-breakpad:748 Change-Id: I554b46d309649cf404523722bd9ee39e17a10139 Reviewed-on: https://chromium-review.googlesource.com/720809 Reviewed-by: Primiano Tucci <primiano@chromium.org> Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
* Convert {mini|micro}dump_stackwalk argument parsing to getopt.Tobias Sargeant2017-10-162-105/+131
| | | | | | | Bug: google-breakpad:748 Change-Id: I70b16ba6456df0be038d6c7170eb22b093fdc65d Reviewed-on: https://chromium-review.googlesource.com/718756 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* ios: Adds a no-Mach exception handlerAdam Harrison2017-10-123-0/+447
| | | | | | | | | | This exception_handler_no_mach does not use Mach for exception handling so that clients such as tvOS and watchOS that do not support mach messages can handle POSIX signals. Change-Id: I4a4574e58834bc590e110e6ecd1825f8af1437a2 Reviewed-on: https://chromium-review.googlesource.com/714276 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Android: Use sys/types.h instead of stdint.h for sys/user.hJoshua Peraza2017-10-061-1/+1
| | | | | | | | | | When using traditional headers, sys/types.h is needed to define __u64 for sys/user.h. Previously, we thought this would be provided by stdint.h, but it is not. Change-Id: I0e648712f4ef1e303104a5264d3d2d0b218f5d45 Reviewed-on: https://chromium-review.googlesource.com/705267 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Fix minidump_dump_test after 6d0287851fabMark Mentovai2017-10-051-1/+0
| | | | | | Change-Id: I9957f27cd134f862b9831e4b1d90f8a014eb37b6 Reviewed-on: https://chromium-review.googlesource.com/701740 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Dump Crashpad extension structures in minidump_dumpMark Mentovai2017-09-274-18/+463
| | | | | | | | | | | | | | | | | | | | | | | | | | This is currently mostly useful to expose the annotations that Crashpad stores in minidumps. Example output: MDRawCrashpadInfo version = 1 report_id = 01234567-89ab-cdef-0123-456789abcdef client_id = fedcba98-7654-3210-fedc-ba9876543210 simple_annotations["channel"] = canary simple_annotations["plat"] = OS X simple_annotations["prod"] = Chrome_Mac simple_annotations["ver"] = 59.0.3069.0 module_list[0].minidump_module_list_index = 0 module_list[0].version = 1 module_list[0].simple_annotations["ptype"] = crashpad-handler module_list[1].minidump_module_list_index = 28 module_list[1].version = 1 module_list[1].list_annotations[0] = abort() called Change-Id: I00ba291f93ea3a37fc3754c651b3ccc542e5b8b2 Reviewed-on: https://chromium-review.googlesource.com/688416 Reviewed-by: Robert Sesek <rsesek@chromium.org>
* Fix MSVC build on 64-bitOrgad Shaneh2017-09-257-17/+17
| | | | | | | | | | | Mostly int<->size_t implicit conversions. Warning 4366 (The result of the unary '&' operator may be unaligned) appears in minidump.cc:907, but I don't know why. It looks aligned to me. Change-Id: I641942adc324f8f9832b20662083dc83498688a8 Reviewed-on: https://chromium-review.googlesource.com/637390 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Refresh refresh_binaries.batOrgad Shaneh2017-09-241-12/+8
| | | | | | Change-Id: I15687f35e560eb1e25bb4d7483c8f6fe5fdf210e Reviewed-on: https://chromium-review.googlesource.com/637391 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Restore missing #include <stdint.h> to Android <sys/user.h>Mark Mentovai2017-09-201-0/+4
| | | | | | | | | | This was lost in afa9c52715db, but it turns out that it’s still necessary. Bug: google-breakpad:733 Change-Id: I4e0e4e4d2e80c22df1ff6b82e471905773c940a3 Reviewed-on: https://chromium-review.googlesource.com/675732 Reviewed-by: Joshua Peraza <jperaza@chromium.org>
* Replace remaining references to 'struct ucontext' with 'ucontext_t'Mark Mentovai2017-09-206-33/+33
| | | | | | | | | | | | | | | | | This relands https://chromium.googlesource.com/breakpad/breakpad/src/+/e3035bc406cee8a4d765e59ad46eb828705f17f4, which was accidentally committed to breakpad/breakpad/src, the read-only mirror of src in breakpad/breakpad. (Well, it should have been read-only.) See https://crbug.com/766164. This fixes issues with glibc-2.26. See https://bugs.gentoo.org/show_bug.cgi?id=628782 , https://sourceware.org/git/?p=glibc.git;h=251287734e89a52da3db682a8241eb6bccc050c9 , and https://sourceware.org/ml/libc-alpha/2017-08/msg00010.html for context. Change-Id: Id66f474d636dd2afa450bab925c5514a800fdd6f Reviewed-on: https://chromium-review.googlesource.com/674304 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Fix googletest/googlemock buildMark Mentovai2017-09-193-15/+15
| | | | | | | | | | | | | | 1. testing.gyp is a gyp file, not a gypi file. It is only referenced in “dependencies” sections. The gypi extension is used for files that are included by an “includes” section. 2. Update paths in testing.gyp to reflect the real locations of googletest and googlemock following their merge into a single repository. Change-Id: If9c356d93aa5ffda54af46fbed648baa2274dac6 Reviewed-on: https://chromium-review.googlesource.com/673404 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* android: Don’t compete with NDK API >= 21 over NDK structuresMark Mentovai2017-09-182-39/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Chrome uses API 16 for 32-bit builds and API 21 for 64-bit builds. The NDK’s <link.h> provides r_debug and link_map structure definitions only at API 21 and above. Breakpad used a custom <link.h> to define these structures only during 64-bit builds, which worked for Chrome’s purposes. However, other consumers may wish to build Breakpad at arbitrary API levels without regard to bitness. This alters Breakpad’s custom <link.h> to correctly check the NDK API level rather than target CPU bitness. Likewise for <sys/user.h> on 32-bit x86, which provided a typedef for user_fpxregs_struct to user_fxsr_struct. API 21 and above, as well as the unified headers at any API level, always name the structure user_fpxregs_struct. Definitions for 64-bit ARM’s user_regs_struct and user_fpsimd_struct have been removed from Breakpad’s copy of <sys/user.h>. The header claims that these fallback definitions are only necessary with NDK r10, which should no longer be in use even by Chromium, which now uses NDK r12b. This removes the Chromium-specific ANDROID_NDK_MAJOR_VERSION macro from use entirely. Fixes https://stackoverflow.com/questions/44141159/ and b/65630828. Bug: google-breakpad:733 Change-Id: I5841906297cd15b15ce48b73fd8332fd40afc9a0 Reviewed-on: https://chromium-review.googlesource.com/665740 Reviewed-by: Primiano Tucci <primiano@chromium.org> Reviewed-by: Joshua Peraza <jperaza@chromium.org>
* drop bundled gflags from the checkoutMike Frysinger2017-09-133-534/+2
| | | | | | | | | | | | The only code using gflags is google_crash_report_sender, and nothing builds or tests that code currently. Switch it over to using system versions of gflags so we can drop the local prebuilts. Tested local builds by hand of the tool. Bug: google-breakpad:360 Change-Id: I75d79b176468c948773079a54d87e70709feaf87 Reviewed-on: https://chromium-review.googlesource.com/665799 Reviewed-by: Mark Mentovai <mark@chromium.org>
* drop glog from the checkoutMike Frysinger2017-09-131-0/+0
| | | | | | | | | Nothing appears to be using this anymore, so stop bundling it. Bug: google-breakpad:360 Change-Id: Id95b36994379da92f8ef2a81754b3da5f1f79cae Reviewed-on: https://chromium-review.googlesource.com/665503 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Parse DWARF 4 line tables correctlyMark Mentovai2017-09-131-0/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Breakpad’s DWARF line table reader only understood line tables at the level of DWARF 2. This wasn’t a problem because LLVM only produced line tables at this level, even when generating DWARF 4. But LLVM would like to output DWARF 4 line tables when generating DWARF 4, and Breakpad needs to understand this format. (Meanwhile, it seems that GCC has used DWARF 4 line tables with DWARF 4 output since 4.5.0, 2010-04-14.) DWARF 3 line tables are fully compatible with DWARF 2 (assuming that nothing needs “prologue end,” “epilogue begin,” or “isa”, and opcodes related to these fields are properly skipped). DWARF 4 changes the line number program header slightly to include a “maximum operations per instruction” field. This field must be recognized, but can safely be ignored (and assumed to be always 1) if VLIW architectures are not supported (they aren’t). DWARF 4 also introduces a “discriminator”, whose opcode can also be skipped if these values are not needed (they shouldn’t be). This recognizes the “maximum operations per instruction” field when processing DWARF 4 line tables, but asserts that its value is 1 and otherwise ignores it. This is not compatible with VLIW architectures that set this field to a value other than 1. Such architectures are irrelevant to Breakpad, and mainline GCC and the proposed LLVM patch always set this field to 1. There are other things that could be extracted from DWARF 3 and 4 line tables that aren’t currently extracted (although these are currently irrelevant to Breakpad too). Bug: google-breakpad:745 Change-Id: I5bf9c0b1aa654849c9cce64e60682447d10be8ba Reviewed-on: https://chromium-review.googlesource.com/663441 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Increase the maximum number of modules allowed in minidumps.Tobias Sargeant2017-09-011-3/+3
| | | | | | | Bug: google-breakpad:743 Change-Id: I2e40b5cc36c012c18a1c4637634fb139b0d8e14d Reviewed-on: https://chromium-review.googlesource.com/647886 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Fix memory leak in ppc64 stackwalkerTobias Sargeant2017-08-301-2/+3
| | | | | | | | BUG=757166 Change-Id: I967a6903332b9c3d16b583f7fa4d3c9c44c2f729 Reviewed-on: https://chromium-review.googlesource.com/643267 Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
* Add crash reason extraction to microdump processorTobias Sargeant2017-08-215-1/+1457
| | | | | | | | BUG=754715 Change-Id: I00fe62ed06dbbab4c8f6c416d56e2d444be11571 Reviewed-on: https://chromium-review.googlesource.com/621307 Reviewed-by: Robert Sesek <rsesek@chromium.org>
* Fix memory leak in ppc stackwalkerTobias Sargeant2017-08-181-2/+3
| | | | | | | | BUG=756317 Change-Id: Id096372e5a0d1e7c70e95304b1f0c181f57d3882 Reviewed-on: https://chromium-review.googlesource.com/619126 Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
* Add crash reason and address to microdumps.Tobias Sargeant2017-08-144-0/+111
| | | | | | | | | | | | This will allow us to provide the right information for webview renderer crashes. At the moment the crash information for the browser process is captured (from the debuggerd output) instead. BUG=754715 Change-Id: I409546311b6e38fe1cf804097c18d7bb2a015d83 Reviewed-on: https://chromium-review.googlesource.com/612381 Reviewed-by: Robert Sesek <rsesek@chromium.org>
* Increase maximum number of regions for minidump_stackwalk.Lars Volker2017-07-281-1/+3
| | | | | | | | | | Change I361d8812df7b2977fe2630289059d31c3c9a4cc3 increased the maximum number of threads for minidump_stackwalk. This change also increases the maximum number of regions. Change-Id: I61efd4453df8809bd9cd657546d1d6727cd10281 Reviewed-on: https://chromium-review.googlesource.com/588384 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Handle very large stack tracesLeonard Mosescu2017-07-1214-81/+108
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The main motivation for this change is to handle very large stack traces, normally the result of infinite recursion. This part is actually fairly simple, relaxing a few self-imposed limits on how many frames we can unwind and the max size for stack memory. Relaxing these limits requires stricter and more consistent checks for stack unwinding. There are a number of unwinding invariants that apply to all the platforms: 1. stack pointer (and frame pointer) must be within the stack memory (frame pointer, if preset, must point to the right frame too) 2. unwinding must monotonically increase SP (except for the first frame unwind, this must be a strict increase) 3. Instruction pointer (return address) must point to a valid location 4. stack pointer (and frame pointer) must be appropriately aligned This change is focused on 2), which is enough to guarantee that the unwinding doesn't get stuck in an infinite loop. 1) is implicitly validated part of accessing the stack memory (explicit checks might be nice though). 4) is ABI specific and while it may be valuable in catching suspicious frames is not in the scope of this change. 3) is also an interesting check but thanks to just-in-time compilation it's more complex than just calling StackWalker::InstructionAddressSeemsValid() and we don't want to drop parts of the callstack due to an overly conservative check. Bug: chromium:735989 Change-Id: I9aaba77c7fd028942d77c87d51b5e6f94e136ddd Reviewed-on: https://chromium-review.googlesource.com/563771 Reviewed-by: Mark Mentovai <mark@chromium.org> Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
* A couple of minor fixesLeonard Mosescu2017-07-112-4/+4
| | | | | | | | | | | | | | | | 1. Fixing ExceptionHandlerTest.FirstChanceHandlerRuns: exit() is not an async-signal-safe function (http://man7.org/linux/man-pages/man7/signal-safety.7.html) 2. Fixing entry point signature in minidump_dump Changed "const char* argv[]" to "char* argv[]" to match the standard entry point signature 3. Updating .gitignore to exclude unit test artifacts Change-Id: I9662898d0bd97769621fb6476a720105821c60f0 Reviewed-on: https://chromium-review.googlesource.com/562356 Reviewed-by: Mark Mentovai <mark@chromium.org> Reviewed-by: Ivan Penkov <ivanpe@chromium.org> Reviewed-by: Joshua Peraza <jperaza@chromium.org>
* Fix asan buildsEric Holk2017-06-221-23/+23
| | | | | | | | | | | | | When rolling this into Chrome, we got compile failures due to DoNullPointerDereference being undefined but the new FirstChanceHandlerRuns tests depends on this and was still defined. The fix is to only enable the FirstChanceHandlerRuns test on non-asan builds. Bug: Change-Id: I5a3da0a21e2d0dd663ffc01137496d16905293a6 Reviewed-on: https://chromium-review.googlesource.com/544186 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Add first chance exception handler APIEric Holk2017-06-193-0/+44
| | | | | | | | | | | | | | This change adds the option for Breakpad hosts to register a callback that gets the first chance to handle an exception. The handler will return true if it handled the exception and false otherwise. The primary use case is V8's trap-based bounds checking support for WebAssembly. Bug: Change-Id: I5aa5b87d1229f1cef905a00404fa2027ee86be56 Reviewed-on: https://chromium-review.googlesource.com/509994 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Merge exec and non-exec segments while merging executable bit.Peter Collingbourne2017-05-261-3/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The bfd and gold linkers create segments like this: r/x, r/w where the r/x segment covers the start of the ELF file. lld's segments look like this: r, r/x, r/w where the r segment covers the start of the ELF file. So we cannot rely on the location of the r/x to tell where the start of the ELF is. But we can still rely on the r and r/x mappings being adjacent. So what we do is when we see an r segment followed by an r/x, merge the r into the r/x and claim that it is executable. This way, the minidump writer will continue to see a single executable segment covering the entire executable. Testing: "make check" passes when breakpad is compiled with lld compiled from trunk (requires bug fix from LLVM r303689). Also patched change into chromium and tested these builds: $ cat args.gn is_chrome_branded = true is_debug = false is_official_build = true use_lld = true allow_posix_link_time_opt = false is_cfi = false $ cat args.gn target_os = "android" target_cpu = "arm" is_debug = false is_official_build = true is_chrome_branded = true With both builds breakpad_unittests passes and chrome/chrome_modern_public_apk create good minidumps after navigating to chrome://inducebrowsercrashforrealz (checked that minidump contains stack trace entry for content::HandleDebugURL). Bug: chromium:716484 Change-Id: Ib6ed3a8420b83acf4a5962843930fb006734cb95 Reviewed-on: https://chromium-review.googlesource.com/513610 Reviewed-by: Primiano Tucci <primiano@chromium.org>
* Teach the ELF parser to handle multiple PT_NOTE phdrs.Peter Collingbourne2017-05-264-38/+76
| | | | | | | | | | | | | | | It is legal for an ELF to contain multiple PT_NOTEs, and that is in fact what lld's output looks like. Testing: "make check" and breakpad_unittests when patched into chromium. Bug: chromium:716484 Change-Id: I01d3f8679961e2cb7e789d4007de8914c6af357d Reviewed-on: https://chromium-review.googlesource.com/513512 Reviewed-by: Primiano Tucci <primiano@chromium.org> Reviewed-by: Ted Mielczarek <ted@mielczarek.org> Reviewed-by: Mark Mentovai <mark@chromium.org>
* Make the cross-compilation glue for dump_syms Mac handle x86_64h.Markus Stange2017-05-262-4/+25
| | | | | | | | | | x86_64h has a different cpusubtype from x86_64. The h is for Haswell. BUG= Change-Id: Icf884e5699fe120c12d13aa57cd62db5b69a2ce6 Reviewed-on: https://chromium-review.googlesource.com/457171 Reviewed-by: Ted Mielczarek <ted@mielczarek.org>
* Don't attempt to use PTRACE_GETREGS if it isn't defined.John Budorick2017-05-251-1/+5
| | | | | | | | | | | Follow up to https://chromium-review.googlesource.com/c/484479/, which does not compile on arm64. Bug: chromium:725754 Change-Id: Iaa6fbc332564909a10e2602a1026c14fb25625f4 Reviewed-on: https://chromium-review.googlesource.com/515044 Reviewed-by: Mark Mentovai <mark@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Simplify ELF parser code.Peter Collingbourne2017-05-254-40/+17
| | | | | | | | | | | | | The layout of Elf32_Nhdr and Elf64_Nhdr is the same, so remove templating and code that extracts the elfclass from the ELF file. Testing: "make check" and breakpad_unittests when patched into chromium. Bug: chromium:716484 Change-Id: I41442cfff48afc6ae1a5b604d22b67550a910376 Reviewed-on: https://chromium-review.googlesource.com/514450 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Wrap config.h include in HAVE_CONFIG_H.John Budorick2017-05-241-1/+4
| | | | | | | | Bug: breakpad:730 Change-Id: I5a24b96258e1114378061512239d3e18f3f753f0 Reviewed-on: https://chromium-review.googlesource.com/514283 Reviewed-by: Mark Mentovai <mark@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org>
generated by cgit v1.2.1 (git 2.18.0) at 2024-06-29 03:40:50 +0000