[libc++] Make ABI annotations explicit for windows-specific code

[philnik777]

This doesn't show up in the CI, since we don't have abilists for windows. I'm also not sure whether we want them, so I don't think we can easily test this change.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/140507.diff

3 Files Affected:

• (modified) libcxx/include/__locale_dir/support/windows.h (+6)
• (modified) libcxx/include/__thread/support/windows.h (+2)
• (modified) libcxx/src/support/win32/thread_win32.cpp (+2)

diff --git a/libcxx/include/__locale_dir/support/windows.h b/libcxx/include/__locale_dir/support/windows.h
index 0d3089c150081..99f6667729a9d 100644
--- a/libcxx/include/__locale_dir/support/windows.h
+++ b/libcxx/include/__locale_dir/support/windows.h
@@ -154,6 +154,7 @@ class __locale_t {
 };
 
 #if defined(_LIBCPP_BUILDING_LIBRARY)
+_LIBCPP_BEGIN_EXPLICIT_ABI_ANNOTATIONS
 _LIBCPP_EXPORTED_FROM_ABI __locale_t __newlocale(int __mask, const char* __locale, __locale_t __base);
 inline _LIBCPP_HIDE_FROM_ABI void __freelocale(__locale_t __loc) { ::_free_locale(__loc); }
 inline _LIBCPP_HIDE_FROM_ABI char* __setlocale(int __category, const char* __locale) {
@@ -163,6 +164,7 @@ inline _LIBCPP_HIDE_FROM_ABI char* __setlocale(int __category, const char* __loc
   return __new_locale;
 }
 _LIBCPP_EXPORTED_FROM_ABI __lconv_t* __localeconv(__locale_t& __loc);
+_LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS
 #endif // _LIBCPP_BUILDING_LIBRARY
 
 //
@@ -178,8 +180,10 @@ inline _LIBCPP_HIDE_FROM_ABI long double __strtold(const char* __nptr, char** __
   return ::_strtold_l(__nptr, __endptr, __loc);
 }
 #else
+_LIBCPP_BEGIN_EXPLICIT_ABI_ANNOTATIONS
 _LIBCPP_EXPORTED_FROM_ABI float __strtof(const char*, char**, __locale_t);
 _LIBCPP_EXPORTED_FROM_ABI long double __strtold(const char*, char**, __locale_t);
+_LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS
 #endif
 
 inline _LIBCPP_HIDE_FROM_ABI double __strtod(const char* __nptr, char** __endptr, __locale_t __loc) {
@@ -274,6 +278,7 @@ inline _LIBCPP_HIDE_FROM_ABI int __mbtowc(wchar_t* __pwc, const char* __pmb, siz
   return ::_mbtowc_l(__pwc, __pmb, __max, __loc);
 }
 
+_LIBCPP_BEGIN_EXPLICIT_ABI_ANNOTATIONS
 _LIBCPP_EXPORTED_FROM_ABI size_t __mbrlen(const char* __restrict, size_t, mbstate_t* __restrict, __locale_t);
 
 _LIBCPP_EXPORTED_FROM_ABI size_t
@@ -285,6 +290,7 @@ _LIBCPP_EXPORTED_FROM_ABI _LIBCPP_ATTRIBUTE_FORMAT(__printf__, 4, 5) int __snpri
 
 _LIBCPP_EXPORTED_FROM_ABI
 _LIBCPP_ATTRIBUTE_FORMAT(__printf__, 3, 4) int __asprintf(char** __ret, __locale_t __loc, const char* __format, ...);
+_LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS
 
 _LIBCPP_DIAGNOSTIC_PUSH
 _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wgcc-compat")
diff --git a/libcxx/include/__thread/support/windows.h b/libcxx/include/__thread/support/windows.h
index 5dc4fa14f45b6..9e2d62ec72050 100644
--- a/libcxx/include/__thread/support/windows.h
+++ b/libcxx/include/__thread/support/windows.h
@@ -19,6 +19,7 @@
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
+_LIBCPP_BEGIN_EXPLICIT_ABI_ANNOTATIONS
 
 using __libcpp_timespec_t = ::timespec;
 
@@ -128,6 +129,7 @@ _LIBCPP_EXPORTED_FROM_ABI void* __libcpp_tls_get(__libcpp_tls_key __key);
 
 _LIBCPP_EXPORTED_FROM_ABI int __libcpp_tls_set(__libcpp_tls_key __key, void* __p);
 
+_LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___THREAD_SUPPORT_WINDOWS_H
diff --git a/libcxx/src/support/win32/thread_win32.cpp b/libcxx/src/support/win32/thread_win32.cpp
index 3a67d759f0f5e..606104e32b453 100644
--- a/libcxx/src/support/win32/thread_win32.cpp
+++ b/libcxx/src/support/win32/thread_win32.cpp
@@ -16,6 +16,7 @@
 #include <fibersapi.h>
 
 _LIBCPP_BEGIN_NAMESPACE_STD
+_LIBCPP_BEGIN_EXPLICIT_ABI_ANNOTATIONS
 
 static_assert(sizeof(__libcpp_mutex_t) == sizeof(SRWLOCK), "");
 static_assert(alignof(__libcpp_mutex_t) == alignof(SRWLOCK), "");
@@ -211,4 +212,5 @@ int __libcpp_tls_set(__libcpp_tls_key __key, void* __p) {
   return 0;
 }
 
+_LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS
 _LIBCPP_END_NAMESPACE_STD

[mstorsjo]

This seems to fail to compile:

llvm-project/libcxx/src/support/win32/locale_win32.cpp:18:1: error: cannot add 'abi_tag' attribute in a redeclaration
   18 | _LIBCPP_BEGIN_NAMESPACE_STD
      | ^
llvm-project/runtimes/build-aarch64/include/c++/v1/__config:586:39: note: expanded from macro '_LIBCPP_BEGIN_NAMESPACE_STD'
  586 | #  define _LIBCPP_BEGIN_NAMESPACE_STD _LIBCPP_BEGIN_UNVERSIONED_NAMESPACE_STD inline namespace _LIBCPP_ABI_NAMESPACE {
      |                                       ^
llvm-project/runtimes/build-aarch64/include/c++/v1/__config:581:40: note: expanded from macro '_LIBCPP_BEGIN_UNVERSIONED_NAMESPACE_STD'
  581 |     _LIBCPP_PUSH_EXTENSION_DIAGNOSTICS _LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS namespace _LIBCPP_NAMESPACE_VISIBILITY std {
      |                                        ^
llvm-project/runtimes/build-aarch64/include/c++/v1/__config:566:5: note: expanded from macro '_LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS'
  566 |     _Pragma(_LIBCPP_TOSTRING(clang attribute _LibcxxExplicitABIAnnotations.push(__attribute__((                        \
      |     ^
<scratch space>:28:110: note: expanded from here
   28 |  clang attribute _LibcxxExplicitABIAnnotations.push(__attribute__(( __exclude_from_explicit_instantiation__, __abi_tag__("ne210000"))), apply_to = function)                                                                      |                                                                                                              ^
llvm-project/runtimes/build-aarch64/include/c++/v1/__locale_dir/support/windows.h:292:48: note: previous declaration is here
  292 | _LIBCPP_ATTRIBUTE_FORMAT(__printf__, 3, 4) int __asprintf(char** __ret, __locale_t __loc, const char* __format, ...);
      |                                                ^

[mstorsjo]

This doesn't show up in the CI, since we don't have abilists for windows. I'm also not sure whether we want them,

I think it would be useful to have them anyway; for mingw we want to break ABI unless we have good reason for it (and the impact or very low). For msvc we don’t strictly try to maintain a fixed ABI, but knowing when it changes and when it doesn’t probably still is useful?

[philnik777]

This doesn't show up in the CI, since we don't have abilists for windows. I'm also not sure whether we want them,

I think it would be useful to have them anyway; for mingw we want to break ABI unless we have good reason for it (and the impact or very low). For msvc we don’t strictly try to maintain a fixed ABI, but knowing when it changes and when it doesn’t probably still is useful?

I guess so. Is there a tool like nm we can use to extract that information on windows? I guess mingw has something very similar, probably even nm itself, but I'm not aware of an equivalent for MSVC.

I've uploaded a new version which should hopefully fix the issue. I don't have a windows environment available, so it's not easy to test for me.

[mstorsjo]

I guess so. Is there a tool like nm we can use to extract that information on windows? I guess mingw has something very similar, probably even nm itself, but I'm not aware of an equivalent for MSVC.

Yeah mingw has got nm or llvm-nm. For msvc style environments, clang-cl probably ships with llvm-nm as well. If not, and for pure MSVC environments (which libc++ don’t support now) you’d maybe use dumpbin.exe which would indeed be a more involved matter.

I've uploaded a new version which should hopefully fix the issue.

Thanks! Libc++ builds now, but building user code now fails

/home/martin/clang-nightly-mon/aarch64-w64-mingw32/include/c++/v1/__locale_dir/support/windows.h:186:1: error: cannot add 'abi_tag' attribute in a redeclaration
  186 | _LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS
      | ^
/home/martin/clang-nightly-mon/aarch64-w64-mingw32/include/c++/v1/__config:566:5: note: expanded from macro '_LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS'
  566 |     _Pragma(_LIBCPP_TOSTRING(clang attribute _LibcxxExplicitABIAnnotations.push(__attribute__((                        \
      |     ^
<scratch space>:126:110: note: expanded from here
  126 |  clang attribute _LibcxxExplicitABIAnnotations.push(__attribute__(( __exclude_from_explicit_instantiation__, __abi_tag__("fe210000"))), apply_to = function)
      |                                                                                                              ^
/home/martin/clang-nightly-mon/aarch64-w64-mingw32/include/stdlib.h:606:24: note: previous declaration is here
  606 |   unsigned int __cdecl _rotl(unsigned int _Val,int _Shift);
      |                        ^
/home/martin/clang-nightly-mon/aarch64-w64-mingw32/include/c++/v1/__locale_dir/support/windows.h:186:1: error: unterminated '#pragma clang attribute push' at end of file
  186 | _LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS
      | ^
/home/martin/clang-nightly-mon/aarch64-w64-mingw32/include/c++/v1/__config:566:5: note: expanded from macro '_LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS'
  566 |     _Pragma(_LIBCPP_TOSTRING(clang attribute _LibcxxExplicitABIAnnotations.push(__attribute__((                        \
      |     ^
<scratch space>:126:8: note: expanded from here
  126 |  clang attribute _LibcxxExplicitABIAnnotations.push(__attribute__(( __exclude_from_explicit_instantiation__, __abi_tag__("fe210000"))), apply_to = function)
      |        ^

I don't have a windows environment available, so it's not easy to test for me.

FWIW, I do all this build testing on Linux; a cross compiling llvm-mingw environment is available by just untarring a package from https://github.com/mstorsjo/llvm-mingw/releases.

[philnik777]

I guess so. Is there a tool like nm we can use to extract that information on windows? I guess mingw has something very similar, probably even nm itself, but I'm not aware of an equivalent for MSVC.

Yeah mingw has got nm or llvm-nm. For msvc style environments, clang-cl probably ships with llvm-nm as well. If not, and for pure MSVC environments (which libc++ don’t support now) you’d maybe use dumpbin.exe which would indeed be a more involved matter.

Makes sense. Does llvm-nm have support for MSVC binaries?

I've uploaded a new version which should hopefully fix the issue.

Thanks! Libc++ builds now, but building user code now fails

/home/martin/clang-nightly-mon/aarch64-w64-mingw32/include/c++/v1/__locale_dir/support/windows.h:186:1: error: cannot add 'abi_tag' attribute in a redeclaration
  186 | _LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS
      | ^
/home/martin/clang-nightly-mon/aarch64-w64-mingw32/include/c++/v1/__config:566:5: note: expanded from macro '_LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS'
  566 |     _Pragma(_LIBCPP_TOSTRING(clang attribute _LibcxxExplicitABIAnnotations.push(__attribute__((                        \
      |     ^
<scratch space>:126:110: note: expanded from here
  126 |  clang attribute _LibcxxExplicitABIAnnotations.push(__attribute__(( __exclude_from_explicit_instantiation__, __abi_tag__("fe210000"))), apply_to = function)
      |                                                                                                              ^
/home/martin/clang-nightly-mon/aarch64-w64-mingw32/include/stdlib.h:606:24: note: previous declaration is here
  606 |   unsigned int __cdecl _rotl(unsigned int _Val,int _Shift);
      |                        ^
/home/martin/clang-nightly-mon/aarch64-w64-mingw32/include/c++/v1/__locale_dir/support/windows.h:186:1: error: unterminated '#pragma clang attribute push' at end of file
  186 | _LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS
      | ^
/home/martin/clang-nightly-mon/aarch64-w64-mingw32/include/c++/v1/__config:566:5: note: expanded from macro '_LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS'
  566 |     _Pragma(_LIBCPP_TOSTRING(clang attribute _LibcxxExplicitABIAnnotations.push(__attribute__((                        \
      |     ^
<scratch space>:126:8: note: expanded from here
  126 |  clang attribute _LibcxxExplicitABIAnnotations.push(__attribute__(( __exclude_from_explicit_instantiation__, __abi_tag__("fe210000"))), apply_to = function)
      |        ^

I don't have a windows environment available, so it's not easy to test for me.

FWIW, I do all this build testing on Linux; a cross compiling llvm-mingw environment is available by just untarring a package from https://github.com/mstorsjo/llvm-mingw/releases.

I've just tried that, but didn't get that far. Still, I think I've found the problem and updated the PR.

[mstorsjo]

This seems to work, and fixes my issues. (Haven’t reviewed the code itself though.)

[mstorsjo]

Makes sense. Does llvm-nm have support for MSVC binaries?

Yes; the binaries aren’t really different in any way, and all the llvm tools use the same internal code anyway.

FWIW, I do all this build testing on Linux; a cross compiling llvm-mingw environment is available by just untarring a package from https://github.com/mstorsjo/llvm-mingw/releases.

I've just tried that, but didn't get that far. Still, I think I've found the problem and updated the PR.

Thanks! I can try to provide more specific instructions at some other point if relevant then.