Improve Windows toolchain support for non-standard environments.

[zturner]

Summary

Typically clang finds Visual Studio by the user explicitly setting
up a Visual Studio environment via vcvarsall.  But we still try to
behave intelligently and fallback to different methods of finding
Visual Studio when this is not done.  This patch improves various
fallback codepaths to make Visual Studio locating more robust.

Specifically, this patch:

* Adds support for searching environment variables for VS 12.0
* Correctly locates include folders for Windows SDK 8.x (this was
  previously broken, and would cause clang to error).  Windows
  SDK 8.x segregates its includes into \shared, \um, and \winrt.
* Prefers locating link.exe in the same location as cl.exe.  This
  is helpful in case another link.exe is in the path earlier than
  Visual Studio (e.g. GnuWin32)
* Minor cleanup in the registry reading code to make it more
  robust in the presence of long pathnames.

[zturner]

Fix an issue with a stack smash.

There is still an outstanding issue remaining after this patch, which is that link cannot locate libraries. I think we can address this in a followup patch by adding some library paths to the link line similar to how we already do for system include paths that we pass to cl.

[hans]

lib/Driver/Tools.cpp

7820

I'm not a big fan of our code for finding a msvc installation when vcvars.bat hasn't been run. I think that if the user wants to run clang-cl as a drop-in replacement for cl.exe, it should be dropped into the same environment.

But since we have it, we might as well make it work better. The TODO sounds reasonable, as long as it still checks PATH first, before going off and looking in other places.

Also, will cl.exe and link.exe etc. actually run if they're not on PATH? I recall having problems executing them without being on PATH because they failed to load some dll.

Style nit: FIXME without name is more common than TODO, and the |text| syntax isn't used.

7939

nit: it's

7942

smartPath?

[zturner]

lib/Driver/Tools.cpp

7820

Consider the case where you have a script which invokes clang many times with different sets of options each time. Sometimes m32, sometimes m64, for example. In this case it's cumbersome and difficult for the script to configure the environment beforehand, because there's no way to run a batch file "in process". If you try to run vcvars from a script it will just modify the environment of the shell process that is spawned to run the batch file.

I'm not a huge fan of it either though, and I actually think an ideal solution is to allow clang to accept a command line option that will just tell it the path to the root of the Visual Studio installation to use, and clang can just set up a consistent environment prior to invoking cl or link.

7942

Heh. Probably not the best name in the world. I'll fix that.

[zturner]

Fix review issues.

[zturner]

+majnemer

Also prefer finding the clang cl.exe fallback location by using the visual studio directory which is being used for includes instead of looking in the PATH first.

VC system headers are tied to a particular cl.exe and should not be mixed. Previously, we were looking for cl.exe in the PATH, and using a different algorithm to find the VC include directories. Now, the two methods use the same algorithm. Or at the very least, they try to. This also makes it possible for clang to fallback to cl even when cl is not in the PATH.

[hans]

Using getVisualStudioDir() for both headers and binaries seems like a big win!

lib/Driver/Tools.cpp

7819

Shouldn't it be searching for FallbackName rather than hard-coding cl.exe?

7820

I think VSDir can be passed to can_execute directly and the Twine gets constructed implicitly.

7825

It seems the code above is now doing what this fixme is suggesting :)

7845

I realize you didn't add them, but again I think we can drop the Twines. (hopefully)

7942

Maybe we should repurpose FindFallback to a function that finds vs executables and just pass in link.exe instead of doing this dance? We could then cut down on the comment too, because it would be obvious that the link.exe and cl.exe are from the same visual studio.

[zturner]

Fix suggested review issues.

[hans]

lgtm

lib/Driver/Tools.cpp

7825

nit: I'd just use VSDir.str(). Same for FilePath below.

[zturner]

This has been submitted

lib/Driver/Tools.cpp

7825

I did use VSDir.str(), did you mean you'd just use VSDir? Won't compile, there's no implicit conversion from SmallString to Twine.

lib/Driver/ToolChains.h

