[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
Hu Jialun via lldb-commits
lldb-commits at lists.llvm.org
Tue Jan 7 07:02:47 PST 2025
https://github.com/SuibianP updated https://github.com/llvm/llvm-project/pull/121269
>From b475e34ef1522b2919fc881d88bfa817bb719aa4 Mon Sep 17 00:00:00 2001
From: Hu Jialun <jialunhu.intern at razer.com>
Date: Sat, 28 Dec 2024 22:39:33 +0800
Subject: [PATCH] [lldb-dap] Implement runInTerminal for Windows
Currently, the named pipe is passed by name and a transient ofstream is
constructed at each I/O request. This assumes,
- Blocking semantics: FIFO I/O waits for the other side to connect.
- Buffered semantics: Closing one side does not discard existing data.
The former can be replaced by WaitNamedPipe/ConnectNamedPipe on Win32,
but the second cannot be easily worked around. It is also impossible to
have another "keep-alive" pipe server instance, as server-client pairs
are fixed on connection on Win32 and the client may get connected to it
instead of the real one.
Refactor FifoFile[IO] to use an open file handles rather than file name.
---
lldb/tools/lldb-dap/FifoFiles.cpp | 110 +++++++++++++++++++++-----
lldb/tools/lldb-dap/FifoFiles.h | 31 ++++----
lldb/tools/lldb-dap/RunInTerminal.cpp | 27 +++++--
lldb/tools/lldb-dap/RunInTerminal.h | 4 +-
lldb/tools/lldb-dap/lldb-dap.cpp | 13 ++-
5 files changed, 138 insertions(+), 47 deletions(-)
diff --git a/lldb/tools/lldb-dap/FifoFiles.cpp b/lldb/tools/lldb-dap/FifoFiles.cpp
index 1f1bba80bd3b11..f2544cfe974a54 100644
--- a/lldb/tools/lldb-dap/FifoFiles.cpp
+++ b/lldb/tools/lldb-dap/FifoFiles.cpp
@@ -9,7 +9,13 @@
#include "FifoFiles.h"
#include "JSONUtils.h"
-#if !defined(_WIN32)
+#include "llvm/Support/FileSystem.h"
+
+#if defined(_WIN32)
+#include <Windows.h>
+#include <fcntl.h>
+#include <io.h>
+#else
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
@@ -24,26 +30,65 @@ using namespace llvm;
namespace lldb_dap {
-FifoFile::FifoFile(StringRef path) : m_path(path) {}
+std::error_code EC;
+FifoFile::FifoFile(StringRef path)
+ : m_path(path), m_file(fopen(path.data(), "r+")) {
+ if (m_file == nullptr) {
+ EC = std::error_code(errno, std::generic_category());
+ llvm::errs() << "Failed to open fifo file: " << path << EC.message()
+ << "\n";
+ return;
+ }
+ if (setvbuf(m_file, NULL, _IONBF, 0))
+ llvm::errs() << "Error setting unbuffered mode on C FILE\n";
+}
+FifoFile::FifoFile(StringRef path, FILE *f) : m_path(path), m_file(f) {}
FifoFile::~FifoFile() {
+ fclose(m_file);
#if !defined(_WIN32)
unlink(m_path.c_str());
#endif
+ // Unreferenced named pipes are deleted automatically on Win32
}
-Expected<std::shared_ptr<FifoFile>> CreateFifoFile(StringRef path) {
-#if defined(_WIN32)
- return createStringError(inconvertibleErrorCode(), "Unimplemented");
+// This probably belongs to llvm::sys::fs as another FSEntity type
+std::error_code createNamedPipe(const Twine &Prefix, StringRef Suffix,
+ int &ResultFd,
+ SmallVectorImpl<char> &ResultPath) {
+ const char *Middle = Suffix.empty() ? "-%%%%%%" : "-%%%%%%.";
+ auto EC = sys::fs::getPotentiallyUniqueFileName(
+#ifdef _WIN32
+ "\\\\.\\pipe\\LOCAL\\"
+#else
+ "/tmp/"
+#endif
+ + Prefix + Middle + Suffix,
+ ResultPath);
+ if (EC)
+ return EC;
+ ResultPath.push_back(0);
+ const char *path = ResultPath.data();
+#ifdef _WIN32
+ HANDLE h = ::CreateNamedPipeA(
+ path, PIPE_ACCESS_DUPLEX | FILE_FLAG_FIRST_PIPE_INSTANCE,
+ PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, 1, 1024, 1024, 0, NULL);
+ if (h == INVALID_HANDLE_VALUE)
+ return std::error_code(::GetLastError(), std::system_category());
+ ResultFd = _open_osfhandle((intptr_t)h, _O_TEXT | _O_RDWR);
+ if (ResultFd == -1)
+ return std::error_code(::GetLastError(), std::system_category());
#else
- if (int err = mkfifo(path.data(), 0600))
- return createStringError(std::error_code(err, std::generic_category()),
- "Couldn't create fifo file: %s", path.data());
- return std::make_shared<FifoFile>(path);
+ if (mkfifo(path, 0600) == -1)
+ return std::error_code(errno, std::generic_category());
+ EC = openFileForWrite(ResultPath, ResultFd, sys::fs::CD_OpenExisting, sys::fs::OF_None, 0600);
+ if (EC)
+ return EC;
#endif
+ return std::error_code();
}
-FifoFileIO::FifoFileIO(StringRef fifo_file, StringRef other_endpoint_name)
+FifoFileIO::FifoFileIO(FifoFile fifo_file, StringRef other_endpoint_name)
: m_fifo_file(fifo_file), m_other_endpoint_name(other_endpoint_name) {}
Expected<json::Value> FifoFileIO::ReadJSON(std::chrono::milliseconds timeout) {
@@ -52,13 +97,20 @@ Expected<json::Value> FifoFileIO::ReadJSON(std::chrono::milliseconds timeout) {
std::optional<std::string> line;
std::future<void> *future =
new std::future<void>(std::async(std::launch::async, [&]() {
- std::ifstream reader(m_fifo_file, std::ifstream::in);
- std::string buffer;
- std::getline(reader, buffer);
- if (!buffer.empty())
- line = buffer;
+ rewind(m_fifo_file.m_file);
+ constexpr size_t buffer_size = 2048;
+ char buffer[buffer_size];
+ char *ptr = fgets(buffer, buffer_size, m_fifo_file.m_file);
+ if (ptr == nullptr || *ptr == 0)
+ return;
+ size_t len = strlen(buffer);
+ if (len <= 1)
+ return;
+ buffer[len - 1] = '\0'; // remove newline
+ line = buffer;
}));
- if (future->wait_for(timeout) == std::future_status::timeout || !line)
+
+ if (future->wait_for(timeout) == std::future_status::timeout)
// Indeed this is a leak, but it's intentional. "future" obj destructor
// will block on waiting for the worker thread to join. And the worker
// thread might be stuck in blocking I/O. Intentionally leaking the obj
@@ -69,6 +121,11 @@ Expected<json::Value> FifoFileIO::ReadJSON(std::chrono::milliseconds timeout) {
return createStringError(inconvertibleErrorCode(),
"Timed out trying to get messages from the " +
m_other_endpoint_name);
+ if (!line) {
+ return createStringError(inconvertibleErrorCode(),
+ "Failed to get messages from the " +
+ m_other_endpoint_name);
+ }
delete future;
return json::parse(*line);
}
@@ -78,8 +135,12 @@ Error FifoFileIO::SendJSON(const json::Value &json,
bool done = false;
std::future<void> *future =
new std::future<void>(std::async(std::launch::async, [&]() {
- std::ofstream writer(m_fifo_file, std::ofstream::out);
- writer << JSONToString(json) << std::endl;
+ rewind(m_fifo_file.m_file);
+ auto payload = JSONToString(json);
+ if (fputs(payload.c_str(), m_fifo_file.m_file) == EOF ||
+ fputc('\n', m_fifo_file.m_file) == EOF) {
+ return;
+ }
done = true;
}));
if (future->wait_for(timeout) == std::future_status::timeout || !done) {
@@ -98,4 +159,17 @@ Error FifoFileIO::SendJSON(const json::Value &json,
return Error::success();
}
+Error FifoFileIO::WaitForPeer() {
+#ifdef _WIN32
+ if (!::ConnectNamedPipe((HANDLE)_get_osfhandle(fileno(m_fifo_file.m_file)),
+ NULL) &&
+ GetLastError() != ERROR_PIPE_CONNECTED) {
+ return createStringError(
+ std::error_code(GetLastError(), std::system_category()),
+ "Failed to connect to the " + m_other_endpoint_name);
+ }
+#endif
+ return Error::success();
+}
+
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/FifoFiles.h b/lldb/tools/lldb-dap/FifoFiles.h
index 633ebeb2aedd45..694d6e21866741 100644
--- a/lldb/tools/lldb-dap/FifoFiles.h
+++ b/lldb/tools/lldb-dap/FifoFiles.h
@@ -11,8 +11,10 @@
#include "llvm/Support/Error.h"
#include "llvm/Support/JSON.h"
+#include "llvm/Support/raw_ostream.h"
#include <chrono>
+#include <fstream>
namespace lldb_dap {
@@ -21,21 +23,18 @@ namespace lldb_dap {
/// The file is destroyed when the destructor is invoked.
struct FifoFile {
FifoFile(llvm::StringRef path);
+ FifoFile(llvm::StringRef path, FILE *f);
+ // FifoFile(llvm::StringRef path, FILE *f);
~FifoFile();
std::string m_path;
+ FILE *m_file;
};
-/// Create a fifo file in the filesystem.
-///
-/// \param[in] path
-/// The path for the fifo file.
-///
-/// \return
-/// A \a std::shared_ptr<FifoFile> if the file could be created, or an
-/// \a llvm::Error in case of failures.
-llvm::Expected<std::shared_ptr<FifoFile>> CreateFifoFile(llvm::StringRef path);
+std::error_code createNamedPipe(const llvm::Twine &Prefix,
+ llvm::StringRef Suffix, int &ResultFd,
+ llvm::SmallVectorImpl<char> &ResultPath);
class FifoFileIO {
public:
@@ -45,7 +44,7 @@ class FifoFileIO {
/// \param[in] other_endpoint_name
/// A human readable name for the other endpoint that will communicate
/// using this file. This is used for error messages.
- FifoFileIO(llvm::StringRef fifo_file, llvm::StringRef other_endpoint_name);
+ FifoFileIO(FifoFile fifo_file, llvm::StringRef other_endpoint_name);
/// Read the next JSON object from the underlying input fifo file.
///
@@ -71,12 +70,14 @@ class FifoFileIO {
/// \return
/// An \a llvm::Error object indicating whether the data was consumed by
/// a reader or not.
- llvm::Error SendJSON(
- const llvm::json::Value &json,
- std::chrono::milliseconds timeout = std::chrono::milliseconds(20000));
+ llvm::Error
+ SendJSON(const llvm::json::Value &json,
+ std::chrono::milliseconds timeout = std::chrono::milliseconds(2000));
+
+ llvm::Error WaitForPeer();
-private:
- std::string m_fifo_file;
+ // private:
+ FifoFile m_fifo_file;
std::string m_other_endpoint_name;
};
diff --git a/lldb/tools/lldb-dap/RunInTerminal.cpp b/lldb/tools/lldb-dap/RunInTerminal.cpp
index 4fe09e2885a8e5..14c9db74e4cdaa 100644
--- a/lldb/tools/lldb-dap/RunInTerminal.cpp
+++ b/lldb/tools/lldb-dap/RunInTerminal.cpp
@@ -97,7 +97,7 @@ static Error ToError(const RunInTerminalMessage &message) {
RunInTerminalLauncherCommChannel::RunInTerminalLauncherCommChannel(
StringRef comm_file)
- : m_io(comm_file, "debug adaptor") {}
+ : m_io(FifoFile(comm_file), "debug adaptor") {}
Error RunInTerminalLauncherCommChannel::WaitUntilDebugAdaptorAttaches(
std::chrono::milliseconds timeout) {
@@ -122,9 +122,13 @@ void RunInTerminalLauncherCommChannel::NotifyError(StringRef error) {
}
RunInTerminalDebugAdapterCommChannel::RunInTerminalDebugAdapterCommChannel(
- StringRef comm_file)
+ FifoFile comm_file)
: m_io(comm_file, "runInTerminal launcher") {}
+Error RunInTerminalDebugAdapterCommChannel::WaitForLauncher() {
+ return m_io.WaitForPeer();
+}
+
// Can't use \a std::future<llvm::Error> because it doesn't compile on Windows
std::future<lldb::SBError>
RunInTerminalDebugAdapterCommChannel::NotifyDidAttach() {
@@ -158,13 +162,24 @@ std::string RunInTerminalDebugAdapterCommChannel::GetLauncherError() {
}
Expected<std::shared_ptr<FifoFile>> CreateRunInTerminalCommFile() {
+ int comm_fd;
SmallString<256> comm_file;
- if (std::error_code EC = sys::fs::getPotentiallyUniqueTempFileName(
- "lldb-dap-run-in-terminal-comm", "", comm_file))
+ if (std::error_code EC = createNamedPipe("lldb-dap-run-in-terminal-comm", "",
+ comm_fd, comm_file))
return createStringError(EC, "Error making unique file name for "
"runInTerminal communication files");
-
- return CreateFifoFile(comm_file.str());
+ FILE *cf = fdopen(comm_fd, "r+");
+ if (setvbuf(cf, NULL, _IONBF, 0))
+ return createStringError(std::error_code(errno, std::generic_category()),
+ "Error setting unbuffered mode on C FILE");
+ // There is no portable way to conjure an ofstream from HANDLE, so use FILE *
+ // llvm::raw_fd_stream does not support getline() and there is no
+ // llvm::buffer_istream
+
+ if (cf == NULL)
+ return createStringError(std::error_code(errno, std::generic_category()),
+ "Error converting file descriptor to C FILE");
+ return std::make_shared<FifoFile>(comm_file, cf);
}
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/RunInTerminal.h b/lldb/tools/lldb-dap/RunInTerminal.h
index b20f8beb6071dd..b725bdb87e9875 100644
--- a/lldb/tools/lldb-dap/RunInTerminal.h
+++ b/lldb/tools/lldb-dap/RunInTerminal.h
@@ -98,7 +98,7 @@ class RunInTerminalLauncherCommChannel {
class RunInTerminalDebugAdapterCommChannel {
public:
- RunInTerminalDebugAdapterCommChannel(llvm::StringRef comm_file);
+ RunInTerminalDebugAdapterCommChannel(FifoFile comm_file);
/// Notify the runInTerminal launcher that it was attached.
///
@@ -118,6 +118,8 @@ class RunInTerminalDebugAdapterCommChannel {
/// default error message if a certain timeout if reached.
std::string GetLauncherError();
+ llvm::Error WaitForLauncher();
+
private:
FifoFileIO m_io;
};
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 7e8f7b5f6df679..f4994041b6eaee 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -2007,7 +2007,7 @@ llvm::Error request_runInTerminal(DAP &dap,
return comm_file_or_err.takeError();
FifoFile &comm_file = *comm_file_or_err.get();
- RunInTerminalDebugAdapterCommChannel comm_channel(comm_file.m_path);
+ RunInTerminalDebugAdapterCommChannel comm_channel(comm_file);
lldb::pid_t debugger_pid = LLDB_INVALID_PROCESS_ID;
#if !defined(_WIN32)
@@ -2025,6 +2025,9 @@ llvm::Error request_runInTerminal(DAP &dap,
}
});
+ auto err = comm_channel.WaitForLauncher();
+ if (err)
+ return err;
if (llvm::Expected<lldb::pid_t> pid = comm_channel.GetLauncherPid())
attach_info.SetProcessID(*pid);
else
@@ -2032,7 +2035,9 @@ llvm::Error request_runInTerminal(DAP &dap,
dap.debugger.SetAsync(false);
lldb::SBError error;
+ llvm::errs() << "Attaching to target\n";
dap.target.Attach(attach_info, error);
+ llvm::errs() << "Attached to target\n";
if (error.Fail())
return llvm::createStringError(llvm::inconvertibleErrorCode(),
@@ -4860,11 +4865,6 @@ static void printHelp(LLDBDAPOptTable &table, llvm::StringRef tool_name) {
static void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg,
llvm::StringRef comm_file,
lldb::pid_t debugger_pid, char *argv[]) {
-#if defined(_WIN32)
- llvm::errs() << "runInTerminal is only supported on POSIX systems\n";
- exit(EXIT_FAILURE);
-#else
-
// On Linux with the Yama security module enabled, a process can only attach
// to its descendants by default. In the runInTerminal case the target
// process is launched by the client so we need to allow tracing explicitly.
@@ -4899,7 +4899,6 @@ static void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg,
comm_channel.NotifyError(error);
llvm::errs() << error << "\n";
exit(EXIT_FAILURE);
-#endif
}
/// used only by TestVSCode_redirection_to_console.py
More information about the lldb-commits
mailing list