874708 - Nightly crash on startup Galaxy S4 ( SIV ) GT-I9505

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Kevin Brosnan [Ex-Mozilla]]

We are crashing on startup. No crash id. This bit of the log looks interesting. CCing a Brian/Sriram as the device is in MV on my desk.

05-21 17:26:02.543 D/GeckoProfile(20291): Found profile dir: /data/data/org.mozilla.fennec/files/mozilla/2ziuifha.default
05-21 17:26:02.563 D/GeckoFavicons(20291): Creating Favicons instance
05-21 17:26:02.573 D/qdoverlay(  213): Unset pipe=VG0 dpy=0; Unset pipe=RGB1 dpy=0; 
05-21 17:26:02.593 D/skia    (20291): new locale en-Latn-US
05-21 17:26:02.613 D/AbsListView(20291): onVisibilityChanged() is called, visibility : 8
05-21 17:26:02.613 D/AbsListView(20291): unregisterIRListener() is called 
05-21 17:26:02.623 D/AbsListView(20291): Get MotionRecognitionManager
05-21 17:26:02.623 D/MotionRecognitionService(  734):  ssp status : true
05-21 17:26:02.633 D/libEGL  (20291): loaded /system/lib/egl/libEGL_adreno200.so
05-21 17:26:02.633 D/libEGL  (20291): loaded /system/lib/egl/libGLESv1_CM_adreno200.so
05-21 17:26:02.633 D/libEGL  (20291): loaded /system/lib/egl/libGLESv2_adreno200.so
05-21 17:26:02.643 I/Adreno200-EGL(20291): <qeglDrvAPI_eglInitialize:265>: EGL 1.4 QUALCOMM build:  (CL3544079)
05-21 17:26:02.643 I/Adreno200-EGL(20291): Build Date: 03/28/13 Thu
05-21 17:26:02.643 I/Adreno200-EGL(20291): Local Branch: adreno_20130328
05-21 17:26:02.643 I/Adreno200-EGL(20291): Remote Branch: 
05-21 17:26:02.643 I/Adreno200-EGL(20291): Local Patches: 
05-21 17:26:02.643 I/Adreno200-EGL(20291): Reconstruct Branch: 
05-21 17:26:02.643 D/GeckoViewsFactory(20291): Warning: unknown custom view: org.mozilla.gecko.BrowserToolbarLayout
05-21 17:26:02.643 D/GeckoViewsFactory(20291): Warning: unknown custom view: org.mozilla.gecko.BrowserToolbarLayout
05-21 17:26:02.663 D/GeckoViewsFactory(20291): Warning: unknown custom view: org.mozilla.gecko.TabCounter
05-21 17:26:02.663 D/GeckoViewsFactory(20291): Warning: unknown custom view: org.mozilla.gecko.TabCounter

Comment 1

[Aaron Train [:aaronmt]]

Crash on my device (GT-I9505) as well. We can't startup-crash on flagship devices; so tracking 24.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Logcat from debug build

I threw my local debug build on Aaron's device and that crashes too. Based on the logcat output (attached) it looks like it's crashing in loadSQLiteLibsNative in mozglue. I can't jimdb the SIGSEGV because the device isn't rooted.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Bisecting inbound builds shows this was caused by ef5b7b1039ac, bug 848764.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Also for some reason the "run-as" command on Galaxy S4 seems to not find the packages specified, so I can't attach the gdbserver on this device without rooting it. I don't even know if rooting it will work, but I assume it will.

Using package org.mozilla.fennec_kats.
Launching org.mozilla.fennec_kats... Done
Attaching to pid 22482... as non-root... as root... 
"gdbserver" output:
 Cannot attach to lwp 22482: Operation not permitted (1)
 Exiting
"run-as" output:
 run-as: Package 'org.mozilla.fennec_kats' is unknown
"su -c" output:
 /system/bin/sh: su: not found
/Users/kats/android/jdb/moz-gdb/bin/../utils/gdbinit:133: Error in sourced command file:
failed to run gdbserver
(gdb) q

