[Clang][Driver] Don't pun -fuse-ld=lld as -fuse-ld=lld-link with msvc

[Ericson2314]

Summary

Besides the Mingw toolchain, there is also the CrossWindows toolchain,
which means GNU-style cli but genuine windows headers + libraries. LLD's
MinGW driver is as good a fit as binutil's ld, but there is no easy way
to select it when lld was being rewritten to lld-link.

This makes lld always be the GNU-style one, consistent with the non-msvc
case. It's a small breaking change for Windows, but the only
straightforward way.

It may seem silly to worry about the gnu-style flags, but it is useful
for mixing software designed with MinGW in mind with "real" windows
software that needs a MSVC C++ ABI, C++ threads, etc.

Testing in conjunction with https://github.com/NixOS/nixpkgs/pull/72366,
which might be the first fully automated way to cross compile to
Windows. I haven't taught Nixpkgs to wrap clang-cl like it wraps clang,
so even for some packages that have separate MSVC and MinGW builds
systems (e.g. zlib, opensll), I seem to be having better luck with the
MinGW-style builds so am going with that.

[Ericson2314]

Split out some changes into other diffs. 1 we are stacked on, the other 2 are independent and not stacked on,

[mstorsjo]

I'm not familiar with where/how the CrossWindows toolchain is used, but you say (and the code seems to agree) that it's intended to be able to use ld.bfd for linking code built for the MSVC ABI with MSVC/WinSDK libs? I'm surprised that even works at all (or does it, or is it just set up and tested for minimal cases and then unused?), as ld.bfd lacks support for a lot of COFF specific features that MSVC ABI uses (which probably can be worked around with --allow-multiple-definitions for some bits at least, but I'm not confident that it generally would work even with that).

I presume all of this is about how to call the linker, as compiling with clang -target <arch>-windows-msvc, using the MSVC toolchain, should work just fine?

I think a better distinguisher would be whether clang was invoked as clang[++] or clang-cl (i.e. driver mode). Normally when linking MSVC style, the build system would call link (or the link-like tool) with the linker flags directly. Occasionally one could be linking via the cl frontend, as cl obj1.obj obj2.obj -Feout.exe somelib.lib -link -linkpath:some/path -other_link_style_option.

Now when doing linking via the clang GCC-style driver, clang obj1.obj obj2.obj -o out.exe -lsomelib -Lsome/path -Wl,--other-unix-linker-option, one expects to use unix linking style flags. Hypothesis: There's no predecent for actually using an MSVC link-style linker via this syntax. Or is there?

If there isn't, we could make that the distinguishing factor instead.

