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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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 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>
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>
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>
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>
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>
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>
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>
* 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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
The license file comes from the upstream libdisasm tarball/repo.
Change-Id: I04a4002db72f778dd67dbcd71d3b5d1205a8c21d
Reviewed-on: https://chromium-review.googlesource.com/441884
Reviewed-by: Ted Mielczarek <ted@mielczarek.org>
