aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
...
* 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-113-4/+9
| | | | | | | | | | | | | | | | 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>
* roll lss depsMike Frysinger2017-06-162-2/+2
| | | | | | | | | | This should hopefully fix the ARMv8/IT fix to work on clang & gcc. BUG=chromium:686903 Change-Id: Ib99f05a0cd8df2cb2d393e2ff951d3109cdb5f5b Reviewed-on: https://chromium-review.googlesource.com/538213 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>
* 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>
* Only use O_CLOEXEC on platforms that support itLars Volker2017-05-106-0/+141
| | | | | | | | | | | | | | Change a9fca58 made use of the O_CLOEXEC flag, which is not supported on older Linux kernels. This change makes the use contingent on kernel support. Testing: I manually compiled breakpad on CentOS 5.8 running kernel 2.6.18-308.8.2.el5.centos.plusxen. Bug: 730 Change-Id: I21dff928cfba3c156a56708913f65a0c7b5396a6 Reviewed-on: https://chromium-review.googlesource.com/498528 Reviewed-by: Mike Frysinger <vapier@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>
* Make minidump name generation on Linux as random as possibleGabriele Svelto2017-04-124-12/+182
| | | | | | | | | | | | | | | This patch ensures that two crashes taken within the same second have different minidump names. The random characters used in the minidump filename are now read from /dev/urandom where possible or generated via arc4random(). If neither is available we fall back to regular rand() but mixing the address of an object to the current time when generating the random seed to make it slightly less predictable. BUG=681 Change-Id: I2e97454859ed386e199b2628d6b7e87e16481b75 Reviewed-on: https://chromium-review.googlesource.com/445784 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Dump MH_DYLINKER images in upload_system_symbols.Robert Sesek2017-03-303-4/+6
| | | | | | Change-Id: I18291efe211f88ae0607a9055d027b520ef13291 Reviewed-on: https://chromium-review.googlesource.com/462676 Reviewed-by: Mark Mentovai <mark@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>
* Increase maximum number of threads for minidump_stackwalk.Lars Volker2017-03-281-0/+4
| | | | | | | | BUG= Change-Id: I361d8812df7b2977fe2630289059d31c3c9a4cc3 Reviewed-on: https://chromium-review.googlesource.com/459010 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* minidump_dump: fix up init pathsMike Frysinger2017-03-252-3/+5
| | | | | | | | | | Fix some build & test failures in the previous minidump_dump code. BUG=chromium:598947 Change-Id: Ia8fce453265167368de96747a8a92af930e78245 Reviewed-on: https://chromium-review.googlesource.com/458881 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* minidump_dump: dump stack memory like hexdumpMike Frysinger2017-03-243-14/+97
| | | | | | | | | | | | | | | The current stack output is one line byte string which is not easy for humans to parse. Extend the print mode to support a hexdump-like view and switch to that by default. Now we get something like: Stack 00000000 20 67 7b 53 94 7f 00 00 01 00 00 00 00 00 00 00 | g{S...........| 00000010 00 70 c4 44 9a 25 00 00 08 65 7a 53 94 7f 00 00 |.p.D.%...ezS...| BUG=chromium:598947 Change-Id: I868e1cf4faa435a14c5f1c35f94a5db4a49b6a6d Reviewed-on: https://chromium-review.googlesource.com/404008 Reviewed-by: Mark Mentovai <mark@chromium.org>
* minidump_dump: add proper cli processingMike Frysinger2017-03-241-8/+53
| | | | | | | | | | | | In preparation for adding more flexibility to this tool, add a proper parser for the command line flags. This uses the style as seen in other breakpad tools. BUG=chromium:598947 Change-Id: I95495e6ca7093be34d0d426f98a6c22880ff24a3 Reviewed-on: https://chromium-review.googlesource.com/457019 Reviewed-by: Mark Mentovai <mark@chromium.org>
* add -Wunused-local-typedefs to the set of warning flagsTobias Sargeant2017-03-242-1/+2
| | | | | | | | This helps catch issues when rolling in Chromium. Change-Id: I2e3f33df4a69b501d8c04799ee0ac03a4bed9ac9 Reviewed-on: https://chromium-review.googlesource.com/459477 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Remove unused typedefGordana Cmiljanovic2017-03-241-1/+0
| | | | | | | | | | | This removes unused typedef left in change: https://chromium-review.googlesource.com/c/447697/ and fixes error: dump_symbols.cc:613:35: error: unused typedef 'Word' [-Werror,-Wunused-local-typedef] Change-Id: Ib5a82cd8af9a58ebf173b0f338fa9ad341819ef3 Reviewed-on: https://chromium-review.googlesource.com/459518 Reviewed-by: Mark Mentovai <mark@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>
* Sanity check frame pointer while stackwalkingJoshua Peraza2017-03-222-1/+8
| | | | | | | | BUG= Change-Id: Ib9b0fd5ba7f829f8be8cf856ab371c6540279ee5 Reviewed-on: https://chromium-review.googlesource.com/458526 Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
* Use string instead of std::stringJoshua Peraza2017-03-221-1/+1
| | | | | | | | BUG= Change-Id: I06d1a836f8ff59a6abb7e420cd35fe52610ce091 Reviewed-on: https://chromium-review.googlesource.com/457872 Reviewed-by: Ivan Penkov <ivanpe@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>
* dump_context: arm: name the registers in the dump outputMike Frysinger2017-03-181-5/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The current ARM minidump_dump output makes people remember or look up how registers are mapped in the ISA. Let's use human friendly names instead so they don't have to. Currently it looks like: MDRawContextARM context_flags = 0x40000006 iregs[ 0] = 0x3c48b000 iregs[ 1] = 0x3 iregs[ 2] = 0x20 iregs[ 3] = 0x0 iregs[ 4] = 0x1c iregs[ 5] = 0x3c48b000 iregs[ 6] = 0x20 iregs[ 7] = 0x3c48b04c iregs[ 8] = 0x39100611 iregs[ 9] = 0x1c iregs[10] = 0x0 iregs[11] = 0xbe61c200 iregs[12] = 0xfb9c1fec iregs[13] = 0xbe61bd28 iregs[14] = 0x39e19b1c iregs[15] = 0x357dd74c cpsr = 0x680b0010 float_save.fpscr = 0x0 Now it looks like: MDRawContextARM context_flags = 0x40000006 r0 = 0x3c48b000 r1 = 0x3 r2 = 0x20 r3 = 0x0 r4 = 0x1c r5 = 0x3c48b000 r6 = 0x20 r7 = 0x3c48b04c r8 = 0x39100611 r9 = 0x1c r10 = 0x0 r11 = 0xbe61c200 r12 = 0xfb9c1fec sp = 0xbe61bd28 lr = 0x39e19b1c pc = 0x357dd74c cpsr = 0x680b0010 float_save.fpscr = 0x0 BUG=chromium:665083 Change-Id: I46d87c4ff7303a7efcd60da1d0b67ae7a5465c8f Reviewed-on: https://chromium-review.googlesource.com/457197 Reviewed-by: Ivan Penkov <ivanpe@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 issues with Windows dump_syms_unittestJon Turney2017-03-177-2954/+2961
| | | | | | | | | | | | | | | | | | | | | | | | | | * Turn DumpSymsRegressionTest into a parameterized test so it's easier to see which test file is failing * Convert dump_syms_regtest.sym to DOS line endings, being careful to preserve the required spaces at the end of 'STACK WIN' lines * In test #4 (omap_reorder_bbs), since the .exe corresponding to the .pdb is not present, no INFO line is generated in the .sym file. Update .sym file. * Stop collecting stderr from dump_syms. Future work: perhaps it's worth collecting stderr to compare with a different file to verify that "Couldn't locate EXE or DLL file" is output when expected? * Regenerate testdata for test #5 (dump_syms_regtest64), which currently does not pass, seemingly due a mis-match in the PDB age between the .pdb file and the .sym file. Also add the .exe corresponding to the .pdb present, to provide CFI BUG= Change-Id: I54fab866437c9e1bad3a5534cef4fe4b6ae47cd2 Reviewed-on: https://chromium-review.googlesource.com/453178 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* [MIPS]: Don't terminate stackwalk when $sp value doesn't change between framesDragan Mladjenovic2017-03-163-7/+1
| | | | | | | | | Currently on MIPS we accidentally terminate stackwalk if $sp value doesn't change between frames which results in incomplete callchain terminated at the point of first tailcall encountered. Change-Id: I8f1ed1df958d8f0a9eb11fd7800062184d8f1ee2 Reviewed-on: https://chromium-review.googlesource.com/449755 Reviewed-by: Mark Mentovai <mark@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>
* Use NSURLSession if the min version we support is iOS 7+.George Kola2017-03-091-7/+5
| | | | | | | | | | | | | | | Because many apps still support iOS 8, they were defaulting to deprecated NSURLConnection even if the code ran on iOS 10. NSURLConnection requires a run loop and hence the code did not always upload if the queue ran on a thread without a Run Loop. This should improve break pad uploads BUG= Change-Id: I7bff80ea977fd1ab13c8812ed933ef842dab417f Reviewed-on: https://chromium-review.googlesource.com/451880 Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Reviewed-by: Mark Mentovai <mark@chromium.org>
* Fix segfault when interpreting bad debug_fileJoshua Peraza2017-03-081-1/+3
| | | | | | | | BUG=chromium:661037 Change-Id: Ia4da0bd9787c232a6a199cfdfccfbed60c2515c2 Reviewed-on: https://chromium-review.googlesource.com/450090 Reviewed-by: Joshua Peraza <jperaza@chromium.org>
* [MIPS]: Relax dwarf section loading to accept both PROGBITS and MIPS_DWARF ↵Dragan Mladjenovic2017-03-071-4/+22
| | | | | | | | section types Change-Id: I0862d930d92687dee47daa8d4dc3a21524c1c893 Reviewed-on: https://chromium-review.googlesource.com/447697 Reviewed-by: Joshua Peraza <jperaza@chromium.org>
* Fix dump_syms clang compilation on WindowsMaciej Pawlowski2017-03-021-8/+8
| | | | | | | | | | | | Clang complains about bad format strings (DWORD is an unsigned long, not unsigned int) and signed/unsigned comparison. This change is necessary for https://codereview.chromium.org/2712423002/ BUG=245456 Change-Id: I58da92d43d90ac535c165fca346ee6866dfce22e Reviewed-on: https://chromium-review.googlesource.com/448037 Reviewed-by: Mark Mentovai <mark@chromium.org>
* roll lss depsMike Frysinger2017-03-012-2/+2
| | | | | | | | | We want the updated ARMv8/IT fix in the clone code path that newer versions of clang warn about. Change-Id: Ibd2d301d94cd09affc4225d013547ee1e24fac82 Reviewed-on: https://chromium-review.googlesource.com/448001 Reviewed-by: Mark Seaborn <mseaborn@chromium.org>
* win: Set LargeAddressAware on symuploadScott Graham2017-02-281-0/+5
| | | | | | | | | | | | | | | | | | | | This was set manually on Chrome's built binary before https://codereview.chromium.org/2173533002 but wasn't added to the build file. After this change: c:\src\breakpad\src\src>dumpbin /headers tools\windows\symupload\Release\symupload.exe | grep large Application can handle large (>2GB) addresses This change only affects x86 builds. R=mark@chromium.org BUG=chromium:696911 Change-Id: I8f1bd5535af242edde51e70c60cf33b6170855ea Reviewed-on: https://chromium-review.googlesource.com/447780 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>
* Handle ntdll only emitting PUBLIC at func entryScott Graham2017-02-233-1/+69
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This handles a case encountered in ntdll.dll symbols for Windows 7, where a PUBLIC would be emitted only for the entry point to the function. The body of the function, however, is split in a PGO-ish fashion to another remote location in the binary. Because of this, there were large gaps in the RVA space that would be attributed to the "last" function that happened to have an entry point before the gap. In practice, something like this: 0x100 Func1 0x110 Func2 0x120 Func3 0x130 Func4 ... 0x800 LaterFuncs The bodies of Func1/2/3 tend to be implemented as a fast-path check, followed by a jmp to somewhere in the range between 0x130 and 0x800. Because no symbols are emitted for this range, everything is attributed to Func4, causing crash misattribution. In this CL, the change is: after emitting the entry point symbol, also walk in the original OMAP entries through the untranslated binary, and for each block until we resolve to a new symbol (via the same mechanism as we found the entry point) emit another PUBLIC indicating that there's another block that belongs to that symbol. This effectively breaks up the "0x130 - 0x800" range above. R=mark@chromium.org BUG=chromium:678874 Change-Id: Ib3741abab2e7158c81e3e34bca4340ce4d3153a1 Reviewed-on: https://chromium-review.googlesource.com/446717 Reviewed-by: Mark Mentovai <mark@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>
* minidump: mark Read as override in derived classesMike Frysinger2017-02-201-11/+10
| | | | | | | | | | | | | | | | | | | | The base class here declares Read as virtual, so make sure it's marked as override in the derived classes. This fixes some build errors with clang. src/google_breakpad/processor/minidump.h:853:8: error: 'Read' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override] bool Read(uint32_t expected_size_); ^ src/google_breakpad/processor/minidump.h:153:16: note: overridden virtual function is here virtual bool Read(uint32_t expected_size) = 0; ^ Change-Id: Ie4e5fec097b7f37739433a9deb39e7ed60471461 Reviewed-on: https://chromium-review.googlesource.com/444385 Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
* travis: add a clang buildMike Frysinger2017-02-172-15/+13
| | | | | | | | | We rework the matrix a bit to avoid the implicit explosion of duplicated results. Change-Id: I9a2d91b3a6a55bf2843e0e90d59fe5710bd639c7 Reviewed-on: https://chromium-review.googlesource.com/444544 Reviewed-by: Ted Mielczarek <ted@mielczarek.org>
* Fix compile errors arising from compiling breakpad with clang.Tobias Sargeant2017-02-171-16/+17
| | | | | | | | | | | | | | | | | | | | | These compile errors occur when building the check target with: CXX=clang++-3.8 CXXFLAGS="-Werror -Wconstant-conversion -g -O2 -std=c++11" src/processor/stackwalker_mips.cc:60:9: error: comparison of constant 18446744073709551615 with expression of type 'bool' is always false [Werror,-Wtautological-constant-out-of-range-compare] > 0xffffffffffffffff) { ^ ~~~~~~~~~~~~~~~~~~ src/processor/stackwalker_mips.cc:68:66: error: comparison of constant 4294967295 with expression of type 'bool' is always false [-Werror,-Wtautological-constant-out-of-range-compare] if ((memory_ && memory_->GetBase() + memory_->GetSize() - 1) > 0xffffffff) { ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~ Change-Id: I29eed8f4a67b9feeb274aa1fc6c79a019135e8d6 Reviewed-on: https://chromium-review.googlesource.com/438445 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* autotools: move -W flags to configure detectionMike Frysinger2017-02-169-74/+449
| | | | | | | | This lets us use the flags with clang, and to add more flags easily. Change-Id: I51bb53ffd5ab6da769cdfb422a2c88442f1ff9ad Reviewed-on: https://chromium-review.googlesource.com/441864 Reviewed-by: Ivan Penkov <ivanpe@chromium.org>