[Lldb-commits] [lldb] 62be834 - Recommit "[lldb] Fix TestSettings.test_pass_host_env_vars on windows"

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-04-02T11:52:56+02:00
New Revision: 62be83463a3713612bd85cfa45140ef92c130d57

URL: https://github.com/llvm/llvm-project/commit/62be83463a3713612bd85cfa45140ef92c130d57
DIFF: https://github.com/llvm/llvm-project/commit/62be83463a3713612bd85cfa45140ef92c130d57.diff

LOG: Recommit "[lldb] Fix TestSettings.test_pass_host_env_vars on windows"

This patch was reverted because it introduced a failure in
TestHelloWorld.py. The reason for that was running "ls" shell command
failed as it was evaluated in an environment with an empty path. This
has now been fixed with D77123, which ensures that all shell commands
inherit the host environment, so this patch should be safe to recommit.

The original commit message was:

A defensive check in ProcessLauncherWindows meant that we would never
attempt to launch a process with a completely empty environment -- the
host environment would be used instead. Instead, I make the function add
an extra null wchar_t at the end of an empty environment. The
documentation on this is a bit fuzzy, but it seems to be what is needed
to make windows accept these kinds of environments.

Reviewers: amccarth, friss

Differential Revision: https://reviews.llvm.org/D76835

Added: 
    

Modified: 
    lldb/source/Host/windows/ProcessLauncherWindows.cpp
    lldb/test/API/commands/settings/TestSettings.py

Removed: 
    


################################################################################
diff  --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
index 31101944d75c..00470f558e94 100644
--- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp
+++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
@@ -23,10 +23,9 @@ using namespace lldb_private;
 namespace {
 void CreateEnvironmentBuffer(const Environment &env,
                              std::vector<char> &buffer) {
-  if (env.size() == 0)
-    return;
-
-  // Environment buffer is a null terminated list of null terminated strings
+  // The buffer is a list of null-terminated UTF-16 strings, followed by an
+  // extra L'\0' (two bytes of 0).  An empty environment must have one
+  // empty string, followed by an extra L'\0'.
   for (const auto &KV : env) {
     std::wstring warg;
     if (llvm::ConvertUTF8toWide(Environment::compose(KV), warg)) {
@@ -38,6 +37,9 @@ void CreateEnvironmentBuffer(const Environment &env,
   // One null wchar_t (to end the block) is two null bytes
   buffer.push_back(0);
   buffer.push_back(0);
+  // Insert extra two bytes, just in case the environment was empty.
+  buffer.push_back(0);
+  buffer.push_back(0);
 }
 
 bool GetFlattenedWindowsCommandString(Args args, std::string &command) {
@@ -94,8 +96,7 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info,
 
   LPVOID env_block = nullptr;
   ::CreateEnvironmentBuffer(launch_info.GetEnvironment(), environment);
-  if (!environment.empty())
-    env_block = environment.data();
+  env_block = environment.data();
 
   executable = launch_info.GetExecutableFile().GetPath();
   GetFlattenedWindowsCommandString(launch_info.GetArguments(), commandLine);

diff  --git a/lldb/test/API/commands/settings/TestSettings.py b/lldb/test/API/commands/settings/TestSettings.py
index c0cdc085f129..29360856a735 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -286,7 +286,6 @@ def do_test_run_args_and_env_vars(self, use_launchsimple):
                 "Environment variable 'MY_ENV_VAR' successfully passed."])
 
     @skipIfRemote  # it doesn't make sense to send host env to remote target
-    @skipIf(oslist=["windows"])
     def test_pass_host_env_vars(self):
         """Test that the host env vars are passed to the launched process."""
         self.build()