The same issue has come up for build systems (meson tried to tackle this recently, don't remember how/if they did yet), when just invoking "clang" on a windows platform (instead of intentionally e.g. "clang-cl"), not knowing whether the implicit default triple is an msvc or mingw triple, which means they don't know in what form to pass linker options. For cross cases where there's an explicit -target option, this are slightly clearer (at least from some perspectives), but with the default target, you're really working blindly.

[rnk]

I think a better distinguisher would be whether clang was invoked as clang[++] or clang-cl (i.e. driver mode).

I think Martin said most of what I wanted to say. Making this dependent on the driver mode seems reasonable and would be the easier way forward.

If you still want to make clang-cl -fuse-ld=lld call ld.lld with gnu-style flags, you will need to pre-fix the various in-tree test suites. If you look through the LLDB, compiler-rt, and debuginfo-tests test suites, you will see many instances of -fuse-ld=lld that rely on the current behavior. See cases like:
https://github.com/llvm/llvm-project/blob/dd0c00b5f8b303f78565ab407d37a643e99a9e75/compiler-rt/test/asan/lit.cfg.py#L96
Maybe that case in particular doesn't apply, but it needs to be looked into.

[thakis]

We currently do cross builds of chrome/win on linux, and this breaks that.

As far as I know, no linker other than lld-link is able to write PDB files, so when targeting windows-msvc we definitely shouldn't change the current behavior. I don't have an opinion on windows-mingw.

[thakis]

In D69760#1734524, @thakis wrote:

We currently do cross builds of chrome/win on linux, and this breaks that.

As far as I know, no linker other than lld-link is able to write PDB files, so when targeting windows-msvc we definitely shouldn't change the current behavior. I don't have an opinion on windows-mingw.

Actually, we don't call the linker through the compiler driver when targeting Windows, so this doesn't break our cross build. I manually use clang-cl -fuse-ld=lld often though, so it'd make me type a lot more :)

[hans]

In D69760#1734534, @thakis wrote:

In D69760#1734524, @thakis wrote:

We currently do cross builds of chrome/win on linux, and this breaks that.

As far as I know, no linker other than lld-link is able to write PDB files, so when targeting windows-msvc we definitely shouldn't change the current behavior. I don't have an opinion on windows-mingw.

Actually, we don't call the linker through the compiler driver when targeting Windows, so this doesn't break our cross build. I manually use clang-cl -fuse-ld=lld often though, so it'd make me type a lot more :)

IIUC, with Martin and Reid's suggestion to key this off the driver mode rather than the triple, "clang-cl -fuse-ld=lld" would still work though? I like that approach as well.

[Ericson2314]

While I am fine making -fuse-ld=lld mean "use that lld driver most appropriate for the clang driver", in principle the clang driver and lld driver choices are orthogonal, and I'd want to expose that with something like -fuse-ld=ld.lld to compliment -fuse-ld=lld-link. How does that sound? Should we support -fuse-ld=ld.bfd and -fuse-ld=ld64.lld too?

[Ericson2314]

This comment was removed by Ericson2314.

[Ericson2314]

This comment was removed by Ericson2314.

[mstorsjo]

In D69760#1739044, @Ericson2314 wrote:

While I am fine making -fuse-ld=lld mean "use that lld driver most appropriate for the clang driver", in principle the clang driver and lld driver choices are orthogonal

In principle, but as ld.lld and lld-link uses different syntaxes that only really are handled correctly if passed through from the gcc and cl drivers respectively, I find it mostly theoretical to want to use them in the opposite combination forms. (Additionally, in practice, people using the cl driver in many cases call (lld-)link directly as it requires very little boilerplate options, contrary to unix style environments where one is supposed to pass in paths to crt2.o and crtbegin.o and the builtins library, etc. So if one wants to use a link style linker it's in most cases easiest to just call it directly instead of using the $CC frontend.)

and I'd want to expose that with something like -fuse-ld=ld.lld to compliment -fuse-ld=lld-link.

I don't mind adding these two cases as explicit overrides over the generic logic

How does that sound? Should we support -fuse-ld=ld.bfd and -fuse-ld=ld64.lld too?

I don't see where you'd need those? -fuse-ld=bfd should pick ld.bfd in most cases, right? In which case wouldn't it? The mapping between parameter name to -fuse-ld and system linker naming scheme varies a bit between the platforms, and I'd presume the other existing toolchains in clang does this right as it is right now. Or can you clarify and elaborate in which cases you'd want to use them?

clang/lib/Driver/Driver.cpp

case llvm::Triple::Win32:
         TC = std::make_unique<toolchains::MinGW>(*this, Target, Args);
         break;
       case llvm::Triple::Itanium:
         TC = std::make_unique<toolchains::CrossWindowsToolChain>(*this, Target,
                                                                   Args);
         break;
       case llvm::Triple::MSVC:
       case llvm::Triple::UnknownEnvironment:
-        if (Args.getLastArgValue(options::OPT_fuse_ld_EQ)
-                .startswith_lower("bfd"))
+        auto linkerFlavor = Args.getLastArgValue(options::OPT_fuse_ld_EQ);
+        if (linkerFlavor.startswith_lower("bfd") || linkerFlavor.equals_lower("lld")
           TC = std::make_unique<toolchains::CrossWindowsToolChain>(
               *this, Target, Args);
         else
           TC =
               std::make_unique<toolchains::MSVCToolChain>(*this, Target, Args);
         break;
       }
       break;

clang/lib/Driver/ToolChains/MSVC.cpp

for (const auto &Input : Inputs) {
     // or -L. Render it, even if MSVC doesn't understand it.
     A.renderAsInput(Args, CmdArgs);
   }
 
   TC.addProfileRTLibs(Args, CmdArgs);
 
   std::vector<const char *> Environment;
 
-  // We need to special case some linker paths.  In the case of lld, we need to
-  // translate 'lld' into 'lld-link', and in the case of the regular msvc
+  // We need to special case some linker paths.  In the case of the regular msvc
   // linker, we need to use a special search algorithm.
   llvm::SmallString<128> linkPath;
   StringRef Linker = Args.getLastArgValue(options::OPT_fuse_ld_EQ, "link");
-  if (Linker.equals_lower("lld"))
-    Linker = "lld-link";
 
   if (Linker.equals_lower("link")) {
     // If we're using the MSVC linker, it's not sufficient to just use link
     // from the program PATH, because other environments like GnuWin32 install
     // their own link.exe which may come first.
     linkPath = FindVisualStudioExecutable(TC, "link.exe");
 
     if (!TC.FoundMSVCInstall() && !llvm::sys::fs::can_execute(linkPath)) {
if (TC.getIsVS2017OrNewer() &&
           Environment.push_back(Args.MakeArgString(EnvVar));
         }
         Cursor += EnvVar.size() + 1 /*null-terminator*/;
       }
     }
   SkipSettingEnvironment:;
 #endif
   } else {
-    linkPath = TC.GetProgramPath(Linker.str().c_str());
+    linkPath = TC.GetLinkerPath();
   }
 
   auto LinkCmd = std::make_unique<Command>(
       JA, *this, Args.MakeArgString(linkPath), CmdArgs, Inputs);
   if (!Environment.empty())
     LinkCmd->setEnvironment(Environment);
   C.addCommand(std::move(LinkCmd));
 }

