An OEM asks for sub-second granularity, and that's most easily done if
we only have one timestamp generator. I'm not convinced sub-second
granularity is particularly useful myself, and I definitely don't think
that nanosecond resolution is meaningful but I do like this cleanup, and
if I'm going to use sub-second precision I may as well use the maximum
precision available to me.
Also reduce some duplication of code reading cmdline/comm.
Bug: https://issuetracker.google.com/161860597
Test: head /data/tombstones/*
Change-Id: I035ecfd4a3338ccd84dae0ef973a998a7c7c5056
Tags appear in the addresses printed in the memory dump, which seems
like a reasonable place to put them because tagged addresses will
also appear in other places in the tombstone, such as registers and
the fault address.
Bug: 135772972
Change-Id: I52da338347ff6b7503cf5ac80763c540695dc061
Previously, we would do a simple bounds check before deciding
whether to dump the memory around a register. On 64-bit platforms,
the register's value was required to be less than (4 << 60). However,
after stripping tags on AArch64 as part of r.android.com/1365229, all
pointer values became less than (4 << 60), so the check became useless
for filtering out invalid pointers. As a result, we would attempt to
dump memory for all registers, which for a register not containing
a valid pointer would typically consist of 16 lines of dashes.
One possible fix may be to replace the constant (4 << 60) with the
process's actual address space limit (known as TASK_SIZE inside the
kernel; typically 39 bits on AArch64 and 48 bits on x86_64), but the
kernel provides no API for retrieving a process's TASK_SIZE value. We
could guess it by looking at for example the highest bit set in the
value of getauxval(AT_EXECFN), which points to an address on the stack
which typically is mapped at the end of the address space on program
startup, but at least on AArch64 it is possible to dynamically extend
TASK_SIZE at runtime by providing a hint to mmap(), so this is not
always sufficient.
Instead, it seems best to remove most of the early bounds check, and
simply issue ptrace() calls for each register value, bailing out of
the entire output if none of the calls ended up succeeding. This also
has the nice side effect of avoiding 16 lines of noise per register
whose value looks like a pointer but actually points to unmapped
memory. We still retain part of the bounds check in order to avoid
integer overflow during the dump (including overflows into the tag
part of the address on architectures that support tagging).
Bug: 154272452
Change-Id: I94e4b7124b7735b92fd83a49c80ebded3483cd4e
We do not install a 32-bit version of libminijail to 64/32 devices,
which means that "atest -a debuggerd_test" always fails on 32-bit.
Fix it by statically linking libminijail.
Change-Id: I1e5610d1353b4f5b718c1259825421c0c07d7c24
After r.android.com/1288984 we started failing to dump memory contents
for heap addresses because the tag started causing any addresses to
fail this bounds check. Add an untag_address() call to the bounds check
so that the tag is ignored.
Bug: 154272452
Change-Id: I3a6d1a078b21871bd93164150a123549f83289f6
It's impractical to test the contents of the stack trace, but we
should at least test that *a* stack trace is present, which would
have caught the bug fixed by r.android.com/1306754 .
Bug: 135772972
Change-Id: Ic5e0b997caa53c7eeec4e5185df5c043c9d4fe3d
Teach debuggerd to use the new scudo APIs proposed in
https://reviews.llvm.org/D77283 for extracing MTE error reports from crashed
processes, and include those reports in tombstones if possible.
Bug: 135772972
Change-Id: I082dfd0ac9d781cfed2b8c34cc73562614bb0dbb
log/log.h primarily concerns itself with writing logs. The few users
who read logs should directly include log/log_read.h.
Bug: 78370064
Test: build
Change-Id: Ie95c55ea2ffc76fc95768323d445ada6ad4f2520
If crash_dump dies before it gets a chance to write to the pipe we use
to let the debugged-process know that it successfully started, we
weren't cleaning up the child we fork to start it, leaving a zombie
child.
Bug: http://b/152119184
Test: debuggerd_test
Change-Id: Id01cc05f693995e9998941774f74ab8e3d8b4d8a
On aarch64, the top 8 bits of the address (i.e. the tag bits) of
the fault address in si_addr are always clear. This isn't ideal for
MTE which will require these bits in order to correctly diagnose
tag mismatches.
A proposed kernel patch [1] exposes the full fault address including
the tag bits as part of the ucontext. Change debuggerd to read this
fault address if available.
[1] https://patchwork.kernel.org/patch/11435077/
Bug: 135772972
Change-Id: Ia05be574113860f4e9ecc36a310c4b740e0c4afb
GWP-ASan uses frame-pointer based unwinding internally on
allocation/deallocation to collect stack traces that are used when
crashes are reported.
This should be generic, so pull it out into libunwindstack so it can be
used by MTE as well.
Bug: 152412331
Test: atest debuggerd_test
Change-Id: I27b32263aac63446f5fe398af108676b70cd3971
Similar to r.android.com/1247247 I'll be adding more of them for MTE.
Also, change the protocol between the crasher and crash_dump to make
it easier to add new fields and change the referenced data structures
without needing to worry about versioning. The version number for
static executables is now always 1 (where the protocol will never
change), while the version number for dynamic executables is always
4 (where the protocol can change, because the linker and crash_dump
are version locked).
Bug: 135772972
Change-Id: Ib4696d0544d7c87cb429aaaa15f18c3640059e16
We're now using it in contexts that don't have all of the registers available,
such as GWP-ASan and soon MTE, so it doesn't make sense to have it be a
member function of Regs.
Bug: 135772972
Change-Id: I18b104ea0adb78588d7e475d0624cefc701ba52c
- Create a static library libunwindstack_no_dex without DEX support.
- Use it in libdebuggerd_handler_fallback, whose only use is in the
linker, which shouldn't need that support.
- Use it in init_first_stage, which doesn't need DEX support either.
- Also need a libbacktrace_no_dex since it's in the dependency chain
from init_first_stage to libunwindstack_no_dex.
Also restrict the *_no_dex libs and libdebuggerd_handler_fallback as
much as possible to avoid inadvertent use of these reduced
functionality libs.
Test: m init_first_stage on Cuttlefish
where BOARD_BUILD_SYSTEM_ROOT_IMAGE=false
Test: m system_image com.android.runtime
Test: Build & boot
Test: atest linker-unit-tests libunwindstack_unit_test debuggerd_test
Bug: 142944931
Bug: 151466650
Change-Id: Iaacb29bfe602f3ca12a00a712e2a64c45ff0118b
A future change will introduce a version lock between linker and
crash_dump. Move crash_dump into the runtime APEX alongside linker in order to
ensure that they will be the same version even if the runtime APEX is updated.
Bug: 135772972
Change-Id: Ic2eae31b6927eb0e8a62315ac141f50933c00bcc
Merged-In: Ic2eae31b6927eb0e8a62315ac141f50933c00bcc
We're now passing around a couple of addresses for GWP-ASan in addition
to abort_msg_address and fdsan_table_address, and I'm going to need to add
more of them for MTE. Move them into a data structure in order to simplify
various function signatures.
Bug: 135772972
Change-Id: Ie01e1bd93a9ab64f21865f56574696825a6a125f
On userdebug/eng devices, check a system property to see whether we
should create tombstones or not. OEMs that would rather have core dumps
can set this property and configure /proc/sys/kernel/core_pattern
appropriately.
Bug: https://issuetracker.google.com/149663286
Test: set the property, cause a crash
Change-Id: If894b4582a1820b64bdae819cec593b7710cb6e3
GWP-ASan can provide information about a crash that it caused. Grab the
GWP-ASan regions from the globals shared by the linker for crash-handler
purpopses, pull the information from GWP-ASan, and display it.
This adds two regions:
1. Causality tracking by GWP-ASan. We now print a cause header about
the crash, like `Cause: [GWP-ASan]: Use After Free on a 1-byte
allocation at 0x7365bb3ff8`
2. Allocation and deallocation stack traces.
Bug: 135634846
Test: atest debuggerd_test
Change-Id: Id28d5400c9a9a053fcde83a4788f971e677d4643
use Android.bp instead of Android.mk to build and install the
crash_dump.policy files. This also allows mainline modules to pull
the files into their apex (dependency wasn't handled for Android.mk)
Bug: 147914640
Test: build, examine generated filesystem
Change-Id: Iae92d4f9d683ccfddf1716e7eb2877b7bff0c737
This takes a lot of space, isn't convincingly useful, and makes it
likely that the far more valuable stuff that comes after it gets
truncated. So let's just drop it.
Bug: http://b/139860930
Test: manual crasher, presubmit
Change-Id: Ie417ffc07e3cb17e95fdb3d183f8c87de0f34b89
1 page isn't enough to log on AArch64, and clean pages are free, so
increase the stack size to 8 pages.
Bug: http://b/144887737
Test: treehugger
Change-Id: I731b3bc27ab37f4b830a9478a04cd34d4f7648d3
GWP-ASan's crash information retrieval services requires a Printf()
function (declared by the system/implementing allocator). In this
instance, because _LOG is called with additional arguments (the log_t),
this function must be wrapped to conform to printf_t defined by
GWP-ASan.
We can easily wrap the variadic version.
Bug: 135634846
Test: atest debuggerd_test
Change-Id: I17209cd2b7455ce889e2f8194969f606cac329eb
A thread's PSTATE can sometimes be critical for understanding a crash,
especially with MTE and other new features that store per-thread state
in PSTATE.
Bug: 135772972
Change-Id: I1bee25bffe7eea395f04b6449dc9227298cf866e
logger_entry and logger_entry_v2 were used for the kernel logger,
which we have long since deprecated. logger_entry_v3 is the same as
logger_entry_v4 without a uid field, so it is trivially removable,
especially since we're now always providing uids in log messages.
liblog and logd already get updated in sync with each other, so we
have no reason for backwards compatibility with their format.
Test: build, unit tests
Change-Id: I27c90609f28c8d826e5614fdb3fe59bde22b5042
debuggerd_client.race seems to have suddenly started to flake, for no
apparent reason. This doesn't seem to reproduce locally, so increase
the timeouts to rule out our test VMs being slow.
Bug: http://b/142571257
Test: treehugger
Change-Id: Ic54a78b8da36cb1163cec7e7976c73c3da628a30
C++20 wants members to be ordered unlike C99.
Bug: 139945549
Test: mm
Change-Id: I3cbca589511c1e0bbc10c691949e18de77e16031
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
We're missing useful crashes, especially on hwasan builds.
Bug: http://b/140580637
Test: run crasher
Change-Id: Ib5d8d3bd3fc4d7fec77d0b10302e5595f97a3515
There is still some flakiness, so increase the timeout values.
Also remove the TEMP_FAILURE_RETRY macro usage in TIMEOUT calls.
That macro disables the ability of the alarm code to interrupt
the system call.
Bug: 141045754
Test: Unit tests pass.
Change-Id: Ia3c95dccc3076a3fd5ef6432097a57e4ccee4df3
The fdsan code uses getrlimit/ugetrlimit so need to allow that when
running the debuggerd unit tests.
Bug: 141045754
Test: Ran the offending tests hundreds of times without failure.
Change-Id: Iece94f03e7895d61ca8a8f3ab17dce7e54ddf9cd
Catch as many early-boot crashes as we can by starting tombstoned
immediately after /data is mounted.
Bug: http://b/139864948
Test: adb shell su 0 dmesg | grep "starting service"
Change-Id: I7f8821102191a445e87020f3efa59a2e0620d9db
Since only privileged processes with CAP_SYS_ADMIN can read kernel
stack traces from /proc/*/stack, we dump the waiting channels
instead to provide some insight as to where the process might
be stuck in the kernel.
Bug: 135458700
Fixes: 135458700
Test: adb shell am hang; Check /data/anr/<anr-file> for
wchan data.
Change-Id: I9f13511ad89a259ce5e5465155db15d45d2c46d8
Test: Ran new unit tests.
Test: Ran crasher stack-overflow, crasher64 stack-overflow and verified
Test: stack overflow cause is shown.
Test: Ran stack overflow app and verified tombstone includes stack-overflow
Test: message.
Change-Id: I9bb01186dff5ed81c77d84b6aaedb5332ddd7256
This is for Android Telemetry to be able to categorise the processes
that produce tombstones.
Test: atest debugerd_test:TombstoneTest
Change-Id: Ie635347c9839eb58bfd27739050bd68cbdbf98da
Modify the unwinder library to indicate that at least one of the stack
frames contains an elf file that is unreadable.
Modify debuggerd to display a note about the unreadable frame and a possible
way to fix it.
Bug: 129769339
Test: New unit tests pass.
Test: Ran an app that crashes and has an unreadable file and verified the
Test: message is displayed. Then setenforce 0 and verify the message is
Test: not displayed.
Change-Id: Ibc4fe1d117e9b5840290454e90914ddc698d3cc2
There appears to be a kernel bug that causes SIGHUP and SIGCONT to be
sent to the parent process group we spawn from if the process group
contains stopped jobs (e.g. the parent itself, because of wait_for_gdb).
Call setsid in all of our children to prevent this from happening.
Bug: http://b/31124563
Test: adb shell 'setprop debug.debuggerd.wait_for_gdb 1; killall -ABRT surfaceflinger'
Change-Id: I1a48d70886880a5bfbe2deb80d48deece55faf09
Somehow the code was still including this include from libbacktrace.
I think the libbacktrace include directory was coming from some
transitive includes. I verified that nothing in debuggerd is using
the libbacktace.so shared library.
Bug: 120606663
Test: Builds, unit tests pass.
Change-Id: I85c2837c5a539ccefc5a7140949988058d21697a
Update the entries only when the list is modified by the runtime.
Check that the list wasn't concurrently modified when being read.
Bug: 124287208
Test: libunwindstack_test
Test: art/test.py -b --host -r -t 137-cfi
Change-Id: I87ba70322053a01b3d5be1fdf6310e1dc21bb084
Update debuggerd to print BuildId information by default.
Bug: 120975492
Test: New unit tests pass.
Test: debuggerd -b <PID> shows build id information.
Test: tombstones include build id information.
Change-Id: I019b031113d0b77385516223c63455b868924440
If a process is ptraced already, we might not be able to exec crash_dump
due to selinux. Since we can be called for non-fatal events, we
shouldn't abort in that case.
Bug: http://b/128054996
Test: treehugger
Change-Id: I1442041caa7af908df2ab87b9e010c44082e7587
Currently, moving or copying a Maps object leads to double free of MapInfo.
Even moving a Maps object did not prevent this, as after a move
the object only has to be in an "unspecified but valid state", which can
be the original state for a vector of raw pointers (but not for a vector
of unique_ptrs).
Changing to unique_ptrs is the most failsafe way to make sure we never
accidentally destruct MapInfo.
Test: atest libuwindstack_test
Failed LocalUnwinderTest#unwind_after_dlopen which also fails at master.
Change-Id: Id1c9739b334da5c1ba532fd55366e115940a66d3
It should be dlopen'ed lazily by libdexfile_support now.
Also change debuggerd_test to not link libunwindstack and its dependencies
statically - the static libs can overlap with the dynamic ones.
Test: mmma system/core/debuggerd/
Test: atest debuggerd_test
Test: mmma system/core/{libunwindstack,libbacktrace}, run host gtests (cannot get atest to work)
Bug: 124827589
Bug: 123186083
Change-Id: I9e7bf9bcbae499af4e1be4c9854bce441e2a7b55
This is necessary since the dynamic one is now using dlopen(), which isn't
available in static builds.
Test: m
Test: mmma system/core/{libunwindstack,libbacktrace}, run host gtests (cannot get atest to work)
Bug: 123403798
Bug: 123186083
Change-Id: I06a9cdfe7e7cc01427ffd54b66c8ebab88782260
Small modifications to the dump_stack method and added unit tests to
verify the output.
Bug: 120606663
Test: Unit tests pass, debuggerd run on processes on target.
Change-Id: Id385a915b751abda3dd6baebed6c3ce498c3bf6e
With our method returning 'bool', a "return -1" is interpretted
as 'true'. We change this to an explicit 'false', as desired.
Test: TreeHugger
Change-Id: I222858b797bc4242a2dc6d4fe81df3d2586d055a
This reverts commit 444e23d2fc.
The rest of the topic doesn't need to be reverted.
Reason for revert: Breaks renderscript on marlin and sailfish.
Test: Manual repro of http://b/121110092#comment1 on reported branch
Test: "atest CtsRenderscriptTestCases" on that branch
Test: mmma system/core/{libunwindstack,libbacktrace}, run host gtests
Test: Make image, flash, and reboot device.
Bug: 121110092, 119632407
Change-Id: If1976b19ce386c95bc5bd4fd6d523745c167de18
There is a problem about tombstone, which it will fail to
generate tombstone file in some scenarios due to socket
communication exception.
Reproduce step:
step 1: reboot device
step 2: ps -ef |grep zygote , get the pid of zygote64
(Attention: zygote64 should never been killed or reboot,
otherwise we can get the tombstone file)
step 3: kill -5 pid of zygote64
step 4: cd data/tombstones/, and could not find the tombstone
file of zygote64.
[Cause Analysis]
1. There are following logs by logcat:
11-19 15:38:43.789 569 569 F libc : Fatal signal 5 (SIGTRAP),
code 0 (SI_USER) in tid 569 (main), pid 569 (main)
11-19 15:38:43.829 6115 6115 I crash_dump64: obtaining output
fd from tombstoned, type: kDebuggerdTombstone
11-19 15:38:43.830 569 5836 I Zygote : Process 6114 exited
cleanly (0)
11-19 15:38:43.830 777 777 I /system/bin/tombstoned: received
crash request for pid 569
11-19 15:38:43.831 6115 6115 I crash_dump64: performing dump of
process 569 (target tid = 569)
...
11-19 15:38:43.937 777 777 W /system/bin/tombstoned: crash
socket received short read of length 0 (expected 12)
2. The last log was print by function of crash_request_cb in
file of tombstoned.cpp, following related code:
rc = TEMP_FAILURE_RETRY(read(sockfd, &request, sizeof(request)));
if (rc == -1) {
PLOG(WARNING) << "failed to read from crash socket";
goto fail;
} else if (rc != sizeof(request)) {
LOG(WARNING) << "crash socket received short read of length " << rc << " (expected "
<< sizeof(request) << ")";
goto fail;
}
Tombstoned read message by socket, and now the message length is
zero. Some socket communication exception occurs at that time.
We try to let crash_dump resend the socket message when the
communication is abnormal. Just as this CL.
Test: 1 reboot device
2 ps -ef |grep zygote , get the pid of zygote64
(Attention: zygote64 should never been killed or reboot,
otherwise we can get the tombstone file)
3 kill -5 pid of zygote64
4 cd data/tombstones/, and could find the tombstone file of
zygote64.
Change-Id: Ic152b081024d6c12f757927079fd221b63445b18
Make XOM related crashes a little less mysterious by adding an abort
cause explaining the crash.
Bug: 77958880
Test: Abort cause in tombstone for a XOM-related crash.
Change-Id: I7af1bc251d9823bc755ad98d8b3b87c12bbaecba
Previously, when we received simultaneous dump requests, we were CASing
a file descriptor value into a variable, and then failing to close it
if the CAS failed.
Bug: http://b/118412443
Test: debuggerd_test
Change-Id: I075c35a239426002eb9416da3d268c3d1a18e9d2
TEMP_FAILURE_RETRY's result was unused for the call to read(), so now
mark it as such to silence a possible unused result warning. For
__read_chk(), this function is an internal implementation detail of
FORTIFY in Bionic. Under clang-tidy, FORTIFY checks are actually
removed, so this now results in an unknown function being called. The
code should not be explicitly depending on an implementation detail, but
we can just suppress the failing case to retain test coverage of the
actual implementation.
Bug: http://b/110779387
Test: Build using WITH_TIDY=1
Change-Id: If83ac1d6f3b6dc32c0d0fb56d8e675e53b586f78
Previously, if an intercept ends before we ask for a file descriptor
when doing a backtrace, we'll create a tombstone file instead.
Bug: http://b/114139908
Bug: http://b/115349586
Test: debuggerd_test32
Change-Id: I23c7bb8ae5a982a4374a862d0a4f17bee03eb1d9
system_server is sometimes failing to dump with the following error:
libdebuggerd_client: received packet of unexpected length from tombstoned: expected 128, received -1
Improve the logging to try to figure out what's going on.
Bug: http://b/114139908
Test: treehugger
Change-Id: Iee1bdc0891b9fc7bd80a330495ec22a530febddb
Make it possible for code such as fdsan that generates debugging
tombstones via raise(DEBUGGER_SIGNAL) to pass an abort message as well.
Bug: http://b/112770187
Test: debuggerd_test
Change-Id: Idc34263241c18033573e466da3a45aa6f716ddb3
This commit only prints the raw value of the owner tag, pretty-printing
will come in a follow-up commit.
Test: debuggerd `pidof adbd`
Test: static_crasher fdsan_file + manual inspection of tombstone
Change-Id: Idb7375a12e410d5b51e6fcb6885d4beb20bccd0e
Pass the address of the fdsan table down to crash_dump so that we can
dump the fdsan table along with the open file descriptor list.
Test: debuggerd_test
Test: manually ran an old static_crasher
Change-Id: Icbac5487109f2db1e1061c4d46de11b016b299e3
adbd has been built as a static executable since the same binary was
copied to the recovery partition where shared library is not supported.
However, since we now support shared library in the recovery partition,
adbd is built as a dynamic executable.
In addition, the dependency from adbd to libdebuggerd_handler is removed
as debuggerd is handled by the dynamic linker.
A few more modules in /system/core are marked as recovery_available:
true as they are transitive dependencies of the dynamic linker.
This change also includes ld.config.recovery.txt which is the linker
config file for the recovery mode. It is installed to /etc/ld.config.txt
and contains linker namespace config for the dynamic binaries under
/sbin.
Bug: 63673171
Test: `adb reboot recovery; adb devices` shows the device ID
Test: Select 'mount /system' in the recovery mode, then `adb shell`.
$ lsof -p `pidof adbd` shows that libm.so, libc.so, etc. are loaded from
the /lib directory.
Change-Id: I363d5a787863f1677ee40afb5d5841321ddaae77
Include the illegal instruction in the header if we get a
SIGILL. Otherwise (since these tend to be one-off bit flips), we don't
usually have any information to try to confirm our suspicion that any
given instance is actually a one-off bit flip.
Also add `SIGILL` as a crasher option to easily generate such crashes.
Before:
signal 4 (SIGILL), code 1 (ILL_ILLOPC), fault addr 0xab1456da
After:
signal 4 (SIGILL), code 1 (ILL_ILLOPC), fault addr 0xab1456da (*pc=0xe7f0def0)
Bug: http://b/77274448
Test: ran crasher
Change-Id: I5f8dedca5eea2b117b1b1e48430214b38e1366ed
adbd (and its dependencies) are marked as recovery_available:true so
that recovery version of the binary is built separately from the one for
system partition. This allows us to stop copying the system version to
the recovery partition and also opens up the way to enable shared
libraries in the recovery partition. Then we can also build adbd as a
dynamic executable.
Bug: 79146551
Test: m -j adbd.recovery
Change-Id: Ib95614c7435f9d0afc02a0c7d5ae1a94e439e32a
Switch from _exit to raising SIGABRT when we recurse in the fallback
handler, so that waiters see an abort instead of a regular exit.
Bug: http://b/79717060
Test: debuggerd_test32
Test: debuggerd_test64
Change-Id: Iddee1cb1b759690adf07bbb8cd0fda2faac87571
* New lld could create files that map to non-zero
offset at run time.
Test: debuggerd_test
Bug: 79590156
Change-Id: I12db0ebef489ba8a1e648a29d214f8d3c3703996
We can't actually link an unlinked file back onto disk if it wasn't
opened with O_TMPFILE. Switch to using a temporary filename instead.
Bug: http://b/77729983
Test: agampe
Change-Id: I1970497114f0056065a1ba65f6358f08b51ec551
70d8f28945 broke a test that was not
expecting to see the new detail about the signal's sender.
Bug: http://b/78594105
Test: ran tests
Change-Id: Idfa3a53b9e664308efdba560ffbb1401c1904530
We have a LOG(FATAL) that can potentially happen before we turn off
SIGABRT. Move the signal handler defusing to the very start of main.
Bug: http://b/77920633
Test: treehugger
Change-Id: I7a2f2a0f2bed16e54467388044eca254102aa6a0
Suicide doesn't change:
signal 6 (SIGABRT), code -6 (SI_TKILL), fault addr --------
But homicide now looks like this (this is `sleep 666` killed by
`kill -SEGV` as root:
signal 11 (SIGSEGV), code 0 (SI_USER from pid 4446, uid 0), fault addr --------
Bug: http://b/78594105
Test: manual
Change-Id: I8c2feafba8cc5a3db85e8250004d428a464c5d9e
Instead of creating tombstone FDs in place and passing them out to
crash_dump directly, create them as O_TMPFILEs and link them into place
when crash_dump reports success, to avoid creating empty tombstones
in cases like an aborting thread racing with another thread that
manages to cleanly exit_group before the dump finishes.
Bug: http://b/77729983
Test: debuggerd_test
Test: adb shell 'for x in `seq 0 50`; do crasher; done'
Change-Id: I31ce4fd4a524abf8bde57152450209483d9d0ba9
Let the logging implementation be the imposer of limits.
Bug: http://b/64759619
Test: debuggerd_test
Change-Id: I8bc73bf2301ce071668993b740880224846a4e75
The recent change to detect missing source files broke reading makefiles
for mips, since this didn't specify a source file.
Bug: 73904572
Test: lunch aosp_mips-eng; m nothing
Test: lunch aosp_arm-eng; m crash_dump.policy
Test: lunch aosp_arm64-eng; m crash_dump.policy
Test: lunch aosp_x86-eng; m crash_dump.policy
Test: lunch aosp_x86_64-eng; m crash_dump.policy
Change-Id: I28864b5af59267f1ab83084128f2c59b04039374
Calls to abort() will always result in our signal handler being called,
because abort will manually unblock SIGABRT before raising it. This
can lead to deadlock when handling address space exhaustion in the
fallback handler. To fix this, switch our mutex to a recursive mutex,
and manually keep track of our lock count.
Bug: http://b/72929749
Test: debuggerd_test --gtest_filter="CrasherTest.seccomp_crash_oom"
Change-Id: I609f263ce93550350b17757189326b627129d4a7
Add a comment explaining why we define PROT_READ/PROT_WRITE, even
though a current libminijail supports both cosntants.
Bug: http://b/73273658
Test: treehugger
Change-Id: I51c1be1b1b569e94dbc9045a90bc28221b7dc9c7
When generating crash_dump.*.policy, replace PROT_READ and PROT_WRITE
to numeric constants to make the policy backward compatible with old
libminijail.so.
Bug: 73273658
Test: use the new policy in OMR1 devices
Change-Id: I936a733340ad4df8aef6562c03eb10c29ffdada2
A race condition occurs when one thread takes more than a second to get
scheduled to handle the signal we send to ask it to dump its stack.
When this happens, the main thread will continue on, close the fd, and
then ask the next thread to dump, but the slow thread will then wake up
and try to write to the new thread's fd, or trigger an assertion in
__linker_enable_fallback_allocator.
Do a few things to make this less bad:
- encode both target tid and fd in the shared atomic, so that we know
who each fd is for
- switch __linker_enable_fallback_allocator to return success instead
of aborting, and bail out if it's already in use
- write to the output fd right when we get to it, instead of doing it
whenever the dumping code decides to, to reduce the likelihood that
the timeout expires
Test: debuggerd_test
Change-Id: Ife0f6dae388b601e7f991605f14d7a0274013f6b
Commit 3e235911 in bionic switched LP32's sigaction implementation over
to using the rt_sigaction syscall, matching LP64. Update our seccomp
policy to match.
Bug: http://b/73119572
Test: debuggerd_test32
Change-Id: I0a662a1c874298d434468d2dcdb4ebf9f276110c
Use the art dex file library to read the dex data.
Add unit tests for the UnwindDexFile code.
Bug: 72070049
Test: All unit tests continue to pass.
Test: Dumped the backtrace of the 137-cfi test while running in interpreter
Test: mode and verified that the stack trace is correct. Did this on host
Test: and for arm/arm64.
Change-Id: Ia6f343318c5dd6968a954015a7d59fdf101575b0
The stack dump was not printing leading zeros for data after the
change to remove uintptr_t types from the libbacktrace API.
Bug: 65682279
Test: Created an arm tombstone and an arm64 tombstone and verified
Test: that the stack data has leading zeros.
Change-Id: I1fbec2c4fa7c8b0fab18894c5628d18c5a580299
In order to support the offline unwinding properly, get rid of the
usage of non-fixed type uintptr_t from all API calls.
In addition, completely remove the old local and remote unwinding code
that used libunwind.
The next step will be to move the offline unwinding to the new unwinder.
Bug: 65682279
Test: Ran unit tests for libbacktrace/debuggerd.
Test: Ran debuggerd -b on a few arm and arm64 processes.
Test: Ran crasher and crasher64 and verified tombstones look correct.
Change-Id: Ib0c6cee3ad6785a102b74908a3d8e5e93e5c6b33
Set and restore PR_SET_PTRACER when performing a dump, so that when
Android is running on a kernel that has the Yama LSM enabled (and the
value of ptrace_scope is > 0), crash_dump can attach to processes and
print nice, symbolized stack traces.
Bug: 70992745
Test: kill -6 `pidof surfaceflinger` && logcat -d -b crash
# in both sailfish and Chrome OS
Change-Id: If4646442c6000fdcc69cf4ab95fdc71ae74baaaf
The abort message was accidentally relocated to be printed below the
registers, backtrace, and stack, which isn't very helpful. Move it back
to its rightful place.
Test: treehugger
Change-Id: I8aa5b63e58081f27ccdb42481fed8d9eb3a892a4
When a process crashes, both ActivityManager and init will try to kill
its process group when they notice. The recent change to minimize the
amount of time a process is paused results in crash dumps being killed
before they finish as a result of this. Since anything that needs to be
low-latency is probably not going to be too happy if it crashes, just
wait for completion whenever we're processing a real crash.
Bug: http://b/70343110
Test: debuggerd_test
Change-Id: I894bb06efd264b1ba005df06f7326a72f4b767bb
Add some helper macros that perform regex string matching to
<android-base/test_utils.h>.
Test: libbase_test32/64 on host
Change-Id: I1b0f03dc73f8b4fdfb8ac6c75d59ef421e0e9640
Add a benchmark to measure how long we pause a process when dumping.
Bug: http://b/62112103
Test: manually ran it
Change-Id: Iceec2f722915b0ae26144c86dcbeb35793f963da
Reduce the amount of time that a process remains paused by pausing its
threads, fetching their registers, and then performing unwinding on a
copy of its address space. This also works around a kernel change
that's in 4.9 that prevents ptrace from reading memory of processes
that we don't have immediate permissions to ptrace (even if we
previously ptraced them).
Bug: http://b/62112103
Bug: http://b/63989615
Test: treehugger
Change-Id: I7b9cc5dd8f54a354bc61f1bda0d2b7a8a55733c4
Add a static GetLoadBias method to the Elf object that only reads just
enough to get the load bias.
Add a method to MapInfo that gets the load bias. First attempt to get
it if the elf object already exists. If no elf object was created, use
the new static method to get the load bias.
In BacktraceMap, add a custom iterator so that when code dereferences
a map element, that's when the load bias will be retrieved if it hasn't
already been set.
Bug: 69871050
Test: New unit tests, verify tombstones have non-zero load bias values for
Test: libraries with a non-zero load bias.
Change-Id: I125f4abc827589957fce2f0df24b0f25d037d732
Always check to see if the fallback handler has been called and is
not trying to dump a specific thread.
Bug: 69110957
Test: Verified on a system where the prctl value changes, that before the
Test: change it dumps multiple tombstones, and after the change it
Test: works as expected.
Test: Ran debuggerd unit tests.
Test: Dumped process using debuggerd -b <PID> and debuggerd <PID>.
Change-Id: Id98bbe96cced9335f7c3e17088bb4ab2ad2e7a64
Nobody is looking at the mismatches, and it can cause problems
with tombstone parsers.
Also, fix the dump_header_info test and remove unused properties_fake.cpp.
Test: Ran unit tests, verified tombstones still work.
Change-Id: I4261646016b4e84b26a5aee72f3227f1ce48ec9a
This is needed if they will ever handle ro. properties that have
values longer than 92 characters.
Bug: 23102347
Bug: 34954705
Test: read and write properties with value length > 92 characters
Change-Id: I44aa135c97ec010f12162c30f743387810ae2c5d
Update the tests to match new output (and stop pluralizing '1 entries').
Test: `debuggerd_test{32,64} --gtest_filter="TombstoneTest.*" on hikey960
Change-Id: I16b0335715303252fad3a35d6a053a50fefdac30
Tombstones (especially ones with lots of VMAs) are regularly truncated.
We can at least show the number of VMAs, though, for anyone interested
in knowing whether they got close to the default 64Ki limit.
Bug: http://b/66911122
Test: ran crasher, examined tombstone
Change-Id: I286db66f28f132307d573dbe5164efc969dc6ddc
Apply the same fix from c2e98f63 to intercept_manager.cpp.
Bug: http://b/64543673
Test: debuggerd_test
Change-Id: Ibfb919e059fa62f8336cfc1426d03ef015590136
If a function crashes by jumping into unexecutable code, the old method
could not unwind through that. Add a fallback method to set the pc from
the default return address location.
In addition, add a new finished check for steps. This will provide a method
to indicate that this step is the last step. This prevents cases where
the fallback method might be triggered incorrectly.
Update the libbacktrace code to unwind using the new methodology.
Update the unwind tool to use the new unwind methodology.
Add a new option to crasher that calls through a null function.
Create a new object, Unwinder, that encapsulates the a basic unwind. For now,
libbacktrace will still use the custom code.
Added new unit tests to cover the new cases. Also add a test that
crashes calling a nullptr as a function, and then has call frames in
the signal stack.
Bug: 65842173
Test: Pass all unit tests, verify crasher dumps properly.
Change-Id: Ia18430ab107e9f7bdf0e14a9b74710b1280bd7f4
Android.bp assumed only an armv7-a-neon core needs to set HAS_VFP_D32.
In fact, an armv8 core also has 32 double-word floating point registers
for A32 and T32 ISAs (AArch32 or 32-bit armv8).
Bug: 65568426
Test: lunch aosp_arm64; emulator # on oc-mr1-dev; boot to home screen.
Check crashglue.o actually uses VFP_D16-31 for 32-bit armv8 core.
Change-Id: I34584a27fa24a55bb4809ccd7f99a8122971df0e
The order of arguments is wrong - we're passing flags=static_cast<unsigned>(-1)
and backlog=LEV_OPT_CLOSE_ON_FREE (which is 2).
On versions of libevent prior to 2.1.8, this ends up accidentally setting
OPT_LEAVE_SOCKETS_BLOCKING, OPT_CLOSE_ON_EXEC, OPT_REUSABLE and OPT_THREADSAFE
and limiting our backlog to two. These unintentional changes are relatively
benign; we never make our sockets block, we never exec, we never reuse
sockets and the additional locking overhead should be negligible. The
backlog of two might be a problem in theory, but there haven't been any
reports of issues caused by it.
Things get worse on 2.1.8 - that version introduces several new flags,
one of which is OPT_DISABLED. This disables the new listener by default,
which means that our event loop returns early because it has no active listeners
for any of its events.
Bug: 64543673
Test: Manual.
Change-Id: I9954bc7fe1af761de1a950d935dd2e6ce7e2c5f5
Move libdebuggerd headers into their own directory for namespacing,
move some includes to the top of their implementing files, delete some
dead code.
Test: mma, treehugger
Change-Id: Ie4c44e32e2ab3bc678092899d257fd4ed634aa34
Instead of printing a useless "ptrace attach failed: strerror(EPERM)"
message, print the name and pid of a competing tracer when we fail to
attach because a process is already being ptraced.
Bug: http://b/31531918
Test: debuggerd_test32, debuggerd_test64 on aosp_angler
Test: strace -p `pidof surfaceflinger`; debuggerd -b surfaceflinger
Change-Id: Ifd3f80fe03de30ff38c0e0068560a7b12875f29d
In debuggerd, when dumping a tombstone, run the new unwinder and verify
the old and new unwinder are the same. If not, dump enough information
in the tombstones to figure out how to duplicate the failure.
Bug: 23762183
Test: Builds, ran and forced a mismatch and verified output.
Change-Id: Ia178bde64d67e623d4f35086ebda68aebbff0c3c
- Change the field name load_base to load_bias (which is what it really is).
- Add a rel_pc field so that callers do not need to compute it themselves.
- Remove the BacktraceMap::GetRelativePc() since nobody should need to
compute this themselves.
Bug: 23762183
Test: Compiles and unit tests pass (debuggerd, libbacktrace).
Change-Id: I2cb579767120adf08c407a58f3c487ee3f2b45fc
Print diagnostics when the user requests a dump that is guaranteed to
fail, such as trying to dump a process you can't send a signal to.
Bug: http://b/63008395
Change-Id: I5c6bf2a5751f858e0534990b8d2ab6932eb9f11d
Test: manually tested
Some processes have lots of threads and minidebug-info. Unwinding
these can take more than the original two seconds.
Bug: 62828735
Test: m
Test: debuggerd_test
Test: adb shell kill -s 6 `pid system_server`
Change-Id: I0041bd01753135ef9d86783a3c6a5cbca1c5bbad
The debuggerd_client.race tests the crash_dump process to finalize the
killed process within 2 seconds. The 2 seconds timeout for finalizing a
process, which has 1024 threads, is bit small for low-speed devices.
This CL lowers the bar in order to make such devices pass the test.
Wraping up 128 threads within 2 seconds looks safe.
Bug: 62600479
Test: debuggerd_test passes on low-speed devices.
Change-Id: I3089415961422e6933405d2c872913273425caff
For java traces, log the kind of dump as well as the PID of the
completed dump. This makes it easier to correlate dump requests with the
actual file they're written to.
Sample log statement:
E /system/bin/tombstoned: Traces for pid 4737 written to: /data/anr/trace_00
The message for native traces / tombstones remains unchanged because
several tools parse it.
Test: manual
Bug: 32064548
Change-Id: I7b3792dd5ae312ee0bc055c22ec3f7c747152072
The only case where tombstoned creates files for java traces is
when the process is signalled "by hand" using "shell kill -3", or
by the program itself. Such traces do not correspond to an ANR, so
name those files "trace_XX".
When dumpstate / system_server want to dump java traces, they set up
a tombstoned intercept and manage the lifetime of any associated file
that themselves.
Bug: 32064548
Test: manual, debuggerd_test
Change-Id: I97006ec7c0cd35de4b9564f535e77af846cc3891
Example:
signal 5 (SIGTRAP), code -32763 (PTRACE_EVENT_STOP), fault addr 0x274e00005fb3
I'm tempted to say that %d isn't the best choice for si_code, but as long as
we're fully decoding all the values, I don't think it matters.
Bug: http://b/62856172
Test: manual debuggerd run
Change-Id: Ieeca690828e1e12f4162bbadece53f4aa7b9537a
Kernel can use vsyscall for system calls. The vsyscall implementation in
the kernel gives one more depth in the backtrace. It leads to failures
on CrasherTest. This CL makes tests find a system call frame not only in
the first line but also in all lines on the backtrace.
Bug: 62600694
Test: passes all CrasherTests.
Change-Id: Ice383bb94db097e7e9a9e4f74d8fa5ecc528122a
Make it easy to find out where a specific crash's tombstone was written
to by adding a log.
Bug: http://b/62268830
Test: crasher
Change-Id: I1961dfb19f76a42a8448ebafd4be153b73cb6800
Don't pause the threads we're going to dump until after we're about to
fetch their backtraces.
Bug: http://b/62112103
Test: debuggerd_test
Change-Id: Id7ab0464842b35f98f3b3ebc42fb76161d8afbd2
Add some tracing to figure out where time is going during a dump.
Bug: http://b/62112103
Test: systrace.py sched freq idle bionic
Change-Id: Ic2a212beeb0bb0350b4d9c2cd7a4e70adc97752d
The SELinux changes that this depends on have now landed.
This change also adds a few lower level unit tests of intercept
functionality.
Test: make; debuggerd_test
Change-Id: I0be5e85e7097e26b71db269c9ed92d9b438bfb28
If a process tries to dump itself (e.g. system_server during ANRs),
crash_dump will block trying to write to its pipe if it's not
sufficiently large. Increase the pipe size to the max, and add a test
to make sure that it's always at least 1MB (the default value).
Bug: http://b/38427757
Test: debuggerd_test
Change-Id: Iddb0cb1e5ce9e687efa9e94c2748a1edfe09f119
crash_dump inherits its signal mask from the thread that forked it,
which always has all of its signals blocked, now that sigchain respects
sa_mask.
Manually clear the signal mask, and reduce the timeout to a
still-generous 2 seconds.
Bug: http://b/38427757
Test: manually inserted sleep in crash_dump
Change-Id: If1c9adb68777b71fb19d9b0f47d6998733ed8f52
All intercept requests and crash dump requests must now specify a
dump_type, which can be one of kDebuggerdNativeBacktrace,
kDebuggerdTombstone or kDebuggerdJavaBacktrace. Each process can have
only one outstanding intercept registered at a time.
There's only one non-trivial change in this changeset; and that is
to crash_dump. We now pass the type of dump via a command line
argument instead of inferring it from the (resent) signal, this allows
us to connect to tombstoned before we wait for the signal as the
protocol requires.
Test: debuggerd_test
Change-Id: I189b215acfecd08ac52ab29117e3465da00e3a37
.. for ART and the frameworks to link against. In the new stack dumping
scheme (see related bug), the Java runtime will communicate with
tombstoned in order to obtain a FD to which it can write its traces.
Also move things around to separate headers that are private
implementation details from headers that constitute the public debuggerd
API. There are currently only three such headers :
- tombstoned/tombstoned.h
- debuggerd/client.h
- debuggerd/handler.h
Bug: 32064548
Test: make
Change-Id: If1b8578550e373d84828b180bbe585f1088d1aa3
The changes here involve :
- Creating and opening a new socket to receive trace dump requests on. Having
different sockets allows us to install different sets of access control rules.
- A minor refactor to allow us to share common pieces of implementation
between the java and native dumping code. This will also allow us to
add a unit test for all file / directory related logic.
There are two java trace specific additions here :
- We use SO_PEERCRED instead of trusting the PID written to the seocket
because requests come in from untrusted processes.
- Java trace dumps are not interceptible.
kJavaTraceDumpsEnabled is set to false for now but the value of the flag
will be flipped in a future change.
Bug: 32064548
Test: Manual; Currently working on a unit_test for CrashType.
Change-Id: I1d62cc7a7035fd500c3e2b831704a2934d725e35
bionic's cached values for getpid/gettid can be invalid if the crashing
process manually invoked clone to create a thread or process, which
will lead the crash_dump refusing to do anything, because it sees the
actual values.
Use the getpid/gettid syscalls directly to ensure correct values on
this end.
Bug: http://b/37769298
Test: debuggerd_test
Change-Id: I0b1e652beb1a66e564a48b88ed7fa971d61c6ff9
Move the name of the "private/libc_logging.h" header to <async_safe/log.h>.
For use of libc_malloc_debug_backtrace, remove the libc_logging library.
The library now includes the async safe log functions.
Remove the references to libc_logging.cpp in liblog, it isn't needed because
the code is already protected by a check of the __ANDROID__ define.
Test: Compiled and boot bullhead device.
Test: Run debuggerd unit tests.
Test: Run liblog unit tests on target and host.
Test: Run libmemunreachable unit tests (these tests are flaky though).
Change-Id: Ie79d7274febc31f210b610a2c4da958b5304e402
Applications can set abort messages via android_set_abort_message
without actually aborting. This leads to following non-fatal dumps
printing their output to logcat in the same format as a regular crash.
Bug: http://b/37754992
Test: debuggerd_test
Change-Id: I9c5e942984dfda36448860202b0ff1c2950bdd07
Crashes that happen before tombstoned is running are extremely hard to
diagnose, because tombstones aren't written to disk, and the window of
opportunity to get logs via `adb logcat` is small (potentially
nonexistent).
Solve this by adding a world-writable /dev/kmsg_debug on userdebug
builds, and writing to it in addition to logcat when tombstoned hasn't
started yet.
Bug: http://b/36574794
Test: stop tombstoned; crasher; dmesg
Change-Id: Ib22c02a002afb602933155fb2c9b7a8abbe9ed38
If send_signal failed, debuggerd_trigger_dump should return
instead of further dealing
Change-Id: I0a1ac4255344d1da7a21917ad559b9de4d5a4fee
Signed-off-by: Liu Changcheng <changcheng.liu@intel.com>
Signed-off-by: Jerry Liu <primerlink@gmail.com>
Make sure that we can get the process/thread names for nondumpable
processes with capabilities.
Bug: http://b/36237221
Test: /data/nativetest/debuggerd_test/debuggerd_test32
Change-Id: Iedd4eae18065c2e64abeebff15e3b9b09a630550
Add some tests for edge cases which may have triggered b/36685795.
Bug: http://b/36685795
Test: debuggerd_test
Change-Id: I20670684c8dae422af157be21e44ba5d6d3214d3
Previously, there was no way to detect when tombstoned processed an
intercept request packet, making it possible for a intercept request
followed by a crash_dump being processed in the wrong order.
Add a response to intercept registration, to eliminate this race.
Test: debuggerd_test
Change-Id: If38c6d14081ebc86ff1ed0edd7afaeafc40a8381
Also make it loop, so that upon failing to start a dequeued crash
request, we continue to the next one.
Bug: http://b/36685795
Test: debuggerd_test
Change-Id: I94889125f16f4681c6fa0fa9cac456302602ce01
Previously, we would increment num_concurrent_dumps and fail to
decrement it if we failed to start the request. Change this to
only increment after we've successfully started the dump.
Bug: http://b/36685795
Test: debuggerd_test
Change-Id: I66169ed56ed44271e1d8fe1298d95260be7a32a3
Use an intermediate pipe to detect and report when a requested dump has
completed.
Bug: http://b/35241370
Bug: http://b/35813071
Test: debuggerd_test
Test: manually triggered a background ANR
Change-Id: If14aedf6071288360f1a7853d5a2ee79db121759
This just means we were asked to dump, not that something necessarily went
wrong.
Bug: http://b/36191903
Test: builds
Change-Id: I5638b38f3a13081b1e971512f43238010febb59c
There was at least one failure due to si_code being unitialized
and then examined.
Test: Run the 32 bit and 64 bit version of the unit tests on angler.
Change-Id: I5455a2cd29afafcd26a49f696e61141bb48478dc
It's possible that a device map has memory controlled by a single entry
device driver. Thus, you can deadlock if a process is touching that
device memory and we try to unwind it and also touch that device memory.
Simply skip any attempts to step through, or get function names from
device memory maps.
Bug: 36130325
Test: Ran new unit tests, ran bionic unit tests, ran art ThreadStress.
Change-Id: Ibc62d7ec8106c619ee08968f05e04aea55d7cbfa
Processes that don't have dumpable set to 1 cannot have their
process/thread names read by processes that don't have all of their
capabilities. Fetch these names in crash_dump before dropping
privileges.
Bug: http://b/36237221
Test: debuggerd_test
Test: debuggerd -b `pidof android.hardware.bluetooth@1.0-service`
Change-Id: I174769e7b3c1ea9f11f9c8cbdff83028a4225783
The .clang-format files in the base, debuggerd, adb, libprocinfo, and
fastboot subdirectories each differ slightly from the top level
.clang-format-2 and .clang-format-4, but not in a substantially
meaningful way, as the source files in those directories have not been
re-formatted with clang-format. Therefore, let's reduce the
differences and use only the two top level clang-format files.
Secondly perform some small clean-up of the top level .clang-format
files. AllowShortBlocksOnASingleLine is already false in the Google
style, so it can be removed. AllowShortFunctionsOnASingleLine should
not change between the -2 and -4 versions, so leave it at the Google
default style in both, which is 'All'.
The diff stats for these changes are:
./base/
Old:
640 insertions(+), 531 deletions(-)
New:
563 insertions(+), 808 deletions(-)
./debuggerd/
Old:
910 insertions(+), 886 deletions(-)
New:
991 insertions(+), 1023 deletions(-)
./adb/
Old:
2623 insertions(+), 2886 deletions(-)
New:
2655 insertions(+), 3103 deletions(-)
./libprocinfo/
Old:
2 insertions(+), 1 deletion(-)
New:
4 insertions(+), 18 deletions(-)
./fastboot/
Old:
618 insertions(+), 743 deletions(-)
New:
726 insertions(+), 882 deletions(-)
./init/
Old:
1755 insertions(+), 1866 deletions(-)
New:
1715 insertions(+), 1952 deletions(-)
Test: Above clang-format stats
Change-Id: I3f7b8ab0660c8394c5008ba95ea15e70dd22b55b
If a process that's getting dumped dies before crash_dump starts (e.g.
because seccomp immediately kills it after it execs crash_dump),
improve the error message to not just say "target died before we could
attach".
Bug: http://b/36077710
Test: inserted an exit in the handler, inspected output
Change-Id: I7d394c66d60d328b096b15654b3648e1ed711728
Avoid breaking internal code when AOSP automerges to internal.
This will be reverted after fixing up the uses on the other end.
Bug: http://b/35858739
Test: treehugger
Change-Id: If1ee03d8d7c218d3ad9f451cfe9a9077753dda02
So _LOG could be overridden by customized logging
implementations in non-Android systems.
Bug: 35919515
Test: Test on device
Change-Id: I0885c15353c0b1bf66f6f156e7f502f326b85d57
