Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

posix_memalign is not available on some Android 4.1 devices #647

Closed
mdmt1 opened this issue Mar 11, 2018 · 11 comments
Closed

posix_memalign is not available on some Android 4.1 devices #647

mdmt1 opened this issue Mar 11, 2018 · 11 comments
Assignees
Milestone

Comments

@mdmt1
Copy link

mdmt1 commented Mar 11, 2018

posix_memalign is declared in stdlib.h as being available since Android 4.1:

#if __ANDROID_API__ >= 16
int posix_memalign(void** __memptr, size_t __alignment, size_t __size) __INTRODUCED_IN(16);
#endif /* __ANDROID_API__ >= 16 */

However, on some devices that were launched with pre-4.1 version of Android and later upgraded to 4.1, bionic seems to not have been updated.

This causes java.lang.UnsatisfiedLinkError: Cannot load library: reloc_library[1306]: 143 cannot locate 'posix_memalign'... in System.loadLibrary.

I have found related issue here.

As far as I can tell, there are no Android 4.2 devices with this problem.

@DanAlbert
Copy link
Member

Do you have examples of devices like this? The linked bug report says the 4.1+ is fine.

@mdmt1
Copy link
Author

mdmt1 commented Mar 13, 2018

Do you have examples of devices like this?

I have encountered it on Motorola RAZR i (x86), Android 4.1.

The linked bug report says the 4.1+ is fine.

I'm not sure how you came to this conclusion. It says that ZTE U5 (Android 4.1) also exhibits this issue. Later in the comments it also says:

Webperf team has also encountered this issue on Chrome for android on Android 4.1(X86). Upstream has known this issue.
posix_memalign was missing in the system library: libc.so.

After some searching in chromium repo, I have found this:

#define HAVE_POSIX_MEMALIGN 0 /* #define HAVE_POSIX_MEMALIGN 1 -- forced to 0. See https://crbug.com/604451 */

Linked bug report says:

We had at least one case of Android device "forgetting" to implement posix_memalign (despite the fact that is mandated by the NDK for the SDK level we target in chrome) and causing chrome to crash at startup.

