[clang] 1bc5c84 - [Driver] Add --ld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

Fangrui Song via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 20 09:34:57 PDT 2020


Author: Fangrui Song
Date: 2020-07-20T09:34:39-07:00
New Revision: 1bc5c84710a8c73ef21295e63c19d10a8c71f2f5

URL: https://github.com/llvm/llvm-project/commit/1bc5c84710a8c73ef21295e63c19d10a8c71f2f5
DIFF: https://github.com/llvm/llvm-project/commit/1bc5c84710a8c73ef21295e63c19d10a8c71f2f5.diff

LOG: [Driver] Add --ld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

Supersedes D80225. Add --ld-path= to avoid strange target specific
prefixes and make -fuse-ld= focus on its intended job: "linker flavor".
(-f* affects generated code or language features. --ld-path does not
affect codegen, so it is not named -f*)

The way --ld-path= works is similar to "Command Search and Execution" in POSIX.1-2017 2.9.1 Simple Commands.

If --ld-path= specifies

* an absolute path, the value specifies the linker.
* a relative path without a path component separator (/), the value is searched using the -B, COMPILER_PATH, then PATH.
* a relative path with a path component separator, the linker is found relative to the current working directory.

-fuse-ld= and --ld-path= can be composed, e.g. `-fuse-ld=lld --ld-path=/usr/bin/ld.lld`

The driver can base its linker option decision on the flavor -fuse-ld=, but it should not do fragile
flavor checking with --ld-path=.

Reviewed By: whitequark, keith

Differential Revision: https://reviews.llvm.org/D83015

Added: 
    clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/ld.bfd
    clang/test/Driver/ld-path.c

Modified: 
    clang/include/clang/Basic/DiagnosticDriverKinds.td
    clang/include/clang/Driver/Options.td
    clang/lib/Driver/ToolChain.cpp
    clang/test/Driver/fuse-ld.c
    clang/test/Misc/warning-flags.c

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 3c266846c689..e6b51cfd7a8a 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -458,6 +458,9 @@ def warn_drv_msvc_not_found : Warning<
   "try running Clang from a developer command prompt">,
   InGroup<DiagGroup<"msvc-not-found">>;
 
+def warn_drv_use_ld_non_word : Warning<
+  "'-fuse-ld=' taking a path is deprecated. Use '--ld-path=' instead">;
+
 def warn_drv_fine_grained_bitfield_accesses_ignored : Warning<
   "option '-ffine-grained-bitfield-accesses' cannot be enabled together with a sanitizer; flag ignored">,
   InGroup<OptionIgnored>;

diff  --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index d549e4b58507..c2d349866814 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3280,6 +3280,7 @@ defm : BooleanFFlag<"keep-inline-functions">, Group<clang_ignored_gcc_optimizati
 def fprofile_dir : Joined<["-"], "fprofile-dir=">, Group<f_Group>;
 
 def fuse_ld_EQ : Joined<["-"], "fuse-ld=">, Group<f_Group>, Flags<[CoreOption]>;
+def ld_path_EQ : Joined<["--"], "ld-path=">;
 
 defm align_labels : BooleanFFlag<"align-labels">, Group<clang_ignored_gcc_optimization_f_Group>;
 def falign_labels_EQ : Joined<["-"], "falign-labels=">, Group<clang_ignored_gcc_optimization_f_Group>;

diff  --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index b7256eb08ac6..2984537c23b4 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -547,18 +547,42 @@ std::string ToolChain::GetProgramPath(const char *Name) const {
 }
 
 std::string ToolChain::GetLinkerPath() const {
+  // Get -fuse-ld= first to prevent -Wunused-command-line-argument. -fuse-ld= is
+  // considered as the linker flavor, e.g. "bfd", "gold", or "lld".
   const Arg* A = Args.getLastArg(options::OPT_fuse_ld_EQ);
   StringRef UseLinker = A ? A->getValue() : CLANG_DEFAULT_LINKER;
 
+  // --ld-path= takes precedence over -fuse-ld= and specifies the executable
+  // name. -B, COMPILER_PATH and PATH and consulted if the value does not
+  // contain a path component separator.
+  if (const Arg *A = Args.getLastArg(options::OPT_ld_path_EQ)) {
+    std::string Path(A->getValue());
+    if (!Path.empty()) {
+      if (llvm::sys::path::parent_path(Path).empty())
+        Path = GetProgramPath(A->getValue());
+      if (llvm::sys::fs::can_execute(Path))
+        return std::string(Path);
+    }
+    getDriver().Diag(diag::err_drv_invalid_linker_name) << A->getAsString(Args);
+    return GetProgramPath(getDefaultLinker());
+  }
+  // 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());
+
+  // Extending -fuse-ld= to an absolute or relative path is unexpected. Checking
+  // for the linker flavor is brittle. In addition, prepending "ld." or "ld64."
+  // to a relative path is surprising. This is more complex due to priorities
+  // among -B, COMPILER_PATH and PATH. --ld-path= should be used instead.
+  if (UseLinker.find('/') != StringRef::npos)
+    getDriver().Diag(diag::warn_drv_use_ld_non_word);
+
   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())

