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

Unify the platform headers #120

Closed
DanAlbert opened this Issue Jun 10, 2016 · 16 comments

Comments

Projects
None yet
8 participants
@DanAlbert
Copy link
Member

DanAlbert commented Jun 10, 2016

This is something we've been working on for a while, but we didn't have a public bug for it. I'm filing one so that folks working on external build systems can follow along.

Currently, we have a set of libc headers for each API version, which may or may not actually reflect reality. Unifying them into one canonical and correct set of headers with availability attributes and ifdefs will improve the lives of app developers using the NDK by making the headers correct, and should also make implementing the NDK compatibility library easier. There are probably lots of cases that can’t be easily handled automatically, but this should hopefully be sufficient for the vast majority of functions.

We're through most of this effort now. The AOSP bionic headers are now unified, but they still need to reach the NDK and we need to fix Clang to recognize versioned Android triples. We also still need to add support to ndk-build and standalone toolchains.

@DanAlbert DanAlbert added the headers label Jun 10, 2016

@DanAlbert DanAlbert added this to the r13 milestone Jun 10, 2016

@jmgao

This comment has been minimized.

Copy link
Contributor

jmgao commented Jun 10, 2016

The clang fix has been ~~upstreamed~~reverted.

@DanAlbert

This comment has been minimized.

Copy link
Member Author

DanAlbert commented Jun 16, 2016

What should we do about the asm/ and machine/ headers? We can either let ndk-build (and our 4.5 other build systems) deal with it, or we can merge all those into arch-generic headers.

@enh: Where do those headers actually come from? @jmgao thinks they're generated from the kernel headers but isn't sure. If these aren't generated then I think option 2 is the way to go.

@DoDoENT

This comment has been minimized.

Copy link

DoDoENT commented Jun 30, 2016

I would just like to mention that some functions in math.h and locale.h used to be defined in headers prior android-21 and are only declared in android-21 and later. Therefore, when using android-21 as APP_PLATFORM with purpose of running same binary on pre-Lollipop devices, one must implement missing functions in their own code.

For our project, this looks like this:

/**
 * prior android-21 functions below were implemented as inline functions, as of android-21 they are implemented inside library
 * devices that do not have Android 5.0 will use this implementation (because SDK used for building is android-21).
 * Behaviour on Android 5 appears to be OK (at least on Nexus 5 and Nexus 10)
 */

extern int rand(void) {
    return (int)lrand48();
}

extern void srand(unsigned int __s) {
    srand48(__s);
}

extern double atof(const char *nptr)
{
    return (strtod(nptr, NULL));
}

extern int abs(int __n) {
    return (__n < 0) ? -__n : __n;
}

extern long labs(long __n) {
    return (__n < 0L) ? -__n : __n;
}

extern long long llabs(long long __n) {
    return (__n < 0LL) ? -__n : __n;
}

// armeabi-v7a-hard links to lm_hard which has this functions defined
#if !defined(__ARCH_ABI_ID__) || __ARCH_ABI_ID__ != 7
extern int __isnanf(float f) {
    return isnanf(f);
}
#endif

extern size_t __ctype_get_mb_cur_max() {
    return 4;
}

extern double log2(double n) {
    // log(n)/log(2) is log2.
    return log(n) / log(2);
}

extern float log2f(float n) {
    return logf(n) / logf(2);
}

extern void srandom(unsigned int seed) {
    srand48(seed);
}

struct lconv* localeconv(void) {
  static struct lconv C_LCONV[1] =  { {
    .decimal_point = ".",
    .thousands_sep = "",
    .grouping = "",
    .int_curr_symbol = "",
    .currency_symbol = "",
    .mon_decimal_point = "",
    .mon_thousands_sep = "",
    .mon_grouping = "",
    .positive_sign = "",
    .negative_sign = "",
    .int_frac_digits = CHAR_MAX,
    .frac_digits = CHAR_MAX,
    .p_cs_precedes = CHAR_MAX,
    .p_sep_by_space = CHAR_MAX,
    .n_cs_precedes = CHAR_MAX,
    .n_sep_by_space = CHAR_MAX,
    .p_sign_posn = CHAR_MAX,
    .n_sign_posn = CHAR_MAX,
    .int_p_cs_precedes = CHAR_MAX,
    .int_p_sep_by_space = CHAR_MAX,
    .int_n_cs_precedes = CHAR_MAX,
    .int_n_sep_by_space = CHAR_MAX,
    .int_p_sign_posn = CHAR_MAX,
    .int_n_sign_posn = CHAR_MAX,
  } };

