diff --git a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h index 7e1402f9a7d93..876cff12468ef 100644 --- a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h +++ b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h @@ -81,7 +81,6 @@ class ProcessLauncherWindows : public ProcessLauncher { HostProcess LaunchProcess(const ProcessLaunchInfo &launch_info, Status &error) override; -protected: /// Get the list of Windows handles that should be inherited by the child /// process and update `STARTUPINFOEXW` with the handle list. /// diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py b/lldb/packages/Python/lldbsuite/test/dotest.py index f5cfb924c70d7..8761b45088640 100644 --- a/lldb/packages/Python/lldbsuite/test/dotest.py +++ b/lldb/packages/Python/lldbsuite/test/dotest.py @@ -583,7 +583,9 @@ def setupSysPath(): lldbDir = os.path.dirname(lldbtest_config.lldbExec) - lldbDAPExec = os.path.join(lldbDir, "lldb-dap") + lldbDAPExec = os.path.join( + lldbDir, "lldb-dap.exe" if sys.platform == "win32" else "lldb-dap" + ) 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 0d2e14264ede2..50e08db749546 100644 --- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp +++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp @@ -81,9 +81,7 @@ GetFlattenedWindowsCommandStringW(llvm::ArrayRef args) { if (args.empty()) return L""; - std::vector args_ref; - for (int i = 0; args[i] != nullptr; ++i) - args_ref.push_back(args[i]); + std::vector args_ref(args.begin(), args.end()); return llvm::sys::flattenWindowsCommandLine(args_ref); } diff --git a/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_console.py b/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_console.py index fc3ec356bab94..5d10282faa772 100644 --- a/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_console.py +++ b/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_console.py @@ -34,7 +34,7 @@ def verify_stopped_on_entry(self, stopped_events: List[Dict[str, Any]]): self.assertEqual(seen_stopped_event, 1, "expect only one stopped entry event.") @skipIfAsan - @skipIfWindows + @expectedFailureWindows @skipIf(oslist=["linux"], archs=["arm$"]) # Always times out on buildbot def test_basic_functionality(self): """ @@ -83,7 +83,7 @@ def test_basic_functionality(self): self.continue_to_exit() @skipIfAsan - @skipIfWindows + @expectedFailureWindows @skipIf(oslist=["linux"], archs=["arm$"]) # Always times out on buildbot def test_stopOnEntry(self): """ 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 3b5a0d843e620..865b399eb01a1 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 @@ -10,25 +11,101 @@ import json +@contextmanager +def fifo(*args, **kwargs): + if sys.platform == "win32": + import ctypes + + comm_file = r"\\.\pipe\lldb-dap-run-in-terminal-comm" + PIPE_ACCESS_DUPLEX = 0x00000003 + PIPE_TYPE_MESSAGE = 0x00000004 + PIPE_READMODE_MESSAGE = 0x00000002 + PIPE_WAIT = 0x00000000 + PIPE_UNLIMITED_INSTANCES = 255 + kernel32 = ctypes.windll.kernel32 + + pipe = kernel32.CreateNamedPipeW( + comm_file, + PIPE_ACCESS_DUPLEX, + PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | 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) + + +def read_pipe_message(pipe): + import ctypes + + ERROR_MORE_DATA = 234 + kernel32 = ctypes.windll.kernel32 + buffer = b"" + while True: + chunk = ctypes.create_string_buffer(4096) + bytes_read = ctypes.wintypes.DWORD() + success = kernel32.ReadFile(pipe, chunk, 4096, ctypes.byref(bytes_read), None) + buffer += chunk.raw[: bytes_read.value] + if success: + break + if ctypes.GetLastError() != ERROR_MORE_DATA: + break + return buffer.decode() + + @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 + + ctypes.windll.kernel32.ConnectNamedPipe(pipe, None) + self.assertIn("pid", read_pipe_message(pipe)) + 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 + + ctypes.windll.kernel32.ConnectNamedPipe(pipe, None) + return read_pipe_message(pipe) + else: + with open(fifo_file, "r") as file: + return file.readline() @skipIfLinux # FIXME: doesn't seem to work on Ubuntu 16.04. @skipIfAsan - @skipIfWindows @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) def test_runInTerminal(self): """ @@ -76,7 +153,6 @@ def test_runInTerminal(self): self.continue_to_exit() @skipIfAsan - @skipIfWindows @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) def test_runInTerminalWithObjectEnv(self): """ @@ -101,7 +177,6 @@ def test_runInTerminalWithObjectEnv(self): self.continue_to_exit() @skipIfLinux # FIXME: doesn't seem to work on Ubuntu 16.04. - @skipIfWindows @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) def test_runInTerminalInvalidTarget(self): self.build_and_create_debug_adapter() @@ -119,7 +194,6 @@ def test_runInTerminalInvalidTarget(self): ) @skipIfLinux # FIXME: doesn't seem to work on Ubuntu 16.04. - @skipIfWindows @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) def test_missingArgInRunInTerminalLauncher(self): proc = subprocess.run( @@ -133,99 +207,96 @@ def test_missingArgInRunInTerminalLauncher(self): ) @skipIfLinux # FIXME: doesn't seem to work on Ubuntu 16.04. - @skipIfWindows @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) 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) + with fifo(directory=self.getBuildDir()) as (comm_file, pipe): + proc = subprocess.Popen( + [ + self.lldbDAPExec, + "--comm-file", + comm_file, + "--launch-target", + "INVALIDPROGRAM", + ], + universal_newlines=True, + stderr=subprocess.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) @skipIfLinux # FIXME: doesn't seem to work on Ubuntu 16.04. - @skipIfWindows @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) 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, - ) - - 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", + "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() - stdout, _ = proc.communicate() self.assertIn("foo", stdout) @skipIfLinux # FIXME: doesn't seem to work on Ubuntu 16.04. - @skipIfWindows @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) def test_FakeAttachedRunInTerminalLauncherAndCheckEnvironment(self): - comm_file = os.path.join(self.getBuildDir(), "comm-file") - os.mkfifo(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"}, + ) - 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) - self.read_pid_message(comm_file) - self.send_did_attach_message(comm_file) + stdout, _ = proc.communicate() - stdout, _ = proc.communicate() self.assertIn("FOO=BAR", stdout) @skipIfLinux # FIXME: doesn't seem to work on Ubuntu 16.04. - @skipIfWindows @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) 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) - - _, stderr = proc.communicate() - self.assertIn("Timed out trying to get messages from the debug adapter", stderr) + 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"}, + ) + + self.read_pid_message(comm_file, pipe) + + _, stderr = proc.communicate() + + self.assertIn("Timed out trying to get messages from the debug adapter", stderr) \ No newline at end of file diff --git a/lldb/tools/lldb-dap/FifoFiles.cpp b/lldb/tools/lldb-dap/FifoFiles.cpp index 1f1bba80bd3b1..8f4485b921d94 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 #include #include @@ -24,17 +28,88 @@ 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) { + assert(path.starts_with("\\\\.\\pipe\\") && + "FifoFile path should start with '\\\\.\\pipe\\'"); + pipe = CreateFileA(m_path.c_str(), GENERIC_READ | GENERIC_WRITE, 0, NULL, + OPEN_EXISTING, FILE_FLAG_OVERLAPPED, NULL); + DWORD mode = PIPE_READMODE_MESSAGE; + SetNamedPipeHandleState(pipe, &mode, NULL, NULL); + } +#endif + m_pipe = pipe; +} 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 } +void FifoFile::WriteLine(llvm::StringRef line) { +#ifdef _WIN32 + DWORD written; + std::string str = line.str() + "\n"; + WriteFile(m_pipe, str.data(), static_cast(str.size()), &written, NULL); + FlushFileBuffers(m_pipe); +#else + std::ofstream writer(m_path, std::ofstream::out); + writer << line.data() << 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; + + while (true) { + BOOL success = + ReadFile(m_pipe, read_buffer, sizeof(read_buffer), &bytes_read, NULL); + buffer.append(read_buffer, bytes_read); + if (success || GetLastError() != ERROR_MORE_DATA) + break; + } + + return buffer; +#else + std::ifstream reader(m_path, std::ifstream::in); + std::string buffer; + std::getline(reader, buffer); + return buffer; +#endif +} + Expected> 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_MESSAGE | PIPE_READMODE_MESSAGE | 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(path, pipe_handle); #else if (int err = mkfifo(path.data(), 0600)) return createStringError(std::error_code(err, std::generic_category()), @@ -43,8 +118,10 @@ Expected> CreateFifoFile(StringRef path) { #endif } -FifoFileIO::FifoFileIO(StringRef fifo_file, StringRef other_endpoint_name) - : m_fifo_file(fifo_file), m_other_endpoint_name(other_endpoint_name) {} +FifoFileIO::FifoFileIO(std::shared_ptr fifo_file, + StringRef other_endpoint_name) + : m_fifo_file(std::move(fifo_file)), + m_other_endpoint_name(other_endpoint_name) {} Expected FifoFileIO::ReadJSON(std::chrono::milliseconds timeout) { // We use a pointer for this future, because otherwise its normal destructor @@ -52,9 +129,7 @@ Expected FifoFileIO::ReadJSON(std::chrono::milliseconds timeout) { std::optional line; std::future *future = new std::future(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 +153,7 @@ Error FifoFileIO::SendJSON(const json::Value &json, bool done = false; std::future *future = new std::future(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) { @@ -98,4 +172,4 @@ Error FifoFileIO::SendJSON(const json::Value &json, return Error::success(); } -} // namespace lldb_dap +} // namespace lldb_dap \ No newline at end of file diff --git a/lldb/tools/lldb-dap/FifoFiles.h b/lldb/tools/lldb-dap/FifoFiles.h index 633ebeb2aedd4..96e6b7f640e61 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/Pipe.h" #include "llvm/Support/Error.h" #include "llvm/Support/JSON.h" @@ -20,11 +21,27 @@ 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(llvm::StringRef line); + + 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; }; /// Create a fifo file in the filesystem. @@ -45,7 +62,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(llvm::StringRef fifo_file, llvm::StringRef other_endpoint_name); + FifoFileIO(std::shared_ptr fifo_file, + llvm::StringRef other_endpoint_name); /// Read the next JSON object from the underlying input fifo file. /// @@ -76,7 +94,7 @@ class FifoFileIO { std::chrono::milliseconds timeout = std::chrono::milliseconds(20000)); private: - std::string m_fifo_file; + std::shared_ptr 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 f58c97ae1363d..a67c7e0c3da7b 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp @@ -69,9 +69,9 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) { CreateRunInTerminalCommFile(); if (!comm_file_or_err) return comm_file_or_err.takeError(); - FifoFile &comm_file = *comm_file_or_err.get(); + std::shared_ptr comm_file = *comm_file_or_err; - RunInTerminalDebugAdapterCommChannel comm_channel(comm_file.m_path); + RunInTerminalDebugAdapterCommChannel comm_channel(comm_file); lldb::pid_t debugger_pid = LLDB_INVALID_PROCESS_ID; #if !defined(_WIN32) @@ -80,11 +80,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.cwd, comm_file->GetPath(), debugger_pid, arguments.console == protocol::eConsoleExternalTerminal); dap.SendReverseRequest("runInTerminal", std::move(reverse_request)); - + // We need to wait for the client to connect to the pipe. + comm_file->Connect(); if (llvm::Expected pid = comm_channel.GetLauncherPid()) attach_info.SetProcessID(*pid); else @@ -104,16 +105,20 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) { std::future did_attach_message_success = comm_channel.NotifyDidAttach(); - // We just attached to the runInTerminal launcher, which was waiting to be - // attached. We now resume it, so it can receive the didAttach notification - // and then perform the exec. Upon continuing, the debugger will stop the - // process right in the middle of the exec. To the user, what we are doing is - // transparent, as they will only be able to see the process since the exec, - // completely unaware of the preparatory work. +// We just attached to the runInTerminal launcher, which was waiting to be +// attached. We now resume it, so it can receive the didAttach notification +// and then perform the exec. Upon continuing, the debugger will stop the +// process right in the middle of the exec. To the user, what we are doing is +// transparent, as they will only be able to see the process since the exec, +// completely unaware of the preparatory work. +// +// On Windows, the debuggee itself is waiting to be attached to. There is no +// need to continue. +#ifndef _WIN32 dap.target.GetProcess().Continue(); +#endif - // Now that the actual target is just starting (i.e. exec was just invoked), - // we return the debugger to its sync state. + // Return the debugger to its prior async state. scope_sync_mode.reset(); // If sending the notification failed, the launcher should be dead by now and diff --git a/lldb/tools/lldb-dap/RunInTerminal.cpp b/lldb/tools/lldb-dap/RunInTerminal.cpp index 9f309dd78221a..28be4a9904942 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 #include #include @@ -97,7 +99,7 @@ static Error ToError(const RunInTerminalMessage &message) { RunInTerminalLauncherCommChannel::RunInTerminalLauncherCommChannel( StringRef comm_file) - : m_io(comm_file, "debug adapter") {} + : m_io(std::make_shared(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(lldb::pid_t pid) { + return m_io.SendJSON(RunInTerminalMessagePid(pid).ToJSON()); } void RunInTerminalLauncherCommChannel::NotifyError(StringRef error) { @@ -123,7 +129,11 @@ void RunInTerminalLauncherCommChannel::NotifyError(StringRef error) { RunInTerminalDebugAdapterCommChannel::RunInTerminalDebugAdapterCommChannel( StringRef comm_file) - : m_io(comm_file, "runInTerminal launcher") {} + : m_io(std::make_shared(comm_file), "runInTerminal launcher") {} + +RunInTerminalDebugAdapterCommChannel::RunInTerminalDebugAdapterCommChannel( + std::shared_ptr comm_file) + : m_io(std::move(comm_file), "runInTerminal launcher") {} // Can't use \a std::future because it doesn't compile on Windows std::future @@ -159,12 +169,19 @@ std::string RunInTerminalDebugAdapterCommChannel::GetLauncherError() { Expected> 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..0bed2cf2138be 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(lldb::pid_t 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(std::shared_ptr 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 740f6b63472e1..1e57e6529e0e6 100644 --- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp @@ -65,7 +65,10 @@ #undef GetObject #include typedef int socklen_t; +#include "lldb/Host/windows/ProcessLauncherWindows.h" #include "lldb/Host/windows/PythonPathSetup/PythonPathSetup.h" +#include "llvm/Support/ConvertUTF.h" +#include "llvm/Support/Program.h" #else #include #include @@ -144,6 +147,22 @@ static void PrintVersion() { llvm::outs() << "liblldb: " << lldb::SBDebugger::GetVersionString() << '\n'; } +llvm::Error +notifyError(RunInTerminalLauncherCommChannel &comm_channel, std::string message, + std::optional 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 --launch-target is provided, this instance of lldb-dap becomes a // runInTerminal launcher. It will ultimately launch the program specified in // the --launch-target argument, which is the original program the user wanted @@ -163,14 +182,14 @@ static void PrintVersion() { // // 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, - char *argv[]) { -#if defined(_WIN32) - return llvm::createStringError( - "runInTerminal is only supported on POSIX systems"); -#else +static llvm::Expected LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, + llvm::StringRef comm_file, + lldb::pid_t debugger_pid, + char *argv[], int argc) { + // 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 @@ -180,6 +199,91 @@ static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, (void)prctl(PR_SET_PTRACER, debugger_pid, 0, 0, 0); #endif +#ifdef _WIN32 + RunInTerminalLauncherCommChannel comm_channel(comm_file); + + llvm::ArrayRef args_arr = llvm::ArrayRef(argv, argc); + auto wcommand_line_or_err = + lldb_private::GetFlattenedWindowsCommandStringW(args_arr); + if (!wcommand_line_or_err) + 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); + + auto attributelist_or_err = + lldb_private::ProcThreadAttributeList::Create(startupinfoex); + if (!attributelist_or_err) { + return notifyError(comm_channel, "Could not open inherited handles", + attributelist_or_err.getError()); + } + + 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 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=*/wcommand_line_or_err->data(), + /*lpProcessAttributes=*/NULL, /*lpThreadAttributes=*/NULL, + /*bInheritHandles=*/!inherited_handles.empty(), + /*dwCreationFlags=*/CREATE_SUSPENDED, /*lpEnvironment=*/NULL, + /*lpCurrentDirectory=*/NULL, + /*lpStartupInfo=*/reinterpret_cast(&startupinfoex), + /*lpProcessInformation=*/&pi); + + if (!result) + return notifyError(comm_channel, "Failed to launch target process"); + + auto cleanup_and_return = [&](llvm::Error err) -> llvm::Expected { + 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 cleanup_and_return(std::move(err)); + + if (llvm::Error err = comm_channel.WaitUntilDebugAdapterAttaches( + std::chrono::milliseconds(timeout_in_ms))) + return cleanup_and_return(std::move(err)); + + // The debugger attached to the process. We can resume it. + if (ResumeThread(pi.hThread) == (DWORD)-1) + return cleanup_and_return( + notifyError(comm_channel, "Failed to resume the target process")); + + // Wait for child to complete to match POSIX behavior. + WaitForSingleObject(pi.hProcess, INFINITE); + DWORD code = 0; + if (!::GetExitCodeProcess(pi.hProcess, &code)) + return cleanup_and_return(notifyError( + comm_channel, "Failed to get the target's process return code")); + + CloseHandle(pi.hThread); + CloseHandle(pi.hProcess); + return code; +#else RunInTerminalLauncherCommChannel comm_channel(comm_file); if (llvm::Error err = comm_channel.NotifyPid()) return err; @@ -187,10 +291,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; @@ -199,10 +299,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 } @@ -445,12 +542,14 @@ int main(int argc, char *argv[]) { break; } } - if (llvm::Error err = - LaunchRunInTerminalTarget(*target_arg, comm_file->getValue(), pid, - argv + target_args_pos)) { - llvm::errs() << llvm::toString(std::move(err)) << '\n'; + auto return_code_or_err = LaunchRunInTerminalTarget( + *target_arg, comm_file->getValue(), pid, argv + target_args_pos, + argc - target_args_pos); + if (!return_code_or_err) { + llvm::errs() << llvm::toString(return_code_or_err.takeError()) << '\n'; return EXIT_FAILURE; } + return *return_code_or_err; } else { llvm::errs() << "\"--launch-target\" requires \"--comm-file\" to be " "specified\n"; diff --git a/lldb/unittests/DAP/FifoFilesTest.cpp b/lldb/unittests/DAP/FifoFilesTest.cpp index bbc1b608e91bd..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(fifo_path, "writer"); - FifoFileIO reader(fifo_path, "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(fifo_path, "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(fifo_path, "writer"); + FifoFileIO writer(*fifo, "writer"); // No reader, should timeout. llvm::json::Object obj;