The variables in the CL are not initialized. Even if it's safe not to
initialize them here, MSAN doesn't know that.
Bug: 394028
Change-Id: I597a7d76aa19d5789decd0f85150fa31c9655269
Reviewed-on: https://chromium-review.googlesource.com/1001573
Reviewed-by: Lei Zhang <thestig@chromium.org>
Store the information in the exception record's exception_information
field.
Change-Id: Ie215cae2f070fdab63c3d05cc1bc4fb4b7b095fa
Reviewed-on: https://chromium-review.googlesource.com/990799
Reviewed-by: Mark Mentovai <mark@chromium.org>
Noticed while adding an include guard presubmit check in
Chromium.
Change-Id: I9e677412d881b32a58d695208045b575bb8f8be6
Reviewed-on: https://chromium-review.googlesource.com/934448
Reviewed-by: Mark Mentovai <mark@chromium.org>
old_handlers is zeroish whenever an NSException is thrown. This caused PROT_WRITE to never be set and resulted in an EXC_BAD_ACCESS when trying to set the handler to NULL.
Change-Id: Ibb7da448204431c7602b1001f3a5216303c4c9d1
Reviewed-on: https://chromium-review.googlesource.com/899907
Reviewed-by: Mark Mentovai <mark@chromium.org>
The previous change to fix compiling on Android < N forgot to include a
helper in the ifdef, thus not fixing the problem. This change extends
the ifdef to include all helpers used by the test.
Change-Id: Ibb3030f54a81b5609a0b55ccef387a3cba22d088
Reviewed-on: https://chromium-review.googlesource.com/895240
Reviewed-by: Mike Frysinger <vapier@chromium.org>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Commit 7a8980997d introduced additional
tests into MicrodumpWriterTest, two of which throw warnings which break
"make check" under default settings on Linux, because the Makefiles are
configured with -Werror=sign-compare.
This patch just makes the signedness of the assertion arguments match.
Change-Id: Ib522f44205c84f91bc9b93276fad60ebbf005f60
Reviewed-on: https://chromium-review.googlesource.com/418938
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
If signal_fd is -1 still, we end up calling close(-1). Not generally
a problem, but it's bad form, and coverity is upset by it.
Change-Id: I46f9c7ca4be7b43af5b609dd8e3f03a0700af418
Reviewed-on: https://chromium-review.googlesource.com/414544
Reviewed-by: Primiano Tucci <primiano@chromium.org>
The stack interest range is passed in MicrodumpExtraInfo from the client.
If the extracted stack does not contain a pointer in this range, then we
assume that this is not a WebView crash, and do not generate a microdump.
If the stack extraction fails, we still generate a microdump (without a
stack).
BUG=664460
Change-Id: Ic762497f76f074a3621c7ec88a8c20ed768b9211
Reviewed-on: https://chromium-review.googlesource.com/412781
Reviewed-by: Primiano Tucci <primiano@chromium.org>
code.google.com is obsolete.
Fix all broken markdown links while at it.
Change-Id: I6a337bf4b84eacd5f5c749a4ee61331553279009
Reviewed-on: https://chromium-review.googlesource.com/411800
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Calling _exit() is something iOS inherited from Mac OS X Breakpad, and isn't
necessary on iOS. This is necessary because recently iOS has started
re-launching the application if breakpad catches a startup crash and calls exit
during startup.
BUG=chromium:645146
Change-Id: Ibb5a681282a886259424655aa8506a80a1fd4f4c
Reviewed-on: https://chromium-review.googlesource.com/397058
Reviewed-by: Mark Mentovai <mark@chromium.org>
Patch provided by Thomas Schweitzer.
BUG=
Change-Id: I1721db8cab7774b433ff6703a0ddc1eab6620c0b
Reviewed-on: https://chromium-review.googlesource.com/379898
Reviewed-by: Mark Mentovai <mark@chromium.org>
Fix unused variable error. Code that uses the kWaitForHandlerThreadMs
constant is inside and ifdef so in some compile configurations constant
was unused. Move it where it's used.
And do the same with other constants as requested during review.
BUG=
Change-Id: I4f4c8f36c982092d53438ed6d2a0a97772402d69
Reviewed-on: https://chromium-review.googlesource.com/374378
Reviewed-by: Mark Mentovai <mark@chromium.org>
On Linux, breakpad relies on /proc/[pid]/maps to associate symbols from
addresses. ChromeOS' hugepage implementation replaces some segments
with anonymous private pages, which is a restriction of current
implementation in Linux kernel at the time of writing. Thus, breakpad
can no longer symbolize addresses from those text segments replaced by
hugepages.
This patch tries to recover the mappings. Because hugepages are always
inserted in between some .text sections, it tries to infer the names and
offsets of the segments, by looking at segments immediately precede and
succeed them.
For example, a text segment before hugepage optimization
02001000-03002000 r-xp /opt/google/chrome/chrome
can be broken into
02001000-02200000 r-xp /opt/google/chrome/chrome
02200000-03000000 r-xp
03000000-03002000 r-xp /opt/google/chrome/chrome
BUG=crbug.com/628040
R=mark@chromium.org
Review URL: https://codereview.chromium.org/2161713002 .
Patch from Ting-Yuan (Leo) Huang <laszio@chromium.org>.
This change is resolving an issue that was caused by the combination of:
- Android system libraries being relro packed in N+.
- Breakpad dealing with relro packed libraries in a hack way.
This is a fix for http://crbug/611824.
I also found an use-after-free issue (bug in Minidump::SeekToStreamType). I disallowed the MinidumpStreamInfo copy and assign constructors and the compiler detected another similar issue in Minidump::Print. Then I disabled the copy and assign constructors for most classes in minidump.h (just in case). There are a couple of classes where I couldn't disallow them (since assign is used). This will require a small refactor so I left it out of this CL.
R=mark@chromium.org
Review URL: https://codereview.chromium.org/2060663002 .
__builtin_trap() causes a SIGTRAP on arm64 (at least with GCC 4.9).
SIGTRAP is not handled by breakpad, causing crashes induced by
__builtin_trap() to be missed.
Note that on x86 and arm, instead, __builtin_trap() raises a SIGILL,
which is already handled by breakapd.
BUG=chromium:614865
R=vapier@chromium.org
Review URL: https://codereview.chromium.org/2042853002 .
src/client/linux/minidump_writer/minidump_writer.cc:273 obtains the
stack info by calling GetStackInfo(). That method will return the
stack base address, aligned to the bottom of the memory page that
'stack_pointer' is in. After that it will cap the size of the memory
area to be copied into the minidump to 'max_stack_len', starting from
the base address, if the caller requested so. This will be the case
when collecting reduced stacks, as introduced by this change:
https://breakpad.appspot.com/487002/
In such cases the caller will request 2048 bytes of memory. However
GetStackInfo() will have aligned the base address to the page
boundary, by default 4096 bytes. If the stack, which grows towards the
base address from the top ends before the 2048 bytes of the first
block, then we will not collect any useful part of the stack.
As a fix we skip chunks of 'max_stack_len' bytes starting from
the base address until the stack_pointer is actually contained in the
chunk, which we will add to the minidump file.
BUG=https://bugs.chromium.org/p/google-breakpad/issues/detail?id=695R=ivanpe@chromium.org
Review URL: https://codereview.chromium.org/1959643004 .
Patch from Lars Volker <lv@cloudera.com>.
When a crash occurs as a result of an allocation failure, it is useful
to know approximately what regions of the virtual address space remain
available, so that we know whether the crash should be attributed to
memory fragmentation, or some other cause.
BUG=525938
R=primiano@chromium.org
Review URL: https://codereview.chromium.org/1796803003 .
Reason for revert:
It is causing breakpad crash reports to be invalid (see the associated
bug).
Merging empty holes in r-x mappings was originally introduced in
https://breakpad.appspot.com/7714003 to deal with the first generation
of relro packing, which could introduce holes within a .so mapping:
[libchrome.so]
[guard region]
[libchrome.so]
However, the logic is broken for the case of two *different* adjacent
.so mappings with a guard region in the middle:
[libfoo.so]
[guard region]
[libchrome.so]
In this case the guard region is mistakenly associated with libfoo.so,
but that is not the right thing to do. In fact, the second generation of
rerlo packing added the guard region to prevent mmaps from overlapping
and to give room for the non-zero vaddr of relro-packed libraries, which
require an anticipated load bias.
As the first generation of relro packing is not used anymore, there is
no reason to keep this buggy code, which causes failures in decoding
crashes where an arbitrary library is mapped immediately before a rerlo
packed library.
Original issue's description:
> Extend mapping merge to include reserved but unused mappings.
>
> When parsing /proc/pid/maps, current code merges adjacent entries that
> refer to the same library and where the start of the second is equal to
> the end of the first, for example:
>
> 40022000-40025000 r-xp 00000000 b3:11 827 /system/lib/liblog.so
> 40025000-40026000 r--p 00002000 b3:11 827 /system/lib/liblog.so
> 40026000-40027000 rw-p 00003000 b3:11 827 /system/lib/liblog.so
>
> When the system linker loads a library it first reserves all the address
> space required, from the smallest start to the largest end address, using
> an anonymous mapping, and then maps loaded segments inside that reservation.
> If the loaded segments do not fully occupy the reservation this leaves
> gaps, and these gaps prevent merges that should occur from occurring:
>
> 40417000-4044a000 r-xp 00000000 b3:11 820 /system/lib/libjpeg.so
> > 4044a000-4044b000 ---p 00000000 00:00 0
> 4044b000-4044c000 r--p 00033000 b3:11 820 /system/lib/libjpeg.so
> 4044c000-4044d000 rw-p 00034000 b3:11 820 /system/lib/libjpeg.so
>
> Where the segments that follow this gap do not contain executable code
> the failure to merge does not affect breakpad operation. However, where
> they do then the merge needs to occur. Packing relocations in a large
> library splits the executable segment into two, resulting in:
>
> 73b0c000-73b21000 r-xp 00000000 b3:19 786460
> /data/.../libchrome.2160.0.so
> > 73b21000-73d12000 ---p 00000000 00:00 0
> 73d12000-75a90000 r-xp 00014000 b3:19 786460
> /data/.../libchrome.2160.0.so
> 75a90000-75c0d000 rw-p 01d91000 b3:19 786460
> /data/.../libchrome.2160.0.so
>
> Here the mapping at 73d12000-75a90000 must be merged into 73b0c000-73b21000
> so that breakpad correctly calculates the base address for text.
>
> This change enables the full merge by also merging anonymous maps which
> result from unused reservation, identified as '---p' with offset 0, and
> which follow on from an executable mapping, into that executable mapping.
>
> BUG=chromium:394703
BUG=chromium:499747
R=primiano@chromium.org, rmcilroy@chromium.org
Review URL: https://codereview.chromium.org/1923383002 .
This preserves full build ids in minidumps, which are useful for
tracking down the right version of system libraries from Linux
distributions.
The default build id produced by GNU binutils' ld is a 160-bit SHA-1
hash of some parts of the binary, which is exactly 20 bytes:
https://sourceware.org/binutils/docs-2.26/ld/Options.html#index-g_t_002d_002dbuild_002did-292
The bulk of the changes here are to change the signatures of the
FileID methods to use a wasteful_vector instead of raw pointers, since
build ids can be of arbitrary length.
The previous change that added support for this in the processor code
preserved the return value of `Minidump::debug_identifier()` as the
current `GUID+age` treatment for backwards-compatibility, and exposed
the full build id from `Minidump::code_identifier()`, which was
previously stubbed out for Linux dumps. This change keeps the debug ID
in the `dump_syms` output the same to match.
R=mark@chromium.org, thestig@chromium.org
BUG=
Review URL: https://codereview.chromium.org/1688743002 .
The Linux dumpers use absolute paths for shared libraries referenced by
dumps, so they fail to locate them if the crash originated in a chroot.
This CL enables callers to specify a root prefix, which is prepended to
mapping paths before opening them.
BUG=chromium:591792
TEST=make check
Review URL: https://codereview.chromium.org/1761023002/
This reverts CL https://codereview.chromium.org/1563223004/
This reverts commit 7cc0d8562bf8b20b88cc941ba72593cb7230ecf6.
CL 1563223004 introduces two bugs on iOS.
- Encoding the minidump name with extra percent causing crash server to fail
processing the file.
- Using a released pointer causing random crashes on upload. The
data, resp, err pointers returned in the NSURLSession completion
handler is released at the end of the block. When used later (to get
the crash ID), it causes a crash.
BUG=569158
R=blundell@chromium.org, mark@chromium.org
Review URL: https://codereview.chromium.org/1619603002 .
Patch from Olivier Robin <olivierrobin@chromium.org>.
Fixes the following compilation warning when using recent version of
the iOS or OS X SDK by using the recommended new API:
../../breakpad/src/common/mac/HTTPMultipartUpload.m:56:10: error: 'stringByAddingPercentEscapesUsingEncoding:' is deprecated: first deprecated in iOS 9.0 - Use -stringByAddingPercentEncodingWithAllowedCharacters: instead, which always uses the recommended UTF-8 encoding, and which encodes for a specific URL component or subcomponent since each URL component or subcomponent has different rules for what characters are valid. [-Werror,-Wdeprecated-declarations]
[key stringByAddingPercentEscapesUsingEncoding:NSUTF8StringEncoding];
^
CFURLCreateStringByAddingPercentEscapes
../../breakpad/src/common/mac/HTTPMultipartUpload.m:207:29: error: 'sendSynchronousRequest:returningResponse:error:' is deprecated: first deprecated in iOS 9.0 - Use [NSURLSession dataTaskWithRequest:completionHandler:] (see NSURLSession.h [-Werror,-Wdeprecated-declarations]
data = [NSURLConnection sendSynchronousRequest:req
^
../../breakpad/src/client/mac/handler/minidump_generator.cc:158:6: error: 'CFPropertyListCreateFromXMLData' is deprecated: first deprecated in iOS 8.0 - Use CFPropertyListCreateWithData instead. [-Werror,-Wdeprecated-declarations]
(CFPropertyListCreateFromXMLData(NULL, data, kCFPropertyListImmutable,
^
BUG=https://bugs.chromium.org/p/google-breakpad/issues/detail?id=675
BUG=569158
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1563223004 .
This works around a bug in M that prevents Breakpad from using
ftruncate() in the renderer process.
To do this, skip the calls to ftruncate() when allocating bigger
minidump files and strictly depends on write() to append to the end.
It might be less efficient but this is probably less of an issue on
SD cards. It is much better than not getting crash reports.
BUG=542840
Original CL: https://codereview.appspot.com/273880044/
Original CL Author: acleung@chromium.org
Review URL: https://codereview.chromium.org/1407233016 .
Debug Gecko builds don't build with -D_DEBUG, so the ifdef in
crash_generation_server doesn't work right. The MSDN documentation for
assert says that it's enabled based on the absence of the NDEBUG define,
so using that seems sensible.
R=thestig@chromium.org
BUG=
Review URL: https://codereview.chromium.org/1398453002 .
The Windows client gyp files were missing proc_maps_linux.cc for the
unittest build. Adding that revealed some build errors due to it
unconditionally including <inttypes.h>. Removing the workarounds in
breakpad_types.h (and a few other places) made that build, which means
that Visual C++ 2013 is now our minimum supported version of MSVC.
Additionally I tried building with VC++ 2015 and fixed a few warnings
(which were failing the build because we have /WX enabled) to ensure
that that builds as well.
BUG=https://code.google.com/p/google-breakpad/issues/detail?id=669R=mark@chromium.org
Review URL: https://codereview.chromium.org/1353893002 .
Although strictly the GPU fingerprint is defined by the build fingerprint,
there is not currently a straightforward mapping from build fingerprint
to useful GPU / GL driver information.
In order to aid debugging of WebView crashes that occur in GL drivers,
and to better understand the range of drivers and versions for feature
blacklisting purposes, it is useful to have GPU fingerprints in breakpad
microdumps.
Landing this patch on behalf of Tobias Sargeant<tobiasjs@chromium.org>
BUG=chromium:536769
R=primiano@chromium.org, thestig@chromium.org
Review URL: https://codereview.chromium.org/1334473003 .