diff  --git a/clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/ld.bfd b/clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/ld.bfd
new file mode 100755
index 000000000000..b23e55619b2f
--- /dev/null
+++ b/clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/ld.bfd
@@ -0,0 +1 @@
+#!/bin/true

diff  --git a/clang/test/Driver/fuse-ld.c b/clang/test/Driver/fuse-ld.c
index f2ca9fb36194..678913dd84c3 100644
--- a/clang/test/Driver/fuse-ld.c
+++ b/clang/test/Driver/fuse-ld.c
@@ -2,6 +2,7 @@
 // RUN:     -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 \
 // RUN:     -target x86_64-unknown-linux \
 // RUN:   | FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+// CHECK-ABSOLUTE-LD: warning: '-fuse-ld=' taking a path is deprecated. Use '--ld-path=' instead
 // CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld
 
 

diff  --git a/clang/test/Driver/ld-path.c b/clang/test/Driver/ld-path.c
new file mode 100644
index 000000000000..a982ea3739a1
--- /dev/null
+++ b/clang/test/Driver/ld-path.c
@@ -0,0 +1,66 @@
+/// This tests uses the PATH environment variable.
+// UNSUPPORTED: system-windows
+
+// RUN: cd %S
+
+/// If --ld-path= specifies a word (without /), -B and COMPILER_PATH are
+/// consulted to locate the linker.
+// RUN: %clang %s -### -B %S/Inputs/basic_freebsd_tree/usr/bin --ld-path=ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD
+// RUN: env COMPILER_PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### --ld-path=ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD
+/// Then PATH is consulted.
+// RUN: env PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### --ld-path=ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD
+
+// BFD: Inputs/basic_freebsd_tree/usr/bin/ld.bfd"
+
+// RUN: env PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### --ld-path=ld.gold \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=GOLD
+
+// GOLD: Inputs/basic_freebsd_tree/usr/bin/ld.gold"
+
+// RUN: env COMPILER_PATH= PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### --ld-path=not_exist \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=NOT_EXIST
+
+// NOT_EXIST: error: invalid linker name in argument '--ld-path=not_exist'
+
+// RUN: %clang %s -### --ld-path= \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=EMPTY
+
+// EMPTY: error: invalid linker name in argument '--ld-path='
+
+/// If --ld-path= contains a slash, PATH is not consulted.
+// RUN: env COMPILER_PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### --ld-path=./ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=NO_BFD
+
+// NO_BFD: error: invalid linker name in argument '--ld-path=./ld.bfd'
+
+/// --ld-path can specify an absolute path.
+// RUN: %clang %s -### --ld-path=%S/Inputs/basic_freebsd_tree/usr/bin/ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD
+
+// RUN: %clang %s -### --ld-path=Inputs/basic_freebsd_tree/usr/bin/ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD
+
+/// --ld-path= and -fuse-ld= can be used together. --ld-path= takes precedence.
+/// -fuse-ld= can be used to specify the linker flavor.
+// RUN: %clang %s -### -Werror --ld-path=%S/Inputs/basic_freebsd_tree/usr/bin/ld.bfd -fuse-ld=gold \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD --implicit-check-not=error:
+
+/// --ld-path= respects -working-directory.
+// RUN: %clang %s -### --ld-path=usr/bin/ld.bfd -working-directory=%S/Inputs/basic_freebsd_tree \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=USR_BIN_BFD
+
+// USR_BIN_BFD: "usr/bin/ld.bfd"

diff  --git a/clang/test/Misc/warning-flags.c b/clang/test/Misc/warning-flags.c
index e4f9069b88c8..168f2a3551ba 100644
--- a/clang/test/Misc/warning-flags.c
+++ b/clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@ This test serves two purposes:
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (68):
+CHECK: Warnings without flags (69):
 
 CHECK-NEXT:   ext_expected_semi_decl_list
 CHECK-NEXT:   ext_explicit_specialization_storage_class
@@ -47,6 +47,7 @@ CHECK-NEXT:   warn_double_const_requires_fp64
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
 CHECK-NEXT:   warn_drv_clang_unsupported
 CHECK-NEXT:   warn_drv_pch_not_first_include
+CHECK-NEXT:   warn_drv_use_ld_non_word
 CHECK-NEXT:   warn_dup_category_def
 CHECK-NEXT:   warn_enum_value_overflow
 CHECK-NEXT:   warn_expected_qualified_after_typename


        


More information about the cfe-commits mailing list