llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-platform-windows Author: Charles Zablit (charles-zablit) <details> <summary>Changes</summary> This NFC patch extracts the logic for creating the `inherited_handles` vector in `ProcessLauncherWindows.cpp` in its own method. This is a preface to https://github.com/llvm/llvm-project/pull/168729. It will make it easier to separate the case where we want to use a PTY and the one where we don't. --- Full diff: https://github.com/llvm/llvm-project/pull/170301.diff 2 Files Affected: - (modified) lldb/include/lldb/Host/windows/ProcessLauncherWindows.h (+31) - (modified) lldb/source/Host/windows/ProcessLauncherWindows.cpp (+51-32) ``````````diff diff --git a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h index 81aea5b2022a5..ecb49e899aaf7 100644 --- a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h +++ b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h @@ -11,6 +11,7 @@ #include "lldb/Host/ProcessLauncher.h" #include "lldb/Host/windows/windows.h" +#include "llvm/Support/ErrorOr.h" namespace lldb_private { @@ -23,6 +24,36 @@ class ProcessLauncherWindows : public ProcessLauncher { protected: HANDLE GetStdioHandle(const ProcessLaunchInfo &launch_info, int fd); + + /// Get the list of Windows handles that should be inherited by the child + /// process and update `STARTUPINFOEXW` with the handle list. + /// + /// If no handles need to be inherited, an empty vector is returned. + /// + /// Otherwise, the function populates the + /// `PROC_THREAD_ATTRIBUTE_HANDLE_LIST` attribute in `startupinfoex` with the + /// collected handles using `UpdateProcThreadAttribute`. On success, the + /// vector of inherited handles is returned. + /// + /// \param launch_info + /// The process launch configuration. + /// + /// \param startupinfoex + /// The extended STARTUPINFO structure for the process being created. + /// + /// \param stdout_handle + /// \param stderr_handle + /// \param stdin_handle + /// Optional explicit standard stream handles to use for the child process. + /// + /// \returns + /// `std::vector<HANDLE>` containing all handles that the child must + /// inherit. + llvm::ErrorOr<std::vector<HANDLE>> + GetInheritedHandlesW(const ProcessLaunchInfo &launch_info, + STARTUPINFOEXW &startupinfoex, + HANDLE stdout_handle = NULL, HANDLE stderr_handle = NULL, + HANDLE stdin_handle = NULL); }; } diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp index d62d8dbb355ce..deb8f69c9a43f 100644 --- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp +++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp @@ -14,6 +14,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/Support/ConvertUTF.h" #include "llvm/Support/Program.h" +#include "llvm/Support/WindowsError.h" #include <string> #include <vector> @@ -90,6 +91,9 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, STARTUPINFOW &startupinfo = startupinfoex.StartupInfo; PROCESS_INFORMATION pi = {}; + startupinfo.cb = sizeof(startupinfoex); + startupinfo.dwFlags |= STARTF_USESTDHANDLES; + HANDLE stdin_handle = GetStdioHandle(launch_info, STDIN_FILENO); HANDLE stdout_handle = GetStdioHandle(launch_info, STDOUT_FILENO); HANDLE stderr_handle = GetStdioHandle(launch_info, STDERR_FILENO); @@ -102,22 +106,13 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, ::CloseHandle(stderr_handle); }); - startupinfo.cb = sizeof(startupinfoex); - startupinfo.dwFlags |= STARTF_USESTDHANDLES; - startupinfo.hStdError = - stderr_handle ? stderr_handle : ::GetStdHandle(STD_ERROR_HANDLE); - startupinfo.hStdInput = - stdin_handle ? stdin_handle : ::GetStdHandle(STD_INPUT_HANDLE); - startupinfo.hStdOutput = - stdout_handle ? stdout_handle : ::GetStdHandle(STD_OUTPUT_HANDLE); - - std::vector<HANDLE> inherited_handles; - if (startupinfo.hStdError) - inherited_handles.push_back(startupinfo.hStdError); - if (startupinfo.hStdInput) - inherited_handles.push_back(startupinfo.hStdInput); - if (startupinfo.hStdOutput) - inherited_handles.push_back(startupinfo.hStdOutput); + auto inherited_handles_or_err = GetInheritedHandlesW( + launch_info, startupinfoex, stdout_handle, stderr_handle, stdin_handle); + if (!inherited_handles_or_err) { + error = Status(inherited_handles_or_err.getError()); + return HostProcess(); + } + std::vector<HANDLE> inherited_handles = inherited_handles; SIZE_T attributelist_size = 0; InitializeProcThreadAttributeList(/*lpAttributeList=*/nullptr, @@ -136,22 +131,6 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, } auto delete_attributelist = llvm::make_scope_exit( [&] { DeleteProcThreadAttributeList(startupinfoex.lpAttributeList); }); - for (size_t i = 0; i < launch_info.GetNumFileActions(); ++i) { - const FileAction *act = launch_info.GetFileActionAtIndex(i); - if (act->GetAction() == FileAction::eFileActionDuplicate && - act->GetFD() == act->GetActionArgument()) - inherited_handles.push_back(reinterpret_cast<HANDLE>(act->GetFD())); - } - if (!inherited_handles.empty()) { - if (!UpdateProcThreadAttribute( - startupinfoex.lpAttributeList, /*dwFlags=*/0, - PROC_THREAD_ATTRIBUTE_HANDLE_LIST, inherited_handles.data(), - inherited_handles.size() * sizeof(HANDLE), - /*lpPreviousValue=*/nullptr, /*lpReturnSize=*/nullptr)) { - error = Status(::GetLastError(), eErrorTypeWin32); - return HostProcess(); - } - } const char *hide_console_var = getenv("LLDB_LAUNCH_INFERIORS_WITHOUT_CONSOLE"); @@ -215,6 +194,46 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, return HostProcess(pi.hProcess); } +llvm::ErrorOr<std::vector<HANDLE>> ProcessLauncherWindows::GetInheritedHandlesW( + const ProcessLaunchInfo &launch_info, STARTUPINFOEXW &startupinfoex, + HANDLE stdout_handle, HANDLE stderr_handle, HANDLE stdin_handle) { + std::vector<HANDLE> inherited_handles; + STARTUPINFOW startupinfo = startupinfoex.StartupInfo; + + startupinfo.hStdError = + stderr_handle ? stderr_handle : GetStdHandle(STD_ERROR_HANDLE); + startupinfo.hStdInput = + stdin_handle ? stdin_handle : GetStdHandle(STD_INPUT_HANDLE); + startupinfo.hStdOutput = + stdout_handle ? stdout_handle : GetStdHandle(STD_OUTPUT_HANDLE); + + if (startupinfo.hStdError) + inherited_handles.push_back(startupinfo.hStdError); + if (startupinfo.hStdInput) + inherited_handles.push_back(startupinfo.hStdInput); + if (startupinfo.hStdOutput) + inherited_handles.push_back(startupinfo.hStdOutput); + + for (size_t i = 0; i < launch_info.GetNumFileActions(); ++i) { + const FileAction *act = launch_info.GetFileActionAtIndex(i); + if (act->GetAction() == FileAction::eFileActionDuplicate && + act->GetFD() == act->GetActionArgument()) + inherited_handles.push_back(reinterpret_cast<HANDLE>(act->GetFD())); + } + + if (inherited_handles.empty()) + return inherited_handles; + + if (!UpdateProcThreadAttribute( + startupinfoex.lpAttributeList, /*dwFlags=*/0, + PROC_THREAD_ATTRIBUTE_HANDLE_LIST, inherited_handles.data(), + inherited_handles.size() * sizeof(HANDLE), + /*lpPreviousValue=*/nullptr, /*lpReturnSize=*/nullptr)) + return llvm::mapWindowsError(::GetLastError()); + + return inherited_handles; +} + HANDLE ProcessLauncherWindows::GetStdioHandle(const ProcessLaunchInfo &launch_info, int fd) { `````````` </details> https://github.com/llvm/llvm-project/pull/170301 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