Comment 5

[Mike Hommey [:glandium]]

Can you build with DEFINES += -DMOZ_DEBUG_LINKER added to mozglue/linker/Makefile.in and attach a new logcat?

Comment 6

[Aaron Train [:aaronmt]]

Attachment: Log with linker debugging

Using http://people.mozilla.com/~kgupta/tmp/linker.apk

Comment 7

[Aaron Train [:aaronmt]]

Attachment: More logs with dump

Log from http://people.mozilla.org/~mhommey/fennec-24.0a1.en-US.android-arm.apk

Comment 8

[Mike Hommey [:glandium]]

So, thanks to a newer log from Aaron mapped to a build where i had the debugging info, I can interpret from that attached log:

> Caught segmentation fault @0x6e6d6409
So here, the linker is catching a segfault. The log itself comes from http://hg.mozilla.org/mozilla-central/file/8eebe35aae63/mozglue/linker/ElfLoader.cpp#l827 and the printed address is info->si_addr.
It happens to be inside a thread stack, so it doesn't make much sense that there would be an access error there, except if we tried to execute it, but it seems unlikely. The other weird thing is that we don't even get to the segfault handler printing something when mozglue is built with optimizations on.

> Redispatching to registered handler @0x401bec51
Since that address doesn't match a mapped library, it passes to another signal handler, which happens to be bionic's. And it calls the handler with the exact parameters it got.

> Fatal signal 11 (SIGSEGV), thread 5992 (Gecko)
This is bionic's signal handler displaying info about the crash, printed from https://github.com/android/platform_bionic/blob/master/linker/debugger.cpp#L138
It then connects to the debuggerd daemon to give it some info about the crash https://github.com/android/platform_bionic/blob/master/linker/debugger.cpp#L190
and then it returns and segfault for real, for debuggerd to catch and display. So theoretically, debuggerd is getting the same crash that triggered the signal handler in the first place...

> signal 11 (SIGSEGV), code 2 (SEGV_ACCERR), fault addr 798444c8
... but the address debuggerd gives for the fault address (and it uses info->si_addr for that, cf. https://github.com/android/platform_system_core/blob/d7cab8bff1334ba48620a16d1b98f2ae623fee7d/debuggerd/tombstone.c#L180 ) is different... That address corresponds to the linker itself reading DT_DYNAMIC data from libnss3.so in http://hg.mozilla.org/mozilla-central/file/8eebe35aae63/mozglue/linker/CustomElf.cpp#l470

Comment 9

[Kartikaya Gupta (email:[e-mail])]

For posterity, glandium and I discussed this a little on IRC yesterday. I noticed that this output:

> Fatal signal 11 (SIGSEGV), thread 5992 (Gecko)

actually comes from https://github.com/android/platform_bionic/blob/master/linker/debugger.cpp#L143 (line 143 instead of line 138) which means that either (1) debuggerd_signal_handler is getting called with a null siginfo_t or (2) the haveSiginfo function is returning false. These imply that there is another signal handler chained somewhere, either between us and bionic in case (1), or on top of us in case (2).

However, we wrap the signal and sigaction functions in mozglue. So it seems pretty unlikely that (2) would ever happen, because any attempt to do that would our wrapper which moves the signal handler down the chain to below our handler. So it is likely to be case (1). The other odd thing is the stack address we are getting in si_addr, as glandium mentioned in the last comment. This can be explained if there is a signal handler on top of ours that is not using SA_SIGINFO, but because we wrap the functions I don't see how that could happen. It doesn't make much sense either way.

Comment 10

[Aaron Train [:aaronmt]]

Using an updated build from Mike with a fix and that works for me (as conversed in IRC). I think it's fine with getting a bandaid fix up there for Nightly users before diving into the crux of the issue with on-demand decompression on the S4.

Comment 11

[Mike Hommey [:glandium]]

Well, the build with a mitigation was the one crashing on the compositor ; the build that didn't crash essentially had on-demand decompression disabled, but by means of decompressing everything ahead, not by actually backing out 848764. I think a reasonable way forward here is to setup a blacklist of devices where we disable on-demand decompression at runtime.

Comment 12

[Mike Hommey [:glandium]]

So, thanks to some tracking I added in a custom build that Aaron ran, I was able to figure out what is going on:
libsc-a3xx.so is essentially doing this:
  sighandler_t old6 = signal(6, my_handler);
  sighandler_t old11 = signal(11, my_handler);
  do_stuff();
  signal(6, old6);
  signal(11, old11);

This effectively resets the flags set with sigaction and when we're back to our segfault handler and it's invoked, the siginfo and context arguments are not set by the kernel because of the lack of SA_SIGINFO flag. Also, since we're multi-threaded, when one thread is in do_stuff() and another should be hitting our sigsegv handler, it doesn't, and that crashes; that's why the first attempt at fixing things failed with crashes in the compositor. Also, I don't know what their segfault handler does, but if it happens that there is a legitimate crash happening when it's set on a separate thread, we may lose crash reports (although that's probably a corner case).

