Re: [PATCH] D17952: [Clang] Accept absolute paths in the -fuse-ld option

2016-03-08 Thread whitequark via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL262996: Accept absolute paths in the -fuse-ld option. 
(authored by whitequark).

Repository:
  rL LLVM

http://reviews.llvm.org/D17952

Files:
  cfe/trunk/lib/Driver/ToolChain.cpp
  cfe/trunk/test/Driver/fuse-ld.c

Index: cfe/trunk/test/Driver/fuse-ld.c
===
--- cfe/trunk/test/Driver/fuse-ld.c
+++ cfe/trunk/test/Driver/fuse-ld.c
@@ -1,4 +1,10 @@
 // RUN: %clang %s -### \
+// RUN: -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+// CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld
+
+
+// RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-FREEBSD-LD
 // CHECK-FREEBSD-LD: ld
Index: cfe/trunk/lib/Driver/ToolChain.cpp
===
--- cfe/trunk/lib/Driver/ToolChain.cpp
+++ cfe/trunk/lib/Driver/ToolChain.cpp
@@ -342,19 +342,26 @@
 
 std::string ToolChain::GetLinkerPath() const {
   if (Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ)) {
-StringRef Suffix = A->getValue();
+StringRef UseLinker = A->getValue();
 
-// If we're passed -fuse-ld= with no argument, or with the argument ld,
-// then use whatever the default system linker is.
-if (Suffix.empty() || Suffix == "ld")
-  return GetProgramPath("ld");
-
-llvm::SmallString<8> LinkerName("ld.");
-LinkerName.append(Suffix);
-
-std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-if (llvm::sys::fs::exists(LinkerPath))
-  return LinkerPath;
+if (llvm::sys::path::is_absolute(UseLinker)) {
+  // If we're passed -fuse-ld= with what looks like an absolute path,
+  // don't attempt to second-guess that.
+  if (llvm::sys::fs::exists(UseLinker))
+return UseLinker;
+} else {
+  // 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("ld");
+
+  llvm::SmallString<8> LinkerName("ld.");
+  LinkerName.append(UseLinker);
+
+  std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
+  if (llvm::sys::fs::exists(LinkerPath))
+return LinkerPath;
+}
 
 getDriver().Diag(diag::err_drv_invalid_linker_name) << 
A->getAsString(Args);
 return "";


Index: cfe/trunk/test/Driver/fuse-ld.c
===
--- cfe/trunk/test/Driver/fuse-ld.c
+++ cfe/trunk/test/Driver/fuse-ld.c
@@ -1,4 +1,10 @@
 // RUN: %clang %s -### \
+// RUN: -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+// CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld
+
+
+// RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-FREEBSD-LD
 // CHECK-FREEBSD-LD: ld
Index: cfe/trunk/lib/Driver/ToolChain.cpp
===
--- cfe/trunk/lib/Driver/ToolChain.cpp
+++ cfe/trunk/lib/Driver/ToolChain.cpp
@@ -342,19 +342,26 @@
 
 std::string ToolChain::GetLinkerPath() const {
   if (Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ)) {
-StringRef Suffix = A->getValue();
+StringRef UseLinker = A->getValue();
 
-// If we're passed -fuse-ld= with no argument, or with the argument ld,
-// then use whatever the default system linker is.
-if (Suffix.empty() || Suffix == "ld")
-  return GetProgramPath("ld");
-
-llvm::SmallString<8> LinkerName("ld.");
-LinkerName.append(Suffix);
-
-std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-if (llvm::sys::fs::exists(LinkerPath))
-  return LinkerPath;
+if (llvm::sys::path::is_absolute(UseLinker)) {
+  // If we're passed -fuse-ld= with what looks like an absolute path,
+  // don't attempt to second-guess that.
+  if (llvm::sys::fs::exists(UseLinker))
+return UseLinker;
+} else {
+  // 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("ld");
+
+  llvm::SmallString<8> LinkerName("ld.");
+  LinkerName.append(UseLinker);
+
+  std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
+  if (llvm::sys::fs::exists(LinkerPath))
+return LinkerPath;
+}
 
 getDriver().Diag(diag::err_drv_invalid_linker_name) << A->getAsString(Args);
 return "";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17952: [Clang] Accept absolute paths in the -fuse-ld option

2016-03-08 Thread Eric Christopher via cfe-commits
echristo accepted this revision.
echristo added a reviewer: echristo.
echristo added a comment.
This revision is now accepted and ready to land.

LGTM, thanks.

-eric


Repository:
  rL LLVM

http://reviews.llvm.org/D17952



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17952: [Clang] Accept absolute paths in the -fuse-ld option

2016-03-08 Thread whitequark via cfe-commits
whitequark updated this revision to Diff 50097.
whitequark added a comment.

Added a test


Repository:
  rL LLVM

http://reviews.llvm.org/D17952

Files:
  cfe/trunk/lib/Driver/ToolChain.cpp
  cfe/trunk/test/Driver/fuse-ld.c

Index: cfe/trunk/test/Driver/fuse-ld.c
===
--- cfe/trunk/test/Driver/fuse-ld.c
+++ cfe/trunk/test/Driver/fuse-ld.c
@@ -1,4 +1,10 @@
 // RUN: %clang %s -### \
+// RUN: -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+// CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld
+
+
+// RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-FREEBSD-LD
 // CHECK-FREEBSD-LD: ld
Index: cfe/trunk/lib/Driver/ToolChain.cpp
===
--- cfe/trunk/lib/Driver/ToolChain.cpp
+++ cfe/trunk/lib/Driver/ToolChain.cpp
@@ -342,19 +342,26 @@
 
 std::string ToolChain::GetLinkerPath() const {
   if (Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ)) {
-StringRef Suffix = A->getValue();
-
-// If we're passed -fuse-ld= with no argument, or with the argument ld,
-// then use whatever the default system linker is.
-if (Suffix.empty() || Suffix == "ld")
-  return GetProgramPath("ld");
-
-llvm::SmallString<8> LinkerName("ld.");
-LinkerName.append(Suffix);
-
-std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-if (llvm::sys::fs::exists(LinkerPath))
-  return LinkerPath;
+StringRef UseLinker = A->getValue();
+
+if (llvm::sys::path::is_absolute(UseLinker)) {
+  // If we're passed -fuse-ld= with what looks like an absolute path,
+  // don't attempt to second-guess that.
+  if (llvm::sys::fs::exists(UseLinker))
+return UseLinker;
+} else {
+  // 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("ld");
+
+  llvm::SmallString<8> LinkerName("ld.");
+  LinkerName.append(UseLinker);
+
+  std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
+  if (llvm::sys::fs::exists(LinkerPath))
+return LinkerPath;
+}
 
 getDriver().Diag(diag::err_drv_invalid_linker_name) << 
A->getAsString(Args);
 return "";


Index: cfe/trunk/test/Driver/fuse-ld.c
===
--- cfe/trunk/test/Driver/fuse-ld.c
+++ cfe/trunk/test/Driver/fuse-ld.c
@@ -1,4 +1,10 @@
 // RUN: %clang %s -### \
+// RUN: -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+// CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld
+
+
+// RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-FREEBSD-LD
 // CHECK-FREEBSD-LD: ld
Index: cfe/trunk/lib/Driver/ToolChain.cpp
===
--- cfe/trunk/lib/Driver/ToolChain.cpp
+++ cfe/trunk/lib/Driver/ToolChain.cpp
@@ -342,19 +342,26 @@
 
 std::string ToolChain::GetLinkerPath() const {
   if (Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ)) {
-StringRef Suffix = A->getValue();
-
-// If we're passed -fuse-ld= with no argument, or with the argument ld,
-// then use whatever the default system linker is.
-if (Suffix.empty() || Suffix == "ld")
-  return GetProgramPath("ld");
-
-llvm::SmallString<8> LinkerName("ld.");
-LinkerName.append(Suffix);
-
-std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-if (llvm::sys::fs::exists(LinkerPath))
-  return LinkerPath;
+StringRef UseLinker = A->getValue();
+
+if (llvm::sys::path::is_absolute(UseLinker)) {
+  // If we're passed -fuse-ld= with what looks like an absolute path,
+  // don't attempt to second-guess that.
+  if (llvm::sys::fs::exists(UseLinker))
+return UseLinker;
+} else {
+  // 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("ld");
+
+  llvm::SmallString<8> LinkerName("ld.");
+  LinkerName.append(UseLinker);
+
+  std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
+  if (llvm::sys::fs::exists(LinkerPath))
+return LinkerPath;
+}
 
 getDriver().Diag(diag::err_drv_invalid_linker_name) << A->getAsString(Args);
 return "";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17952: [Clang] Accept absolute paths in the -fuse-ld option

2016-03-08 Thread Eric Christopher via cfe-commits
echristo added a subscriber: echristo.
echristo added a comment.

Needs a testcase, but otherwise should be fine.


Repository:
  rL LLVM

http://reviews.llvm.org/D17952



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17952: [Clang] Accept absolute paths in the -fuse-ld option

2016-03-08 Thread Jonathan Roelofs via cfe-commits
jroelofs added a subscriber: jroelofs.
jroelofs added a comment.

Testcases?


Repository:
  rL LLVM

http://reviews.llvm.org/D17952



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17952: [Clang] Accept absolute paths in the -fuse-ld option

2016-03-07 Thread Sean Silva via cfe-commits
silvas added inline comments.


Comment at: cfe/trunk/lib/Driver/ToolChain.cpp:352
@@ +351,3 @@
+return UseLinker;
+} else {
+  // If we're passed -fuse-ld= with no argument, or with the argument ld,

Makes sense.


Repository:
  rL LLVM

http://reviews.llvm.org/D17952



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17952: [Clang] Accept absolute paths in the -fuse-ld option

2016-03-07 Thread whitequark via cfe-commits
whitequark updated this revision to Diff 50021.
whitequark added a comment.

Windows compatibility


Repository:
  rL LLVM

http://reviews.llvm.org/D17952

Files:
  cfe/trunk/lib/Driver/ToolChain.cpp

Index: cfe/trunk/lib/Driver/ToolChain.cpp
===
--- cfe/trunk/lib/Driver/ToolChain.cpp
+++ cfe/trunk/lib/Driver/ToolChain.cpp
@@ -342,19 +342,26 @@
 
 std::string ToolChain::GetLinkerPath() const {
   if (Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ)) {
-StringRef Suffix = A->getValue();
-
-// If we're passed -fuse-ld= with no argument, or with the argument ld,
-// then use whatever the default system linker is.
-if (Suffix.empty() || Suffix == "ld")
-  return GetProgramPath("ld");
-
-llvm::SmallString<8> LinkerName("ld.");
-LinkerName.append(Suffix);
-
-std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-if (llvm::sys::fs::exists(LinkerPath))
-  return LinkerPath;
+StringRef UseLinker = A->getValue();
+
+if (llvm::sys::path::is_absolute(UseLinker)) {
+  // If we're passed -fuse-ld= with what looks like an absolute path,
+  // don't attempt to second-guess that.
+  if (llvm::sys::fs::exists(UseLinker))
+return UseLinker;
+} else {
+  // 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("ld");
+
+  llvm::SmallString<8> LinkerName("ld.");
+  LinkerName.append(UseLinker);
+
+  std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
+  if (llvm::sys::fs::exists(LinkerPath))
+return LinkerPath;
+}
 
 getDriver().Diag(diag::err_drv_invalid_linker_name) << 
A->getAsString(Args);
 return "";


Index: cfe/trunk/lib/Driver/ToolChain.cpp
===
--- cfe/trunk/lib/Driver/ToolChain.cpp
+++ cfe/trunk/lib/Driver/ToolChain.cpp
@@ -342,19 +342,26 @@
 
 std::string ToolChain::GetLinkerPath() const {
   if (Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ)) {
-StringRef Suffix = A->getValue();
-
-// If we're passed -fuse-ld= with no argument, or with the argument ld,
-// then use whatever the default system linker is.
-if (Suffix.empty() || Suffix == "ld")
-  return GetProgramPath("ld");
-
-llvm::SmallString<8> LinkerName("ld.");
-LinkerName.append(Suffix);
-
-std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-if (llvm::sys::fs::exists(LinkerPath))
-  return LinkerPath;
+StringRef UseLinker = A->getValue();
+
+if (llvm::sys::path::is_absolute(UseLinker)) {
+  // If we're passed -fuse-ld= with what looks like an absolute path,
+  // don't attempt to second-guess that.
+  if (llvm::sys::fs::exists(UseLinker))
+return UseLinker;
+} else {
+  // 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("ld");
+
+  llvm::SmallString<8> LinkerName("ld.");
+  LinkerName.append(UseLinker);
+
+  std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
+  if (llvm::sys::fs::exists(LinkerPath))
+return LinkerPath;
+}
 
 getDriver().Diag(diag::err_drv_invalid_linker_name) << A->getAsString(Args);
 return "";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17952: [Clang] Accept absolute paths in the -fuse-ld option

2016-03-07 Thread Sean Silva via cfe-commits
silvas added a subscriber: silvas.
silvas added a comment.

Some nits but this looks fine to me. I've been wanting this for a while now.



Comment at: cfe/trunk/lib/Driver/ToolChain.cpp:347
@@ +346,3 @@
+
+if (UseLinker[0] == '/') {
+  // If we're passed -fuse-ld= with what looks like an absolute path,

Use llvm::sys::path::is_absolute so that this works on windows.


Comment at: cfe/trunk/lib/Driver/ToolChain.cpp:352
@@ +351,3 @@
+return UseLinker;
+} else {
+  // If we're passed -fuse-ld= with no argument, or with the argument ld,

No need for else after return.


Repository:
  rL LLVM

http://reviews.llvm.org/D17952



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D17952: [Clang] Accept absolute paths in the -fuse-ld option

2016-03-07 Thread whitequark via cfe-commits
whitequark created this revision.
whitequark added a reviewer: majnemer.
whitequark added a subscriber: cfe-commits.
whitequark set the repository for this revision to rL LLVM.
Herald added a subscriber: aemerson.

This patch extends the -fuse-ld option to accept a full path to an executable
and use it verbatim to invoke the linker. There are generally two reasons
to desire this.

The first reason relates to the sad truth is that Clang is retargetable,
Binutils are not.

While any Clang from a binary distribution is sufficient to compile code 
for a wide range of architectures and prefixed BFD linkers (e.g.
installed as /usr/bin/arm-none-linux-gnueabi-ld) as well as cross-compiled
libc's (for non-bare-metal targets) are widely available, including on all
Debian derivatives, it is impossible to use them together because
the -fuse-ld= option allows to specify neither a linker prefix nor 
a full path to one.

The second reason is linker development, both when porting existing linkers
to new architectures and when working on a new linker such as LLD.

Repository:
  rL LLVM

http://reviews.llvm.org/D17952

Files:
  cfe/trunk/lib/Driver/ToolChain.cpp

Index: cfe/trunk/lib/Driver/ToolChain.cpp
===
--- cfe/trunk/lib/Driver/ToolChain.cpp
+++ cfe/trunk/lib/Driver/ToolChain.cpp
@@ -342,19 +342,26 @@
 
 std::string ToolChain::GetLinkerPath() const {
   if (Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ)) {
-StringRef Suffix = A->getValue();
-
-// If we're passed -fuse-ld= with no argument, or with the argument ld,
-// then use whatever the default system linker is.
-if (Suffix.empty() || Suffix == "ld")
-  return GetProgramPath("ld");
-
-llvm::SmallString<8> LinkerName("ld.");
-LinkerName.append(Suffix);
-
-std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-if (llvm::sys::fs::exists(LinkerPath))
-  return LinkerPath;
+StringRef UseLinker = A->getValue();
+
+if (UseLinker[0] == '/') {
+  // If we're passed -fuse-ld= with what looks like an absolute path,
+  // don't attempt to second-guess that.
+  if (llvm::sys::fs::exists(UseLinker))
+return UseLinker;
+} else {
+  // 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("ld");
+
+  llvm::SmallString<8> LinkerName("ld.");
+  LinkerName.append(UseLinker);
+
+  std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
+  if (llvm::sys::fs::exists(LinkerPath))
+return LinkerPath;
+}
 
 getDriver().Diag(diag::err_drv_invalid_linker_name) << 
A->getAsString(Args);
 return "";


Index: cfe/trunk/lib/Driver/ToolChain.cpp
===
--- cfe/trunk/lib/Driver/ToolChain.cpp
+++ cfe/trunk/lib/Driver/ToolChain.cpp
@@ -342,19 +342,26 @@
 
 std::string ToolChain::GetLinkerPath() const {
   if (Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ)) {
-StringRef Suffix = A->getValue();
-
-// If we're passed -fuse-ld= with no argument, or with the argument ld,
-// then use whatever the default system linker is.
-if (Suffix.empty() || Suffix == "ld")
-  return GetProgramPath("ld");
-
-llvm::SmallString<8> LinkerName("ld.");
-LinkerName.append(Suffix);
-
-std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-if (llvm::sys::fs::exists(LinkerPath))
-  return LinkerPath;
+StringRef UseLinker = A->getValue();
+
+if (UseLinker[0] == '/') {
+  // If we're passed -fuse-ld= with what looks like an absolute path,
+  // don't attempt to second-guess that.
+  if (llvm::sys::fs::exists(UseLinker))
+return UseLinker;
+} else {
+  // 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("ld");
+
+  llvm::SmallString<8> LinkerName("ld.");
+  LinkerName.append(UseLinker);
+
+  std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
+  if (llvm::sys::fs::exists(LinkerPath))
+return LinkerPath;
+}
 
 getDriver().Diag(diag::err_drv_invalid_linker_name) << A->getAsString(Args);
 return "";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits