[PATCH] D28441: [libc++] [CMake] Link with /nodefaultlibs on Windows

Fri Jan 13 18:33:52 PST 2017

EricWF added a comment.

In https://reviews.llvm.org/D28441#639023, @smeenai wrote:

> One potential issue with always forcing the non-debug msvcrt is that any apps that link against libc++ would also then need to use the non-debug CRT libraries, otherwise you'd get all sorts of fun runtime errors (e.g. cross-domain frees). I think the default Visual Studio settings are to link against `msvcrtd`, `ucrtd`, etc. for debug builds and `msvcrt`, `ucrt`, etc. for release builds, so having libc++ unconditionally use the non-debug versions is probably bad for interoperability.

I agree. Forcing the use of release libraries is just a starting point.

> I think the right thing to do would be to either always compile two versions of libc++, one linked against the debug libraries and the other against the non-debug libraries, or make the Debug configuration use the debug libraries and the Release configuration use the release libraries (and have `config.py` deal with this as well).

Is it OK if this is implemented *after* this patch?

================
Comment at: CMakeLists.txt:43
+if (WIN32 AND NOT MINGW)
+  set(LIBCXX_TARGETING_WINDOWS ON)
+else()
----------------
rnk wrote:
> smeenai wrote:
> > EricWF wrote:
> > > EricWF wrote:
> > > > smeenai wrote:
> > > > > Not the biggest fan of this name, since it's not obvious why MinGW shouldn't count as targeting Windows. I thought of `LIBCXX_TARGETING_NATIVE_WINDOWS` or `LIBCXX_TARGETING_MSVCRT` instead, but MinGW is also native Windows and targets MSVCRT, so those names aren't any better from that perspective either. I can't think of anything else at the moment, but I'm sure there's a better name.
> > > > Thanks for the feedback. I'm not exactly sure how this macro should be defined either.
> > > > 
> > > > I thought that `MinGW` provided its own C library runtime wrappers that forward to `MSVCRT`. 
> > > > 
> > > > The difference I was imagining between Native Windows builds and `MinGW` is that it's possible to use
> > > > `pthread` with `MinGW` but not with native Windows targets. 
> > > Another distinction I would like this macro to embody is whether on not the compiler defines `_MSC_VER`.  `clang-cl`, `clang++`  on Windows, and MSVC `cl` all define this macro.
> > If I recall correctly, MinGW uses `mscvrt.dll` but not `msvcrt.lib` (see my other comment for the distinction). I'm fine with this name for now then; we can always bikeshed later.
> > 
> > Btw it's also possible to use pthread with native Windows targets, via [pthreads-win32](http://www.sourceware.org/pthreads-win32/) or other libraries.
> I'd call this LIBCXX_TARGETING_MSVCRT. "msvcrt.dll" usually refers to an ancient version (6?) of the Visual C runtime that is distributed as part of Windows. Typically it is found at C:/Windows/system32/msvcrt.dll. Mingw uses this, because it is available on all Windows systems they care to support. This DLL basically never receives updates, so I wouldn't want to build libc++ functionality on top of it. Over time, it seems that mingw has had to implement more and more C runtime functionality itself, and I wouldn't want libc++ to have to do that.
> 
> Until recently, MSVC users were required to redistribute the version of the CRT they chose to link against, and it was expected that all DLLs sharing CRT resources had to link against the same CRT version. Of course, this caused problems, so they went back to the single, OS-provided CRT in VS 2015 (https://blogs.msdn.microsoft.com/vcblog/2015/03/03/introducing-the-universal-crt/).
> 
> My conclusion is that it's no longer worth thinking of that old DLL as the Visual C runtime. It's just an artifact of history at this point. If you say libc++ targets the MSVCRT, you're probably talking about the modern, universal CRT.
> 
> ---
> 
> If you want this option to imply both the C++ ABI and the C runtime, then LIBCXX_TARGETING_MSVC would probably be a more appropriate name. Clang and LLVM have some support for using the Itanium C++ ABI with the MSVCRT, but it is more experimental, and not ABI compatible with any existing software. Saleem could say more about where he thinks this target will go in the future and whether it's worth adding to the libc++ support configuration matrix.
> My conclusion is that it's no longer worth thinking of that old DLL as the Visual C runtime. It's just an artifact of history at this point. If you say libc++ targets the MSVCRT, you're probably talking about the modern, universal CRT.

According to the [docs](https://msdn.microsoft.com/en-us/library/abx4dbyh.aspx) `msvcrt` now contains the "startup files" for the universal CRT.

> If you want this option to imply both the C++ ABI and the C runtime, then LIBCXX_TARGETING_MSVC would probably be a more appropriate name.

Ack. Changing it to `LIBCXX_TARGETING_MSVC`  and enabling it using the `MSVC` CMake variable.

================
Comment at: lib/CMakeLists.txt:112
+  add_library_flags(vcruntime) # C++ runtime
+  add_library_flags(msvcrt) # C runtime startup files
+  add_library_flags(iso_stdio_wide_specifiers)
----------------
smeenai wrote:
> EricWF wrote:
> > halyavin wrote:
> > > halyavin wrote:
> > > > As far as I know, applications shouldn't use msvcrt.dll ([[ https://blogs.msdn.microsoft.com/oldnewthing/20140411-00/?p=1273 | Windows is not a Microsoft Visual C/C++ Run-Time delivery channel ]]) Can we survive on ucrt only?
> > > Oh, it is static library that doesn't correspond to a dll.
> > I don't think that link suggests that applications shouldn't link `msvcrt.dll`. 
> > 
> > Instead all of the doc I see suggests that `msvcrt.dll` is linked implicitly by `/MD`. However since libc++ removes `/MD` and adds `/nodefaultlibs` we need to manually re-create the `/MD` link without the MSVC STL. That involves manually linking `msvcrt.dll`.
> There's a distinction between `msvcrt.lib` and `msvcrt.dll`. `msvcrt.lib` is a static library which contains the entry point (`_DllMainCRTStartup at 12`, etc.). It's basically the equivalent of crtbegin for Windows. `msvcrt.dll` is the unversioned legacy version of the C runtime, which is what you're not supposed to use. It's kinda confusing since the normal convention is for `X.lib` to be the import library corresponding to `X.dll`, but that's not the case for `msvcrt`.
`msvcrt.dll` doesn't exist anymore according to [this](https://msdn.microsoft.com/en-us/library/abx4dbyh.aspx)

https://reviews.llvm.org/D28441