I have a working patch that makes fennec work on the S4 without the workaround from bug 875824, but it needs to be modified to work on x86 and validated to work on armv6 (it only supports arm and thumb and was only tested on thumb). That patch essentially replaces the first instructions in libc's bsd_signal, sysv_signal and sigaction with a branch instruction calling into our own wrappers, which then call the sigaction system call directly. This happens to be enough on android because the sigaction libc stub is mostly just calling the kernel directly. It is not the case on glibc systems, though, and i suspect android may at some point in the future add some things like a default restorer such that gdb will be happier with signal handlers.

So I think it would be better if we could call back into the original functions, but since we're modifying them, it means we'd need something like what we're doing in the DllInterceptor on Windows, which is more complicated and fragile. Another option would be to look for all GOT entries pointing to these functions in the libraries that have already been loaded and change them to point to our wrappers, but it might fail in some subtle ways, especially with prelinking. It might also add some unwanted startup overhead.

Comment 13

[Mike Hommey [:glandium]]

Attachment: WIP

This is the current cleaned up work in progress (as in, with all the additional debugging removed)

Comment 14

[Mike Hommey [:glandium]]

Attachment: WIP

One more cleanup.

Comment 15

[Kartikaya Gupta (email:[e-mail])]

I'm wondering if it's worth putting in this workaround at all. The SGS4 is a pretty beefy device and may not benefit that much from decompression-on-demand anyway. Unless there are other devices that are also affected we could just disable the decompression-on-demand for this device and be done with it.

Comment 16

[Mike Hommey [:glandium]]

I suspect all devices with an adreno 3xx are affected. Thanks Qualcomm.

That being said, even on a device like the S4, i think on-demand decompression can have a great benefit, combined with binary reordering.

Comment 17

[Mike Hommey [:glandium]]

Attachment: Hook libc's sigaction to avoid system libraries replacing our segfault handler temporarily and restoring it wrongly

Comment 18

[Mike Hommey [:glandium]]

Comment on attachment 754907 [details] [diff] [review]
Hook libc's sigaction to avoid system libraries replacing our segfault handler temporarily and restoring it wrongly

Apparently, this breaks android x86.

Comment 19

[Mike Hommey [:glandium]]

Attachment: Hook libc's sigaction to avoid system libraries replacing our segfault handler temporarily and restoring it wrongly

Comment 20

[Nathan Froyd [:froydnj]]

Comment on attachment 755350 [details] [diff] [review]
Hook libc's sigaction to avoid system libraries replacing our segfault handler temporarily and restoring it wrongly

Review of attachment 755350 [details] [diff] [review]:
-----------------------------------------------------------------

I love working with libraries and signals, don't you?

Comment 21

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f228b1d9566
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ef966de04d5
(this doesn't reopen bug 875824)

Comment 22

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8ef966de04d5

Comment 23

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1f228b1d9566