[lldb-dap] Add runInTerminal support for Windows

Author: DrSergei

lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py

@@ -43,7 +43,6 @@ def isTestSupported(self):
         except:
             return False
 
-    @skipIfWindows
     @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
     def test_runInTerminal(self):
         if not self.isTestSupported():
@@ -113,7 +112,6 @@ def test_runInTerminalWithObjectEnv(self):
         self.assertIn("FOO", request_envs)
         self.assertEqual("BAR", request_envs["FOO"])
 
-    @skipIfWindows
     @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
     def test_runInTerminalInvalidTarget(self):
         if not self.isTestSupported():
@@ -132,7 +130,6 @@ def test_runInTerminalInvalidTarget(self):
             response["message"],
         )
 
-    @skipIfWindows
     @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
     def test_missingArgInRunInTerminalLauncher(self):
         if not self.isTestSupported():

lldb/tools/lldb-dap/FifoFiles.cpp

@@ -24,26 +24,53 @@ using namespace llvm;
 
 namespace lldb_dap {
 
-FifoFile::FifoFile(StringRef path) : m_path(path) {}
+#if defined(_WIN32)
+FifoFile::FifoFile(StringRef path, HANDLE handle, bool is_server)
+    : m_path(path), m_is_server(is_server), m_pipe_fd(handle) {}
+#else
+FifoFile::FifoFile(StringRef path, bool is_server)
+    : m_path(path), m_is_server(is_server) {}
+#endif
 
 FifoFile::~FifoFile() {
-#if !defined(_WIN32)
-  unlink(m_path.c_str());
+#if defined(_WIN32)
+  if (m_pipe_fd == INVALID_HANDLE_VALUE)
+    return;
+  if (m_is_server)
+    DisconnectNamedPipe(m_pipe_fd);
+  CloseHandle(m_pipe_fd);
+#else
+  if (m_is_server)
+    unlink(m_path.c_str());
 #endif
 }
 
-Expected<std::shared_ptr<FifoFile>> CreateFifoFile(StringRef path) {
+Expected<std::shared_ptr<FifoFile>> CreateFifoFile(StringRef path,
+                                                   bool is_server) {
 #if defined(_WIN32)
-  return createStringError(inconvertibleErrorCode(), "Unimplemented");
+  if (!is_server)
+    return std::make_shared<FifoFile>(path, INVALID_HANDLE_VALUE, is_server);
+  HANDLE handle =
+      CreateNamedPipeA(path.data(), PIPE_ACCESS_DUPLEX,
+                       PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, 1,
+                       1024 * 16, 1024 * 16, 0, NULL);
+  if (handle == INVALID_HANDLE_VALUE)
+    return createStringError(
+        std::error_code(GetLastError(), std::generic_category()),
+        "Couldn't create fifo file: %s", path.data());
+  return std::make_shared<FifoFile>(path, handle, is_server);
 #else
+  if (!is_server)
+    return std::make_shared<FifoFile>(path, is_server);
   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);
+  return std::make_shared<FifoFile>(path, is_server);
 #endif
 }
 
-FifoFileIO::FifoFileIO(StringRef fifo_file, StringRef other_endpoint_name)
+FifoFileIO::FifoFileIO(std::shared_ptr<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,11 +79,27 @@ 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);
+#if defined(_WIN32)
+        std::string buffer;
+        buffer.reserve(4096);
+        char ch;
+        DWORD bytes_read = 0;
+        while (ReadFile(m_fifo_file->m_pipe_fd, &ch, 1, &bytes_read, NULL) &&
+               (bytes_read == 1)) {
+          buffer.push_back(ch);
+          if (ch == '\n') {
+            break;
+          }
+        }
+        if (!buffer.empty())
+          line = std::move(buffer);
+#else
+        std::ifstream reader(m_fifo_file->m_path, std::ifstream::in);
         std::string buffer;
         std::getline(reader, buffer);
         if (!buffer.empty())
           line = buffer;
+#endif
       }));
   if (future->wait_for(timeout) == std::future_status::timeout || !line)
     // Indeed this is a leak, but it's intentional. "future" obj destructor
@@ -78,9 +121,18 @@ 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);
+#if defined(_WIN32)
+        std::string buffer = JSONToString(json);
+        buffer.append("\n");
+        DWORD bytes_write = 0;
+        WriteFile(m_fifo_file->m_pipe_fd, buffer.c_str(), buffer.size(),
+                  &bytes_write, NULL);
+        done = bytes_write == buffer.size();
+#else
+        std::ofstream writer(m_fifo_file->m_path, std::ofstream::out);
         writer << JSONToString(json) << std::endl;
         done = true;
