https://github.com/charles-zablit updated https://github.com/llvm/llvm-project/pull/168733
>From 71c4fd1953dfa65a215ce347bee3c5d51db41e11 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Wed, 19 Nov 2025 16:17:31 +0000 Subject: [PATCH 1/4] [lldb][windows] refactor CreateProcessW setup --- .../Host/windows/ProcessLauncherWindows.h | 31 ++++++- .../Host/windows/ProcessLauncherWindows.cpp | 82 ++++++++++--------- 2 files changed, 73 insertions(+), 40 deletions(-) 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. >From 987ae27154aac1fdbb2de4e3429036803853b169 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Wed, 26 Nov 2025 17:10:39 +0000 Subject: [PATCH 2/4] address comments --- lldb/include/lldb/Host/windows/ProcessLauncherWindows.h | 2 +- lldb/source/Host/windows/ProcessLauncherWindows.cpp | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h index 153086eb9b17c..a04077a5e1d03 100644 --- a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h +++ b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h @@ -55,4 +55,4 @@ class ProcessLauncherWindows : public ProcessLauncher { }; } -#endif \ No newline at end of file +#endif diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp index 42de009b173e5..c6518a811a2c6 100644 --- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp +++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp @@ -26,9 +26,8 @@ ProcessLauncherWindows::CreateEnvironmentBufferW(const Environment &env) { std::vector<std::wstring> env_entries; for (const auto &KV : env) { std::wstring wentry; - if (llvm::ConvertUTF8toWide(Environment::compose(KV), wentry)) { + if (llvm::ConvertUTF8toWide(Environment::compose(KV), wentry)) env_entries.push_back(std::move(wentry)); - } } std::sort(env_entries.begin(), env_entries.end(), [](const std::wstring &a, const std::wstring &b) { @@ -36,7 +35,6 @@ ProcessLauncherWindows::CreateEnvironmentBufferW(const Environment &env) { }); 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'); >From a3029fc8046b2af9ad65ec7052dcf7a8fdead9eb Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Thu, 27 Nov 2025 11:52:11 +0000 Subject: [PATCH 3/4] reverse moving the methods into the class --- .../Host/windows/ProcessLauncherWindows.h | 29 ------------------- .../Host/windows/ProcessLauncherWindows.cpp | 29 +++++++++++++++---- 2 files changed, 24 insertions(+), 34 deletions(-) diff --git a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h index a04077a5e1d03..81aea5b2022a5 100644 --- a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h +++ b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h @@ -11,9 +11,6 @@ #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 { @@ -26,32 +23,6 @@ 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); }; } diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp index c6518a811a2c6..d158638221bf5 100644 --- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp +++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp @@ -21,8 +21,22 @@ using namespace lldb; using namespace lldb_private; -std::vector<wchar_t> -ProcessLauncherWindows::CreateEnvironmentBufferW(const Environment &env) { +/// 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) { std::vector<std::wstring> env_entries; for (const auto &KV : env) { std::wstring wentry; @@ -44,8 +58,14 @@ ProcessLauncherWindows::CreateEnvironmentBufferW(const Environment &env) { return buffer; } -llvm::ErrorOr<std::wstring> -ProcessLauncherWindows::GetFlattenedWindowsCommandStringW(Args args) { +/// 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) { if (args.empty()) return L""; @@ -62,7 +82,6 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, error.Clear(); std::string executable; - std::vector<char> environment; STARTUPINFOEXW startupinfoex = {}; STARTUPINFOW &startupinfo = startupinfoex.StartupInfo; PROCESS_INFORMATION pi = {}; >From f245a29a00edb0c192587471914c4973293b99a8 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Fri, 28 Nov 2025 16:57:02 +0000 Subject: [PATCH 4/4] fix test failure with empty env --- lldb/source/Host/windows/ProcessLauncherWindows.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp index d158638221bf5..d62d8dbb355ce 100644 --- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp +++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp @@ -27,7 +27,7 @@ using namespace lldb_private; /// 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. +/// The keys are sorted to comply with the CreateProcess API calling convention. /// /// Ensure that the resulting buffer is used in conjunction with /// CreateProcessW and be sure that dwCreationFlags includes @@ -35,7 +35,7 @@ using namespace lldb_private; /// /// \param env The Environment object to convert. /// \returns The sorted sequence of environment variables and their values, -/// separated by null terminators. +/// separated by null terminators. The vector is guaranteed to never be empty. static std::vector<wchar_t> CreateEnvironmentBufferW(const Environment &env) { std::vector<std::wstring> env_entries; for (const auto &KV : env) { @@ -53,6 +53,10 @@ static std::vector<wchar_t> CreateEnvironmentBufferW(const Environment &env) { buffer.insert(buffer.end(), env_entry.begin(), env_entry.end()); buffer.push_back(L'\0'); } + + if (buffer.empty()) + buffer.push_back(L'\0'); // If there are no environment variables, we have + // to ensure there are 4 zero bytes in the buffer. buffer.push_back(L'\0'); return buffer; @@ -167,7 +171,6 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, std::vector<wchar_t> environment = CreateEnvironmentBufferW(launch_info.GetEnvironment()); - LPVOID env_block = environment.empty() ? nullptr : environment.data(); auto wcommandLineOrErr = GetFlattenedWindowsCommandStringW(launch_info.GetArguments()); @@ -189,7 +192,7 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, BOOL result = ::CreateProcessW( wexecutable.c_str(), pwcommandLine, NULL, NULL, - /*bInheritHandles=*/!inherited_handles.empty(), flags, env_block, + /*bInheritHandles=*/!inherited_handles.empty(), flags, environment.data(), wworkingDirectory.size() == 0 ? NULL : wworkingDirectory.c_str(), reinterpret_cast<STARTUPINFOW *>(&startupinfoex), &pi); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
