Skip to content

libc++: std/utilities/charconv/charconv.msvc/test.pass.cpp fails after #91651 #113265

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

Open
pirama-arumuga-nainar opened this issue Oct 22, 2024 · 14 comments
Assignees
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. test-suite

Comments

@pirama-arumuga-nainar
Copy link
Collaborator

After #91651, the above test fails in an internal Android builder when targeting i386-linux-gnu. Tagging @michaelrj-google.

Command Output (stdout):
--
# COMPILED WITH
/tmpfs/src/git/out/stage2/./bin/clang++ /tmpfs/src/git/out/llvm-project/libcxx/test/std/utilities/charconv/charconv.msvc/test.pass.cpp -pthread --target=i386-unknown-linux-gnu -nostdinc++ -I /tmpfs/src/git/out/stage2/runtimes/runtimes-i386-unknown-linux-gnu-bins/libcxx/test-suite-install/include/i386-unknown-linux-gnu/c++/v1 -I /tmpfs/src/git/out/stage2/runtimes/runtimes-i386-unknown-linux-gnu-bins/libcxx/test-suite-install/include/c++/v1 -I /tmpfs/src/git/out/llvm-project/libcxx/test/support -std=c++26 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-noexcept-type -Wno-atomic-alignment -Wno-reserved-module-identifier -Wdeprecated-copy -Wdeprecated-copy-dtor -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -Wno-local-type-template-args -Wno-c++11-extensions -Wno-unknown-pragmas -Wno-pass-failed -Wno-mismatched-new-delete -Wno-redundant-move -Wno-self-move -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_NONE -Werror=thread-safety -Wuser-defined-warnings  -lc++experimental -nostdlib++ -L /tmpfs/src/git/out/stage2/runtimes/runtimes-i386-unknown-linux-gnu-bins/libcxx/test-suite-install/lib/i386-unknown-linux-gnu -Wl,-rpath,/tmpfs/src/git/out/stage2/runtimes/runtimes-i386-unknown-linux-gnu-bins/libcxx/test-suite-install/lib/i386-unknown-linux-gnu -lc++ -latomic -o /tmpfs/src/git/out/stage2/runtimes/runtimes-i386-unknown-linux-gnu-bins/libcxx/test/std/utilities/charconv/charconv.msvc/Output/test.pass.cpp.dir/t.tmp.exe
# executed command: /tmpfs/src/git/out/stage2/./bin/clang++ /tmpfs/src/git/out/llvm-project/libcxx/test/std/utilities/charconv/charconv.msvc/test.pass.cpp -pthread --target=i386-unknown-linux-gnu -nostdinc++ -I /tmpfs/src/git/out/stage2/runtimes/runtimes-i386-unknown-linux-gnu-bins/libcxx/test-suite-install/include/i386-unknown-linux-gnu/c++/v1 -I /tmpfs/src/git/out/stage2/runtimes/runtimes-i386-unknown-linux-gnu-bins/libcxx/test-suite-install/include/c++/v1 -I /tmpfs/src/git/out/llvm-project/libcxx/test/support -std=c++26 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-noexcept-type -Wno-atomic-alignment -Wno-reserved-module-identifier -Wdeprecated-copy -Wdeprecated-copy-dtor -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -Wno-local-type-template-args -Wno-c++11-extensions -Wno-unknown-pragmas -Wno-pass-failed -Wno-mismatched-new-delete -Wno-redundant-move -Wno-self-move -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_NONE -Werror=thread-safety -Wuser-defined-warnings -lc++experimental -nostdlib++ -L /tmpfs/src/git/out/stage2/runtimes/runtimes-i386-unknown-linux-gnu-bins/libcxx/test-suite-install/lib/i386-unknown-linux-gnu -Wl,-rpath,/tmpfs/src/git/out/stage2/runtimes/runtimes-i386-unknown-linux-gnu-bins/libcxx/test-suite-install/lib/i386-unknown-linux-gnu -lc++ -latomic -o /tmpfs/src/git/out/stage2/runtimes/runtimes-i386-unknown-linux-gnu-bins/libcxx/test/std/utilities/charconv/charconv.msvc/Output/test.pass.cpp.dir/t.tmp.exe
# EXECUTED AS
/tmpfs/src/git/prebuilts/python/linux-x86/bin/python3.11 /tmpfs/src/git/out/llvm-project/libcxx/utils/run.py --execdir /tmpfs/src/git/out/stage2/runtimes/runtimes-i386-unknown-linux-gnu-bins/libcxx/test/std/utilities/charconv/charconv.msvc/Output/test.pass.cpp.dir --  /tmpfs/src/git/out/stage2/runtimes/runtimes-i386-unknown-linux-gnu-bins/libcxx/test/std/utilities/charconv/charconv.msvc/Output/test.pass.cpp.dir/t.tmp.exe
# executed command: /tmpfs/src/git/prebuilts/python/linux-x86/bin/python3.11 /tmpfs/src/git/out/llvm-project/libcxx/utils/run.py --execdir /tmpfs/src/git/out/stage2/runtimes/runtimes-i386-unknown-linux-gnu-bins/libcxx/test/std/utilities/charconv/charconv.msvc/Output/test.pass.cpp.dir -- /tmpfs/src/git/out/stage2/runtimes/runtimes-i386-unknown-linux-gnu-bins/libcxx/test/std/utilities/charconv/charconv.msvc/Output/test.pass.cpp.dir/t.tmp.exe
# .---command stdout------------
# | USAGE:
# | test.exe              : generates seed data from random_device.
# | Generated seed data.
# | SEED DATA:
# | 4150976393 3711790134 1069703474 706999699 1971770358 3309215770 315823703 3438922270 3375396010 3573917276 1464116950 2968488237 2635585040 1301939270 4016078018 1858822477 864698069 2180268580 1399627027 2957610965 1551827513 394111556 1070962202 970487149 669668700 739835117 1090220901 2797540012 367867186 1501822844 622556964 527597392 3229660502 2944447654 2393434273 2663154033 2826098939 4243961491 1993504978 775043048 2509465299 2993586794 547971811 3873481846 1998480752 3540768727 1044319585 4008618232 3721234028 3007175688 2727512756 2329462586 3332352455 772414078 3250242665 1610100940 858847743 2661313874 2336756750 1776455284 4276195205 358646463 2104487040 3920762637 4090424221 1967619469 845149557 1862608782 4245042032 1075069006 3384430723 2807250094 1212711042 4082166226 100160162 408235072 246788583 2511265277 1553045601 3720410015 2421671760 1106341172 1163252853 3491961620 4142752856 3117210174 852553540 3447870601 1130786657 347219773 21846054 332659153 2555859477 3894862018 90948087 1758713998 4095479472 501782802 536974067 660340020 507753747 840335557 3409036656 1175756794 3033903060 3836095384 1563534043 4279435890 3297322105 1224514310 3772157393 1500986051 3165361068 3372580099 4144187513 3361014105 678448207 100179316 2449638254 2278222765 416412939 4199731825 138470610 3928315356 3521305251 3497554947 1625795678 2291361087 3009357047 14986102 2595281852 1816002357 3107531342 2485593975 1620117285 931082003 3479195811 1818749847 2463086069 604679334 3151626016 2732198782 825284663 2696848734 1887231690 4185925266 3949292971 2645268376 1672063031 1933426132 870700167 3556840969 2340948033 2731198097 4056657444 3784742498 1916832168 2919511413 2305431511 705852525 2602904227 1057141694 636368990 1362217366 2884082280 34054436 1550372851 1302565347 3625978560 816780201 2949439255 1120081332 3441104273 2287615940 3231300080 4047377209 2336248441 2191564582 2138086008 3871426583 940310672 365959020 850555418 1560970233 1373027441 2345302169 2489801108 3359167155 1474872415 1299583502 3113889144 2941495221 4254351253 3736188484 3969589144 2519679394 3512644815 1738725367 2183914604 3123315133 2451021082 3587296439 1046078801 3568499145 3599543086 1504839079 3193535693 294573448 149586005 3818588504 637673461 976535988 716371099 3348011911 2811324363 1061143303 579686703 1284655308 3895206504 2312622801 2746754362 849617174 3033413469 3058629025 3957136779 1235984739 2014581996 2494980555 3630096385 1980231820 4009578254 4250165964 1021860465 657969244 2808200143 3586383089 720610499 1176226099 4143163377 3320000793 685830474 2998346433 1651189432 3484343575 710937051 1219193113 2335957507 376269951 1258747235 3120884617 3140536513 3954344430 3981648271 404735575 1137973607 2206565899 2360019245 3919411120 3932232387 3058901574 3145994458 3742359061 327891592 136239408 1054962576 1902850847 1953895082 2870131113 2621484381 1727576122 797704630 2711782750 2394640531 3318704870 3886686298 796730276 312091540 4290851122 961930759 4030077102 1106836230 864717451 1655339308 1919690857 148688060 3152401784 1519367618 271366555 3142161758 2836282297 3126466022 2671248331 2737850281 4074578276 3033031081 698145470 137229229 1485662621 3014994257 245427596 2334857026 1497942082 1122585913 1814010099 3075817175 986647821 3478059528 2015463172 332954090 4169526230 976393972 2452566976 551154409 4034912632 3019144764 1585172570 3949198877 2763305758 809851124 2013674917 421006400 547099941 2442800830 3996379329 956821263 2832534433 1598972084 864735102 3854107049 327345594 1317141824 430690269 1171662308 2089258147 3566408174 2179288951 2983821623 3839468069 4162606967 2309337789 2854917878 4283050187 4267470755 1065657258 3047765024 2924449256 2711227044 2867821929 3161151787 2332896213 41419395 3132315322 2059871500 2447391597 3536604486 2570484782 660052137 2474614903 1572218142 2379013057 4085540905 4034755551 1686444318 1791063176 2144931750 2525138776 338909971 1253445777 3897891160 1743069410 2935128263 4
# `-----------------------------
# .---command stderr------------
# | round-trip failed for 0xC5A9E6BDDE4C5D33
# | This is a randomized test.
# | DO NOT IGNORE/RERUN THIS FAILURE.
# | You must report it to the STL maintainers.
# `-----------------------------
# error: command failed with exit status: 250
@github-actions github-actions bot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Oct 22, 2024
@michaelrj-google
Copy link
Contributor

Ack, will look into it when I get a chance.

@michaelrj-google
Copy link
Contributor

Do you have any more information on how to replicate this (e.g. cmake arguments?) I'm not very familiar with the libcxx tests.

@frederick-vs-ja
Copy link
Contributor

@pirama-arumuga-nainar
Copy link
Collaborator Author

@ldionne
Copy link
Member

ldionne commented Nov 22, 2024

Those are tests timing out. I disabled them under msan since they're seemingly just too slow to run under that sanitizer.

@kongy
Copy link
Collaborator

kongy commented Jan 23, 2025

This test remains flaky in Android. Note that we are seeing a failure, not a test timeout.

@philnik777
Copy link
Contributor

This test remains flaky in Android. Note that we are seeing a failure, not a test timeout.

And where would you be able to look that that failure?

@kongy
Copy link
Collaborator

kongy commented Jan 23, 2025

And where would you be able to look that that failure?

The failure message is in #1.

If you want to reproduce the ToT Android LLVM toolchain build, you will need to first install the Android repo tool, then:

$ mkdir llvm-toolchain && cd llvm-toolchain
$ repo init -u https://android.googlesource.com/platform/manifest -b llvm-toolchain
$ repo sync -c
$ toolchain/llvm_android/build.py --build-llvm-next --llvm-rev=main --no-build=windows

