Set up the MSVC environment in a more flexible way

For invoking MS tools such as link.exe and lib.exe, which require
compatible PATH, LIB, LIBPATH etc.

We previously wrapped the invokation of the tool with a batch file taking
care of setting up the environment and then calling the actual tool.

I changed this to first invoke another batch file setting up its
environment and then dumping all environment variables to a file. LDC then
parses this file and updates its own environment variables. Later spawned
child processes then inherit the updated environment, and the tools will
be located by LLVM in updated PATH.

This allows LDC to keep track of the used Visual C++ installation (and
could so set up additional runtime libs required since VS 2015, something
we would have needed some time back when we still supported VS 2013). It's
also required as a prerequisite for integrating LLD as in-process linker
on Windows.

I quickly looked into the option of setting up the environment manually,
but the MS batch files are full of quirks, mixtures of registry and
file-system lookups, checking for existence of misc. headers to filter out
bogus installations etc. With VS 2017, there's nothing in the registry at
all anymore (at least for the Build Tools); MS expose a COM interface for
querying the VS installer... But at least they still provide the batch
files.

CMakeLists.txt

@@ -754,9 +754,8 @@ if(MSVC)
     install(DIRECTORY vcbuild/ DESTINATION ${CMAKE_INSTALL_PREFIX}/bin FILES_MATCHING PATTERN "*.bat")
     # Also put the VCBuild scripts in the build/bin folder, so that ${PROJECT_BINARY_DIR}/bin/ldc2 is functional.
     # This is necessary for the IR tests that use ${PROJECT_BINARY_DIR}/bin/ldc2.
-    configure_file(vcbuild/amd64.bat   ${PROJECT_BINARY_DIR}/bin/amd64.bat   COPYONLY)
+    configure_file(vcbuild/dumpEnv.bat ${PROJECT_BINARY_DIR}/bin/dumpEnv.bat COPYONLY)
     configure_file(vcbuild/msvcEnv.bat ${PROJECT_BINARY_DIR}/bin/msvcEnv.bat COPYONLY)
-    configure_file(vcbuild/x86.bat     ${PROJECT_BINARY_DIR}/bin/x86.bat     COPYONLY)
 endif()
 
 if(${CMAKE_SYSTEM_NAME} MATCHES "Linux")

driver/linker.cpp

