On 25/03/2020 01:04, Adrian McCarthy wrote:
> I took a stab at this, but I'm not seeing any new test failures. 

That is odd.

I was doing some stuff on windows today, so I figured I'd take a stab at
this. I was kind of right that the check in ProcessLauncher windows
prevents us from passing an empty environment.

However, the interesting part starts when I tried to remove that check.
Then the test started behaving nondeterministically -- sometimes passing
and sometimes failing due to ERROR_INVALID_PARAMETER being returned from
CreateProcessW. I can see how windows might need some environment
variables to start up a process correctly, but I do not understand why
this should be nondeterministic...

This is beyond my knowledge of windows. It might be interesting to
reduce this to a simple test case (independent of lldb) and show it to
some windows expert.

I am attaching a patch with the lldb changes I've made, in case you want
to play around with it.

> I assume
> the problem you're seeing is in TestSettings.py, but I've never figured
> out how to run individual Python-based lldb tests since all the
> dotest.py stuff was re-written.

These days we have multiple ways of achieving that. :)

One way would be via the "check-lldb-api-commands-settings" target which
would run all tests under api/commands/settings (i.e. TestSettings.py
and TestQuoting.py).

Another option would be via the lldb-dotest script.
"python bin\lldb-dotest -p TestSettings.py" would just run that single
file. That script passes all of its options to dotest, so you can use
any of the dotest options that you used to use.

pl
From 0e01f02b9b01c9700787c640ac96923acdf0cb0f Mon Sep 17 00:00:00 2001
From: Pavel Labath <pa...@labath.sk>
Date: Wed, 25 Mar 2020 13:39:05 +0100
Subject: [PATCH] [lldb] (Almost?) fix TestSettings.test_pass_host_env_vars

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.

After applying this fix, the relevant test starts to pass, but it does
so (at least on my machine) only sporadically. Sometimes, CreateProcessW
seems to fail with ERROR_INVALID_PARAMETER.

My windows-fu is not sufficient to understand what is going on here.
---
 lldb/source/Host/windows/ProcessLauncherWindows.cpp | 6 +-----
 lldb/test/API/commands/settings/TestSettings.py     | 1 -
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
index 31101944d..77c7bc269 100644
--- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp
+++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
@@ -23,9 +23,6 @@ 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
   for (const auto &KV : env) {
     std::wstring warg;
@@ -94,8 +91,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 c0cdc085f..29360856a 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -286,7 +286,6 @@ class SettingsCommandTestCase(TestBase):
                 "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()
-- 
2.19.2.windows.1

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to