Towards the end of the build, libcxx tests will be ran, and there is an around 50% chance that the test would fail.

@michaelrj-google
Copy link
Contributor

Is there a way to skip the unrelated tests? Running the full suite with the provided command takes ~20 minutes on my workstation and I've yet to get the error. Being able to rebuild faster would greatly help in debugging.

@pirama-arumuga-nainar
Copy link
Collaborator Author

pirama-arumuga-nainar commented Jan 23, 2025

For the llvm-toolchain repro above, once cmake for stage2 is complete, you can ctrl-c out of the stage2 build and run ninja check-cxx-i386-unknown-linux-gnu in out/stage2.

@michaelrj-google
Copy link
Contributor

michaelrj-google commented Jan 24, 2025

Alright, I have managed to replicate. The randomized test gave me a different failing test case so I'll talk them through separately. Also: The only part that's important is the "round-trip failed for <hex>". The hex is the binary representation of the float.

  1. The case at the top is 0xC5A9E6BDDE4C5D33, which decodes to NaN. The cause is fairly simple: NaN just doesn't round trip. We need to add a check to skip the round trip check for NaN here: https://github.com/llvm/llvm-project/blob/main/libcxx/test/std/utilities/charconv/charconv.msvc/test.cpp#L625

  2. The case I found was 0x462F7B643D5A5DED which decodes to 1.247130438231756e+30. This seems like a real failure, but I don't have time to debug it before EoD. I'll try to get to it tomorrow. If someone else wants to try, add the following to the end of test_random_errors in https://github.com/llvm/llvm-project/blob/main/libcxx/test/std/utilities/charconv/charconv.from.chars/floating_point.pass.cpp

