[PATCH] D34290: [Driver] Do a PATH lookup when using only program name with -no-canonical-prefixes

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL305600: [Driver] Do a PATH lookup if needed when using 
-no-canonical-prefixes (authored by phosek).

Changed prior to commit:
  https://reviews.llvm.org/D34290?vs=102881=102894#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34290

Files:
  cfe/trunk/tools/driver/driver.cpp


Index: cfe/trunk/tools/driver/driver.cpp
===
--- cfe/trunk/tools/driver/driver.cpp
+++ cfe/trunk/tools/driver/driver.cpp
@@ -53,8 +53,15 @@
 using namespace llvm::opt;
 
 std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) {
-  if (!CanonicalPrefixes)
-return Argv0;
+  if (!CanonicalPrefixes) {
+SmallString<128> ExecutablePath(Argv0);
+// Do a PATH lookup if Argv0 isn't a valid path.
+if (!llvm::sys::fs::exists(ExecutablePath))
+  if (llvm::ErrorOr P =
+  llvm::sys::findProgramByName(ExecutablePath))
+ExecutablePath = *P;
+return ExecutablePath.str();
+  }
 
   // This just needs to be some symbol in the binary; C++ doesn't
   // allow taking the address of ::main however.


Index: cfe/trunk/tools/driver/driver.cpp
===
--- cfe/trunk/tools/driver/driver.cpp
+++ cfe/trunk/tools/driver/driver.cpp
@@ -53,8 +53,15 @@
 using namespace llvm::opt;
 
 std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) {
-  if (!CanonicalPrefixes)
-return Argv0;
+  if (!CanonicalPrefixes) {
+SmallString<128> ExecutablePath(Argv0);
+// Do a PATH lookup if Argv0 isn't a valid path.
+if (!llvm::sys::fs::exists(ExecutablePath))
+  if (llvm::ErrorOr P =
+  llvm::sys::findProgramByName(ExecutablePath))
+ExecutablePath = *P;
+return ExecutablePath.str();
+  }
 
   // This just needs to be some symbol in the binary; C++ doesn't
   // allow taking the address of ::main however.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34290: [Driver] Do a PATH lookup when using only program name with -no-canonical-prefixes

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rL LLVM

https://reviews.llvm.org/D34290



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

[PATCH] D34290: [Driver] Do a PATH lookup when using only program name with -no-canonical-prefixes

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 102881.
phosek marked an inline comment as done.
phosek edited the summary of this revision.

Repository:
  rL LLVM

https://reviews.llvm.org/D34290

Files:
  tools/driver/driver.cpp


Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -53,8 +53,15 @@
 using namespace llvm::opt;
 
 std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) {
-  if (!CanonicalPrefixes)
-return Argv0;
+  if (!CanonicalPrefixes) {
+SmallString<128> ExecutablePath(Argv0);
+// Do a PATH lookup if Argv0 isn't a valid path.
+if (!llvm::sys::fs::exists(ExecutablePath))
+  if (llvm::ErrorOr P =
+  llvm::sys::findProgramByName(ExecutablePath))
+ExecutablePath = *P;
+return ExecutablePath.str();
+  }
 
   // This just needs to be some symbol in the binary; C++ doesn't
   // allow taking the address of ::main however.


Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -53,8 +53,15 @@
 using namespace llvm::opt;
 
 std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) {
-  if (!CanonicalPrefixes)
-return Argv0;
+  if (!CanonicalPrefixes) {
+SmallString<128> ExecutablePath(Argv0);
+// Do a PATH lookup if Argv0 isn't a valid path.
+if (!llvm::sys::fs::exists(ExecutablePath))
+  if (llvm::ErrorOr P =
+  llvm::sys::findProgramByName(ExecutablePath))
+ExecutablePath = *P;
+return ExecutablePath.str();
+  }
 
   // This just needs to be some symbol in the binary; C++ doesn't
   // allow taking the address of ::main however.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34290: [Driver] Do a PATH lookup when using only program name with -no-canonical-prefixes

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: tools/driver/driver.cpp:58
+SmallString<128> ExecutablePath(Argv0);
+// Do a PATH lookup, if there are no directory components.
+if (llvm::sys::path::filename(ExecutablePath) == ExecutablePath)

hans wrote:
> What if "clang" is in the current directory? Presumably that should then be 
> preferred over PATH.. will we get that right with this patch?
> 
> Another way would be to check access() on Argv0, which is what we do when 
> trying to execute anyway. Is there any downside to that?
I don't think there should be any downside to that approach.


Repository:
  rL LLVM

https://reviews.llvm.org/D34290



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

[PATCH] D34290: [Driver] Do a PATH lookup when using only program name with -no-canonical-prefixes

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Thanks!

Please add to the description that this is for PR9576.




Comment at: tools/driver/driver.cpp:58
+SmallString<128> ExecutablePath(Argv0);
+// Do a PATH lookup, if there are no directory components.
+if (llvm::sys::path::filename(ExecutablePath) == ExecutablePath)

What if "clang" is in the current directory? Presumably that should then be 
preferred over PATH.. will we get that right with this patch?

Another way would be to check access() on Argv0, which is what we do when 
trying to execute anyway. Is there any downside to that?


Repository:
  rL LLVM

https://reviews.llvm.org/D34290



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

[PATCH] D34290: [Driver] Do a PATH lookup when using only program name with -no-canonical-prefixes

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.

When -no-canonical-prefixes option is used and argv0 contains only
a program name, we need to do a PATH lookup to get an executable path,
otherwise the return value won't be a valid path and any subsequent
uses of it (e.g. invoking -cc1) will fail with an error.


Repository:
  rL LLVM

https://reviews.llvm.org/D34290

Files:
  tools/driver/driver.cpp


Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -53,8 +53,15 @@
 using namespace llvm::opt;
 
 std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) {
-  if (!CanonicalPrefixes)
-return Argv0;
+  if (!CanonicalPrefixes) {
+SmallString<128> ExecutablePath(Argv0);
+// Do a PATH lookup, if there are no directory components.
+if (llvm::sys::path::filename(ExecutablePath) == ExecutablePath)
+  if (llvm::ErrorOr P =
+  llvm::sys::findProgramByName(ExecutablePath))
+ExecutablePath = *P;
+return ExecutablePath.str();
+  }
 
   // This just needs to be some symbol in the binary; C++ doesn't
   // allow taking the address of ::main however.


Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -53,8 +53,15 @@
 using namespace llvm::opt;
 
 std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) {
-  if (!CanonicalPrefixes)
-return Argv0;
+  if (!CanonicalPrefixes) {
+SmallString<128> ExecutablePath(Argv0);
+// Do a PATH lookup, if there are no directory components.
+if (llvm::sys::path::filename(ExecutablePath) == ExecutablePath)
+  if (llvm::ErrorOr P =
+  llvm::sys::findProgramByName(ExecutablePath))
+ExecutablePath = *P;
+return ExecutablePath.str();
+  }
 
   // This just needs to be some symbol in the binary; C++ doesn't
   // allow taking the address of ::main however.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits