MINGW-packages icon indicating copy to clipboard operation
MINGW-packages copied to clipboard

ucrt gcc implements strtof using __mingw_strtod unless -lucrt is passed

Open anarazel opened this issue 1 year ago • 4 comments

Hi,

freshly updated msys, using the ucrt64 shell, and the ucrt gcc.

We hit the issue that strtof() returns inaccurate results, even though that shouldn't be the case when using ucrt. Some debugging later it turns out that using -lucrt avoids the problem. I inquired on IRC whether it is expected that -lucrt is required to fully utilize ucrt, and it sounds that it is not.

Repro:

$ type gcc
gcc is hashed (/ucrt64/bin/gcc)

$ echo $MSYSTEM
UCRT64

$ cat test.c
#include <stdlib.h>
#include <stdio.h>

void main(void)
{
        printf("%.32a\n", (double)strtof("7038531e-32", NULL));
}

$ gcc test.c -o test.exe
$ gcc test.c -o test-lucrt.exe -lucrt

$ objdump.exe -S test.exe |grep -A5 '<strtof>:'
0000000140002620 <strtof>:
   140002620:   48 83 ec 38             sub    $0x38,%rsp
   140002624:   0f 11 74 24 20          movups %xmm6,0x20(%rsp)
   140002629:   e8 b2 00 00 00          call   1400026e0 <__mingw_strtod>
   14000262e:   66 48 0f 7e c2          movq   %xmm0,%rdx
   140002633:   66 0f 28 f0             movapd %xmm0,%xmm6
   
$ objdump.exe -S test-lucrt.exe |grep -A5 '<strtof>:'
0000000140001900 <strtof>:
   140001900:   ff 25 1e 6a 00 00       jmp    *0x6a1e(%rip)        # 140008324 <__imp_strtof>
   140001906:   90                      nop
   140001907:   90                      nop
   140001908:   0f 1f 84 00 00 00 00    nopl   0x0(%rax,%rax,1)
   14000190f:   00

$ ./test.exe
0x1.5c87fc00000000000000000000000000p-84

$ ./test-lucrt.exe
0x1.5c87fa00000000000000000000000000p-84

Regards,

Andres

anarazel avatar Aug 07 '22 06:08 anarazel

It's because libmingwex.a provides wrapper calling __mingw_strtod for all CRTs but shouldn't do that for UCRT.

__mingw_strtod precision is another issue that would be good to fix.

mati865 avatar Aug 07 '22 15:08 mati865

Hi! Should this be reported to mingw-w64 developers?

lb90 avatar Aug 12 '22 21:08 lb90

Yes, it's mingw-w64 bug.

mati865 avatar Aug 12 '22 22:08 mati865

Let's ping @mstorsjo then 🙂

lb90 avatar Aug 13 '22 09:08 lb90

I'd rather recommend bringing this to mingw-w64 developers via official channel like mailing list or IRC.

mati865 avatar Aug 26 '22 14:08 mati865

FWIW, I do have this on my list of things to tend to, but I don’t manage to clear it like I used to :-(

mstorsjo avatar Aug 26 '22 14:08 mstorsjo

We hit the issue that strtof() returns inaccurate results, even though that shouldn't be the case when using ucrt. Some debugging later it turns out that using -lucrt avoids the problem. I inquired on IRC whether it is expected that -lucrt is required to fully utilize ucrt, and it sounds that it is not.

No, you're not really supposed to need that.

(Technical side note: It shouldn't matter if you specify -lucrt or -lmsvcrt - the thing that matters is whether the linker tries the CRT library or libmingwex.a first. libmsvcrt.a is a copy of libucrt.a if that's the installation's default CRT - i.e. libmsvcrt.a is just "the toolchain's default CRT library". The library that always links against msvcrt.dll is named libmsvcrt-os.a.)

In practice, libmingwex.a does contain a whole bunch of functions, where mingw-w64 have wanted to replace the functions that msvcrt.dll provided (or provide implementations of functions that msvcrt.dll just didn't provide) - but as UCRT is C99 compliant, most of them aren't needed in that configuration. In the best case, the libmingwex.a replacements are equally functional but just bloat the binary a little (as they're statically linked into all executables that use them), but in worst case, they can be less functional than what UCRT could provide.

There's a whole bunch of functions that overlap between what UCRT provides and what libmingwex.a provides - I've tried to slowly reduce that overlap, but it's a huge amount of functions and a wide-sweeping patch to move all of them around is a lot of code churn, so I mostly do such cleanup triggered by a concrete need - like this one. (Also, for any function touching long doubles, on x86 we need to still keep using the libmingwex.a version, since UCRT's versions only works for the MSVC case of double == long double.)

Also, if you manually specify -lucrt so that it's tried before libmingwex.a, there's also a chance that you'd hit a bug where you'd end up directly calling some function which usually is worked around. If that happens, it's still a bug we should fix though.

In any case, for the case of strtof, it should be quite easy to move the frontend function over to libmsvcr*.a so that it doesn't get used in UCRT builds. There's also a separate function named __mingw_strtof (also with the name __strtof), with a different implementation than strtof - and it seems like that one gives better precision than the current implementation of strtof. I'll run some tests with these (I think libcxx's testsuite did exercise this function somewhat, so I'll see how other alternatives work) and propose a patch or two to mingw-w64 after that.

mstorsjo avatar Aug 27 '22 21:08 mstorsjo

I pushed a fix for this now: https://sourceforge.net/p/mingw-w64/mingw-w64/ci/955fa57b120f701e9da9ffb7a1d7c855528bfdef/

mstorsjo avatar Aug 30 '22 20:08 mstorsjo

Fixed in upstream

MehdiChinoune avatar Sep 17 '22 18:09 MehdiChinoune