https://github.com/SuibianP updated https://github.com/llvm/llvm-project/pull/142374
>From 098fb7f1cf576d8bcdfd88724a4c9fdf7837c68d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jialun=20Hu=20=EF=BC=88=E8=83=A1=E4=BD=B3=E4=BC=A6?= =?UTF-8?q?=EF=BC=89?= <jialun...@razer.com> Date: Mon, 2 Jun 2025 16:16:11 +0800 Subject: [PATCH] [lldb-dap] Reimplement runInTerminal with signals runInTerminal is currently implemented using JSON messages over FIFOs, which feels too clunky for the simple job of passing a PID. Instead, take advantage of si_pid available from sa_sigaction handler, and send a user signal instead. Both synchronisation and timeout are preserved with the protocol below, 1. Debugger waits for SIGUSR1 under timeout with pselect 2. Launcher forks into child, sends SIGUSR1 to debugger and suspends 3. Debugger receives SIGUSR1, captures the sender (launcher child) PID, attaches to and resumes it 4. Launcher child resumes and exec's into target 5. Launcher parent waitpid's and kills irresponsive child after timeout With this, the entirety of FifoFiles and RunInTerminal can be dropped. Refs: https://github.com/llvm/llvm-project/pull/121269#issuecomment-2901401482 --- .../runInTerminal/TestDAP_runInTerminal.py | 90 +--------- lldb/tools/lldb-dap/CMakeLists.txt | 2 - lldb/tools/lldb-dap/FifoFiles.cpp | 101 ----------- lldb/tools/lldb-dap/FifoFiles.h | 85 --------- .../tools/lldb-dap/Handler/RequestHandler.cpp | 99 +++++++--- lldb/tools/lldb-dap/JSONUtils.cpp | 5 +- lldb/tools/lldb-dap/JSONUtils.h | 5 +- lldb/tools/lldb-dap/Options.td | 15 +- lldb/tools/lldb-dap/RunInTerminal.cpp | 170 ------------------ lldb/tools/lldb-dap/RunInTerminal.h | 131 -------------- lldb/tools/lldb-dap/tool/lldb-dap.cpp | 104 ++++++----- lldb/unittests/DAP/CMakeLists.txt | 1 - lldb/unittests/DAP/FifoFilesTest.cpp | 102 ----------- 13 files changed, 147 insertions(+), 763 deletions(-) delete mode 100644 lldb/tools/lldb-dap/FifoFiles.cpp delete mode 100644 lldb/tools/lldb-dap/FifoFiles.h delete mode 100644 lldb/tools/lldb-dap/RunInTerminal.cpp delete mode 100644 lldb/tools/lldb-dap/RunInTerminal.h delete mode 100644 lldb/unittests/DAP/FifoFilesTest.cpp 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 65c931210d400..0c8203f46a155 100644 --- a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py +++ b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py @@ -14,6 +14,7 @@ import shutil import json from threading import Thread +import signal class TestDAP_runInTerminal(lldbdap_testcase.DAPTestCaseBase): @@ -30,6 +31,7 @@ def readErrorMessage(self, fifo_file): return file.readline() def isTestSupported(self): + return True # For some strange reason, this test fails on python3.6 if not (sys.version_info.major == 3 and sys.version_info.minor >= 7): return False @@ -144,97 +146,21 @@ def test_missingArgInRunInTerminalLauncher(self): ) self.assertNotEqual(proc.returncode, 0) self.assertIn( - '"--launch-target" requires "--comm-file" to be specified', proc.stderr + '"--launch-target" requires "--debugger-pid" to be specified', proc.stderr ) - @skipIfWindows - @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) - def test_FakeAttachedRunInTerminalLauncherWithInvalidProgram(self): - if not self.isTestSupported(): - return - 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.readPidMessage(comm_file) - self.sendDidAttachMessage(comm_file) - self.assertIn("No such file or directory", self.readErrorMessage(comm_file)) - - _, stderr = proc.communicate() - self.assertIn("No such file or directory", stderr) - - @skipIfWindows - @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) - def test_FakeAttachedRunInTerminalLauncherWithValidProgram(self): - if not self.isTestSupported(): - return - 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.readPidMessage(comm_file) - self.sendDidAttachMessage(comm_file) - - stdout, _ = proc.communicate() - self.assertIn("foo", stdout) - - @skipIfWindows - @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) - def test_FakeAttachedRunInTerminalLauncherAndCheckEnvironment(self): - if not self.isTestSupported(): - return - 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.readPidMessage(comm_file) - self.sendDidAttachMessage(comm_file) - - stdout, _ = proc.communicate() - self.assertIn("FOO=BAR", stdout) - @skipIfWindows @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) def test_NonAttachedRunInTerminalLauncher(self): if not self.isTestSupported(): return - comm_file = os.path.join(self.getBuildDir(), "comm-file") - os.mkfifo(comm_file) + signal.signal(signal.SIGUSR1, signal.SIG_IGN) proc = subprocess.Popen( [ self.lldbDAPExec, - "--comm-file", - comm_file, + "--debugger-pid", + str(os.getpid()), "--launch-target", "echo", "foo", @@ -244,7 +170,5 @@ def test_NonAttachedRunInTerminalLauncher(self): env={**os.environ, "LLDB_DAP_RIT_TIMEOUT_IN_MS": "1000"}, ) - self.readPidMessage(comm_file) - _, stderr = proc.communicate() - self.assertIn("Timed out trying to get messages from the debug adapter", stderr) + self.assertIn("runInTerminal target did not resume in time", stderr) diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt index 40cba16c141f0..3e7e88afa9ced 100644 --- a/lldb/tools/lldb-dap/CMakeLists.txt +++ b/lldb/tools/lldb-dap/CMakeLists.txt @@ -14,7 +14,6 @@ add_lldb_library(lldbDAP DAPLog.cpp EventHelper.cpp ExceptionBreakpoint.cpp - FifoFiles.cpp FunctionBreakpoint.cpp InstructionBreakpoint.cpp JSONUtils.cpp @@ -22,7 +21,6 @@ add_lldb_library(lldbDAP OutputRedirector.cpp ProgressEvent.cpp ProtocolUtils.cpp - RunInTerminal.cpp SourceBreakpoint.cpp Transport.cpp Variables.cpp diff --git a/lldb/tools/lldb-dap/FifoFiles.cpp b/lldb/tools/lldb-dap/FifoFiles.cpp deleted file mode 100644 index 1f1bba80bd3b1..0000000000000 --- a/lldb/tools/lldb-dap/FifoFiles.cpp +++ /dev/null @@ -1,101 +0,0 @@ -//===-- FifoFiles.cpp -------------------------------------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "FifoFiles.h" -#include "JSONUtils.h" - -#if !defined(_WIN32) -#include <sys/stat.h> -#include <sys/types.h> -#include <unistd.h> -#endif - -#include <chrono> -#include <fstream> -#include <future> -#include <optional> - -using namespace llvm; - -namespace lldb_dap { - -FifoFile::FifoFile(StringRef path) : m_path(path) {} - -FifoFile::~FifoFile() { -#if !defined(_WIN32) - unlink(m_path.c_str()); -#endif -} - -Expected<std::shared_ptr<FifoFile>> CreateFifoFile(StringRef path) { -#if defined(_WIN32) - return createStringError(inconvertibleErrorCode(), "Unimplemented"); -#else - if (int err = mkfifo(path.data(), 0600)) - return createStringError(std::error_code(err, std::generic_category()), - "Couldn't create fifo file: %s", path.data()); - return std::make_shared<FifoFile>(path); -#endif -} - -FifoFileIO::FifoFileIO(StringRef 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) { - // We use a pointer for this future, because otherwise its normal destructor - // would wait for the getline to end, rendering the timeout useless. - std::optional<std::string> line; - std::future<void> *future = - new std::future<void>(std::async(std::launch::async, [&]() { - std::ifstream reader(m_fifo_file, std::ifstream::in); - std::string buffer; - std::getline(reader, buffer); - if (!buffer.empty()) - line = buffer; - })); - if (future->wait_for(timeout) == std::future_status::timeout || !line) - // Indeed this is a leak, but it's intentional. "future" obj destructor - // will block on waiting for the worker thread to join. And the worker - // thread might be stuck in blocking I/O. Intentionally leaking the obj - // as a hack to avoid blocking main thread, and adding annotation to - // supress static code inspection warnings - - // coverity[leaked_storage] - return createStringError(inconvertibleErrorCode(), - "Timed out trying to get messages from the " + - m_other_endpoint_name); - delete future; - return json::parse(*line); -} - -Error FifoFileIO::SendJSON(const json::Value &json, - std::chrono::milliseconds timeout) { - 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; - done = true; - })); - if (future->wait_for(timeout) == std::future_status::timeout || !done) { - // Indeed this is a leak, but it's intentional. "future" obj destructor will - // block on waiting for the worker thread to join. And the worker thread - // might be stuck in blocking I/O. Intentionally leaking the obj as a hack - // to avoid blocking main thread, and adding annotation to supress static - // code inspection warnings" - - // coverity[leaked_storage] - return createStringError(inconvertibleErrorCode(), - "Timed out trying to send messages to the " + - m_other_endpoint_name); - } - delete future; - return Error::success(); -} - -} // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/FifoFiles.h b/lldb/tools/lldb-dap/FifoFiles.h deleted file mode 100644 index 633ebeb2aedd4..0000000000000 --- a/lldb/tools/lldb-dap/FifoFiles.h +++ /dev/null @@ -1,85 +0,0 @@ -//===-- FifoFiles.h ---------------------------------------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#ifndef LLDB_TOOLS_LLDB_DAP_FIFOFILES_H -#define LLDB_TOOLS_LLDB_DAP_FIFOFILES_H - -#include "llvm/Support/Error.h" -#include "llvm/Support/JSON.h" - -#include <chrono> - -namespace lldb_dap { - -/// Struct that controls the life of a fifo file in the filesystem. -/// -/// The file is destroyed when the destructor is invoked. -struct FifoFile { - FifoFile(llvm::StringRef path); - - ~FifoFile(); - - std::string m_path; -}; - -/// Create a fifo file in the filesystem. -/// -/// \param[in] path -/// The path for the fifo file. -/// -/// \return -/// A \a std::shared_ptr<FifoFile> if the file could be created, or an -/// \a llvm::Error in case of failures. -llvm::Expected<std::shared_ptr<FifoFile>> CreateFifoFile(llvm::StringRef path); - -class FifoFileIO { -public: - /// \param[in] fifo_file - /// The path to an input fifo file that exists in the file system. - /// - /// \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); - - /// Read the next JSON object from the underlying input fifo file. - /// - /// The JSON object is expected to be a single line delimited with \a - /// std::endl. - /// - /// \return - /// An \a llvm::Error object indicating the success or failure of this - /// operation. Failures arise if the timeout is hit, the next line of text - /// from the fifo file is not a valid JSON object, or is it impossible to - /// read from the file. - llvm::Expected<llvm::json::Value> ReadJSON(std::chrono::milliseconds timeout); - - /// Serialize a JSON object and write it to the underlying output fifo file. - /// - /// \param[in] json - /// The JSON object to send. It will be printed as a single line delimited - /// with \a std::endl. - /// - /// \param[in] timeout - /// A timeout for how long we should until for the data to be consumed. - /// - /// \return - /// An \a llvm::Error object indicating whether the data was consumed by - /// a reader or not. - llvm::Error SendJSON( - const llvm::json::Value &json, - std::chrono::milliseconds timeout = std::chrono::milliseconds(20000)); - -private: - std::string m_fifo_file; - std::string m_other_endpoint_name; -}; - -} // namespace lldb_dap - -#endif // LLDB_TOOLS_LLDB_DAP_FIFOFILES_H diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp index 93bc80a38e29d..cfcbabbb16a93 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp @@ -14,10 +14,12 @@ #include "LLDBUtils.h" #include "Protocol/ProtocolBase.h" #include "Protocol/ProtocolRequests.h" -#include "RunInTerminal.h" #include "lldb/API/SBDefines.h" #include "lldb/API/SBEnvironment.h" #include "llvm/Support/Error.h" +#include <cerrno> +#include <csignal> +#include <limits> #include <mutex> #if !defined(_WIN32) @@ -51,6 +53,70 @@ static uint32_t SetLaunchFlag(uint32_t flags, bool flag, return flags; } +// Assume single thread +class PidReceiver { +private: + inline static volatile std::sig_atomic_t pid; + sigset_t oldmask; + struct sigaction oldaction; + + static_assert(std::numeric_limits<volatile sig_atomic_t>::max() >= + std::numeric_limits<pid_t>::max(), + "sig_atomic_t must be able to hold a pid_t value"); + + static void sig_handler(int, siginfo_t *info, void *) { + // Store the PID from the signal info + pid = info->si_pid; + } + + PidReceiver(const PidReceiver &) = delete; + PidReceiver &operator=(const PidReceiver &) = delete; + PidReceiver(PidReceiver &&) = delete; + +public: + PidReceiver() { + pid = LLDB_INVALID_PROCESS_ID; + // sigprocmask and sigaction can only fail by programmer error + // Defer SIGUSR1, otherwise we might receive it out of pselect and hang + sigset_t mask; + sigemptyset(&mask); + sigaddset(&mask, SIGUSR1); + assert(!sigprocmask(SIG_BLOCK, &mask, &oldmask)); + + // Set up sigaction for SIGUSR1 with SA_SIGINFO + struct sigaction sa; + std::memset(&sa, 0, sizeof(sa)); + sa.sa_sigaction = sig_handler; + sa.sa_flags = SA_SIGINFO; + sigemptyset(&sa.sa_mask); + assert(!sigaction(SIGUSR1, &sa, &oldaction)); + } + + ~PidReceiver() { + // Restore the old signal mask + assert(!sigprocmask(SIG_SETMASK, &oldmask, nullptr)); + assert(!sigaction(SIGUSR1, &oldaction, nullptr)); + } + + llvm::Expected<lldb::pid_t> WaitForPid() { + // Wait for the signal to be received + while (pid == LLDB_INVALID_PROCESS_ID) { + struct timespec timeout; + timeout.tv_sec = 5; + timeout.tv_nsec = 0; + auto ret = pselect(0, nullptr, nullptr, nullptr, &timeout, &oldmask); + if (ret == -1 && errno != EINTR) { + return llvm::createStringError( + std::error_code(errno, std::generic_category()), "pselect failed"); + } else if (ret == 0) { + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Timed out waiting for signal"); + } + } + return pid; + } +}; + static llvm::Error RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) { if (!dap.clientFeatures.contains( @@ -65,26 +131,19 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) { dap.is_attach = true; lldb::SBAttachInfo attach_info; - llvm::Expected<std::shared_ptr<FifoFile>> comm_file_or_err = - CreateRunInTerminalCommFile(); - if (!comm_file_or_err) - return comm_file_or_err.takeError(); - FifoFile &comm_file = *comm_file_or_err.get(); - - RunInTerminalDebugAdapterCommChannel comm_channel(comm_file.m_path); - lldb::pid_t debugger_pid = LLDB_INVALID_PROCESS_ID; #if !defined(_WIN32) debugger_pid = getpid(); #endif + auto pid_receiver = PidReceiver(); llvm::json::Object reverse_request = CreateRunInTerminalReverseRequest( arguments.configuration.program, arguments.args, arguments.env, - arguments.cwd, comm_file.m_path, debugger_pid); + arguments.cwd, debugger_pid); dap.SendReverseRequest<LogFailureResponseHandler>("runInTerminal", std::move(reverse_request)); - if (llvm::Expected<lldb::pid_t> pid = comm_channel.GetLauncherPid()) + if (llvm::Expected<lldb::pid_t> pid = pid_receiver.WaitForPid()) attach_info.SetProcessID(*pid); else return pid.takeError(); @@ -95,13 +154,7 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) { if (error.Fail()) return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Failed to attach to the target process. %s", - comm_channel.GetLauncherError().c_str()); - // This will notify the runInTerminal launcher that we attached. - // We have to make this async, as the function won't return until the launcher - // resumes and reads the data. - std::future<lldb::SBError> did_attach_message_success = - comm_channel.NotifyDidAttach(); + "Failed to attach to the target process."); // We just attached to the runInTerminal launcher, which was waiting to be // attached. We now resume it, so it can receive the didAttach notification @@ -115,15 +168,7 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) { // we return the debugger to its sync state. scope_sync_mode.reset(); - // If sending the notification failed, the launcher should be dead by now and - // the async didAttach notification should have an error message, so we - // return it. Otherwise, everything was a success. - did_attach_message_success.wait(); - error = did_attach_message_success.get(); - if (error.Success()) - return llvm::Error::success(); - return llvm::createStringError(llvm::inconvertibleErrorCode(), - error.GetCString()); + return llvm::Error::success(); } void BaseRequestHandler::Run(const Request &request) { diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 573f3eba00f62..bf10742e18bb8 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -1238,15 +1238,14 @@ llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit) { 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) { + lldb::pid_t debugger_pid) { llvm::json::Object run_in_terminal_args; // This indicates the IDE to open an embedded terminal, instead of opening // the terminal in a new window. run_in_terminal_args.try_emplace("kind", "integrated"); // The program path must be the first entry in the "args" field - std::vector<std::string> req_args = {DAP::debug_adapter_path.str(), - "--comm-file", comm_file.str()}; + std::vector<std::string> req_args = {DAP::debug_adapter_path.str()}; if (debugger_pid != LLDB_INVALID_PROCESS_ID) { req_args.push_back("--debugger-pid"); req_args.push_back(std::to_string(debugger_pid)); diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h index 08699a94bbd87..4ed66e6992be2 100644 --- a/lldb/tools/lldb-dap/JSONUtils.h +++ b/lldb/tools/lldb-dap/JSONUtils.h @@ -466,9 +466,6 @@ llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit); /// \param[in] cwd /// The working directory for the run in terminal request. /// -/// \param[in] comm_file -/// The fifo file used to communicate the with the target launcher. -/// /// \param[in] debugger_pid /// The PID of the lldb-dap instance that will attach to the target. The /// launcher uses it on Linux tell the kernel that it should allow the @@ -480,7 +477,7 @@ llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit); 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); + lldb::pid_t debugger_pid); /// Create a "Terminated" JSON object that contains statistics /// diff --git a/lldb/tools/lldb-dap/Options.td b/lldb/tools/lldb-dap/Options.td index aecf91797ac70..669d6a7c0df05 100644 --- a/lldb/tools/lldb-dap/Options.td +++ b/lldb/tools/lldb-dap/Options.td @@ -31,16 +31,11 @@ def connection "Connections are specified like 'tcp://[host]:port' or " "'unix:///path'.">; -def launch_target: S<"launch-target">, - MetaVarName<"<target>">, - HelpText<"Launch a target for the launchInTerminal request. Any argument " - "provided after this one will be passed to the target. The parameter " - "--comm-file must also be specified.">; - -def comm_file: S<"comm-file">, - MetaVarName<"<file>">, - HelpText<"The fifo file used to communicate the with the debug adapter " - "when using --launch-target.">; +def launch_target + : S<"launch-target">, + MetaVarName<"<target>">, + HelpText<"Launch a target for the launchInTerminal request. Any argument " + "provided after this one will be passed to the target.">; def debugger_pid: S<"debugger-pid">, MetaVarName<"<pid>">, diff --git a/lldb/tools/lldb-dap/RunInTerminal.cpp b/lldb/tools/lldb-dap/RunInTerminal.cpp deleted file mode 100644 index 9f309dd78221a..0000000000000 --- a/lldb/tools/lldb-dap/RunInTerminal.cpp +++ /dev/null @@ -1,170 +0,0 @@ -//===-- RunInTerminal.cpp ---------------------------------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "RunInTerminal.h" -#include "JSONUtils.h" - -#if !defined(_WIN32) -#include <sys/stat.h> -#include <sys/types.h> -#include <unistd.h> -#endif - -#include <chrono> -#include <future> - -#include "llvm/Support/FileSystem.h" - -using namespace llvm; - -namespace lldb_dap { - -const RunInTerminalMessagePid *RunInTerminalMessage::GetAsPidMessage() const { - return static_cast<const RunInTerminalMessagePid *>(this); -} - -const RunInTerminalMessageError * -RunInTerminalMessage::GetAsErrorMessage() const { - return static_cast<const RunInTerminalMessageError *>(this); -} - -RunInTerminalMessage::RunInTerminalMessage(RunInTerminalMessageKind kind) - : kind(kind) {} - -RunInTerminalMessagePid::RunInTerminalMessagePid(lldb::pid_t pid) - : RunInTerminalMessage(eRunInTerminalMessageKindPID), pid(pid) {} - -json::Value RunInTerminalMessagePid::ToJSON() const { - return json::Object{{"kind", "pid"}, {"pid", static_cast<int64_t>(pid)}}; -} - -RunInTerminalMessageError::RunInTerminalMessageError(StringRef error) - : RunInTerminalMessage(eRunInTerminalMessageKindError), error(error) {} - -json::Value RunInTerminalMessageError::ToJSON() const { - return json::Object{{"kind", "error"}, {"value", error}}; -} - -RunInTerminalMessageDidAttach::RunInTerminalMessageDidAttach() - : RunInTerminalMessage(eRunInTerminalMessageKindDidAttach) {} - -json::Value RunInTerminalMessageDidAttach::ToJSON() const { - return json::Object{{"kind", "didAttach"}}; -} - -static Expected<RunInTerminalMessageUP> -ParseJSONMessage(const json::Value &json) { - if (const json::Object *obj = json.getAsObject()) { - if (std::optional<StringRef> kind = obj->getString("kind")) { - if (*kind == "pid") { - if (std::optional<int64_t> pid = obj->getInteger("pid")) - return std::make_unique<RunInTerminalMessagePid>( - static_cast<lldb::pid_t>(*pid)); - } else if (*kind == "error") { - if (std::optional<StringRef> error = obj->getString("error")) - return std::make_unique<RunInTerminalMessageError>(*error); - } else if (*kind == "didAttach") { - return std::make_unique<RunInTerminalMessageDidAttach>(); - } - } - } - - return createStringError(inconvertibleErrorCode(), - "Incorrect JSON message: " + JSONToString(json)); -} - -static Expected<RunInTerminalMessageUP> -GetNextMessage(FifoFileIO &io, std::chrono::milliseconds timeout) { - if (Expected<json::Value> json = io.ReadJSON(timeout)) - return ParseJSONMessage(*json); - else - return json.takeError(); -} - -static Error ToError(const RunInTerminalMessage &message) { - if (message.kind == eRunInTerminalMessageKindError) - return createStringError(inconvertibleErrorCode(), - message.GetAsErrorMessage()->error); - return createStringError(inconvertibleErrorCode(), - "Unexpected JSON message: " + - JSONToString(message.ToJSON())); -} - -RunInTerminalLauncherCommChannel::RunInTerminalLauncherCommChannel( - StringRef comm_file) - : m_io(comm_file, "debug adapter") {} - -Error RunInTerminalLauncherCommChannel::WaitUntilDebugAdapterAttaches( - std::chrono::milliseconds timeout) { - if (Expected<RunInTerminalMessageUP> message = - GetNextMessage(m_io, timeout)) { - if (message.get()->kind == eRunInTerminalMessageKindDidAttach) - return Error::success(); - else - return ToError(*message.get()); - } else - return message.takeError(); -} - -Error RunInTerminalLauncherCommChannel::NotifyPid() { - return m_io.SendJSON(RunInTerminalMessagePid(getpid()).ToJSON()); -} - -void RunInTerminalLauncherCommChannel::NotifyError(StringRef error) { - if (Error err = m_io.SendJSON(RunInTerminalMessageError(error).ToJSON(), - std::chrono::seconds(2))) - llvm::errs() << llvm::toString(std::move(err)) << "\n"; -} - -RunInTerminalDebugAdapterCommChannel::RunInTerminalDebugAdapterCommChannel( - StringRef 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() { - return std::async(std::launch::async, [&]() { - lldb::SBError error; - if (llvm::Error err = - m_io.SendJSON(RunInTerminalMessageDidAttach().ToJSON())) - error.SetErrorString(llvm::toString(std::move(err)).c_str()); - return error; - }); -} - -Expected<lldb::pid_t> RunInTerminalDebugAdapterCommChannel::GetLauncherPid() { - if (Expected<RunInTerminalMessageUP> message = - GetNextMessage(m_io, std::chrono::seconds(20))) { - if (message.get()->kind == eRunInTerminalMessageKindPID) - return message.get()->GetAsPidMessage()->pid; - return ToError(*message.get()); - } else { - return message.takeError(); - } -} - -std::string RunInTerminalDebugAdapterCommChannel::GetLauncherError() { - // We know there's been an error, so a small timeout is enough. - if (Expected<RunInTerminalMessageUP> message = - GetNextMessage(m_io, std::chrono::seconds(1))) - return toString(ToError(*message.get())); - else - return toString(message.takeError()); -} - -Expected<std::shared_ptr<FifoFile>> CreateRunInTerminalCommFile() { - SmallString<256> comm_file; - if (std::error_code EC = sys::fs::getPotentiallyUniqueTempFileName( - "lldb-dap-run-in-terminal-comm", "", comm_file)) - return createStringError(EC, "Error making unique file name for " - "runInTerminal communication files"); - - return CreateFifoFile(comm_file.str()); -} - -} // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/RunInTerminal.h b/lldb/tools/lldb-dap/RunInTerminal.h deleted file mode 100644 index 457850c8ea538..0000000000000 --- a/lldb/tools/lldb-dap/RunInTerminal.h +++ /dev/null @@ -1,131 +0,0 @@ -//===-- RunInTerminal.h ----------------------------------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#ifndef LLDB_TOOLS_LLDB_DAP_RUNINTERMINAL_H -#define LLDB_TOOLS_LLDB_DAP_RUNINTERMINAL_H - -#include "FifoFiles.h" -#include "lldb/API/SBError.h" - -#include <future> -#include <memory> -#include <string> - -namespace lldb_dap { - -enum RunInTerminalMessageKind { - eRunInTerminalMessageKindPID = 0, - eRunInTerminalMessageKindError, - eRunInTerminalMessageKindDidAttach, -}; - -struct RunInTerminalMessage; -struct RunInTerminalMessagePid; -struct RunInTerminalMessageError; -struct RunInTerminalMessageDidAttach; - -struct RunInTerminalMessage { - RunInTerminalMessage(RunInTerminalMessageKind kind); - - virtual ~RunInTerminalMessage() = default; - - /// Serialize this object to JSON - virtual llvm::json::Value ToJSON() const = 0; - - const RunInTerminalMessagePid *GetAsPidMessage() const; - - const RunInTerminalMessageError *GetAsErrorMessage() const; - - RunInTerminalMessageKind kind; -}; - -using RunInTerminalMessageUP = std::unique_ptr<RunInTerminalMessage>; - -struct RunInTerminalMessagePid : RunInTerminalMessage { - RunInTerminalMessagePid(lldb::pid_t pid); - - llvm::json::Value ToJSON() const override; - - lldb::pid_t pid; -}; - -struct RunInTerminalMessageError : RunInTerminalMessage { - RunInTerminalMessageError(llvm::StringRef error); - - llvm::json::Value ToJSON() const override; - - std::string error; -}; - -struct RunInTerminalMessageDidAttach : RunInTerminalMessage { - RunInTerminalMessageDidAttach(); - - llvm::json::Value ToJSON() const override; -}; - -class RunInTerminalLauncherCommChannel { -public: - RunInTerminalLauncherCommChannel(llvm::StringRef comm_file); - - /// Wait until the debug adapter attaches. - /// - /// \param[in] timeout - /// How long to wait to be attached. - // - /// \return - /// An \a llvm::Error object in case of errors or if this operation times - /// out. - llvm::Error WaitUntilDebugAdapterAttaches(std::chrono::milliseconds timeout); - - /// Notify the debug adapter this process' pid. - /// - /// \return - /// An \a llvm::Error object in case of errors or if this operation times - /// out. - llvm::Error NotifyPid(); - - /// Notify the debug adapter that there's been an error. - void NotifyError(llvm::StringRef error); - -private: - FifoFileIO m_io; -}; - -class RunInTerminalDebugAdapterCommChannel { -public: - RunInTerminalDebugAdapterCommChannel(llvm::StringRef comm_file); - - /// Notify the runInTerminal launcher that it was attached. - /// - /// \return - /// A future indicated whether the runInTerminal launcher received the - /// message correctly or not. - std::future<lldb::SBError> NotifyDidAttach(); - - /// Fetch the pid of the runInTerminal launcher. - /// - /// \return - /// An \a llvm::Error object in case of errors or if this operation times - /// out. - llvm::Expected<lldb::pid_t> GetLauncherPid(); - - /// Fetch any errors emitted by the runInTerminal launcher or return a - /// default error message if a certain timeout if reached. - std::string GetLauncherError(); - -private: - FifoFileIO m_io; -}; - -/// Create a fifo file used to communicate the debug adapter with -/// the runInTerminal launcher. -llvm::Expected<std::shared_ptr<FifoFile>> CreateRunInTerminalCommFile(); - -} // namespace lldb_dap - -#endif // LLDB_TOOLS_LLDB_DAP_RUNINTERMINAL_H diff --git a/lldb/tools/lldb-dap/tool/lldb-dap.cpp b/lldb/tools/lldb-dap/tool/lldb-dap.cpp index 9b9de5e21a742..a18ffdd7ced76 100644 --- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp @@ -10,7 +10,6 @@ #include "DAPLog.h" #include "EventHelper.h" #include "Handler/RequestHandler.h" -#include "RunInTerminal.h" #include "Transport.h" #include "lldb/API/SBDebugger.h" #include "lldb/API/SBStream.h" @@ -67,7 +66,9 @@ typedef int socklen_t; #else #include <netinet/in.h> #include <sys/socket.h> +#include <sys/types.h> #include <sys/un.h> +#include <sys/wait.h> #include <unistd.h> #endif @@ -162,7 +163,6 @@ 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) @@ -170,37 +170,55 @@ static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, "runInTerminal is only supported on POSIX systems"); #else - // On Linux with the Yama security module enabled, a process can only attach - // to its descendants by default. In the runInTerminal case the target - // process is launched by the client so we need to allow tracing explicitly. + auto pid = fork(); + if (pid < 0) { + return llvm::createStringError( + std::error_code(errno, std::generic_category()), "fork failed"); + } + if (pid == 0) { + // On Linux with the Yama security module enabled, a process can only attach + // to its descendants by default. In the runInTerminal case the target + // process is launched by the client so we need to allow tracing explicitly. #if defined(__linux__) - if (debugger_pid != LLDB_INVALID_PROCESS_ID) - (void)prctl(PR_SET_PTRACER, debugger_pid, 0, 0, 0); + if (debugger_pid != LLDB_INVALID_PROCESS_ID) + (void)prctl(PR_SET_PTRACER, debugger_pid, 0, 0, 0); #endif - - RunInTerminalLauncherCommChannel comm_channel(comm_file); - if (llvm::Error err = comm_channel.NotifyPid()) - return err; - - // 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; + if (kill(debugger_pid, SIGUSR1)) { + return llvm::createStringError( + std::error_code(errno, std::generic_category()), + "Failed to notify debugger of target pid"); + } + if (raise(SIGSTOP)) { + return llvm::createStringError( + std::error_code(errno, std::generic_category()), + "Target process failed to stop itself"); + } + const char *target = target_arg.getValue(); + execvp(target, argv); + std::string error = std::strerror(errno); + return llvm::createStringError(llvm::inconvertibleErrorCode(), + std::move(error)); } - - 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)); + if (pid > 0) { + // 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; + std::this_thread::sleep_for(std::chrono::milliseconds(timeout_in_ms)); + // the child should have either continued or exited by now, kill it if not + auto status = waitpid(pid, nullptr, WNOHANG | WCONTINUED); + if (status < 0) { + kill(pid, SIGKILL); + return llvm::createStringError( + std::error_code(errno, std::generic_category()), "waitpid failed"); + } + if (status == 0) { + kill(pid, SIGKILL); + return llvm::createStringError("runInTerminal target did not resume in " + "time (debugger process died?)"); + } + } + return llvm::Error::success(); #endif } @@ -407,17 +425,15 @@ int main(int argc, char *argv[]) { } if (llvm::opt::Arg *target_arg = input_args.getLastArg(OPT_launch_target)) { - if (llvm::opt::Arg *comm_file = input_args.getLastArg(OPT_comm_file)) { + if (llvm::opt::Arg *debugger_pid = + input_args.getLastArg(OPT_debugger_pid)) { lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; - llvm::opt::Arg *debugger_pid = input_args.getLastArg(OPT_debugger_pid); - if (debugger_pid) { - llvm::StringRef debugger_pid_value = debugger_pid->getValue(); - if (debugger_pid_value.getAsInteger(10, pid)) { - llvm::errs() << "'" << debugger_pid_value - << "' is not a valid " - "PID\n"; - return EXIT_FAILURE; - } + llvm::StringRef debugger_pid_value = debugger_pid->getValue(); + if (debugger_pid_value.getAsInteger(10, pid)) { + llvm::errs() << "'" << debugger_pid_value + << "' is not a valid " + "PID\n"; + return EXIT_FAILURE; } int target_args_pos = argc; for (int i = 0; i < argc; i++) { @@ -426,14 +442,14 @@ int main(int argc, char *argv[]) { break; } } - if (llvm::Error err = - LaunchRunInTerminalTarget(*target_arg, comm_file->getValue(), pid, - argv + target_args_pos)) { + if (llvm::Error err = LaunchRunInTerminalTarget(*target_arg, pid, + argv + target_args_pos)) { llvm::errs() << llvm::toString(std::move(err)) << '\n'; return EXIT_FAILURE; } + return EXIT_SUCCESS; } else { - llvm::errs() << "\"--launch-target\" requires \"--comm-file\" to be " + llvm::errs() << "\"--launch-target\" requires \"--debugger-pid\" to be " "specified\n"; return EXIT_FAILURE; } diff --git a/lldb/unittests/DAP/CMakeLists.txt b/lldb/unittests/DAP/CMakeLists.txt index d8576ff3f371b..2d45e66793506 100644 --- a/lldb/unittests/DAP/CMakeLists.txt +++ b/lldb/unittests/DAP/CMakeLists.txt @@ -1,6 +1,5 @@ add_lldb_unittest(DAPTests DAPTest.cpp - FifoFilesTest.cpp Handler/DisconnectTest.cpp Handler/ContinueTest.cpp JSONUtilsTest.cpp diff --git a/lldb/unittests/DAP/FifoFilesTest.cpp b/lldb/unittests/DAP/FifoFilesTest.cpp deleted file mode 100644 index bbc1b608e91bd..0000000000000 --- a/lldb/unittests/DAP/FifoFilesTest.cpp +++ /dev/null @@ -1,102 +0,0 @@ -//===-- FifoFilesTest.cpp -------------------------------------------------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "FifoFiles.h" -#include "llvm/Support/FileSystem.h" -#include "llvm/Testing/Support/Error.h" -#include "gtest/gtest.h" -#include <chrono> -#include <thread> - -using namespace lldb_dap; -using namespace llvm; - -namespace { - -std::string MakeTempFifoPath() { - llvm::SmallString<128> temp_path; - llvm::sys::fs::createUniquePath("lldb-dap-fifo-%%%%%%", temp_path, - /*MakeAbsolute=*/true); - return temp_path.str().str(); -} - -} // namespace - -TEST(FifoFilesTest, CreateAndDestroyFifoFile) { - std::string fifo_path = MakeTempFifoPath(); - auto fifo = CreateFifoFile(fifo_path); - EXPECT_THAT_EXPECTED(fifo, llvm::Succeeded()); - - // File should exist. - EXPECT_TRUE(llvm::sys::fs::exists(fifo_path)); - - // Destructor should remove the file. - fifo->reset(); - EXPECT_FALSE(llvm::sys::fs::exists(fifo_path)); -} - -TEST(FifoFilesTest, SendAndReceiveJSON) { - std::string fifo_path = MakeTempFifoPath(); - auto fifo = CreateFifoFile(fifo_path); - EXPECT_THAT_EXPECTED(fifo, llvm::Succeeded()); - - FifoFileIO writer(fifo_path, "writer"); - FifoFileIO reader(fifo_path, "reader"); - - llvm::json::Object obj; - obj["foo"] = "bar"; - obj["num"] = 42; - - // Writer thread. - std::thread writer_thread([&]() { - EXPECT_THAT_ERROR(writer.SendJSON(llvm::json::Value(std::move(obj)), - std::chrono::milliseconds(500)), - llvm::Succeeded()); - }); - - // Reader thread. - std::thread reader_thread([&]() { - auto result = reader.ReadJSON(std::chrono::milliseconds(500)); - EXPECT_THAT_EXPECTED(result, llvm::Succeeded()); - auto *read_obj = result->getAsObject(); - - ASSERT_NE(read_obj, nullptr); - EXPECT_EQ((*read_obj)["foo"].getAsString(), "bar"); - EXPECT_EQ((*read_obj)["num"].getAsInteger(), 42); - }); - - writer_thread.join(); - reader_thread.join(); -} - -TEST(FifoFilesTest, ReadTimeout) { - std::string fifo_path = MakeTempFifoPath(); - auto fifo = CreateFifoFile(fifo_path); - EXPECT_THAT_EXPECTED(fifo, llvm::Succeeded()); - - FifoFileIO reader(fifo_path, "reader"); - - // No writer, should timeout. - auto result = reader.ReadJSON(std::chrono::milliseconds(100)); - EXPECT_THAT_EXPECTED(result, llvm::Failed()); -} - -TEST(FifoFilesTest, WriteTimeout) { - std::string fifo_path = MakeTempFifoPath(); - auto fifo = CreateFifoFile(fifo_path); - EXPECT_THAT_EXPECTED(fifo, llvm::Succeeded()); - - FifoFileIO writer(fifo_path, "writer"); - - // No reader, should timeout. - llvm::json::Object obj; - obj["foo"] = "bar"; - EXPECT_THAT_ERROR(writer.SendJSON(llvm::json::Value(std::move(obj)), - std::chrono::milliseconds(100)), - llvm::Failed()); -} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
