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
This commit is contained in:
Chandler Carruth 2014-12-02 00:52:01 +00:00
parent 7e32aa1015
commit 84c0f65446
2 changed files with 8 additions and 6 deletions

View File

@ -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())

View File

@ -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;