clang/test/Driver/fuse-ld.c

 // RUN:     -target i686-unknown-windows-msvc 2>&1 \
 // RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-LINK
 // CHECK-WINDOWS-MSVC-LINK: "{{.*}}link.exe"
 // CHECK-WINDOWS-MSVC-LINK-SAME: "-out:{{.*}}"
 
 // RUN: %clang %s -### -fuse-ld=lld \
 // RUN:     -target i686-unknown-windows-msvc 2>&1 \
 // RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-LLD
-// CHECK-WINDOWS-MSVC-LLD: "{{.*}}lld-link"
-// CHECK-WINDOWS-MSVC-LLD-SAME: "-out:{{.*}}"
+// CHECK-WINDOWS-MSVC-LLD: "{{.*}}ld.lld"
+// CHECK-WINDOWS-MSVC-LLD-SAME: "-o"
+
+// RUN: %clang-cl %s -### -fuse-ld=lld \
+// RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-LLD
+// CHECK-cl-WINDOWS-MSVC-LLD: "{{.*}}ld.lld"
+// CHECK-cl-WINDOWS-MSVC-LLD-SAME: "-o"
 
 // RUN: %clang %s -### -fuse-ld=lld-link \
 // RUN:     -target i686-unknown-windows-msvc 2>&1 \
 // RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-LLD-LINK
 // CHECK-WINDOWS-MSVC-LLD-LINK: "{{.*}}lld-link"
 // CHECK-WINDOWS-MSVC-LLD-LINK-SAME: "-out:{{.*}}"
 
 // RUN: %clang %s -### -fuse-ld=bfd \
 // RUN:     -target i686-unknown-windows-msvc \
 // RUN:     -B %S/Inputs/Windows/usr/bin 2>&1 \
 // RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-BFD
 // CHECK-WINDOWS-MSVC-BFD: "{{.*}}ld.bfd"
 // CHECK-WINDOWS-MSVC-BFD-SAME: "-o"

clang/test/Driver/windows-cross.c

 
 // CHECK-NOSTARTFILES: armv7-windows-itanium-ld" "--sysroot={{.*}}/Inputs/Windows/ARM/8.1" "-m" "thumb2pe" "-shared" "-Bdynamic" "--enable-auto-image-base" "--entry" "_DllMainCRTStartup" "--allow-multiple-definition" "-o" "shared.dll" "--out-implib" "shared.lib" "{{.*}}.o" "-lmsvcrt" "{{.*[\\/]}}clang_rt.builtins-arm.lib"
 
 // RUN: %clang -### -target armv7-windows-itanium --sysroot %S/Inputs/Windows/ARM/8.1 -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=ld -shared -rtlib=compiler-rt -stdlib=libc++ -nostartfiles -nodefaultlibs -o shared.dll %s 2>&1 \
 // RUN:   | FileCheck %s --check-prefix CHECK-STANDALONE
 
 // CHECK-STANDALONE: armv7-windows-itanium-ld" "--sysroot={{.*}}/Inputs/Windows/ARM/8.1" "-m" "thumb2pe" "-shared" "-Bdynamic" "--enable-auto-image-base" "--entry" "_DllMainCRTStartup" "--allow-multiple-definition" "-o" "shared.dll" "--out-implib" "shared.lib" "{{.*}}.o"
 
