[libcxx-commits] [libcxx] [libc++] Don't export ABI tagged __str_to_float_c_locale specializations on MinGW (PR #229456)

[Martin Storsjö via libcxx-commits]

mstorsjo wrote:

> Commit [c7da05f](https://github.com/llvm/llvm-project/commit/c7da05f9828542daaf634a6f4277212b5f6194d5) (PR #229456: "[libc++] Don't export ABI tagged __str_to_float_c_locale specializations on MinGW") broke the MinGW/Windows cross-compilation of libc++:

FYI, libc++ does have CI build testing for mingw targets - so it's not entirely broken. Admittedly, the CI setup only covers UCRT configurations, not for msvcrt.dll. But locally I run CI that tests building for msvcrt.dll as well, and that one has run successfully for me too.

> **Root Cause:** In commit [c7da05f](https://github.com/llvm/llvm-project/commit/c7da05f9828542daaf634a6f4277212b5f6194d5), `__str_to_float_c_locale<float>` was moved out of `#if defined(_LIBCPP_MSVCRT)` so that it applies unconditionally on Windows, and the MinGW-specific specialization in `libcxx/src/support/win32/locale_win32.cpp` was removed. However, in standard MinGW-w64 environments targeting `msvcrt.dll`, `::_strtof_l` is not exposed in the global namespace in `<locale.h>`/`<stdlib.h>` (unlike UCRT environments).

In this case, it turns out that the `_strtof_l` function is getting polyfilled for msvcrt.dll since commit https://github.com/mingw-w64/mingw-w64/commit/03ad04eba63ef518e76daf4ceec36ce82d41a587 in 2022, but I presume that you're using an older version of mingw-w64 than this.

> Could you please revert this commit or provide a fix for MinGW environments lacking `_strtof_l` in the global namespace?

Reverting is not ideal at this point, as that causes other sorts of breakage, where the libc++.dll exported symbols contains an ABI tag, causing some operations to only work if you build with the exact same C++ version as used for building libc++.dll itself.

> **Suggested fix:** We can mirror the pattern already introduced for long double: provide a plain (non-template) exported helper `__strtof_c_locale` in `locale_win32.cpp` so that no `abi_tag` is inherited:
> 
> In `libcxx/src/support/win32/locale_win32.cpp`:
> 
> ```
> #if !defined(_LIBCPP_MSVCRT)
> float __strtof_c_locale(const char* nptr, char** endptr) {
>   __locale_guard __current(__get_c_locale());
>   return std::strtof(nptr, endptr);
> }
> 
> 
> long double __strtold_c_locale(const char* nptr, char** endptr) {
>   __locale_guard __current(__get_c_locale());
>   return std::strtold(nptr, endptr);
> }
> #endif
> ```

Such a workaround might be safe to do. However do note that using the `__locale_guard __current(__get_c_locale());` workaround usually comes with significant performance loss, which may be a pity if we otherwise actually could use the real `_strtof_l`. See https://github.com/llvm/llvm-project/issues/56202 for an existing case of that.

Having `_strtof_l` fall back on a real `_strtod_l` which takes a locale parameter would be better, but that may require a couple lines of extra code for doing the float specific range checks - see https://github.com/mingw-w64/mingw-w64/commit/03ad04eba63ef518e76daf4ceec36ce82d41a587.

https://github.com/llvm/llvm-project/pull/229456