llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Charles Zablit (charles-zablit) <details> <summary>Changes</summary> This patch refactors and documents the setup of the `CreateProcessW` invocation in `ProcessLauncherWindows`. It's a dependency of https://github.com/llvm/llvm-project/pull/168729. `CreateEnvironmentBufferW` now sorts the environment variable keys before concatenating them into a string. From [the `CreateProcess` documentation](https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw): > 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 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. `````````` </details> https://github.com/llvm/llvm-project/pull/168733 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
