[lldb][windows] refactor FileAction (#179274)

lldb/include/lldb/Host/FileAction.h

@@ -14,6 +14,10 @@
 
 namespace lldb_private {
 
+/// Represents a file descriptor action to be performed during process launch.
+///
+/// FileAction encapsulates operations like opening, closing, or duplicating
+/// file descriptors that should be applied when spawning a new process.
 class FileAction {
 public:
   enum Action {
@@ -25,30 +29,58 @@ public:
 
   FileAction();
 
+  /// Reset this FileAction to its default state.
   void Clear();
 
+  /// Configure this action to close a file descriptor.
   bool Close(int fd);
 
+  /// Configure this action to duplicate a file descriptor.
+  ///
+  /// \param[in] fd
+  ///     The file descriptor to duplicate.
+  /// \param[in] dup_fd
+  ///     The target file descriptor number.
   bool Duplicate(int fd, int dup_fd);
 
+  /// Configure this action to open a file.
+  ///
+  /// \param[in] fd
+  ///     The file descriptor to use for the opened file.
+  /// \param[in] file_spec
+  ///     The file to open.
+  /// \param[in] read
+  ///     Open for reading.
+  /// \param[in] write
+  ///     Open for writing.
   bool Open(int fd, const FileSpec &file_spec, bool read, bool write);
 
+  /// Get the file descriptor this action applies to.
   int GetFD() const { return m_fd; }
 
+  /// Get the type of action.
   Action GetAction() const { return m_action; }
 
+  /// Get the action-specific argument.
+  ///
+  /// For eFileActionOpen, returns the open flags (O_RDONLY, etc.).
+  /// For eFileActionDuplicate, returns the target fd to duplicate to.
   int GetActionArgument() const { return m_arg; }
 
+  /// Get the file specification for open actions.
   const FileSpec &GetFileSpec() const;
 
   void Dump(Stream &stream) const;
 
 protected:
-  Action m_action = eFileActionNone; // The action for this file
-  int m_fd = -1;                     // An existing file descriptor
-  int m_arg = -1; // oflag for eFileActionOpen*, dup_fd for eFileActionDuplicate
-  FileSpec
-      m_file_spec; // A file spec to use for opening after fork or posix_spawn
+  /// The action for this file.
+  Action m_action = eFileActionNone;
+  /// The file descriptor this action applies to.
+  int m_fd = -1;
+  /// oflag for eFileActionOpen, dup_fd for eFileActionDuplicate.
+  int m_arg = -1;
+  /// File spec to use for opening after fork or posix_spawn.
+  FileSpec m_file_spec;
 };
 
 } // namespace lldb_private

lldb/include/lldb/Host/ProcessLaunchInfo.h

@@ -19,6 +19,7 @@
 #include "lldb/Host/Host.h"
 #ifdef _WIN32
 #include "lldb/Host/windows/PseudoConsole.h"
+#include "lldb/Host/windows/WindowsFileAction.h"
 #else
 #include "lldb/Host/PseudoTerminal.h"
 #endif
@@ -29,8 +30,10 @@ namespace lldb_private {
 
 #if defined(_WIN32)
 using PTY = PseudoConsole;
+using FileActionImpl = WindowsFileAction;
 #else
 using PTY = PseudoTerminal;
+using FileActionImpl = FileAction;
 #endif
 
 // ProcessLaunchInfo
@@ -46,7 +49,7 @@ public:
                     const FileSpec &stderr_file_spec,
                     const FileSpec &working_dir, uint32_t launch_flags);
 
-  void AppendFileAction(const FileAction &info) {
+  void AppendFileAction(const FileActionImpl &info) {
     m_file_actions.push_back(info);
   }
 
@@ -54,6 +57,10 @@ public:
 
   bool AppendDuplicateFileAction(int fd, int dup_fd);
 
+#ifdef _WIN32
+  bool AppendDuplicateFileAction(HANDLE fh, HANDLE dup_fh);
+#endif
+
   bool AppendOpenFileAction(int fd, const FileSpec &file_spec, bool read,
                             bool write);
 
@@ -168,7 +175,8 @@ protected:
   std::string m_plugin_name;
   FileSpec m_shell;
   Flags m_flags; // Bitwise OR of bits from lldb::LaunchFlags
-  std::vector<FileAction> m_file_actions; // File actions for any other files
+  std::vector<FileActionImpl>
+      m_file_actions; // File actions for any other files
   std::shared_ptr<PTY> m_pty;
   uint32_t m_resume_count = 0; // How many times do we resume after launching
   Host::MonitorChildProcessCallback m_monitor_callback;

lldb/include/lldb/Host/windows/WindowsFileAction.h

@@ -0,0 +1,72 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_HOST_WINDOWS_WINDOWSFILEACTION_H
+#define LLDB_HOST_WINDOWS_WINDOWSFILEACTION_H
+
+#include "lldb/Host/FileAction.h"
+#include "lldb/lldb-types.h"
+
+typedef void *HANDLE;
+
+namespace lldb_private {
+
+/// A Windows-specific extension of FileAction that supports HANDLE-based
+/// file operations in addition to the standard file descriptor operations.
+class WindowsFileAction : public FileAction {
+public:
+  WindowsFileAction() = default;
+
+  /// Allow implicit conversion from a base FileAction. The Windows-specific
+  /// handle fields default to INVALID_HANDLE_VALUE.
+  WindowsFileAction(const FileAction &fa) : FileAction(fa) {}
+
+  /// Reset this WindowsFileAction to its default state.
+  void Clear() {
+    FileAction::Clear();
+    m_handle = LLDB_INVALID_PIPE;
+    m_arg_handle = LLDB_INVALID_PIPE;
+  }
+
+  /// Configure this action to duplicate a Windows file handle.
+  ///
+  /// \param[in] fh
+  ///     The source file handle to duplicate.
+  /// \param[in] dup_fh
+  ///     The target file handle.
+  bool Duplicate(HANDLE fh, HANDLE dup_fh);
+
+  /// Configure this action to associate a Windows file handle with a file.
+  ///
+  /// \param[in] fh
+  ///     The file handle to use for the opened file.
+  /// \param[in] file_spec
+  ///     The file to open.
+  /// \param[in] read
+  ///     Open for reading.
+  /// \param[in] write
+  ///     Open for writing.
+  bool Open(HANDLE fh, const FileSpec &file_spec, bool read, bool write);
+
+  /// Get the Windows HANDLE for this action's file.
+  ///
+  /// If a HANDLE was stored directly, it is returned. Otherwise, the standard
+  /// handles for STDIN/STDOUT/STDERR are returned based on the stored fd.
+  HANDLE GetHandle() const;
+
+  /// Get the Windows HANDLE argument for eFileActionDuplicate actions.
+  HANDLE GetActionArgumentHandle() const;
+
+private:
+  HANDLE m_handle = LLDB_INVALID_PIPE;
+  HANDLE m_arg_handle = LLDB_INVALID_PIPE;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_HOST_WINDOWS_WINDOWSFILEACTION_H

lldb/source/Host/CMakeLists.txt

@@ -80,6 +80,7 @@ if (CMAKE_SYSTEM_NAME MATCHES "Windows")
     windows/ProcessLauncherWindows.cpp
     windows/PseudoConsole.cpp
     windows/ProcessRunLock.cpp
+    windows/WindowsFileAction.cpp
     )
 else()
   add_host_subdirectory(posix

lldb/source/Host/common/ProcessLaunchInfo.cpp

@@ -22,6 +22,7 @@
 
 #ifdef _WIN32
 #include "lldb/Host/windows/PseudoConsole.h"
+#include "lldb/Host/windows/WindowsFileAction.h"
 #else
 #include <climits>
 #endif
@@ -92,6 +93,17 @@ bool ProcessLaunchInfo::AppendDuplicateFileAction(int fd, int dup_fd) {
   return false;
 }
 
+#ifdef _WIN32
+bool ProcessLaunchInfo::AppendDuplicateFileAction(HANDLE fh, HANDLE dup_fh) {
+  WindowsFileAction file_action;
+  if (file_action.Duplicate(fh, dup_fh)) {
+    AppendFileAction(file_action);
+    return true;
+  }
+  return false;
+}
+#endif
+
 bool ProcessLaunchInfo::AppendOpenFileAction(int fd, const FileSpec &file_spec,
                                              bool read, bool write) {
   FileAction file_action;

lldb/source/Host/windows/ProcessLauncherWindows.cpp

@@ -9,6 +9,7 @@
 #include "lldb/Host/windows/ProcessLauncherWindows.h"
 #include "lldb/Host/HostProcess.h"
 #include "lldb/Host/windows/PseudoConsole.h"
+#include "lldb/Host/windows/WindowsFileAction.h"
 #include "lldb/Host/windows/windows.h"
 
 #include "llvm/ADT/SmallVector.h"
@@ -279,10 +280,19 @@ llvm::ErrorOr<std::vector<HANDLE>> ProcessLauncherWindows::GetInheritedHandles(
 
   if (launch_info) {
     for (size_t i = 0; i < launch_info->GetNumFileActions(); ++i) {
-      const FileAction *act = launch_info->GetFileActionAtIndex(i);
-      if (act->GetAction() == FileAction::eFileActionDuplicate &&
+      const WindowsFileAction *act = static_cast<const WindowsFileAction *>(
+          launch_info->GetFileActionAtIndex(i));
+      if (std::find(inherited_handles.begin(), inherited_handles.end(),
+                    act->GetHandle()) != inherited_handles.end())
+        continue;
+      if (act->GetAction() != FileAction::eFileActionDuplicate)
+        continue;
+      if (act->GetActionArgument() != -1 &&
           act->GetFD() == act->GetActionArgument())
-        inherited_handles.push_back(reinterpret_cast<HANDLE>(act->GetFD()));
+        inherited_handles.push_back(act->GetHandle());
+      else if (act->GetActionArgumentHandle() != INVALID_HANDLE_VALUE &&
+               act->GetHandle() == act->GetActionArgumentHandle())
+        inherited_handles.push_back(act->GetHandle());
     }
   }
 
@@ -322,16 +332,20 @@ HANDLE ProcessLauncherWindows::GetStdioHandle(const llvm::StringRef path,
   DWORD share = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE;
   DWORD create = 0;
   DWORD flags = 0;
-  if (fd == STDIN_FILENO) {
+  switch (fd) {
+  case STDIN_FILENO:
     access = GENERIC_READ;
     create = OPEN_EXISTING;
     flags = FILE_ATTRIBUTE_READONLY;
-  }
-  if (fd == STDOUT_FILENO || fd == STDERR_FILENO) {
+    break;
+  case STDERR_FILENO:
+    flags = FILE_FLAG_WRITE_THROUGH;
+  case STDOUT_FILENO:
     access = GENERIC_WRITE;
     create = CREATE_ALWAYS;
-    if (fd == STDERR_FILENO)
-      flags = FILE_FLAG_WRITE_THROUGH;
+    break;
+  default:
+    break;
   }
 
   std::wstring wpath;

lldb/source/Host/windows/WindowsFileAction.cpp

@@ -0,0 +1,64 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include <fcntl.h>
+
+#include "lldb/Host/PosixApi.h"
+#include "lldb/Host/windows/WindowsFileAction.h"
+#include "lldb/Host/windows/windows.h"
+
+using namespace lldb_private;
+
+bool WindowsFileAction::Duplicate(HANDLE fh, HANDLE dup_fh) {
+  Clear();
+  if (fh != INVALID_HANDLE_VALUE && dup_fh != INVALID_HANDLE_VALUE) {
+    m_action = eFileActionDuplicate;
+    m_handle = fh;
+    m_arg_handle = dup_fh;
+    return true;
+  }
+  return false;
+}
+
+bool WindowsFileAction::Open(HANDLE fh, const FileSpec &file_spec, bool read,
+                             bool write) {
+  if ((read || write) && fh != INVALID_HANDLE_VALUE && file_spec) {
+    m_action = eFileActionOpen;
+    m_handle = fh;
+    if (read && write)
+      m_arg = O_NOCTTY | O_CREAT | O_RDWR;
+    else if (read)
+      m_arg = O_NOCTTY | O_RDONLY;
+    else
+      m_arg = O_NOCTTY | O_CREAT | O_WRONLY | O_TRUNC;
+    m_file_spec = file_spec;
+    return true;
+  } else {
+    Clear();
+  }
+  return false;
+}
+
+HANDLE WindowsFileAction::GetHandle() const {
+  if (m_handle != INVALID_HANDLE_VALUE)
+    return m_handle;
+  switch (m_fd) {
+  case STDIN_FILENO:
+    return GetStdHandle(STD_INPUT_HANDLE);
+  case STDOUT_FILENO:
+    return GetStdHandle(STD_OUTPUT_HANDLE);
+  case STDERR_FILENO:
+    return GetStdHandle(STD_ERROR_HANDLE);
+  default:
+    return INVALID_HANDLE_VALUE;
+  }
+}
+
+HANDLE WindowsFileAction::GetActionArgumentHandle() const {
+  return m_arg_handle;
+}

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

@@ -859,7 +859,7 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
     debugserver_args.AppendArgument(llvm::formatv("--fd={0}", *comm_fd).str());
     // Send "comm_fd" down to the inferior so it can use it to communicate back
     // with this process.
-    launch_info.AppendDuplicateFileAction((int64_t)*comm_fd, (int64_t)*comm_fd);
+    launch_info.AppendDuplicateFileAction(*comm_fd, *comm_fd);
   } else {
     llvm::StringRef url = std::get<llvm::StringRef>(comm);
     LLDB_LOG(log, "debugserver listens on: {0}", url);
@@ -885,7 +885,7 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
     pipe_t write = socket_pipe.GetWritePipe();
     debugserver_args.AppendArgument(llvm::StringRef("--pipe"));
     debugserver_args.AppendArgument(llvm::to_string(write));
-    launch_info.AppendDuplicateFileAction((int64_t)write, (int64_t)write);
+    launch_info.AppendDuplicateFileAction(write, write);
 #endif
   }
 

lldb/tools/lldb-server/lldb-platform.cpp

@@ -325,8 +325,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()));
-  launch_info.AppendDuplicateFileAction((int64_t)shared_socket.GetSendableFD(),
-                                        (int64_t)shared_socket.GetSendableFD());
+  launch_info.AppendDuplicateFileAction(shared_socket.GetSendableFD(),
+                                        shared_socket.GetSendableFD());
   if (gdb_port) {
     self_args.AppendArgument(llvm::StringRef("--gdbserver-port"));
     self_args.AppendArgument(llvm::to_string(gdb_port));

lldb/unittests/Host/HostTest.cpp

@@ -157,8 +157,7 @@ TEST(Host, LaunchProcessDuplicatesHandle) {
       "--gtest_filter=Host.LaunchProcessDuplicatesHandle");
   info.GetArguments().AppendArgument(
       ("--test-arg=" + llvm::Twine((uint64_t)pipe.GetWritePipe())).str());
-  info.AppendDuplicateFileAction((uint64_t)pipe.GetWritePipe(),
-                                 (uint64_t)pipe.GetWritePipe());
+  info.AppendDuplicateFileAction(pipe.GetWritePipe(), pipe.GetWritePipe());
   info.SetMonitorProcessCallback(&ProcessLaunchInfo::NoOpMonitorCallback);
   ASSERT_THAT_ERROR(Host::LaunchProcess(info).takeError(), llvm::Succeeded());
   pipe.CloseWriteFileDescriptor();