-// RUN: %clang -### -target armv7-windows-itanium --sysroot %S/Inputs/Windows/ARM/8.1 -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=lld-link2 -shared -o shared.dll -x c++ %s 2>&1 \
+// RUN: %clang -### -target armv7-windows-itanium --sysroot %S/Inputs/Windows/ARM/8.1 -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=lld-link -shared -o shared.dll -x c++ %s 2>&1 \
 // RUN:    | FileCheck %s --check-prefix CHECK-FUSE-LD
 
-// CHECK-FUSE-LD: "{{.*}}lld-link2"
+// CHECK-FUSE-LD: "{{.*}}lld-link"
 
-// RUN: %clang -### -target armv7-windows-itanium --sysroot %S/Inputs/Windows/ARM/8.1 -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=lld-link2 -shared -o shared.dll -fsanitize=address -x c++ %s 2>&1 \
+// RUN: %clang -### -target armv7-windows-itanium --sysroot %S/Inputs/Windows/ARM/8.1 -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=lld-link -shared -o shared.dll -fsanitize=address -x c++ %s 2>&1 \
 // RUN:    | FileCheck %s --check-prefix CHECK-SANITIZE-ADDRESS
 
 // CHECK-SANITIZE-ADDRESS: "-fsanitize=address"
 // CHECK-SANITIZE-ADDRESS: "{{.*}}clang_rt.asan_dll_thunk-arm.lib"
 
-// RUN: %clang -### -target armv7-windows-itanium --sysroot %S/Inputs/Windows/ARM/8.1 -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=lld-link2 -o test.exe -fsanitize=address -x c++ %s 2>&1 \
+// RUN: %clang -### -target armv7-windows-itanium --sysroot %S/Inputs/Windows/ARM/8.1 -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=lld-link -o test.exe -fsanitize=address -x c++ %s 2>&1 \
 // RUN:    | FileCheck %s --check-prefix CHECK-SANITIZE-ADDRESS-EXE
 
 // CHECK-SANITIZE-ADDRESS-EXE: "-fsanitize=address"
 // CHECK-SANITIZE-ADDRESS-EXE: "{{.*}}clang_rt.asan_dynamic-arm.lib" "{{.*}}clang_rt.asan_dynamic_runtime_thunk-arm.lib" "--undefined" "__asan_seh_interceptor"
 
-// RUN: %clang -### -target i686-windows-itanium -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=lld-link2 -o test.exe -fsanitize=address -x c++ %s 2>&1 \
+// RUN: %clang -### -target i686-windows-itanium -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=lld-link -o test.exe -fsanitize=address -x c++ %s 2>&1 \
 // RUN:    | FileCheck %s --check-prefix CHECK-SANITIZE-ADDRESS-EXE-X86
 
 // CHECK-SANITIZE-ADDRESS-EXE-X86: "-fsanitize=address"
 // CHECK-SANITIZE-ADDRESS-EXE-X86: "{{.*}}clang_rt.asan_dynamic-i386.lib" "{{.*}}clang_rt.asan_dynamic_runtime_thunk-i386.lib" "--undefined" "___asan_seh_interceptor"
 
-// RUN: %clang -### -target armv7-windows-itanium --sysroot %S/Inputs/Windows/ARM/8.1 -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=lld-link2 -shared -o shared.dll -fsanitize=tsan -x c++ %s 2>&1 \
+// RUN: %clang -### -target armv7-windows-itanium --sysroot %S/Inputs/Windows/ARM/8.1 -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=lld-link -shared -o shared.dll -fsanitize=tsan -x c++ %s 2>&1 \
 // RUN:    | FileCheck %s --check-prefix CHECK-SANITIZE-TSAN
 
 // CHECK-SANITIZE-TSAN: error: unsupported argument 'tsan' to option 'fsanitize='
 // CHECK-SANITIZE-TSAN-NOT: "-fsanitize={{.*}}"
 
 // RUN: %clang -### -target armv7-windows-itanium -isystem-after "Windows Kits/10/Include/10.0.10586.0/ucrt" -isystem-after "Windows Kits/10/Include/10.0.10586.0/um" -isystem-after "Windows Kits/10/Include/10.0.10586.0/shared" -c %s -o /dev/null 2>&1 \
 // RUN:     | FileCheck %s --check-prefix CHECK-ISYSTEM-AFTER
 // CHECK-ISYSTEM-AFTER: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"

lld/MinGW/Driver.cpp

