https://github.com/charles-zablit updated https://github.com/llvm/llvm-project/pull/179274
>From a262b0cde556cc87065ee99770d89ac72a644604 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Tue, 3 Feb 2026 14:18:43 +0000 Subject: [PATCH] [lldb][windows] refactor FileAction --- lldb/include/lldb/Host/FileAction.h | 85 +++++++++++-- lldb/include/lldb/Host/ProcessLaunchInfo.h | 1 + lldb/source/Host/common/FileAction.cpp | 112 ++++++++++++++++-- lldb/source/Host/common/ProcessLaunchInfo.cpp | 11 ++ .../Host/windows/ProcessLauncherWindows.cpp | 26 ++-- .../gdb-remote/GDBRemoteCommunication.cpp | 4 +- lldb/tools/lldb-server/lldb-platform.cpp | 3 +- lldb/unittests/Host/HostTest.cpp | 3 +- 8 files changed, 214 insertions(+), 31 deletions(-) diff --git a/lldb/include/lldb/Host/FileAction.h b/lldb/include/lldb/Host/FileAction.h index b2cc8be32d296..1f65c42c648f3 100644 --- a/lldb/include/lldb/Host/FileAction.h +++ b/lldb/include/lldb/Host/FileAction.h @@ -10,10 +10,16 @@ #define LLDB_HOST_FILEACTION_H #include "lldb/Utility/FileSpec.h" +#include "lldb/lldb-types.h" #include <string> +#include <variant> 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 +31,93 @@ class FileAction { FileAction(); + /// Reset this FileAction to its default state. void Clear(); + /// Configure this action to close a file descriptor. bool Close(int fd); - bool Duplicate(int fd, int dup_fd); + /// Configure this action to duplicate a file descriptor. + /// + /// \param[in] fd + /// The file descriptor to duplicate. + /// \param[in] dup_file + /// The target file descriptor number. + bool Duplicate(int fd, int dup_file); + bool Duplicate(void* fh, void* dup_fh); + +#ifdef _WIN32 + /// Configure this action to open a file (Windows handle version). + /// + /// This method will open a CRT file descriptor to the handle and + /// store that descriptor internally. + /// + /// \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(void *fh, const FileSpec &file_spec, bool read, bool write); +#endif + /// 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); - int GetFD() const { return m_fd; } + /// Get the file descriptor this action applies to. + int GetFD() const; + +#ifdef _WIN32 + /// Get the Windows handle for this file descriptor. + /// + /// The handle is converted from the file descriptor which is stored + /// internally. The initial file descriptor must have been registered in the + /// CRT before. + void *GetHandle() const; +#endif + /// Get the type of action. Action GetAction() const { return m_action; } - int GetActionArgument() const { return m_arg; } +#ifdef _WIN32 + /// Get the action-specific argument. + /// + /// For eFileActionOpen, returns the open flags (O_RDONLY, etc.). + /// For eFileActionDuplicate, returns the target fd to duplicate to. + void* GetActionArgumentHandle() const; +#endif + + /// 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; + /// 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; + /// An existing file descriptor. + std::variant<int, void*> m_file = -1; + /// oflag for eFileActionOpen, dup_fd for eFileActionDuplicate. + std::variant<int, void*> m_arg = -1; + /// File spec to use for opening after fork or posix_spawn. + FileSpec m_file_spec; }; } // namespace lldb_private diff --git a/lldb/include/lldb/Host/ProcessLaunchInfo.h b/lldb/include/lldb/Host/ProcessLaunchInfo.h index 023d150503da3..aaa674eb3fefb 100644 --- a/lldb/include/lldb/Host/ProcessLaunchInfo.h +++ b/lldb/include/lldb/Host/ProcessLaunchInfo.h @@ -53,6 +53,7 @@ class ProcessLaunchInfo : public ProcessInfo { bool AppendCloseFileAction(int fd); bool AppendDuplicateFileAction(int fd, int dup_fd); + bool AppendDuplicateFileAction(void* fh, void* dup_fh); bool AppendOpenFileAction(int fd, const FileSpec &file_spec, bool read, bool write); diff --git a/lldb/source/Host/common/FileAction.cpp b/lldb/source/Host/common/FileAction.cpp index ec271f7b920d8..734978408a90e 100644 --- a/lldb/source/Host/common/FileAction.cpp +++ b/lldb/source/Host/common/FileAction.cpp @@ -12,6 +12,10 @@ #include "lldb/Host/PosixApi.h" #include "lldb/Utility/Stream.h" +#ifdef _WIN32 +#include "lldb/Host/windows/windows.h" +#endif + using namespace lldb_private; // FileAction member functions @@ -20,18 +24,58 @@ FileAction::FileAction() : m_file_spec() {} void FileAction::Clear() { m_action = eFileActionNone; - m_fd = -1; + m_file = -1; m_arg = -1; m_file_spec.Clear(); } +#ifdef _WIN32 +HANDLE FileAction::GetHandle() const { + if (std::holds_alternative<HANDLE>(m_file)) + return std::get<HANDLE>(m_file); + int file = std::get<int>(m_file); + switch (file) { + 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; + // return (HANDLE)_get_osfhandle(file); + } +} +#endif + const FileSpec &FileAction::GetFileSpec() const { return m_file_spec; } +#ifdef _WIN32 +bool FileAction::Open(HANDLE fh, const FileSpec &file_spec, bool read, + bool write) { + if ((read || write) && fh != INVALID_HANDLE_VALUE && file_spec) { + m_action = eFileActionOpen; + m_file = 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; +} +#endif + bool FileAction::Open(int fd, const FileSpec &file_spec, bool read, bool write) { if ((read || write) && fd >= 0 && file_spec) { m_action = eFileActionOpen; - m_fd = fd; + m_file = fd; if (read && write) m_arg = O_NOCTTY | O_CREAT | O_RDWR; else if (read) @@ -46,39 +90,89 @@ bool FileAction::Open(int fd, const FileSpec &file_spec, bool read, return false; } +int FileAction::GetFD() const { + #ifdef _WIN32 + if (std::holds_alternative<int>(m_file)) + return std::get<int>(m_file); + return -1; + // return _open_osfhandle((intptr_t)std::get<HANDLE>(m_file), NULL); + #else + return std::get<int>(m_file); + #endif +} + bool FileAction::Close(int fd) { Clear(); if (fd >= 0) { m_action = eFileActionClose; - m_fd = fd; + m_file = fd; + return true; } - return m_fd >= 0; + return false; } bool FileAction::Duplicate(int fd, int dup_fd) { Clear(); if (fd >= 0 && dup_fd >= 0) { m_action = eFileActionDuplicate; - m_fd = fd; + m_file = fd; m_arg = dup_fd; + return true; + } + return false; +} + +bool FileAction::Duplicate(HANDLE fh, HANDLE dup_fh) { + Clear(); + if (fh != INVALID_HANDLE_VALUE && dup_fh != INVALID_HANDLE_VALUE) { + m_action = eFileActionDuplicate; + m_file = fh; + m_arg = dup_fh; + return true; } - return m_fd >= 0; + return false; +} + +#ifdef _WIN32 +/// Get the action-specific argument. +/// +/// For eFileActionOpen, returns the open flags (O_RDONLY, etc.). +/// For eFileActionDuplicate, returns the target fd to duplicate to. +HANDLE FileAction::GetActionArgumentHandle() const { + if (std::holds_alternative<HANDLE>(m_arg)) + return std::get<void *>(m_arg); + return INVALID_HANDLE_VALUE; +} +#endif + +/// Get the action-specific argument. +/// +/// For eFileActionOpen, returns the open flags (O_RDONLY, etc.). +/// For eFileActionDuplicate, returns the target fd to duplicate to. +int FileAction::GetActionArgument() const { +#ifdef _WIN32 + if (std::holds_alternative<int>(m_arg)) + return std::get<int>(m_arg); + return -1; +#else + return std::get<int>(m_arg); +#endif } void FileAction::Dump(Stream &stream) const { stream.PutCString("file action: "); switch (m_action) { case eFileActionClose: - stream.Printf("close fd %d", m_fd); + stream.Printf("close fd %d", m_file); break; case eFileActionDuplicate: - stream.Printf("duplicate fd %d to %d", m_fd, m_arg); + stream.Printf("duplicate fd %d to %d", m_file, m_arg); break; case eFileActionNone: stream.PutCString("no action"); break; case eFileActionOpen: - stream.Printf("open fd %d with '%s', OFLAGS = 0x%x", m_fd, + stream.Printf("open fd %d with '%s', OFLAGS = 0x%x", m_file, m_file_spec.GetPath().c_str(), m_arg); break; } diff --git a/lldb/source/Host/common/ProcessLaunchInfo.cpp b/lldb/source/Host/common/ProcessLaunchInfo.cpp index e82cc11187fe5..085caaf09f84a 100644 --- a/lldb/source/Host/common/ProcessLaunchInfo.cpp +++ b/lldb/source/Host/common/ProcessLaunchInfo.cpp @@ -85,6 +85,17 @@ bool ProcessLaunchInfo::AppendDuplicateFileAction(int fd, int dup_fd) { return false; } +#ifdef _WIN32 +bool ProcessLaunchInfo::AppendDuplicateFileAction(void* fh, void* dup_fh) { + FileAction 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; diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp index ec1a20ebb2200..959748adab1b0 100644 --- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp +++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp @@ -241,9 +241,15 @@ llvm::ErrorOr<std::vector<HANDLE>> ProcessLauncherWindows::GetInheritedHandles( 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 (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(act->GetHandle()); + else if (act->GetActionArgumentHandle() != INVALID_HANDLE_VALUE && act->GetHandle() == act->GetActionArgumentHandle()) + inherited_handles.push_back(act->GetHandle()); } if (inherited_handles.empty()) @@ -273,16 +279,20 @@ ProcessLauncherWindows::GetStdioHandle(const ProcessLaunchInfo &launch_info, 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; } const std::string path = action->GetFileSpec().GetPath(); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index d7e4b2b9546b2..f14a7663c12d2 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -860,7 +860,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); @@ -886,7 +886,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 } diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 59b1eb419bc2b..0e2828fb18ea9 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -326,8 +326,7 @@ 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)); diff --git a/lldb/unittests/Host/HostTest.cpp b/lldb/unittests/Host/HostTest.cpp index c88c764f24646..6ae74d3a2300b 100644 --- a/lldb/unittests/Host/HostTest.cpp +++ b/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(); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
