llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: John Harrison (ashgti) <details> <summary>Changes</summary> 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. --- Patch is 23.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/130026.diff 12 Files Affected: - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+2-2) - (modified) lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py (+1-1) - (modified) lldb/tools/lldb-dap/CMakeLists.txt (+2-2) - (modified) lldb/tools/lldb-dap/DAP.cpp (+32-98) - (modified) lldb/tools/lldb-dap/DAP.h (+5-15) - (modified) lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp (+1-1) - (removed) lldb/tools/lldb-dap/IOStream.cpp (-73) - (removed) lldb/tools/lldb-dap/IOStream.h (-42) - (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+2-1) - (added) lldb/tools/lldb-dap/Transport.cpp (+146) - (added) lldb/tools/lldb-dap/Transport.h (+54) - (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+1-1) ``````````diff 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/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py b/lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py index 6d1c25e8e4534..b0abe2a38dac4 100644 --- a/lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py +++ b/lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py @@ -43,7 +43,7 @@ def test_terminated_event(self): self.continue_to_breakpoints(breakpoint_ids) self.continue_to_exit() - statistics = self.dap_server.wait_for_terminated()["statistics"] + statistics = self.dap_server.wait_for_terminated()["body"]["$__lldb_statistics"] self.assertGreater(statistics["totalDebugInfoByteSize"], 0) self.assertGreater(statistics["totalDebugInfoEnabled"], 0) self.assertGreater(statistics["totalModuleCountHasDebugInfo"], 0) 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 1f7b25e7c5bcc..36989ad532a2d 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -11,6 +11,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" @@ -64,8 +65,8 @@ namespace lldb_dap { DAP::DAP(std::string name, llvm::StringRef path, std::ofstream *log, lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode, std::vector<std::string> pre_init_commands) - : name(std::move(name)), debug_adapter_path(path), log(log), - input(std::move(input)), output(std::move(output)), + : client_name(std::move(name)), debug_adapter_path(path), log(log), + 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), @@ -219,65 +220,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); - - if (log) { - auto now = std::chrono::duration<double>( - std::chrono::system_clock::now().time_since_epoch()); - *log << llvm::formatv("{0:f9} {1} <-- ", now.count(), name).str() - << std::endl - << "Content-Length: " << json_str.size() << "\r\n\r\n" - << llvm::formatv("{0:2}", json).str() << std::endl; - } -} - -// 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; - - if (log) { - auto now = std::chrono::duration<double>( - std::chrono::system_clock::now().time_since_epoch()); - *log << llvm::formatv("{0:f9} {1} --> ", now.count(), name).str() - << std::endl - << "Content-Length: " << length << "\r\n\r\n"; + // 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; } - return json_str; + 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": { @@ -704,36 +667,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 (auto error = json_value.takeError()) { - std::string error_str = llvm::toString(std::move(error)); - if (log) - *log << "error: failed to parse JSON: " << error_str << std::endl - << json << std::endl; - return PacketStatus::JSONMalformed; - } - - if (log) { - *log << llvm::formatv("{0:2}", *json_value).str() << std::endl; - } - - llvm::json::Object *object_ptr = json_value->getAsObject(); - if (!object_ptr) { - if (log) - *log << "error: json packet isn't a object" << std::endl; - 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"); @@ -838,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 8b2e498a28c95..f2f52c34af5ea 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> @@ -145,11 +145,10 @@ struct SendEventRequestHandler : public lldb::SBCommandPluginInterface { }; struct DAP { - std::string name; + std::string 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/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp index 5bb73a7ec0d85..7b7d8d5cedaa6 100644 --- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp @@ -111,7 +111,7 @@ void ProgressEventThreadFunction(DAP &dap) { // them prevent multiple threads from writing simultaneously so no locking // is required. static void EventThreadFunction(DAP &dap) { - llvm::set_thread_name(dap.name + ".event_handler"); + llvm::set_thread_name(dap.client_name + ".event_handler"); lldb::SBEvent event; lldb::SBListener listener = dap.debugger.GetListener(); dap.broadcaster.AddListener(listener, eBroadcastBitStopEventThread); 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/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 7094bf60bfbc2..932145b1799bd 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -1526,7 +1526,8 @@ static void addStatistic(lldb::SBTarget &target, llvm::json::Object &event) { const char *key = keys.GetStringAtIndex(i); FilterAndGetValueForKey(statistics, key, stats_body); } - event.try_emplace("statistics", std::move(stats_body)); + llvm::json::Object body{{"$__lldb_statistics", std::move(stats_body)}}; + event.try_emplace("body", std::move(body)); } llvm::json::Object CreateTerminatedEventObject(lldb::SBTarget &target) { 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 = fo... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/130026 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