  return C_LCONV;
}

With this file included in our code, we managed to run our binary even on Gingerbread, although it was compiled with APP_PLATFORM=android-23. This is why I thought there would be similar hack for issue #126, but unfortunately #126 is a bit tougher nut to crack...

@1bsyl

This comment has been minimized.

Copy link

1bsyl commented Jun 30, 2016

Thanks a lot for this file !

In fact, there are much more functions. Basically all the functions that were "inline" that in "arch-9" and that are "extern" in "arch-23".

(eg. sigemptyset, sigaddset, etc.)

@DoDoENT

This comment has been minimized.

Copy link

DoDoENT commented Jun 30, 2016

@1bsyl this file is for our project. Please update it with functions that are missing and share it back (feel free to open a new git repo on github).

@1bsyl

This comment has been minimized.

Copy link

1bsyl commented Jun 30, 2016

That would be, but not tested as I will remain on a lower APP_PLATFORM:

From the NDK file "android-9/arch-arm/usr/include/signal.h":

static __inline__ int sigismember(sigset_t *set, int signum)
{
    unsigned long *local_set = (unsigned long *)set;
    signum--;
    return (int)((local_set[signum/LONG_BIT] >> (signum%LONG_BIT)) & 1);
}


static __inline__ int sigaddset(sigset_t *set, int signum)
{
    unsigned long *local_set = (unsigned long *)set;
    signum--;
    local_set[signum/LONG_BIT] |= 1UL << (signum%LONG_BIT);
    return 0;
}


static __inline__ int sigdelset(sigset_t *set, int signum)
{
    unsigned long *local_set = (unsigned long *)set;
    signum--;
    local_set[signum/LONG_BIT] &= ~(1UL << (signum%LONG_BIT));
    return 0;
}


static __inline__ int sigemptyset(sigset_t *set)
{
    memset(set, 0, sizeof *set);
    return 0;
}

static __inline__ int sigfillset(sigset_t *set)
{
    memset(set, ~0, sizeof *set);
    return 0;
}
@enh

This comment has been minimized.

Copy link
Contributor

enh commented Jun 30, 2016

the unified headers already handle this. see the legacy_* files in https://android.googlesource.com/platform/bionic/+/master/libc/include/android/ (but please do let us know if there's anything not already there).

@DoDoENT

This comment has been minimized.

Copy link

DoDoENT commented Jun 30, 2016

@enh , I haven't seen log2, log2f and localeconv functions on link you posted.

@enh

This comment has been minimized.

Copy link
Contributor

enh commented Jun 30, 2016

as far as i can tell, none of those were ever inlines in the NDK headers.

@DoDoENT

This comment has been minimized.

Copy link

DoDoENT commented Jun 30, 2016

@enh, true, but those were missing at lower API levels.

@bog-dan-ro

This comment has been minimized.

Copy link

bog-dan-ro commented Jun 30, 2016

What the NDK folders layout will look like?
Currently you have:

  • NDK
    • platforms
      • android-XX
        • arch-YYYY
          • usr/....

You are going to remove android-XX folder and merge all platforms in arch-YYYY ?

@enh

This comment has been minimized.

Copy link
Contributor

enh commented Jun 30, 2016

which API level a function appears in is handled in the headers themselves: https://android.googlesource.com/platform/bionic/+/master/libc/include/math.h#195

@DanAlbert DanAlbert modified the milestones: r13, r14 Aug 15, 2016

Mazda-- pushed a commit to codeaurora-unofficial/platform-development that referenced this issue Oct 17, 2016

