Explicitly specify MSVC mangling of iostream globals

[kastiglione]

Summary

With MS ABI, the char [] definitions of the iostream globals (iostream.cpp)
result in mangled symbol names that don't match the mangled symbol names of the
declarations (<iostream>).

For example, istream cin mangles to:

?cin@__1@std@@3V?$basic_istream@DU?$char_traits@D@__1@std@@@12@A

but its definition in iostream.cpp is char cin[sizeof(istream)], which
incorrectly mangles to:

?cin@__1@std@@3PADA

To work aound this issue, this change manually sets the symbol name using an
__asm__ label, for win32 only.

[majnemer]

I don't think _WIN32 is the right guard because mingw defines that macro and it uses the Itanium mangling.

I'd use something like _MSC_VER.

If you want this to work with MSVC (instead of just clang-cl), you could use #pragma comment(linker, "/alternatename:foo=bar") to achieve similar results.

[kastiglione]

Thanks @majnemer, I'll update accordingly.

[compnerd]

What is the state of this ptach? @majnemer had a valid point, this should be updated with that.

[kastiglione]

Change #ifdefs from _WIN32 to _MSC_VER

[kastiglione]

@majnemer, @compnerd: Using /alternatename will require the mangling of the definition be maintained in addition to the mangling of the declaration. I myself don't need MSVC compatibility for this, so if there are no proponents for a MSVC compatible change at this point, I'm inclined to stay with an __asm__ approach since /alternatename potentially makes this fix more fragile.

[EricWF]

In D23378#512640, @majnemer wrote:

I don't think _WIN32 is the right guard because mingw defines that macro and it uses the Itanium mangling.

I'd use something like _MSC_VER.

Libc++ now has macros for the object format it's targeting. I think these changes should be guarded with #if defined(_LIBCPP_OBJECT_FORMAT_COFF) now.

src/iostream.cpp

18–19

Please guard these changes with #if defined(_LIBCPP_OBJECT_FORMAT_COFF) instead.

[EricWF]

LGTM minus inline coments.

