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, 2 insertions, 1 deletions

diff --git a/llvm/lib/Support/Windows/Program.inc b/llvm/lib/Support/Windows/Program.inc
index 6467dddb3da..5bd9021a1b7 100644
--- a/llvm/lib/Support/Windows/Program.inc
+++ b/llvm/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;