aboutsummaryrefslogtreecommitdiff
path: root/src/client
Commit message (Collapse)AuthorAgeFilesLines
...
* Remove barrier to fix Android build.Lars Volker2018-01-311-1/+3
| | | | | | | | | | | The unittest for #752 made use of pthread_barrier_t, which is not supported on Android. This change replaces the barrier code with a simple sleep, which proved sufficient to trigger the race. It only affects the test and does not affect the original fix for #752. Change-Id: I82c32cf00899176fa09089e716ed85850b8711e6 Reviewed-on: https://chromium-review.googlesource.com/895168 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Do not use non-standard stdext::checked_array_iterator with libc++.Peter Collingbourne2018-01-181-1/+1
| | | | | | | Bug: chromium:801780 Change-Id: Id1b0b2330d7d609bda62869bcda5bb2f6fde12bd Reviewed-on: https://chromium-review.googlesource.com/872458 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Only restore the signal handler if sigaction has not changedLars Volker2018-01-092-0/+60
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Restoring the signal handler in ExceptionHandler::SignalHandler() can lead to a race in scenarios where multiple threads crash within a short time. This can cause threads to alternately try to write a minidump without ever terminating the process. The first thread to write a minidump will reset the signal handler to the SIG_DFL using signal() in InstallDefaultHandler(). The next thread to execute SignalHandler() will detect this and will reset the signal handler to SignalHandler(). If the first thread takes too long to write its minidump (e.g. when there are many threads), the chances increase that the second thread will enter SignalHandler() before the first one leaves the critical section. After resetting the signal handler, the second thread will fail to write a minidump (since the file already exists) and will try to reset the signal handler to the default by calling RestoreHandlersLocked(). However, in the meantime the first thread will have entered SignalHandler() again and will overwrite it one more time. After that, no further attempts will be made to restore the default signal handler and both threads will continue to re-raise the signal and attempt to write minidump files. This change adds a check to make sure that cur_handler.sa_sigaction is still pointing to SignalHandler() before re-installing the handler. To test this we start a large number of sleeping threads and two threads that will crash simultaneously. Without the fix, this would reproducibly lead to a loop between the two crashing threads. Bug: 752 Change-Id: I784328cfff17ddc7476d6668354570ab867ba405 Reviewed-on: https://chromium-review.googlesource.com/855137 Reviewed-by: Mike Frysinger <vapier@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>
* Reconcile -[BreakpadController withBreakpadRef:] with its documentation.Robert Sesek2017-12-151-3/+1
| | | | | | | | | | The header states that if the controller is not -start:'ed that it will call the block with a NULL BreakpadRef. As previously implemented, it asserted if it was not started. Change-Id: I3a329a773c0484dc1b74013717b68426758ea2cd Reviewed-on: https://chromium-review.googlesource.com/829834 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>
* Create LongStringDictionary and replace SimpleStringDictionary on iOSYi Wang2017-11-032-19/+29
| | | | | | | | | | | 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-022-29/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* 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-272-19/+29
| | | | | | | | | client/ios/Breakpad.mm. Bug: Change-Id: I401028f5d90417d79fb109b510aaa9660a039b44 Reviewed-on: https://chromium-review.googlesource.com/688301 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-177-7/+7
| | | | | | | | | | | | 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>
* 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>
* Fix MSVC build on 64-bitOrgad Shaneh2017-09-253-5/+5
| | | | | | | | | | | 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>
* 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>
* drop bundled gflags from the checkoutMike Frysinger2017-09-131-1/+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>
* 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>
* A couple of minor fixesLeonard Mosescu2017-07-111-1/+1
| | | | | | | | | | | | | | | | 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>
* 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>
* Fix race in ExceptionHandler::GenerateDump()Lars Volker2017-05-101-3/+9
| | | | | | | | | | | | | | | | | | | | | When writing a minidump on Linux, we called clone() in linux/handler/exception_handler.cc with the CLONE_FILES flag. If the parent process died while the child waited for the continuation signal, the write side of the pipe 'fdes' stayed open in the child. The child would not receive a SIGPIPE and would wait forever. To fix this, we clone without CLONE_FILES and then close the read-side of fdes in the master before the ptrace call. That way, if the master dies, the child will receive a SIGPIPE and will die, too. To test this I added a sleep() call before SendContinueSignalToChild() and then killed the master, manually observing that the child would die, too. Bug: 728 Change-Id: Ifd72de835a34e7d9852ae1a362e707fdc6c96c7e Reviewed-on: https://chromium-review.googlesource.com/464708 Reviewed-by: Mike Frysinger <vapier@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-292-1/+22
| | | | | | | | | | | | | 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>
* Fix sporadic failure of InstructionPointerMemory test on WindowsJon Turney2017-03-171-7/+12
| | | | | | | | | | | | | | | | | | If another memory region of interest (e.g. a thread stack) randomly happens to lie immediately before the page allocated by this test, the memory regions can be coalesced in the minidump generated. Relax this test so it correctly handles the case where the expected 256 bytes around the IP aren't at the start of the minidump memory region. Alternatively, that could be avoided by reserving the page before the page used for this test, in which case this test is degenerate with InstructionPointerMemoryMinBound and can be removed. BUG= Change-Id: Ib1bfb242b2c0acaa090df68334a02ac434ad880c Reviewed-on: https://chromium-review.googlesource.com/456702 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Fix Windows client ExceptionHandlerTest testsJon Turney2017-03-114-6/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | ExceptionHandlerTest.InvalidParameterMiniDumpTest and ExceptionHandlerTest.PureVirtualCallMiniDumpTest both also exercise a feature that if the MiniDumpWithFullMemory MINIDUMP_TYPE is used, both UUID.dmp and UUID-full.dmp files are written. This is currently broken, and requesting a minidump with MiniDumpWithFullMemory MINIDUMP_TYPE fails, as the file handle for the full dump is not set. Call GenerateFullDumpFile() if MiniDumpWithFullMemory is requested, to generate a filename for the full dump file and set the file handle. Currently GenerateFullDumpFile() also generates another UUID for the full dump filename, so also make the private method MinidumpGenerator::GenerateDumpFilePath() idempotent (so the same UUID is reused) (Note that calling Generate(|Full)DumpFile() more than once is not permitted, so there's no behaviour where this changed the UUID to preserve) BUG= Change-Id: I74304f38b398f53da1c24f368dedfba8463da9e5 Reviewed-on: https://chromium-review.googlesource.com/452978 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* iOS client identifies itself via URL paramsRoman Margold2017-03-101-0/+44
| | | | | | | | | | | For iOS apps, product and version information is now automatically provided as part of the crash report upload URL to allow for early rejections. Change-Id: Ia19c490c38023f9e23ec8a537f7a203ff1e642d7 Reviewed-on: https://chromium-review.googlesource.com/436164 Reviewed-by: Roman Margold <rmargold@chromium.org> Reviewed-by: Joshua Peraza <jperaza@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>
* There is no need to use the main queue just for perform selector.George Kola2017-02-131-34/+31
| | | | | | | | | | | | | | | | | | | | | | | | | We were using the main queue to queue up a perform selector and then the code [self sendStoredCrashReports] was immediately doing a dispatch_async. This unnecessary thread switching is not needed. We simplify the above logic and use dispatch_after to queue the block on the internal queue after a delay Note that main queue is typically more loaded and it is better for non-UI code to not use the main queue. This may also help improve crash log upload. This change also switches from @synchronized to dispatch_once as that is faster Reference: http://googlemac.blogspot.com/2006/10/synchronized-swimming.html BUG= Change-Id: I81035149cbbf13a3058ca3a11e6efd23980f19ad Reviewed-on: https://chromium-review.googlesource.com/441364 Reviewed-by: Joshua Peraza <jperaza@chromium.org>
* windows: fix build on pre-Win10 systemsMike Frysinger2017-02-111-0/+5
| | | | | | | | | | The use of DBG_PRINTEXCEPTION_WIDE_C was added for Win10 support, but that define doesn't exist in older versions which means we fail to build. Put it behind an ifdef check to work everywhere. Change-Id: Ibab8bddd5c19b4b50e356f59edeb3873c3104569 Reviewed-on: https://chromium-review.googlesource.com/441525 Reviewed-by: Mark Mentovai <mark@chromium.org>
* windows: update gtest/gmock pathsMike Frysinger2017-02-114-15/+19
| | | | | | | | | The Windows build has rotted a bit with the gtest/gmock updates. Update all of the paths to fix things up again. Change-Id: Id67ce76abfd331c0543aa4bd1138e9cc13a18c75 Reviewed-on: https://chromium-review.googlesource.com/441584 Reviewed-by: Mark Mentovai <mark@chromium.org>
* fix write() unused-result warningMike Frysinger2017-02-083-4/+6
| | | | | | | | | | | src/client/linux/microdump_writer/microdump_writer_unittest.cc:98:47: error: ignoring return value of 'ssize_t write(int, const void*, size_t)', declared with attribute warn_unused_result [-Werror=unused-result] write(STDOUT_FILENO, identifiable_string, 0); Change-Id: I3f2305fbec0dbd1464de9aeff051e7cba2ee69a2 Reviewed-on: https://chromium-review.googlesource.com/438545 Reviewed-by: Ivan Penkov <ivanpe@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-035-40/+232
| | | | | | | | | | | | 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>
* Revert "iOS client identifies itself via URL params"Roman Margold2017-02-011-37/+0
| | | | This reverts commit 262a3f50fe5948c2570bbce2cd696e253a88af79.
* iOS client identifies itself via URL paramsRoman Margold2017-02-011-0/+37
| | | | Recently, Crash started applying quotas for crash report uploads to protect the service and its client products from misbehaving product or product version. For the protection to be effective, products need to identify themselves during report upload via URL parameters. This new code makes iOS apps using Breakpad provide the parameters automatically.
* Sanitize dumped stacks to remove data that may be identifiable.Tobias Sargeant2017-01-319-62/+362
| | | | | | | | | | | | | | | 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 module (2)Tobias Sargeant2017-01-191-0/+8
| | | | | | | | | | | Follow-up CL to add relevant code to the copy constructor and assignment operator for MinidumpDescriptor BUG=664460 Change-Id: I71c0ad01d8686a9215a718cebc9d11a215ea342c Reviewed-on: https://chromium-review.googlesource.com/430711 Reviewed-by: Robert Sesek <rsesek@chromium.org>
* Add API to skip dump if crashing thread doesn't reference a given moduleTobias Sargeant2017-01-1810-62/+149
| | | | | | | | | | | | | | | | | | | | | | | | 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>
* Fix unit tests expecting no output when a microdump is suppressed.Tobias Sargeant2016-12-121-9/+9
| | | | | | | | BUG= Change-Id: Ie4d190c68ecbd8709874a3f1ceb872b94b36914f Reviewed-on: https://chromium-review.googlesource.com/419036 Reviewed-by: Primiano Tucci <primiano@chromium.org>