[libc++][Android] Adjust FP conversion tests for Bionic

[rprichard]

Summary

1. Bionic also includes '+' for NAN

However, it did not do so for Android L (APIs 21 and 22), so define a
compiler macro, ANDROID_PRINTF_NAN_NO_SIGN, to adjust the behavior of
the facet.num.put.members/put_long_double.pass.cpp test.

2. Detect broken strtold of NAN on LP64

Converting the string "NAN" to a long double is broken on 64-bit
Android prior to Android O (API 26), so define
ANDROID_BROKEN_STRTOLD_NAN to disable parts of two tests.

See https://android-review.googlesource.com/q/Id7d46ac2d8acb8770b5e8c445e87cfabfde6f111

[rprichard]

L MR1 (5.1) is API 22.

[Mordante]

I think we can use a simpler test, WDYT?

libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.pass.cpp

10736

Would this work too?

[rprichard]

libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.pass.cpp

10736

It ties the expected test output to the build API level of the test, which may not be what we want.

i.e. There are three API levels:

• the build API level of libc++ itself
• the build API of the libc++ tests (or the application code)
• the runtime API level of the device

Currently for the NDK, the libc++ build API is typically 19 or 21, the app build API is typically low-ish (maybe 21/23?), and the runtime API steadily increases as people buy newer devices. It's normal for the build API for application code to be low (to target all the old devices in the wild), and the NDK only packages a single libc++_{shared.so,static.a} per CPU architecture.

This use of __ANDROID_API__ would break the test config in D139147, where the build API for libc++ and the tests is 21 (AndroidNDK.cmake, llvm-libc++abi-android-ndk.cfg.in), but the runtime API is 33 (Dockerfile.android).

[Mordante]

A few nits and I'm happy.

libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.pass.cpp

10736

I see, can you add the gist of this information to the comment in features.py.

10736

We typically only use _LIBCPP for configuration options in libc++ itself and not for the test suite.

[rprichard]

Add a comment explaning why we don't just check __ANDROID_API__.

Also disable tests that convert a "NAN" string to long double on 64-bit targets. This bug was fixed in Android O. See https://android-review.googlesource.com/q/Id7d46ac2d8acb8770b5e8c445e87cfabfde6f111.

Remove the __LIBCPP_TESTING_ prefix from the macros in favor of:

• ANDROID_PRINTF_NAN_NO_SIGN
• ANDROID_BROKEN_STRTOLD_NAN

[Mordante]

Can you rebase this patch so we can see the CI run. (Mainly to make sure it doesn't break things unexpectedly.)

libcxx/test/std/strings/string.conversions/stold.pass.cpp

48

115

libcxx/utils/libcxx/test/features.py

169

175

This is the prefix we use in our test suite.

188

How about testing this macro at line 181 insted?

189

libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.pass.cpp

#if defined(_AIX)
     std::string NaN = "NaNQ";
     std::string nan_padding25 = "*********************";
     std::string pnan_sign = "+";
     std::string pnan_padding25 = "********************";
 #else
     std::string nan= "nan";
     std::string NaN = "NAN";
     std::string nan_padding25 = "**********************";
-#if defined(TEST_HAS_GLIBC) || defined(_WIN32)
+#if defined(TEST_HAS_GLIBC) || defined(_WIN32) || \
+        (defined(__BIONIC__) && !defined(ANDROID_PRINTF_NAN_NO_SIGN))
     std::string pnan_sign = "+";
     std::string pnan_padding25 = "*********************";
 #else
     std::string pnan_sign = "";
     std::string pnan_padding25 = "**********************";
 #endif
 #endif
     {

libcxx/test/std/localization/locale.categories/category.numeric/locale.num.get/facet.num.get.members/get_long_double.pass.cpp

long double v = -1;
         cpp17_input_iterator<const char*> iter =
             f.get(cpp17_input_iterator<const char*>(str),
                   cpp17_input_iterator<const char*>(str+sizeof(str)),
                   ios, err, v);
         assert(base(iter) == str+sizeof(str)-1);
         assert(err == ios.goodbit);
         assert(v == -INFINITY);
     }
+#ifndef ANDROID_BROKEN_STRTOLD_NAN
     {
         const char str[] = "nan";
         std::hex(ios);
         std::ios_base::iostate err = ios.goodbit;
         cpp17_input_iterator<const char*> iter =
             f.get(cpp17_input_iterator<const char*>(str),
                   cpp17_input_iterator<const char*>(str+sizeof(str)),
                   ios, err, v);
         assert(base(iter) == str+sizeof(str)-1);
         assert(err == ios.goodbit);
         assert(std::isnan(v));
     }
     {
         const char str[] = "NAN";
         std::hex(ios);
         std::ios_base::iostate err = ios.goodbit;
         cpp17_input_iterator<const char*> iter =
             f.get(cpp17_input_iterator<const char*>(str),
                   cpp17_input_iterator<const char*>(str+sizeof(str)),
                   ios, err, v);
         assert(base(iter) == str+sizeof(str)-1);
         assert(err == ios.goodbit);
         assert(std::isnan(v));
     }
+#endif // ANDROID_BROKEN_STRTOLD_NAN
     {
         const char str[] = "1.189731495357231765021264e+49321";
         std::ios_base::iostate err = ios.goodbit;
         v = 0;
         cpp17_input_iterator<const char*> iter =
             f.get(cpp17_input_iterator<const char*>(str),
                   cpp17_input_iterator<const char*>(str+sizeof(str)),
                   ios, err, v);

libcxx/test/std/strings/string.conversions/stold.pass.cpp

assert(std::stold(" 10") == 10);
         assert(std::stold("1.e60", &idx) == 1.e60L);
         assert(idx == 5);
     }
     {
         size_t idx = 0;
         assert(std::stold("INF", &idx) == INFINITY);
         assert(idx == 3);
     }
+#ifndef ANDROID_BROKEN_STRTOLD_NAN
     {
         size_t idx = 0;
         assert(std::isnan(std::stold("NAN", &idx)));
         assert(idx == 3);
     }
+#endif
 
 #ifndef TEST_HAS_NO_EXCEPTIONS
     {
         size_t idx = 0;
         try {
             (void)std::stold("", &idx);
             assert(false);
         } catch (const std::invalid_argument&) {
assert(std::stold(L" 10") == 10);
         assert(std::stold(L"1.e60", &idx) == 1.e60L);
         assert(idx == 5);
     }
     {
         size_t idx = 0;
         assert(std::stold(L"INF", &idx) == INFINITY);
         assert(idx == 3);
     }
+#ifndef ANDROID_BROKEN_STRTOLD_NAN
     {
         size_t idx = 0;
         assert(std::isnan(std::stold(L"NAN", &idx)));
         assert(idx == 3);
     }
+#endif
 #ifndef TEST_HAS_NO_EXCEPTIONS
     {
         size_t idx = 0;
         try {
             (void)std::stold(L"", &idx);
             assert(false);
         } catch (const std::invalid_argument&) {
             assert(idx == 0);

libcxx/utils/libcxx/test/features.py

Feature(name='has-unix-headers',
           when=lambda cfg: sourceBuilds(cfg, """
             #include <unistd.h>
             #include <sys/wait.h>
             int main(int, char**) {
               return 0;
             }
           """)),
 
+  # For Android, some of libc++'s floating-point conversions are implemented
+  # using Bionic (libc.so) on the device, which has bugs and behavioral
+  # differences on older OS versions. We want to adjust test behavior using the
+  # device version, not __ANDROID_API__, because it is useful/interesting to
+  # build test programs against an old Android API level and run them on a new
+  # Android OS.
+
+  # printf's output for NAN has changed between versions of Android. Considering
+  # NAN and -NAN:
+  #  - Android L (APIs 21 and 22): "%f" and "%+f" print "nan".
+  #  - Android M (API 23) and up: "%f" prints "nan" or "-nan", "%+f" prints
+  #    "+nan" or "-nan".
+  # Set this feature for L, where the sign is never printed.
+  Feature(name='android-printf-nan-no-sign',
+          when=lambda cfg: '__ANDROID__' in compilerMacros(cfg) and not programSucceeds(cfg, """
+            #include <android/api-level.h>
+            #include <math.h>
+            #include <stdio.h>
+            int main(int, char**) {
+              // Never enable the workaround on Android M or newer.
+              if (android_get_device_api_level() >= 23) return 0;
+              char buf[100];
+              snprintf(buf, sizeof(buf), "%+f", NAN);
+              return buf[0] != '+';
+            }
+          """),
+          actions=[AddCompileFlag('-DANDROID_PRINTF_NAN_NO_SIGN')]),
+
+  # strtold does not handle "NAN" correctly on 64-bit Android prior to Android O
+  # (API 26). (It produces 0.0 instead of NAN.) Set a macro when the device's
+  # strtold is broken.
+  # See https://android-review.googlesource.com/q/Id7d46ac2d8acb8770b5e8c445e87cfabfde6f111.
+  Feature(name='android-broken-strtold-nan',
+          when=lambda cfg: '__ANDROID__' in compilerMacros(cfg) and not programSucceeds(cfg, """
+            #include <android/api-level.h>
+            #include <math.h>
+            int main(int, char**) {
+              // Never enable the workaround on Android O or newer, or on
+              // unaffected architectures.
+            #if defined(__LP64__)
+              if (android_get_device_api_level() >= 26) return 0;
+              long double result = strtold("NAN", NULL);
+              return !isnan(result);
+            #else
+              return 0;
+            #endif
+            }
+          """),
+          actions=[AddCompileFlag('-DANDROID_BROKEN_STRTOLD_NAN')]),
+
   # Whether Bash can run on the executor.
   # This is not always the case, for example when running on embedded systems.
   #
   # For the corner case of bash existing, but it being missing in the path
   # set in %{exec} as "--env PATH=one-single-dir", the executor does find
   # and executes bash, but bash then can't find any other common shell
   # utilities. Test executing "bash -c 'bash --version'" to see if bash
   # manages to find binaries to execute.