hamzasood updated this revision to Diff 92050.
hamzasood added a comment.

@zturner: "it's just a vector of pointers"
Oh...... I was so focussed on reducing copies that I completely forgot it's 
just an array of pointers being copied, not the entire environment block 
contents. In that case, yeah. It's a dumb optimisation.

- setEnvironment is now just one function that accepts an ArrayRef.


https://reviews.llvm.org/D30991

Files:
  include/clang/Driver/Job.h
  lib/Driver/Job.cpp
  lib/Driver/ToolChains/MSVC.cpp

Index: lib/Driver/ToolChains/MSVC.cpp
===================================================================
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -34,7 +34,7 @@
 // Include the necessary headers to interface with the Windows registry and
 // environment.
 #if defined(LLVM_ON_WIN32)
-#define USE_WIN32
+  #define USE_WIN32
 #endif
 
 #ifdef USE_WIN32
@@ -47,20 +47,20 @@
 #endif
 
 #ifdef _MSC_VER
-// Don't support SetupApi on MinGW.
-#define USE_MSVC_SETUP_API
+  // Don't support SetupApi on MinGW.
+  #define USE_MSVC_SETUP_API
 
-// Make sure this comes before MSVCSetupApi.h
-#include <comdef.h>
+  // Make sure this comes before MSVCSetupApi.h
+  #include <comdef.h>
 
-#include "MSVCSetupApi.h"
-#include "llvm/Support/COM.h"
-_COM_SMARTPTR_TYPEDEF(ISetupConfiguration, __uuidof(ISetupConfiguration));
-_COM_SMARTPTR_TYPEDEF(ISetupConfiguration2, __uuidof(ISetupConfiguration2));
-_COM_SMARTPTR_TYPEDEF(ISetupHelper, __uuidof(ISetupHelper));
-_COM_SMARTPTR_TYPEDEF(IEnumSetupInstances, __uuidof(IEnumSetupInstances));
-_COM_SMARTPTR_TYPEDEF(ISetupInstance, __uuidof(ISetupInstance));
-_COM_SMARTPTR_TYPEDEF(ISetupInstance2, __uuidof(ISetupInstance2));
+  #include "MSVCSetupApi.h"
+  #include "llvm/Support/COM.h"
+  _COM_SMARTPTR_TYPEDEF(ISetupConfiguration, __uuidof(ISetupConfiguration));
+  _COM_SMARTPTR_TYPEDEF(ISetupConfiguration2, __uuidof(ISetupConfiguration2));
+  _COM_SMARTPTR_TYPEDEF(ISetupHelper, __uuidof(ISetupHelper));
+  _COM_SMARTPTR_TYPEDEF(IEnumSetupInstances, __uuidof(IEnumSetupInstances));
+  _COM_SMARTPTR_TYPEDEF(ISetupInstance, __uuidof(ISetupInstance));
+  _COM_SMARTPTR_TYPEDEF(ISetupInstance2, __uuidof(ISetupInstance2));
 #endif
 
 using namespace clang::driver;
@@ -446,6 +446,8 @@
 
   TC.addProfileRTLibs(Args, CmdArgs);
 
+  std::vector<const char *> Environment;
+
   // We need to special case some linker paths.  In the case of lld, we need to
   // translate 'lld' into 'lld-link', and in the case of the regular msvc
   // linker, we need to use a special search algorithm.
@@ -459,6 +461,68 @@
     // from the program PATH, because other environments like GnuWin32 install
     // their own link.exe which may come first.
     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
+    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)
+        goto SkipSettingEnvironment;
+
+      size_t EnvCount = 0;
+      size_t EnvBlockLen = 0;
+      while (EnvBlockWide[EnvBlockLen] != L'\0') {
+        ++EnvCount;
+        EnvBlockLen += std::wcslen(&EnvBlockWide[EnvBlockLen])
+                       + 1/*string null-terminator*/;
+      }
+      ++EnvBlockLen;  // add the block null-terminator
+
+      std::string EnvBlock;
+      if (!llvm::convertUTF16ToUTF8String(
+             llvm::ArrayRef<char>(reinterpret_cast<char *>(EnvBlockWide.get()),
+                                  EnvBlockLen * sizeof(EnvBlockWide[0])),
+             EnvBlock))
+        goto SkipSettingEnvironment;
+
+      Environment.reserve(EnvCount);
+
+      // Now loop over each string in the block and copy them into the
+      // environment vector, adjusting the PATH variable as needed when we
+      // find it.
+      for (const char *Cursor = EnvBlock.data(); *Cursor != '\0';) {
+        llvm::StringRef EnvVar(Cursor);
+        if (EnvVar.startswith_lower("path=")) {
+          using SubDirectoryType = toolchains::MSVCToolChain::SubDirectoryType;
+          constexpr size_t PrefixLen = 5;  // strlen("path=")
+          Environment.push_back(Args.MakeArgString(
+              EnvVar.substr(0, PrefixLen)
+            + TC.getSubDirectoryPath(SubDirectoryType::Bin)
+            + llvm::Twine(llvm::sys::EnvPathSeparator)
+            + TC.getSubDirectoryPath(SubDirectoryType::Bin, HostArch)
+            + (EnvVar.size() > PrefixLen
+                  ? llvm::Twine(llvm::sys::EnvPathSeparator)
+                    + EnvVar.substr(PrefixLen)
+                  : "")
+          ));
+        } else {
+          Environment.push_back(Args.MakeArgString(EnvVar));
+        }
+        Cursor += EnvVar.size() + 1/*null-terminator*/;
+      }
+    }
+  SkipSettingEnvironment:
+    ;
+#endif
   } else {
     linkPath = Linker;
     llvm::sys::path::replace_extension(linkPath, "exe");
@@ -465,8 +529,11 @@
     linkPath = TC.GetProgramPath(linkPath.c_str());
   }
 
