[lldb] Inherit DuplicateFileAction(HANDLE, HANDLE) handles on windows

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.

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<char> 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<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);
+
+  size_t attributelist_size = 0;
+  InitializeProcThreadAttributeList(/*lpAttributeList=*/nullptr,
+                                    /*dwAttributeCount=*/1, /*dwFlags=*/0,
+                                    &attributelist_size);
+
+  startupinfoex.lpAttributeList =
+      static_cast<LPPROC_THREAD_ATTRIBUTE_LIST>(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<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");
   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<STARTUPINFO *>(&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();
 

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

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));

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!");
 
@@ -126,4 +125,3 @@ TEST(Host, LaunchProcessDuplicatesHandle) {
   ASSERT_THAT_EXPECTED(bytes_read, llvm::Succeeded());
   ASSERT_EQ(llvm::StringRef(msg, *bytes_read), test_msg);
 }
-#endif