[Driver] Allow using a canonical form of '-fuse-ld=' when cross-compiling on Windows.

[ikudrin]

Summary

Right now, we have to add an .exe suffix when using this switch, like -fuse-ld=lld.exe.
If the suffix is omitted, llvm::sys::fs::exists() cannot find the file on Windows,
while llvm::sys::fs::can_Execute() automatically tries both variants.

[Hahnfeld]

lib/Driver/ToolChain.cpp

415 ↗

(On Diff #135426)

I don't think we should do this for absolute paths?

[ikudrin]

lib/Driver/ToolChain.cpp

415 ↗

(On Diff #135426)

Why not? You can omit ".exe" suffix on Windows when calling an application, right?

[amccarth]

This must be new-ish code, since I've routinely used -fuse-ld=lld (without .exe) on Windows.

[Hahnfeld]

In D43621#1015888, @amccarth wrote:

This must be new-ish code, since I've routinely used -fuse-ld=lld (without .exe) on Windows.

Well, since 12/2016: rC289668

[amccarth]

Right now, we have to add an .exe suffix when using this switch, like -fuse-ld=lld.exe.

That's weird, because lots of lldb tests compile and link test binaries on Windows with -fuse-ld=lld (without the .exe). What makes you say the .exe is necessary?

[ruiu]

That's weird, because lots of lldb tests compile and link test binaries on Windows with -fuse-ld=lld (without the .exe). What makes you say the .exe is necessary?

Maybe he is using clang (not clang-cl) and want to use ld.lld.exe instead of lld-link?

[amccarth]

In D43621#1017027, @ruiu wrote:

That's weird, because lots of lldb tests compile and link test binaries on Windows with -fuse-ld=lld (without the .exe). What makes you say the .exe is necessary?

Maybe he is using clang (not clang-cl) and want to use ld.lld.exe instead of lld-link?

The LLDB tests use clang, not clang-cl. Here's a typical invocation:

D:\src\llvm\build\ninja_release\bin\clang.exe  main.o -g -O0 -fno-builtin -m32  -ID:\src\llvm\mono\llvm-project\lldb\packages\Python\lldbsuite\test\lang\c\conflicting-symbol/../../../make/../../../../../include -ID:\src\llvm\mono\llvm-project\lldb\packages\Python\lldbsuite\test\lang\c\conflicting-symbol/ -include D:\src\llvm\mono\llvm-project\lldb\packages\Python\lldbsuite\test\lang\c\conflicting-symbol/../../../make/test_common.h -ID:\src\llvm\mono\llvm-project\lldb\packages\Python\lldbsuite\test\lang\c\conflicting-symbol/../../../make/  -fno-limit-debug-info   -L. -LOne -lOne -LTwo -lTwo  -fuse-ld=lld --driver-mode=g++  -o "a.out"

Note that it's running clang.exe and passing -fuse-ld=lld.

[ikudrin]

Not all toolchains call ToolChain::GetLinkerPath. For example, MSVC toolchain uses its own code:

void visualstudio::Linker::ConstructJob(...) {
...
  StringRef Linker = Args.getLastArgValue(options::OPT_fuse_ld_EQ, "link");
  if (Linker.equals_lower("lld"))
    Linker = "lld-link";
...
}

In my case, I am trying to cross-compile:

> ...\clang.exe a.cpp -fuse-ld=lld -target i686-pc-linux-gnu
clang.exe: error: invalid linker name in argument '-fuse-ld=lld'

[ikudrin]

• Add a test.

[ruiu]

This change looks good to me, but I think we shouldn't check in an executable binary file. Can you create ld.foo.exe in the test? Since you don't actually run ld.foo.exe, creating that executable should be very easy.

[rnk]

In D43621#1019724, @ruiu wrote:

This change looks good to me, but I think we shouldn't check in an executable binary file. Can you create ld.foo.exe in the test? Since you don't actually run ld.foo.exe, creating that executable should be very easy.

If I read the diff correctly, it is an empty file, so this should just work.

[rnk]

lgtm

cfe/trunk/lib/Driver/ToolChain.cpp

 
 std::string ToolChain::GetLinkerPath() const {
   const Arg* A = Args.getLastArg(options::OPT_fuse_ld_EQ);
   StringRef UseLinker = A ? A->getValue() : CLANG_DEFAULT_LINKER;
 
   if (llvm::sys::path::is_absolute(UseLinker)) {
     // If we're passed what looks like an absolute path, don't attempt to
     // second-guess that.
-    if (llvm::sys::fs::exists(UseLinker))
+    if (llvm::sys::fs::can_execute(UseLinker))
       return UseLinker;
   } else if (UseLinker.empty() || UseLinker == "ld") {
     // If we're passed -fuse-ld= with no argument, or with the argument ld,
     // then use whatever the default system linker is.
     return GetProgramPath(getDefaultLinker());
   } else {
     llvm::SmallString<8> LinkerName;
     if (Triple.isOSDarwin())
       LinkerName.append("ld64.");
     else
       LinkerName.append("ld.");
     LinkerName.append(UseLinker);
 
     std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-    if (llvm::sys::fs::exists(LinkerPath))
+    if (llvm::sys::fs::can_execute(LinkerPath))
       return LinkerPath;
   }
 
   if (A)
     getDriver().Diag(diag::err_drv_invalid_linker_name) << A->getAsString(Args);
 
   return GetProgramPath(getDefaultLinker());
 }

cfe/trunk/test/Driver/fuse-ld-windows.c

+// REQUIRES: system-windows
+
+// We used to require adding ".exe" suffix when cross-compiling on Windows.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN:     -B %S/Inputs/fuse_ld_windows -fuse-ld=foo 2>&1 \
+// RUN:   | FileCheck %s
+
+// Check that the old variant still works.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN:     -B %S/Inputs/fuse_ld_windows -fuse-ld=foo.exe 2>&1 \
+// RUN:   | FileCheck %s
+
+// With the full path, the extension can be omitted, too,
+// because Windows allows that.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN:     -fuse-ld=%S/Inputs/fuse_ld_windows/ld.foo 2>&1 \
+// RUN:   | FileCheck %s
+
+// Check that the full path with the extension works too.
+// RUN: %clang %s -### -o %t.o -target i386-unknown-linux \
+// RUN:     -fuse-ld=%S/Inputs/fuse_ld_windows/ld.foo.exe 2>&1 \
+// RUN:   | FileCheck %s
+
+// CHECK-NOT: invalid linker name
+// CHECK: /Inputs/fuse_ld_windows{{/|\\\\}}ld.foo