Make a good guess about where MSVC and Windows SDK libraries are for linking.

[zturner]

Summary

When a user doesn't run vcvarsall, clang doesn't know where system
libraries are located.  This patch causes clang to still use the
configured environment variables when present, but when they are
not present, to make a (very good) guess.

[zturner]

Note that after applying both this patch and my previous patch, I can now compile and link an executable without first running vcvarsall, and the environment that it sets up is identical to that which would have been created by first running vcvarsall.

[hans]

lgtm

lib/Driver/ToolChains.h

749 ↗

(On Diff #15153)

I'd find it more natural to put the path parameter last, since it's an output.

lib/Driver/WindowsToolChain.cpp

215 ↗

(On Diff #15153)

I think \brief only works for comments starting with /, and one-line / comments are treated as brief automatically. And if we want to use doxygen comments for these functions, they should probably go in the header file.

(Phabricator is formatting my slashes. Where it says /, I mean three slashes)

226 ↗

(On Diff #15153)

I have no idea if this is correct, but I guess it looks reasonable :)

[zturner]

lib/Driver/WindowsToolChain.cpp

226 ↗

(On Diff #15153)

I found someone with an installation of the 7.0 SDK, and this is what he said his looked like. So it seems correct.

[zturner]

• Rebased on top of the fixed include file / executable search patch patch.
• updated patch to correctly use the target triple instead of looking directly at the m32 / m64 command line options
• Correctly handle ARM lib paths.
• Fix incorrect lib path issue pointed out by Aaron Ballman related to Windows SDK 7.x amd64 folder.

[rnk]

lib/Driver/Tools.cpp

7853 ↗

(On Diff #15217)

Close the parenthetical before the comma

7856 ↗

(On Diff #15217)

The style guide says to use upper case local variables. Some code is inconsistent, but this code uses mostly upper case locals, so let's lean that way here.

7857 ↗

(On Diff #15217)

Hm, that seems to be a common pattern elsewhere, so OK.

Our style guide currently says to add const before auto if you know it's const.

7861 ↗

(On Diff #15217)

Can we shorten this up a bunch with a helper that takes the arch and returns a string for where to look?

I think this would be cleaner if you do a single call to append, like so:

StringRef SubDir = getVCArchSubDir(WTC.getArch());
llvm::sys::path::append(VSDir, "VC", "lib", SubDir);

Appending the empty string is a no-op, so the basic x86 case can return "", but the ppc case will have to return false or something else weird.

I expect this helper will also apply to VC/bin in the other patch.

It's also nice to avoid hardcoded backslashes so that we can eventually support cross-compilation.

7878 ↗

(On Diff #15217)

WindowsSdkLibPath

lib/Driver/WindowsToolChain.cpp

230 ↗

(On Diff #15217)

A similar helper like getWindowsSDKArchSubDir(WTC.getArch(), sdkMajor) might help here.

251 ↗

(On Diff #15217)

You can use a range for loop over the local array.

[zturner]

Updated with previous comments. After offline discussion, replicating the switch statements instead of making a helper function was agreed upon as the best approach.

[zturner]

Rebase against ToT, someone renamed toolchains::Windows to toolchains::MSVCToolChain

[zturner]

Hopefully this fixes the remainder of the issues.

[hans]

lgtm2

lib/Driver/MSVCToolChain.cpp

237 ↗

(On Diff #15270)

Clearing this seems redundant?

243 ↗

(On Diff #15270)

nit: s/an/a/, s/names/name/ ?

247 ↗

(On Diff #15270)

is winv6.3 newer than win8? (i have no idea, just noticed the number is lower)

[zturner]

winv6.3 ins windows 8.1

[zturner]

Closed by commit rL220425 (authored by @zturner).

cfe/trunk/lib/Driver/MSVCToolChain.cpp

bool MSVCToolChain::getWindowsSDKDir(std::string &path, int &major,
   bool hasSDKDir = getSystemRegistryString(
       "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();
 }
 
+// Gets the library path required to link against the Windows SDK.
+bool MSVCToolChain::getWindowsSDKLibraryPath(std::string &path) const {
+  std::string sdkPath;
+  int sdkMajor = 0;
+  int sdkMinor = 0;
+
+  path.clear();
+  if (!getWindowsSDKDir(sdkPath, sdkMajor, sdkMinor))
+    return false;
+
+  llvm::SmallString<128> libPath(sdkPath);
+  llvm::sys::path::append(libPath, "Lib");
+  if (sdkMajor <= 7) {
+    switch (getArch()) {
+    // In Windows SDK 7.x, x86 libraries are directly in the Lib folder.
+    case llvm::Triple::x86:
+      break;
+    case llvm::Triple::x86_64:
+      llvm::sys::path::append(libPath, "x64");
+      break;
+    case llvm::Triple::arm:
+      // It is not necessary to link against Windows SDK 7.x when targeting ARM.
+      return false;
+    default:
+      return false;
+    }
+  } else {
+    // Windows SDK 8.x installs libraries in a folder whose names depend on the
+    // version of the OS you're targeting.  By default choose the newest, which
+    // usually corresponds to the version of the OS you've installed the SDK on.
+    char *tests[] = {"winv6.3", "win8", "win7"};
+    bool found = false;
+    for (char *test : tests) {
+      llvm::SmallString<128> testPath(libPath);
+      llvm::sys::path::append(testPath, test);
+      if (llvm::sys::fs::exists(testPath.c_str())) {
+        libPath = testPath;
+        found = true;
+        break;
+      }
+    }
+
+    if (!found)
+      return false;
+
+    llvm::sys::path::append(libPath, "um");
+    switch (getArch()) {
+    case llvm::Triple::x86:
+      llvm::sys::path::append(libPath, "x86");
+      break;
+    case llvm::Triple::x86_64:
+      llvm::sys::path::append(libPath, "x64");
+      break;
+    case llvm::Triple::arm:
+      llvm::sys::path::append(libPath, "arm");
+      break;
+    default:
+      return false;
+    }
+  }
+
+  path = libPath.str();
+  return true;
+}
+
 // Get the location to use for Visual Studio binaries.  The location priority
 // is: %VCINSTALLDIR% > %PATH% > newest copy of Visual Studio installed on
 // system (as reported by the registry).
 bool MSVCToolChain::getVisualStudioBinariesFolder(const char *clangProgramPath,
                                                   std::string &path) const {
   path.clear();
 
   SmallString<128> BinDir;

cfe/trunk/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 getWindowsSDKLibraryPath(std::string &path) const;
   bool getVisualStudioInstallDir(std::string &path) const;
   bool getVisualStudioBinariesFolder(const char *clangProgramPath,
                                      std::string &path) const;
 
 protected:
   void AddSystemIncludeWithSubfolder(const llvm::opt::ArgList &DriverArgs,
                                      llvm::opt::ArgStringList &CC1Args,
                                      const std::string &folder,

cfe/trunk/lib/Driver/Tools.cpp

void visualstudio::Link::ConstructJob(Compilation &C, const JobAction &JA,
   }
 
   if (!Args.hasArg(options::OPT_nostdlib) &&
       !Args.hasArg(options::OPT_nostartfiles) &&
       !C.getDriver().IsCLMode()) {
     CmdArgs.push_back("-defaultlib:libcmt");
   }
 
+  if (!llvm::sys::Process::GetEnv("LIB")) {
+    // If the VC environment hasn't been configured (perhaps because the user
+    // did not run vcvarsall), try to build a consistent link environment.  If
+    // the environment variable is set however, assume the user knows what he's
+    // doing.
+    std::string VisualStudioDir;
+    const auto &MSVC = static_cast<const toolchains::MSVCToolChain &>(getToolChain());
+    if (MSVC.getVisualStudioInstallDir(VisualStudioDir)) {
+      SmallString<128> LibDir(VisualStudioDir);
+      llvm::sys::path::append(LibDir, "VC", "lib");
+      switch (MSVC.getArch()) {
+      case llvm::Triple::x86:
+        // x86 just puts the libraries directly in lib
+        break;
+      case llvm::Triple::x86_64:
+        llvm::sys::path::append(LibDir, "amd64");
+        break;
+      case llvm::Triple::arm:
+        llvm::sys::path::append(LibDir, "arm");
+        break;
+      default:
+        break;
+      }
+      CmdArgs.push_back(
+          Args.MakeArgString(std::string("-libpath:") + LibDir.c_str()));
+    }
+
+    std::string WindowsSdkLibPath;
+    if (MSVC.getWindowsSDKLibraryPath(WindowsSdkLibPath))
+      CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:") +
+                                           WindowsSdkLibPath.c_str()));
+  }
+
   CmdArgs.push_back("-nologo");
 
   if (Args.hasArg(options::OPT_g_Group)) {
     CmdArgs.push_back("-debug");
   }
 
   bool DLL = Args.hasArg(options::OPT__SLASH_LD, options::OPT__SLASH_LDd);