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@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