Technically we have all the rights in Chrome to use posix_memalign. After the bump to JB (and the consequent drop of ICS) The SDK level we are targeting exposes that in the NDK.
I suppose that code in #1 predates the bump to the JB sdk, which was a fairly recent thing.
Sadly, however, some vendor forgot to alias posix_memalign in their libc violating the NDK. :-(

To learn more, I downloaded the Android 4.1 firmware for Motorola RAZR i and extracted libc.so.

objdump --dynamic-syms libc.so | rg memalign

00018860 g DF .text 0000002e memalign
000164d0 g DF .text 0000025f dlposix_memalign
000162b0 g DF .text 00000220 dlmemalign

For comparison, libc.so from system.img of Android 4.1 x86 virtual device.

objdump --dynamic-syms libc.so | rg memalign

000161b0 g DF .text 00000030 memalign
00016020 g DF .text 00000013 dlmemalign
00015e30 g DF .text 00000021 posix_memalign

I also searched for all new symbols in API 16 in the latest NDK (16.1.4479499) by running rg 'INTRODUCED_IN\(16\)' in sysroot directory.

Only posix_memalign is missing. All others (faccessat, tdelete, tdestroy, tfind, tsearch, readahead and all symbols from sys/xattr.h) are present.

@enh
Copy link
Contributor

enh commented Mar 13, 2018

ah, x86-only? that sounds familiar.... https://issuetracker.google.com/28245055

@enh
Copy link
Contributor

enh commented Mar 13, 2018

from that bug it looks like i should have bumped the API level for x86 posix_memalign to 17, but didn't. so i guess that's the fix here?

@mdmt1
Copy link
Author

mdmt1 commented Mar 13, 2018

In Android 4.1 posix_memalign was defined like this:

int posix_memalign(void **memptr, size_t alignment, size_t size) {
  int ret = 0;

  *memptr = dlmemalign(alignment, size);

  if (*memptr == 0) {
    ret = ENOMEM;
  }

  return ret;
}

And memalign was aliased to dlmemalign.

Maybe posix_memalign can be declared like this?

#if __ANDROID_API__ >= 17
int posix_memalign(void** __memptr, size_t __alignment, size_t __size) __INTRODUCED_IN(17);
#else
int posix_memalign(void **memptr, size_t alignment, size_t size) {
  int ret = 0;

  *memptr = memalign(alignment, size);

  if (*memptr == 0) {
    ret = ENOMEM;
  }

  return ret;
}
#endif /* __ANDROID_API__ >= 17 */

@DanAlbert DanAlbert added this to the r17 milestone Mar 13, 2018
@rprichard
Copy link
Collaborator

If we do bump the API level to 17, maybe we'd want to add something for API 16? We had an out-of-line version of the function in android_support until recently for the sake of API < 16:

https://android-review.googlesource.com/c/platform/ndk/+/610876

@DanAlbert
Copy link
Member

Yeah, I think we may want to just put it back in libandroid_support. The alternative is that we add the inline directly to bionic until such time that we can drop API 16. @enh, thoughts? Neither is particularly difficult.

@enh
Copy link
Contributor

enh commented Mar 14, 2018

either sgtm.

@mdmt1 mdmt1 closed this as completed Mar 16, 2018
@mdmt1 mdmt1 reopened this Mar 16, 2018
@mdmt1
Copy link
Author

mdmt1 commented Mar 16, 2018

Accidentally pressed the "Close issue" button, disregard.

@DanAlbert DanAlbert self-assigned this Mar 16, 2018
@DanAlbert
Copy link
Member

This is "fixed" in r17 in that it's back in libandroid_support, but for completeness I need to get the headers/stub libs to stop pretending it was in android-16.

hubot pushed a commit to aosp-mirror/platform_bionic that referenced this issue Apr 20, 2018
Apparently this didn't make it to all android-16 devices. As far as
we know it did make it for all android-17 devices.

Test: make checkbuild
Bug: android/ndk#647
Change-Id: I2f07cfb1254e2a203c1c10b91b0be46bf37ea853
@DanAlbert
Copy link
Member

Merged into r17.

lazartrsic pushed a commit to MIPS/ndk that referenced this issue Jun 1, 2018
This wasn't available for all android-16 devices.

Test: ./checkbuild.py && ./run_tests.py
Bug: android/ndk#647
Change-Id: Idd03a15b0b61dc40e594598fac158a90efe398b0
(cherry picked from commit 05aeeb1ec1fb97a79415f06b55425f40c07c0974)
Kudo added a commit to Kudo/folly that referenced this issue Oct 19, 2018
Summary:
  According to this android/ndk#647,
  posix_memalign may not exist on Android API 16.
  From Android NDK r17c, the API exists for Android API 17+.

    #if __ANDROID_API__ >= 17
    int posix_memalign(void** __memptr, size_t __alignment, size_t __size) __INTRODUCED_IN(17);
    #endif /* __ANDROID_API__ >= 17 */

  Change the code to use posix_memalign only after Android API 17+.
facebook-github-bot pushed a commit to facebook/folly that referenced this issue Oct 19, 2018
Summary:
According to this android/ndk#647,
  posix_memalign may not exist on Android API 16.
  From Android NDK r17c, the API exists for Android API 17+.
```
    #if __ANDROID_API__ >= 17
    int posix_memalign(void** __memptr, size_t __alignment, size_t __size) __INTRODUCED_IN(17);
    #endif /* __ANDROID_API__ >= 17 */
```
  Change the code to use posix_memalign only after Android API 17+.
  This would also fix issue for OSS React Native to pack latest folly and building with clang.
  See: facebook/react-native#20302 and facebook/react-native#20342
Pull Request resolved: #953

Reviewed By: yfeldblum

Differential Revision: D10469757

Pulled By: Orvid

fbshipit-source-id: c63838f3f6e723ef3de77187f39597a4063043db
sandraiser pushed a commit to sandraiser/folly that referenced this issue Mar 4, 2019
Summary:
According to this android/ndk#647,
  posix_memalign may not exist on Android API 16.
  From Android NDK r17c, the API exists for Android API 17+.
```
    #if __ANDROID_API__ >= 17
    int posix_memalign(void** __memptr, size_t __alignment, size_t __size) __INTRODUCED_IN(17);
    #endif /* __ANDROID_API__ >= 17 */
```
  Change the code to use posix_memalign only after Android API 17+.
  This would also fix issue for OSS React Native to pack latest folly and building with clang.
  See: facebook/react-native#20302 and facebook/react-native#20342
Pull Request resolved: facebook#953

Reviewed By: yfeldblum

Differential Revision: D10469757

Pulled By: Orvid

fbshipit-source-id: c63838f3f6e723ef3de77187f39597a4063043db
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants