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.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@223099 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Support/Unix/Program.inc b/lib/Support/Unix/Program.inc
index 0670ad3..0f45df1 100644 (file)
--- a/lib/Support/Unix/Program.inc
+++ b/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())

diff --git a/lib/Support/Windows/Program.inc b/lib/Support/Windows/Program.inc
index 6467ddd..5bd9021 100644 (file)
--- a/lib/Support/Windows/Program.inc
+++ b/lib/Support/Windows/Program.inc
@@ -62,7 +62,8 @@ ErrorOr<std::string> sys::findProgramByName(StringRef Name,
   SmallVector<StringRef, 12> PathExts;
   PathExts.push_back("");
   PathExts.push_back(".exe"); // FIXME: This must be in %PATHEXT%.
-  SplitString(std::getenv("PATHEXT"), PathExts, ";");
+  if (const char *PathExtEnv = std::getenv("PATHEXT"))
+    SplitString(PathExtEnv, PathExts, ";");
 
   SmallVector<wchar_t, MAX_PATH> U16Result;
   DWORD Len = MAX_PATH;