{
    const char* s    = "1.247130438231756e+30";
    const char* last = s + std::strlen(s) - 1;

    double value                   = 0.25;
    std::from_chars_result result = std::from_chars(s, last, value);

    assert(result.ec == std::errc{});
    assert(result.ptr == last);

    assert(value == 1.247130438231756e+30);
  }

@kongy
Copy link
Collaborator

kongy commented Jan 24, 2025

Some of the other failing cases we observed:

0x3F92EF16EEE175FB
0x45ECF9CF7E05C4DD
0xC6771E7E3EE56FC5

@michaelrj-google
Copy link
Contributor

Update: I was wrong about that first case being NaN, the tool I was using to convert had an error, and the test already prevents NaNs from getting generated. Additionally, the errors don't seem to be replicating when targeting x86_64 (the code I provided also has an error, it shouldn't be strlen(s) - 1 it should just be strlen(s). Clearly I need to be more careful writing updates right before signing off).

On the more positive side, I can consistently replicate the error by manually setting bits to one of the hex values. By doing even more hacky stuff to the test code I've managed to examine the result. It looks like we're dealing with rounding issues, specifically with odd numbers. 0xC5A9E6BDDE4C5D33 is getting parsed back as 0xC5A9E6BDDE4C5D32, and 0x462F7B643D5A5DED is coming back as 0X462F7B643D5A5DEE. I haven't checked the other three posted yet, but I can tell they're all odd. Before going back in the one thing I'll say is that this only seems to affect the least significant bit, so it shouldn't come up in real world code very much.

@michaelrj-google
Copy link
Contributor

Current Status:
I've narrowed the issue down to the libc code specifically on i386. On x86_64 the tests come back clean, and I'm pretty confident the other targets that libc officially supports are fine as well.

Details:
I found that the input to the decimal_exp_to_float function is identical, and the result i just off by 1 bit. As mentioned above, it seems like this is an issue specifically with it rounding down even when it should round towards closest.

I have some other work with more pressing deadlines, so this is going on the backburner for a bit, but if you want to try yourself, here's the diff I used for testing:

