diff --git a/lldb/source/Host/windows/PipeWindows.cpp b/lldb/source/Host/windows/PipeWindows.cpp index e3f5b629a0590..30b9d1c41695b 100644 --- a/lldb/source/Host/windows/PipeWindows.cpp +++ b/lldb/source/Host/windows/PipeWindows.cpp @@ -88,8 +88,9 @@ Status PipeWindows::CreateNew(llvm::StringRef name, std::string pipe_path = g_pipe_name_prefix.str(); pipe_path.append(name.str()); - SECURITY_ATTRIBUTES sa{sizeof(SECURITY_ATTRIBUTES), 0, - child_process_inherit ? TRUE : FALSE}; + // We always create inheritable handles, but we won't pass them to a child + // process unless explicitly requested (cf. ProcessLauncherWindows.cpp). + SECURITY_ATTRIBUTES sa{sizeof(SECURITY_ATTRIBUTES), 0, TRUE}; // Always open for overlapped i/o. We implement blocking manually in Read // and Write. @@ -165,8 +166,9 @@ Status PipeWindows::OpenNamedPipe(llvm::StringRef name, assert(is_read ? !CanRead() : !CanWrite()); - SECURITY_ATTRIBUTES attributes{sizeof(SECURITY_ATTRIBUTES), 0, - child_process_inherit ? TRUE : FALSE}; + // We always create inheritable handles, but we won't pass them to a child + // process unless explicitly requested (cf. ProcessLauncherWindows.cpp). + SECURITY_ATTRIBUTES attributes{sizeof(SECURITY_ATTRIBUTES), 0, TRUE}; std::string pipe_path = g_pipe_name_prefix.str(); pipe_path.append(name.str()); diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp index 065ba9271ad0d..bc35667ea9a23 100644 --- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp +++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp @@ -10,6 +10,7 @@ #include "lldb/Host/HostProcess.h" #include "lldb/Host/ProcessLaunchInfo.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Support/ConvertUTF.h" #include "llvm/Support/Program.h" @@ -65,14 +66,23 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, std::string executable; std::vector environment; - STARTUPINFO startupinfo = {}; + STARTUPINFOEX startupinfoex = {}; + STARTUPINFO &startupinfo = startupinfoex.StartupInfo; PROCESS_INFORMATION pi = {}; HANDLE stdin_handle = GetStdioHandle(launch_info, STDIN_FILENO); HANDLE stdout_handle = GetStdioHandle(launch_info, STDOUT_FILENO); HANDLE stderr_handle = GetStdioHandle(launch_info, STDERR_FILENO); - - startupinfo.cb = sizeof(startupinfo); + auto close_handles = llvm::make_scope_exit([&] { + if (stdin_handle) + ::CloseHandle(stdin_handle); + if (stdout_handle) + ::CloseHandle(stdout_handle); + if (stderr_handle) + ::CloseHandle(stderr_handle); + }); + + startupinfo.cb = sizeof(startupinfoex); startupinfo.dwFlags |= STARTF_USESTDHANDLES; startupinfo.hStdError = stderr_handle ? stderr_handle : ::GetStdHandle(STD_ERROR_HANDLE); @@ -81,6 +91,48 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, startupinfo.hStdOutput = stdout_handle ? stdout_handle : ::GetStdHandle(STD_OUTPUT_HANDLE); + std::vector 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); + + size_t attributelist_size = 0; + InitializeProcThreadAttributeList(/*lpAttributeList=*/nullptr, + /*dwAttributeCount=*/1, /*dwFlags=*/0, + &attributelist_size); + + startupinfoex.lpAttributeList = + static_cast(malloc(attributelist_size)); + auto free_attributelist = + llvm::make_scope_exit([&] { free(startupinfoex.lpAttributeList); }); + if (!InitializeProcThreadAttributeList(startupinfoex.lpAttributeList, + /*dwAttributeCount=*/1, /*dwFlags=*/0, + &attributelist_size)) { + error = Status(::GetLastError(), eErrorTypeWin32); + return HostProcess(); + } + 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(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"); if (hide_console_var && @@ -89,7 +141,8 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, startupinfo.wShowWindow = SW_HIDE; } - DWORD flags = CREATE_NEW_CONSOLE | CREATE_UNICODE_ENVIRONMENT; + DWORD flags = CREATE_NEW_CONSOLE | CREATE_UNICODE_ENVIRONMENT | + EXTENDED_STARTUPINFO_PRESENT; if (launch_info.GetFlags().Test(eLaunchFlagDebug)) flags |= DEBUG_ONLY_THIS_PROCESS; @@ -114,9 +167,10 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, WCHAR *pwcommandLine = wcommandLine.empty() ? nullptr : &wcommandLine[0]; BOOL result = ::CreateProcessW( - wexecutable.c_str(), pwcommandLine, NULL, NULL, TRUE, flags, env_block, + wexecutable.c_str(), pwcommandLine, NULL, NULL, + /*bInheritHandles=*/!inherited_handles.empty(), flags, env_block, wworkingDirectory.size() == 0 ? NULL : wworkingDirectory.c_str(), - &startupinfo, &pi); + reinterpret_cast(&startupinfoex), &pi); if (!result) { // Call GetLastError before we make any other system calls. @@ -131,13 +185,6 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, ::CloseHandle(pi.hThread); } - if (stdin_handle) - ::CloseHandle(stdin_handle); - if (stdout_handle) - ::CloseHandle(stdout_handle); - if (stderr_handle) - ::CloseHandle(stderr_handle); - if (!result) return HostProcess(); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index d8c7e436f3f8b..332b9255f226f 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -924,9 +924,7 @@ Status GDBRemoteCommunication::StartDebugserverProcess( debugserver_args.AppendArgument(fd_arg.GetString()); // Send "pass_comm_fd" down to the inferior so it can use it to // communicate back with this process. Ignored on Windows. -#ifndef _WIN32 launch_info.AppendDuplicateFileAction((int)pass_comm_fd, (int)pass_comm_fd); -#endif } // use native registers, not the GDB registers diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 10d79c63af994..5b0a8ade01025 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -274,10 +274,8 @@ static Status spawn_process(const char *progname, const FileSpec &prog, self_args.AppendArgument(llvm::StringRef("platform")); self_args.AppendArgument(llvm::StringRef("--child-platform-fd")); self_args.AppendArgument(llvm::to_string(shared_socket.GetSendableFD())); -#ifndef _WIN32 launch_info.AppendDuplicateFileAction((int)shared_socket.GetSendableFD(), (int)shared_socket.GetSendableFD()); -#endif if (gdb_port) { self_args.AppendArgument(llvm::StringRef("--gdbserver-port")); self_args.AppendArgument(llvm::to_string(gdb_port)); diff --git a/lldb/unittests/Host/HostTest.cpp b/lldb/unittests/Host/HostTest.cpp index 9306a868ebe35..52224bfd28e61 100644 --- a/lldb/unittests/Host/HostTest.cpp +++ b/lldb/unittests/Host/HostTest.cpp @@ -90,7 +90,6 @@ TEST(Host, LaunchProcessSetsArgv0) { ASSERT_THAT(exit_status.get_future().get(), 0); } -#ifdef LLVM_ON_UNIX TEST(Host, LaunchProcessDuplicatesHandle) { static constexpr llvm::StringLiteral test_msg("Hello subprocess!"); @@ -130,4 +129,3 @@ TEST(Host, LaunchProcessDuplicatesHandle) { ASSERT_THAT_EXPECTED(bytes_read, llvm::Succeeded()); ASSERT_EQ(llvm::StringRef(msg, *bytes_read), test_msg); } -#endif