[Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure

[STL_MSFT]

Summary

This is a reasonably non-intrusive change, which I've verified works for both x86 and x64 DevDiv-internal builds.

The idea is to change bool IsVS2017OrNewer into a 3-state ToolsetLayout VSLayout. Either a build is DevDiv-internal, released VS 2017 or newer, or released VS 2015 or older. When looking at the directory structure, if instead of "VC" we see "x86ret", "x86chk", "amd64ret", or "amd64chk", we recognize this as a DevDiv-internal build.

After we get past the directory structure validation, we use this knowledge to regenerate paths appropriately. llvmArchToDevDivInternalArch() knows how we use "i386" subdirectories, and MSVCToolChain::getSubDirectoryPath() uses that. It also knows that DevDiv-internal builds have an "inc" subdirectory instead of "include".

This may still not be the "right" fix in any sense, but I believe that it's non-intrusive in the sense that if the special directory names aren't found, no codepaths are affected. (ToolsetLayout::OlderVS and ToolsetLayout::VS2017OrNewer correspond to IsVS2017OrNewer being false or true, respectively.) I searched for all references to IsVS2017OrNewer, which are places where Clang cares about VS's directory structure, and the only one that isn't being patched is some logic to deal with cross-compilation. I'm fine with that not working for DevDiv-internal builds for the moment (we typically test the native compilers), so I added a comment.

Is there still time to get a patch like this into 5.0.0, or is it too late?

[hintonda]

Looks good, but I'll defer to the owners.

Minor nit...

lib/Driver/ToolChains/MSVC.cpp

148

Perhaps remove the else and grab the subdir name only once before doing comparisons.

[thakis]

This approach looks good to me.

Is there any chance we could have a test for this? See test/Driver for examples; use python bin\llvm-lit.py ../llvm/tools/clang/test/Driver/my_test.cpp to run a single test, ninja check-clang to run all of them. You need some unix binaries to run tests -- if you don't have those flying around, you can download gnuwin-7.zip from http://commondatastorage.googleapis.com/chromium-browser-clang/index.html?path=tools/ and add them to your %PATH%.

[STL_MSFT]

This addresses Don Hinton's feedback. It also unifies IsVS2017OrNewer and IsDevDivInternal into a 3-state enum class ToolsetLayout.

[STL_MSFT]

In D36860#845469, @thakis wrote:

This approach looks good to me.

Thanks!

Is there any chance we could have a test for this?

Unfortunately, testing this involves having a VS toolset with the DevDiv-internal directory structure. Preparing that is a laborious manual process, and requires installation of the Windows SDK (I sent repro instructions to cfe-dev). Even given a real installation of VS, I don't think I could write automated test code to extract a copy with the necessary directory structure (doing so would almost involve replicating the codepaths being touched here). But now that I know that this code exists and that Clang occasionally changes it, especially in response to new VS directory structures, I'll watch for new betas/RCs and verify them against our internal builds.

[thakis]

Many driver tests check in a basic representative directory structure (e.g. test/Driver/Inputs/basic_freebsd_tree/ and its many siblings).

But if you're happy with others breaking this code, I suppose having no tests is ok; I imagine most people won't use this code :-)

[STL_MSFT]

In D36860#846232, @thakis wrote:

Many driver tests check in a basic representative directory structure (e.g. test/Driver/Inputs/basic_freebsd_tree/ and its many siblings).

But if you're happy with others breaking this code, I suppose having no tests is ok; I imagine most people won't use this code :-)

I can't check in copies of cl.exe and link.exe (the former's version is inspected, so it can't just be a dummy file). I'm happy without upstream tests for this logic. In the long run, I hope that we can eliminate this internal directory structure and make it identical to the released directory structure.

lib/Driver/ToolChains/MSVC.h

std::string getSubDirectoryPath(SubDirectoryType Type,
                                   llvm::Triple::ArchType TargetArch) const;
 
   // Convenience overload.
   // Uses the current target arch.
   std::string getSubDirectoryPath(SubDirectoryType Type) const {
     return getSubDirectoryPath(Type, getArch());
   }
 
-  bool getIsVS2017OrNewer() const { return IsVS2017OrNewer; }
+  enum class ToolsetLayout {
+    OlderVS,
+    VS2017OrNewer,
+    DevDivInternal,
+  };
+  bool getIsVS2017OrNewer() const { return VSLayout == ToolsetLayout::VS2017OrNewer; }
 
   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;
 
