https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/130026
>From c340c38a82880114471938b19512670d7f94245e Mon Sep 17 00:00:00 2001 From: John Harrison <harj...@google.com> Date: Thu, 6 Mar 2025 10:18:36 +0100 Subject: [PATCH 1/5] [lldb-dap] Refactoring IOStream into Transport handler. Instead of having two discrete InputStream and OutputStream helpers, this merges the two into a unifed 'Transport' handler. This handler is responsible for reading the DAP message headers, parsing the resulting JSON and converting the messages into `lldb_dap::protocol::Message`s for both input and output. --- .../test/tools/lldb-dap/dap_server.py | 4 +- lldb/tools/lldb-dap/CMakeLists.txt | 4 +- lldb/tools/lldb-dap/DAP.cpp | 110 ++++--------- lldb/tools/lldb-dap/DAP.h | 18 +-- lldb/tools/lldb-dap/IOStream.cpp | 73 --------- lldb/tools/lldb-dap/IOStream.h | 42 ----- lldb/tools/lldb-dap/Transport.cpp | 146 ++++++++++++++++++ lldb/tools/lldb-dap/Transport.h | 54 +++++++ 8 files changed, 240 insertions(+), 211 deletions(-) delete mode 100644 lldb/tools/lldb-dap/IOStream.cpp delete mode 100644 lldb/tools/lldb-dap/IOStream.h create mode 100644 lldb/tools/lldb-dap/Transport.cpp create mode 100644 lldb/tools/lldb-dap/Transport.h diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py index 9471594b66012..0fea3419d9725 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py @@ -337,7 +337,7 @@ def send_recv(self, command): self.send_packet( { "type": "response", - "seq": -1, + "seq": 0, "request_seq": response_or_request["seq"], "success": True, "command": "runInTerminal", @@ -349,7 +349,7 @@ def send_recv(self, command): self.send_packet( { "type": "response", - "seq": -1, + "seq": 0, "request_seq": response_or_request["seq"], "success": True, "command": "startDebugging", diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt index 9a2d604f4d573..8a76cb58dbcab 100644 --- a/lldb/tools/lldb-dap/CMakeLists.txt +++ b/lldb/tools/lldb-dap/CMakeLists.txt @@ -28,14 +28,14 @@ add_lldb_tool(lldb-dap FifoFiles.cpp FunctionBreakpoint.cpp InstructionBreakpoint.cpp - IOStream.cpp JSONUtils.cpp LLDBUtils.cpp OutputRedirector.cpp ProgressEvent.cpp + Protocol.cpp RunInTerminal.cpp SourceBreakpoint.cpp - Protocol.cpp + Transport.cpp Watchpoint.cpp Handler/ResponseHandler.cpp diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index edd3b31be8ff7..6d19eb2917a1b 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -12,6 +12,7 @@ #include "JSONUtils.h" #include "LLDBUtils.h" #include "OutputRedirector.h" +#include "Transport.h" #include "lldb/API/SBBreakpoint.h" #include "lldb/API/SBCommandInterpreter.h" #include "lldb/API/SBCommandReturnObject.h" @@ -67,7 +68,7 @@ DAP::DAP(llvm::StringRef client_name, llvm::StringRef path, std::ofstream *log, lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode, std::vector<std::string> pre_init_commands) : client_name(client_name), debug_adapter_path(path), log(log), - input(std::move(input)), output(std::move(output)), + transport(client_name, std::move(input), std::move(output)), broadcaster("lldb-dap"), exception_breakpoints(), pre_init_commands(std::move(pre_init_commands)), focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false), @@ -221,52 +222,27 @@ void DAP::StopEventHandlers() { } } -// Send the JSON in "json_str" to the "out" stream. Correctly send the -// "Content-Length:" field followed by the length, followed by the raw -// JSON bytes. -void DAP::SendJSON(const std::string &json_str) { - output.write_full("Content-Length: "); - output.write_full(llvm::utostr(json_str.size())); - output.write_full("\r\n\r\n"); - output.write_full(json_str); -} - // Serialize the JSON value into a string and send the JSON packet to // the "out" stream. void DAP::SendJSON(const llvm::json::Value &json) { - std::string json_str; - llvm::raw_string_ostream strm(json_str); - strm << json; - static std::mutex mutex; - std::lock_guard<std::mutex> locker(mutex); - SendJSON(json_str); - - DAP_LOG(log, "({0}) <-- {1}", client_name, json_str); -} - -// Read a JSON packet from the "in" stream. -std::string DAP::ReadJSON() { - std::string length_str; - std::string json_str; - int length; - - if (!input.read_expected(log, "Content-Length: ")) - return json_str; - - if (!input.read_line(log, length_str)) - return json_str; - - if (!llvm::to_integer(length_str, length)) - return json_str; - - if (!input.read_expected(log, "\r\n")) - return json_str; - - if (!input.read_full(log, length, json_str)) - return json_str; - - DAP_LOG(log, "({0}) --> {1}", client_name, json_str); - return json_str; + // FIXME: Instead of parsing the output message from JSON, pass the `Message` + // as parameter to `SendJSON`. + protocol::Message M; + llvm::json::Path::Root root; + if (!protocol::fromJSON(json, M, root)) { + if (log) { + std::string error; + llvm::raw_string_ostream OS(error); + root.printErrorContext(json, OS); + *log << "encoding failure: " << error << "\n"; + } + return; + } + auto status = transport.Write(log, M); + if (status.Fail() && log) + *log << llvm::formatv("failed to send {0}: {1}\n", llvm::json::Value(M), + status.AsCString()) + .str(); } // "OutputEvent": { @@ -693,29 +669,10 @@ void DAP::SetTarget(const lldb::SBTarget target) { } } -PacketStatus DAP::GetNextObject(llvm::json::Object &object) { - std::string json = ReadJSON(); - if (json.empty()) - return PacketStatus::EndOfFile; - - llvm::StringRef json_sref(json); - llvm::Expected<llvm::json::Value> json_value = llvm::json::parse(json_sref); - if (!json_value) { - DAP_LOG_ERROR(log, json_value.takeError(), - "({1}) failed to parse JSON: {0}", client_name); - return PacketStatus::JSONMalformed; - } - - llvm::json::Object *object_ptr = json_value->getAsObject(); - if (!object_ptr) { - DAP_LOG(log, "({0}) error: json packet isn't a object", client_name); - return PacketStatus::JSONNotObject; - } - object = *object_ptr; - return PacketStatus::Success; -} - -bool DAP::HandleObject(const llvm::json::Object &object) { +bool DAP::HandleObject(const protocol::Message &M) { + // FIXME: Directly handle `Message` instead of serializing to JSON. + llvm::json::Value v = toJSON(M); + llvm::json::Object object = *v.getAsObject(); const auto packet_type = GetString(object, "type"); if (packet_type == "request") { const auto command = GetString(object, "command"); @@ -818,19 +775,16 @@ llvm::Error DAP::Loop() { StopEventHandlers(); }); while (!disconnecting) { - llvm::json::Object object; - lldb_dap::PacketStatus status = GetNextObject(object); - - if (status == lldb_dap::PacketStatus::EndOfFile) { - break; - } - - if (status != lldb_dap::PacketStatus::Success) { - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "failed to send packet"); + auto next = transport.Read(log); + if (auto Err = next.takeError()) { + // On EOF, simply break out of the loop. + std::error_code ec = llvm::errorToErrorCode(std::move(Err)); + if (ec == Transport::kEOF) + break; + return llvm::errorCodeToError(ec); } - if (!HandleObject(object)) { + if (!HandleObject(*next)) { return llvm::createStringError(llvm::inconvertibleErrorCode(), "unhandled packet"); } diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 3ff1992b61f5b..3254d0c1422cc 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -14,11 +14,12 @@ #include "FunctionBreakpoint.h" #include "Handler/RequestHandler.h" #include "Handler/ResponseHandler.h" -#include "IOStream.h" #include "InstructionBreakpoint.h" #include "OutputRedirector.h" #include "ProgressEvent.h" +#include "Protocol.h" #include "SourceBreakpoint.h" +#include "Transport.h" #include "lldb/API/SBBroadcaster.h" #include "lldb/API/SBCommandInterpreter.h" #include "lldb/API/SBDebugger.h" @@ -39,7 +40,6 @@ #include "llvm/Support/Error.h" #include "llvm/Support/JSON.h" #include "llvm/Support/Threading.h" -#include <map> #include <memory> #include <mutex> #include <optional> @@ -148,8 +148,7 @@ struct DAP { llvm::StringRef client_name; llvm::StringRef debug_adapter_path; std::ofstream *log; - InputStream input; - OutputStream output; + Transport transport; lldb::SBFile in; OutputRedirector out; OutputRedirector err; @@ -233,8 +232,6 @@ struct DAP { // the "out" stream. void SendJSON(const llvm::json::Value &json); - std::string ReadJSON(); - void SendOutput(OutputType o, const llvm::StringRef output); void SendProgressEvent(uint64_t progress_id, const char *message, @@ -307,8 +304,7 @@ struct DAP { /// listeing for its breakpoint events. void SetTarget(const lldb::SBTarget target); - PacketStatus GetNextObject(llvm::json::Object &object); - bool HandleObject(const llvm::json::Object &object); + bool HandleObject(const protocol::Message &M); /// Disconnect the DAP session. lldb::SBError Disconnect(); @@ -382,12 +378,6 @@ struct DAP { InstructionBreakpoint *GetInstructionBreakpoint(const lldb::break_id_t bp_id); InstructionBreakpoint *GetInstructionBPFromStopReason(lldb::SBThread &thread); - -private: - // Send the JSON in "json_str" to the "out" stream. Correctly send the - // "Content-Length:" field followed by the length, followed by the raw - // JSON bytes. - void SendJSON(const std::string &json_str); }; } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/IOStream.cpp b/lldb/tools/lldb-dap/IOStream.cpp deleted file mode 100644 index ee22a297ec248..0000000000000 --- a/lldb/tools/lldb-dap/IOStream.cpp +++ /dev/null @@ -1,73 +0,0 @@ -//===-- IOStream.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 "IOStream.h" -#include "lldb/Utility/IOObject.h" -#include "lldb/Utility/Status.h" -#include <fstream> -#include <string> - -using namespace lldb_dap; - -bool OutputStream::write_full(llvm::StringRef str) { - if (!descriptor) - return false; - - size_t num_bytes = str.size(); - auto status = descriptor->Write(str.data(), num_bytes); - return status.Success(); -} - -bool InputStream::read_full(std::ofstream *log, size_t length, - std::string &text) { - if (!descriptor) - return false; - - std::string data; - data.resize(length); - - auto status = descriptor->Read(data.data(), length); - if (status.Fail()) - return false; - - text += data.substr(0, length); - return true; -} - -bool InputStream::read_line(std::ofstream *log, std::string &line) { - line.clear(); - while (true) { - std::string next; - if (!read_full(log, 1, next)) - return false; - - // If EOF is encoutnered, '' is returned, break out of this loop. - if (next.empty()) - return false; - - line += next; - - if (llvm::StringRef(line).ends_with("\r\n")) - break; - } - line.erase(line.size() - 2); - return true; -} - -bool InputStream::read_expected(std::ofstream *log, llvm::StringRef expected) { - std::string result; - if (!read_full(log, expected.size(), result)) - return false; - if (expected != result) { - if (log) - *log << "Warning: Expected '" << expected.str() << "', got '" << result - << "\n"; - return false; - } - return true; -} diff --git a/lldb/tools/lldb-dap/IOStream.h b/lldb/tools/lldb-dap/IOStream.h deleted file mode 100644 index e9fb8e11c92da..0000000000000 --- a/lldb/tools/lldb-dap/IOStream.h +++ /dev/null @@ -1,42 +0,0 @@ -//===-- IOStream.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_IOSTREAM_H -#define LLDB_TOOLS_LLDB_DAP_IOSTREAM_H - -#include "lldb/lldb-forward.h" -#include "llvm/ADT/StringRef.h" -#include <fstream> -#include <string> - -namespace lldb_dap { - -struct InputStream { - lldb::IOObjectSP descriptor; - - explicit InputStream(lldb::IOObjectSP descriptor) - : descriptor(std::move(descriptor)) {} - - bool read_full(std::ofstream *log, size_t length, std::string &text); - - bool read_line(std::ofstream *log, std::string &line); - - bool read_expected(std::ofstream *log, llvm::StringRef expected); -}; - -struct OutputStream { - lldb::IOObjectSP descriptor; - - explicit OutputStream(lldb::IOObjectSP descriptor) - : descriptor(std::move(descriptor)) {} - - bool write_full(llvm::StringRef str); -}; -} // namespace lldb_dap - -#endif diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp new file mode 100644 index 0000000000000..f49bf373c4aff --- /dev/null +++ b/lldb/tools/lldb-dap/Transport.cpp @@ -0,0 +1,146 @@ +//===-- Transport.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 "Transport.h" +#include "Protocol.h" +#include "c++/v1/__system_error/error_code.h" +#include "lldb/Utility/IOObject.h" +#include "lldb/Utility/Status.h" +#include "lldb/lldb-forward.h" +#include "llvm/ADT/StringExtras.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/raw_ostream.h" +#include <string> +#include <system_error> +#include <utility> + +using namespace llvm; +using namespace lldb; +using namespace lldb_private; +using namespace lldb_dap; +using namespace lldb_dap::protocol; + +static Expected<std::string> ReadFull(IOObjectSP &descriptor, size_t length) { + if (!descriptor || !descriptor->IsValid()) + return createStringError("transport input is closed"); + + std::string data; + data.resize(length); + + auto status = descriptor->Read(data.data(), length); + if (status.Fail()) + return status.takeError(); + + // If we got back zero then we have reached EOF. + if (length == 0) + return createStringError(Transport::kEOF, "end-of-file"); + + return data.substr(0, length); +} + +static Expected<std::string> ReadUntil(IOObjectSP &descriptor, + StringRef delimiter) { + std::string buffer; + buffer.reserve(delimiter.size() + 1); + while (!llvm::StringRef(buffer).ends_with(delimiter)) { + auto next = ReadFull(descriptor, 1); + if (auto Err = next.takeError()) + return std::move(Err); + buffer += *next; + } + return buffer.substr(0, buffer.size() - delimiter.size()); +} + +static Error ReadExpected(IOObjectSP &descriptor, StringRef want) { + auto got = ReadFull(descriptor, want.size()); + if (auto Err = got.takeError()) + return Err; + if (*got != want) { + return createStringError("want %s, got %s", want.str().c_str(), + got->c_str()); + } + return Error::success(); +} + +namespace lldb_dap { + +const std::error_code Transport::kEOF = + std::error_code(0x1001, std::generic_category()); + +Transport::Transport(StringRef client_name, IOObjectSP input, IOObjectSP output) + : m_client_name(client_name), m_input(std::move(input)), + m_output(std::move(output)) {} + +Expected<protocol::Message> Transport::Read(std::ofstream *log) { + // If we don't find the expected header we have reached EOF. + if (auto Err = ReadExpected(m_input, "Content-Length: ")) + return std::move(Err); + + auto rawLength = ReadUntil(m_input, "\r\n\r\n"); + if (auto Err = rawLength.takeError()) + return std::move(Err); + + size_t length; + if (!to_integer(*rawLength, length)) + return createStringError("invalid content length %s", rawLength->c_str()); + + auto rawJSON = ReadFull(m_input, length); + if (auto Err = rawJSON.takeError()) + return std::move(Err); + if (rawJSON->length() != length) + return createStringError( + "malformed request, expected %ld bytes, got %ld bytes", length, + rawJSON->length()); + + if (log) { + auto now = std::chrono::duration<double>( + std::chrono::system_clock::now().time_since_epoch()); + *log << formatv("{0:f9} <-- ({1}) {2}\n", now.count(), m_client_name, + *rawJSON) + .str(); + } + + auto JSON = json::parse(*rawJSON); + if (auto Err = JSON.takeError()) { + return createStringError("malformed JSON %s\n%s", rawJSON->c_str(), + llvm::toString(std::move(Err)).c_str()); + } + + protocol::Message M; + llvm::json::Path::Root Root; + if (!fromJSON(*JSON, M, Root)) { + std::string error; + raw_string_ostream OS(error); + Root.printErrorContext(*JSON, OS); + return createStringError("malformed request: %s", error.c_str()); + } + return std::move(M); +} + +lldb_private::Status Transport::Write(std::ofstream *log, + const protocol::Message &M) { + if (!m_output || !m_output->IsValid()) + return Status("transport output is closed"); + + std::string JSON = formatv("{0}", toJSON(M)).str(); + + if (log) { + auto now = std::chrono::duration<double>( + std::chrono::system_clock::now().time_since_epoch()); + *log << formatv("{0:f9} --> ({1}) {2}\n", now.count(), m_client_name, JSON) + .str(); + } + + std::string Output; + raw_string_ostream OS(Output); + OS << "Content-Length: " << JSON.length() << "\r\n\r\n" << JSON; + size_t num_bytes = Output.size(); + return m_output->Write(Output.data(), num_bytes); +} + +} // end namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h new file mode 100644 index 0000000000000..0150b762ceb10 --- /dev/null +++ b/lldb/tools/lldb-dap/Transport.h @@ -0,0 +1,54 @@ +//===-- Transport.h -------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// +// +// Debug Adapter Protocol transport layer for encoding and decoding protocol +// messages. +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_TOOLS_LLDB_DAP_TRANSPORT_H +#define LLDB_TOOLS_LLDB_DAP_TRANSPORT_H + +#include "Protocol.h" +#include "lldb/Utility/Status.h" +#include "lldb/lldb-forward.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Error.h" +#include <fstream> + +namespace lldb_dap { + +/// A transport class that performs the Debug Adapter Protocol communication +/// with the client. +class Transport { +public: + Transport(llvm::StringRef client_name, lldb::IOObjectSP input, + lldb::IOObjectSP output); + ~Transport() = default; + + Transport(const Transport &rhs) = delete; + void operator=(const Transport &rhs) = delete; + + /// Error code returned this if EOF is encountered. + static const std::error_code kEOF; + + /// Writes a Debug Adater Protocol message to the output stream. + lldb_private::Status Write(std::ofstream *log, const protocol::Message &M); + + /// Reads the next Debug Adater Protocol message from the input stream. + llvm::Expected<protocol::Message> Read(std::ofstream *log); + +private: + llvm::StringRef m_client_name; + lldb::IOObjectSP m_input; + lldb::IOObjectSP m_output; +}; + +} // namespace lldb_dap + +#endif >From 138d4ca7d8adff56330a65dcf202a11b1447f86f Mon Sep 17 00:00:00 2001 From: John Harrison <harj...@google.com> Date: Sat, 8 Mar 2025 17:56:54 -0800 Subject: [PATCH 2/5] Adjusting `Transport::Read` to return an `optional<string>` and adding logging directly to the `Transport` class. --- lldb/tools/lldb-dap/DAP.cpp | 45 ++++------ lldb/tools/lldb-dap/DAP.h | 37 ++++++-- lldb/tools/lldb-dap/Transport.cpp | 141 ++++++++++++++---------------- lldb/tools/lldb-dap/Transport.h | 17 ++-- lldb/tools/lldb-dap/lldb-dap.cpp | 12 ++- 5 files changed, 132 insertions(+), 120 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 6d19eb2917a1b..a3a0b0ad2f452 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -64,12 +64,12 @@ const char DEV_NULL[] = "/dev/null"; namespace lldb_dap { -DAP::DAP(llvm::StringRef client_name, llvm::StringRef path, std::ofstream *log, - lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode, - std::vector<std::string> pre_init_commands) - : client_name(client_name), debug_adapter_path(path), log(log), - transport(client_name, std::move(input), std::move(output)), - broadcaster("lldb-dap"), exception_breakpoints(), +DAP::DAP(llvm::StringRef path, std::ofstream *log, + const ReplMode default_repl_mode, + const std::vector<std::string> &pre_init_commands, + llvm::StringRef client_name, Transport &transport) + : debug_adapter_path(path), log(log), client_name(client_name), + transport(transport), broadcaster("lldb-dap"), exception_breakpoints(), pre_init_commands(std::move(pre_init_commands)), focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false), enable_auto_variable_summaries(false), @@ -79,7 +79,7 @@ DAP::DAP(llvm::StringRef client_name, llvm::StringRef path, std::ofstream *log, configuration_done_sent(false), waiting_for_run_in_terminal(false), progress_event_reporter( [&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }), - reverse_request_seq(0), repl_mode(repl_mode) {} + reverse_request_seq(0), repl_mode(default_repl_mode) {} DAP::~DAP() = default; @@ -227,22 +227,17 @@ void DAP::StopEventHandlers() { void DAP::SendJSON(const llvm::json::Value &json) { // FIXME: Instead of parsing the output message from JSON, pass the `Message` // as parameter to `SendJSON`. - protocol::Message M; + protocol::Message message; llvm::json::Path::Root root; - if (!protocol::fromJSON(json, M, root)) { - if (log) { - std::string error; - llvm::raw_string_ostream OS(error); - root.printErrorContext(json, OS); - *log << "encoding failure: " << error << "\n"; - } + if (!fromJSON(json, message, root)) { + DAP_LOG_ERROR(log, root.getError(), "({1}) encoding failed: {0}", + client_name); return; } - auto status = transport.Write(log, M); - if (status.Fail() && log) - *log << llvm::formatv("failed to send {0}: {1}\n", llvm::json::Value(M), - status.AsCString()) - .str(); + auto err = transport.Write(message); + if (err) { + DAP_LOG_ERROR(log, std::move(err), "({1}) write failed: {0}", client_name); + } } // "OutputEvent": { @@ -775,13 +770,9 @@ llvm::Error DAP::Loop() { StopEventHandlers(); }); while (!disconnecting) { - auto next = transport.Read(log); - if (auto Err = next.takeError()) { - // On EOF, simply break out of the loop. - std::error_code ec = llvm::errorToErrorCode(std::move(Err)); - if (ec == Transport::kEOF) - break; - return llvm::errorCodeToError(ec); + std::optional<protocol::Message> next = transport.Read(); + if (!next) { + break; } if (!HandleObject(*next)) { diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 3254d0c1422cc..cc357158283dc 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -125,30 +125,30 @@ struct Variables { struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {}; + explicit StartDebuggingRequestHandler(DAP &d) : dap(d){}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit ReplModeRequestHandler(DAP &d) : dap(d) {}; + explicit ReplModeRequestHandler(DAP &d) : dap(d){}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct SendEventRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit SendEventRequestHandler(DAP &d) : dap(d) {}; + explicit SendEventRequestHandler(DAP &d) : dap(d){}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct DAP { - llvm::StringRef client_name; llvm::StringRef debug_adapter_path; std::ofstream *log; - Transport transport; + llvm::StringRef client_name; + Transport &transport; lldb::SBFile in; OutputRedirector out; OutputRedirector err; @@ -209,12 +209,33 @@ struct DAP { // will contain that expression. std::string last_nonempty_var_expression; - DAP(llvm::StringRef client_name, llvm::StringRef path, std::ofstream *log, - lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode, - std::vector<std::string> pre_init_commands); + /// Creates a new DAP sessions. + /// + /// \param[in] path + /// Path to the lldb-dap binary. + /// \param[in] log + /// Log file stream, if configured. + /// \param[in] default_repl_mode + /// Default repl mode behavior, as configured by the binary. + /// \param[in] pre_init_commands + /// LLDB commands to execute as soon as the debugger instance is allocaed. + /// \param[in] client_name + /// Debug session client name, for example 'stdin/stdout' or 'client_1'. + /// \param[in] transport + /// Transport for this debug session. + DAP(llvm::StringRef path, std::ofstream *log, + const ReplMode default_repl_mode, + const std::vector<std::string> &pre_init_commands, + llvm::StringRef client_name, Transport &transport); + ~DAP(); + + /// DAP is not copyable. + /// @{ DAP(const DAP &rhs) = delete; void operator=(const DAP &rhs) = delete; + /// @} + ExceptionBreakpoint *GetExceptionBreakpoint(const std::string &filter); ExceptionBreakpoint *GetExceptionBreakpoint(const lldb::break_id_t bp_id); diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp index f49bf373c4aff..fdfecf250fb16 100644 --- a/lldb/tools/lldb-dap/Transport.cpp +++ b/lldb/tools/lldb-dap/Transport.cpp @@ -7,16 +7,16 @@ //===----------------------------------------------------------------------===// #include "Transport.h" +#include "DAPLog.h" #include "Protocol.h" -#include "c++/v1/__system_error/error_code.h" #include "lldb/Utility/IOObject.h" #include "lldb/Utility/Status.h" #include "lldb/lldb-forward.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/raw_ostream.h" #include <string> -#include <system_error> #include <utility> using namespace llvm; @@ -28,18 +28,11 @@ using namespace lldb_dap::protocol; static Expected<std::string> ReadFull(IOObjectSP &descriptor, size_t length) { if (!descriptor || !descriptor->IsValid()) return createStringError("transport input is closed"); - std::string data; data.resize(length); - auto status = descriptor->Read(data.data(), length); if (status.Fail()) return status.takeError(); - - // If we got back zero then we have reached EOF. - if (length == 0) - return createStringError(Transport::kEOF, "end-of-file"); - return data.substr(0, length); } @@ -48,99 +41,101 @@ static Expected<std::string> ReadUntil(IOObjectSP &descriptor, std::string buffer; buffer.reserve(delimiter.size() + 1); while (!llvm::StringRef(buffer).ends_with(delimiter)) { - auto next = ReadFull(descriptor, 1); + Expected<std::string> next = + ReadFull(descriptor, buffer.empty() ? delimiter.size() : 1); if (auto Err = next.takeError()) return std::move(Err); + // '' is returned on EOF. + if (next->empty()) + return buffer; buffer += *next; } return buffer.substr(0, buffer.size() - delimiter.size()); } -static Error ReadExpected(IOObjectSP &descriptor, StringRef want) { - auto got = ReadFull(descriptor, want.size()); - if (auto Err = got.takeError()) - return Err; - if (*got != want) { - return createStringError("want %s, got %s", want.str().c_str(), - got->c_str()); - } - return Error::success(); -} +/// DAP message format +/// ``` +/// Content-Length: (?<length>\d+)\r\n\r\n(?<content>.{\k<length>}) +/// ``` +static const StringLiteral kHeaderContentLength = "Content-Length: "; +static const StringLiteral kHeaderSeparator = "\r\n\r\n"; namespace lldb_dap { -const std::error_code Transport::kEOF = - std::error_code(0x1001, std::generic_category()); - -Transport::Transport(StringRef client_name, IOObjectSP input, IOObjectSP output) - : m_client_name(client_name), m_input(std::move(input)), +Transport::Transport(StringRef client_name, std::ofstream *log, + IOObjectSP input, IOObjectSP output) + : m_client_name(client_name), m_log(log), m_input(std::move(input)), m_output(std::move(output)) {} -Expected<protocol::Message> Transport::Read(std::ofstream *log) { - // If we don't find the expected header we have reached EOF. - if (auto Err = ReadExpected(m_input, "Content-Length: ")) - return std::move(Err); +std::optional<Message> Transport::Read() { + Expected<std::string> message_header = + ReadFull(m_input, kHeaderContentLength.size()); + if (!message_header) { + DAP_LOG_ERROR(m_log, message_header.takeError(), "({1}) read failed: {0}", + m_client_name); + return std::nullopt; + } - auto rawLength = ReadUntil(m_input, "\r\n\r\n"); - if (auto Err = rawLength.takeError()) - return std::move(Err); + // '' returned on EOF. + if (message_header->empty()) + return std::nullopt; + if (*message_header != kHeaderContentLength) { + DAP_LOG(m_log, "({0}) read failed: expected '{1}' and got '{2}'", + m_client_name, kHeaderContentLength, *message_header); + return std::nullopt; + } + + Expected<std::string> raw_length = ReadUntil(m_input, kHeaderSeparator); + if (!raw_length) { + DAP_LOG_ERROR(m_log, raw_length.takeError(), "({1}) read failed: {0}", + m_client_name); + return std::nullopt; + } size_t length; - if (!to_integer(*rawLength, length)) - return createStringError("invalid content length %s", rawLength->c_str()); - - auto rawJSON = ReadFull(m_input, length); - if (auto Err = rawJSON.takeError()) - return std::move(Err); - if (rawJSON->length() != length) - return createStringError( - "malformed request, expected %ld bytes, got %ld bytes", length, - rawJSON->length()); - - if (log) { - auto now = std::chrono::duration<double>( - std::chrono::system_clock::now().time_since_epoch()); - *log << formatv("{0:f9} <-- ({1}) {2}\n", now.count(), m_client_name, - *rawJSON) - .str(); + if (!to_integer(*raw_length, length)) { + DAP_LOG(m_log, "({0}) read failed: invalid content length {1}", + m_client_name, *raw_length); + return std::nullopt; } - auto JSON = json::parse(*rawJSON); - if (auto Err = JSON.takeError()) { - return createStringError("malformed JSON %s\n%s", rawJSON->c_str(), - llvm::toString(std::move(Err)).c_str()); + Expected<std::string> raw_json = ReadFull(m_input, length); + if (!raw_json) { + DAP_LOG_ERROR(m_log, raw_json.takeError(), "({1}) read failed: {0}", + m_client_name); + return std::nullopt; + } + if (raw_json->length() != length) { + DAP_LOG(m_log, "({0}) read failed: expected {1} bytes and got {2} bytes", + m_client_name, length, raw_json->length()); + return std::nullopt; } - protocol::Message M; - llvm::json::Path::Root Root; - if (!fromJSON(*JSON, M, Root)) { - std::string error; - raw_string_ostream OS(error); - Root.printErrorContext(*JSON, OS); - return createStringError("malformed request: %s", error.c_str()); + DAP_LOG(m_log, "<-- ({0}) {1}", m_client_name, *raw_json); + + llvm::Expected<Message> message = json::parse<Message>(*raw_json); + if (!message) { + DAP_LOG_ERROR(m_log, message.takeError(), "({1}) read failed: {0}", + m_client_name); + return std::nullopt; } - return std::move(M); + + return std::move(*message); } -lldb_private::Status Transport::Write(std::ofstream *log, - const protocol::Message &M) { +Error Transport::Write(const Message &message) { if (!m_output || !m_output->IsValid()) - return Status("transport output is closed"); + return createStringError("transport output is closed"); - std::string JSON = formatv("{0}", toJSON(M)).str(); + std::string json = formatv("{0}", toJSON(message)).str(); - if (log) { - auto now = std::chrono::duration<double>( - std::chrono::system_clock::now().time_since_epoch()); - *log << formatv("{0:f9} --> ({1}) {2}\n", now.count(), m_client_name, JSON) - .str(); - } + DAP_LOG(m_log, "--> ({0}) {1}", m_client_name, json); std::string Output; raw_string_ostream OS(Output); - OS << "Content-Length: " << JSON.length() << "\r\n\r\n" << JSON; + OS << kHeaderContentLength << json.length() << kHeaderSeparator << json; size_t num_bytes = Output.size(); - return m_output->Write(Output.data(), num_bytes); + return m_output->Write(Output.data(), num_bytes).takeError(); } } // end namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h index 0150b762ceb10..a77f30f3aaf49 100644 --- a/lldb/tools/lldb-dap/Transport.h +++ b/lldb/tools/lldb-dap/Transport.h @@ -15,11 +15,11 @@ #define LLDB_TOOLS_LLDB_DAP_TRANSPORT_H #include "Protocol.h" -#include "lldb/Utility/Status.h" #include "lldb/lldb-forward.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include <fstream> +#include <optional> namespace lldb_dap { @@ -27,24 +27,25 @@ namespace lldb_dap { /// with the client. class Transport { public: - Transport(llvm::StringRef client_name, lldb::IOObjectSP input, - lldb::IOObjectSP output); + Transport(llvm::StringRef client_name, std::ofstream *log, + lldb::IOObjectSP input, lldb::IOObjectSP output); ~Transport() = default; + /// Transport is not copyable. + /// @{ Transport(const Transport &rhs) = delete; void operator=(const Transport &rhs) = delete; - - /// Error code returned this if EOF is encountered. - static const std::error_code kEOF; + /// @} /// Writes a Debug Adater Protocol message to the output stream. - lldb_private::Status Write(std::ofstream *log, const protocol::Message &M); + llvm::Error Write(const protocol::Message &M); /// Reads the next Debug Adater Protocol message from the input stream. - llvm::Expected<protocol::Message> Read(std::ofstream *log); + std::optional<protocol::Message> Read(); private: llvm::StringRef m_client_name; + std::ofstream *m_log; lldb::IOObjectSP m_input; lldb::IOObjectSP m_output; }; diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index ab40b75e5425d..660bc9c046b6f 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -11,6 +11,7 @@ #include "EventHelper.h" #include "Handler/RequestHandler.h" #include "RunInTerminal.h" +#include "Transport.h" #include "lldb/API/SBDebugger.h" #include "lldb/API/SBStream.h" #include "lldb/Host/Config.h" @@ -325,8 +326,9 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name, std::thread client([=, &dap_sessions_condition, &dap_sessions_mutex, &dap_sessions]() { llvm::set_thread_name(client_name + ".runloop"); - DAP dap = DAP(client_name, program_path, log, io, io, default_repl_mode, - pre_init_commands); + Transport transport{client_name, log, io, io}; + DAP dap = DAP(program_path, log, default_repl_mode, pre_init_commands, + client_name, transport); if (auto Err = dap.ConfigureIO()) { llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), @@ -565,8 +567,10 @@ int main(int argc, char *argv[]) { lldb::IOObjectSP output = std::make_shared<NativeFile>( stdout_fd, File::eOpenOptionWriteOnly, false); - DAP dap = DAP("stdin/stdout", program_path, log.get(), std::move(input), - std::move(output), default_repl_mode, pre_init_commands); + std::string client_name = "stdin/stdout"; + Transport transport{client_name, log.get(), input, output}; + DAP dap = DAP(program_path, log.get(), default_repl_mode, pre_init_commands, + client_name, transport); // stdout/stderr redirection to the IDE's console if (auto Err = dap.ConfigureIO(stdout, stderr)) { >From 9fdcba9adb186651f5da7f68bfbfdb7e20d054a3 Mon Sep 17 00:00:00 2001 From: John Harrison <harj...@google.com> Date: Mon, 10 Mar 2025 14:46:38 -0700 Subject: [PATCH 3/5] Adjusting the reader helpers to take `lldb_private::IOObject *` instead of the shared_ptr. --- lldb/tools/lldb-dap/Transport.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp index fdfecf250fb16..c5c6651db1c35 100644 --- a/lldb/tools/lldb-dap/Transport.cpp +++ b/lldb/tools/lldb-dap/Transport.cpp @@ -25,9 +25,7 @@ using namespace lldb_private; using namespace lldb_dap; using namespace lldb_dap::protocol; -static Expected<std::string> ReadFull(IOObjectSP &descriptor, size_t length) { - if (!descriptor || !descriptor->IsValid()) - return createStringError("transport input is closed"); +static Expected<std::string> ReadFull(IOObject *descriptor, size_t length) { std::string data; data.resize(length); auto status = descriptor->Read(data.data(), length); @@ -36,7 +34,7 @@ static Expected<std::string> ReadFull(IOObjectSP &descriptor, size_t length) { return data.substr(0, length); } -static Expected<std::string> ReadUntil(IOObjectSP &descriptor, +static Expected<std::string> ReadUntil(IOObject *descriptor, StringRef delimiter) { std::string buffer; buffer.reserve(delimiter.size() + 1); @@ -68,8 +66,13 @@ Transport::Transport(StringRef client_name, std::ofstream *log, m_output(std::move(output)) {} std::optional<Message> Transport::Read() { + if (!m_input || !m_input->IsValid()) { + DAP_LOG(m_log, "({0}) input is closed", m_client_name); + return std::nullopt; + } + IOObject *input = m_input.get(); Expected<std::string> message_header = - ReadFull(m_input, kHeaderContentLength.size()); + ReadFull(input, kHeaderContentLength.size()); if (!message_header) { DAP_LOG_ERROR(m_log, message_header.takeError(), "({1}) read failed: {0}", m_client_name); @@ -85,7 +88,7 @@ std::optional<Message> Transport::Read() { return std::nullopt; } - Expected<std::string> raw_length = ReadUntil(m_input, kHeaderSeparator); + Expected<std::string> raw_length = ReadUntil(input, kHeaderSeparator); if (!raw_length) { DAP_LOG_ERROR(m_log, raw_length.takeError(), "({1}) read failed: {0}", m_client_name); @@ -99,7 +102,7 @@ std::optional<Message> Transport::Read() { return std::nullopt; } - Expected<std::string> raw_json = ReadFull(m_input, length); + Expected<std::string> raw_json = ReadFull(input, length); if (!raw_json) { DAP_LOG_ERROR(m_log, raw_json.takeError(), "({1}) read failed: {0}", m_client_name); >From 9090a31278702b254e1dc76017c0ee0fe5750a43 Mon Sep 17 00:00:00 2001 From: John Harrison <harj...@google.com> Date: Mon, 10 Mar 2025 14:53:20 -0700 Subject: [PATCH 4/5] Aplying clang-format. --- lldb/tools/lldb-dap/DAP.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index cc357158283dc..aaf9c3bbbaa66 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -125,21 +125,21 @@ struct Variables { struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit StartDebuggingRequestHandler(DAP &d) : dap(d){}; + explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit ReplModeRequestHandler(DAP &d) : dap(d){}; + explicit ReplModeRequestHandler(DAP &d) : dap(d) {}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct SendEventRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit SendEventRequestHandler(DAP &d) : dap(d){}; + explicit SendEventRequestHandler(DAP &d) : dap(d) {}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; >From 40d149d9fbfce20b531dda8534d58399f0f56a96 Mon Sep 17 00:00:00 2001 From: John Harrison <harj...@google.com> Date: Tue, 11 Mar 2025 10:28:18 -0700 Subject: [PATCH 5/5] Applying reviewer feedback. --- lldb/tools/lldb-dap/DAP.cpp | 6 ++---- lldb/tools/lldb-dap/Transport.cpp | 4 ++-- lldb/tools/lldb-dap/lldb-dap.cpp | 12 ++++++------ 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index a3a0b0ad2f452..4594e001eadcf 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -701,9 +701,8 @@ bool DAP::HandleObject(const protocol::Message &M) { // Result should be given, use null if not. if (GetBoolean(object, "success").value_or(false)) { llvm::json::Value Result = nullptr; - if (auto *B = object.get("body")) { + if (auto *B = object.get("body")) Result = std::move(*B); - } (*response_handler)(Result); } else { llvm::StringRef message = GetString(object, "message"); @@ -771,9 +770,8 @@ llvm::Error DAP::Loop() { }); while (!disconnecting) { std::optional<protocol::Message> next = transport.Read(); - if (!next) { + if (!next) break; - } if (!HandleObject(*next)) { return llvm::createStringError(llvm::inconvertibleErrorCode(), diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp index c5c6651db1c35..cb8319e5e6624 100644 --- a/lldb/tools/lldb-dap/Transport.cpp +++ b/lldb/tools/lldb-dap/Transport.cpp @@ -55,8 +55,8 @@ static Expected<std::string> ReadUntil(IOObject *descriptor, /// ``` /// Content-Length: (?<length>\d+)\r\n\r\n(?<content>.{\k<length>}) /// ``` -static const StringLiteral kHeaderContentLength = "Content-Length: "; -static const StringLiteral kHeaderSeparator = "\r\n\r\n"; +static constexpr StringLiteral kHeaderContentLength = "Content-Length: "; +static constexpr StringLiteral kHeaderSeparator = "\r\n\r\n"; namespace lldb_dap { diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 660bc9c046b6f..f15af1c54dd63 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -326,9 +326,9 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name, std::thread client([=, &dap_sessions_condition, &dap_sessions_mutex, &dap_sessions]() { llvm::set_thread_name(client_name + ".runloop"); - Transport transport{client_name, log, io, io}; - DAP dap = DAP(program_path, log, default_repl_mode, pre_init_commands, - client_name, transport); + Transport transport(client_name, log, io, io); + DAP dap(program_path, log, default_repl_mode, pre_init_commands, + client_name, transport); if (auto Err = dap.ConfigureIO()) { llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), @@ -568,9 +568,9 @@ int main(int argc, char *argv[]) { stdout_fd, File::eOpenOptionWriteOnly, false); std::string client_name = "stdin/stdout"; - Transport transport{client_name, log.get(), input, output}; - DAP dap = DAP(program_path, log.get(), default_repl_mode, pre_init_commands, - client_name, transport); + Transport transport(client_name, log.get(), input, output); + DAP dap(program_path, log.get(), default_repl_mode, pre_init_commands, + client_name, transport); // stdout/stderr redirection to the IDE's console if (auto Err = dap.ConfigureIO(stdout, stderr)) { _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits