https://github.com/charles-zablit updated https://github.com/llvm/llvm-project/pull/174635
>From ac2d700c7b2f3d92b03b451ed9531d2874546deb Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Wed, 7 Jan 2026 19:39:39 +0000 Subject: [PATCH 01/11] [lldb-dap][windows] add support for the integratedTerminal flag --- .../Host/windows/ProcessLauncherWindows.h | 81 ++++++++- .../Host/windows/ProcessLauncherWindows.cpp | 54 +++--- .../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 +- 10 files changed, 436 insertions(+), 109 deletions(-) diff --git a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h index b835885dd47c6..f31dfee62bb52 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; @@ -68,9 +73,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. /// @@ -81,12 +83,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 @@ -95,12 +97,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/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp index 76feceadf46f3..a9489bb925ebc 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,8 @@ 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 +75,16 @@ 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<ProcThreadAttributeList> ProcThreadAttributeList::Create(STARTUPINFOEXW &startupinfoex) { SIZE_T attributelist_size = 0; @@ -137,8 +139,6 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, error = attributelist_or_err.getError(); return HostProcess(); } - llvm::scope_exit delete_attributelist( - [&] { DeleteProcThreadAttributeList(startupinfoex.lpAttributeList); }); std::vector<HANDLE> inherited_handles; if (use_pty) { @@ -149,8 +149,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(); @@ -223,7 +224,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; @@ -241,12 +242,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; @@ -267,6 +269,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; @@ -287,7 +298,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 0e78a5f36a4e4..159d07057bb59 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 b06a43d7421c1..a1976e94094e2 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp @@ -105,7 +105,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) @@ -118,7 +118,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 0119f4b1083a9..f8de1b306ac65 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 b59e70b47491b2e25defbd107c09a9f560a41842 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Wed, 7 Jan 2026 19:46:28 +0000 Subject: [PATCH 02/11] 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 159d07057bb59..9a14034a11e97 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): >From b408e9e0bb29852bdf42fd0075547ba7dcfcd20a Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Wed, 7 Jan 2026 19:49:10 +0000 Subject: [PATCH 03/11] fixup! fix formatting --- .../API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py | 4 ++-- 1 file changed, 2 insertions(+), 2 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 9a14034a11e97..cd47f6829d5cf 100644 --- a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py +++ b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py @@ -14,10 +14,9 @@ @contextmanager def fifo(*args, **kwargs): if sys.platform == "win32": - comm_file = r"\\.\pipe\lldb-dap-run-in-terminal-comm" - import ctypes + comm_file = r"\\.\pipe\lldb-dap-run-in-terminal-comm" PIPE_ACCESS_DUPLEX = 0x00000003 PIPE_TYPE_BYTE = 0x00000000 PIPE_READMODE_BYTE = 0x00000000 @@ -84,6 +83,7 @@ def send_did_attach_message(fifo_file, pipe=None): 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 >From cd05d63ee6d20c76d2af9e2f34f8504ed4e20dc6 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Thu, 8 Jan 2026 16:17:59 +0000 Subject: [PATCH 04/11] address comments --- .../Host/windows/ProcessLauncherWindows.h | 1 + .../runInTerminal/TestDAP_runInTerminal.py | 85 +++++++------------ lldb/tools/lldb-dap/FifoFiles.cpp | 17 ++-- lldb/tools/lldb-dap/FifoFiles.h | 16 +++- .../tools/lldb-dap/Handler/RequestHandler.cpp | 7 +- lldb/tools/lldb-dap/RunInTerminal.cpp | 8 +- lldb/tools/lldb-dap/RunInTerminal.h | 2 +- lldb/tools/lldb-dap/tool/lldb-dap.cpp | 47 ++++++---- 8 files changed, 99 insertions(+), 84 deletions(-) diff --git a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h index f31dfee62bb52..8db4389df900d 100644 --- a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h +++ b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h @@ -15,6 +15,7 @@ #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/ErrorOr.h" +#include "llvm/Support/WindowsError.h" #include <optional> 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 cd47f6829d5cf..23fcb6c433aa0 100644 --- a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py +++ b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py @@ -189,34 +189,9 @@ def test_missingArgInRunInTerminalLauncher(self): '"--launch-target" requires "--comm-file" to be specified', proc.stderr ) - @skipIfWindows def test_FakeAttachedRunInTerminalLauncherWithInvalidProgram(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", - "INVALIDPROGRAM", - ], - universal_newlines=True, - stderr=subprocess.PIPE, - ) - - self.read_pid_message(comm_file) - self.send_did_attach_message(comm_file) - self.assertIn("No such file or directory", self.read_error_message(comm_file)) - - _, stderr = proc.communicate() - self.assertIn("No such file or directory", stderr) - - @skipUnlessWindows - def test_FakeAttachedRunInTerminalLauncherWithInvalidProgramWindows(self): with fifo(directory=self.getBuildDir()) as (comm_file, pipe): - subprocess.Popen( + proc = subprocess.Popen( [ self.lldbDAPExec, "--comm-file", @@ -227,11 +202,19 @@ def test_FakeAttachedRunInTerminalLauncherWithInvalidProgramWindows(self): universal_newlines=True, stderr=subprocess.PIPE, ) - - self.assertIn( - "Failed to launch target process", - self.read_error_message(comm_file, pipe), - ) + if sys.platform == "win32": + _, stderr = proc.communicate() + self.assertIn("Failed to launch target process", stderr) + else: + self.read_pid_message(comm_file, pipe) + self.send_did_attach_message(comm_file, pipe) + self.assertIn( + "No such file or directory", + self.read_error_message(comm_file, pipe), + ) + + _, stderr = proc.communicate() + self.assertIn("No such file or directory", stderr) def test_FakeAttachedRunInTerminalLauncherWithValidProgram(self): with fifo(directory=self.getBuildDir()) as (comm_file, pipe): @@ -269,26 +252,24 @@ def test_FakeAttachedRunInTerminalLauncherAndCheckEnvironment(self): stdout, _ = proc.communicate() self.assertIn("FOO=BAR", stdout) - @skipIfWindows def test_NonAttachedRunInTerminalLauncher(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, - stderr=subprocess.PIPE, - env={**os.environ, "LLDB_DAP_RIT_TIMEOUT_IN_MS": "1000"}, - ) - - self.read_pid_message(comm_file) + 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, + stderr=subprocess.PIPE, + env={**os.environ, "LLDB_DAP_RIT_TIMEOUT_IN_MS": "1000"}, + ) - _, stderr = proc.communicate() - self.assertIn("Timed out trying to get messages from the debug adapter", stderr) + self.read_pid_message(comm_file, pipe) + _, stderr = proc.communicate() + self.assertIn( + "Timed out trying to get messages from the debug adapter", stderr + ) diff --git a/lldb/tools/lldb-dap/FifoFiles.cpp b/lldb/tools/lldb-dap/FifoFiles.cpp index 3775e2f6b0143..a5fa8121260eb 100644 --- a/lldb/tools/lldb-dap/FifoFiles.cpp +++ b/lldb/tools/lldb-dap/FifoFiles.cpp @@ -38,7 +38,12 @@ FifoFile::FifoFile(StringRef path, lldb::pipe_t pipe) : m_path(path) { } FifoFile::~FifoFile() { -#if !defined(_WIN32) +#ifdef _WIN32 + if (m_pipe != INVALID_HANDLE_VALUE) { + DisconnectNamedPipe(m_pipe); + CloseHandle(m_pipe); + } +#else unlink(m_path.c_str()); #endif } @@ -108,8 +113,10 @@ Expected<std::shared_ptr<FifoFile>> CreateFifoFile(StringRef path) { #endif } -FifoFileIO::FifoFileIO(FifoFile fifo_file, StringRef other_endpoint_name) - : m_fifo_file(fifo_file), m_other_endpoint_name(other_endpoint_name) {} +FifoFileIO::FifoFileIO(std::shared_ptr<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 @@ -117,7 +124,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::string buffer = m_fifo_file.ReadLine(); + std::string buffer = m_fifo_file->ReadLine(); if (!buffer.empty()) line = buffer; })); @@ -141,7 +148,7 @@ Error FifoFileIO::SendJSON(const json::Value &json, bool done = false; std::future<void> *future = new std::future<void>(std::async(std::launch::async, [&]() { - m_fifo_file.WriteLine(JSONToString(json)); + 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 e21471fcc78c0..9d3dc7314627f 100644 --- a/lldb/tools/lldb-dap/FifoFiles.h +++ b/lldb/tools/lldb-dap/FifoFiles.h @@ -9,7 +9,9 @@ #ifndef LLDB_TOOLS_LLDB_DAP_FIFOFILES_H #define LLDB_TOOLS_LLDB_DAP_FIFOFILES_H +#ifdef _WIN32 #include "lldb/Host/windows/PipeWindows.h" +#endif #include "llvm/Support/Error.h" #include "llvm/Support/JSON.h" @@ -31,6 +33,15 @@ struct FifoFile { std::string ReadLine(); + llvm::StringRef GetPath() { return m_path; } + + /// FifoFile is not copyable. + /// @{ + FifoFile(const FifoFile &rhs) = delete; + void operator=(const FifoFile &rhs) = delete; + /// @} + +protected: std::string m_path; lldb::pipe_t m_pipe; }; @@ -53,7 +64,8 @@ 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(FifoFile 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. /// @@ -84,7 +96,7 @@ class FifoFileIO { std::chrono::milliseconds timeout = std::chrono::milliseconds(20000)); private: - FifoFile m_fifo_file; + std::shared_ptr<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 a1976e94094e2..b70db5d6aab15 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp @@ -103,7 +103,7 @@ 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); @@ -114,11 +114,12 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) { llvm::json::Object reverse_request = CreateRunInTerminalReverseRequest( arguments.configuration.program, arguments.args, arguments.env, - arguments.cwd, comm_file.m_path, debugger_pid, arguments.stdio, + arguments.cwd, comm_file->GetPath(), debugger_pid, arguments.stdio, 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. + 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 7d20ca1c13987..490a4f4361ee8 100644 --- a/lldb/tools/lldb-dap/RunInTerminal.cpp +++ b/lldb/tools/lldb-dap/RunInTerminal.cpp @@ -99,7 +99,7 @@ static Error ToError(const RunInTerminalMessage &message) { RunInTerminalLauncherCommChannel::RunInTerminalLauncherCommChannel( StringRef comm_file) - : m_io(FifoFileIO(FifoFile(comm_file), "debug adapter")) {} + : m_io(std::make_shared<FifoFile>(comm_file), "debug adapter") {} Error RunInTerminalLauncherCommChannel::WaitUntilDebugAdapterAttaches( std::chrono::milliseconds timeout) { @@ -129,11 +129,11 @@ void RunInTerminalLauncherCommChannel::NotifyError(StringRef error) { RunInTerminalDebugAdapterCommChannel::RunInTerminalDebugAdapterCommChannel( StringRef comm_file) - : m_io(comm_file, "runInTerminal launcher") {} + : m_io(std::make_shared<FifoFile>(comm_file), "runInTerminal launcher") {} RunInTerminalDebugAdapterCommChannel::RunInTerminalDebugAdapterCommChannel( - FifoFile comm_file) - : m_io(comm_file, "runInTerminal launcher") {} + std::shared_ptr<FifoFile> comm_file) + : m_io(std::move(comm_file), "runInTerminal launcher") {} // Can't use \a std::future<llvm::Error> because it doesn't compile on Windows std::future<lldb::SBError> diff --git a/lldb/tools/lldb-dap/RunInTerminal.h b/lldb/tools/lldb-dap/RunInTerminal.h index ff727603709d3..1c6d92bbbc774 100644 --- a/lldb/tools/lldb-dap/RunInTerminal.h +++ b/lldb/tools/lldb-dap/RunInTerminal.h @@ -101,7 +101,7 @@ class RunInTerminalLauncherCommChannel { class RunInTerminalDebugAdapterCommChannel { public: RunInTerminalDebugAdapterCommChannel(llvm::StringRef comm_file); - RunInTerminalDebugAdapterCommChannel(FifoFile comm_file); + RunInTerminalDebugAdapterCommChannel(std::shared_ptr<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 f8de1b306ac65..b427f7ccaa416 100644 --- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp @@ -320,22 +320,13 @@ static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, 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) { + auto attributelist_or_err = + lldb_private::ProcThreadAttributeList::Create(startupinfoex); + if (!attributelist_or_err) { return notifyError(comm_channel, "Could not open inherited handles", - attributelist_cleanup_or_err.getError()); + attributelist_or_err.getError()); } - auto attributelist_cleanup = std::move(*attributelist_cleanup_or_err); if (!stdio.empty()) { llvm::SmallVector<llvm::StringRef, 3> files; @@ -349,6 +340,15 @@ static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, files[1], STDOUT_FILENO); stderr_handle = lldb_private::ProcessLauncherWindows::GetStdioHandle( files[2], STDERR_FILENO); + // Only close the handles we created + llvm::scope_exit close_handles([&] { + if (stdin_handle) + CloseHandle(stdin_handle); + if (stdout_handle) + CloseHandle(stdout_handle); + if (stderr_handle) + CloseHandle(stderr_handle); + }); } auto inherited_handles_or_err = lldb_private::ProcessLauncherWindows::GetInheritedHandles( @@ -375,22 +375,35 @@ static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, if (!result) return notifyError(comm_channel, "Failed to launch target process"); + auto cleanupAndReturn = [&](llvm::Error err) -> llvm::Error { + if (pi.hProcess) + TerminateProcess(pi.hProcess, 1); + if (pi.hThread) + CloseHandle(pi.hThread); + if (pi.hProcess) + CloseHandle(pi.hProcess); + return err; + }; + // 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; + return cleanupAndReturn(std::move(err)); if (llvm::Error err = comm_channel.WaitUntilDebugAdapterAttaches( std::chrono::milliseconds(timeout_in_ms))) - return err; + return cleanupAndReturn(std::move(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"); + return cleanupAndReturn( + 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? + CloseHandle(pi.hThread); + CloseHandle(pi.hProcess); + return llvm::Error::success(); #else if (!stdio.empty()) { llvm::SmallVector<llvm::StringRef, 3> files; >From d25e889385d4d205aa449ae9199cdfa099474703 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Thu, 8 Jan 2026 16:36:08 +0000 Subject: [PATCH 05/11] remove declaration --- .../lldb/Host/windows/ProcessLauncherWindows.h | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h index 8db4389df900d..70ac0fb9155ba 100644 --- a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h +++ b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h @@ -150,21 +150,6 @@ llvm::ErrorOr<std::wstring> GetFlattenedWindowsCommandStringW(Args args); /// \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 >From 6e379b02d62f7e8018f5de28ffa16737462f1acf Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Thu, 8 Jan 2026 16:43:26 +0000 Subject: [PATCH 06/11] fix build issue linux --- lldb/tools/lldb-dap/FifoFiles.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lldb/tools/lldb-dap/FifoFiles.h b/lldb/tools/lldb-dap/FifoFiles.h index 9d3dc7314627f..87ac2c66e6382 100644 --- a/lldb/tools/lldb-dap/FifoFiles.h +++ b/lldb/tools/lldb-dap/FifoFiles.h @@ -9,9 +9,7 @@ #ifndef LLDB_TOOLS_LLDB_DAP_FIFOFILES_H #define LLDB_TOOLS_LLDB_DAP_FIFOFILES_H -#ifdef _WIN32 -#include "lldb/Host/windows/PipeWindows.h" -#endif +#include "lldb/Host/Pipe.h" #include "llvm/Support/Error.h" #include "llvm/Support/JSON.h" >From 1b88a83ecc5e569c35beafbf80093a035acc8925 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Thu, 8 Jan 2026 17:05:47 +0000 Subject: [PATCH 07/11] fix unit test linux --- lldb/unittests/DAP/FifoFilesTest.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lldb/unittests/DAP/FifoFilesTest.cpp b/lldb/unittests/DAP/FifoFilesTest.cpp index bd8aa64ef1ecd..4931cfaa30d42 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.get(), "writer"); - FifoFileIO reader(*fifo.get(), "reader"); + FifoFileIO writer(std::move(*fifo), "writer"); + FifoFileIO reader(std::move(*fifo), "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.get(), "reader"); + FifoFileIO reader(std::move(*fifo), "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.get(), "writer"); + FifoFileIO writer(std::move(*fifo), "writer"); // No reader, should timeout. llvm::json::Object obj; >From 031aa126b30ca3cdd52f33f9b0c3c1852bfcfa98 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Thu, 8 Jan 2026 18:06:12 +0000 Subject: [PATCH 08/11] remove move --- lldb/unittests/DAP/FifoFilesTest.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lldb/unittests/DAP/FifoFilesTest.cpp b/lldb/unittests/DAP/FifoFilesTest.cpp index 4931cfaa30d42..e59fc54cc9543 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(std::move(*fifo), "writer"); - FifoFileIO reader(std::move(*fifo), "reader"); + FifoFileIO writer(*fifo, "writer"); + FifoFileIO reader(*fifo, "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(std::move(*fifo), "reader"); + FifoFileIO reader(*fifo, "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(std::move(*fifo), "writer"); + FifoFileIO writer(*fifo, "writer"); // No reader, should timeout. llvm::json::Object obj; >From ec9a2d8ed974ac162c922a6577a89673c3f6426f Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Thu, 8 Jan 2026 18:31:56 +0000 Subject: [PATCH 09/11] fix formatting --- lldb/source/Host/windows/ProcessLauncherWindows.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp index a9489bb925ebc..b1fbaa6e23aa8 100644 --- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp +++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp @@ -83,7 +83,7 @@ llvm::ErrorOr<std::wstring> GetFlattenedWindowsCommandStringW(char *args[]) { return llvm::sys::flattenWindowsCommandLine(args_ref); } -} +} // namespace lldb_private llvm::ErrorOr<ProcThreadAttributeList> ProcThreadAttributeList::Create(STARTUPINFOEXW &startupinfoex) { >From 16e51d729c38f7a2310491a7a97bc58b41309066 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Mon, 12 Jan 2026 18:30:47 +0000 Subject: [PATCH 10/11] fix early CloseHandle call --- lldb/tools/lldb-dap/tool/lldb-dap.cpp | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/lldb/tools/lldb-dap/tool/lldb-dap.cpp b/lldb/tools/lldb-dap/tool/lldb-dap.cpp index b427f7ccaa416..d8979b1869bdd 100644 --- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp @@ -340,16 +340,20 @@ static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, files[1], STDOUT_FILENO); stderr_handle = lldb_private::ProcessLauncherWindows::GetStdioHandle( files[2], STDERR_FILENO); - // Only close the handles we created - llvm::scope_exit close_handles([&] { - if (stdin_handle) - CloseHandle(stdin_handle); - if (stdout_handle) - CloseHandle(stdout_handle); - if (stderr_handle) - CloseHandle(stderr_handle); - }); } + + llvm::scope_exit close_handles([&] { + // Only close the handles we created + if (stdio.empty()) + return; + if (stdin_handle) + CloseHandle(stdin_handle); + if (stdout_handle) + CloseHandle(stdout_handle); + if (stderr_handle) + CloseHandle(stderr_handle); + }); + auto inherited_handles_or_err = lldb_private::ProcessLauncherWindows::GetInheritedHandles( startupinfoex, /*launch_info*=*/nullptr, stdout_handle, stderr_handle, >From e19b1084eb7f6341487aefded0d4f8c9cbee875f Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Mon, 19 Jan 2026 17:01:17 +0000 Subject: [PATCH 11/11] separate the stdio parameter in 3 separate parameters --- .../tools/lldb-dap/Handler/RequestHandler.cpp | 44 +++---- lldb/tools/lldb-dap/JSONUtils.cpp | 26 ++-- lldb/tools/lldb-dap/JSONUtils.h | 15 ++- .../lldb-dap/Protocol/ProtocolRequests.cpp | 5 +- .../lldb-dap/Protocol/ProtocolRequests.h | 10 +- lldb/tools/lldb-dap/tool/Options.td | 19 ++- lldb/tools/lldb-dap/tool/lldb-dap.cpp | 112 +++++++----------- 7 files changed, 111 insertions(+), 120 deletions(-) diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp index b70db5d6aab15..364410c8411ad 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp @@ -58,31 +58,16 @@ static uint32_t SetLaunchFlag(uint32_t flags, bool flag, return flags; } -static void -SetupIORedirection(const std::vector<std::optional<std::string>> &stdio, - lldb::SBLaunchInfo &launch_info) { - size_t n = std::max(stdio.size(), static_cast<size_t>(3)); - for (size_t i = 0; i < n; i++) { - std::optional<std::string> path; - if (stdio.size() <= i) - path = stdio.back(); - else - path = stdio[i]; - if (!path) - continue; - switch (i) { - case 0: - launch_info.AddOpenFileAction(i, path->c_str(), true, false); - break; - case 1: - case 2: - launch_info.AddOpenFileAction(i, path->c_str(), false, true); - break; - default: - launch_info.AddOpenFileAction(i, path->c_str(), true, true); - break; - } - } +static void SetupIORedirection(const std::optional<std::string> &stdin_path, + const std::optional<std::string> &stdout_path, + const std::optional<std::string> &stderr_path, + lldb::SBLaunchInfo &launch_info) { + if (stdin_path) + launch_info.AddOpenFileAction(0, stdin_path->c_str(), true, false); + if (stdout_path) + launch_info.AddOpenFileAction(1, stdout_path->c_str(), false, true); + if (stderr_path) + launch_info.AddOpenFileAction(2, stderr_path->c_str(), false, true); } static llvm::Error @@ -114,7 +99,8 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) { llvm::json::Object reverse_request = CreateRunInTerminalReverseRequest( arguments.configuration.program, arguments.args, arguments.env, - arguments.cwd, comm_file->GetPath(), debugger_pid, arguments.stdio, + arguments.cwd, comm_file->GetPath(), debugger_pid, arguments.stdin_path, + arguments.stdout_path, arguments.stderr_path, arguments.console == protocol::eConsoleExternalTerminal); dap.SendReverseRequest<LogFailureResponseHandler>("runInTerminal", std::move(reverse_request)); @@ -213,8 +199,10 @@ llvm::Error BaseRequestHandler::LaunchProcess( launch_info.SetEnvironment(env, true); } - if (!arguments.stdio.empty() && !arguments.disableSTDIO) - SetupIORedirection(arguments.stdio, launch_info); + if ((arguments.stdin_path || arguments.stdout_path || arguments.stdin_path) && + !arguments.disableSTDIO) + SetupIORedirection(arguments.stdin_path, arguments.stdout_path, + arguments.stderr_path, launch_info); launch_info.SetDetachOnError(arguments.detachOnError); launch_info.SetShellExpandArguments(arguments.shellExpandArguments); diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 9a2142cd847ab..5bb96e3e121e1 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -703,7 +703,9 @@ llvm::json::Object CreateRunInTerminalReverseRequest( llvm::StringRef program, const std::vector<std::string> &args, const llvm::StringMap<std::string> &env, llvm::StringRef cwd, llvm::StringRef comm_file, lldb::pid_t debugger_pid, - const std::vector<std::optional<std::string>> &stdio, bool external) { + const std::optional<std::string> &stdin_path, + const std::optional<std::string> &stdout_path, + const std::optional<std::string> &stderr_path, bool external) { llvm::json::Object run_in_terminal_args; if (external) { // This indicates the IDE to open an external terminal window. @@ -722,17 +724,17 @@ llvm::json::Object CreateRunInTerminalReverseRequest( } req_args.push_back("--launch-target"); req_args.push_back(program.str()); - if (!stdio.empty()) { - req_args.push_back("--stdio"); - std::stringstream ss; - for (const std::optional<std::string> &file : stdio) { - if (file) - ss << *file; - ss << ":"; - } - std::string files = ss.str(); - files.pop_back(); - req_args.push_back(std::move(files)); + if (stdin_path) { + req_args.push_back("--stdin-path"); + req_args.push_back(*stdin_path); + } + if (stdout_path) { + req_args.push_back("--stdout-path"); + req_args.push_back(*stdout_path); + } + if (stderr_path) { + req_args.push_back("--stderr-path"); + req_args.push_back(*stderr_path); } req_args.insert(req_args.end(), args.begin(), args.end()); run_in_terminal_args.try_emplace("args", req_args); diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h index 15449d6ece62a..0c2f51b776691 100644 --- a/lldb/tools/lldb-dap/JSONUtils.h +++ b/lldb/tools/lldb-dap/JSONUtils.h @@ -340,9 +340,14 @@ std::pair<int64_t, bool> UnpackLocation(int64_t location_id); /// launcher uses it on Linux tell the kernel that it should allow the /// debugger process to attach. /// -/// \param[in] stdio -/// An array of file paths for redirecting the program's standard IO -/// streams. +/// \param[in] stdin_path +/// A file path for redirecting the program's stdout stream. +/// +/// \param[in] stdout_path +/// A file path for redirecting the program's stdin stream. +/// +/// \param[in] stderr_path +/// A file path for redirecting the program's stderr stream. /// /// \param[in] external /// If set to true, the program will run in an external terminal window @@ -355,7 +360,9 @@ llvm::json::Object CreateRunInTerminalReverseRequest( llvm::StringRef program, const std::vector<std::string> &args, const llvm::StringMap<std::string> &env, llvm::StringRef cwd, llvm::StringRef comm_file, lldb::pid_t debugger_pid, - const std::vector<std::optional<std::string>> &stdio, bool external); + const std::optional<std::string> &stdin_path, + const std::optional<std::string> &stdout_path, + const std::optional<std::string> &stderr_path, bool external); /// Create a "Terminated" JSON object that contains statistics /// diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp index c3225f6ba0e35..40d6abe0ec019 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -307,7 +307,10 @@ bool fromJSON(const json::Value &Params, LaunchRequestArguments &LRA, O.mapOptional("shellExpandArguments", LRA.shellExpandArguments) && O.mapOptional("runInTerminal", LRA.console) && O.mapOptional("console", LRA.console) && - O.mapOptional("stdio", LRA.stdio) && parseEnv(Params, LRA.env, P); + O.mapOptional("stdin_path", LRA.stdin_path) && + O.mapOptional("stdout_path", LRA.stdout_path) && + O.mapOptional("stderr_path", LRA.stderr_path) && + parseEnv(Params, LRA.env, P); if (!success) return false; // Validate that we have a well formed launch request. diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index 9a99af9068ef1..2c23b7fd184a2 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -301,8 +301,14 @@ struct LaunchRequestArguments { /// terminal or external terminal. Console console = eConsoleInternal; - /// An array of file paths for redirecting the program's standard IO streams. - std::vector<std::optional<std::string>> stdio; + /// A file path for redirecting the program's stdin stream. + std::optional<std::string> stdin_path; + + /// A file path for redirecting the program's stdout stream. + std::optional<std::string> stdout_path; + + /// A file path for redirecting the program's stderr stream. + std::optional<std::string> stderr_path; /// @} }; diff --git a/lldb/tools/lldb-dap/tool/Options.td b/lldb/tools/lldb-dap/tool/Options.td index 339a64fed6c32..2dbcab8732d83 100644 --- a/lldb/tools/lldb-dap/tool/Options.td +++ b/lldb/tools/lldb-dap/tool/Options.td @@ -47,11 +47,20 @@ def debugger_pid: S<"debugger-pid">, HelpText<"The PID of the lldb-dap instance that sent the launchInTerminal " "request when using --launch-target.">; -def stdio: S<"stdio">, - MetaVarName<"<stdin:stdout:stderr:...>">, - HelpText<"An array of file paths for redirecting the program's standard IO " - "streams. A colon-separated list of entries. Empty value means no " - "redirection.">; +def stdin_path: S<"stdin-path">, + MetaVarName<"<stdin>">, + HelpText<"A file path for redirecting the program's stdin stream. Empty " + "value means no redirection.">; + +def stdout_path: S<"stdout-path">, + MetaVarName<"<stdout>">, + HelpText<"A file path for redirecting the program's stdout stream. Empty " + "value means no redirection.">; + +def stderr_path: S<"stderr-path">, + MetaVarName<"<stderr>">, + HelpText<"A file path for redirecting the program's stderr stream. Empty " + "value means no redirection.">; def repl_mode : S<"repl-mode">, diff --git a/lldb/tools/lldb-dap/tool/lldb-dap.cpp b/lldb/tools/lldb-dap/tool/lldb-dap.cpp index d8979b1869bdd..470fd4315b850 100644 --- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp @@ -213,54 +213,34 @@ struct FDGroup { bool write = false; }; -static llvm::Error RedirectToFile(const FDGroup &fdg, llvm::StringRef file) { - if (!fdg.read && !fdg.write) - return llvm::Error::success(); - int target_fd = lldb_private::FileSystem::Instance().Open( - file.str().c_str(), fdg.GetFlags(), 0666); - if (target_fd == -1) - return llvm::errorCodeToError( - std::error_code(errno, std::generic_category())); - for (int fd : fdg.fds) { - if (target_fd == fd) - continue; - if (::dup2(target_fd, fd) == -1) +static llvm::Error SetupIORedirection(llvm::StringRef stdin_path, + llvm::StringRef stdout_path, + llvm::StringRef stderr_path) { + if (!stdin_path.empty()) { + int target_fd = lldb_private::FileSystem::Instance().Open( + stdin_path.str().c_str(), O_NOCTTY | O_RDONLY, 0666); + if (target_fd == -1) return llvm::errorCodeToError( std::error_code(errno, std::generic_category())); + ::close(target_fd); } - ::close(target_fd); - return llvm::Error::success(); -} - -static llvm::Error -SetupIORedirection(const llvm::SmallVectorImpl<llvm::StringRef> &files) { - llvm::SmallDenseMap<llvm::StringRef, FDGroup> groups; - for (size_t i = 0; i < files.size(); i++) { - if (files[i].empty()) - continue; - auto group = groups.find(files[i]); - if (group == groups.end()) - group = groups.insert({files[i], {{static_cast<int>(i)}}}).first; - else - group->second.fds.push_back(i); - switch (i) { - case 0: - group->second.read = true; - break; - case 1: - case 2: - group->second.write = true; - break; - default: - group->second.read = true; - group->second.write = true; - break; - } + if (!stdout_path.empty()) { + int target_fd = lldb_private::FileSystem::Instance().Open( + stdout_path.str().c_str(), O_NOCTTY | O_CREAT | O_WRONLY | O_TRUNC, + 0666); + if (target_fd == -1) + return llvm::errorCodeToError( + std::error_code(errno, std::generic_category())); + ::close(target_fd); } - for (const auto &[file, group] : groups) { - if (llvm::Error err = RedirectToFile(group, file)) - return llvm::createStringError( - llvm::formatv("{0}: {1}", file, llvm::toString(std::move(err)))); + if (!stderr_path.empty()) { + int target_fd = lldb_private::FileSystem::Instance().Open( + stderr_path.str().c_str(), O_NOCTTY | O_CREAT | O_WRONLY | O_TRUNC, + 0666); + if (target_fd == -1) + return llvm::errorCodeToError( + std::error_code(errno, std::generic_category())); + ::close(target_fd); } return llvm::Error::success(); } @@ -285,11 +265,11 @@ SetupIORedirection(const llvm::SmallVectorImpl<llvm::StringRef> &files) { // // In case of errors launching the target, a suitable error message will be // emitted to the debug adapter. -static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, - llvm::StringRef comm_file, - lldb::pid_t debugger_pid, - llvm::StringRef stdio, - char *argv[]) { +static llvm::Error +LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, llvm::StringRef comm_file, + lldb::pid_t debugger_pid, llvm::StringRef stdin_path, + llvm::StringRef stdout_path, + llvm::StringRef stderr_path, char *argv[]) { // 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 = @@ -328,29 +308,23 @@ static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, attributelist_or_err.getError()); } - if (!stdio.empty()) { - llvm::SmallVector<llvm::StringRef, 3> files; - stdio.split(files, ':'); - while (files.size() < 3) - files.push_back(files.back()); - + if (!stdin_path.empty()) stdin_handle = lldb_private::ProcessLauncherWindows::GetStdioHandle( - files[0], STDIN_FILENO); + stdout_path, STDIN_FILENO); + if (!stdout_path.empty()) stdout_handle = lldb_private::ProcessLauncherWindows::GetStdioHandle( - files[1], STDOUT_FILENO); + stdout_path, STDOUT_FILENO); + if (!stderr_path.empty()) stderr_handle = lldb_private::ProcessLauncherWindows::GetStdioHandle( - files[2], STDERR_FILENO); - } + stderr_path, STDERR_FILENO); llvm::scope_exit close_handles([&] { // Only close the handles we created - if (stdio.empty()) - return; - if (stdin_handle) + if (!stdin_path.empty() && stdin_handle) CloseHandle(stdin_handle); - if (stdout_handle) + if (!stdout_path.empty() && stdout_handle) CloseHandle(stdout_handle); - if (stderr_handle) + if (!stderr_path.empty() && stderr_handle) CloseHandle(stderr_handle); }); @@ -705,10 +679,12 @@ int main(int argc, char *argv[]) { break; } } - llvm::StringRef stdio = input_args.getLastArgValue(OPT_stdio); - if (llvm::Error err = - LaunchRunInTerminalTarget(*target_arg, comm_file->getValue(), pid, - stdio, argv + target_args_pos)) { + llvm::StringRef stdin_path = input_args.getLastArgValue(OPT_stdin_path); + llvm::StringRef stdout_path = input_args.getLastArgValue(OPT_stdout_path); + llvm::StringRef stderr_path = input_args.getLastArgValue(OPT_stderr_path); + if (llvm::Error err = LaunchRunInTerminalTarget( + *target_arg, comm_file->getValue(), pid, stdin_path, stdout_path, + stderr_path, argv + target_args_pos)) { llvm::errs() << llvm::toString(std::move(err)) << '\n'; return EXIT_FAILURE; } _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
