[lldb][windows] refactor CreateProcessW setup #168733

charles-zablit · 2025-11-19T16:24:37Z

This patch refactors and documents the setup of the CreateProcessW invocation in ProcessLauncherWindows. It's a dependency of #168729.

CreateEnvironmentBufferW now sorts the environment variable keys before concatenating them into a string. From the CreateProcess documentation:

An application must manually pass the current directory information to the new process. To do so, the application must explicitly create these environment variable strings, sort them alphabetically (because the system uses a sorted environment), and put them into the environment block. Typically, they will go at the front of the environment block, due to the environment block sort order.

GetFlattenedWindowsCommandStringW now returns an error which will be surfaced, instead of failing silently.

Types were converted to their wide equivalent (i.e appending W to them, see STARTUPINFOEX) since we are calling the W variant of CreateProcess.

llvmbot · 2025-11-19T16:25:11Z

@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-lldb

Author: Charles Zablit (charles-zablit)

Changes

This patch refactors and documents the setup of the CreateProcessW invocation in ProcessLauncherWindows. It's a dependency of #168729.

CreateEnvironmentBufferW now sorts the environment variable keys before concatenating them into a string. From the CreateProcess documentation:
> An application must manually pass the current directory information to the new process. To do so, the application must explicitly create these environment variable strings, sort them alphabetically (because the system uses a sorted environment), and put them into the environment block. Typically, they will go at the front of the environment block, due to the environment block sort order.

GetFlattenedWindowsCommandStringW now returns an error which will be surfaced, instead of failing silently.

Types were converted to their wide equivalent (i.e appending W to them, see STARTUPINFOEX) since we are calling the W variant of CreateProcess.

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

2 Files Affected:

(modified) lldb/include/lldb/Host/windows/ProcessLauncherWindows.h (+30-1)
(modified) lldb/source/Host/windows/ProcessLauncherWindows.cpp (+43-39)

diff --git a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h
index 81aea5b2022a5..153086eb9b17c 100644
--- a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h
+++ b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h
@@ -11,6 +11,9 @@
 
 #include "lldb/Host/ProcessLauncher.h"
 #include "lldb/Host/windows/windows.h"
+#include "lldb/Utility/Args.h"
+#include "lldb/Utility/Environment.h"
+#include "llvm/Support/ErrorOr.h"
 
 namespace lldb_private {
 
@@ -23,7 +26,33 @@ class ProcessLauncherWindows : public ProcessLauncher {
 
 protected:
   HANDLE GetStdioHandle(const ProcessLaunchInfo &launch_info, int fd);
+
+  /// Create a UTF-16 environment block to use with CreateProcessW.
+  ///
+  /// The buffer is a sequence 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'.
+  ///
+  /// The keys are sorted to comply with the CreateProcess' calling convention.
+  ///
+  /// Ensure that the resulting buffer is used in conjunction with
+  /// CreateProcessW and be sure that dwCreationFlags includes
+  /// CREATE_UNICODE_ENVIRONMENT.
+  ///
+  /// \param env The Environment object to convert.
+  /// \returns The sorted sequence of environment variables and their values,
+  /// separated by null terminators.
+  static std::vector<wchar_t> CreateEnvironmentBufferW(const Environment &env);
+
+  /// Flattens an Args object into a Windows command-line wide string.
+  ///
+  /// Returns an empty string if args is empty.
+  ///
+  /// \param args The Args object to flatten.
+  /// \returns A wide string containing the flattened command line.
+  static llvm::ErrorOr<std::wstring>
+  GetFlattenedWindowsCommandStringW(Args args);
 };
 }
 
-#endif
+#endif
\ No newline at end of file
diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
index f5adadaf061bf..42de009b173e5 100644
--- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp
+++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
@@ -21,42 +21,41 @@
 using namespace lldb;
 using namespace lldb_private;
 
-static void CreateEnvironmentBuffer(const Environment &env,
-                                    std::vector<char> &buffer) {
-  // 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'.
+std::vector<wchar_t>
+ProcessLauncherWindows::CreateEnvironmentBufferW(const Environment &env) {
+  std::vector<std::wstring> env_entries;
   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));
+    std::wstring wentry;
+    if (llvm::ConvertUTF8toWide(Environment::compose(KV), wentry)) {
+      env_entries.push_back(std::move(wentry));
     }
   }
-  // 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);
+  std::sort(env_entries.begin(), env_entries.end(),
+            [](const std::wstring &a, const std::wstring &b) {
+              return _wcsicmp(a.c_str(), b.c_str()) < 0;
+            });
+
+  std::vector<wchar_t> buffer;
+  buffer.clear();
+  for (const auto &env_entry : env_entries) {
+    buffer.insert(buffer.end(), env_entry.begin(), env_entry.end());
+    buffer.push_back(L'\0');
+  }
+  buffer.push_back(L'\0');
+
+  return buffer;
 }
 
