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

This is a follow-up to llvm#126935, which enables passing handles to a child
process on windows systems. Unlike on unix-like systems, the handles
need to be created with the "inheritable" flag because ...

Author: labath

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,41 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info,
   startupinfo.hStdOutput =
       stdout_handle ? stdout_handle : ::GetStdHandle(STD_OUTPUT_HANDLE);
 
+  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); });
+  std::vector<HANDLE> inherited_handles;
+  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 +134,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;
 
@@ -116,7 +162,7 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info,
   BOOL result = ::CreateProcessW(
       wexecutable.c_str(), pwcommandLine, NULL, NULL, TRUE, 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 +177,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/unittests/Host/HostTest.cpp

@@ -90,31 +90,35 @@ 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!");
 
   SubsystemRAII<FileSystem> subsystems;
 
   if (test_arg) {
-    Pipe pipe(LLDB_INVALID_PIPE, test_arg);
+    Pipe pipe(LLDB_INVALID_PIPE,
+              reinterpret_cast<lldb::pipe_t>(test_arg.getValue()));
     llvm::Expected<size_t> bytes_written =
         pipe.Write(test_msg.data(), test_msg.size());
     if (bytes_written && *bytes_written == test_msg.size())
       exit(0);
     exit(1);
   }
   Pipe pipe;
-  ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).takeError(),
+  ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/true).takeError(),
                     llvm::Succeeded());
   ProcessLaunchInfo info;
   info.SetExecutableFile(FileSpec(TestMainArgv0),
                          /*add_exe_file_as_first_arg=*/true);
   info.GetArguments().AppendArgument(
       "--gtest_filter=Host.LaunchProcessDuplicatesHandle");
   info.GetArguments().AppendArgument(
-      ("--test-arg=" + llvm::Twine::utohexstr(pipe.GetWritePipe())).str());
-  info.AppendDuplicateFileAction(pipe.GetWritePipe(), pipe.GetWritePipe());
+      ("--test-arg=" +
+       llvm::Twine(reinterpret_cast<uint64_t>(pipe.GetWritePipe())))
+          .str());
+  info.AppendDuplicateFileAction(
+      reinterpret_cast<uint64_t>(pipe.GetWritePipe()),
+      reinterpret_cast<uint64_t>(pipe.GetWritePipe()));
   info.SetMonitorProcessCallback(&ProcessLaunchInfo::NoOpMonitorCallback);
   ASSERT_THAT_ERROR(Host::LaunchProcess(info).takeError(), llvm::Succeeded());
   pipe.CloseWriteFileDescriptor();
@@ -125,4 +129,3 @@ TEST(Host, LaunchProcessDuplicatesHandle) {
   ASSERT_THAT_EXPECTED(bytes_read, llvm::Succeeded());
   ASSERT_EQ(llvm::StringRef(msg, *bytes_read), test_msg);
 }
-#endif