The lld linker has native support for creating packed relocation
sections, and as a result we can expect files with these sections to
have symbols.
Bug: chromium:742655
Change-Id: I48a50bff041146f51b3a8b730d7a778f832787f6
Reviewed-on: https://chromium-review.googlesource.com/754239
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
This relands fd0a0d2b7a which was reverted
in 5dad29423e, 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>
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>
This silences a warning in newer versions of clang that complains
about "register" being a deprecated keyword.
Bug: chromium:780692
Change-Id: If354b9b18421e3e910849b385c44207e0ce02590
Reviewed-on: https://chromium-review.googlesource.com/750362
Reviewed-by: Mark Mentovai <mark@chromium.org>
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>
C++ doesn't allow skipping initialization with a goto. This means that
this code is illegal:
void func(bool b) {
if(b) goto END;
int value = 0; //error C2362 with /permissive-
//... value used here
END:
return;
}
Adding an extra scope makes the code legal. This problem is only
detected with /permissive- but now that compiling with this
switch is practical we might as well stay /permissive- clean:
https://blogs.msdn.microsoft.com/vcblog/2016/11/16/permissive-switch/
Note that compiling /permissive- clean only works with the 10.0.16299.0
SDK which currently has other issues...
Bug: 773476
Change-Id: I54e64aaef46d70a817cf7da272f76d9ae5f6a6f7
Reviewed-on: https://chromium-review.googlesource.com/740287
Reviewed-by: Mark Mentovai <mark@chromium.org>
This hides the need to provide mutable C strings, and unifies
existing basename calls and variations in a single location.
Change-Id: Idfb449c47b1421f1a751efc3d7404f15f8b369ca
Reviewed-on: https://chromium-review.googlesource.com/725731
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
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>
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>
Note that the current MicrodumpProcessor::Process implementation has a
bug due to the fact that it creates a local Microdump instance, and then
holds onto a pointer to the object returned by microdump.GetMemory()
which is destroyed when microdump goes out of scope. This CL fixes the
crash by making Microdump outlive MicrodumpProcessor, which is the same
pattern that Minidump/MinidumpProcessor uses.
Bug: google-breakpad:748
Change-Id: I554b46d309649cf404523722bd9ee39e17a10139
Reviewed-on: https://chromium-review.googlesource.com/720809
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
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>
When using traditional headers, sys/types.h is needed to define __u64
for sys/user.h. Previously, we thought this would be provided by
stdint.h, but it is not.
Change-Id: I0e648712f4ef1e303104a5264d3d2d0b218f5d45
Reviewed-on: https://chromium-review.googlesource.com/705267
Reviewed-by: Mark Mentovai <mark@chromium.org>
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>
This was lost in afa9c52715, but it turns out that it’s still
necessary.
Bug: google-breakpad:733
Change-Id: I4e0e4e4d2e80c22df1ff6b82e471905773c940a3
Reviewed-on: https://chromium-review.googlesource.com/675732
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
1. testing.gyp is a gyp file, not a gypi file. It is only referenced in
“dependencies” sections. The gypi extension is used for files that are
included by an “includes” section.
2. Update paths in testing.gyp to reflect the real locations of
googletest and googlemock following their merge into a single
repository.
Change-Id: If9c356d93aa5ffda54af46fbed648baa2274dac6
Reviewed-on: https://chromium-review.googlesource.com/673404
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Chrome uses API 16 for 32-bit builds and API 21 for 64-bit builds. The
NDK’s <link.h> provides r_debug and link_map structure definitions only
at API 21 and above. Breakpad used a custom <link.h> to define these
structures only during 64-bit builds, which worked for Chrome’s
purposes. However, other consumers may wish to build Breakpad at
arbitrary API levels without regard to bitness. This alters Breakpad’s
custom <link.h> to correctly check the NDK API level rather than target
CPU bitness.
Likewise for <sys/user.h> on 32-bit x86, which provided a typedef for
user_fpxregs_struct to user_fxsr_struct. API 21 and above, as well as
the unified headers at any API level, always name the structure
user_fpxregs_struct.
Definitions for 64-bit ARM’s user_regs_struct and user_fpsimd_struct
have been removed from Breakpad’s copy of <sys/user.h>. The header
claims that these fallback definitions are only necessary with NDK r10,
which should no longer be in use even by Chromium, which now uses NDK
r12b. This removes the Chromium-specific ANDROID_NDK_MAJOR_VERSION macro
from use entirely.
Fixes https://stackoverflow.com/questions/44141159/ and b/65630828.
Bug: google-breakpad:733
Change-Id: I5841906297cd15b15ce48b73fd8332fd40afc9a0
Reviewed-on: https://chromium-review.googlesource.com/665740
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
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>
Nothing appears to be using this anymore, so stop bundling it.
Bug: google-breakpad:360
Change-Id: Id95b36994379da92f8ef2a81754b3da5f1f79cae
Reviewed-on: https://chromium-review.googlesource.com/665503
Reviewed-by: Mark Mentovai <mark@chromium.org>
Breakpad’s DWARF line table reader only understood line tables at the
level of DWARF 2. This wasn’t a problem because LLVM only produced line
tables at this level, even when generating DWARF 4. But LLVM would like
to output DWARF 4 line tables when generating DWARF 4, and Breakpad
needs to understand this format. (Meanwhile, it seems that GCC has used
DWARF 4 line tables with DWARF 4 output since 4.5.0, 2010-04-14.)
DWARF 3 line tables are fully compatible with DWARF 2 (assuming that
nothing needs “prologue end,” “epilogue begin,” or “isa”, and opcodes
related to these fields are properly skipped). DWARF 4 changes the line
number program header slightly to include a “maximum operations per
instruction” field. This field must be recognized, but can safely be
ignored (and assumed to be always 1) if VLIW architectures are not
supported (they aren’t). DWARF 4 also introduces a “discriminator”,
whose opcode can also be skipped if these values are not needed (they
shouldn’t be).
This recognizes the “maximum operations per instruction” field when
processing DWARF 4 line tables, but asserts that its value is 1 and
otherwise ignores it.
This is not compatible with VLIW architectures that set this field to a
value other than 1. Such architectures are irrelevant to Breakpad, and
mainline GCC and the proposed LLVM patch always set this field to 1.
There are other things that could be extracted from DWARF 3 and 4 line
tables that aren’t currently extracted (although these are currently
irrelevant to Breakpad too).
Bug: google-breakpad:745
Change-Id: I5bf9c0b1aa654849c9cce64e60682447d10be8ba
Reviewed-on: https://chromium-review.googlesource.com/663441
Reviewed-by: Mike Frysinger <vapier@chromium.org>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Add a .gyp file for building all windows tools, and add hook to run gyp
to create corresponding .sln files.
This doesn't try to build for platform:x64. This fails due to various
errors caused by the assumption that size_t can be converted to an unsigned
int without loss of information, which is not true on Windows x64 (LLP64),
where size_t is 64 bits, but int is only 32 bits.
There are test failures. client_tests failures are as described in [1].
dump_syms_unittest are as discussed in the description of [2].
[1] https://bugs.chromium.org/p/google-breakpad/issues/detail?id=520
[2] https://codereview.chromium.org/1782453003
BUG=
Change-Id: I965244eb3746f87f30160fd0577e1cc9eb7a8b08
Reviewed-on: https://chromium-review.googlesource.com/441026
Reviewed-by: Mike Frysinger <vapier@chromium.org>
This avoids compile time errors:
In file included from ./src/testing/googletest/include/gtest/gtest.h:1874:0,
from ./src/breakpad_googletest_includes.h:33,
from src/common/mac/macho_reader_unittest.cc:39:
src/common/mac/macho_reader_unittest.cc: In member function 'virtual void LoadCommand_SegmentBE32_Test::TestBody()':
./src/testing/googletest/include/gtest/internal/gtest-internal.h:133:55: error:
converting 'false' to pointer type for argument 1 of 'char testing::internal::IsNullLiteralHelper(testing::internal::Secret*)' [-Werror=conversion-null]
(sizeof(::testing::internal::IsNullLiteralHelper(x)) == 1)
^
...
src/common/mac/macho_reader_unittest.cc:1117:3: note: in expansion of macro 'EXPECT_EQ'
EXPECT_EQ(false, actual_segment.bits_64);
Change-Id: I0cf88160dbe17b0feebed3c91ad65491b81023fd
Reviewed-on: https://chromium-review.googlesource.com/439004
Reviewed-by: Mark Mentovai <mark@chromium.org>
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>
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>
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>
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.
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>
The breakpad symbol uploader prints messages of this form:
Uploaded symbols for windows-x86/eventlog_provider.dll.pdb/...
This is confusing because many people see this message and assume that
symbols are being uploaded to a symbol server. This changes the message
to clarify what is happening.
BUG=677226
Change-Id: Id6fdd8497d0cb97be43c4af010058aab9d84375c
Reviewed-on: https://chromium-review.googlesource.com/434187
Reviewed-by: Mark Mentovai <mark@chromium.org>
This CL hits lots of source files because:
1. An update to the CodeModule virtual class. I added an is_loaded
method to specify whether the module is loaded. There were several
mocks/test classes that needed to be updated with an implementation.
An alternative to this route would be to modify
MinidumpUnloadedModule::code_file to prepend "Unloaded_" to the
module name.
2. Added an unloaded_modules parameter to
StackFrameSymbolizer::FillSourceLineInfo.
BUG=
Change-Id: Ic9c7f7c7b7e932a154a5d4ccf292c1527d8da09f
Reviewed-on: https://chromium-review.googlesource.com/430241
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
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>
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>
The implementations of Module/UnloadedModule and
ModuleList/UnloadedModuleList are very similar. They have been made
separate classes because they operate on different structs, complicating
factoring code into a base class and have sufficiently different
implementation that templates would not be suitable.
When unloaded modules have partially overlapping ranges, the module
shrink down feature is used to move the start of the higher range to the
end of the lower range. If two unloaded modules overlap identically, the
second module will not be added to the range map and the failure
ignored.
Places where MinidumpUnloadedModule differs from MinidumpModule:
code_identifier: the android/linux case is deleted since cv_records
never exist.
debug_file/debug_identifier/version: always return empty strings.
Read: an expected size is provided as opposed to MD_MODULE_SIZE. A
seek is used if there are extra, unused bytes.
Places where MinidumpUnloadedModuleList differs from
MinidumpModuleList:
Read: entry and header size is provided in the header in
addition to count. This changes the checks and handling of padding.
Failures from StoreRange are ignored.
GetMainModule: always returns NULL.
BUG=
Change-Id: I52e93d3ccc38483f50a6418fede8b506ec879aaa
Reviewed-on: https://chromium-review.googlesource.com/421566
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
