[libc++] Introduce a new attribute keyword for Clang improves compatibility with Mingw-GCC

[kikairoya]

A new ABI annotation keyword _LIBCPP_HIDE_FROM_ABI_MINGW_OR_AFTER_V1 needs to be introduced and attached to ostream::sentry::sentry, ostream::sentry::~sentry and istream::sentry to improve binary compatibility on MinGW platform.

In MinGW environment, Clang handles dllexport attribute of internal class that defined in class template in different way from GCC. This incompatibility should be fixed but breaks ABI of libc++, so we need to introduce the new keyword to keep ABI in MinGW environment with old and patched Clang and to stay ABI compatible on other platforms.

This attribute is attached only for basic_ostream::sentry::sentry, basic_ostream::sentry::~sentry and basic_istream::sentry::sentry. Other entities won't be affected by patching Clang so doesn't need to be annotate.

Background

Clang (targeting MinGW a.k.a. windows-gnu, slightly different from windows-msvc) handles template instantiation:

• When exporting: extern template __declspec(dllexport) class TheTemplateClass<T>;
  allows exporting the outer template instantiation, but not its nested types (e.g., InnerClass).
• When importing: extern template __declspec(dllimport) class TheTemplateClass<T>;
  try to import the outer template instantiation, but not its nested types - they will be instantiated in client object.

But MinGW-GCC handles template instantiation differently:

• When exporting: extern template __declspec(dllexport) class TheTemplateClass<T>;
  allows exporting the outer template instantiation, but not its nested types (e.g., InnerClass).
• When importing: extern template __declspec(dllimport) class TheTemplateClass<T>;
  causes MinGW-GCC to also try importing nested types such as TheTemplateClass::InnerClass,
  even if they were never exported. This leads to linker errors like: undefined reference to TheTemplateClass<T>::InnerClass::...

This difference causes link-time problems ( duplicated symbol or undefined reference ) or run-time problems ( illegal memory access, crash or other strange errors ) as reported in #135910 , so we are trying to align the behavior of Clang to MinGW-GCC.

But modifying Clang breaks libc++:

ld.lld: error: undefined symbol: std::__1::basic_ostream<char, std::__1::char_traits<char>>::sentry::sentry(std::__1::basic_ostream<char, std::__1::char_traits<char>>&)
>>> referenced by tools/clang/utils/TableGen/CMakeFiles/clang-tblgen.dir/NeonEmitter.cpp.obj:(std::__1::basic_ostream<char, std::__1::char_traits<char>>& std::__1::__put_character_sequence[abi:nn200100]<char, std::__1::char_traits<char>>(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, char const*, unsigned int))
>>> referenced by tools/clang/utils/TableGen/CMakeFiles/clang-tblgen.dir/NeonEmitter.cpp.obj:((anonymous namespace)::Intrinsic::emitReverseVariable((anonymous namespace)::Variable&, (anonymous namespace)::Variable&))

ld.lld: error: undefined symbol: std::__1::basic_ostream<char, std::__1::char_traits<char>>::sentry::~sentry()
>>> referenced by tools/clang/utils/TableGen/CMakeFiles/clang-tblgen.dir/NeonEmitter.cpp.obj:(std::__1::basic_ostream<char, std::__1::char_traits<char>>& std::__1::__put_character_sequence[abi:nn200100]<char, std::__1::char_traits<char>>(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, char const*, unsigned int))
>>> referenced by tools/clang/utils/TableGen/CMakeFiles/clang-tblgen.dir/NeonEmitter.cpp.obj:((anonymous namespace)::Intrinsic::emitReverseVariable((anonymous namespace)::Variable&, (anonymous namespace)::Variable&))

so we need to fix symbol visibility annotation in libc++ prior to patch Clang.

Effects

What attaching _LIBCPP_HIDE_FROM_ABI_MINGW_OR_AFTER_V1 to ostream::sentry::sentrys does:

• in MinGW environment:
  Expanded to _LIBCPP_HIDE_FROM_ABI.
  • While building a DLL:
    Virtually no-op while Clang and MinGW-GCC doesn't export them from a past.
  • Using a DLL from client-code:
    Forces instantiate in client code and prohibits trying to import from DLL.
    This is same to what former Clang does and gains compatibility with patched Clang and MinGW-GCC.
• for all other platforms including MSVC:
  Expanded to _LIBCPP_HIDE_FROM_ABI_V1. Keeps V1 ABI stable.

Thus, this change introduces no significant side-effects except for need to add inline together.

Other options that were not adopted

1. Modify GCC's behavior:
   @mstorsjo reported to GCC as defect over 5 years ago but there is no response yet: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89087 .
   There seems to be no chance of fixing it.