-static bool GetFlattenedWindowsCommandString(Args args, std::wstring &command) {
+llvm::ErrorOr<std::wstring>
+ProcessLauncherWindows::GetFlattenedWindowsCommandStringW(Args args) {
   if (args.empty())
-    return false;
+    return L"";
 
   std::vector<llvm::StringRef> args_ref;
   for (auto &entry : args.entries())
     args_ref.push_back(entry.ref());
 
-  llvm::ErrorOr<std::wstring> result =
-      llvm::sys::flattenWindowsCommandLine(args_ref);
-  if (result.getError())
-    return false;
-
-  command = *result;
-  return true;
+  return llvm::sys::flattenWindowsCommandLine(args_ref);
 }
 
 HostProcess
@@ -66,8 +65,8 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info,
 
   std::string executable;
   std::vector<char> environment;
-  STARTUPINFOEX startupinfoex = {};
-  STARTUPINFO &startupinfo = startupinfoex.StartupInfo;
+  STARTUPINFOEXW startupinfoex = {};
+  STARTUPINFOW &startupinfo = startupinfoex.StartupInfo;
   PROCESS_INFORMATION pi = {};
 
   HANDLE stdin_handle = GetStdioHandle(launch_info, STDIN_FILENO);
@@ -149,28 +148,33 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info,
   if (launch_info.GetFlags().Test(eLaunchFlagDisableSTDIO))
     flags &= ~CREATE_NEW_CONSOLE;
 
-  LPVOID env_block = nullptr;
-  ::CreateEnvironmentBuffer(launch_info.GetEnvironment(), environment);
-  env_block = environment.data();
-
-  executable = launch_info.GetExecutableFile().GetPath();
-  std::wstring wcommandLine;
-  GetFlattenedWindowsCommandString(launch_info.GetArguments(), wcommandLine);
+  std::vector<wchar_t> environment =
+      CreateEnvironmentBufferW(launch_info.GetEnvironment());
+  LPVOID env_block = environment.empty() ? nullptr : environment.data();
 
-  std::wstring wexecutable, wworkingDirectory;
-  llvm::ConvertUTF8toWide(executable, wexecutable);
-  llvm::ConvertUTF8toWide(launch_info.GetWorkingDirectory().GetPath(),
-                          wworkingDirectory);
+  auto wcommandLineOrErr =
+      GetFlattenedWindowsCommandStringW(launch_info.GetArguments());
+  if (!wcommandLineOrErr) {
+    error = Status(wcommandLineOrErr.getError());
+    return HostProcess();
+  }
+  std::wstring wcommandLine = *wcommandLineOrErr;
   // If the command line is empty, it's best to pass a null pointer to tell
   // CreateProcessW to use the executable name as the command line.  If the
   // command line is not empty, its contents may be modified by CreateProcessW.
   WCHAR *pwcommandLine = wcommandLine.empty() ? nullptr : &wcommandLine[0];
 
+  std::wstring wexecutable, wworkingDirectory;
+  llvm::ConvertUTF8toWide(launch_info.GetExecutableFile().GetPath(),
+                          wexecutable);
+  llvm::ConvertUTF8toWide(launch_info.GetWorkingDirectory().GetPath(),
+                          wworkingDirectory);
+
   BOOL result = ::CreateProcessW(
       wexecutable.c_str(), pwcommandLine, NULL, NULL,
       /*bInheritHandles=*/!inherited_handles.empty(), flags, env_block,
       wworkingDirectory.size() == 0 ? NULL : wworkingDirectory.c_str(),
-      reinterpret_cast<STARTUPINFO *>(&startupinfoex), &pi);
+      reinterpret_cast<STARTUPINFOW *>(&startupinfoex), &pi);
 
   if (!result) {
     // Call GetLastError before we make any other system calls.

github-actions · 2025-11-19T16:31:37Z

🐧 Linux x64 Test Results

33163 tests passed
494 tests skipped

adrian-prantl · 2025-11-19T20:43:56Z

lldb/source/Host/windows/ProcessLauncherWindows.cpp

+            });
+
+  std::vector<wchar_t> buffer;
+  buffer.clear();


What's the purpose of of this?

compnerd · 2025-11-19T20:59:50Z

lldb/include/lldb/Host/windows/ProcessLauncherWindows.h

 };
 }

-#endif
+#endif


missing newline

compnerd · 2025-11-19T21:00:36Z

lldb/include/lldb/Host/windows/ProcessLauncherWindows.h

