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 -#include -#include -#endif - -#include -#include -#include -#include - -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> 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(path); -#endif -} - -FifoFileIO::FifoFileIO(StringRef fifo_file, StringRef other_endpoint_name) - : m_fifo_file(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 - // would wait for the getline to end, rendering the timeout useless. - 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); - 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 *future = - new std::future(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 - -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 if the file could be created, or an -/// \a llvm::Error in case of failures. -llvm::Expected> 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 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 +#include +#include #include #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::max() >= + std::numeric_limits::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 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> 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("runInTerminal", std::move(reverse_request)); - if (llvm::Expected pid = comm_channel.GetLauncherPid()) + if (llvm::Expected 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 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 &args, const llvm::StringMap &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 req_args = {DAP::debug_adapter_path.str(), - "--comm-file", comm_file.str()}; + std::vector 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 &args, const llvm::StringMap &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<"">, - 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<"">, - HelpText<"The fifo file used to communicate the with the debug adapter " - "when using --launch-target.">; +def launch_target + : S<"launch-target">, + MetaVarName<"">, + 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<"">, 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 -#include -#include -#endif - -#include -#include - -#include "llvm/Support/FileSystem.h" - -using namespace llvm; - -namespace lldb_dap { - -const RunInTerminalMessagePid *RunInTerminalMessage::GetAsPidMessage() const { - return static_cast(this); -} - -const RunInTerminalMessageError * -RunInTerminalMessage::GetAsErrorMessage() const { - return static_cast(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(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 -ParseJSONMessage(const json::Value &json) { - if (const json::Object *obj = json.getAsObject()) { - if (std::optional kind = obj->getString("kind")) { - if (*kind == "pid") { - if (std::optional pid = obj->getInteger("pid")) - return std::make_unique( - static_cast(*pid)); - } else if (*kind == "error") { - if (std::optional error = obj->getString("error")) - return std::make_unique(*error); - } else if (*kind == "didAttach") { - return std::make_unique(); - } - } - } - - return createStringError(inconvertibleErrorCode(), - "Incorrect JSON message: " + JSONToString(json)); -} - -static Expected -GetNextMessage(FifoFileIO &io, std::chrono::milliseconds timeout) { - if (Expected 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 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 because it doesn't compile on Windows -std::future -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 RunInTerminalDebugAdapterCommChannel::GetLauncherPid() { - if (Expected 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 message = - GetNextMessage(m_io, std::chrono::seconds(1))) - return toString(ToError(*message.get())); - else - return toString(message.takeError()); -} - -Expected> 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 -#include -#include - -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; - -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 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 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> 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 #include +#include #include +#include #include #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 -#include - -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()); -}