Revert "[lldb] Inherit DuplicateFileAction(HANDLE, HANDLE) handles on windows (#137978)"

This reverts commit 7c5f5f3 due to
failures on the lldb-remote-linux-win bot.

Author: labath

lldb/source/Host/windows/PipeWindows.cpp

@@ -88,9 +88,8 @@ Status PipeWindows::CreateNew(llvm::StringRef name,
   std::string pipe_path = g_pipe_name_prefix.str();
   pipe_path.append(name.str());
 
-  // 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};
+  SECURITY_ATTRIBUTES sa{sizeof(SECURITY_ATTRIBUTES), 0,
+                         child_process_inherit ? TRUE : FALSE};
 
   // Always open for overlapped i/o.  We implement blocking manually in Read
   // and Write.

lldb/source/Host/windows/ProcessLauncherWindows.cpp

@@ -10,7 +10,6 @@
 #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"
@@ -66,23 +65,14 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info,
 
   std::string executable;
   std::vector<char> environment;
-  STARTUPINFOEX startupinfoex = {};
-  STARTUPINFO &startupinfo = startupinfoex.StartupInfo;
+  STARTUPINFO 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);
-  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.cb = sizeof(startupinfo);
   startupinfo.dwFlags |= STARTF_USESTDHANDLES;
   startupinfo.hStdError =
       stderr_handle ? stderr_handle : ::GetStdHandle(STD_ERROR_HANDLE);
@@ -91,48 +81,6 @@ 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 &&
@@ -141,8 +89,7 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info,
     startupinfo.wShowWindow = SW_HIDE;
   }
 
-  DWORD flags = CREATE_NEW_CONSOLE | CREATE_UNICODE_ENVIRONMENT |
-                EXTENDED_STARTUPINFO_PRESENT;
+  DWORD flags = CREATE_NEW_CONSOLE | CREATE_UNICODE_ENVIRONMENT;
   if (launch_info.GetFlags().Test(eLaunchFlagDebug))
     flags |= DEBUG_ONLY_THIS_PROCESS;
 
@@ -167,10 +114,9 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info,
   WCHAR *pwcommandLine = wcommandLine.empty() ? nullptr : &wcommandLine[0];
 
   BOOL result = ::CreateProcessW(
-      wexecutable.c_str(), pwcommandLine, NULL, NULL,
-      /*bInheritHandles=*/!inherited_handles.empty(), flags, env_block,
+      wexecutable.c_str(), pwcommandLine, NULL, NULL, TRUE, flags, env_block,
       wworkingDirectory.size() == 0 ? NULL : wworkingDirectory.c_str(),
-      reinterpret_cast<STARTUPINFO *>(&startupinfoex), &pi);
+      &startupinfo, &pi);
 
   if (!result) {
     // Call GetLastError before we make any other system calls.
@@ -185,6 +131,13 @@ 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,7 +924,9 @@ 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,8 +274,10 @@ 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,6 +90,7 @@ 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!");
 
@@ -129,3 +130,4 @@ TEST(Host, LaunchProcessDuplicatesHandle) {
   ASSERT_THAT_EXPECTED(bytes_read, llvm::Succeeded());
   ASSERT_EQ(llvm::StringRef(msg, *bytes_read), test_msg);
 }
+#endif