void AddSystemIncludeWithSubfolder(const llvm::opt::ArgList &DriverArgs,
                                      const Twine &subfolder1,
                                      const Twine &subfolder2 = "",
                                      const Twine &subfolder3 = "") const;
 
   Tool *buildLinker() const override;
   Tool *buildAssembler() const override;
 private:
   std::string VCToolChainPath;
-  bool IsVS2017OrNewer = false;
+  ToolsetLayout VSLayout = ToolsetLayout::OlderVS;
   CudaInstallationDetector CudaInstallation;
 };
 
 } // end namespace toolchains
 } // end namespace driver
 } // end namespace clang
 
 #endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_MSVC_H

lib/Driver/ToolChains/MSVC.cpp

 
 // Defined below.
 // Forward declare this so there aren't too many things above the constructor.
 static bool getSystemRegistryString(const char *keyPath, const char *valueName,
                                     std::string &value, std::string *phValue);
 
 // Check various environment variables to try and find a toolchain.
 static bool findVCToolChainViaEnvironment(std::string &Path,
-                                          bool &IsVS2017OrNewer) {
+                                          MSVCToolChain::ToolsetLayout &VSLayout) {
   // These variables are typically set by vcvarsall.bat
   // when launching a developer command prompt.
   if (llvm::Optional<std::string> VCToolsInstallDir =
           llvm::sys::Process::GetEnv("VCToolsInstallDir")) {
     // This is only set by newer Visual Studios, and it leads straight to
     // the toolchain directory.
     Path = std::move(*VCToolsInstallDir);
-    IsVS2017OrNewer = true;
+    VSLayout = MSVCToolChain::ToolsetLayout::VS2017OrNewer;
     return true;
   }
   if (llvm::Optional<std::string> VCInstallDir =
           llvm::sys::Process::GetEnv("VCINSTALLDIR")) {
     // If the previous variable isn't set but this one is, then we've found
     // an older Visual Studio. This variable is set by newer Visual Studios too,
     // so this check has to appear second.
     // In older Visual Studios, the VC directory is the toolchain.
     Path = std::move(*VCInstallDir);
-    IsVS2017OrNewer = false;
+    VSLayout = MSVCToolChain::ToolsetLayout::OlderVS;
     return true;
   }
 
   // We couldn't find any VC environment variables. Let's walk through PATH and
   // see if it leads us to a VC toolchain bin directory. If it does, pick the
   // first one that we find.
   if (llvm::Optional<std::string> PathEnv =
           llvm::sys::Process::GetEnv("PATH")) {
for (llvm::StringRef PathEntry : PathEntries) {
       bool IsBin = llvm::sys::path::filename(TestPath).equals_lower("bin");
       if (!IsBin) {
         // Strip any architecture subdir like "amd64".
         TestPath = llvm::sys::path::parent_path(TestPath);
         IsBin = llvm::sys::path::filename(TestPath).equals_lower("bin");
       }
       if (IsBin) {
         llvm::StringRef ParentPath = llvm::sys::path::parent_path(TestPath);
-        if (llvm::sys::path::filename(ParentPath) == "VC") {
+        llvm::StringRef ParentFilename = llvm::sys::path::filename(ParentPath);
+        if (ParentFilename == "VC") {
           Path = ParentPath;
-          IsVS2017OrNewer = false;
+          VSLayout = MSVCToolChain::ToolsetLayout::OlderVS;
+          return true;
+        }
+        if (ParentFilename == "x86ret" || ParentFilename == "x86chk"
+          || ParentFilename == "amd64ret" || ParentFilename == "amd64chk") {
+          Path = ParentPath;
+          VSLayout = MSVCToolChain::ToolsetLayout::DevDivInternal;
           return true;
         }
 
       } else {
         // This could be a new (>=VS2017) toolchain. If it is, we should find
         // path components with these prefixes when walking backwards through
         // the path.
         // Note: empty strings match anything.
         llvm::StringRef ExpectedPrefixes[] = {"",     "Host",  "bin", "",
                                               "MSVC", "Tools", "VC"};
for (llvm::StringRef PathEntry : PathEntries) {
 
         // We've found a new toolchain!
         // Back up 3 times (/bin/Host/arch) to get the root path.
         llvm::StringRef ToolChainPath(PathEntry);
         for (int i = 0; i < 3; ++i)
           ToolChainPath = llvm::sys::path::parent_path(ToolChainPath);
 
         Path = ToolChainPath;
-        IsVS2017OrNewer = true;
+        VSLayout = MSVCToolChain::ToolsetLayout::VS2017OrNewer;
         return true;
       }
 
     NotAToolChain:
       continue;
     }
   }
   return false;
 }
 
 // Query the Setup Config server for installs, then pick the newest version
 // and find its default VC toolchain.
 // This is the preferred way to discover new Visual Studios, as they're no
 // longer listed in the registry.
 static bool findVCToolChainViaSetupConfig(std::string &Path,
-                                          bool &IsVS2017OrNewer) {
+                                          MSVCToolChain::ToolsetLayout &VSLayout) {
 #if !defined(USE_MSVC_SETUP_API)
   return false;
 #else
   // FIXME: This really should be done once in the top-level program's main
   // function, as it may have already been initialized with a different
   // threading model otherwise.
   llvm::sys::InitializeCOMRAII COM(llvm::sys::COMThreadingMode::SingleThreaded);
   HRESULT HR;
#else
 
   llvm::SmallString<256> ToolchainPath(VCRootPath);
   llvm::sys::path::append(ToolchainPath, "Tools", "MSVC",
                           ToolsVersionFile->get()->getBuffer().rtrim());
   if (!llvm::sys::fs::is_directory(ToolchainPath))
     return false;
 
   Path = ToolchainPath.str();
-  IsVS2017OrNewer = true;
+  VSLayout = MSVCToolChain::ToolsetLayout::VS2017OrNewer;
   return true;
 #endif
 }
 
 // Look in the registry for Visual Studio installs, and use that to get
 // a toolchain path. VS2017 and newer don't get added to the registry.
 // So if we find something here, we know that it's an older version.
 static bool findVCToolChainViaRegistry(std::string &Path,
-                                       bool &IsVS2017OrNewer) {
+                                       MSVCToolChain::ToolsetLayout &VSLayout) {
   std::string VSInstallPath;
   if (getSystemRegistryString(R"(SOFTWARE\Microsoft\VisualStudio\$VERSION)",
                               "InstallDir", VSInstallPath, nullptr) ||
       getSystemRegistryString(R"(SOFTWARE\Microsoft\VCExpress\$VERSION)",
                               "InstallDir", VSInstallPath, nullptr)) {
     if (!VSInstallPath.empty()) {
       llvm::SmallString<256> VCPath(llvm::StringRef(
           VSInstallPath.c_str(), VSInstallPath.find(R"(\Common7\IDE)")));
       llvm::sys::path::append(VCPath, "VC");
 
       Path = VCPath.str();
-      IsVS2017OrNewer = false;
+      VSLayout = MSVCToolChain::ToolsetLayout::OlderVS;
       return true;
     }
   }
   return false;
 }
 
 // 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,
if (Linker.equals_lower("link")) {
     linkPath = FindVisualStudioExecutable(TC, "link.exe");
 
 #ifdef USE_WIN32
     // When cross-compiling with VS2017 or newer, link.exe expects to have
     // its containing bin directory at the top of PATH, followed by the
     // native target bin directory.
     // e.g. when compiling for x86 on an x64 host, PATH should start with:
     // /bin/HostX64/x86;/bin/HostX64/x64
+    // This doesn't attempt to handle ToolsetLayout::DevDivInternal.
     if (TC.getIsVS2017OrNewer() &&
         llvm::Triple(llvm::sys::getProcessTriple()).getArch() != TC.getArch()) {
       auto HostArch = llvm::Triple(llvm::sys::getProcessTriple()).getArch();
 
       auto EnvBlockWide =
           std::unique_ptr<wchar_t[], decltype(&FreeEnvironmentStringsW)>(
               GetEnvironmentStringsW(), FreeEnvironmentStringsW);
       if (!EnvBlockWide)
MSVCToolChain::MSVCToolChain(const Driver &D, const llvm::Triple &Triple,
   getProgramPaths().push_back(getDriver().getInstalledDir());
   if (getDriver().getInstalledDir() != getDriver().Dir)
     getProgramPaths().push_back(getDriver().Dir);
 
   // Check the environment first, since that's probably the user telling us
   // what they want to use.
   // Failing that, just try to find the newest Visual Studio version we can
   // and use its default VC toolchain.
-  findVCToolChainViaEnvironment(VCToolChainPath, IsVS2017OrNewer) ||
-      findVCToolChainViaSetupConfig(VCToolChainPath, IsVS2017OrNewer) ||
-      findVCToolChainViaRegistry(VCToolChainPath, IsVS2017OrNewer);
+  findVCToolChainViaEnvironment(VCToolChainPath, VSLayout) ||
+      findVCToolChainViaSetupConfig(VCToolChainPath, VSLayout) ||
+      findVCToolChainViaRegistry(VCToolChainPath, VSLayout);
 }
 
 Tool *MSVCToolChain::buildLinker() const {
   if (VCToolChainPath.empty())
     getDriver().Diag(clang::diag::warn_drv_msvc_not_found);
   return new tools::visualstudio::Linker(*this);
 }
 
case ArchType::x86_64:
     return "amd64";
   case ArchType::arm:
     return "arm";
   default:
     return "";
   }
 }
 
+// Similar to the above function, but for DevDiv internal builds.
+static const char *llvmArchToDevDivInternalArch(llvm::Triple::ArchType Arch) {
+  using ArchType = llvm::Triple::ArchType;
+  switch (Arch) {
+  case ArchType::x86:
+    return "i386";
+  case ArchType::x86_64:
+    return "amd64";
+  case ArchType::arm:
+    return "arm";
+  default:
+    return "";
+  }
+}
+
 // Get the path to a specific subdirectory in the current toolchain for
 // a given target architecture.
 // VS2017 changed the VC toolchain layout, so this should be used instead
 // of hardcoding paths.
 std::string
 MSVCToolChain::getSubDirectoryPath(SubDirectoryType Type,
                                    llvm::Triple::ArchType TargetArch) const {
+  const char *SubdirName;
+  const char *IncludeName;
+  switch (VSLayout) {
+  case ToolsetLayout::OlderVS:
+    SubdirName = llvmArchToLegacyVCArch(TargetArch);
+    IncludeName = "include";
+    break;
+  case ToolsetLayout::VS2017OrNewer:
+    SubdirName = llvmArchToWindowsSDKArch(TargetArch);
+    IncludeName = "include";
+    break;
+  case ToolsetLayout::DevDivInternal:
+    SubdirName = llvmArchToDevDivInternalArch(TargetArch);
+    IncludeName = "inc";
+    break;
+  }
+
   llvm::SmallString<256> Path(VCToolChainPath);
   switch (Type) {
   case SubDirectoryType::Bin:
-    if (IsVS2017OrNewer) {
-      bool HostIsX64 =
+    if (VSLayout == ToolsetLayout::VS2017OrNewer) {
+      const bool HostIsX64 =
           llvm::Triple(llvm::sys::getProcessTriple()).isArch64Bit();
-      llvm::sys::path::append(Path, "bin", (HostIsX64 ? "HostX64" : "HostX86"),
-                              llvmArchToWindowsSDKArch(TargetArch));
-
-    } else {
-      llvm::sys::path::append(Path, "bin", llvmArchToLegacyVCArch(TargetArch));
+      const char *const HostName = HostIsX64 ? "HostX64" : "HostX86";
+      llvm::sys::path::append(Path, "bin", HostName, SubdirName);
+    } else { // OlderVS or DevDivInternal
+      llvm::sys::path::append(Path, "bin", SubdirName);
     }
     break;
   case SubDirectoryType::Include:
-    llvm::sys::path::append(Path, "include");
+    llvm::sys::path::append(Path, IncludeName);
     break;
   case SubDirectoryType::Lib:
-    llvm::sys::path::append(
-        Path, "lib", IsVS2017OrNewer ? llvmArchToWindowsSDKArch(TargetArch)
-                                     : llvmArchToLegacyVCArch(TargetArch));
+    llvm::sys::path::append(Path, "lib", SubdirName);
     break;
   }
   return Path.str();
 }
 
 #ifdef USE_WIN32
 static bool readFullStringValue(HKEY hkey, const char *valueName,
                                 std::string &value) {