+#endif
       }));
   if (future->wait_for(timeout) == std::future_status::timeout || !done) {
     // Indeed this is a leak, but it's intentional. "future" obj destructor will
@@ -98,4 +150,20 @@ Error FifoFileIO::SendJSON(const json::Value &json,
   return Error::success();
 }
 
+#if defined(_WIN32)
+bool FifoFileIO::Connect() {
+  if (m_fifo_file->m_is_server) {
+    return ConnectNamedPipe(m_fifo_file->m_pipe_fd, NULL);
+  }
+  if (!WaitNamedPipeA(m_fifo_file->m_path.c_str(), NMPWAIT_WAIT_FOREVER))
+    return false;
+  m_fifo_file->m_pipe_fd =
+      CreateFileA(m_fifo_file->m_path.c_str(), GENERIC_READ | GENERIC_WRITE, 0,
+                  NULL, OPEN_EXISTING, 0, NULL);
+  if (m_fifo_file->m_pipe_fd == INVALID_HANDLE_VALUE)
+    return false;
+  return true;
+}
+#endif
+
 } // namespace lldb_dap

lldb/tools/lldb-dap/FifoFiles.h

@@ -12,40 +12,62 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/JSON.h"
 
+#if defined(_WIN32)
+#include "lldb/Host/windows/windows.h"
+#endif
+
 #include <chrono>
 
 namespace lldb_dap {
 
+class FifoFileIO;
+
 /// Struct that controls the life of a fifo file in the filesystem.
 ///
 /// The file is destroyed when the destructor is invoked.
 struct FifoFile {
-  FifoFile(llvm::StringRef path);
+#if defined(_WIN32)
+  FifoFile(llvm::StringRef path, HANDLE handle, bool is_server);
+#else
+  FifoFile(llvm::StringRef path, bool is_server);
+#endif
 
   ~FifoFile();
 
   std::string m_path;
+  bool m_is_server;
+#if defined(_WIN32)
+  HANDLE m_pipe_fd = INVALID_HANDLE_VALUE;
+#endif
+
+  friend FifoFileIO;
 };
 
 /// Create a fifo file in the filesystem.
 ///
 /// \param[in] path
 ///     The path for the fifo file.
 ///
+/// \param[in] is_server
+///     If \a is_server is true, then created instance of FifoFile will own
+///     created 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);
+llvm::Expected<std::shared_ptr<FifoFile>> CreateFifoFile(llvm::StringRef path,
+                                                         bool is_server);
 
 class FifoFileIO {
 public:
   /// \param[in] fifo_file
-  ///     The path to an input fifo file that exists in the file system.
+  ///     The std::shared_ptr<FifoFile> to an existing fifo file.
   ///
   /// \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(std::shared_ptr<FifoFile> fifo_file,
+             llvm::StringRef other_endpoint_name);
 
   /// Read the next JSON object from the underlying input fifo file.
   ///
@@ -75,8 +97,12 @@ class FifoFileIO {
       const llvm::json::Value &json,
       std::chrono::milliseconds timeout = std::chrono::milliseconds(20000));
 
+#if defined(_WIN32)
+  bool Connect();
+#endif
+
 private:
-  std::string m_fifo_file;
+  std::shared_ptr<FifoFile> m_fifo_file;
   std::string m_other_endpoint_name;
 };
 

lldb/tools/lldb-dap/Handler/RequestHandler.cpp

@@ -119,9 +119,9 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
       CreateRunInTerminalCommFile();
   if (!comm_file_or_err)
     return comm_file_or_err.takeError();
-  FifoFile &comm_file = *comm_file_or_err.get();
+  std::shared_ptr<FifoFile> comm_file = *comm_file_or_err;
 
-  RunInTerminalDebugAdapterCommChannel comm_channel(comm_file.m_path);
+  RunInTerminalDebugAdapterCommChannel comm_channel(comm_file);
 
   lldb::pid_t debugger_pid = LLDB_INVALID_PROCESS_ID;
 #if !defined(_WIN32)
@@ -130,10 +130,16 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
 
   llvm::json::Object reverse_request = CreateRunInTerminalReverseRequest(
       *arguments.configuration.program, arguments.args, arguments.env,
-      arguments.cwd.value_or(""), comm_file.m_path, debugger_pid);
+      arguments.cwd.value_or(""), comm_file->m_path, debugger_pid);
   dap.SendReverseRequest<LogFailureResponseHandler>("runInTerminal",
                                                     std::move(reverse_request));
 
