Fix ExecuteAndWait with empty environment on Windows #158719

hjyamauchi · 2025-09-15T19:22:49Z

CreateProcessW requires that the environemnt block to be always double null-terminated even with an empty environemnt.

https://learn.microsoft.com/en-us/windows/win32/procthread/environment-variables

The attached test fails this way without the fix.

C:\Users\hiroshi\upstream\llvm-project\llvm\unittests\Support\ProgramTest.cpp(697): error: Value of: ExecutionFailed
Actual: true
Expected: false
Couldn't execute program 'C:\Users\hiroshi\upstream\llvm-project\build\unittests\Support\SupportTests.exe': The parameter is incorrect. (0x57)

llvmbot · 2025-09-15T19:23:24Z

@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-llvm-support

Author: Hiroshi Yamauchi (hjyamauchi)

Changes

CreateProcessW requires that the environemnt block to be always double null-terminated even with an empty environemnt.

https://learn.microsoft.com/en-us/windows/win32/procthread/environment-variables

The attached test fails this way without the fix.

C:\Users\hiroshi\upstream\llvm-project\llvm\unittests\Support\ProgramTest.cpp(697): error: Value of: ExecutionFailed
Actual: true
Expected: false
Couldn't execute program 'C:\Users\hiroshi\upstream\llvm-project\build\unittests\Support\SupportTests.exe': The parameter is incorrect. (0x57)

Full diff: https://github.com/llvm/llvm-project/pull/158719.diff

2 Files Affected:

(modified) llvm/lib/Support/Windows/Program.inc (+4)
(modified) llvm/unittests/Support/ProgramTest.cpp (+18)

diff --git a/llvm/lib/Support/Windows/Program.inc b/llvm/lib/Support/Windows/Program.inc
index 799af5559966c..81385864da85f 100644
--- a/llvm/lib/Support/Windows/Program.inc
+++ b/llvm/lib/Support/Windows/Program.inc
@@ -220,6 +220,10 @@ static bool Execute(ProcessInfo &PI, StringRef Program,
       llvm::append_range(EnvBlock, EnvString);
       EnvBlock.push_back(0);
     }
+    // If an empty environment (*Env is size zero), we need to
+    // terminate with two nulls.
+    if ((*Env).size() == 0)
+      EnvBlock.push_back(0);
     EnvBlock.push_back(0);
   }
 
diff --git a/llvm/unittests/Support/ProgramTest.cpp b/llvm/unittests/Support/ProgramTest.cpp
index d30bf458f233c..89ecfd558d2cb 100644
--- a/llvm/unittests/Support/ProgramTest.cpp
+++ b/llvm/unittests/Support/ProgramTest.cpp
@@ -680,4 +680,22 @@ TEST_F(ProgramEnvTest, TestExecuteWithNoStacktraceHandler) {
   ASSERT_EQ(0, RetCode);
 }
 
+TEST_F(ProgramEnvTest, TestExecuteEmptyEnvironment) {
+  using namespace llvm::sys;
+
+  std::string Executable =
+      sys::fs::getMainExecutable(TestMainArgv0, &ProgramTestStringArg1);
+  StringRef argv[] = {
+      Executable,
+      "--gtest_filter=" // A null invocation to avoid infinite recursion
+  };
+
+  std::string Error;
+  bool ExecutionFailed;
+  int RetCode = ExecuteAndWait(Executable, argv, ArrayRef<StringRef>{}, {}, 0, 0,
+                               &Error, &ExecutionFailed);
+  EXPECT_FALSE(ExecutionFailed) << Error;
+  ASSERT_EQ(0, RetCode);
+}
+
 } // end anonymous namespace

github-actions · 2025-09-15T19:26:46Z

✅ With the latest revision this PR passed the C/C++ code formatter.

mstorsjo

Looks reasonable to me (but wait a little in case others also want to comment on it)

aganea

LGTM with a small nit.

aganea · 2025-09-15T20:22:19Z

llvm/lib/Support/Windows/Program.inc

nit: Env->size()

I would also change the comment above to something to this effect: « Empty environments need to be terminated with two nulls. », no need to repeat « *Env is size zero ».

Sorry I missed this comment. I'll follow up.

CreateProcessW requires that the environemnt block to be always double null-terminated even with an empty environemnt. https://learn.microsoft.com/en-us/windows/win32/procthread/environment-variables The attached test fails this way without the fix. C:\Users\hiroshi\upstream\llvm-project\llvm\unittests\Support\ProgramTest.cpp(697): error: Value of: ExecutionFailed Actual: true Expected: false Couldn't execute program 'C:\Users\hiroshi\upstream\llvm-project\build\unittests\Support\SupportTests.exe': The parameter is incorrect. (0x57)

mstorsjo · 2025-09-21T19:27:06Z

