Windows: Support building stage3, and bootstrapping via MSVC

[kcbanner]

These are changes to support bootstrapping zig on Windows using MSVC. This closes #12703

The corresponding bootstrap script is here: ziglang/zig-bootstrap#140

Summary of changes:

• Place the result of llvm-config --system-libs into it's own list, so it can be handled separately. On windows, the list of libraries is returned like kernel32.lib uuid.lib instead of -lkernel32 -luuid. This would cause addCMakeLibraryList to link them as object files, which would produce incorrect paths. This list now makes it's way to addCMakeSystemLibraryList which handles this case correctly.
• Do a similar approach for clang's system libraries (which is just version.lib)
• Place the zig1/zig2 binaries into the build dir, and not a Release/Debug subdirectory, so they can be found by subsequent steps
• Add ZIG_ENABLE_ZSTD: This controls whether zstd is linked in build.zig. This is done via a new argument, -Ddisable-zstd. This is to support building zig when it links against and MSVC-built LLVM built with LLVM_ENABLE_ZSTD=OFF.
• Fixup zlib path (used during bootstrapping)
• Don't try to link libcxx when being built for msvc - this was bringing in libcxxabi which does not build under msvc due to several conflicts (and I'm not sure this is even supported) see: Unable to build stage3 on Windows: FileNotFound followed by Unable to dump stack trace: InvalidPEMagic #12703 (comment)
• If a .def file is present in the zig cc/c++ argument list, pass it through to lld-link as -DEF:<filename>. This is to handle this case: https://reviews.llvm.org/D134165. Specifically, building libLTO.dll during bootstrapping.

I'm opening this as a draft, as I'm not not sure about one of the changes in lld.zig, specifically:

            if (target.abi == .msvc) {
                argv.appendAssumeCapacity(lib_basename);
                continue;
            }

This change was to support linking system libraries without needing to build the .lib for them (during the first zig build with msvc) however I'm not sure if this should take precedence over the if (comp.crt_files.get(lib_basename)) |crt_file| { branch.

[kubkon]

Why don't we want to link against libc++ when targeting .msvc ABI? Is this circumventing Zig wanting to use its own-built libc++.a when linkLibCpp is specified?

[kcbanner]

This was to handle this problem: #12703 (comment)

I tried just not building/linking libcxxabi at first, which gets you as far as linking (diffs I tried in the comment there).

The problem is that libc++ once built/linked under the msvc-built zig, conflicts with libcmt.lib (which comes from the msvc-built llvm that links with it):

lld-link : error : duplicate symbol: void * __cdecl operator new(unsigned __int64) [C:\cygwin64\home\kcbanner\kit\zig\build-release\stage3.vcxproj]
  >>> defined at d:\a01\_work\12\s\src\vctools\crt\vcstartup\src\heap\new_scalar.cpp:32
  >>>            libcmt.lib(new_scalar.obj)
  >>> defined at C:\cygwin64\home\kcbanner\kit\zig\lib\libcxx\src/new.cpp
  >>>            libc++.lib(new.obj)

[kcbanner]

Actually, I think I was too hasty here. This might only be necessary during bootstrap - when we're linking against an msvc-built llvm (and not a zig-built one).

I'm going to attempt bootstrapping with this check in place, then removing it when building self-hosted. It might be that the solution is a build arg like -Duse-native-libcxx to gate this behavior might be the real solution.

[kcbanner]

Updated this flow to use a new -Ddisable-libcpp flag passed in by the bootstrapping script when building on a *-windows-msvc host.

[kubkon]

ls zstd not supported when targeting windows-msvc?

[kcbanner]