if (args.hasArg(OPT_major_os_version, OPT_minor_os_version,
     StringRef minor = minOSVer ? minOSVer->getValue()
                                : minSubSysVer ? minSubSysVer->getValue() : "";
     StringRef sep = minor.empty() ? "" : ".";
     add("-subsystem:" + subSys + "," + major + sep + minor);
   } else if (auto *a = args.getLastArg(OPT_subs)) {
     add("-subsystem:" + StringRef(a->getValue()));
   }
 
+  config->allowMultipleDefinition =
+  if (args.hasFlag(OPT_allow_multiple_definition, OPT_no_allow_multiple_definition, false))
+    add("-force:multiple");
   if (auto *a = args.getLastArg(OPT_out_implib))
     add("-implib:" + StringRef(a->getValue()));
   if (auto *a = args.getLastArg(OPT_stack))
     add("-stack:" + StringRef(a->getValue()));
   if (auto *a = args.getLastArg(OPT_output_def))
     add("-output-def:" + StringRef(a->getValue()));
   if (auto *a = args.getLastArg(OPT_image_base))
     add("-base:" + StringRef(a->getValue()));

lld/MinGW/Options.td

multiclass EqLong<string name, string help> {
   def NAME # _eq: Joined<["--"], name # "=">, Alias<!cast<Separate>(NAME)>,
     HelpText<help>;
 }
 
 def L: JoinedOrSeparate<["-"], "L">, MetaVarName<"<dir>">,
   HelpText<"Add a directory to the library search path">;
 def Bdynamic: F<"Bdynamic">, HelpText<"Link against shared libraries">;
 def Bstatic: F<"Bstatic">, HelpText<"Do not link against shared libraries">;
+
+defm allow_multiple_definition: B<"allow-multiple-definition",
+    "Allow multiple definitions",
+    "Do not allow multiple definitions (default)">;
+
 def dynamicbase: F<"dynamicbase">, HelpText<"Enable ASLR">;
+
 defm entry: Eq<"entry", "Name of entry point symbol">, MetaVarName<"<entry>">;
 def exclude_all_symbols: F<"exclude-all-symbols">,
     HelpText<"Don't automatically export any symbols">;
 def export_all_symbols: F<"export-all-symbols">,
     HelpText<"Export all symbols even if a def file or dllexport attributes are used">;
 def gc_sections: F<"gc-sections">, HelpText<"Remove unused sections">;
 def help: F<"help">, HelpText<"Print option help">;
 def icf: J<"icf=">, HelpText<"Identical code folding">;

lld/test/MinGW/driver.test

 
 RUN: ld.lld -### foo.o -m i386pep -shared FOO.DEF | FileCheck -check-prefix=DEF2 %s
 DEF2: -def:FOO.DEF
 
 RUN: ld.lld -### foo.o -m i386pep -obar.exe | FileCheck -check-prefix=OUT %s
 RUN: ld.lld -### foo.o -m i386pep -o bar.exe | FileCheck -check-prefix=OUT %s
 OUT: -out:bar.exe
 
+RUN: ld.lld -### foo.o -m i386pep --allow-multiple-definitions | FileCheck -check-prefix=FORCE_MULTIPLE %s
+RUN: ld.lld -### foo.o -m i386pep --no-allow-multiple-definitions --allow-multiple-definitions | FileCheck -check-prefix=FORCE_MULTIPLE %s
+FORCE_MULTIPLE: -force:multiple
+
+RUN: ld.lld -### foo.o -m i386pep --no-allow-multiple-definitions | FileCheck -check-prefix=NO_FORCE_MULTIPLE %s
+RUN: ld.lld -### foo.o -m i386pep --allow-multiple-definitions --no-allow-multiple-definitions | FileCheck -check-prefix=NO_FORCE_MULTIPLE %s
+NO_FORCE_MULTIPLE-NOT: -force:multiple
+
 RUN: ld.lld -### foo.o -m i386pep --out-implib bar | FileCheck -check-prefix=IMPLIB %s
 RUN: ld.lld -### foo.o -m i386pep --out-implib=bar | FileCheck -check-prefix=IMPLIB %s
 RUN: ld.lld -### foo.o -m i386pep -out-implib bar | FileCheck -check-prefix=IMPLIB %s
 RUN: ld.lld -### foo.o -m i386pep -out-implib=bar | FileCheck -check-prefix=IMPLIB %s
 IMPLIB: -implib:bar
 
 RUN: ld.lld -### foo.o -m i386pep -e bar | FileCheck -check-prefix=ENTRY %s
 RUN: ld.lld -### foo.o -m i386pep -ebar | FileCheck -check-prefix=ENTRY %s