Fix suggested review issues.
http://reviews.llvm.org/D5845
Files:
lib/Driver/ToolChains.h
lib/Driver/Tools.cpp
lib/Driver/WindowsToolChain.cpp
Index: lib/Driver/ToolChains.h
===================================================================
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -745,7 +745,15 @@
AddClangCXXStdlibIncludeArgs(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const override;
+ bool getWindowsSDKDir(std::string &path, int &major, int &minor) const;
+ bool getVisualStudioDir(std::string &path) const;
+
protected:
+ void AddSystemIncludeWithSubfolder(const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args,
+ const std::string &folder,
+ const char *subfolder) const;
+
Tool *buildLinker() const override;
Tool *buildAssembler() const override;
};
Index: lib/Driver/Tools.cpp
===================================================================
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -7806,6 +7806,50 @@
CmdArgs.push_back(Args.MakeArgString(LibSanitizer));
}
+// Try to find Exe from a Visual Studio distribution. This first tries to find
+// an installed copy of Visual Studio and, failing that, looks in the PATH,
+// making sure that whatever executable that's found is not a same-named exe
+// from clang itself to prevent clang from falling back to itself.
+static std::string FindVisualStudioExecutable(const ToolChain &TC,
+ const char *Exe,
+ const char *ClangProgramPath) {
+ // Since a particular Visual Studio executable is tied to a set of system
+ // includes and libraries, first try to use the same location that we use for
+ // the include paths to ensure that a consistent build environment is located.
+ const toolchains::Windows &WTC = static_cast<const toolchains::Windows &>(TC);
+ std::string visualStudioDir;
+ if (WTC.getVisualStudioDir(visualStudioDir)) {
+ SmallString<128> VSDir(visualStudioDir);
+ llvm::sys::path::append(VSDir, "VC\\bin");
+ llvm::sys::path::append(VSDir, Exe);
+ if (llvm::sys::fs::can_execute(VSDir.c_str()))
+ return VSDir.str();
+ }
+
+ // If it could not be found, we're already probably broken, but try to
+ // fallback to PATH anyway.
+ llvm::Optional<std::string> OptPath = llvm::sys::Process::GetEnv("PATH");
+ if (!OptPath.hasValue())
+ return Exe;
+
+ const char EnvPathSeparatorStr[] = {llvm::sys::EnvPathSeparator, '\0'};
+ SmallVector<StringRef, 8> PathSegments;
+ llvm::SplitString(OptPath.getValue(), PathSegments, EnvPathSeparatorStr);
+
+ for (StringRef PathSegment : PathSegments) {
+ if (PathSegment.empty())
+ continue;
+
+ SmallString<128> FilePath(PathSegment);
+ llvm::sys::path::append(FilePath, Exe);
+ if (llvm::sys::fs::can_execute(FilePath.c_str()) &&
+ !llvm::sys::fs::equivalent(FilePath.c_str(), ClangProgramPath))
+ return FilePath.str();
+ }
+
+ return Exe;
+}
+
void visualstudio::Link::ConstructJob(Compilation &C, const JobAction &JA,
const InputInfo &Output,
const InputInfoList &Inputs,
@@ -7891,8 +7935,11 @@
A.renderAsInput(Args, CmdArgs);
}
- const char *Exec =
- Args.MakeArgString(getToolChain().GetProgramPath("link.exe"));
+ // It's not sufficient to just use link from the program PATH, because other
+ // environments like GnuWin32 install their own link.exe which may come first.
+ llvm::SmallString<128> linkPath(FindVisualStudioExecutable(
+ getToolChain(), "link.exe", C.getDriver().getClangProgramPath()));
+ const char *Exec = Args.MakeArgString(linkPath);
C.addCommand(llvm::make_unique<Command>(JA, *this, Exec, CmdArgs));
}
@@ -7904,35 +7951,6 @@
C.addCommand(GetCommand(C, JA, Output, Inputs, Args, LinkingOutput));
}
-// Try to find FallbackName on PATH that is not identical to ClangProgramPath.
-// If one cannot be found, return FallbackName.
-// We do this special search to prevent clang-cl from falling back onto itself
-// if it's available as cl.exe on the path.
-static std::string FindFallback(const char *FallbackName,
- const char *ClangProgramPath) {
- llvm::Optional<std::string> OptPath = llvm::sys::Process::GetEnv("PATH");
- if (!OptPath.hasValue())
- return FallbackName;
-
- const char EnvPathSeparatorStr[] = {llvm::sys::EnvPathSeparator, '\0'};
- SmallVector<StringRef, 8> PathSegments;
- llvm::SplitString(OptPath.getValue(), PathSegments, EnvPathSeparatorStr);
-
- for (size_t i = 0, e = PathSegments.size(); i != e; ++i) {
- StringRef PathSegment = PathSegments[i];
- if (PathSegment.empty())
- continue;
-
- SmallString<128> FilePath(PathSegment);
- llvm::sys::path::append(FilePath, FallbackName);
- if (llvm::sys::fs::can_execute(Twine(FilePath)) &&
- !llvm::sys::fs::equivalent(Twine(FilePath), ClangProgramPath))
- return FilePath.str();
- }
-
- return FallbackName;
-}
-
std::unique_ptr<Command> visualstudio::Compile::GetCommand(
Compilation &C, const JobAction &JA, const InputInfo &Output,
const InputInfoList &Inputs, const ArgList &Args,
@@ -8014,7 +8032,8 @@
CmdArgs.push_back(Fo);
const Driver &D = getToolChain().getDriver();
- std::string Exec = FindFallback("cl.exe", D.getClangProgramPath());
+ std::string Exec = FindVisualStudioExecutable(getToolChain(), "cl.exe",
+ D.getClangProgramPath());
return llvm::make_unique<Command>(JA, *this, Args.MakeArgString(Exec),
CmdArgs);
}
Index: lib/Driver/WindowsToolChain.cpp
===================================================================
--- lib/Driver/WindowsToolChain.cpp
+++ lib/Driver/WindowsToolChain.cpp
@@ -18,7 +18,6 @@
#include "llvm/Option/Arg.h"
#include "llvm/Option/ArgList.h"
#include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/Path.h"
// Include the necessary headers to interface with the Windows registry and
// environment.
@@ -77,61 +76,65 @@
return getArch() == llvm::Triple::x86_64;
}
+#ifdef USE_WIN32
+static bool readFullStringValue(HKEY hkey, const char *valueName,
+ std::string &value) {
+ // FIXME: We should be using the W versions of the registry functions, but
+ // doing so requires UTF8 / UTF16 conversions similar to how we handle command
+ // line arguments. The UTF8 conversion functions are not exposed publicly
+ // from LLVM though, so in order to do this we will probably need to create
+ // a registry abstraction in LLVMSupport that is Windows only.
+ DWORD result = 0;
+ DWORD valueSize = 0;
+ DWORD type = 0;
+ // First just query for the required size.
+ result = RegQueryValueEx(hkey, valueName, NULL, &type, NULL, &valueSize);
+ if (result != ERROR_SUCCESS || type != REG_SZ)
+ return false;
+ std::vector<BYTE> buffer(valueSize);
+ result = RegQueryValueEx(hkey, valueName, NULL, NULL, &buffer[0], &valueSize);
+ if (result == ERROR_SUCCESS)
+ value.assign(reinterpret_cast<const char *>(buffer.data()));
+ return result;
+}
+#endif
+
/// \brief Read registry string.
/// This also supports a means to look for high-versioned keys by use
/// of a $VERSION placeholder in the key path.
/// $VERSION in the key path is a placeholder for the version number,
/// causing the highest value path to be searched for and used.
-/// I.e. "HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\VisualStudio\\$VERSION".
-/// There can be additional characters in the component. Only the numberic
-/// characters are compared.
+/// I.e. "SOFTWARE\\Microsoft\\VisualStudio\\$VERSION".
+/// There can be additional characters in the component. Only the numeric
+/// characters are compared. This function only searches HKLM.
static bool getSystemRegistryString(const char *keyPath, const char *valueName,
- char *value, size_t maxLength) {
+ std::string &value, std::string *phValue) {
#ifndef USE_WIN32
return false;
#else
- HKEY hRootKey = NULL;
+ HKEY hRootKey = HKEY_LOCAL_MACHINE;
HKEY hKey = NULL;
- const char* subKey = NULL;
- DWORD valueType;
- DWORD valueSize = maxLength - 1;
+ DWORD valueSize = 0;
long lResult;
bool returnValue = false;
- if (strncmp(keyPath, "HKEY_CLASSES_ROOT\\", 18) == 0) {
- hRootKey = HKEY_CLASSES_ROOT;
- subKey = keyPath + 18;
- } else if (strncmp(keyPath, "HKEY_USERS\\", 11) == 0) {
- hRootKey = HKEY_USERS;
- subKey = keyPath + 11;
- } else if (strncmp(keyPath, "HKEY_LOCAL_MACHINE\\", 19) == 0) {
- hRootKey = HKEY_LOCAL_MACHINE;
- subKey = keyPath + 19;
- } else if (strncmp(keyPath, "HKEY_CURRENT_USER\\", 18) == 0) {
- hRootKey = HKEY_CURRENT_USER;
- subKey = keyPath + 18;
- } else {
- return false;
- }
-
- const char *placeHolder = strstr(subKey, "$VERSION");
- char bestName[256];
- bestName[0] = '\0';
+ const char *placeHolder = strstr(keyPath, "$VERSION");
+ std::string bestName;
// If we have a $VERSION placeholder, do the highest-version search.
if (placeHolder) {
const char *keyEnd = placeHolder - 1;
const char *nextKey = placeHolder;
// Find end of previous key.
- while ((keyEnd > subKey) && (*keyEnd != '\\'))
+ while ((keyEnd > keyPath) && (*keyEnd != '\\'))
keyEnd--;
// Find end of key containing $VERSION.
while (*nextKey && (*nextKey != '\\'))
nextKey++;
- size_t partialKeyLength = keyEnd - subKey;
+ size_t partialKeyLength = keyEnd - keyPath;
char partialKey[256];
if (partialKeyLength > sizeof(partialKey))
partialKeyLength = sizeof(partialKey);
- strncpy(partialKey, subKey, partialKeyLength);
+ strncpy(partialKey, keyPath, partialKeyLength);
partialKey[partialKeyLength] = '\0';
HKEY hTopKey = NULL;
lResult = RegOpenKeyEx(hRootKey, partialKey, 0, KEY_READ | KEY_WOW64_32KEY,
@@ -158,18 +161,18 @@
if (dvalue > bestValue) {
// Test that InstallDir is indeed there before keeping this index.
// Open the chosen key path remainder.
- strcpy(bestName, keyName);
+ bestName = keyName;
// Append rest of key.
- strncat(bestName, nextKey, sizeof(bestName) - 1);
- bestName[sizeof(bestName) - 1] = '\0';
- lResult = RegOpenKeyEx(hTopKey, bestName, 0,
+ bestName.append(nextKey);
+ lResult = RegOpenKeyEx(hTopKey, bestName.c_str(), 0,
KEY_READ | KEY_WOW64_32KEY, &hKey);
if (lResult == ERROR_SUCCESS) {
- lResult = RegQueryValueEx(hKey, valueName, NULL, &valueType,
- (LPBYTE)value, &valueSize);
+ lResult = readFullStringValue(hKey, valueName, value);
if (lResult == ERROR_SUCCESS) {
bestIndex = (int)index;
bestValue = dvalue;
+ if (phValue)
+ *phValue = bestName;
returnValue = true;
}
RegCloseKey(hKey);
@@ -180,84 +183,76 @@
RegCloseKey(hTopKey);
}
} else {
- lResult = RegOpenKeyEx(hRootKey, subKey, 0, KEY_READ | KEY_WOW64_32KEY,
- &hKey);
+ lResult =
+ RegOpenKeyEx(hRootKey, keyPath, 0, KEY_READ | KEY_WOW64_32KEY, &hKey);
if (lResult == ERROR_SUCCESS) {
- lResult = RegQueryValueEx(hKey, valueName, NULL, &valueType,
- (LPBYTE)value, &valueSize);
+ lResult = readFullStringValue(hKey, valueName, value);
if (lResult == ERROR_SUCCESS)
returnValue = true;
+ if (phValue)
+ phValue->clear();
RegCloseKey(hKey);
}
}
return returnValue;
#endif // USE_WIN32
}
/// \brief Get Windows SDK installation directory.
-static bool getWindowsSDKDir(std::string &path) {
- char windowsSDKInstallDir[256];
+bool Windows::getWindowsSDKDir(std::string &path, int &major,
+ int &minor) const {
+ std::string sdkVersion;
// Try the Windows registry.
bool hasSDKDir = getSystemRegistryString(
- "HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Microsoft SDKs\\Windows\\$VERSION",
- "InstallationFolder",
- windowsSDKInstallDir,
- sizeof(windowsSDKInstallDir) - 1);
- // If we have both vc80 and vc90, pick version we were compiled with.
- if (hasSDKDir && windowsSDKInstallDir[0]) {
- path = windowsSDKInstallDir;
- return true;
- }
- return false;
+ "SOFTWARE\\Microsoft\\Microsoft SDKs\\Windows\\$VERSION",
+ "InstallationFolder", path, &sdkVersion);
+ if (!sdkVersion.empty())
+ ::sscanf(sdkVersion.c_str(), "v%d.%d", &major, &minor);
+ return hasSDKDir && !path.empty();
}
// Get Visual Studio installation directory.
-static bool getVisualStudioDir(std::string &path) {
+bool Windows::getVisualStudioDir(std::string &path) const {
// First check the environment variables that vsvars32.bat sets.
- const char* vcinstalldir = getenv("VCINSTALLDIR");
+ const char *vcinstalldir = getenv("VCINSTALLDIR");
if (vcinstalldir) {
- char *p = const_cast<char *>(strstr(vcinstalldir, "\\VC"));
- if (p)
- *p = '\0';
path = vcinstalldir;
+ path = path.substr(0, path.find("\\VC"));
return true;
}
- char vsIDEInstallDir[256];
- char vsExpressIDEInstallDir[256];
+ std::string vsIDEInstallDir;
+ std::string vsExpressIDEInstallDir;
// Then try the windows registry.
- bool hasVCDir = getSystemRegistryString(
- "HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\VisualStudio\\$VERSION",
- "InstallDir", vsIDEInstallDir, sizeof(vsIDEInstallDir) - 1);
- bool hasVCExpressDir = getSystemRegistryString(
- "HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\VCExpress\\$VERSION",
- "InstallDir", vsExpressIDEInstallDir, sizeof(vsExpressIDEInstallDir) - 1);
- // If we have both vc80 and vc90, pick version we were compiled with.
- if (hasVCDir && vsIDEInstallDir[0]) {
- char *p = (char*)strstr(vsIDEInstallDir, "\\Common7\\IDE");
- if (p)
- *p = '\0';
- path = vsIDEInstallDir;
+ bool hasVCDir =
+ getSystemRegistryString("SOFTWARE\\Microsoft\\VisualStudio\\$VERSION",
+ "InstallDir", vsIDEInstallDir, nullptr);
+ if (hasVCDir && !vsIDEInstallDir.empty()) {
+ path = vsIDEInstallDir.substr(0, vsIDEInstallDir.find("\\Common7\\IDE"));
return true;
}
- if (hasVCExpressDir && vsExpressIDEInstallDir[0]) {
- char *p = (char*)strstr(vsExpressIDEInstallDir, "\\Common7\\IDE");
- if (p)
- *p = '\0';
- path = vsExpressIDEInstallDir;
+ bool hasVCExpressDir =
+ getSystemRegistryString("SOFTWARE\\Microsoft\\VCExpress\\$VERSION",
+ "InstallDir", vsExpressIDEInstallDir, nullptr);
+ if (hasVCExpressDir && !vsExpressIDEInstallDir.empty()) {
+ path = vsExpressIDEInstallDir.substr(
+ 0, vsIDEInstallDir.find("\\Common7\\IDE"));
return true;
}
// Try the environment.
+ const char *vs120comntools = getenv("VS120COMNTOOLS");
const char *vs100comntools = getenv("VS100COMNTOOLS");
const char *vs90comntools = getenv("VS90COMNTOOLS");
const char *vs80comntools = getenv("VS80COMNTOOLS");
const char *vscomntools = nullptr;
// Find any version we can
- if (vs100comntools)
+ if (vs120comntools)
+ vscomntools = vs120comntools;
+ else if (vs100comntools)
vscomntools = vs100comntools;
else if (vs90comntools)
vscomntools = vs90comntools;
@@ -272,6 +267,15 @@
return false;
}
+void Windows::AddSystemIncludeWithSubfolder(const ArgList &DriverArgs,
+ ArgStringList &CC1Args,
+ const std::string &folder,
+ const char *subfolder) const {
+ llvm::SmallString<128> path(folder);
+ llvm::sys::path::append(path, subfolder);
+ addSystemInclude(DriverArgs, CC1Args, path.str());
+}
+
void Windows::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
ArgStringList &CC1Args) const {
if (DriverArgs.hasArg(options::OPT_nostdinc))
@@ -298,23 +302,28 @@
}
std::string VSDir;
- std::string WindowsSDKDir;
// When built with access to the proper Windows APIs, try to actually find
// the correct include paths first.
if (getVisualStudioDir(VSDir)) {
- SmallString<128> P;
- P = VSDir;
- llvm::sys::path::append(P, "VC\\include");
- addSystemInclude(DriverArgs, CC1Args, P.str());
- if (getWindowsSDKDir(WindowsSDKDir)) {
- P = WindowsSDKDir;
- llvm::sys::path::append(P, "include");
- addSystemInclude(DriverArgs, CC1Args, P.str());
+ AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, VSDir, "VC\\include");
+
+ std::string WindowsSDKDir;
+ int major, minor;
+ if (getWindowsSDKDir(WindowsSDKDir, major, minor)) {
+ if (major >= 8) {
+ AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
+ "include\\shared");
+ AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
+ "include\\um");
+ AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
+ "include\\winrt");
+ } else {
+ AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
+ "include");
+ }
} else {
- P = VSDir;
- llvm::sys::path::append(P, "VC\\PlatformSDK\\Include");
- addSystemInclude(DriverArgs, CC1Args, P.str());
+ addSystemInclude(DriverArgs, CC1Args, VSDir);
}
return;
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