+#if defined(_WIN32)
+  if (!comm_channel.Connect())
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "Failed to connect to the named pipe.");
+#endif
+
   if (llvm::Expected<lldb::pid_t> pid = comm_channel.GetLauncherPid())
     attach_info.SetProcessID(*pid);
   else

lldb/tools/lldb-dap/JSONUtils.cpp

@@ -1463,7 +1463,7 @@ llvm::json::Object CreateRunInTerminalReverseRequest(
   req_args.push_back("--launch-target");
   req_args.push_back(program.str());
   req_args.insert(req_args.end(), args.begin(), args.end());
-  run_in_terminal_args.try_emplace("args", args);
+  run_in_terminal_args.try_emplace("args", req_args);
 
   if (!cwd.empty())
     run_in_terminal_args.try_emplace("cwd", cwd);

lldb/tools/lldb-dap/RunInTerminal.cpp

@@ -9,7 +9,9 @@
 #include "RunInTerminal.h"
 #include "JSONUtils.h"
 
-#if !defined(_WIN32)
+#if defined(_WIN32)
+#include "lldb/Host/windows/windows.h"
+#else
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
@@ -96,7 +98,7 @@ static Error ToError(const RunInTerminalMessage &message) {
 }
 
 RunInTerminalLauncherCommChannel::RunInTerminalLauncherCommChannel(
-    StringRef comm_file)
+    std::shared_ptr<FifoFile> comm_file)
     : m_io(comm_file, "debug adapter") {}
 
 Error RunInTerminalLauncherCommChannel::WaitUntilDebugAdapterAttaches(
@@ -111,8 +113,8 @@ Error RunInTerminalLauncherCommChannel::WaitUntilDebugAdapterAttaches(
     return message.takeError();
 }
 
-Error RunInTerminalLauncherCommChannel::NotifyPid() {
-  return m_io.SendJSON(RunInTerminalMessagePid(getpid()).ToJSON());
+Error RunInTerminalLauncherCommChannel::NotifyPid(lldb::pid_t pid) {
+  return m_io.SendJSON(RunInTerminalMessagePid(pid).ToJSON());
 }
 
 void RunInTerminalLauncherCommChannel::NotifyError(StringRef error) {
@@ -121,8 +123,12 @@ void RunInTerminalLauncherCommChannel::NotifyError(StringRef error) {
     llvm::errs() << llvm::toString(std::move(err)) << "\n";
 }
 
+#if defined(_WIN32)
+bool RunInTerminalLauncherCommChannel::Connect() { return m_io.Connect(); }
+#endif
+
 RunInTerminalDebugAdapterCommChannel::RunInTerminalDebugAdapterCommChannel(
-    StringRef comm_file)
+    std::shared_ptr<FifoFile> comm_file)
     : m_io(comm_file, "runInTerminal launcher") {}
 
 // Can't use \a std::future<llvm::Error> because it doesn't compile on Windows
@@ -148,6 +154,10 @@ Expected<lldb::pid_t> RunInTerminalDebugAdapterCommChannel::GetLauncherPid() {
   }
 }
 
+#if defined(_WIN32)
+bool RunInTerminalDebugAdapterCommChannel::Connect() { return m_io.Connect(); }
+#endif
+
 std::string RunInTerminalDebugAdapterCommChannel::GetLauncherError() {
   // We know there's been an error, so a small timeout is enough.
   if (Expected<RunInTerminalMessageUP> message =
@@ -158,13 +168,25 @@ std::string RunInTerminalDebugAdapterCommChannel::GetLauncherError() {
 }
 
 Expected<std::shared_ptr<FifoFile>> CreateRunInTerminalCommFile() {
+#if defined(_WIN32)
+  static constexpr llvm::StringLiteral g_pipe_name_prefix = "\\\\.\\Pipe\\";
+  SmallString<256> comm_file;
+  sys::fs::createUniquePath("lldb-dap-run-in-terminal-comm-%%%%%%", comm_file,
+                            +false);
+  return CreateFifoFile((g_pipe_name_prefix + comm_file.str()).str(), true);
+#else
   SmallString<256> comm_file;
   if (std::error_code EC = sys::fs::getPotentiallyUniqueTempFileName(
           "lldb-dap-run-in-terminal-comm", "", comm_file))
     return createStringError(EC, "Error making unique file name for "
                                  "runInTerminal communication files");
+  return CreateFifoFile(comm_file.str(), true);
+#endif
+}
 
-  return CreateFifoFile(comm_file.str());
+Expected<std::shared_ptr<FifoFile>>
+OpenRunInTerminalCommFile(llvm::StringRef fifo_file) {
+  return CreateFifoFile(fifo_file, false);
 }
 
 } // namespace lldb_dap