Backport machine/fenv.h to GB for ARM and mips.
Not needed for x86 because it has always had a working fenv
implementation.

We want to add the fixed implementations of fenv to libandroid_support
rather than dirtying up the bionic headers for these, but with the
inlines and the types in the same header we can't do this easily.
Backport machine/fenv.h for these architectures so we can simplify
this.

These are just copied from the android-21 directory. They aren't
moved because gen-platforms.sh actually removes all the currently
accumulated headers for android-21, so android-21 needs to have a
full set of headers...

Test: ndk/checkbuild.py
Bug: android-ndk/ndk#120
Change-Id: Idc3bebc533230afca563c6911c65e90b51342a4b

@DanAlbert DanAlbert closed this Nov 15, 2016

@Zingam

This comment has been minimized.

Copy link

Zingam commented Jan 11, 2017

Could you clarify how to use/try the unified headers if they are not yet enabled by default. In the release notes for example.

@enh

This comment has been minimized.

Copy link
Contributor

enh commented Jan 11, 2017

docs are here: https://android.googlesource.com/platform/ndk.git/+/master/docs/UnifiedHeaders.md

(it was linked to in the https://github.com/android-ndk/ndk/wiki/Changelog-r14-beta1 release notes under "announcements", and we've clarified/emphasized this stuff more for beta2 when it comes.)

@hrydgard

This comment has been minimized.

Copy link

hrydgard commented Mar 6, 2017

@enh docs are there, yes, but they only say how to disable the unified headers from r15, not how to enable them in r14. How do you do that?

@enh

This comment has been minimized.

Copy link
Contributor

enh commented Mar 6, 2017

ah, yes; normally it's helpful to link to the "always fresh" (master) copy of a file, but in this case you specifically want to see the r14 docs to learn how to enable unified headers: https://android.googlesource.com/platform/ndk.git/+/ndk-r14-release/docs/UnifiedHeaders.md

(danalbert: that's a problem with the changelog too; it links to master.)

miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018

Add a build step for the unified headers.
Install them to $NDK/sysroot.

Bug: android-ndk/ndk#120
Change-Id: I03f989059f1852db9ea69fc9dd9d8556a613320d

miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018

Add ndk-build support for unified headers.
Setting `APP_UNIFIED_HEADERS=true` will build your code with the
unified headers.

Test: ./validate.py --force-unified-headers # added in next patch
Bug: android-ndk/ndk#120
Change-Id: I14424310b4d42d9a555b4c65ad6f4680520c5ad0

miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018

Add a `--force-unified-headers` flag to the tests.
Test: ./validate.py --force-unified-headers
Bug: android-ndk/ndk#120
Change-Id: I3e32989e3658d64e421845e46a7cb7630a37e57a

miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018

Test both legacy and unified headers.
Test: ./checkbuild.py
Bug: android-ndk/ndk#120
Change-Id: I72f22f48ede4c51952183b569a5c7eff8183547e

miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018

Add unified headers support to the libc++ tests.
Test: ./test_libcxx.py --unified-headers -a armeabi-v7a -p 9
Bug: android-ndk/ndk#120
Change-Id: Ib5e6ad3ba647aea3086dd4b91046efcc988c196a

miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018

Support unified headers in standalone toolchains.
These aren't supported as smoothly for GCC. Unified headers require
that `__ANDROID_API__` be defined from the command line. For Clang we
do this in the wrapper scripts that are used to pass the target, but
we don't have wrapper scripts for GCC. Since GCC is deprecated, just
don't bother.

Test: ./run_tests.py --filter standalone-toolchain*
Bug: android-ndk/ndk#120
Change-Id: I71d42d218b83bbe8634a5e473804c9e6d7199101

miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018

Add unified headers support to cmake.
Exposed with `ANDROID_UNIFIED_HEADERS`.

Test: ./validate.py --force-unified-headers
Bug: android-ndk/ndk#120
Change-Id: I3bd01dadec0158af7b22b1824c4150d26d755874
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.