GNOME Bugzilla – Bug 788270
gmodule - failed to load symbol on Android 64bit
Last modified: 2017-11-09 09:08:05 UTC
This issue is related to https://bugzilla.gnome.org/show_bug.cgi?id=776876 In the previous work, it avoided using RTLD_DEFAULT on 64bit Android, but it causes 'undefined symbol' error. If 'dlsym' uses a handle which is created by 'dlopen(NULL)', it always fails to find symbol. To use the handle properly, 'dlopen' should have a file name, or 'dlsym' should take 'RTLD_DEFAULT' or NULL.
Created attachment 360577 [details] [review] Use RTLD_DEFAULT
The problem here is that pre-Android 7.0 would fail with your change, which is basically reverting my change from bug 776876. Or am I missing something here? In pre-7.0, passing RTLD_DEFAULT to dlsym() would fail because RTLD_DEFAULT was defined as 0, and 0 was considered an invalid module handle.
*** Bug 785406 has been marked as a duplicate of this bug. ***
I think there's difference between pre-Android 7.0 and released 7.0. RTLD_DEFAULT is NULL, but dlsym won't fail with NULL. It's really confusing. I added a working code in my github that dlsym runs without dlopen. https://github.com/justinjoy/android-ndk/commit/e02213933b33ba180726d28ace0d852931f0a6bc I might be a bug of bionic, but not sure.
(In reply to Justin J. Kim from comment #4) > I might be a bug of bionic, but not sure. Not me. :) I'd say "it" might be a bug of bionic.
(In reply to Justin J. Kim from comment #4) > I think there's difference between pre-Android 7.0 and released 7.0. > RTLD_DEFAULT is NULL, but dlsym won't fail with NULL. It's really confusing. > > I added a working code in my github that dlsym runs without dlopen. Yes, my point is that (on 64 bit, 32 bit seems alright) 1) pre-7.0: dlsym(RTLD_DEFAULT) fails because RTLD_DEFAULT==0 and dlsym() checks for NULL. mod = dlopen(NULL) and dlsym(mod) works however 2) 7.0: dlsym(RTLD_DEFAULT) works, and dlopen(NULL) not anymore The pre-7.0 behaviour seems like a bug to me, which they seemed to have fixed... while breaking the only possible workaround that was possible pre-7.0. Is RTLD_DEFAULT still #defined to 0 in 7.0? Is the problem only when compiling with target=7.0, or also when compiling with older targets and running on 7.0? Also please find the commit in bionic that changes this behaviour.
(In reply to Sebastian Dröge (slomo) from comment #6) > Is RTLD_DEFAULT still #defined to 0 in 7.0? Yes, https://android.googlesource.com/platform/bionic/+/master/libc/include/dlfcn.h#71 > Is the problem only when > compiling with target=7.0, or also when compiling with older targets and > running on 7.0? This is reproducible only when compiling with target=7.0. > Also please find the commit in bionic that changes this behaviour. Let me try. Then, you will not accept this work around or similar way of patches?
(In reply to Justin J. Kim from comment #7) > Then, you will not accept this work around or similar way of > patches? Your patch breaks GModule on pre-7.0 Android, so that's not acceptable. We need to find a workaround that works in all cases.
Review of attachment 360577 [details] [review]: ::: gmodule/gmodule-dl.c @@ -142,1 +143,1 @@ is_unref = (handle != RTLD_DEFAULT); Sebastian, to be clear, I want to ask you again. If I revert 0d81bb4e318b97780c70a55605cacf7e5b3fcaf7, it won't work. However, this is a partial revert of your works because this line is still alive. `is_unref` will be FALSE so it will not break on 64 bit Android. In addition, I tested how "mod = dlopen(NULL) and dlsym(mod)" works through android versions. Until android-22, "mod = dlopen(NULL) and dlsym(mod)" works on both architectures. but, from android-23 to android-26, only 'dlsym(RTLD_DEFAULT)' returns correct pointer. They behave same on both 32bit and 64bit. As you mentioned, pre-7.0 might be an exceptional case. The only problem is RTLD_DEFAULT is NULL on 64 bit Android.
You mean you tested with Android < 7.0, 64 bit, and your changes work there?
Created attachment 360813 [details] [review] gmodule: accept specific case of 64bit Android.
> You mean you tested with Android < 7.0, 64 bit, and your changes work there? I tested from android-19 to android-26 after seeing your comment. To be honest, I failed to find the point in BIONIC which triggered this problem so I tried to find a solution in glib again. I checked my new patch works for Android < 7.0 (android-24).
Comment on attachment 360813 [details] [review] gmodule: accept specific case of 64bit Android. Looks good to me. This should work also in the case I fixed. Everybody say "thank you" to Google for wasting our time and breaking ABI :)
Review of attachment 360813 [details] [review]: ::: gmodule/gmodule.c @@ +513,3 @@ +/* On Android 64 bit, RTLD_DEFAULT is (void *)0x0 + * so it always fails to create main_module if file_name is NULL */ +#if !defined(__BIONIC__) && !defined(__LP64__) If you’re trying to exclude Android 64-bit here, shouldn’t this condition be: ``` #if !defined(__BIONIC__) || !defined(__LP64__) ```
Created attachment 361030 [details] [review] gmodule: accept specific case of 64bit Android. fix logical operator.
Review of attachment 361030 [details] [review]: Looks good to me. Please push to master. Thanks!
commit 33fad1bb2a25580b48b062bd14a0b7045e0fb22a Author: Justin Kim <justin.kim@collabora.com> Date: Tue Oct 3 17:04:17 2017 +0900 glib: Allow using RTLD_DEFAULT on 64bit Android This is a backport of glib[0] to support Android 64 bit. [0] https://bugzilla.gnome.org/show_bug.cgi?id=788270 https://bugzilla.gnome.org/show_bug.cgi?id=790058
Ah sorry, git-bz picked up both Bugzilla URLs from the commit. The above is for GStreamer.