aboutsummaryrefslogtreecommitdiff
path: root/src/client/linux/minidump_writer
Commit message (Collapse)AuthorAgeFilesLines
* CopyFile: add a C++ APIMike Frysinger2020-09-071-1/+1
| | | | | | | | Having to swizzle to C strings all the time is a bit annoying. Change-Id: I0b80877706e32e873e567685f6b471745da70311 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2396557 Reviewed-by: Mark Mentovai <mark@chromium.org>
* fix pointer style to match the style guideMike Frysinger2020-07-1511-35/+35
| | | | | | | | | | We do this in a lot of places, but we're inconsistent. Normalize the code to the Google C++ style guide. Change-Id: Ic2aceab661ce8f6b993dda21b1cdf5d2198dcbbf Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2262932 Reviewed-by: Sterling Augustine <saugustine@google.com> Reviewed-by: Mark Mentovai <mark@chromium.org>
* Use breakpad_getcontext on all Linux platforms missing getcontextMichael Forney2020-03-161-0/+1
| | | | | | | | | | | | | | | | | | | | getcontext is also not available on musl libc, so generalize breakpad_getcontext so it can be used as a fallback for non-Android platforms as well. On x86_64 and i386, ucontext_t uses an Android-specific offset for storage of FP registers, since its sigset_t differs in size. So, make the definition of MCONTEXT_FPREGS_MEM and UCONTEXT_FPREGS_MEM_OFFSET conditional on whether we are building for Android. On glibc and musl, signal.h and asm/sigcontext.h can't be included together, so in breakpad_context_unittest.cc, only compare the libc and kernel _fpstate when on Android. Bug: google-breakpad:631 Change-Id: If81d73c4101bae946e9a3655b8d1c40a34ab6c38 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2102135 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Use public fpregset_t type to define fpstate_tMichael Forney2020-03-101-1/+2
| | | | | | | | | | | | | | | The _libc_fpstate struct tag is an implementation detail of glibc, and musl uses a different struct tag, _fpstate. Unfortunately, the public type fpregset_t is a pointer type, so is not suitable for allocating storage, or referring to constant storage. Instead, we can use std::remove_pointer<fpregset_t>::type to refer to the pointed-to type, regardless of the struct tag. Bug: google-breakpad:631 Change-Id: Iaf47f15b2d834dd8de839431f65a481e9b0c7f9e Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2096171 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Use standard header locations for poll.h and signal.hMichael Forney2020-03-102-2/+2
| | | | | | | | | | | | | | | | sys/poll.h and sys/signal.h just redirect to the standardized location for these headers, poll.h and signal.h. On musl libc, including the incorrect header path results in a warning, and since breakpad is built with -Werror, this is an error. In exception_handler.cc, signal.h is already included earlier, so we can drop the sys/signal.h include. Bug: google-breakpad:631 Change-Id: If36d326453e3267d38a5b92ed1301f828e46befe Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2097344 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Revert "Remove use of "register" keyword, deprecated in C++17"Mark Mentovai2019-11-011-1/+1
| | | | | | | | | | | | | | This reverts commit 07411862eaa9a9a38e84caed2e97e836e456656f. We were a bit overzealous in removing “register” here. Both clang and GCC correctly disallow “register” as a storage class specifier in C++17 mode by producing an error in ordinary use. However, they require “register” to be specified for explicit register variables, and do not produce an error in this case. Change-Id: I223f2652c6da4215d6e8788d902e767c94b8c29d Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1894875 Reviewed-by: Mark Mentovai <mark@chromium.org>
* linux, client: set module name from DT_SONAMEJoshua Peraza2019-10-301-17/+27
| | | | | | | | | | | | | | | | | | 3e56ef9d changed dump_syms to set the module name from DT_SONAME expecting that clients were already using DT_SONAME when it was present. The Breakpad client previously only used DT_SONAME as the name for a module if it detected that it was likely mapped from a zip file. This patch updates the Breakpad Linux client to always use the DT_SONAME in minidumps if it's present. Also included are changes to address comments that were missed from that review. Bug: 1016924 Change-Id: I4aae8c05e6793d4b0598049a8964ddd4cb0c6194 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1889231 Reviewed-by: Mark Mentovai <mark@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org>
* linux, dump_syms: set module name from DT_SONAMEJoshua Peraza2019-10-241-43/+0
| | | | | | | | | | | | | The Breakpad and Crashpad clients will use an object's DT_SONAME as the name for a module if it exists. Previously, linux dump_syms would assume the basename of an input elf file matches that value, causing symbol lookups to fail if they were mismatched. This patch updates dump_syms to use DT_SONAME as the module name, if present. Bug: 1016924 Change-Id: I5eff0cf06c703841df3fb552cb5a8e1e50a20c64 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1876763 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Remove use of "register" keyword, deprecated in C++17Mark Mentovai2019-08-121-1/+1
| | | | | | | | | | "register" as a storage class specifier has been deprecated since C++11, and has been removed from C++17 while remaining a reserved word. See C++17 5.11 and C.4.3. Change-Id: I2dbab8a7061cb680d902644d39ea1a7fbc930e5c Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1749329 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* linux: fix broken testsJoshua Peraza2019-06-041-2/+2
| | | | | | | | Fixes errors for comparisons between signed and unsigned ints. Change-Id: I7ef151ba84a48a0c8cc449cfaf12b9fef775d5a7 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1642361 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Linux breakpad_unittests: fix ThreadSanitizer problemsMark Mentovai2019-04-263-4/+18
| | | | | | | | | | | | | | | | Some tests were failing because they had expectations about the number of threads in a process, but TSan, and in some cases, ASan, introduce their own threads. Where a sanitizer affects this, the expectations are now used as minimum thread counts, not exact thread counts. See https://www.brooklinen.com/blogs/brookliving/best-thread-count-for-sheets. These problems were detected by ThreadSanitizer at https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8915151099544583616/+/steps/breakpad_unittests__with_patch_/0/stdout Bug: chromium:949098 Change-Id: Ie40f1766bea27e9bcb112bf9e0b8b846fb343012 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1585948 Reviewed-by: Robert Sesek <rsesek@chromium.org>
* Linux MinidumpWriter: fix stack-use-after-scope violationMark Mentovai2019-04-261-2/+4
| | | | | | | | | | | | | | | | | | One form of google_breakpad::WriteMinidump() passed MappingList and AppMemoryList objects by reference to a MinidumpWriter object, instantiating them directly as constructor parameters. The MinidumpWriter stored these objects internally as references, and the underlying objects went out of scope after MinidumpWriter construction. The MinidumpWriter outlived them, causing a violation on any attempt to access them following construction. This bug was detected by AddressSanitizer at https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8915150848087289472/+/steps/breakpad_unittests__with_patch_/0/stdout Bug: chromium:949098 Change-Id: I072ea9f1b64e1eae3e89d4a2b158764ff7970db5 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1585946 Reviewed-by: Robert Sesek <rsesek@chromium.org>
* Make breakpad_unittests work with Chrome's test runner instead of gtest'sMark Mentovai2019-04-191-3/+3
| | | | | | | | | | | | | | | | | | | Chrome's test runner on Linux installs its own StackDumpSignalHandler which swallows signals and doesn't re-raise them. This is sloppy, but apparently there are reasons (https://crbug.com/551681). For breakpad_unittests, it causes problems where a test process expects (via waitpid()) to observe a child crash. Deal with those cases by explicitly restoring the default signal handler. In another case, Chrome's test runner seems to have been arriving at the conclusion that it was to expect output from a child. Transitioning from exit() to _exit() fixes this problem, and it's not necessarily a bad idea to do this in post-fork() children without an execve() anyway. Bug: chromium:949098 Change-Id: I5a6af0c2a09cd8eac9998358f6d5ea665288236f Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1575670 Reviewed-by: Mark Mentovai <mark@chromium.org>
* linux_core_dumper: add missing newline to Android test-disabled messageMark Mentovai2019-04-091-3/+3
| | | | | | | | | | | | | | The [ OK ] LinuxCoreDumperTest.VerifyExceptionDetails line does not appear at the beginning of a line, hiding it from Chromium’s test infrastructure. This causes the test to have an unknown result, which is treated as a failure. https://ci.chromium.org/p/chromium/builders/try/android-kitkat-arm-rel/233129 Bug: google-breakpad:791 Change-Id: I0eb646a219fa40347db884fd28ace647328c5e49 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1558893 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* linux_core_dumper: partially disable test on AndroidMike Frysinger2019-04-071-0/+9
| | | | | | | | | | | | | | | | | | | | | Running this test on android-kitkat-arm-rel fails for some reason: [ RUN ] LinuxCoreDumperTest.VerifyExceptionDetails linux_core_dumper_unittest.cc:170: Failure Expected: (0U) != (dumper.crash_address()), actual: 0 vs 0 linux_core_dumper_unittest.cc:178: Failure Expected equality of these values: 2U Which is: 2 info.size() Which is: 0 [ FAILED ] LinuxCoreDumperTest.VerifyExceptionDetails (7 ms) Disable it for now on Android until someone can look into it. Bug: google-breakpad:791 Change-Id: I40a5e3dbeeb44e5eb0df187e61d55e07d8ad3613 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1546778 Reviewed-by: Mark Mentovai <mark@chromium.org>
* linux_core_dumper: add explicit casts for exception fieldsMike Frysinger2019-03-301-3/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | Some of the fields we save might have signed types depending on the system (e.g. `typedef int pid_t`). Depending on the toolchain, we can trip -Werror=narrowing failures like: src/client/linux/minidump_writer/linux_core_dumper.cc:248:66: error: narrowing conversion of ‘(__pid_t)info->siginfo_t::_sifields.siginfo_t::<anonymous union>::_kill.siginfo_t::<anonymous union>::<anonymous struct>::si_pid’ from ‘__pid_t {aka int}’ to ‘long unsigned int’ inside { } [-Werror=narrowing] set_crash_exception_info({info->si_pid, info->si_uid}); ^^^^^^ src/client/linux/minidump_writer/linux_core_dumper.cc:252:71: error: narrowing conversion of ‘(int)info->siginfo_t::_sifields.siginfo_t::<anonymous union>::_sigsys.siginfo_t::<anonymous union>::<anonymous struct>::_syscall’ from ‘int’ to ‘long unsigned int’ inside { } [-Werror=narrowing] set_crash_exception_info({info->si_syscall, info->si_arch}); ^^^^^^^^^^ Since the exception info fields are all uint64_t which should be large enough to handle all the fields in the siginfo_t structure, add casts for all the assignments to avoid these errors. We have implicit casts even without them, so we aren't changing behavior. Bug: google-breakpad:791 Bug: chromium:945653 Change-Id: Ib04e015998f08b857159ac13e9a065a66d228d49 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1544862 Reviewed-by: Mark Mentovai <mark@chromium.org>
* linux_core_dumper: handle missing SIGSYS supportMike Frysinger2019-03-142-0/+8
| | | | | | | | | | | | If the kernel/C library headers are old, they might not have the fields needed for SIGSYS decoding. Add ifdef checks for that and skip the logic entirely. Easier than adding arch-specific siginfo structs to the codebase. Bug: google-breakpad:791 Change-Id: Ia473e3ffa61fce4c42cf4c1e73a9df044599bc5c Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1524447 Reviewed-by: Mark Mentovai <mark@chromium.org>
* linux_core_dumper: support setting exception_informationMike Frysinger2019-03-124-1/+76
| | | | | | | | | | | | | | Many signals in Linux support additional metadata on a per-signal basis. We can extract that from NT_SIGINFO and pass it through in the exception_information fields. The current core dumper logic doesn't set exception_information at all, so this is an improvement. Bug: google-breakpad:791 Change-Id: I38b78d6494e9bc682441750d98ac9be5b0656f5a Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1497662 Reviewed-by: Mark Mentovai <mark@chromium.org>
* linux_core_dumper: add a fallback definition for NT_SIGINFOMike Frysinger2019-03-071-0/+1
| | | | | | | | | | When building on an old system with outdated headers, this define might not be available. Add a fallback to our existing elf header. Bug: google-breakpad:790 Change-Id: I4dfe7a5cebd414cca3582a1a9cfc983503d5a779 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1507073 Reviewed-by: Mark Mentovai <mark@chromium.org>
* linux_core_dumper: support NT_SIGINFO for reading crashing addressMike Frysinger2019-03-031-0/+23
| | | | | | | | | | | | | | | | | | | | | | | | | The current core dumper only parses NT_PRSTATUS notes. With signal details, this note only includes three fields: signo, code, and errno. We set exception_code to signo and exception_flag to code. The errno value isn't set by the kernel, so there's no need to save it. However, we never fill in exception_address which means all converted crashes look like they happen at address 0. This implies a NULL jump which is usually not the case, so it's just confusing. The prstatus structure doesn't offer anything directly that tracks this. Starting with linux-3.7, the kernel writes out the full siginfo structure in the NT_SIGINFO note. So lets support that to pull out si_addr which, for a bunch of common signals, is the value we want in exception_address. The size of the siginfo_t structure should be locked to 128 bytes at build time for all architectures, so this should hopefully be stable. Bug: google-breakpad:790 Change-Id: I458bad4787b1a8b73fad8fe068e9f23bec957599 Reviewed-on: https://chromium-review.googlesource.com/c/1497661 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Rename MDRawContextARM64 and its context flagsJoshua Peraza2018-08-011-1/+1
| | | | | | | | | | This makes way for the addition of a struct matching Microsoft's layout for ARM64. Change-Id: I115f25290863e7438852691d1ec3c9324a42f7a5 Reviewed-on: https://chromium-review.googlesource.com/1152158 Reviewed-by: Mark Mentovai <mark@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Linux: Write out si_code for SIGBUS exceptions.Lei Zhang2018-04-034-8/+25
| | | | | | | | | Store the information in the exception record's exception_information field. Change-Id: Ie215cae2f070fdab63c3d05cc1bc4fb4b7b095fa Reviewed-on: https://chromium-review.googlesource.com/990799 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Fix minidump on ChromeOSYunlian Jiang2017-12-201-8/+25
| | | | | | | | | | | | | Chrome somehow changed the memory mapping with hugepage enabled. This makes the hack in CrOSPostProcessMappings more general. BUG=chromium:793452 TEST=with this patch on Chromium, minidump_dump *dmp shows the right information on chrome Change-Id: Iff58bf1a712a6e66cbd2d813422db7549a3080a5 Reviewed-on: https://chromium-review.googlesource.com/837963 Reviewed-by: Mark Mentovai <mark@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-172-2/+2
| | | | | | | | | | | | 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>
* Replace remaining references to 'struct ucontext' with 'ucontext_t'Mark Mentovai2017-09-201-1/+1
| | | | | | | | | | | | | | | | | 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>
* Add crash reason and address to microdumps.Tobias Sargeant2017-08-142-0/+73
| | | | | | | | | | | | 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>
* 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>
* 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-251-5/+4
| | | | | | | | | | | | | 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>
* Use __NR_exit_group in MinidumpWriterTest.MinidumpStacksSkippedIfRequested.John Budorick2017-05-241-10/+24
| | | | | | | | | Also adds waits for all child processes spawned in MinidumpWriterTest. Bug: 725754 Change-Id: I3248925993dede2c113ab1989b322a9d9c8f24bd Reviewed-on: https://chromium-review.googlesource.com/513480 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Fixing breakpad on old linux kernelAndrew Ermakovich2017-04-212-29/+53
| | | | | | | | | | Try to read the trace's registers by PTRACE_GETREGS if kernel doesn't support PTRACE_GETREGSET. Bug: Change-Id: I881f3a868789747ca217f22a93370c6914881f9a Reviewed-on: https://chromium-review.googlesource.com/484479 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Treat the process stack as the top of memory for free space histograms.Tobias Sargeant2017-03-291-0/+7
| | | | | | | | | | | | | Because we can't determine the top of userspace mappable memory directly, we rely on the fact that the process stack is allocated at the top of the address space (minus some randomization). Anything after that should not count as free space. BUG=695382 Change-Id: I68453aac9732c2bd4b87236b234518068dec6640 Reviewed-on: https://chromium-review.googlesource.com/446100 Reviewed-by: Primiano Tucci <primiano@chromium.org>
* Fix ASSERT_EQUAL that should have been ASSERT_EQ.Tobias Sargeant2017-03-241-1/+1
| | | | | | | | BUG=703599 Change-Id: I5623705edc41644495aa4f2389056d255e22da8e Reviewed-on: https://chromium-review.googlesource.com/459617 Reviewed-by: Primiano Tucci <primiano@chromium.org>
* Don't generate minidump if crash thread doesn't ref principal mapping.Tobias Sargeant2017-03-232-13/+105
| | | | | | | | | | | | | If the crashing thread doesn't reference the principal mapping we can assume that not only is that thread uninteresting from a debugging perspective, the whole crash is uninteresting. In that case we should not generate a minidump at all. BUG=703599 Change-Id: Ia25bbb8adb79d04dcaf3992c3d2474f3b9b1f796 Reviewed-on: https://chromium-review.googlesource.com/457338 Reviewed-by: Robert Sesek <rsesek@chromium.org>
* [MIPS] Get (ptrace) value of $pc for a threadGordana Cmiljanovic2017-03-221-0/+2
| | | | | | | | | This change is fixing LinuxPtraceDumperTest.SanitizeStackCopy test case. Change-Id: I1eb3becfd4b3660bc5529b5d2a5e35db0b6eb6e0 Reviewed-on: https://chromium-review.googlesource.com/458277 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Improve stack sanitization unittests.Tobias Sargeant2017-02-242-47/+73
| | | | | | | | | | | | Rather than relying on the process stack having all the things that should/shouldn't be sanitized, create synthetic stacks to test all of the important cases. BUG=664460 Change-Id: I959266390e94d6fb83ca8ef11ac19fac89e68c31 Reviewed-on: https://chromium-review.googlesource.com/446108 Reviewed-by: Robert Sesek <rsesek@chromium.org>
* Make stack sanitization elide pointers to non-executable mappings.Tobias Sargeant2017-02-231-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | The address space of every Android Java process is approximately 50% mapped, which means that sanitization tends to be ineffective because most string fragments are plausibly pointers into some mapping. For example, the zygote on 32 bit devices has the following mappings made by dalvik and this covers all 4 byte strings starting with a character between 0x13 and 0x52 (which includes all uppercase characters up to and including 'R'). 12c00000-12d16000 12d16000-32c00000 32c00000-32c01000 32c01000-52c00000 In order to perform stack unwinding we only need pointers into the stack of the thread in question, and pointers to executable mappings. If we reduce the set of considered mappings to those mappings alone, then only ~2% of the address space is left unelided. BUG=664460 Change-Id: I1cc27821659acfb91d658f42a83a24c176505a88 Reviewed-on: https://chromium-review.googlesource.com/446500 Reviewed-by: Robert Sesek <rsesek@chromium.org>
* Use the correct PC when determining whether to skip storing a stack.Tobias Sargeant2017-02-231-5/+7
| | | | | | | | | | | | This addresses a bug in commit 049a1532 that meant that the PC of the crashing thread was always used to determine whether to include a stack, instead of using the PC of the thread in question. BUG=664460 Change-Id: Idcbd5db751e5c00941a1be28607389961c0c75d7 Reviewed-on: https://chromium-review.googlesource.com/446499 Reviewed-by: Robert Sesek <rsesek@chromium.org>
* Address post-submit review comments related to CL #430050Tobias Sargeant2017-02-072-6/+13
| | | | | | | | | | See: https://chromium-review.googlesource.com/c/430050/ BUG=664460 Change-Id: I3cbfbd5b00725bd501f06427eebd976267c4f617 Reviewed-on: https://chromium-review.googlesource.com/438444 Reviewed-by: Primiano Tucci <primiano@chromium.org>
* Remove debugging fprintf in unittest code that prevents rolling breakpadTobias Sargeant2017-02-061-1/+0
| | | | | | | | BUG=664460 Change-Id: I40d8567c659e97415db65cb308c0d39391c44353 Reviewed-on: https://chromium-review.googlesource.com/438364 Reviewed-by: Primiano Tucci <primiano@chromium.org>
* Wire up stack sanitization and skipping to WriteMinidumpTobias Sargeant2017-02-033-33/+214
| | | | | | | | | | | | This makes the parameters stored in the MinidumpDescriptor structure functional for minidumps, analogously to how they are applied to microdumps. BUG=664460 Change-Id: I7578e7a1638cea8f0445b18d4bbdaf5e0a32d808 Reviewed-on: https://chromium-review.googlesource.com/435380 Reviewed-by: Robert Sesek <rsesek@chromium.org>
* Sanitize dumped stacks to remove data that may be identifiable.Tobias Sargeant2017-01-313-54/+245
| | | | | | | | | | | | | | | In order to sanitize the stack contents we erase any pointer-aligned word that could not be interpreted as a pointer into one of the processes' memory mappings, or a small integer (+/-4096). This still retains enough information to unwind stack frames, and also to recover some register values. BUG=682278 Change-Id: I541a13b2e92a9d1aea2c06a50bd769a9e25601d3 Reviewed-on: https://chromium-review.googlesource.com/430050 Reviewed-by: Robert Sesek <rsesek@chromium.org>
* Add API to skip dump if crashing thread doesn't reference a given moduleTobias Sargeant2017-01-183-1/+59
| | | | | | | | | | | | | | | | | | | | | | | | This CL makes it possible to skip a dump if the crashing thread doesn't have any pointers to a given module. The concrete use case is WebView where we would like to skip generating microdump output when webview is unreferenced by the stack and thus cannot be responsible for the crash in a way that would be debuggable. The range of interesting addresses is chosen by examining the process mappings to find the one that contains a pointer that is known to be in the right shared object (i.e. an appropriately chosen function pointer) passed from the client. If the extracted stack does not contain a pointer in this range, then we do not generate a microdump. If the stack extraction fails, we still generate a microdump (without a stack). BUG=664460 Change-Id: If19406a13168264f7751245fc39591bd6cdbf5df Reviewed-on: https://chromium-review.googlesource.com/419476 Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Primiano Tucci <primiano@chromium.org>
* Recover memory mappings before writing dump on ChromeOSTing-Yuan (Leo) Huang2016-07-181-0/+172
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | On Linux, breakpad relies on /proc/[pid]/maps to associate symbols from addresses. ChromeOS' hugepage implementation replaces some segments with anonymous private pages, which is a restriction of current implementation in Linux kernel at the time of writing. Thus, breakpad can no longer symbolize addresses from those text segments replaced by hugepages. This patch tries to recover the mappings. Because hugepages are always inserted in between some .text sections, it tries to infer the names and offsets of the segments, by looking at segments immediately precede and succeed them. For example, a text segment before hugepage optimization 02001000-03002000 r-xp /opt/google/chrome/chrome can be broken into 02001000-02200000 r-xp /opt/google/chrome/chrome 02200000-03000000 r-xp 03000000-03002000 r-xp /opt/google/chrome/chrome BUG=crbug.com/628040 R=mark@chromium.org Review URL: https://codereview.chromium.org/2161713002 . Patch from Ting-Yuan (Leo) Huang <laszio@chromium.org>.
* Server-side workaround to handle overlapping modules.Ivan Penkov2016-06-201-0/+2
| | | | | | | | | | | | | | This change is resolving an issue that was caused by the combination of: - Android system libraries being relro packed in N+. - Breakpad dealing with relro packed libraries in a hack way. This is a fix for http://crbug/611824. I also found an use-after-free issue (bug in Minidump::SeekToStreamType). I disallowed the MinidumpStreamInfo copy and assign constructors and the compiler detected another similar issue in Minidump::Print. Then I disabled the copy and assign constructors for most classes in minidump.h (just in case). There are a couple of classes where I couldn't disallow them (since assign is used). This will require a small refactor so I left it out of this CL. R=mark@chromium.org Review URL: https://codereview.chromium.org/2060663002 .
* Fix stack collection with size limitLars Volker2016-05-241-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | src/client/linux/minidump_writer/minidump_writer.cc:273 obtains the stack info by calling GetStackInfo(). That method will return the stack base address, aligned to the bottom of the memory page that 'stack_pointer' is in. After that it will cap the size of the memory area to be copied into the minidump to 'max_stack_len', starting from the base address, if the caller requested so. This will be the case when collecting reduced stacks, as introduced by this change: https://breakpad.appspot.com/487002/ In such cases the caller will request 2048 bytes of memory. However GetStackInfo() will have aligned the base address to the page boundary, by default 4096 bytes. If the stack, which grows towards the base address from the top ends before the 2048 bytes of the first block, then we will not collect any useful part of the stack. As a fix we skip chunks of 'max_stack_len' bytes starting from the base address until the stack_pointer is actually contained in the chunk, which we will add to the minidump file. BUG=https://bugs.chromium.org/p/google-breakpad/issues/detail?id=695 R=ivanpe@chromium.org Review URL: https://codereview.chromium.org/1959643004 . Patch from Lars Volker <lv@cloudera.com>.
* Revert of Extend mapping merge to include reserved but unused mappings. ↵Primiano Tucci2016-04-281-18/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | (https://breakpad.appspot.com/7714003) Reason for revert: It is causing breakpad crash reports to be invalid (see the associated bug). Merging empty holes in r-x mappings was originally introduced in https://breakpad.appspot.com/7714003 to deal with the first generation of relro packing, which could introduce holes within a .so mapping: [libchrome.so] [guard region] [libchrome.so] However, the logic is broken for the case of two *different* adjacent .so mappings with a guard region in the middle: [libfoo.so] [guard region] [libchrome.so] In this case the guard region is mistakenly associated with libfoo.so, but that is not the right thing to do. In fact, the second generation of rerlo packing added the guard region to prevent mmaps from overlapping and to give room for the non-zero vaddr of relro-packed libraries, which require an anticipated load bias. As the first generation of relro packing is not used anymore, there is no reason to keep this buggy code, which causes failures in decoding crashes where an arbitrary library is mapped immediately before a rerlo packed library. Original issue's description: > Extend mapping merge to include reserved but unused mappings. > > When parsing /proc/pid/maps, current code merges adjacent entries that > refer to the same library and where the start of the second is equal to > the end of the first, for example: > > 40022000-40025000 r-xp 00000000 b3:11 827 /system/lib/liblog.so > 40025000-40026000 r--p 00002000 b3:11 827 /system/lib/liblog.so > 40026000-40027000 rw-p 00003000 b3:11 827 /system/lib/liblog.so > > When the system linker loads a library it first reserves all the address > space required, from the smallest start to the largest end address, using > an anonymous mapping, and then maps loaded segments inside that reservation. > If the loaded segments do not fully occupy the reservation this leaves > gaps, and these gaps prevent merges that should occur from occurring: > > 40417000-4044a000 r-xp 00000000 b3:11 820 /system/lib/libjpeg.so > > 4044a000-4044b000 ---p 00000000 00:00 0 > 4044b000-4044c000 r--p 00033000 b3:11 820 /system/lib/libjpeg.so > 4044c000-4044d000 rw-p 00034000 b3:11 820 /system/lib/libjpeg.so > > Where the segments that follow this gap do not contain executable code > the failure to merge does not affect breakpad operation. However, where > they do then the merge needs to occur. Packing relocations in a large > library splits the executable segment into two, resulting in: > > 73b0c000-73b21000 r-xp 00000000 b3:19 786460 > /data/.../libchrome.2160.0.so > > 73b21000-73d12000 ---p 00000000 00:00 0 > 73d12000-75a90000 r-xp 00014000 b3:19 786460 > /data/.../libchrome.2160.0.so > 75a90000-75c0d000 rw-p 01d91000 b3:19 786460 > /data/.../libchrome.2160.0.so > > Here the mapping at 73d12000-75a90000 must be merged into 73b0c000-73b21000 > so that breakpad correctly calculates the base address for text. > > This change enables the full merge by also merging anonymous maps which > result from unused reservation, identified as '---p' with offset 0, and > which follow on from an executable mapping, into that executable mapping. > > BUG=chromium:394703 BUG=chromium:499747 R=primiano@chromium.org, rmcilroy@chromium.org Review URL: https://codereview.chromium.org/1923383002 .
* Switch the Linux minidump writer to use MDCVInfoELF for CV data.Ted Mielczarek2016-04-055-74/+100
| | | | | | | | | | | | | | | | | | | | | | | | | | This preserves full build ids in minidumps, which are useful for tracking down the right version of system libraries from Linux distributions. The default build id produced by GNU binutils' ld is a 160-bit SHA-1 hash of some parts of the binary, which is exactly 20 bytes: https://sourceware.org/binutils/docs-2.26/ld/Options.html#index-g_t_002d_002dbuild_002did-292 The bulk of the changes here are to change the signatures of the FileID methods to use a wasteful_vector instead of raw pointers, since build ids can be of arbitrary length. The previous change that added support for this in the processor code preserved the return value of `Minidump::debug_identifier()` as the current `GUID+age` treatment for backwards-compatibility, and exposed the full build id from `Minidump::code_identifier()`, which was previously stubbed out for Linux dumps. This change keeps the debug ID in the `dump_syms` output the same to match. R=mark@chromium.org, thestig@chromium.org BUG= Review URL: https://codereview.chromium.org/1688743002 .
* Don't merge the mappings if their exec bit are different.Primiano Tucci2016-03-081-1/+2
| | | | | | | BUG=585534 R=primiano@chromium.org Review URL: https://codereview.chromium.org/1750033002 .
generated by cgit v1.2.1 (git 2.18.0) at 2024-06-01 20:15:31 +0000