https://github.com/SuibianP updated
https://github.com/llvm/llvm-project/pull/121269
>From dc866c2d106292b7fe49f8c1ac45dbd3905b81d6 Mon Sep 17 00:00:00 2001
From: Hu Jialun <jialunhu.int...@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.
---
Win32 provides no way to replace the process image. Under the hood exec*
actually creates a new process with a new PID. DebugActiveProcess also
cannot get notified of process creations.
Create the new process in a suspended state and resume it after attach.
---
lldb/packages/Python/lldbsuite/test/dotest.py | 2 +-
.../runInTerminal/TestDAP_runInTerminal.py | 4 -
.../API/tools/lldb-dap/runInTerminal/main.c | 6 +-
lldb/tools/lldb-dap/FifoFiles.cpp | 120 +++++++++++++++---
lldb/tools/lldb-dap/FifoFiles.h | 27 ++--
lldb/tools/lldb-dap/RunInTerminal.cpp | 36 ++++--
lldb/tools/lldb-dap/RunInTerminal.h | 6 +-
lldb/tools/lldb-dap/lldb-dap.cpp | 50 ++++++--
8 files changed, 193 insertions(+), 58 deletions(-)
diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 681ea1638f2d6f..538cece8b4913d 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -545,7 +545,7 @@ def setupSysPath():
lldbDir = os.path.dirname(lldbtest_config.lldbExec)
- lldbDAPExec = os.path.join(lldbDir, "lldb-dap")
+ lldbDAPExec = os.path.join(lldbDir, "lldb-dap.exe" if os.name == "nt" else
"lldb-dap")
if is_exe(lldbDAPExec):
os.environ["LLDBDAP_EXEC"] = lldbDAPExec
diff --git
a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
index ac96bcc1364a27..e1a75bdcf440d5 100644
--- a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
+++ b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
@@ -43,7 +43,6 @@ def isTestSupported(self):
except:
return False
- @skipIfWindows
@skipIf(archs=no_match(["x86_64"]))
def test_runInTerminal(self):
if not self.isTestSupported():
@@ -90,7 +89,6 @@ def test_runInTerminal(self):
env = self.dap_server.request_evaluate("foo")["body"]["result"]
self.assertIn("bar", env)
- @skipIf(archs=no_match(["x86_64"]))
def test_runInTerminalWithObjectEnv(self):
if not self.isTestSupported():
return
@@ -113,7 +111,6 @@ def test_runInTerminalWithObjectEnv(self):
self.assertIn("FOO", request_envs)
self.assertEqual("BAR", request_envs["FOO"])
- @skipIfWindows
@skipIf(archs=no_match(["x86_64"]))
def test_runInTerminalInvalidTarget(self):
if not self.isTestSupported():
@@ -132,7 +129,6 @@ def test_runInTerminalInvalidTarget(self):
response["message"],
)
- @skipIfWindows
@skipIf(archs=no_match(["x86_64"]))
def test_missingArgInRunInTerminalLauncher(self):
if not self.isTestSupported():
diff --git a/lldb/test/API/tools/lldb-dap/runInTerminal/main.c
b/lldb/test/API/tools/lldb-dap/runInTerminal/main.c
index 676bd830e657b4..9cc25b2e45a494 100644
--- a/lldb/test/API/tools/lldb-dap/runInTerminal/main.c
+++ b/lldb/test/API/tools/lldb-dap/runInTerminal/main.c
@@ -1,11 +1,13 @@
#include <stdio.h>
#include <stdlib.h>
-#include <unistd.h>
+
+#include <threads.h>
+#include <time.h>
int main(int argc, char *argv[]) {
const char *foo = getenv("FOO");
for (int counter = 1;; counter++) {
- sleep(1); // breakpoint
+ thrd_sleep(&(struct timespec){.tv_sec = 1}, NULL); // breakpoint
}
return 0;
}
diff --git a/lldb/tools/lldb-dap/FifoFiles.cpp
b/lldb/tools/lldb-dap/FifoFiles.cpp
index 1f1bba80bd3b11..e3ed7c5d064135 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,27 +30,74 @@ 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";
+ std::terminate();
+ }
+ 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(FifoFile &&other)
+ : m_path(other.m_path), m_file(other.m_file) {
+ other.m_file = nullptr;
+}
FifoFile::~FifoFile() {
+ if (m_file)
+ fclose(m_file);
#if !defined(_WIN32)
+ // Unreferenced named pipes are deleted automatically on Win32
unlink(m_path.c_str());
#endif
}
-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
+ ResultPath.pop_back();
+ return std::error_code();
}
-FifoFileIO::FifoFileIO(StringRef fifo_file, StringRef other_endpoint_name)
- : m_fifo_file(fifo_file), m_other_endpoint_name(other_endpoint_name) {}
+FifoFileIO::FifoFileIO(FifoFile &&fifo_file, StringRef other_endpoint_name)
+ : m_fifo_file(std::move(fifo_file)),
+ m_other_endpoint_name(other_endpoint_name) {}
Expected<json::Value> FifoFileIO::ReadJSON(std::chrono::milliseconds timeout) {
// We use a pointer for this future, because otherwise its normal destructor
@@ -52,13 +105,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 +129,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 +143,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 +167,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..92882aa0927e81 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,22 @@ 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(FifoFile &&other);
+
+ FifoFile(const FifoFile &) = delete;
+ FifoFile &operator=(const FifoFile &) = delete;
~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 +48,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.
///
@@ -75,8 +78,10 @@ class FifoFileIO {
const llvm::json::Value &json,
std::chrono::milliseconds timeout = std::chrono::milliseconds(20000));
+ llvm::Error WaitForPeer();
+
private:
- std::string m_fifo_file;
+ 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..cc10c64e807af6 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) {
@@ -111,8 +111,10 @@ Error
RunInTerminalLauncherCommChannel::WaitUntilDebugAdaptorAttaches(
return message.takeError();
}
-Error RunInTerminalLauncherCommChannel::NotifyPid() {
- return m_io.SendJSON(RunInTerminalMessagePid(getpid()).ToJSON());
+Error RunInTerminalLauncherCommChannel::NotifyPid(lldb::pid_t pid) {
+ if (pid == 0)
+ pid = getpid();
+ return m_io.SendJSON(RunInTerminalMessagePid(pid).ToJSON());
}
void RunInTerminalLauncherCommChannel::NotifyError(StringRef error) {
@@ -122,8 +124,12 @@ void
RunInTerminalLauncherCommChannel::NotifyError(StringRef error) {
}
RunInTerminalDebugAdapterCommChannel::RunInTerminalDebugAdapterCommChannel(
- StringRef comm_file)
- : m_io(comm_file, "runInTerminal launcher") {}
+ FifoFile &comm_file)
+ : m_io(std::move(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>
@@ -158,13 +164,25 @@ 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 for "
+ "runInTerminal comm-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..235108dbb08d89 100644
--- a/lldb/tools/lldb-dap/RunInTerminal.h
+++ b/lldb/tools/lldb-dap/RunInTerminal.h
@@ -87,7 +87,7 @@ class RunInTerminalLauncherCommChannel {
/// \return
/// An \a llvm::Error object in case of errors or if this operation times
/// out.
- llvm::Error NotifyPid();
+ llvm::Error NotifyPid(lldb::pid_t pid = 0);
/// Notify the debug adaptor that there's been an error.
void NotifyError(llvm::StringRef error);
@@ -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 6b12569d90a831..28c3edebef6986 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -2008,7 +2008,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)
@@ -2026,6 +2026,11 @@ llvm::Error request_runInTerminal(DAP &dap,
}
});
+ llvm::errs() << "WaitForLauncher\n";
+ auto err = comm_channel.WaitForLauncher();
+ llvm::errs() << "WaitForLauncher returned\n";
+ if (err)
+ return err;
if (llvm::Expected<lldb::pid_t> pid = comm_channel.GetLauncherPid())
attach_info.SetProcessID(*pid);
else
@@ -4861,11 +4866,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.
@@ -4874,8 +4874,36 @@ static void LaunchRunInTerminalTarget(llvm::opt::Arg
&target_arg,
(void)prctl(PR_SET_PTRACER, debugger_pid, 0, 0, 0);
#endif
+ const char *target = target_arg.getValue();
+
+#ifdef _WIN32
+ /* Win32 provides no way to replace the process image. exec* are misnomers.
+ Neither is the adapter notified of new processes due to DebugActiveProcess
+ semantics. Hence, we create the new process in a suspended state and
resume
+ it after attach.
+ */
+ std::string cmdline;
+ for (char **arg = argv; *arg != nullptr; ++arg) {
+ cmdline += *arg;
+ cmdline += ' ';
+ }
+ STARTUPINFOA si = {};
+ si.cb = sizeof(si);
+ PROCESS_INFORMATION pi = {};
+ bool res = CreateProcessA(target, cmdline.data(), NULL, NULL, FALSE,
+ CREATE_SUSPENDED, NULL, NULL, &si, &pi);
+ if (!res) {
+ llvm::errs() << "Failed to create process: " << GetLastError() << "\n";
+ exit(EXIT_FAILURE);
+ }
+#endif
+
RunInTerminalLauncherCommChannel comm_channel(comm_file);
- if (llvm::Error err = comm_channel.NotifyPid()) {
+ if (llvm::Error err = comm_channel.NotifyPid(
+#ifdef _WIN32
+ pi.dwProcessId
+#endif
+ )) {
llvm::errs() << llvm::toString(std::move(err)) << "\n";
exit(EXIT_FAILURE);
}
@@ -4892,15 +4920,17 @@ static void LaunchRunInTerminalTarget(llvm::opt::Arg
&target_arg,
llvm::errs() << llvm::toString(std::move(err)) << "\n";
exit(EXIT_FAILURE);
}
-
- const char *target = target_arg.getValue();
+#ifdef _WIN32
+ assert(ResumeThread(pi.hThread) != -1);
+ exit(EXIT_SUCCESS);
+#else
execvp(target, argv);
+#endif
std::string error = std::strerror(errno);
comm_channel.NotifyError(error);
llvm::errs() << error << "\n";
exit(EXIT_FAILURE);
-#endif
}
/// used only by TestVSCode_redirection_to_console.py
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