+  /// \param env The Environment object to convert.
+  /// \returns The sorted sequence of environment variables and their values,
+  /// separated by null terminators.
+  static std::vector<wchar_t> CreateEnvironmentBufferW(const Environment &env);


Why is this being moved into the type rather than keeping it standalone?

compnerd · 2025-11-19T21:00:39Z

lldb/include/lldb/Host/windows/ProcessLauncherWindows.h

+  ///
+  /// \param args The Args object to flatten.
+  /// \returns A wide string containing the flattened command line.
+  static llvm::ErrorOr<std::wstring>


compnerd · 2025-11-19T21:02:54Z

lldb/source/Host/windows/ProcessLauncherWindows.cpp

-  // extra L'\0' (two bytes of 0).  An empty environment must have one
-  // empty string, followed by an extra L'\0'.
+std::vector<wchar_t>
+ProcessLauncherWindows::CreateEnvironmentBufferW(const Environment &env) {


I don't understand why this cannot remain an internal detail of the implementation and needs to be moved into ProcessLauncherWindows. The equivalent signature would be:

std::vector<wchar_t> CreateEnvironmentBufferW(const Environment &);

compnerd · 2025-11-19T21:04:10Z

lldb/source/Host/windows/ProcessLauncherWindows.cpp

-          buffer.end(), reinterpret_cast<const char *>(warg.c_str()),
-          reinterpret_cast<const char *>(warg.c_str() + warg.size() + 1));
+    std::wstring wentry;
+    if (llvm::ConvertUTF8toWide(Environment::compose(KV), wentry)) {


I'd drop the unnecessary braces

compnerd · 2025-11-19T21:04:21Z

lldb/source/Host/windows/ProcessLauncherWindows.cpp

+  std::sort(env_entries.begin(), env_entries.end(),
+            [](const std::wstring &a, const std::wstring &b) {
+              return _wcsicmp(a.c_str(), b.c_str()) < 0;
+            });


Why the explicit sort?

compnerd · 2025-11-19T21:04:53Z

lldb/source/Host/windows/ProcessLauncherWindows.cpp

+    buffer.insert(buffer.end(), env_entry.begin(), env_entry.end());
+    buffer.push_back(L'\0');
+  }
+  buffer.push_back(L'\0');


This really makes this far more expensive than it was previously - we now end up double allocating and copying.

compnerd · 2025-11-19T21:05:07Z

lldb/source/Host/windows/ProcessLauncherWindows.cpp

 }

-static bool GetFlattenedWindowsCommandString(Args args, std::wstring &command) {
+llvm::ErrorOr<std::wstring>
+ProcessLauncherWindows::GetFlattenedWindowsCommandStringW(Args args) {


Similar to the other function, why is this being moved into ProcessLauncherWindows?

[lldb][windows] refactor CreateProcessW setup

71c4fd1

charles-zablit requested a review from JDevlieghere as a code owner November 19, 2025 16:24

charles-zablit requested review from DavidSpickett and labath and removed request for JDevlieghere November 19, 2025 16:24

llvmbot added the lldb label Nov 19, 2025

charles-zablit requested a review from adrian-prantl November 19, 2025 16:24

charles-zablit self-assigned this Nov 19, 2025

charles-zablit mentioned this pull request Nov 19, 2025

[DRAFT][lldb][windows] add Windows Virtual Console support #168729

Draft

charles-zablit added the platform:windows label Nov 19, 2025

adrian-prantl approved these changes Nov 19, 2025

View reviewed changes

compnerd reviewed Nov 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lldb][windows] refactor CreateProcessW setup #168733

[lldb][windows] refactor CreateProcessW setup #168733

charles-zablit commented Nov 19, 2025

Uh oh!

llvmbot commented Nov 19, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Nov 19, 2025

Uh oh!

adrian-prantl Nov 19, 2025

Uh oh!

compnerd Nov 19, 2025

Uh oh!

compnerd Nov 19, 2025

Uh oh!

compnerd Nov 19, 2025

Uh oh!

compnerd Nov 19, 2025

Uh oh!

compnerd Nov 19, 2025

Uh oh!

compnerd Nov 19, 2025

Uh oh!

compnerd Nov 19, 2025

Uh oh!

compnerd Nov 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

               };
               }
-              #endif
+              #endif

[lldb][windows] refactor CreateProcessW setup #168733

Are you sure you want to change the base?

[lldb][windows] refactor CreateProcessW setup #168733

Conversation

charles-zablit commented Nov 19, 2025

Uh oh!

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

Uh oh!

Uh oh!

github-actions bot commented Nov 19, 2025

🐧 Linux x64 Test Results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

llvmbot commented Nov 19, 2025 •

edited

Loading