2. Attaching _LIBCPP_EXPORTED_FROM_ABI like:

   --- a/libcxx/include/__ostream/basic_ostream.h
   +++ b/libcxx/include/__ostream/basic_ostream.h
   @@ -71,7 +71,7 @@ protected:
   
    public:
      // 27.7.2.4 Prefix/suffix:
   -  class sentry;
   +  class _LIBCPP_EXPORTED_FROM_ABI sentry;
   
      // 27.7.2.6 Formatted output:
      inline _LIBCPP_HIDE_FROM_ABI_AFTER_V1 basic_ostream& operator<<(basic_ostream& (*__pf)(basic_ostream&)) {
   

   This is simply wrong. Breaks ABI and doesn't work with template argument other than char or wchar_t.

3. Declaring ostream::sentrys with __attribute__((exclude_from_explicit_instantiation)) if __MINGW32__ and when client-side with a new keyword like (empty if in other conditions):

   --- a/libcxx/include/__config
   +++ b/libcxx/include/__config
   @@ -365,6 +365,11 @@ typedef __char32_t char32_t;
    #      define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS
    #      define _LIBCPP_OVERRIDABLE_FUNC_VIS
    #      define _LIBCPP_EXPORTED_FROM_ABI
   +#      if !__has_attribute(exclude_from_explicit_instantiation) || defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
   +#        define _LIBCPP_INNER_CLASS_IN_TEMPLATE_VIS
   +#      else
   +#        define _LIBCPP_INNER_CLASS_IN_TEMPLATE_VIS __attribute__((__exclude_from_explicit_instantiation__))
   +#      endif
    #    elif defined(_LIBCPP_BUILDING_LIBRARY)
    #      if defined(__MINGW32__)
    #        define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS __declspec(dllexport)
   

   and

   --- a/libcxx/include/__ostream/basic_ostream.h
   +++ b/libcxx/include/__ostream/basic_ostream.h
   @@ -72,7 +72,7 @@ protected:
   
    public:
      // 27.7.2.4 Prefix/suffix:
   -  class sentry;
   +  class _LIBCPP_INNER_CLASS_IN_TEMPLATE_VIS sentry;
   
      // 27.7.2.6 Formatted output:
      inline _LIBCPP_HIDE_FROM_ABI_AFTER_V1 basic_ostream& operator<<(basic_ostream& (*__pf)(basic_ostream&)) {
   

   This works well but leaves GCC incompatible, so this idea is not perfect.

4. Instantiate inner class explicitly

   #  if defined(__MINGW32__) || defined(__CYGWIN__)
   extern template _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS basic_ostream<char>::sentry::sentry(basic_ostream<char>& __os);
   extern template _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS basic_ostream<char>::sentry::~sentry();
   #  endif
   

   This works for both of Clang and GCC but scattering # if blocks is not a good style.

5. Similar to this proposal but expand to nothing for non-MinGW platforms like this:

   #  if defined(__MINGW32__) || defined(__CYGWIN__)
   #    define _LIBCPP_HIDE_FROM_ABI_IF_MINGW inline _LIBCPP_HIDE_FROM_ABI
   #  else
   #    define _LIBCPP_HIDE_FROM_ABI_IF_MINGW
   #  endif
   

   This works fine. If keeping non-inline is preferred to keeping consistency of presence or absence of inline between platforms, this is an option.

Conclusion

Due to incompatible behavior on MinGW platform, Clang needs to be modified. But patching Clang breaks libc++ so adjusting visibility of some symbols is required. Any keyword already exist can't be suitable so we're going to introduce a new keyword named _LIBCPP_HIDE_FROM_ABI_MINGW_OR_AFTER_V1.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[mstorsjo]

In MinGW environment, Clang handles dllexport attribute of internal class that defined in class template in different way from GCC. This incompatibility should be fixed but breaks ABI of libc++

I didn't quite see this bit answered here; in which way would that break the ABI, if we'd fix this incompatibility? If we'd make libc++.dll export more symbols than we did before, that wouldn't break anything for preexisting callers of the DLL. New binaries built would obviously require the new libc++.dll though, but that's generally the rule anyway.

But MinGW-GCC handles template instantiation differently:

• When exporting: extern template __declspec(dllexport) class TheTemplateClass<T>;
  allows exporting the outer template instantiation, but not its nested types (e.g., InnerClass).
• When importing: extern template __declspec(dllimport) class TheTemplateClass<T>;
  causes MinGW-GCC to also try importing nested types such as TheTemplateClass::InnerClass,
  even if they were never exported. This leads to linker errors like: undefined reference to TheTemplateClass<T>::InnerClass::...

This is indeed self-inconsistent, and is the issue that I reported at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89087. Now as libstdc++ doesn't use explicit dllexports, but just uses the default fallback behaviour of exporting all symbols, they're not really hit by this issue, so there's not much pressure to fix it on their side. (We could also do the same in libc++, by explicitly passing -Wl,--export-all-symbols when building libc++.dll.)

Can you elaborate on what would break if we'd try to fix this on the Clang side (making Clang do similarly to what GCC does, but self-consistently by making it export the nested class, like the dllimport behaviour seems to expect)?

[kikairoya]

I didn't quite see this bit answered here; in which way would that break the ABI, if we'd fix this incompatibility? If we'd make libc++.dll export more symbols than we did before, that wouldn't break anything for preexisting callers of the DLL. New binaries built would obviously require the new libc++.dll though, but that's generally the rule anyway.

I agree that exporting additional symbols wouldn't break existing binaries at the symbol usage level. However, I guess, from user's perspective, needing to upgrade the DLL might be treated as ABI change. Saying it "breaks ABI" may have been too strong but not completely wrong, as seeing manner of ELF world, exporting new symbols will cause change SOVERSION.

Can you elaborate on what would break if we'd try to fix this on the Clang side (making Clang do similarly to what GCC does, but self-consistently by making it export the nested class, like the dllimport behaviour seems to expect)?

I think that makes a new 'dialect' to be avoided (but I understand should not to emulate GCC's defect, too). As real harm, reverse of #135910 might occur but I don't have any test. Need a time to clarify that harms or not.

Regardless to approach of fixing Clang incompatibility with MinGW-GCC, this new keyword would be viable to able to work libc++ with MinGW-GCC.

[jeremyd2019]

In MinGW environment, Clang handles dllexport attribute of internal class that defined in class template in different way from GCC. This incompatibility should be fixed but breaks ABI of libc++, so we need to introduce the new keyword to keep ABI in MinGW environment with old and patched Clang and to stay ABI compatible on other platforms.

I believe we established that dllexport and dllimport are handled the same between Clang and GCC, and it is the more mundane extern that is handled differently.