[lldb] Fix TestSettings.test_pass_host_env_vars on windows

[labath]

Summary

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. Insted, I make 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.

[amccarth]

The code is correct, so I'm approving, but I suggest a couple fixes to the comments before committing..

lldb/source/Host/windows/ProcessLauncherWindows.cpp

27

You have an extra "list of" between null-terminated and "strings". And I think we should point out that it's UTF-16 encoded, so maybe:

// 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'.

40

"... an extra two bytes ..."

42

There would be no harm in always adding the extra terminator.

[friss]

Thanks for fixing this Pavel!

[labath]

lldb/source/Host/windows/ProcessLauncherWindows.cpp

42

Yeah, I wondered myself what would be less confusing... inserting a seemingly needless terminator, or cluttering the code with an extra if...

lldb/source/Host/windows/ProcessLauncherWindows.cpp

 #include <vector>
 
 using namespace lldb;
 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)) {
       buffer.insert(
           buffer.end(), reinterpret_cast<const char *>(warg.c_str()),
           reinterpret_cast<const char *>(warg.c_str() + warg.size() + 1));
     }
   }
   // 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) {
   if (args.empty())
     return false;
 
   std::vector<llvm::StringRef> args_ref;
   for (auto &entry : args.entries())
ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info,
   if (launch_info.GetFlags().Test(eLaunchFlagDebug))
     flags |= DEBUG_ONLY_THIS_PROCESS;
 
   if (launch_info.GetFlags().Test(eLaunchFlagDisableSTDIO))
     flags &= ~CREATE_NEW_CONSOLE;
 
   LPVOID env_block = nullptr;
   ::CreateEnvironmentBuffer(launch_info.GetEnvironment(), environment);
-  if (!environment.empty())
-    env_block = environment.data();
 
   executable = launch_info.GetExecutableFile().GetPath();
   GetFlattenedWindowsCommandString(launch_info.GetArguments(), commandLine);
 
   std::wstring wexecutable, wcommandLine, wworkingDirectory;
   llvm::ConvertUTF8toWide(executable, wexecutable);
   llvm::ConvertUTF8toWide(commandLine, wcommandLine);
   llvm::ConvertUTF8toWide(launch_info.GetWorkingDirectory().GetCString(),

lldb/test/API/commands/settings/TestSettings.py

def do_test_run_args_and_env_vars(self, use_launchsimple):
 
         self.expect(
             output,
             exe=False,
             substrs=[
                 "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()
 
         # Set some host environment variables now.
         os.environ["MY_HOST_ENV_VAR1"] = "VAR1"
         os.environ["MY_HOST_ENV_VAR2"] = "VAR2"