[Driver] Recognize -fuse-ld={bfd,gold,lld} but don't prepend "ld." or "ld64." for other values

[MaskRay]

Summary

• https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55470 added support for -fuse-ld=bfd to mean ld.bfd and -fuse-ld=gold to mean ld.gold
• rL194328 ported the feature and made -fuse-ld= available for other values (with the ld. prefix)
• D78290 changed the prefix to ld64. on Darwin.

However, the prefix (ld. or ld64.) is actually cumbersome:

• For a relative path, -fuse-ld=dir/foo currently tries to access x86_64-unknown-linux-gnu-ld.dir/foo (if LLVM_DEFAULT_TARGET_TRIPLE is x86_64-unknown-linux-gnu).
• wasm and Windows do not seem to need the ld. convention.

We could teach -fuse-ld= to check whether there is a path separator, and
omit the ld. or ld64. prefix, but then a relative path will be
unnecessarily inconsistent with a single path component.

Let's hard code the currently used values which intend to get a prefix:
bfd, gold, lld. For all other values, don't add a prefix.

GCC currently hard codes -fuse-ld={bfd,gold,lld} but does not support
other values. I am going to make it support arbitrary values.
https://sourceware.org/pipermail/gcc-patches/2020-May/546277.html

[MaskRay]

Fix fuse-ld-windows.c (I don't have Windows (REQUIRES: system-windows), so I failed to catch the issue earlier)

[MaskRay]

Fix fuse-ld-windows.c

[theraven]

I'm a bit nervous about this. I'm aware of at least one out-of-tree toolchain that uses this mechanism to select their proprietary linker. I'd expect an RFC and for this to be highlighted in LLVM Weekly before I'm happy that this won't break downstream consumers.

[MaskRay]

In D80225#2046154, @theraven wrote:

I'm a bit nervous about this. I'm aware of at least one out-of-tree toolchain that uses this mechanism to select their proprietary linker. I'd expect an RFC and for this to be highlighted in LLVM Weekly before I'm happy that this won't break downstream consumers.

Created http://lists.llvm.org/pipermail/cfe-dev/2020-May/065518.html

[rnk]

Personally, I support this. I never liked the magic ld. prefixing. If we want a way to select a proprietary linker, it seems reasonable to ask the user to name the complete binary basename (ld.lld, lld, ld64, myld, etc). The driver shouldn't force the linker to use some special executable naming scheme.

[jyknight]

It's worrying to me that there number of places in LLVM that at the exact argument value of "-fuse-ld=". E.g. in the windows and PS4 toolchains. We already claim to support arbitrary values and full paths, but if you specify "-fuse-ld=/path/to/lld-link" on Windows today, you end up with different behavior than "-fuse-ld=lld" (which gets translated to searching for an "lld-link" binary, but also triggers other conditions).

That's not a reason to not make this particular change, but if conditionals on the flavor of linker are going to be used, that seems like perhaps a reason why we should not accept arbitrary values at all?

[keith]

I've submitted a related change to accept relative paths for -fuse-ld https://reviews.llvm.org/D80660

[MaskRay]

@theraven Are you ok with this change? You seem to have a use case not using ld.bfd, ld.gold or ld.lld . This change will require you to adapt.

In D80225#2049466, @jyknight wrote:

It's worrying to me that there number of places in LLVM that at the exact argument value of "-fuse-ld=". E.g. in the windows and PS4 toolchains. We already claim to support arbitrary values and full paths, but if you specify "-fuse-ld=/path/to/lld-link" on Windows today, you end up with different behavior than "-fuse-ld=lld" (which gets translated to searching for an "lld-link" binary, but also triggers other conditions).

That's not a reason to not make this particular change, but if conditionals on the flavor of linker are going to be used, that seems like perhaps a reason why we should not accept arbitrary values at all?

This is indeed unfortunate, but I can see the reasons people customize linker options for different linkers. An arbitrary value is a bit convenient when people want to compare behavior differences between different versions of a linker. As a linker developer I do this a lot. Some ClangBuiltLinux folks do this. (In addition, an absolute path can sometimes give extra confidence that a particular executable is picked -> -fuse-ld=lld may pick one from PATH if ld.lld does not sit beside clang.) Now @keith also expresses interest in such a feature. I think it is really difficult for us to drop it now.

[MaskRay]

Abandon in favor of D83015

clang/lib/Driver/ToolChain.cpp

 std::string ToolChain::GetProgramPath(const char *Name) const {
   return D.GetProgramPath(Name, *this);
 }
 
 std::string ToolChain::GetLinkerPath() const {
   const Arg* A = Args.getLastArg(options::OPT_fuse_ld_EQ);
   StringRef UseLinker = A ? A->getValue() : CLANG_DEFAULT_LINKER;
 
+  // If we're passed -fuse-ld= with no argument, or with the argument ld,
+  // then use whatever the default system linker is.
+  if (UseLinker.empty() || UseLinker == "ld")
+    return GetProgramPath(getDefaultLinker());
   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::can_execute(UseLinker))
       return std::string(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())
+    // Recognize common abbreviations by appending an OS specific prefix.
+    if (Triple.isOSDarwin()) {
+      if (UseLinker == "lld")
         LinkerName.append("ld64.");
-    else
+    } else if (UseLinker == "bfd" || UseLinker == "gold" || UseLinker == "lld")
       LinkerName.append("ld.");
+
     LinkerName.append(UseLinker);
 
     std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
     if (llvm::sys::fs::can_execute(LinkerPath))
       return LinkerPath;
   }
 
   if (A)

