Chrome's test runner on Linux installs its own StackDumpSignalHandler
which swallows signals and doesn't re-raise them. This is sloppy, but
apparently there are reasons (https://crbug.com/551681). For
breakpad_unittests, it causes problems where a test process expects (via
waitpid()) to observe a child crash. Deal with those cases by
explicitly restoring the default signal handler.
In another case, Chrome's test runner seems to have been arriving at the
conclusion that it was to expect output from a child. Transitioning from
exit() to _exit() fixes this problem, and it's not necessarily a bad
idea to do this in post-fork() children without an execve() anyway.
Bug: chromium:949098
Change-Id: I5a6af0c2a09cd8eac9998358f6d5ea665288236f
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1575670
Reviewed-by: Mark Mentovai <mark@chromium.org>
Eventually, I want to remove the current version of
SetFirstChanceHandler. That is why I changed the name of the current
callback type to FirstChanceHandlerDeprecated.
I also made sure that it is not possible to have two different
FirstChanceHandlers set at the same time.
This is the first of a set of CLs to clean up the API between Chrome,
BreakPad, and V8. See more information in the tracking bug.
R=mark@chromium.org
Bug: chromium:921971
Change-Id: Ia8c2fd9bd875c36dd7ae8bb4a02e538556bc67a1
Reviewed-on: https://chromium-review.googlesource.com/c/1411776
Reviewed-by: Mark Mentovai <mark@chromium.org>
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>
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>
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>
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>
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>
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>
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>
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>
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>
__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 .
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 .
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 .
On Android the size of the alternate stack can be very small (8k).
Even if breakpad uses sigaltstack to increase the size of the alternate
stack during initialization, that call affects only the main thread.
On Android, the libc's pthread initializer reset the sigaltstack to 8k.
When entering a signal handler, the kernel typically pushes the context
on the alternate stack. On arm64, sizeof(CrashContext) is ~5k, which
leaves 3k of usable stack for breakpad.
On top of that, breakpad allocates another struct CrashContext on the
stack. In the case of Android arm64, then, breakpad ends up using
5k + 5k > 8k of stack, which causes a stack overflow.
This got unnoticed in Android L, as the alternate stack didn't have
red-zones between them, so breakpad was often happily overflowing onto
the next thread's stack. This is not the case anymore [1].
This CL moves the CrashContext into a global variable. It should be
safe as the ExceptionHandlers are serialized on a mutex.
[1] https://android.googlesource.com/platform/bionic/+/595752f623ae88f7e4193a6e531a0805f1c6c4dc
BUG=374
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1354923002 .
So far the microdump_writer dumped the log in logcat using the default
system log. This is simple to achieve but has some drawbacks:
1. Creates spam in the system log, pushing back other eventual useful
messages.
2. There is a high chance that the microdump gets lost if some log
spam storm happens immediately after a crash and before the log
is collected by the feedback client.
3. Since Android L, the logger is smartly throttling messages (to
reduce logcat spam). Throttling brekpad logs defeats the all
point of microdumps.
This change is conceptually very simple. Replace the use of
__android_log_write() with __android_log_buf_write(), which takes
an extra bufID argument. The main drawback is that the
__android_log_buf_write is not exported in the NDK and needs to be
dynamically looked up via dlsym.
This choice has been discussed and advocated by Android owners.
See the internal bug b/21753476.
BUG=chromium:512755
R=thestig@chromium.org
Review URL: https://codereview.chromium.org/1286063003 .
git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1490 4c0a9323-5329-0410-9bdc-e9ce6186880e
When LLVM sees an attempt to dereference a NULL pointer, it will generate
invalid opcodes (undefined behavior) which leads to SIGILL which breaks
this unittest. Upstream's recommendation in this case is to add volatile
markings to get the actual dereference to happen.
This is documented in the blog post under "Dereferencing a NULL Pointer":
http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html
git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1473 4c0a9323-5329-0410-9bdc-e9ce6186880e
This is to add build fingerprint and product name/version to
microdumps. Conversely to what happens in the case of minidumps
with MIME fields, due to the nature of minidumps, extra metadata
cannot be reliably injected after the dump is completed.
This CL adds the plumbing to inject two optional fields plus the
corresponding tests.
BUG=chromium:410294
R=thestig@chromium.org
Review URL: https://codereview.chromium.org/1125153008
git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1456 4c0a9323-5329-0410-9bdc-e9ce6186880e
The current code is relying on info->si_pid to figure out whether
the exception handler was triggered by a signal coming from the kernel
(that will re-trigger until the cause that triggered the signal has
been cleared) or from user-space e.g., kill -SIGNAL pid, which will NOT
automatically re-trigger in the next signal handler in the chain.
While the intentions are good (manually re-triggering user-space
signals), the current implementation mistakenly looks at the si_pid
field in siginfo_t, assuming that it is coming from the kernel if
si_pid == 0.
This is wrong. siginfo_t, in fact, is a union and si_pid is meaningful
only for userspace signals. For signals originated by the kernel,
instead, si_pid overlaps with si_addr (the faulting address).
As a matter of facts, the current implementation is mistakenly
re-triggering the signal using tgkill for most of the kernel-space
signals (unless the fault address is exactly 0x0).
This is not completelly correct for the case of SIGSEGV/SIGBUS. The
next handler in the chain will stil see the signal, but the |siginfo|
and the |context| arguments of the handler will be meaningless
(retriggering a signal with tgkill doesn't preserve them).
Therefore, if the next handler in the chain expects those arguments
to be set, it will fail.
Concretelly, this is causing problems to WebView. In some rare
circumstances, the next handler in the chain is a user-space runtime
which does SIGSEGV handling to implement speculative null pointer
managed exceptions (see as an example
http://www.mono-project.com/docs/advanced/runtime/docs/exception-handling/)
The fix herein proposed consists in using the si_code (see SI_FROMUSER
macros) to determine whether a signal is coming form the kernel
(and therefore just re-establish the next signal handler) or from
userspace (and use the tgkill logic).
Repro case:
This issue is visible in Chrome for Android with this simple repro case:
- Add a non-null pointer dereference in the codebase:
*((volatile int*)0xbeef) = 42
Without this change: the next handler (the libc trap) prints:
F/libc ( 595): Fatal signal 11 (SIGSEGV), code 1, fault addr 0x487
where 0x487 is actually the PID of the process (which is wrong).
With this change: the next handler prints:
F/libc ( 595): Fatal signal 11 (SIGSEGV), code 1, fault addr 0xbeef
which is the correct answer.
BUG=chromium:481937
R=mark@chromium.org
Review URL: https://breakpad.appspot.com/6844002
git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1454 4c0a9323-5329-0410-9bdc-e9ce6186880e
Despite the fact that many places imply that sigaction and rt_sigaction
are essentially the same, rt_sigaction's signature is actually
different-- it takes the size of the kernel's sigset_t as an extra argument.
BUG=473973
git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1447 4c0a9323-5329-0410-9bdc-e9ce6186880e
On Android L+, signal and sigaction symbols are provided by libsigchain
that override the system's versions. There is a bug in these functions
where they essentially ignore requests to install SIG_DFL.
Workaround this issue by explicitly performing a syscall to
__NR_rt_sigaction to install SIG_DFL on Android.
BUG=473973
Patch by Chris Hopman <cjhopman@chromium.org>
Review URL: https://breakpad.appspot.com/1804002/
git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1438 4c0a9323-5329-0410-9bdc-e9ce6186880e
Microdumps are a very lightweight variant of minidumps. They are meant
to dump a minimal crash report on the system log (logcat on Android),
containing only the state of the crashing thread.
This is to deal with cases where the user has opted out from crash
uploading but we still want to generate meaningful information on the
device to pull a stacktrace for development purposes.
Conversely to conventional stack traces (e.g. the one generated by
Android's debuggerd or Chromium's base::stacktrace) microdumps do NOT
require unwind tables to be present in the target binary. This allows
to save precious binary size (~1.5 MB for Chrome on Arm, ~10 MB on
arm64).
More information and design doc on crbug.com/410294
BUG=chromium:410294
git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1398 4c0a9323-5329-0410-9bdc-e9ce6186880e
This bug manifests itself on Android x64 (in ExceptionHandlerTest::ExternalDumper), but is possible on any platform.
The compiler is unaware of the code which runs in the signal handler which reads this variable. Without volatile the compiler is free to optimise away writes to this variable which it believes are never read.
BUG=381142, 346626
R=thestig@chromium.org
Review URL: https://breakpad.appspot.com/8664002
git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1343 4c0a9323-5329-0410-9bdc-e9ce6186880e
This is an initial attempt to add Arm64 (aarch64) support to Breakpad for
Linux / Android platforms. This CL adds the Arm64 data structures, but does
not yet implement the Android getcontext support or CPUFillFromThreadInfo /
CPUFillFromUContext.
BUG=354405,335641
R=mark@chromium.org
Review URL: https://breakpad.appspot.com/1354002
git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1301 4c0a9323-5329-0410-9bdc-e9ce6186880e