public:
 
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
                             llvm::opt::ArgStringList &CC1Args) const override;
   void
   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;
 };
 
 
 class LLVM_LIBRARY_VISIBILITY XCore : public ToolChain {
 public:
   XCore(const Driver &D, const llvm::Triple &Triple,

lib/Driver/Tools.cpp

static void addSanitizerRTWindows(const ToolChain &TC, const ArgList &Args,
                                   ArgStringList &CmdArgs,
                                   StringRef RTName) {
   SmallString<128> LibSanitizer(getCompilerRTLibDir(TC));
   llvm::sys::path::append(LibSanitizer,
                           Twine("clang_rt.") + RTName + ".lib");
   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,
                                       const ArgList &Args,
                                       const char *LinkingOutput) const {
   ArgStringList CmdArgs;
 
   if (Output.isFilename()) {
if (A.getOption().matches(options::OPT_l)) {
       continue;
     }
 
     // Otherwise, this is some other kind of linker input option like -Wl, -z,
     // or -L. Render it, even if MSVC doesn't understand it.
     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));
 }
 
 void visualstudio::Compile::ConstructJob(Compilation &C, const JobAction &JA,
                                          const InputInfo &Output,
                                          const InputInfoList &Inputs,
                                          const ArgList &Args,
                                          const char *LinkingOutput) const {
   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,
     const char *LinkingOutput) const {
   ArgStringList CmdArgs;
   CmdArgs.push_back("/nologo");
   CmdArgs.push_back("/c"); // Compile only.
   CmdArgs.push_back("/W0"); // No warnings.
std::unique_ptr<Command> visualstudio::Compile::GetCommand(
 
   // Output filename.
   assert(Output.getType() == types::TY_Object);
   const char *Fo = Args.MakeArgString(std::string("/Fo") +
                                       Output.getFilename());
   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);
 }
 
 
 /// XCore Tools
 // We pass assemble and link construction to the xcc tool.
 

lib/Driver/WindowsToolChain.cpp

 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Config/llvm-config.h"
 #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.
 #if defined(LLVM_ON_WIN32)
 #define USE_WIN32
 #endif
 
 #ifdef USE_WIN32

 bool Windows::isPIEDefault() const {
   return false;
 }
 
 bool Windows::isPICDefaultForced() const {
   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,
                            &hTopKey);
     if (lResult == ERROR_SUCCESS) {
       char keyName[256];
       int bestIndex = -1;
       double bestValue = 0.0;
if (lResult == ERROR_SUCCESS) {
           ep++;
         char numBuf[32];
         strncpy(numBuf, sp, sizeof(numBuf) - 1);
         numBuf[sizeof(numBuf) - 1] = '\0';
         double dvalue = strtod(numBuf, NULL);
         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);
           }
         }
         size = sizeof(keyName) - 1;
       }
       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;
   else if (vs80comntools)
     vscomntools = vs80comntools;
 
   if (vscomntools && *vscomntools) {
     const char *p = strstr(vscomntools, "\\Common7\\Tools");
     path = p ? std::string(vscomntools, p) : vscomntools;
     return true;
   }
   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))
     return;
 
   if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
     SmallString<128> P(getDriver().ResourceDir);
     llvm::sys::path::append(P, "include");
StringRef(cl_include_dir)
         .split(Dirs, ";", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
     for (StringRef Dir : Dirs)
       addSystemInclude(DriverArgs, CC1Args, Dir);
     if (!Dirs.empty())
       return;
   }
 
   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 {
-      P = VSDir;
-      llvm::sys::path::append(P, "VC\\PlatformSDK\\Include");
-      addSystemInclude(DriverArgs, CC1Args, P.str());
+        AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
+                                      "include");
+      }
+    } else {
+      addSystemInclude(DriverArgs, CC1Args, VSDir);
     }
     return;
   }
 
   // As a fallback, select default install paths.
   // FIXME: Don't guess drives and paths like this on Windows.
   const StringRef Paths[] = {
     "C:/Program Files/Microsoft Visual Studio 10.0/VC/include",