@majnemer suggested another solution that would work with both MSVC and clang-cl but Libc++ is far from being able to support MSVC (due to #include_next), so this fix is good for now.

Unfortunately this is going to become more difficult to maintain once libc++ starts using different versioning namespaces. However we can cross that bridge when we come to it.

[smeenai]

src/iostream.cpp

18–19

I don't think that'll be correct, since it's an ABI issue (Itanium vs Microsoft) not an object format issue. _MSC_VER isn't great either though, because for example you can compile with -target i686-windows-itanium -fmsc-version-1900 and still end up with _MSC_VER defined even though you're targeting the Itanium ABI. Idk how to check for the ABI specifically.

[EricWF]

src/iostream.cpp

18–19

Ah thanks for the correct. I added this was a limitation of the object format.

[compnerd]

src/iostream.cpp

18–19

A similar problem exists with the MS ABI RTTI implementation.

I think it may be high time to introduce a _LIBCPP_ABI_IA and _LIBCPP_ABI_MS. Thoughts?

[EricWF]

src/iostream.cpp

18–19

That makes sense to me. How do we detect what ABI we are targeting?

[kastiglione]

Switch from _MSC_VER to _LIBCPP_MSVC

[smeenai]

src/iostream.cpp

18

_LIBCPP_MSVC is definitely not correct. It's only defined for cl, not for clang (not even clang-cl), and cl doesn't even support __asm__.

[kastiglione]

src/iostream.cpp

18

Thanks, faulty misinterpretation of the macro logic.

[kastiglione]

Revert to checking _MSC_VER

[EricWF]

This LGTM again. We're working on getting special builtin macros to detect which ABI is being used, but until then this patch seems correct.

@smeenai mentioned that cl doesn't support __asm__ so technically this patch is incorrect since cl defines _MSC_VER. However libc++ cannot use cl currently due to usages of other constructs it doesn't support, such as #include_next, so I'm not worried about that now.

@kastiglione do you have commit access?

[kastiglione]

@EricWF I don't have commit access. Would you mind committing?

cl doesn't support __asm__ so technically this patch is incorrect since cl defines _MSC_VER.

Is it worth adding && defined(__clang__)?

[EricWF]

In D23378#638185, @kastiglione wrote:

@EricWF I don't have commit access. Would you mind committing?

cl doesn't support __asm__ so technically this patch is incorrect since cl defines _MSC_VER.

Is it worth adding && defined(__clang__)?

Up to you. Once we update clang to provide builtin macros for ABI detection we'll have to change all of these guards anyway.

[kastiglione]

Check for clang too

[kastiglione]

@EricWF I added the __clang__ check, why not. Thanks for the review.

[EricWF]

@kastiglione do you need somebody to commit this for you?

[kastiglione]

@EricWF I do need someone to commit this, please and thanks

[EricWF]

r291337.

src/iostream.cpp

 //===------------------------ iostream.cpp --------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "__std_stream"
 #include "string"
 #include "new"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #ifndef _LIBCPP_HAS_NO_STDIN
-_ALIGNAS_TYPE (istream)  _LIBCPP_FUNC_VIS char cin [sizeof(istream)];
-_ALIGNAS_TYPE (__stdinbuf<char> ) static char __cin [sizeof(__stdinbuf <char>)];
+_ALIGNAS_TYPE (istream) _LIBCPP_FUNC_VIS char cin[sizeof(istream)]
+#if defined(_MSC_VER)
+__asm__("?cin@__1@std@@3V?$basic_istream@DU?$char_traits@D@__1@std@@@12@A")
+#endif
+;
+_ALIGNAS_TYPE (__stdinbuf<char> ) static char __cin[sizeof(__stdinbuf <char>)];
 static mbstate_t mb_cin;
-_ALIGNAS_TYPE (wistream) _LIBCPP_FUNC_VIS char wcin [sizeof(wistream)];
-_ALIGNAS_TYPE (__stdinbuf<wchar_t> ) static char __wcin [sizeof(__stdinbuf <wchar_t>)];
+_ALIGNAS_TYPE (wistream) _LIBCPP_FUNC_VIS char wcin[sizeof(wistream)]
+#if defined(_MSC_VER)
+__asm__("?wcin@__1@std@@3V?$basic_istream@_WU?$char_traits@_W@__1@std@@@12@A")
+#endif
+;
+_ALIGNAS_TYPE (__stdinbuf<wchar_t> ) static char __wcin[sizeof(__stdinbuf <wchar_t>)];
 static mbstate_t mb_wcin;
 #endif
 
 #ifndef _LIBCPP_HAS_NO_STDOUT
-_ALIGNAS_TYPE (ostream)  _LIBCPP_FUNC_VIS char cout[sizeof(ostream)];
+_ALIGNAS_TYPE (ostream) _LIBCPP_FUNC_VIS char cout[sizeof(ostream)]
+#if defined(_MSC_VER)
+__asm__("?cout@__1@std@@3V?$basic_ostream@DU?$char_traits@D@__1@std@@@12@A")
+#endif
+;
 _ALIGNAS_TYPE (__stdoutbuf<char>) static char __cout[sizeof(__stdoutbuf<char>)];
 static mbstate_t mb_cout;
-_ALIGNAS_TYPE (wostream) _LIBCPP_FUNC_VIS char wcout[sizeof(wostream)];
+_ALIGNAS_TYPE (wostream) _LIBCPP_FUNC_VIS char wcout[sizeof(wostream)]
+#if defined(_MSC_VER)
+__asm__("?wcout@__1@std@@3V?$basic_ostream@_WU?$char_traits@_W@__1@std@@@12@A")
+#endif
+;
 _ALIGNAS_TYPE (__stdoutbuf<wchar_t>) static char __wcout[sizeof(__stdoutbuf<wchar_t>)];
 static mbstate_t mb_wcout;
 #endif
 
-_ALIGNAS_TYPE (ostream)  _LIBCPP_FUNC_VIS char cerr[sizeof(ostream)];
+_ALIGNAS_TYPE (ostream) _LIBCPP_FUNC_VIS char cerr[sizeof(ostream)]
+#if defined(_MSC_VER)
+__asm__("?cerr@__1@std@@3V?$basic_ostream@DU?$char_traits@D@__1@std@@@12@A")
+#endif
+;
 _ALIGNAS_TYPE (__stdoutbuf<char>) static char __cerr[sizeof(__stdoutbuf<char>)];
 static mbstate_t mb_cerr;
-_ALIGNAS_TYPE (wostream) _LIBCPP_FUNC_VIS char wcerr[sizeof(wostream)];
+_ALIGNAS_TYPE (wostream) _LIBCPP_FUNC_VIS char wcerr[sizeof(wostream)]
+#if defined(_MSC_VER)
+__asm__("?wcerr@__1@std@@3V?$basic_ostream@_WU?$char_traits@_W@__1@std@@@12@A")
+#endif
+;
 _ALIGNAS_TYPE (__stdoutbuf<wchar_t>) static char __wcerr[sizeof(__stdoutbuf<wchar_t>)];
 static mbstate_t mb_wcerr;
 
-_ALIGNAS_TYPE (ostream)  _LIBCPP_FUNC_VIS char clog[sizeof(ostream)];
-_ALIGNAS_TYPE (wostream) _LIBCPP_FUNC_VIS char wclog[sizeof(wostream)];
+_ALIGNAS_TYPE (ostream) _LIBCPP_FUNC_VIS char clog[sizeof(ostream)]
+#if defined(_MSC_VER)
+__asm__("?clog@__1@std@@3V?$basic_ostream@DU?$char_traits@D@__1@std@@@12@A")
+#endif
+;
+_ALIGNAS_TYPE (wostream) _LIBCPP_FUNC_VIS char wclog[sizeof(wostream)]
+#if defined(_MSC_VER)
+__asm__("?wclog@__1@std@@3V?$basic_ostream@_WU?$char_traits@_W@__1@std@@@12@A")
+#endif
+;
 
 ios_base::Init __start_std_streams;
 
 ios_base::Init::Init()
 {
 #ifndef _LIBCPP_HAS_NO_STDIN
     istream* cin_ptr  = ::new(cin)  istream(::new(__cin)  __stdinbuf <char>(stdin, &mb_cin));
     wistream* wcin_ptr  = ::new(wcin)  wistream(::new(__wcin)  __stdinbuf <wchar_t>(stdin, &mb_wcin));