-  const char *Exec = Args.MakeArgString(linkPath);
-  C.addCommand(llvm::make_unique<Command>(JA, *this, Exec, CmdArgs, Inputs));
+  auto LinkCmd = llvm::make_unique<Command>(
+                   JA, *this, Args.MakeArgString(linkPath), CmdArgs, Inputs);
+  if (!Environment.empty())
+    LinkCmd->setEnvironment(Environment);
+  C.addCommand(std::move(LinkCmd));
 }
 
 void visualstudio::Compiler::ConstructJob(Compilation &C, const JobAction &JA,
Index: lib/Driver/Job.cpp
===================================================================
--- lib/Driver/Job.cpp
+++ lib/Driver/Job.cpp
@@ -301,16 +301,31 @@
   ResponseFileFlag += FileName;
 }
 
+void Command::setEnvironment(llvm::ArrayRef<const char *> NewEnvironment) {
+  Environment.reserve(NewEnvironment.size() + 1);
+  Environment.assign(NewEnvironment.begin(), NewEnvironment.end());
+  Environment.push_back(nullptr);
+}
+
 int Command::Execute(const StringRef **Redirects, std::string *ErrMsg,
                      bool *ExecutionFailed) const {
   SmallVector<const char*, 128> Argv;
 
+  const char **Envp;
+  if (Environment.empty()) {
+    Envp = nullptr;
+  } else {
+    assert(Environment.back() == nullptr 
+           && "Environment vector should be null-terminated by now");
+    Envp = const_cast<const char **>(Environment.data());
+  }
+
   if (ResponseFile == nullptr) {
     Argv.push_back(Executable);
     Argv.append(Arguments.begin(), Arguments.end());
     Argv.push_back(nullptr);
 
-    return llvm::sys::ExecuteAndWait(Executable, Argv.data(), /*env*/ nullptr,
+    return llvm::sys::ExecuteAndWait(Executable, Argv.data(), Envp,
                                      Redirects, /*secondsToWait*/ 0,
                                      /*memoryLimit*/ 0, ErrMsg,
                                      ExecutionFailed);
@@ -337,7 +352,7 @@
     return -1;
   }
 
-  return llvm::sys::ExecuteAndWait(Executable, Argv.data(), /*env*/ nullptr,
+  return llvm::sys::ExecuteAndWait(Executable, Argv.data(), Envp,
                                    Redirects, /*secondsToWait*/ 0,
                                    /*memoryLimit*/ 0, ErrMsg, ExecutionFailed);
 }
Index: include/clang/Driver/Job.h
===================================================================
--- include/clang/Driver/Job.h
+++ include/clang/Driver/Job.h
@@ -11,6 +11,7 @@
 #define LLVM_CLANG_DRIVER_JOB_H
 
 #include "clang/Basic/LLVM.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/Option/Option.h"
@@ -69,6 +70,9 @@
   /// file
   std::string ResponseFileFlag;
 
+  /// See Command::setEnvironment
+  std::vector<const char *> Environment;
+
   /// When a response file is needed, we try to put most arguments in an
   /// exclusive file, while others remains as regular command line arguments.
   /// This functions fills a vector with the regular command line arguments,
@@ -111,6 +115,12 @@
     InputFileList = std::move(List);
   }
 
+  /// \brief Sets the environment to be used by the new process.
+  /// \param NewEnvironment An array of environment variables.
+  /// \remark If the environment remains unset, then the environment
+  ///         from the parent process will be used.
+  void setEnvironment(llvm::ArrayRef<const char *> NewEnvironment);
+
   const char *getExecutable() const { return Executable; }
 
   const llvm::opt::ArgStringList &getArguments() const { return Arguments; }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to