I just checked this again, and I think removing this conditionally was actually unnecessary. The key part of the zstd change was adding the ZIG_ENABLE_ZSTD flag to the cmake script - since the initial LLVM built during bootstrapping on the host is build without ZSTD enabled (since we haven't built it for the host yet), we don't want to require it during the build:

if(ZIG_STATIC_ZSTD AND ZIG_ENABLE_ZSTD)
    list(REMOVE_ITEM LLVM_SYSTEM_LIBRARIES "-lzstd")
    find_library(ZSTD NAMES libzstd.a libzstdstatic.a zstd NAMES_PER_DIR)
    list(APPEND LLVM_LIBRARIES "${ZSTD}")
endif()

Will remove -Ddisable-zstd and the change to this function.

[kubkon]

What is the difference between LLVM_LIBRARIES and LLVM_SYSTEM_LIBRARIES? Will this change not potentially break other targets linking against system version of LLVM such as macOS?

[kubkon]

It is possible to link statically against a system provided LLVM e.g., Homebrew provided LLVM on macOS, in which case, I am wondering if this change doesn't break it as now we are removing -lz from LLVM_SYSTEM_LIBRARIES rather than LLVM_LIBRARIES, assuming they are not the same.

[kcbanner]

I added LLVM_SYSTEM_LIBRARIES to hold the result of llvm-config.exe --system-libs, instead of adding it to LLVM_LIBRARIES. The reason for breaking it out into it's own variable was to handle this difference:

e:\dev\zig-bootstrap\out-win\host\bin>llvm-config.exe --system-libs
psapi.lib shell32.lib ole32.lib uuid.lib advapi32.lib z.lib

$ ./llvm-config.exe  --system-libs
-lpsapi -lshell32 -lole32 -luuid -ladvapi32 -lz

The problem was that addCMakeLibraryList requires a -l prefix in order to pass the library to exe.linkSystemLibrary instead of exe.addObjectFile. There is no -l prefix in the msvc --system-libs case. What was happening was ie. uuid.lib would be passed to exe.addObjectFile which would generate a path like c:\path\to\zig\uuid.lib when linking, which would fail.

For comparison the --libfiles result on msvc vs msys2/mingw:

e:\dev\zig-bootstrap\out-win\host\bin>llvm-config.exe --libfiles
e:\dev\zig-bootstrap\out-win\host\lib\LLVMWindowsManifest.lib e:\dev\zig-bootstrap\out-win\host\lib\LLVMWindowsDriver.lib ... snip

$ ./llvm-config.exe  --libfiles
E:/dev/zig-bootstrap/out/build-llvm-host/lib/libLLVMWindowsManifest.a E:/dev/zig-bootstrap/out/build-llvm-host/lib/libLLVMWindowsDriver.a ... snip

In this case, you do want to pass the full lib path, so I couldn't just check for a .lib extension to make the distinction.

[kcbanner]

To answer your second question, I believe -lz should be in LLVM_SYSTEM_LIBARIES in the ZIG_STATIC_LLVM case on macOS, but I don't have access to a macOS system to check if -lz should up in the llvm-config --libfiles result or the llvm-config --system-libs result.

[kcbanner]

Updated:

• Remove -Ddisable-zstd, no longer needed
• Add -Ddisable-libcpp for use when bootstrapping on an msvc host
• Fix -ENTRY being passed twice if it was specified explicitly and -nostdlib was present.
• Add support for /pdb, /version, /implib, and /subsystem as linker args (passed by CMake): https://github.com/Kitware/CMake/blob/master/Modules/Platform/Windows-Clang.cmake#L77
• If nostdlib is passed, also set ensure_libcpp_on_non_freestanding = false. Previously, using zig c++ could result in a scenario where libc wasn't linked but libcpp was.

I also investigated the ability to build an *-windows-msvc target, and as part of that required adding support for the way that CMake calls us in this case. Specifically, the Windows-Clang.cmake platform code uses some of the link.exe style arguments. However, it doesn't seem possible to fully bootstrap this target yet due to the following issue:

The cmake platform code for Windows-Clang (which is selected when it detects your using Clang with an MSVC target) passes -Xclang --dependent-lib=libcmt to the compile step when it builds all it's test programs to check for compiler options. This causes clang to put directives in the .o file to tell the linker what libs to link. But, it then passes -nostartfiles -nostdlib to the linker ling (which is zig cc here). With the current mapping in zig of -nostdlib resulting in -NODEFAULTLIB being sent to lld-link, this means these directives get ignored. This means that for all the cmake compile programs that use say, main as an entry point, will fail to compile. I tried circumvented this by passing /ENTRY:main to lld-link instead if this is detected, but then you run into cases like llvm's cmake build checking for the compress2 symbol from zlib. It does this by compiling and linking a test program links z.lib, which requires malloc/free from libcmt.lib. Since -NODEFAULTLIB is passed, the directives in the test program's .o aren't followed and these test programs all fail with linker errors.

So, building -msvc isn't possible yet - but it sounds like that once we stop using lld-link, that we could support this case with our own COFF linker. With that being said, some of the changes I made during this investigation might be useful that point, so I included them.

[kubkon]

Would you mind rebasing and fixing conflicts?

[kcbanner]

Will do, however I'm going to convert this back to a draft until I resolve the CBE issues with MSVC. This was ready to go until those changes landed.

[andrewrk]

Place the result of llvm-config --system-libs into it's own list, so it can be handled separately. On windows, the list of libraries is returned like kernel32.lib uuid.lib instead of -lkernel32 -luuid. This would cause addCMakeLibraryList to link them as object files, which would produce incorrect paths. This list now makes it's way to addCMakeSystemLibraryList which handles this case correctly.

Wouldn't the same possibilities also affect the main llvm/clang/lld libraries? For example, LLVMCore.lib rather than -lLLVMCore.

It seems like rather than changing all this stuff, the logic inside parseConfigH and/or addCMakeLibraryList could be improved to handle both -lfoo and foo.lib.

Actually, I'm not sure what the problem is with exe.addObjectFile("foo.lib"). I don't see this new addCMakeSystemLibraryList function doing anything new.

[andrewrk]

I'd like to avoid adding these if possible. It looks like ZIG_ENABLE_LIBCPP is unused and can simply be deleted.

As for ZIG_ENABLE_ZSTD, it will confuse package maintainers and users into believing that Zig depends on zstd, which is not the case. In reality, it is a transitive dependency that we only have to care about when LLVM is built statically, and is built with zstd enabled. In such case, the user must pass -DZIG_STATIC_ZSTD=ON. In all other cases, the user can leave the defaults as-is.

I don't see the purpose of an additional option.

[kcbanner]

The purpose of ZIG_ENABLE_ZSTD was to get around ZIG_STATIC enabling ZIG_STATIC_ZSTD, which would cause a find_library search, which would then fail in cases where it didn't exist (like building zig for the host during bootstrapping).

I changed the way that the defaults were set to allow ZIG_STATIC_ZSTD to be set explicitly off when ZIG_STATIC is enabled. The build.bat script will set this to OFF explicitly when building for the host.

[kcbanner]

Wouldn't the same possibilities also affect the main llvm/clang/lld libraries? For example, LLVMCore.lib rather than -lLLVMCore.

It seems like rather than changing all this stuff, the logic inside parseConfigH and/or addCMakeLibraryList could be improved to handle both -lfoo and foo.lib.

Actually, I'm not sure what the problem is with exe.addObjectFile("foo.lib"). I don't see this new addCMakeSystemLibraryList function doing anything new.

The problem was that libraries such as ntdll.lib (as returned by llvm-config --system-libs) being passed to exe.addObjectFile would result in the linker being passed the incorrect absolute path c:\cygwin64\home\kcbanner\kit\zig\ntdll.lib, for example. The --libs result from llvm-config already provided absolute paths to the .lib, so this wasn't an issue there.

I could instead have addCMakeLibraryList assume a lib name like ntdll.lib with no absolute path prefix is a system lib and call linkSystemLibrary instead. Then I could remove the *_SYSTEM_LIBRARIES lists that I added and use a single list for everything.

[andrewrk]

Really excellent work. The only problem I see is a couple missing cache hash lines (potentially would cause a subsequent compilation that added these new linker args to think it did not need to re-run the linker).

With those added, this is ready to land.

[kcbanner]

Thanks! Hash inputs have been added.