[Lldb-commits] [lldb] [lldb][windows] refactor FileAction (PR #179274)
Charles Zablit via lldb-commits
lldb-commits at lists.llvm.org
Mon Feb 2 08:25:59 PST 2026
https://github.com/charles-zablit created https://github.com/llvm/llvm-project/pull/179274
This patch refactors the FileAction class and its callsites to properly handle Windows file handles.
Before this patch, we were casting Windows file handles to `int` to convert them to file descriptors and casting them back to a Handle when using them. This is error prone. This patch refactors the callsites casting a Handle to an (int) to make sure they are properly converted to file descriptors and back.
>From 5acf9b7b4834426e90f5999e4434bea8ffcb28ec Mon Sep 17 00:00:00 2001
From: Charles Zablit <c_zablit at apple.com>
Date: Mon, 2 Feb 2026 16:18:19 +0000
Subject: [PATCH] [lldb][windows] refactor FileAction
---
lldb/include/lldb/Host/FileAction.h | 67 +++++++++++++++++--
lldb/source/Host/common/FileAction.cpp | 18 +++++
.../Host/windows/ProcessLauncherWindows.cpp | 28 +++++---
.../gdb-remote/GDBRemoteCommunication.cpp | 14 +++-
lldb/tools/lldb-server/lldb-platform.cpp | 8 ++-
lldb/unittests/Host/HostTest.cpp | 8 ++-
6 files changed, 120 insertions(+), 23 deletions(-)
diff --git a/lldb/include/lldb/Host/FileAction.h b/lldb/include/lldb/Host/FileAction.h
index b2cc8be32d296..7d66dbe7fa440 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,79 @@ 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);
+
+#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);
+ /// Get the file descriptor this action applies to.
int GetFD() const { return m_fd; }
+#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; }
+ /// 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
+ 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; ///< File spec to use for opening after fork or posix_spawn.
};
} // namespace lldb_private
diff --git a/lldb/source/Host/common/FileAction.cpp b/lldb/source/Host/common/FileAction.cpp
index ec271f7b920d8..5ae9eead2bb96 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
@@ -25,8 +29,22 @@ void FileAction::Clear() {
m_file_spec.Clear();
}
+#ifdef _WIN32
+HANDLE FileAction::GetHandle() const {
+ return (HANDLE)_get_osfhandle(m_fd);
+#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) {
+ int fd = _open_osfhandle((intptr_t)fh, NULL);
+ return Open(fd, file_spec, read, write);
+}
+#endif
+
bool FileAction::Open(int fd, const FileSpec &file_spec, bool read,
bool write) {
if ((read || write) && fd >= 0 && file_spec) {
diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
index ec1a20ebb2200..8afbf1bf1c38b 100644
--- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp
+++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
@@ -223,7 +223,7 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info,
llvm::ErrorOr<std::vector<HANDLE>> ProcessLauncherWindows::GetInheritedHandles(
const ProcessLaunchInfo &launch_info, STARTUPINFOEXW &startupinfoex,
HANDLE stdout_handle, HANDLE stderr_handle, HANDLE stdin_handle) {
- std::vector<HANDLE> inherited_handles;
+ std::set<HANDLE> unique_handles;
startupinfoex.StartupInfo.hStdError =
stderr_handle ? stderr_handle : GetStdHandle(STD_ERROR_HANDLE);
@@ -233,21 +233,23 @@ llvm::ErrorOr<std::vector<HANDLE>> ProcessLauncherWindows::GetInheritedHandles(
stdout_handle ? stdout_handle : GetStdHandle(STD_OUTPUT_HANDLE);
if (startupinfoex.StartupInfo.hStdError)
- inherited_handles.push_back(startupinfoex.StartupInfo.hStdError);
+ unique_handles.insert(startupinfoex.StartupInfo.hStdError);
if (startupinfoex.StartupInfo.hStdInput)
- inherited_handles.push_back(startupinfoex.StartupInfo.hStdInput);
+ unique_handles.insert(startupinfoex.StartupInfo.hStdInput);
if (startupinfoex.StartupInfo.hStdOutput)
- inherited_handles.push_back(startupinfoex.StartupInfo.hStdOutput);
+ unique_handles.insert(startupinfoex.StartupInfo.hStdOutput);
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()));
+ unique_handles.insert(act->GetHandle());
}
+ std::vector<HANDLE> inherited_handles(unique_handles.begin(), unique_handles.end());
+
if (inherited_handles.empty())
- return inherited_handles;
+ return std::vector<HANDLE>{};
if (!UpdateProcThreadAttribute(
startupinfoex.lpAttributeList, /*dwFlags=*/0,
@@ -273,16 +275,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..264b860165b28 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -860,7 +860,12 @@ 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);
+ #ifdef _WIN32
+ int fd = _open_osfhandle((intptr_t)*comm_fd, NULL);
+ #else
+ int fd = *commd_fd;
+ #endif
+ launch_info.AppendDuplicateFileAction(fd, fd);
} else {
llvm::StringRef url = std::get<llvm::StringRef>(comm);
LLDB_LOG(log, "debugserver listens on: {0}", url);
@@ -886,7 +891,12 @@ 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);
+ #ifdef _WIN32
+ int write_fd = _open_osfhandle((intptr_t)*comm_fd, NULL);
+ #else
+ int write_fd = *commd_fd;
+ #endif
+ launch_info.AppendDuplicateFileAction(write_fd, write_fd);
#endif
}
diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp
index 59b1eb419bc2b..1493c149a4eb2 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -326,8 +326,12 @@ 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());
+ #ifdef _WIN32
+ int fd = _open_osfhandle((intptr_t)shared_socket.GetSendableFD(), NULL);
+ #else
+ int fd = shared_socket.GetSendableFD();
+ #endif
+ launch_info.AppendDuplicateFileAction(fd, fd);
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..2429989218508 100644
--- a/lldb/unittests/Host/HostTest.cpp
+++ b/lldb/unittests/Host/HostTest.cpp
@@ -157,8 +157,12 @@ 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());
+ #ifdef _WIN32
+ int fd = _open_osfhandle((intptr_t)pipe.GetWritePipe(), NULL);
+ #else
+ int fd = pipe.GetWritePipe();
+ #endif
+ info.AppendDuplicateFileAction(fd, fd);
info.SetMonitorProcessCallback(&ProcessLaunchInfo::NoOpMonitorCallback);
ASSERT_THAT_ERROR(Host::LaunchProcess(info).takeError(), llvm::Succeeded());
pipe.CloseWriteFileDescriptor();
More information about the lldb-commits
mailing list