diff --git a/libcxx/src/include/from_chars_floating_point.h b/libcxx/src/include/from_chars_floating_point.h
index 19eeeb28f..302508ddb 100644
--- a/libcxx/src/include/from_chars_floating_point.h
+++ b/libcxx/src/include/from_chars_floating_point.h
@@ -20,6 +20,7 @@
 #include <charconv>
 #include <concepts>
 #include <limits>
+#include <stdio.h>
 
 // Included for the _Floating_type_traits class
 #include "to_chars_floating_point.h"
@@ -392,6 +393,7 @@ __from_chars_result<_Fp> __from_chars_floating_point_decimal(
     // This function expects to parse a positive value. This means it does not
     // take a __first, __n as arguments, since __first points to '-' for
     // negative values.
+    printf("IF: %i, %llu, %.*s\n", __exponent, static_cast<long long>(__fractional.__mantissa), __last - __ptr, __ptr);
     auto __temp = LIBC_NAMESPACE::shared::decimal_exp_to_float<_Fp>(
         {__fractional.__mantissa, __exponent},
         __fractional.__truncated,
@@ -399,6 +401,7 @@ __from_chars_result<_Fp> __from_chars_floating_point_decimal(
         __ptr,
         __last - __ptr);
     __expanded_float = __temp.num;
+    printf("EF: %i, %llu\n", __expanded_float.exponent, static_cast<long long>(__expanded_float.mantissa));
     if (__temp.error == ERANGE) {
       __result.__ec = errc::result_out_of_range;
     }
diff --git a/libcxx/test/std/utilities/charconv/charconv.msvc/test.cpp b/libcxx/test/std/utilities/charconv/charconv.msvc/test.cpp
index ace6d46b8..45cd0344e 100644
--- a/libcxx/test/std/utilities/charconv/charconv.msvc/test.cpp
+++ b/libcxx/test/std/utilities/charconv/charconv.msvc/test.cpp
@@ -609,8 +609,9 @@ void test_floating_prefix(const conditional_t<IsDouble, std::uint64_t, std::uint
     char stdio_buffer[stdio_buffer_size];
 
     for (std::uint32_t frac = 0; frac < Fractions; ++frac) {
-        const UIntType bits      = prefix + frac;
-        const FloatingType input = _Bit_cast<FloatingType>(bits);
+       UIntType bits      = prefix + frac;
+       bits = UIntType(0x462F7B643D5A5DED);
+       const FloatingType input = _Bit_cast<FloatingType>(bits);
 
         {
             const auto to_result = to_chars(buffer, end(buffer), input, chars_format::scientific);
@@ -620,9 +621,13 @@ void test_floating_prefix(const conditional_t<IsDouble, std::uint64_t, std::uint
 
             const auto from_result = from_chars(buffer, last, val);
 
+            const auto to_result_two = to_chars(buffer, end(buffer), _Bit_cast<UIntType>(val));
+
+            buffer[to_result_two.ptr-buffer] = '\0';
+
             assert_message_bits(from_result.ptr == last, "from_result.ptr", bits);
             assert_message_bits(from_result.ec == errc{}, "from_result.ec", bits);
-            assert_message_bits(_Bit_cast<UIntType>(val) == bits, "round-trip", bits);
+            assert_message_bits(_Bit_cast<UIntType>(val) == bits, buffer, bits);
 #endif // TEST_HAS_FROM_CHARS_FLOATING_POINT
         }

The result I get on i386 when running libcxx/test/std/utilities/charconv/charconv.msvc/Output/test.pass.cpp.dir/t.tmp.exe is:

IF: 15, 1247130438231756, 1.247130438231756e+30
EF: 1122, 8861394734308846

The correct values are:

IF: 15, 1247130438231756, 1.247130438231756e+30
EF: 1122, 8861394734308845

(The last bit of the expanded float mantissa is lower)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. test-suite
Projects
None yet
Development

No branches or pull requests

7 participants