Page MenuHomePhabricator

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

Authored by ikudrin on Feb 22 2018, 7:46 AM.

Details

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.

Diff Detail

Event Timeline

ikudrin created this revision.Feb 22 2018, 7:46 AM
Hahnfeld added inline comments.Feb 22 2018, 7:51 AM
lib/Driver/ToolChain.cpp
415

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

ikudrin added inline comments.Feb 22 2018, 7:53 AM
lib/Driver/ToolChain.cpp
415

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

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

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

Well, since 12/2016: rC289668

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 added a subscriber: ruiu.Feb 22 2018, 8:29 PM

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?

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.

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 updated this revision to Diff 135888.Feb 26 2018, 5:50 AM
  • Add a test.
ruiu added a comment.Feb 26 2018, 1:15 PM

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 added a comment.Feb 26 2018, 2:10 PM

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 accepted this revision.Feb 26 2018, 2:10 PM

lgtm

This revision is now accepted and ready to land.Feb 26 2018, 2:10 PM
This revision was automatically updated to reflect the committed changes.