Fix several bugs in r221220's new program finding code.

In both the Unix and Windows variants, std::getenv was called and the
result passed directly to a function accepting a StringRef. This isn't
OK because it might return a null pointer and that causes the StringRef
constructor to assert (and generally produces crash-prone code if
asserts are disabled). Fix this by independently testing the result as
non-null prior to splitting things.

This in turn uncovered another bug in the Unix variant where it would
infinitely recurse if PATH="", or after this fix if PATH isn't set.
There is no need to recurse at all. Slightly re-arrange the code to make
it clear that we can just fixup the Paths argument based on the
environment if we find anything.

I don't know of a particularly useful way to test these routines in
LLVM. I'll commit a test to Clang that ensures that its driver correctly
handles various settings of PATH. However, I have no idea how to
correctly write a Windows test for the PATHEXT change. Any Windows
developers who could provide such a test, please have at. =D

Many thanks to Nick Lewycky and others for helping debug this. =/ It was
quite nasty for us to track down.

llvm-svn: 223099

1 files changed, 6 insertions, 5 deletions

diff --git a/llvm/lib/Support/Unix/Program.inc b/llvm/lib/Support/Unix/Program.inc
index 0670ad39c9a..0f45df1a0da 100644
--- a/llvm/lib/Support/Unix/Program.inc
+++ b/llvm/lib/Support/Unix/Program.inc
@@ -63,11 +63,12 @@ ErrorOr<std::string> sys::findProgramByName(StringRef Name,
   if (Name.find('/') != StringRef::npos)
     return std::string(Name);
 
-  if (Paths.empty()) {
-    SmallVector<StringRef, 16> SearchPaths;
-    SplitString(std::getenv("PATH"), SearchPaths, ":");
-    return findProgramByName(Name, SearchPaths);
-  }
+  SmallVector<StringRef, 16> EnvironmentPaths;
+  if (Paths.empty())
+    if (const char *PathEnv = std::getenv("PATH")) {
+      SplitString(PathEnv, EnvironmentPaths, ":");
+      Paths = EnvironmentPaths;
+    }
 
   for (auto Path : Paths) {
     if (Path.empty())