1030899 - [Android L Preview] - Crash on startup - java.lang.UnsatisfiedLinkError: dlopen failed: cannot locate symbol "issetugid" referenced by "libmozglue.so"...

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aaron Train [:aaronmt]]

E/art     ( 7779): dlopen("/data/app-lib/org.mozilla.firefox-1/libmozglue.so", RTLD_LAZY) failed: dlopen failed: cannot locate symbol "issetugid" referenced by "libmozglue.so"...
D/AndroidRuntime( 7779): Shutting down VM
E/AndroidRuntime( 7779): FATAL EXCEPTION: main
E/AndroidRuntime( 7779): Process: org.mozilla.firefox, PID: 7779
E/AndroidRuntime( 7779): java.lang.UnsatisfiedLinkError: dlopen failed: cannot locate symbol "issetugid" referenced by "libmozglue.so"...
E/AndroidRuntime( 7779): 	at java.lang.Runtime.loadLibrary(Runtime.java:364)
E/AndroidRuntime( 7779): 	at java.lang.System.loadLibrary(System.java:610)
E/AndroidRuntime( 7779): 	at org.mozilla.gecko.mozglue.GeckoLoader.loadMozGlue(GeckoLoader.java:254)
E/AndroidRuntime( 7779): 	at org.mozilla.gecko.GeckoApplication.onCreate(GeckoApplication.java:100)
E/AndroidRuntime( 7779): 	at android.app.Instrumentation.callApplicationOnCreate(Instrumentation.java:1008)
E/AndroidRuntime( 7779): 	at android.app.ActivityThread.handleBindApplication(ActivityThread.java:4397)
E/AndroidRuntime( 7779): 	at android.app.ActivityThread.access$1500(ActivityThread.java:143)
E/AndroidRuntime( 7779): 	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1317)
E/AndroidRuntime( 7779): 	at android.os.Handler.dispatchMessage(Handler.java:102)
E/AndroidRuntime( 7779): 	at android.os.Looper.loop(Looper.java:135)
E/AndroidRuntime( 7779): 	at android.app.ActivityThread.main(ActivityThread.java:5070)
E/AndroidRuntime( 7779): 	at java.lang.reflect.Method.invoke(Native Method)
E/AndroidRuntime( 7779): 	at java.lang.reflect.Method.invoke(Method.java:372)
E/AndroidRuntime( 7779): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:836)
E/AndroidRuntime( 7779): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:631)
I/am_crash(  612): [7779,0,org.mozilla.firefox,11058756,java.lang.UnsatisfiedLinkError,dlopen failed: cannot locate symbol "issetugid" referenced by "libmozglue.so"...,Runtime.java,364]

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Looks like jemalloc uses this. It's in the NDK, so this is a bug in the preview.

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

We should file a bug on the Android tracker. Snorp would you do that?

Comment 3

[Makoto Kato [:m_kato]]

Removed by 
https://android.googlesource.com/platform/bionic/+/b4e099c6f741835b11ecb6f8fd0861b1892ff182

But this function is re-defined at upstream for Firefox.
https://android.googlesource.com/platform/bionic/+/bffbfeed7a595dcbe5843a77d84c409a0225b4e1

I don't know what revision L preview is built.

But issetugid implementation is that it always return 0.  should we remove issetugid code from jemalloc and libevent?

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Makoto Kato (:m_kato) from comment #3)
> Removed by 
> https://android.googlesource.com/platform/bionic/+/
> b4e099c6f741835b11ecb6f8fd0861b1892ff182
> 
> But this function is re-defined at upstream for Firefox.
> https://android.googlesource.com/platform/bionic/+/
> bffbfeed7a595dcbe5843a77d84c409a0225b4e1
> 
> I don't know what revision L preview is built.
> 
> But issetugid implementation is that it always return 0.  should we remove
> issetugid code from jemalloc and libevent?

Looks like that's what we need to do

Comment 7

[Robert Kaiser]

Do we have actual crash reports for that? I'd like to see what kind of data we see in crash reports for Android L. :)

Comment 8

[Kristian Erik Hermansen]

(In reply to Robert Kaiser (:[e-mail]) from comment #7)
> Do we have actual crash reports for that? I'd like to see what kind of data
> we see in crash reports for Android L. :)

I submitted a crash report in Android's built in system. Do you have access to those? I presume they get to you somehow?

Comment 9

[enh]

this is the case in L preview but if you look at AOSP master you'll see i added the backward-compatibility "do nothing" implementation for LP32. you'll need to move away from this for LP64 though, since there won't be an issetugid there, so fixing this bug isn't wasted work.

on an internal master device right now Firefox doesn't start because of inet_nsap_ntoa, but that looks like my fault. will investigate...

Comment 10

[Vlad Filippov (:vladikoff)]

Removing "issetugid" and building that works.
I got it running on the new AndroidTV with the L build: https://vine.co/v/MFtVhKaz9th

The pages do not load yet, but the interface loads. That issue could be limited to just AndroidTV.

Comment 11

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to enh from comment #9)
> this is the case in L preview but if you look at AOSP master you'll see i
> added the backward-compatibility "do nothing" implementation for LP32.
> you'll need to move away from this for LP64 though, since there won't be an
> issetugid there, so fixing this bug isn't wasted work.
> 
> on an internal master device right now Firefox doesn't start because of
> inet_nsap_ntoa, but that looks like my fault. will investigate...

Thanks for the update!

Comment 12

[enh]

https://android-review.googlesource.com/99800 fixes the inet_nsap_ntoa issue. there's still an nsdispatch issue for me to look at (even though that's not in glibc).

Comment 13

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Vlad Filippov from comment #10)
> Removing "issetugid" and building that works.
> I got it running on the new AndroidTV with the L build:
> https://vine.co/v/MFtVhKaz9th
> 
> The pages do not load yet, but the interface loads. That issue could be
> limited to just AndroidTV.

Do we have a bug on the AndroidTV stuff? If not, could you file one? :)

Comment 14

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to enh from comment #12)
> https://android-review.googlesource.com/99800 fixes the inet_nsap_ntoa
> issue. there's still an nsdispatch issue for me to look at (even though
> that's not in glibc).

Why does it matter that it's not in glibc? The NDK has it, AFAICT.

Comment 15

[Robert Kaiser]

(In reply to Kristian Erik Hermansen from comment #8)
> I submitted a crash report in Android's built in system. Do you have access
> to those? I presume they get to you somehow?

Most people at Mozilla, including me, don't have access to anything submitted via Android's reporter. That said, those wouldn't give me the data I'm interested in here anyhow.

Comment 16

[enh]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #14)
> (In reply to enh from comment #12)
> > https://android-review.googlesource.com/99800 fixes the inet_nsap_ntoa
> > issue. there's still an nsdispatch issue for me to look at (even though
> > that's not in glibc).
> 
> Why does it matter that it's not in glibc? The NDK has it, AFAICT.

yes, so for LP32 there will at least be a compatibility stub. but the fact that it's not portable makes it more questionable for LP64. that BSD has it is a plus, but that we have some random hacked up implementation that claims to be a particular revision of NetBSD (but clearly isn't) is a negative.

Comment 17

[Sylvestre Ledru [:Sylvestre]]

Tracking. We will probably take a patch for beta (31) if available.

Comment 18

[enh]

https://android-review.googlesource.com/99793 fixes the visibility of the BSD nsdispatch function.

the current Firefox beta now fails to start with this:

06-30 21:19:25.961  7532  7582 W System.err: java.lang.NoSuchFieldError: no "I" field "mEGLSurface" in class "Lcom/google/android/gles_jni/EGLSurfaceImpl;" or its superclasses
06-30 21:19:25.962  7532  7582 W System.err: 	at org.mozilla.gecko.GeckoAppShell.nativeInit(Native Method)
06-30 21:19:25.962  7532  7582 W System.err: 	at org.mozilla.gecko.GeckoAppShell.runGecko(GeckoAppShell.java:320)
06-30 21:19:25.962  7532  7582 W System.err: 	at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:174)

which is something you'll probably want to deal with on a separate bug. (any private fields you're pulling out of Android classes where the field corresponds to a pointer will be a long in L, on both LP32 and LP64. you'll need to fix your JNI.)

Comment 19

[Kevin Brosnan [Ex-Mozilla]]

Filed bug 1032460.

Comment 20

[Makoto Kato [:m_kato]]

Attachment: Don't use issetguid

Comment 21

[Makoto Kato [:m_kato]]

Comment on attachment 8450771 [details] [diff] [review]
Don't use issetguid

Bionic implementation is that issetguid always return 0.  Also this function is removed by Android L.

Comment 22

[Mike Hommey [:glandium]]

Comment on attachment 8450771 [details] [diff] [review]
Don't use issetguid

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

::: ipc/chromium/src/third_party/libevent/android/event2/event-config.h
@@ +123,5 @@
>  /* Define to 1 if you have the <inttypes.h> header file. */
>  #define _EVENT_HAVE_INTTYPES_H 1
>  
>  /* Define to 1 if you have the `issetugid' function. */
> +/* #undef  _EVENT_HAVE_ISSETUGID */

Check with Josh Aas how he updated libevent in bug 842887, because there's a non negligible chance this change would end up thrown away if we ever update libevent again.

::: memory/mozjemalloc/jemalloc.c
@@ +393,5 @@
> +/* Andorid L has no issetugid(). Also, the implemetation of all version of
> + * bionic always return 0 only.
> + */
> +#define issetugid() 0
> +#endif

There is this in the same file:

#if defined(MOZ_MEMORY_LINUX) && !defined(MOZ_MEMORY_ANDROID)
#define _GNU_SOURCE /* For mremap(2). */
#define issetugid() 0

Merge both.

Comment 23

[Makoto Kato [:m_kato]]

(In reply to Mike Hommey [:glandium] from comment #22)
> Comment on attachment 8450771 [details] [diff] [review]
> Don't use issetguid
> 
> Review of attachment 8450771 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: ipc/chromium/src/third_party/libevent/android/event2/event-config.h
> @@ +123,5 @@
> >  /* Define to 1 if you have the <inttypes.h> header file. */
> >  #define _EVENT_HAVE_INTTYPES_H 1
> >  
> >  /* Define to 1 if you have the `issetugid' function. */
> > +/* #undef  _EVENT_HAVE_ISSETUGID */
> 
> Check with Josh Aas how he updated libevent in bug 842887, because there's a
> non negligible chance this change would end up thrown away if we ever update
> libevent again.
> 
> ::: memory/mozjemalloc/jemalloc.c
> @@ +393,5 @@
> > +/* Andorid L has no issetugid(). Also, the implemetation of all version of
> > + * bionic always return 0 only.
> > + */
> > +#define issetugid() 0
> > +#endif
> 
> There is this in the same file:
> 
> #if defined(MOZ_MEMORY_LINUX) && !defined(MOZ_MEMORY_ANDROID)
> #define _GNU_SOURCE /* For mremap(2). */
> #define issetugid() 0
> 
> Merge both.

issetgid() on bionic is defined in unistd.h, this place that you say is too early to define "#define issetugid() 0".

If modifying from "defined(MOZ_MEMORY_LINUX) && !defined(MOZ_MEMORY_ANDROID)" to "defined(MOZ_MEMORY_LINUX)", we get the following error.

In file included from ../../dist/system_wrappers/unistd.h:3:0,
                 from /home/makoto/Development/hg.mozilla.org/mobile/memory/mozjemalloc/jemalloc.c:376:
/home/makoto/bin/android-ndk-r9d/platforms/android-9/arch-arm/usr/include/unistd.h:95:26: error: macro "issetugid" passed 1 arguments, but takes just 0
 extern int issetugid(void);
                          ^
glandium, do you have other idea?

Comment 24

[Mike Hommey [:glandium]]

Just remove the issetugid use from jemalloc.c. It's issetugid() == 0, and it's always true anyways (except on BSDs, but it doesn't matter much anyways)

Comment 25

[Makoto Kato [:m_kato]]

Attachment: undefine _EVENT_HAVE_ISSETUGID

Pick up code from chromium of https://chromium.googlesource.com/chromium/src.git/+/3820caf242eb35f17c04b165e736ba7a894b20da/third_party/libevent/android/event-config.h.

Comment 26

[Makoto Kato [:m_kato]]

Attachment: Remove issetugid

Comment 27

[Josh Aas]

Comment on attachment 8450834 [details] [diff] [review]
undefine _EVENT_HAVE_ISSETUGID

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

http://mxr.mozilla.org/mozilla-central/source/ipc/chromium/src/third_party/libevent/README.mozilla

Don't modify our copy of libevent directly or upgrading will be a nightmare. Create a patch like the other Mozilla modifications in the libevent dir.

Comment 28

[Josh Aas]

Comment on attachment 8450834 [details] [diff] [review]
undefine _EVENT_HAVE_ISSETUGID

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

I realize you're modifying a file we added, but that file is auto-generated (iirc) so probably still best to patch it rather than directly modify it.

Comment 29

[Makoto Kato [:m_kato]]

Attachment: for ipc v2

add patch file and modify README.mozilla.

Comment 30

[Makoto Kato [:m_kato]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fb2a8327a8bb
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5c88681e091

Comment 31

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/fb2a8327a8bb
https://hg.mozilla.org/mozilla-central/rev/e5c88681e091

Comment 32

[Makoto Kato [:m_kato]]

continue to bug 1036286 to launch firefox.  linker issue is fixed, but there is remained issue.

Comment 34

[Sylvestre Ledru [:Sylvestre]]

Makoto, can we have an uplift in aurora? Thanks

Comment 35

[Makoto Kato [:m_kato]]

Comment on attachment 8450874 [details] [diff] [review]
Remove issetugid

Approval Request Comment
[Feature/regressing bug #]:
Nothing

[User impact if declined]:
User cannot launch Firefox Android on Android L Preview.  This crash cannot be caught by crash reporter.

[Describe test coverage new/current, TBPL]:
landed in m-c.

[Risks and why]: 
Too low.  The implementaion of issetguid of bionic libc in Android always returns 0 only.

[String/UUID change made/needed]:
No

Comment 36

[Makoto Kato [:m_kato]]

Comment on attachment 8451465 [details] [diff] [review]
for ipc v2

Approval Request Comment
[Feature/regressing bug #]:
Nothing

[User impact if declined]:
User cannot launch Firefox Android on Android L Preview.  This crash cannot be caught by crash reporter.

[Describe test coverage new/current, TBPL]:
landed in m-c.

[Risks and why]: 
Too low.  The implementaion of issetguid of bionic libc in Android always returns 0 only.

Also, libevent in chromium doesn't define _EVENT_HAVE_ISSETUGID.

[String/UUID change made/needed]:
No

Comment 37

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9664c3ee8205
https://hg.mozilla.org/releases/mozilla-aurora/rev/14b9fde050ba