@@ -501,6 +501,7 @@ static int linkObjToBinaryGcc(bool sharedLib, bool fullyStatic) {
 #ifdef _WIN32
 
 namespace windows {
+
 bool needsQuotes(const llvm::StringRef &arg) {
   return // not already quoted
       !(arg.size() > 1 && arg[0] == '"' &&
@@ -508,7 +509,7 @@ bool needsQuotes(const llvm::StringRef &arg) {
       (arg.empty() || arg.find(' ') != arg.npos || arg.find('"') != arg.npos);
 }
 
-size_t countPrecedingBackslashes(const std::string &arg, size_t index) {
+size_t countPrecedingBackslashes(llvm::StringRef arg, size_t index) {
   size_t count = 0;
 
   for (size_t i = index - 1; i >= 0; --i) {
@@ -520,7 +521,7 @@ size_t countPrecedingBackslashes(const std::string &arg, size_t index) {
   return count;
 }
 
-std::string quoteArg(const std::string &arg) {
+std::string quoteArg(llvm::StringRef arg) {
   if (!needsQuotes(arg))
     return arg;
 
@@ -529,7 +530,7 @@ std::string quoteArg(const std::string &arg) {
 
   quotedArg.push_back('"');
 
-  const size_t argLength = arg.length();
+  const size_t argLength = arg.size();
   for (size_t i = 0; i < argLength; ++i) {
     if (arg[i] == '"') {
       // Escape all preceding backslashes (if any).
@@ -587,17 +588,17 @@ int executeAndWait(const char *commandLine) {
 
   return exitCode;
 }
-}
 
-int executeMsvcToolAndWait(const std::string &tool,
-                           const std::vector<std::string> &args, bool verbose) {
-  llvm::SmallString<1024> commandLine; // full command line incl. executable
+bool setupMsvcEnvironment() {
+  if (getenv("VSINSTALLDIR"))
+    return true;
 
-  // if the VSINSTALLDIR environment variable is NOT set,
-  // the MSVC environment needs to be set up
-  const bool needMsvcSetup = !getenv("VSINSTALLDIR");
-  if (needMsvcSetup) {
-    /* <command line> => %ComSpec% /s /c "<batch file> <command line>"
+  llvm::SmallString<128> tmpFilePath;
+  if (llvm::sys::fs::createTemporaryFile("ldc_dumpEnv", "", tmpFilePath))
+    return false;
+
+  /* Run `%ComSpec% /s /c "...\dumpEnv.bat <x86|amd64> > <tmpFilePath>"` to dump
+   * the MSVC environment to the temporary file.
    *
    * cmd.exe /c treats the following string argument (the command)
    * in a very peculiar way if it starts with a double-quote.
@@ -613,84 +614,85 @@ int executeMsvcToolAndWait(const std::string &tool,
     comspecEnv = "cmd.exe";
   }
   std::string cmdExecutable = comspecEnv;
-    std::string batchFile = exe_path::prependBinDir(
-        global.params.targetTriple->isArch64Bit() ? "amd64.bat" : "x86.bat");
+  std::string batchFile = exe_path::prependBinDir("dumpEnv.bat");
+  std::string arch =
+      global.params.targetTriple->isArch64Bit() ? "amd64" : "x86";
 
-    commandLine.append(windows::quoteArg(cmdExecutable));
-    commandLine.append(" /s /c \"");
-    commandLine.append(windows::quoteArg(batchFile));
-    commandLine.push_back(' ');
-    commandLine.append(windows::quoteArg(tool));
-  } else {
-    std::string toolPath = getProgram(tool.c_str());
-    commandLine.append(windows::quoteArg(toolPath));
-  }
-
-  const size_t commandLineLengthAfterTool = commandLine.size();
-
-  // append (quoted) args
-  for (size_t i = 0; i < args.size(); ++i) {
-    commandLine.push_back(' ');
-    commandLine.append(windows::quoteArg(args[i]));
-  }
-
-  const bool useResponseFile = (!args.empty() && commandLine.size() > 2000);
-  llvm::SmallString<128> responseFilePath;
-  if (useResponseFile) {
-    const size_t firstArgIndex = commandLineLengthAfterTool + 1;
-    llvm::StringRef content(commandLine.data() + firstArgIndex,
-                            commandLine.size() - firstArgIndex);
-
-    if (llvm::sys::fs::createTemporaryFile("ldc_link", "rsp",
-                                           responseFilePath) ||
-        llvm::sys::writeFileWithEncoding(
-            responseFilePath,
-            content)) // keep encoding (LLVM assumes UTF-8 input)
-    {
-      error(Loc(), "cannot write temporary response file for %s", tool.c_str());
-      return -1;
-    }
-
-    // replace all args by @<responseFilePath>
-    std::string responseFileArg = ("@" + responseFilePath).str();
-    commandLine.resize(firstArgIndex);
-    commandLine.append(windows::quoteArg(responseFileArg));
-  }
-
-  if (needMsvcSetup)
-    commandLine.push_back('"');
-
-  const char *finalCommandLine = commandLine.c_str();
-
-  if (verbose) {
-    fprintf(global.stdmsg, finalCommandLine);
-    fprintf(global.stdmsg, "\n");
-    fflush(global.stdmsg);
-  }
-
-  const int exitCode = windows::executeAndWait(finalCommandLine);
+  llvm::SmallString<512> commandLine;
+  commandLine += quoteArg(cmdExecutable);
+  commandLine += " /s /c \"";
+  commandLine += quoteArg(batchFile);
+  commandLine += ' ';
+  commandLine += arch;
+  commandLine += " > ";
+  commandLine += quoteArg(tmpFilePath);
+  commandLine += '"';
 
+  const int exitCode = executeAndWait(commandLine.c_str());
   if (exitCode != 0) {
-    commandLine.resize(commandLineLengthAfterTool);
-    if (needMsvcSetup)
-      commandLine.push_back('"');
     error(Loc(), "`%s` failed with status: %d", commandLine.c_str(), exitCode);
+    llvm::sys::fs::remove(tmpFilePath);
+    return false;
   }
 
-  if (useResponseFile)
-    llvm::sys::fs::remove(responseFilePath);
+  auto fileBuffer = llvm::MemoryBuffer::getFile(tmpFilePath);
+  llvm::sys::fs::remove(tmpFilePath);
+  if (fileBuffer.getError())
+    return false;
 
-  return exitCode;
+  const auto contents = (*fileBuffer)->getBuffer();
+  const auto size = contents.size();
+
+  // Parse the file.
+  std::vector<std::pair<llvm::StringRef, llvm::StringRef>> env;
+
+  size_t i = 0;
+  // for each line
+  while (i < size) {
+    llvm::StringRef key, value;
+
+    for (size_t j = i; j < size; ++j) {
+      const char c = contents[j];
+      if (c == '=' && key.empty()) {
+        key = contents.slice(i, j);
+        i = j + 1;
+      } else if (c == '\n' || c == '\r' || c == '\0') {
+        if (!key.empty()) {
+          value = contents.slice(i, j);
+        }
+        // break and continue with next line
+        i = j + 1;
+        break;
+      }
     }
 
-#else // !_WIN32
-
-int executeMsvcToolAndWait(const std::string &,
-                           const std::vector<std::string> &, bool) {
-  assert(0);
-  return -1;
+    if (!key.empty() && !value.empty())
+      env.emplace_back(key, value);
   }
 
+  if (global.params.verbose)
+    fprintf(global.stdmsg, "Applying environment variables:\n");
+
+  bool haveVsInstallDir = false;
+
+  for (const auto &pair : env) {
+    const std::string key = pair.first.str();
+    const std::string value = pair.second.str();
+
+    if (global.params.verbose)
+      fprintf(global.stdmsg, "  %s=%s\n", key.c_str(), value.c_str());
+
+    SetEnvironmentVariableA(key.c_str(), value.c_str());
+
+    if (key == "VSINSTALLDIR")
+      haveVsInstallDir = true;
+  }
+
+  return haveVsInstallDir;
+}
+
+} // namespace windows
+
 #endif
 
 //////////////////////////////////////////////////////////////////////////////
@@ -698,7 +700,7 @@ int executeMsvcToolAndWait(const std::string &,
 static int linkObjToBinaryMSVC(bool sharedLib) {
   Logger::println("*** Linking executable ***");
 
-  std::string tool = "link.exe";
+  const std::string tool = "link.exe";
 
   // build arguments
   std::vector<std::string> args;
@@ -798,8 +800,12 @@ static int linkObjToBinaryMSVC(bool sharedLib) {
   }
   logstr << "\n"; // FIXME where's flush ?
 
+#ifdef _WIN32
+  windows::setupMsvcEnvironment();
+#endif
+
   // try to call linker
-  return executeMsvcToolAndWait(tool, args, global.params.verbose);
+  return executeToolAndWait(tool, args, global.params.verbose);
 }
 
 //////////////////////////////////////////////////////////////////////////////
@@ -912,12 +918,13 @@ int createStaticLibrary() {
   }
 #endif
 
-  // try to call archiver
-  const int exitCode =
-      isTargetMSVC ? executeMsvcToolAndWait(tool, args, global.params.verbose)
-                   : executeToolAndWait(tool, args, global.params.verbose);
+#ifdef _WIN32
+  if (isTargetMSVC)
+    windows::setupMsvcEnvironment();
+#endif
 
-  return exitCode;
+  // try to call archiver
+  return executeToolAndWait(tool, args, global.params.verbose);
 }
 
 //////////////////////////////////////////////////////////////////////////////

driver/toobj.cpp

@@ -164,8 +164,7 @@ static void assemble(const std::string &asmpath, const std::string &objpath) {
   }
 
   // Run the compiler to assembly the program.
-  std::string gcc(getGcc());
-  int R = executeToolAndWait(gcc, args, global.params.verbose);
+  int R = executeToolAndWait(getGcc(), args, global.params.verbose);
   if (R) {
     error(Loc(), "Error while invoking external assembler.");
     fatal();

driver/tool.cpp

@@ -11,8 +11,16 @@
 #include "mars.h"
 #include "llvm/Support/Program.h"
 
-int executeToolAndWait(const std::string &tool,
+int executeToolAndWait(const std::string &tool_,
                        std::vector<std::string> const &args, bool verbose) {
+  auto fullToolOrError = llvm::sys::findProgramByName(tool_);
+  if (fullToolOrError.getError()) {
+    error(Loc(), "failed to locate binary %s", tool_.c_str());
+    return -1;
+  }
+
+  const auto tool = std::move(*fullToolOrError);
+
   // Construct real argument list.
   // First entry is the tool itself, last entry must be NULL.
   std::vector<const char *> realargs;

gen/programs.cpp

@@ -21,7 +21,7 @@ static cl::opt<std::string>
 static cl::opt<std::string> ar("ar", cl::desc("Archiver"), cl::Hidden,
                                cl::ZeroOrMore);
 
-static std::string findProgramByName(const std::string &name) {
+static std::string findProgramByName(llvm::StringRef name) {
 #if LDC_LLVM_VER >= 306
   llvm::ErrorOr<std::string> res = llvm::sys::findProgramByName(name);
   return res ? res.get() : std::string();
@@ -35,9 +35,8 @@ static std::string getProgram(const char *name, const cl::opt<std::string> *opt,
   std::string path;
   const char *prog = nullptr;
 
-  if (opt && opt->getNumOccurrences() > 0 && opt->length() > 0 &&
-      (prog = opt->c_str())) {
-    path = findProgramByName(prog);
+  if (opt && !opt->empty()) {
+    path = findProgramByName(opt->c_str());
   }
 
   if (path.empty() && envVar && (prog = getenv(envVar)) && prog[0] != '\0') {

vcbuild/amd64.bat

@@ -1,6 +0,0 @@
-@echo off
-setlocal
-call "%~dp0msvcEnv.bat" amd64
-:: Invoke the actual command, represented by all args
-%*
-endlocal && exit /b %ERRORLEVEL%

vcbuild/dumpEnv.bat

@@ -0,0 +1,7 @@
+@echo off
+setlocal
+:: The single arg specifies the architecture (x86/amd64).
+call "%~dp0msvcEnv.bat" %1 > nul
+:: Dump all environment variables to stdout.
+set
+endlocal

vcbuild/x86.bat

@@ -1,6 +0,0 @@
-@echo off
-setlocal
-call "%~dp0msvcEnv.bat" x86
-:: Invoke the actual command, represented by all args
-%*
-endlocal && exit /b %ERRORLEVEL%