https://github.com/charles-zablit updated https://github.com/llvm/llvm-project/pull/174635
>From b5a61ccc5bb2c8d5fb05f56a39bf51e2141f080a Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Wed, 7 Jan 2026 19:39:39 +0000 Subject: [PATCH 1/2] [lldb-dap][windows] add support for the integratedTerminal flag --- .../Host/windows/ProcessLauncherWindows.h | 81 ++++++++- lldb/packages/Python/lldbsuite/test/dotest.py | 5 +- .../Host/windows/ProcessLauncherWindows.cpp | 103 ++++++++---- .../runInTerminal/TestDAP_runInTerminal.py | 154 +++++++++++++----- lldb/tools/lldb-dap/FifoFiles.cpp | 80 ++++++++- lldb/tools/lldb-dap/FifoFiles.h | 14 +- .../tools/lldb-dap/Handler/RequestHandler.cpp | 4 +- lldb/tools/lldb-dap/RunInTerminal.cpp | 23 ++- lldb/tools/lldb-dap/RunInTerminal.h | 3 + lldb/tools/lldb-dap/tool/lldb-dap.cpp | 124 ++++++++++++-- lldb/unittests/DAP/FifoFilesTest.cpp | 8 +- 11 files changed, 475 insertions(+), 124 deletions(-) diff --git a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h index 553263e2f5a72..6ebf764249a4b 100644 --- a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h +++ b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h @@ -9,10 +9,15 @@ #ifndef lldb_Host_windows_ProcessLauncherWindows_h_ #define lldb_Host_windows_ProcessLauncherWindows_h_ +#include "lldb/Host/ProcessLaunchInfo.h" #include "lldb/Host/ProcessLauncher.h" #include "lldb/Host/windows/windows.h" +#include "llvm/ADT/ScopeExit.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/ErrorOr.h" +#include <optional> + namespace lldb_private { class ProcessLaunchInfo; @@ -22,9 +27,6 @@ class ProcessLauncherWindows : public ProcessLauncher { HostProcess LaunchProcess(const ProcessLaunchInfo &launch_info, Status &error) override; -protected: - HANDLE GetStdioHandle(const ProcessLaunchInfo &launch_info, int fd); - /// Get the list of Windows handles that should be inherited by the child /// process and update `STARTUPINFOEXW` with the handle list. /// @@ -35,12 +37,12 @@ class ProcessLauncherWindows : public ProcessLauncher { /// collected handles using `UpdateProcThreadAttribute`. On success, the /// vector of inherited handles is returned. /// - /// \param launch_info - /// The process launch configuration. - /// /// \param startupinfoex /// The extended STARTUPINFO structure for the process being created. /// + /// \param launch_info + /// The process launch configuration. + /// /// \param stdout_handle /// \param stderr_handle /// \param stdin_handle @@ -49,12 +51,73 @@ class ProcessLauncherWindows : public ProcessLauncher { /// \returns /// `std::vector<HANDLE>` containing all handles that the child must /// inherit. - llvm::ErrorOr<std::vector<HANDLE>> - GetInheritedHandles(const ProcessLaunchInfo &launch_info, - STARTUPINFOEXW &startupinfoex, + static llvm::ErrorOr<std::vector<HANDLE>> + GetInheritedHandles(STARTUPINFOEXW &startupinfoex, + const ProcessLaunchInfo *launch_info = nullptr, HANDLE stdout_handle = NULL, HANDLE stderr_handle = NULL, HANDLE stdin_handle = NULL); + + static HANDLE GetStdioHandle(const ProcessLaunchInfo &launch_info, int fd); + + /// Creates a file handle suitable for redirecting stdin, stdout, + /// or stderr of a child process. + /// + /// \param path The file path to open. If empty, returns NULL (no + /// redirection). + /// \param fd The file descriptor type: STDIN_FILENO, STDOUT_FILENO, or + /// STDERR_FILENO. + /// + /// \return A handle to the opened file, or NULL if the path is empty or the + /// file + /// cannot be opened (INVALID_HANDLE_VALUE is converted to NULL). + /// + /// Behavior by file descriptor: + /// - STDIN_FILENO: Opens existing file for reading (GENERIC_READ, + /// OPEN_EXISTING). + /// - STDOUT_FILENO: Creates/truncates file for writing (GENERIC_WRITE, + /// CREATE_ALWAYS). + /// - STDERR_FILENO: Creates/truncates file for writing with write-through + /// (FILE_FLAG_WRITE_THROUGH ensures immediate disk writes, + /// bypassing system cache for error messages). + /// + /// All handles are created with: + /// - Inheritance enabled (bInheritHandle = TRUE) so child processes can use + /// them. + /// - Shared read/write/delete access to allow other processes to access the + /// file. + static HANDLE GetStdioHandle(const llvm::StringRef path, int fd); }; + +/// Flattens an Args object into a Windows command-line wide string. +/// +/// Returns an empty string if args is empty. +/// +/// \param args The Args object to flatten. +/// \returns A wide string containing the flattened command line. +llvm::ErrorOr<std::wstring> GetFlattenedWindowsCommandStringW(Args args); + +/// Flattens an Args object into a Windows command-line wide string. +/// +/// Returns an empty string if args is empty. +/// +/// \param args The Args object to flatten. +/// \returns A wide string containing the flattened command line. +llvm::ErrorOr<std::wstring> GetFlattenedWindowsCommandStringW(char *args[]); + +/// Allocate and initialize a PROC_THREAD_ATTRIBUTE_LIST structure +/// that can be used with CreateProcess to specify extended process creation +/// attributes (such as inherited handles). +/// +/// \param[in] startupinfoex The STARTUPINFOEXW structure whose lpAttributeList +/// will +/// be initialized. +/// +/// \return On success, returns a scope_exit cleanup object that will +/// automatically +/// delete and free the attribute list when it goes out of scope. +/// On failure, returns the corresponding Windows error code. +llvm::ErrorOr<llvm::scope_exit<std::function<void()>>> +SetupProcThreadAttributeList(STARTUPINFOEXW &startupinfoex); } #endif diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py b/lldb/packages/Python/lldbsuite/test/dotest.py index fcd60a1bc0db8..5b41fc44b35b6 100644 --- a/lldb/packages/Python/lldbsuite/test/dotest.py +++ b/lldb/packages/Python/lldbsuite/test/dotest.py @@ -566,7 +566,10 @@ def setupSysPath(): lldbDir = os.path.dirname(lldbtest_config.lldbExec) - lldbDAPExec = os.path.join(lldbDir, "lldb-dap") + lldbDAPExecName = "lldb-dap" + if sys.platform == "win32": + lldbDAPExecName += ".exe" + lldbDAPExec = os.path.join(lldbDir, lldbDAPExecName) if is_exe(lldbDAPExec): os.environ["LLDBDAP_EXEC"] = lldbDAPExec diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp index 4b7cbec828dd8..3d986025756d0 100644 --- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp +++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp @@ -8,11 +8,9 @@ #include "lldb/Host/windows/ProcessLauncherWindows.h" #include "lldb/Host/HostProcess.h" -#include "lldb/Host/ProcessLaunchInfo.h" #include "lldb/Host/windows/PseudoConsole.h" #include "lldb/Host/windows/windows.h" -#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Support/ConvertUTF.h" #include "llvm/Support/Program.h" @@ -65,14 +63,9 @@ static std::vector<wchar_t> CreateEnvironmentBufferW(const Environment &env) { return buffer; } -/// Flattens an Args object into a Windows command-line wide string. -/// -/// Returns an empty string if args is empty. -/// -/// \param args The Args object to flatten. -/// \returns A wide string containing the flattened command line. -static llvm::ErrorOr<std::wstring> -GetFlattenedWindowsCommandStringW(Args args) { +namespace lldb_private { + +llvm::ErrorOr<std::wstring> GetFlattenedWindowsCommandStringW(Args args) { if (args.empty()) return L""; @@ -83,6 +76,45 @@ GetFlattenedWindowsCommandStringW(Args args) { return llvm::sys::flattenWindowsCommandLine(args_ref); } +llvm::ErrorOr<std::wstring> GetFlattenedWindowsCommandStringW(char *args[]) { + std::vector<llvm::StringRef> args_ref; + for (int i = 0; args[i] != nullptr; ++i) { + args_ref.push_back(args[i]); + } + + return llvm::sys::flattenWindowsCommandLine(args_ref); +} + +llvm::ErrorOr<llvm::scope_exit<std::function<void()>>> +SetupProcThreadAttributeList(STARTUPINFOEXW &startupinfoex) { + SIZE_T attributelist_size = 0; + InitializeProcThreadAttributeList(/*lpAttributeList=*/nullptr, + /*dwAttributeCount=*/1, /*dwFlags=*/0, + &attributelist_size); + + startupinfoex.lpAttributeList = + static_cast<LPPROC_THREAD_ATTRIBUTE_LIST>(malloc(attributelist_size)); + + if (!startupinfoex.lpAttributeList) + return llvm::mapWindowsError(ERROR_OUTOFMEMORY); + + auto cleanup = llvm::scope_exit<std::function<void()>>([&startupinfoex] { + if (startupinfoex.lpAttributeList) { + DeleteProcThreadAttributeList(startupinfoex.lpAttributeList); + free(startupinfoex.lpAttributeList); + startupinfoex.lpAttributeList = nullptr; + } + }); + + if (!InitializeProcThreadAttributeList(startupinfoex.lpAttributeList, + /*dwAttributeCount=*/1, /*dwFlags=*/0, + &attributelist_size)) + return llvm::mapWindowsError(GetLastError()); + + return std::move(cleanup); +} +} // namespace lldb_private + HostProcess ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, Status &error) { @@ -108,24 +140,13 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, if (stderr_handle) ::CloseHandle(stderr_handle); }); - - SIZE_T attributelist_size = 0; - InitializeProcThreadAttributeList(/*lpAttributeList=*/nullptr, - /*dwAttributeCount=*/1, /*dwFlags=*/0, - &attributelist_size); - - startupinfoex.lpAttributeList = - static_cast<LPPROC_THREAD_ATTRIBUTE_LIST>(malloc(attributelist_size)); - llvm::scope_exit free_attributelist( - [&] { free(startupinfoex.lpAttributeList); }); - if (!InitializeProcThreadAttributeList(startupinfoex.lpAttributeList, - /*dwAttributeCount=*/1, /*dwFlags=*/0, - &attributelist_size)) { - error = Status(::GetLastError(), eErrorTypeWin32); + auto attributelist_cleanup_or_err = + lldb_private::SetupProcThreadAttributeList(startupinfoex); + if (!attributelist_cleanup_or_err) { + error = attributelist_cleanup_or_err.getError(); return HostProcess(); } - llvm::scope_exit delete_attributelist( - [&] { DeleteProcThreadAttributeList(startupinfoex.lpAttributeList); }); + auto attributelist_cleanup = std::move(*attributelist_cleanup_or_err); std::vector<HANDLE> inherited_handles; if (use_pty) { @@ -136,8 +157,9 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, return HostProcess(); } } else { - auto inherited_handles_or_err = GetInheritedHandles( - launch_info, startupinfoex, stdout_handle, stderr_handle, stdin_handle); + auto inherited_handles_or_err = + GetInheritedHandles(startupinfoex, &launch_info, stdout_handle, + stderr_handle, stdin_handle); if (!inherited_handles_or_err) { error = Status(inherited_handles_or_err.getError()); return HostProcess(); @@ -210,7 +232,7 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, } llvm::ErrorOr<std::vector<HANDLE>> ProcessLauncherWindows::GetInheritedHandles( - const ProcessLaunchInfo &launch_info, STARTUPINFOEXW &startupinfoex, + STARTUPINFOEXW &startupinfoex, const ProcessLaunchInfo *launch_info, HANDLE stdout_handle, HANDLE stderr_handle, HANDLE stdin_handle) { std::vector<HANDLE> inherited_handles; @@ -228,12 +250,13 @@ llvm::ErrorOr<std::vector<HANDLE>> ProcessLauncherWindows::GetInheritedHandles( if (startupinfoex.StartupInfo.hStdOutput) inherited_handles.push_back(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())); - } + if (launch_info) + 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 (inherited_handles.empty()) return inherited_handles; @@ -254,6 +277,15 @@ ProcessLauncherWindows::GetStdioHandle(const ProcessLaunchInfo &launch_info, const FileAction *action = launch_info.GetFileActionForFD(fd); if (action == nullptr) return NULL; + const std::string path = action->GetFileSpec().GetPath(); + + return GetStdioHandle(path, fd); +} + +HANDLE ProcessLauncherWindows::GetStdioHandle(const llvm::StringRef path, + int fd) { + if (path.empty()) + return NULL; SECURITY_ATTRIBUTES secattr = {}; secattr.nLength = sizeof(SECURITY_ATTRIBUTES); secattr.bInheritHandle = TRUE; @@ -274,7 +306,6 @@ ProcessLauncherWindows::GetStdioHandle(const ProcessLaunchInfo &launch_info, flags = FILE_FLAG_WRITE_THROUGH; } - const std::string path = action->GetFileSpec().GetPath(); std::wstring wpath; llvm::ConvertUTF8toWide(path, wpath); HANDLE result = ::CreateFileW(wpath.c_str(), access, share, &secattr, create, 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 d7e0168239f34..c92cae16b9ea1 100644 --- a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py +++ b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py @@ -2,6 +2,7 @@ Test lldb-dap runInTerminal reverse request """ +from contextlib import contextmanager from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import line_number import lldbdap_testcase @@ -9,25 +10,80 @@ import subprocess import json +@contextmanager +def fifo(*args, **kwargs): + if sys.platform == "win32": + comm_file = r"\\.\pipe\lldb-dap-run-in-terminal-comm" + + import ctypes + PIPE_ACCESS_DUPLEX = 0x00000003 + PIPE_TYPE_BYTE = 0x00000000 + PIPE_READMODE_BYTE = 0x00000000 + PIPE_WAIT = 0x00000000 + PIPE_UNLIMITED_INSTANCES = 255 + kernel32 = ctypes.windll.kernel32 + + pipe = kernel32.CreateNamedPipeW( + comm_file, + PIPE_ACCESS_DUPLEX, + PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, + PIPE_UNLIMITED_INSTANCES, 4096, 4096, 0, None + ) + else: + comm_file = os.path.join(kwargs["directory"], "comm-file") + pipe = None + os.mkfifo(comm_file) + + try: + yield comm_file, pipe + finally: + if pipe is not None: + kernel32.DisconnectNamedPipe(pipe) + kernel32.CloseHandle(pipe) @skipIfBuildType(["debug"]) class TestDAP_runInTerminal(lldbdap_testcase.DAPTestCaseBase): - def read_pid_message(self, fifo_file): - with open(fifo_file, "r") as file: - self.assertIn("pid", file.readline()) + def read_pid_message(self, fifo_file, pipe): + if sys.platform == "win32": + import ctypes + buffer = ctypes.create_string_buffer(4096) + bytes_read = ctypes.wintypes.DWORD() + kernel32 = ctypes.windll.kernel32 + kernel32.ConnectNamedPipe(pipe, None) + kernel32.ReadFile(pipe, buffer, 4096, ctypes.byref(bytes_read), None) + self.assertIn("pid", buffer.value.decode()) + else: + with open(fifo_file, "r") as file: + self.assertIn("pid", file.readline()) @staticmethod - def send_did_attach_message(fifo_file): - with open(fifo_file, "w") as file: - file.write(json.dumps({"kind": "didAttach"}) + "\n") + def send_did_attach_message(fifo_file, pipe=None): + message = json.dumps({"kind": "didAttach"}) + "\n" + if sys.platform == "win32": + import ctypes + kernel32 = ctypes.windll.kernel32 + bytes_written = ctypes.wintypes.DWORD() + kernel32.ConnectNamedPipe(pipe, None) + kernel32.WriteFile(pipe, message.encode(), len(message), ctypes.byref(bytes_written), None) + else: + with open(fifo_file, "w") as file: + file.write(message) @staticmethod - def read_error_message(fifo_file): - with open(fifo_file, "r") as file: - return file.readline() + def read_error_message(fifo_file, pipe=None): + if sys.platform == "win32": + import ctypes + buffer = ctypes.create_string_buffer(4096) + bytes_read = ctypes.wintypes.DWORD() + kernel32 = ctypes.windll.kernel32 + kernel32.ConnectNamedPipe(pipe, None) + kernel32.ReadFile(pipe, buffer, 4096, ctypes.byref(bytes_read), None) + return buffer.value.decode() + else: + with open(fifo_file, "r") as file: + return file.readline() @skipIfAsan - @skipIfWindows def test_runInTerminal(self): """ Tests the "runInTerminal" reverse request. It makes sure that the IDE can @@ -74,7 +130,6 @@ def test_runInTerminal(self): self.continue_to_exit() @skipIfAsan - @skipIfWindows def test_runInTerminalWithObjectEnv(self): """ Tests the "runInTerminal" reverse request. It makes sure that the IDE can @@ -97,7 +152,6 @@ def test_runInTerminalWithObjectEnv(self): self.continue_to_exit() - @skipIfWindows def test_runInTerminalInvalidTarget(self): self.build_and_create_debug_adapter() response = self.launch( @@ -113,7 +167,6 @@ def test_runInTerminalInvalidTarget(self): response["body"]["error"]["format"], ) - @skipIfWindows def test_missingArgInRunInTerminalLauncher(self): proc = subprocess.run( [self.lldbDAPExec, "--launch-target", "INVALIDPROGRAM"], @@ -149,44 +202,55 @@ def test_FakeAttachedRunInTerminalLauncherWithInvalidProgram(self): _, stderr = proc.communicate() self.assertIn("No such file or directory", stderr) - @skipIfWindows - def test_FakeAttachedRunInTerminalLauncherWithValidProgram(self): - comm_file = os.path.join(self.getBuildDir(), "comm-file") - os.mkfifo(comm_file) - - proc = subprocess.Popen( - [ - self.lldbDAPExec, - "--comm-file", - comm_file, - "--launch-target", - "echo", - "foo", - ], - universal_newlines=True, - stdout=subprocess.PIPE, - ) + @skipUnlessWindows + def test_FakeAttachedRunInTerminalLauncherWithInvalidProgramWindows(self): + with fifo(directory=self.getBuildDir()) as (comm_file, pipe): + subprocess.Popen( + [ + self.lldbDAPExec, + "--comm-file", + comm_file, + "--launch-target", + "INVALIDPROGRAM", + ], + universal_newlines=True, + stderr=subprocess.PIPE, + ) + + self.assertIn("Failed to launch target process", self.read_error_message(comm_file, pipe)) - self.read_pid_message(comm_file) - self.send_did_attach_message(comm_file) + def test_FakeAttachedRunInTerminalLauncherWithValidProgram(self): + with fifo(directory=self.getBuildDir()) as (comm_file, pipe): + proc = subprocess.Popen( + [ + self.lldbDAPExec, + "--comm-file", + comm_file, + "--launch-target", + "echo", + "foo", + ], + universal_newlines=True, + stdout=subprocess.PIPE, + ) + + self.read_pid_message(comm_file, pipe) + self.send_did_attach_message(comm_file, pipe) stdout, _ = proc.communicate() self.assertIn("foo", stdout) - @skipIfWindows def test_FakeAttachedRunInTerminalLauncherAndCheckEnvironment(self): - comm_file = os.path.join(self.getBuildDir(), "comm-file") - os.mkfifo(comm_file) - - proc = subprocess.Popen( - [self.lldbDAPExec, "--comm-file", comm_file, "--launch-target", "env"], - universal_newlines=True, - stdout=subprocess.PIPE, - env={**os.environ, "FOO": "BAR"}, - ) - - self.read_pid_message(comm_file) - self.send_did_attach_message(comm_file) + with fifo(directory=self.getBuildDir()) as (comm_file, pipe): + proc = subprocess.Popen( + [self.lldbDAPExec, "--comm-file", comm_file, "--launch-target", "env"], + universal_newlines=True, + stdout=subprocess.PIPE, + env={**os.environ, "FOO": "BAR"}, + ) + + self.read_pid_message(comm_file, pipe) + self.send_did_attach_message(comm_file, pipe) stdout, _ = proc.communicate() self.assertIn("FOO=BAR", stdout) diff --git a/lldb/tools/lldb-dap/FifoFiles.cpp b/lldb/tools/lldb-dap/FifoFiles.cpp index 1f1bba80bd3b1..3775e2f6b0143 100644 --- a/lldb/tools/lldb-dap/FifoFiles.cpp +++ b/lldb/tools/lldb-dap/FifoFiles.cpp @@ -9,7 +9,11 @@ #include "FifoFiles.h" #include "JSONUtils.h" -#if !defined(_WIN32) +#ifdef _WIN32 +#include "lldb/Host/windows/PipeWindows.h" +#include "lldb/Host/windows/windows.h" +#include "llvm/Support/Path.h" +#else #include <sys/stat.h> #include <sys/types.h> #include <unistd.h> @@ -24,7 +28,14 @@ using namespace llvm; namespace lldb_dap { -FifoFile::FifoFile(StringRef path) : m_path(path) {} +FifoFile::FifoFile(StringRef path, lldb::pipe_t pipe) : m_path(path) { +#ifdef _WIN32 + if (pipe == INVALID_HANDLE_VALUE) + pipe = CreateFileA(m_path.c_str(), GENERIC_READ | GENERIC_WRITE, 0, NULL, + OPEN_EXISTING, FILE_FLAG_OVERLAPPED, NULL); +#endif + m_pipe = pipe; +} FifoFile::~FifoFile() { #if !defined(_WIN32) @@ -32,9 +43,63 @@ FifoFile::~FifoFile() { #endif } +void FifoFile::WriteLine(std::string line) { +#ifdef _WIN32 + DWORD written; + line += "\n"; + WriteFile(m_pipe, line.c_str(), static_cast<DWORD>(line.size()), &written, + NULL); + FlushFileBuffers(m_pipe); +#else + std::ofstream writer(m_path, std::ofstream::out); + writer << line << std::endl; +#endif +} + +void FifoFile::Connect() { +#ifdef _WIN32 + ConnectNamedPipe(m_pipe, NULL); +#endif +} + +std::string FifoFile::ReadLine() { +#ifdef _WIN32 + std::string buffer; + char read_buffer[4096]; + DWORD bytes_read; + + if (ReadFile(m_pipe, read_buffer, sizeof(read_buffer) - 1, &bytes_read, + NULL) && + bytes_read > 0) { + read_buffer[bytes_read] = '\0'; + buffer = read_buffer; + } + + return buffer; +#else + std::ifstream reader(m_path, std::ifstream::in); + std::string buffer; + std::getline(reader, buffer); + return buffer; +#endif +} + Expected<std::shared_ptr<FifoFile>> CreateFifoFile(StringRef path) { #if defined(_WIN32) - return createStringError(inconvertibleErrorCode(), "Unimplemented"); + assert(path.starts_with("\\\\.\\pipe\\") && + "FifoFile path should start with '\\\\.\\pipe\\'"); + HANDLE pipe_handle = + CreateNamedPipeA(path.data(), PIPE_ACCESS_DUPLEX, + PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, + PIPE_UNLIMITED_INSTANCES, 4096, 4096, 0, NULL); + + if (pipe_handle == INVALID_HANDLE_VALUE) { + DWORD error = GetLastError(); + return createStringError(std::error_code(error, std::system_category()), + "Couldn't create named pipe: %s", path.data()); + } + + return std::make_shared<FifoFile>(path, pipe_handle); #else if (int err = mkfifo(path.data(), 0600)) return createStringError(std::error_code(err, std::generic_category()), @@ -43,7 +108,7 @@ Expected<std::shared_ptr<FifoFile>> CreateFifoFile(StringRef path) { #endif } -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,9 +117,7 @@ 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); + std::string buffer = m_fifo_file.ReadLine(); if (!buffer.empty()) line = buffer; })); @@ -78,8 +141,7 @@ 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; + m_fifo_file.WriteLine(JSONToString(json)); done = true; })); if (future->wait_for(timeout) == std::future_status::timeout || !done) { diff --git a/lldb/tools/lldb-dap/FifoFiles.h b/lldb/tools/lldb-dap/FifoFiles.h index 633ebeb2aedd4..e21471fcc78c0 100644 --- a/lldb/tools/lldb-dap/FifoFiles.h +++ b/lldb/tools/lldb-dap/FifoFiles.h @@ -9,6 +9,7 @@ #ifndef LLDB_TOOLS_LLDB_DAP_FIFOFILES_H #define LLDB_TOOLS_LLDB_DAP_FIFOFILES_H +#include "lldb/Host/windows/PipeWindows.h" #include "llvm/Support/Error.h" #include "llvm/Support/JSON.h" @@ -20,11 +21,18 @@ namespace lldb_dap { /// /// The file is destroyed when the destructor is invoked. struct FifoFile { - FifoFile(llvm::StringRef path); + FifoFile(llvm::StringRef path, lldb::pipe_t pipe = LLDB_INVALID_PIPE); ~FifoFile(); + void Connect(); + + void WriteLine(std::string line); + + std::string ReadLine(); + std::string m_path; + lldb::pipe_t m_pipe; }; /// Create a fifo file in the filesystem. @@ -45,7 +53,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. /// @@ -76,7 +84,7 @@ class FifoFileIO { std::chrono::milliseconds timeout = std::chrono::milliseconds(20000)); private: - std::string m_fifo_file; + FifoFile m_fifo_file; std::string m_other_endpoint_name; }; diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp index c60c86219f67d..b0a4e52cbeedb 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp @@ -99,7 +99,7 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) { 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) @@ -112,7 +112,7 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) { arguments.console == protocol::eConsoleExternalTerminal); dap.SendReverseRequest<LogFailureResponseHandler>("runInTerminal", std::move(reverse_request)); - + comm_file.Connect(); // we need to wait for the client to connect to the pipe. if (llvm::Expected<lldb::pid_t> pid = comm_channel.GetLauncherPid()) attach_info.SetProcessID(*pid); else diff --git a/lldb/tools/lldb-dap/RunInTerminal.cpp b/lldb/tools/lldb-dap/RunInTerminal.cpp index 9f309dd78221a..7d20ca1c13987 100644 --- a/lldb/tools/lldb-dap/RunInTerminal.cpp +++ b/lldb/tools/lldb-dap/RunInTerminal.cpp @@ -9,7 +9,9 @@ #include "RunInTerminal.h" #include "JSONUtils.h" -#if !defined(_WIN32) +#ifdef _WIN32 +#include "lldb/Host/windows/windows.h" +#else #include <sys/stat.h> #include <sys/types.h> #include <unistd.h> @@ -97,7 +99,7 @@ static Error ToError(const RunInTerminalMessage &message) { RunInTerminalLauncherCommChannel::RunInTerminalLauncherCommChannel( StringRef comm_file) - : m_io(comm_file, "debug adapter") {} + : m_io(FifoFileIO(FifoFile(comm_file), "debug adapter")) {} Error RunInTerminalLauncherCommChannel::WaitUntilDebugAdapterAttaches( std::chrono::milliseconds timeout) { @@ -112,7 +114,11 @@ Error RunInTerminalLauncherCommChannel::WaitUntilDebugAdapterAttaches( } Error RunInTerminalLauncherCommChannel::NotifyPid() { - return m_io.SendJSON(RunInTerminalMessagePid(getpid()).ToJSON()); + return NotifyPid(getpid()); +} + +Error RunInTerminalLauncherCommChannel::NotifyPid(int pid) { + return m_io.SendJSON(RunInTerminalMessagePid(pid).ToJSON()); } void RunInTerminalLauncherCommChannel::NotifyError(StringRef error) { @@ -125,6 +131,10 @@ RunInTerminalDebugAdapterCommChannel::RunInTerminalDebugAdapterCommChannel( StringRef comm_file) : m_io(comm_file, "runInTerminal launcher") {} +RunInTerminalDebugAdapterCommChannel::RunInTerminalDebugAdapterCommChannel( + FifoFile comm_file) + : m_io(comm_file, "runInTerminal launcher") {} + // Can't use \a std::future<llvm::Error> because it doesn't compile on Windows std::future<lldb::SBError> RunInTerminalDebugAdapterCommChannel::NotifyDidAttach() { @@ -159,12 +169,19 @@ std::string RunInTerminalDebugAdapterCommChannel::GetLauncherError() { Expected<std::shared_ptr<FifoFile>> CreateRunInTerminalCommFile() { SmallString<256> comm_file; +#if _WIN32 + char pipe_name[MAX_PATH]; + sprintf(pipe_name, "\\\\.\\pipe\\lldb-dap-run-in-terminal-comm-%d", + GetCurrentProcessId()); + return CreateFifoFile(pipe_name); +#else 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()); +#endif } } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/RunInTerminal.h b/lldb/tools/lldb-dap/RunInTerminal.h index 457850c8ea538..ff727603709d3 100644 --- a/lldb/tools/lldb-dap/RunInTerminal.h +++ b/lldb/tools/lldb-dap/RunInTerminal.h @@ -89,6 +89,8 @@ class RunInTerminalLauncherCommChannel { /// out. llvm::Error NotifyPid(); + llvm::Error NotifyPid(int pid); + /// Notify the debug adapter that there's been an error. void NotifyError(llvm::StringRef error); @@ -99,6 +101,7 @@ class RunInTerminalLauncherCommChannel { class RunInTerminalDebugAdapterCommChannel { public: RunInTerminalDebugAdapterCommChannel(llvm::StringRef comm_file); + RunInTerminalDebugAdapterCommChannel(FifoFile comm_file); /// Notify the runInTerminal launcher that it was attached. /// diff --git a/lldb/tools/lldb-dap/tool/lldb-dap.cpp b/lldb/tools/lldb-dap/tool/lldb-dap.cpp index e3b9d57e7d3a1..861c69451c0a4 100644 --- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp @@ -71,6 +71,9 @@ #undef GetObject #include <io.h> typedef int socklen_t; +#include "lldb/Host/windows/ProcessLauncherWindows.h" +#include "llvm/Support/ConvertUTF.h" +#include "llvm/Support/Program.h" #else #include <netinet/in.h> #include <sys/socket.h> @@ -179,6 +182,22 @@ static llvm::Error LaunchClient(const llvm::opt::InputArgList &args) { return ClientLauncher::GetLauncher(*client)->Launch(launch_args); } +llvm::Error +notifyError(RunInTerminalLauncherCommChannel &comm_channel, std::string message, + std::optional<std::error_code> error_code = std::nullopt) { + comm_channel.NotifyError(message); + + std::error_code ec = error_code.value_or( +#ifdef _WIN32 + std::error_code(GetLastError(), std::system_category()) +#else + llvm::inconvertibleErrorCode() +#endif + ); + + return llvm::createStringError(ec, std::move(message)); +} + #if not defined(_WIN32) struct FDGroup { int GetFlags() const { @@ -271,10 +290,10 @@ static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, lldb::pid_t debugger_pid, llvm::StringRef stdio, char *argv[]) { -#if defined(_WIN32) - return llvm::createStringError( - "runInTerminal is only supported on POSIX systems"); -#else + // This env var should be used only for tests. + const char *timeout_env_var = getenv("LLDB_DAP_RIT_TIMEOUT_IN_MS"); + int timeout_in_ms = + timeout_env_var != nullptr ? atoi(timeout_env_var) : 20000; // On Linux with the Yama security module enabled, a process can only attach // to its descendants by default. In the runInTerminal case the target @@ -285,6 +304,94 @@ static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, #endif lldb_private::FileSystem::Initialize(); + +#ifdef _WIN32 + RunInTerminalLauncherCommChannel comm_channel(comm_file); + + auto wcommandLineOrErr = + lldb_private::GetFlattenedWindowsCommandStringW(argv); + if (!wcommandLineOrErr) + return notifyError(comm_channel, "Failed to process arguments"); + + STARTUPINFOEXW startupinfoex = {}; + startupinfoex.StartupInfo.cb = sizeof(STARTUPINFOEXW); + startupinfoex.StartupInfo.dwFlags |= STARTF_USESTDHANDLES; + + HANDLE stdin_handle = GetStdHandle(STD_INPUT_HANDLE); + HANDLE stdout_handle = GetStdHandle(STD_OUTPUT_HANDLE); + HANDLE stderr_handle = GetStdHandle(STD_ERROR_HANDLE); + llvm::scope_exit close_handles([&] { + if (stdin_handle) + ::CloseHandle(stdin_handle); + if (stdout_handle) + ::CloseHandle(stdout_handle); + if (stderr_handle) + ::CloseHandle(stderr_handle); + }); + + auto attributelist_cleanup_or_err = + lldb_private::SetupProcThreadAttributeList(startupinfoex); + if (!attributelist_cleanup_or_err) { + return notifyError(comm_channel, "Could not open inherited handles", + attributelist_cleanup_or_err.getError()); + } + auto attributelist_cleanup = std::move(*attributelist_cleanup_or_err); + + if (!stdio.empty()) { + llvm::SmallVector<llvm::StringRef, 3> files; + stdio.split(files, ':'); + while (files.size() < 3) + files.push_back(files.back()); + + stdin_handle = lldb_private::ProcessLauncherWindows::GetStdioHandle( + files[0], STDIN_FILENO); + stdout_handle = lldb_private::ProcessLauncherWindows::GetStdioHandle( + files[1], STDOUT_FILENO); + stderr_handle = lldb_private::ProcessLauncherWindows::GetStdioHandle( + files[2], STDERR_FILENO); + } + auto inherited_handles_or_err = + lldb_private::ProcessLauncherWindows::GetInheritedHandles( + startupinfoex, /*launch_info*=*/nullptr, stdout_handle, stderr_handle, + stdin_handle); + + if (!inherited_handles_or_err) + return notifyError(comm_channel, "Failed to get inherited handles", + inherited_handles_or_err.getError()); + std::vector<HANDLE> inherited_handles = std::move(*inherited_handles_or_err); + + PROCESS_INFORMATION pi = {}; + + // Start the process in a suspended state, while we attach the debugger. + BOOL result = CreateProcessW( + /*lpApplicationName=*/NULL, /*lpCommandLine=*/&(*wcommandLineOrErr)[0], + /*lpProcessAttributes=*/NULL, /*lpThreadAttributes=*/NULL, + /*bInheritHandles=*/!inherited_handles.empty(), + /*dwCreationFlags=*/CREATE_SUSPENDED, /*lpEnvironment=*/NULL, + /*lpCurrentDirectory=*/NULL, + /*lpStartupInfo=*/reinterpret_cast<STARTUPINFOW *>(&startupinfoex), + /*lpProcessInformation=*/&pi); + + if (!result) + return notifyError(comm_channel, "Failed to launch target process"); + + // Notify the pid of the process to debug to the debugger. It will attach to + // the newly created process. + if (llvm::Error err = comm_channel.NotifyPid(pi.dwProcessId)) + return err; + + if (llvm::Error err = comm_channel.WaitUntilDebugAdapterAttaches( + std::chrono::milliseconds(timeout_in_ms))) + return err; + + // The debugger attached to the process. We can resume it. + if (!ResumeThread(pi.hThread)) + return notifyError(comm_channel, "Failed to resume the target process"); + + // Wait for child to complete to match POSIX behavior. + WaitForSingleObject(pi.hProcess, INFINITE); + ExitProcess(0); // TODO: Should we return the exit code of the process? +#else if (!stdio.empty()) { llvm::SmallVector<llvm::StringRef, 3> files; stdio.split(files, ':'); @@ -310,10 +417,6 @@ static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, // We will wait to be attached with a timeout. We don't wait indefinitely // using a signal to prevent being paused forever. - // This env var should be used only for tests. - const char *timeout_env_var = getenv("LLDB_DAP_RIT_TIMEOUT_IN_MS"); - int timeout_in_ms = - timeout_env_var != nullptr ? atoi(timeout_env_var) : 20000; if (llvm::Error err = comm_channel.WaitUntilDebugAdapterAttaches( std::chrono::milliseconds(timeout_in_ms))) { return err; @@ -322,10 +425,7 @@ static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, const char *target = target_arg.getValue(); execvp(target, argv); - std::string error = std::strerror(errno); - comm_channel.NotifyError(error); - return llvm::createStringError(llvm::inconvertibleErrorCode(), - std::move(error)); + return notifyError(comm_channel, std::strerror(errno)); #endif } diff --git a/lldb/unittests/DAP/FifoFilesTest.cpp b/lldb/unittests/DAP/FifoFilesTest.cpp index bbc1b608e91bd..bd8aa64ef1ecd 100644 --- a/lldb/unittests/DAP/FifoFilesTest.cpp +++ b/lldb/unittests/DAP/FifoFilesTest.cpp @@ -45,8 +45,8 @@ TEST(FifoFilesTest, SendAndReceiveJSON) { auto fifo = CreateFifoFile(fifo_path); EXPECT_THAT_EXPECTED(fifo, llvm::Succeeded()); - FifoFileIO writer(fifo_path, "writer"); - FifoFileIO reader(fifo_path, "reader"); + FifoFileIO writer(*fifo.get(), "writer"); + FifoFileIO reader(*fifo.get(), "reader"); llvm::json::Object obj; obj["foo"] = "bar"; @@ -79,7 +79,7 @@ TEST(FifoFilesTest, ReadTimeout) { auto fifo = CreateFifoFile(fifo_path); EXPECT_THAT_EXPECTED(fifo, llvm::Succeeded()); - FifoFileIO reader(fifo_path, "reader"); + FifoFileIO reader(*fifo.get(), "reader"); // No writer, should timeout. auto result = reader.ReadJSON(std::chrono::milliseconds(100)); @@ -91,7 +91,7 @@ TEST(FifoFilesTest, WriteTimeout) { auto fifo = CreateFifoFile(fifo_path); EXPECT_THAT_EXPECTED(fifo, llvm::Succeeded()); - FifoFileIO writer(fifo_path, "writer"); + FifoFileIO writer(*fifo.get(), "writer"); // No reader, should timeout. llvm::json::Object obj; >From c2791c10e53dd8011ab2f7ca237fcb1be04d7291 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Wed, 7 Jan 2026 19:46:28 +0000 Subject: [PATCH 2/2] fix formatting --- .../runInTerminal/TestDAP_runInTerminal.py | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) 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 c92cae16b9ea1..a814b3682c359 100644 --- a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py +++ b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py @@ -10,12 +10,14 @@ import subprocess import json + @contextmanager def fifo(*args, **kwargs): if sys.platform == "win32": comm_file = r"\\.\pipe\lldb-dap-run-in-terminal-comm" import ctypes + PIPE_ACCESS_DUPLEX = 0x00000003 PIPE_TYPE_BYTE = 0x00000000 PIPE_READMODE_BYTE = 0x00000000 @@ -27,7 +29,11 @@ def fifo(*args, **kwargs): comm_file, PIPE_ACCESS_DUPLEX, PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, - PIPE_UNLIMITED_INSTANCES, 4096, 4096, 0, None + PIPE_UNLIMITED_INSTANCES, + 4096, + 4096, + 0, + None, ) else: comm_file = os.path.join(kwargs["directory"], "comm-file") @@ -41,11 +47,13 @@ def fifo(*args, **kwargs): kernel32.DisconnectNamedPipe(pipe) kernel32.CloseHandle(pipe) + @skipIfBuildType(["debug"]) class TestDAP_runInTerminal(lldbdap_testcase.DAPTestCaseBase): def read_pid_message(self, fifo_file, pipe): if sys.platform == "win32": import ctypes + buffer = ctypes.create_string_buffer(4096) bytes_read = ctypes.wintypes.DWORD() kernel32 = ctypes.windll.kernel32 @@ -61,10 +69,13 @@ def send_did_attach_message(fifo_file, pipe=None): message = json.dumps({"kind": "didAttach"}) + "\n" if sys.platform == "win32": import ctypes + kernel32 = ctypes.windll.kernel32 bytes_written = ctypes.wintypes.DWORD() kernel32.ConnectNamedPipe(pipe, None) - kernel32.WriteFile(pipe, message.encode(), len(message), ctypes.byref(bytes_written), None) + kernel32.WriteFile( + pipe, message.encode(), len(message), ctypes.byref(bytes_written), None + ) else: with open(fifo_file, "w") as file: file.write(message) @@ -217,7 +228,10 @@ def test_FakeAttachedRunInTerminalLauncherWithInvalidProgramWindows(self): stderr=subprocess.PIPE, ) - self.assertIn("Failed to launch target process", self.read_error_message(comm_file, pipe)) + self.assertIn( + "Failed to launch target process", + self.read_error_message(comm_file, pipe), + ) def test_FakeAttachedRunInTerminalLauncherWithValidProgram(self): with fifo(directory=self.getBuildDir()) as (comm_file, pipe): _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