clang/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:     -fuse-ld=%S/Inputs/fuse_ld_windows/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:     -fuse-ld=%S/Inputs/fuse_ld_windows/foo.exe 2>&1 \
 // RUN:   | FileCheck %s
 
 // CHECK-NOT: invalid linker name
-// CHECK: /Inputs/fuse_ld_windows{{/|\\\\}}ld.foo
+// CHECK: /Inputs/fuse_ld_windows{{/|\\\\}}foo

clang/test/Driver/fuse-ld.c

 // RUN:   | FileCheck %s --check-prefix=CHECK-FREEBSD-LD
 // CHECK-FREEBSD-LD: ld
 
 // RUN: %clang %s -### -fuse-ld=bfd \
 // RUN:     --sysroot=%S/Inputs/basic_freebsd_tree \
 // RUN:     -target x86_64-unknown-freebsd \
 // RUN:     -B%S/Inputs/basic_freebsd_tree/usr/bin 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-FREEBSD-BFD
+// RUN: %clang %s -### -fuse-ld=ld.bfd \
+// RUN:     --sysroot=%S/Inputs/basic_freebsd_tree \
+// RUN:     -target x86_64-unknown-freebsd \
+// RUN:     -B%S/Inputs/basic_freebsd_tree/usr/bin 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=CHECK-FREEBSD-BFD
+
 // CHECK-FREEBSD-BFD: Inputs/basic_freebsd_tree/usr/bin{{/|\\+}}ld.bfd
 
 // RUN: %clang %s -### -fuse-ld=gold \
 // RUN:     --sysroot=%S/Inputs/basic_freebsd_tree \
 // RUN:     -target x86_64-unknown-freebsd \
 // RUN:     -B%S/Inputs/basic_freebsd_tree/usr/bin 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-FREEBSD-GOLD
+// RUN: %clang %s -### -fuse-ld=ld.gold \
+// RUN:     --sysroot=%S/Inputs/basic_freebsd_tree \
+// RUN:     -target x86_64-unknown-freebsd \
+// RUN:     -B%S/Inputs/basic_freebsd_tree/usr/bin 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=CHECK-FREEBSD-GOLD
+
 // CHECK-FREEBSD-GOLD: Inputs/basic_freebsd_tree/usr/bin{{/|\\+}}ld.gold
 
 // RUN: %clang %s -### -fuse-ld=plib \
 // RUN:     --sysroot=%S/Inputs/basic_freebsd_tree \
 // RUN:     -target x86_64-unknown-freebsd \
 // RUN:     -B%S/Inputs/basic_freebsd_tree/usr/bin 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-FREEBSD-PLIB
 // CHECK-FREEBSD-PLIB: error: invalid linker name