This testcase fails in my test configurations (e.g. https://github.com/mstorsjo/llvm-mingw/actions/runs/17894399079/job/50879057137).

This is because my build environments make binaries that depend on a dynamically linked libc++.dll. When trying to execute SupportTests.exe with an empty environment, it fails because it fails to find libc++.dll in the PATH.

It would be possible to avoid this issue by copying in libc++.dll and all other potential dependencies (libunwind.dll, or libstdc++-6.dll and libgcc_s_seh-1.dll) into the directory of SupportTests.exe, but that's kinda ugly for this case.

This primarily is an issue in mingw build configurations. In theory you could have similar issues in MSVC style environments as well, if linking with /MD, but as long as the CRT DLL is available system-wide, and likewise msvcp140.dll also is installed in e.g. windows\system32, the necessary DLLs are found in the end.

What do you think is the best way around this? Just waive this testcase for mingw environments? Or build a separate simple dummy exe as subject for this testcase?

I think the former is simpler in practice, and mostly accurate.

hjyamauchi · 2025-09-22T16:27:55Z

I think so too

pawosm-arm · 2025-09-27T16:09:21Z

Somehow this managed to cause a regression on Linux! In our LLVM builds we are using libc++ as a standard C++ library. I assume this is still somewhat unusual thing to do, but so far all test cases were passing. Except the one introduced by this commit. It fails as such:

-- Testing: 98831 tests, 256 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80..
FAIL: LLVM-Unit :: Support/./SupportTests/151/406 (89601 of 98831)
******************** TEST 'LLVM-Unit :: Support/./SupportTests/151/406' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:unittests/Support/./SupportTests-LLVM-Unit-3565525-151-406.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=406 GTEST_SHARD_INDEX=151 unittests/Support/./SupportTests
--

Script:
--
unittests/Support/./SupportTests --gtest_filter=ProgramEnvTest.TestExecuteEmptyEnvironment
--
unittests/Support/SupportTests: error while loading shared libraries: libc++.so.1: cannot open shared object file: No such file or directory
llvm/unittests/Support/ProgramTest.cpp:698: Failure
Expected equality of these values:
  0
  RetCode
    Which is: -1


llvm/unittests/Support/ProgramTest.cpp:698
Expected equality of these values:
  0
  RetCode
    Which is: -1



********************

Note that LD_LIBRARY_PATH is set to the place where libc++.so.1 exists when executing tests, but apparently it isn't sufficient here. Forcing the rpath to be set (e.g. by providing a compiler config file) does help with this test case, but breaks the others.

…y the lit tests (#538)" It was a horrible solution to the problem introduced by llvm/llvm-project#158719 It caused unexpected issues while testing on RHEL8. This reverts commit 0cc7c83.

hjyamauchi · 2025-09-29T16:18:33Z

Maybe disable the test like #160277?

pawosm-arm · 2025-09-29T16:21:07Z

Maybe disable the test like #160277?

Locally, we do this: sed -i "s/ifndef __MINGW32__/ifndef LLVM_ON_UNIX/" llvm/unittests/Support/ProgramTest.cpp

CreateProcessW requires that the environemnt block to be always double null-terminated even with an empty environemnt. https://learn.microsoft.com/en-us/windows/win32/procthread/environment-variables The attached test fails this way without the fix. C:\Users\hiroshi\upstream\llvm-project\llvm\unittests\Support\ProgramTest.cpp(697): error: Value of: ExecutionFailed Actual: true Expected: false Couldn't execute program 'C:\Users\hiroshi\upstream\llvm-project\build\unittests\Support\SupportTests.exe': The parameter is incorrect. (0x57) (Cherry picked from commit 81c55e6)

llvmbot added platform:windows llvm:support labels Sep 15, 2025

hjyamauchi force-pushed the emptyenv branch from 3adfd86 to e915dad Compare September 15, 2025 19:23

hjyamauchi force-pushed the emptyenv branch 2 times, most recently from 6fb0158 to 98d5e1c Compare September 15, 2025 19:28

cachemeifyoucan requested review from compnerd and rnk September 15, 2025 20:06

mstorsjo approved these changes Sep 15, 2025

View reviewed changes

mstorsjo requested a review from aganea September 15, 2025 20:07

compnerd approved these changes Sep 15, 2025

View reviewed changes

aganea approved these changes Sep 15, 2025

View reviewed changes

hjyamauchi force-pushed the emptyenv branch from 98d5e1c to 9716444 Compare September 15, 2025 21:06

hjyamauchi merged commit 81c55e6 into llvm:main Sep 16, 2025
9 checks passed

hjyamauchi added a commit to hjyamauchi/llvm-project that referenced this pull request Sep 16, 2025

Fix comment from llvm#158719

a01e29f

hjyamauchi added a commit that referenced this pull request Sep 16, 2025

Fix comment from #158719 (#159157)

7c861bc

llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Sep 16, 2025

Automerge: Fix comment from llvm/llvm-project#158719 (#159157)

8a59eba

pawosm-arm mentioned this pull request Sep 27, 2025

Revert "[ATfL] Make sure rpath is added to the helper programs used by the lit tests (#538)" arm/arm-toolchain#554

Merged

Fix ExecuteAndWait with empty environment on Windows #158719

Fix ExecuteAndWait with empty environment on Windows #158719

Uh oh!

Conversation

hjyamauchi commented Sep 15, 2025

Uh oh!

llvmbot commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mstorsjo left a comment

Choose a reason for hiding this comment

Uh oh!

aganea left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aganea Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

hjyamauchi Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

aganea Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

hjyamauchi Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

hjyamauchi Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mstorsjo commented Sep 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hjyamauchi commented Sep 22, 2025

Uh oh!

pawosm-arm commented Sep 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hjyamauchi commented Sep 29, 2025

Uh oh!

pawosm-arm commented Sep 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

llvmbot commented Sep 15, 2025 •

edited

Loading

github-actions bot commented Sep 15, 2025 •

edited

Loading

aganea left a comment •

edited

Loading

mstorsjo commented Sep 21, 2025 •

edited

Loading

pawosm-arm commented Sep 27, 2025 •

edited

Loading