From ec8406d8f43c8e459034f8bddf3112c26a31a81b Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Tue, 2 Dec 2014 00:52:01 +0000 Subject: [PATCH] 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 --- llvm/lib/Support/Unix/Program.inc | 11 ++++++----- llvm/lib/Support/Windows/Program.inc | 3 ++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Support/Unix/Program.inc b/llvm/lib/Support/Unix/Program.inc index 0670ad3..0f45df1 100644 --- a/llvm/lib/Support/Unix/Program.inc +++ b/llvm/lib/Support/Unix/Program.inc @@ -63,11 +63,12 @@ ErrorOr sys::findProgramByName(StringRef Name, if (Name.find('/') != StringRef::npos) return std::string(Name); - if (Paths.empty()) { - SmallVector SearchPaths; - SplitString(std::getenv("PATH"), SearchPaths, ":"); - return findProgramByName(Name, SearchPaths); - } + SmallVector 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/llvm/lib/Support/Windows/Program.inc b/llvm/lib/Support/Windows/Program.inc index 6467ddd..5bd9021 100644 --- a/llvm/lib/Support/Windows/Program.inc +++ b/llvm/lib/Support/Windows/Program.inc @@ -62,7 +62,8 @@ ErrorOr sys::findProgramByName(StringRef Name, SmallVector 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 U16Result; DWORD Len = MAX_PATH; -- 2.7.4