x86 API level 16 and 17 incorrect ldexp result

[m1h4]

Description

We are having problems with the boost multiprecision library failing to initialize which were traced to a call to std::ldexp which is returning NaN instead of 536870912 for a long double input: std::ldexp(0.5L, 30). Making the same call with a double input std::ldexp(0.5, 30) works fine.

Can be easily reproduced in a new vanila project with c++ support by pasting the above line in the generated native-lib.cpp.

As far as we've been able to test, this only occures on x86 api level 16 and 17. Higher api levels are not affected as well as arm7 abi-s.

• NDK Version: 15.2.4203891
• Build sytem: cmake
• Host OS: Mac
• Compiler: Clang
• ABI: x86
• STL: c++_static
• NDK API level: 9
• Device API level: 16 and 17, higher are not affected

[m1h4]

Just tested and the problem is also present with the latest 16.0.4293906-beta1 NDK.

[enh]

interesting that you're seeing this with r16 too, because one difference there is that libandroid_support.a contains ldexpl for x86 in r16 but didn't in r15 (see #502).

[m1h4]

Just checked and the problem is also present on API 10 and API 15 so basically any x86 image below API 18 (tested with 16.0.4293906-beta1).

To clarify the original report:
The problem seems to be the implementation of ldexpl for these API levels.
When the overloaded std::ldexp resolves to ldexp (when using a regular double input) the result is correct for the specified input values.

[enh]

i suspect that

commit a0ee07829a9ba7e99ef68e8c12551301cc797f0f
Author: Elliott Hughes <[email protected]>
Date:   Wed Jan 30 19:06:37 2013 -0800

    Upgrade libm.
    
    This brings us up to date with FreeBSD HEAD, fixes various bugs, unifies
    the set of functions we support on ARM, MIPS, and x86, fixes "long double",
    adds ISO C99 support, and adds basic unit tests.
    
    It turns out that our "long double" functions have always been broken
    for non-normal numbers. This patch fixes that by not using the upstream
    implementations and just forwarding to the regular "double" implementation
    instead (since "long double" on Android is just "double" anyway, which is
    what BSD doesn't support).
    
    All the tests pass on ARM, MIPS, and x86, plus glibc on x86-64.
    
    Bug: 3169850
    Bug: 8012787
    Bug: https://code.google.com/p/android/issues/detail?id=6697
    Change-Id: If0c343030959c24bfc50d4d21c9530052c581837

fixed ldexpl for x86 (by fixing the scalbn family). that's pretty much the last time that any of these functions changed, and it would tie in well with the API 18 release date.

the odd part here is that the r16beta1 libandroid_support.a should include an ldexpl. ah, except it's a weak reference. and i guess i don't know what that resolves to at run time. (though your example suggests "the incorrect copy in libm".)

that presumably means that none of

$ nm ./sources/cxx-stl/llvm-libc++/libs/x86/libandroid_support.a | grep -w W
00000000 W acosl
00000000 W acoshl
00000000 W asinl
00000000 W atan2l
00000000 W atanhl
00000000 W coshl
00000000 W expl
00000000 W hypotl
00000000 W lgammal
00000000 W logl
00000000 W log10l
00000000 W log2l
00000000 W remainderl
00000000 W sinhl
00000000 W sqrtl
00000000 W asinhl
00000000 W atanl
00000000 W cbrtl
00000000 W cosl
00000000 W erfcl
00000000 W erfl
00000000 W exp2l
00000000 W expm1l
00000000 W log1pl
00000000 W logbl
00000000 W nextafterl
00000000 W nexttoward
00000000 W nexttowardl
00000000 W remquol
00000000 W rintl
00000000 W sinl
00000000 W tanl
00000000 W tanhl
00000000 W ldexpl
00000000 W scalbnl

are actually helping. maybe we do want to fix https://issuetracker.google.com/64450768 and use _RENAME for all the <math.h> *l functions sooner rather than later...

presumably if you edit <math.h> to say

long double ldexpl(long double __x, int __exponent) __RENAME(ldexp);

then your code works?

i think we want to add a __RENAME32 (or somesuch) so we can add these annotations to all the long double functions in <math.h> (since double == long double for Android LP32), which will make all this nonsense go away and let us shrink libandroid_support.a still further...

[m1h4]

Yes, replacing ldexpl calls with ldexp works fine for the given problematic inputs in our use-case (boost::multiprecision). What you are saying sounds like the way to go in fixing these issues (aliasing the long double functions to double).

This is the output of objdump on a empty new c++ android project with a call to ldexpl added to the generated native code:

i686-linux-android-objdump -TC libnative-lib.so 

libnative-lib.so:     file format elf32-i386

DYNAMIC SYMBOL TABLE:
00000000      DF *UND*	00000000  LIBC        __cxa_finalize
00000000      DF *UND*	00000000  LIBC        __cxa_atexit
00000000      DF *UND*	00000000  LIBC        __stack_chk_fail
000004a0 g    DF .text	00000059  Base        Java_com_sample_myapplication_MainActivity_stringFromJNI
00000000      DF *UND*	00000000  LIBC        ldexpl
00002004 g    D  *ABS*	00000000  Base        _edata
00002004 g    D  *ABS*	00000000  Base        _end
00002004 g    D  *ABS*	00000000  Base        __bss_start

We can see that the call is not handled through libandroid_support.a but is linked dynamically to the (buggy) system libm.so...

[enh]

https://android-review.googlesource.com/#/c/platform/bionic/+/485247 is the proposed fix (in the platform).

[m1h4]

Thanks for the update!

How does this impact the NDK exactly? Does the NDK get packaged with the headers from platform/bionic or? Can we expect the next beta release of the r16 NDK to have these changes included?

[enh]

TL;DR, yes, we hope to get this into r16beta2.

step 1 is for me to submit the change mentioned above. step 2 is that we run a script that copies all the latest headers into the NDK. step 3 is to cherrypick that batch of new headers to the r16 release branch.

step 0 though is for me to manually cherrypick my libc change into my NDK and check that it doesn't break anything (it seems quite likely to upset the current libandroid_support), and fix libandroid_support appropriately.

[m1h4]

Ok, thank you for the clarification!

[enh]

i think at this point everything's done and this should be in beta2...