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

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

lib/Driver/WindowsToolChain.cpp

215

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

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

[zturner]

lib/Driver/WindowsToolChain.cpp

226

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

7875

Close the parenthetical before the comma

7878

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.

7879

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.

7883

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.

7900

WindowsSdkLibPath

lib/Driver/WindowsToolChain.cpp

228

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

249

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).

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, bool x64) 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;
 

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 was set by the user, we trust the user to know what he's
+    // doing.
+    std::string VisualStudioDir;
+    const toolchains::Windows &WTC =
+        static_cast<const toolchains::Windows &>(getToolChain());
+    if (WTC.getVisualStudioDir(VisualStudioDir)) {
+      SmallString<128> VSDir(VisualStudioDir);
+      llvm::sys::path::append(VSDir, "VC\\lib");
+      if (Args.hasArg(options::OPT_m64))
+        llvm::sys::path::append(VSDir, "amd64");
+
+      CmdArgs.push_back(
+          Args.MakeArgString(std::string("-libpath:") + VSDir.c_str()));
+    }
+
+    std::string WindowsSdkLibPath;
+    if (WTC.getWindowsSDKLibraryPath(WindowsSdkLibPath,
+                                     Args.hasArg(options::OPT_m64))) {
+      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);
 
   if (DLL) {
     CmdArgs.push_back(Args.MakeArgString("-dll"));

lib/Driver/WindowsToolChain.cpp

 #define USE_WIN32
 #endif
 
 #ifdef USE_WIN32
   #define WIN32_LEAN_AND_MEAN
   #define NOGDI
   #define NOMINMAX
   #include <windows.h>
+#include <VersionHelpers.h>
 #endif
 
 using namespace clang::driver;
 using namespace clang::driver::toolchains;
 using namespace clang;
 using namespace llvm::opt;
 
 Windows::Windows(const Driver &D, const llvm::Triple& Triple,
bool Windows::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();
 }
 
+// \brief Gets the library path required to link against the Windows SDK.
+bool Windows::getWindowsSDKLibraryPath(std::string &path, bool x64) const {
+  std::string sdkPath;
+  int sdkMajor = 0;
+  int sdkMinor = 0;
+
+  if (!getWindowsSDKDir(sdkPath, sdkMajor, sdkMinor))
+    return false;
+  llvm::SmallString<128> tempPath(sdkPath);
+  llvm::sys::path::append(tempPath, "Lib");
+  bool unknownSdkLayout = false;
+  if (sdkMajor <= 7) {
+    if (x64)
+      llvm::sys::path::append(tempPath, "amd64");
+  } else {
+    if (IsWindows8Point1OrGreater())
+      llvm::sys::path::append(tempPath, "winv6.3\\um");
+    else if (IsWindows8OrGreater())
+      llvm::sys::path::append(tempPath, "win8\\um");
+    else if (IsWindows7OrGreater())
+      llvm::sys::path::append(tempPath, "win7\\um");
+    else
+      unknownSdkLayout = true;
+
+    if (x64)
+      llvm::sys::path::append(tempPath, "x64");
+    else
+      llvm::sys::path::append(tempPath, "x86");
+  }
+  if (!unknownSdkLayout)
+    path = tempPath.c_str();
+  return !unknownSdkLayout;
+}
+
 // Get Visual Studio installation directory.
 bool Windows::getVisualStudioDir(std::string &path) const {
   // First check the environment variables that vsvars32.bat sets.
   const char *vcinstalldir = getenv("VCINSTALLDIR");
   if (vcinstalldir) {
     path = vcinstalldir;
     path = path.substr(0, path.find("\\VC"));
     return true;
   }