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 a3d924d495fb1..d892c01f0bc71 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 @@ -11,6 +11,7 @@ import signal import sys import threading +import warnings import time from typing import ( Any, @@ -383,6 +384,10 @@ def _process_recv_packets(self) -> None: """Process received packets, updating the session state.""" with self._recv_condition: for packet in self._recv_packets: + if packet and ("seq" not in packet or packet["seq"] == 0): + warnings.warn( + f"received a malformed packet, expected 'seq != 0' for {packet!r}" + ) # Handle events that may modify any stateful properties of # the DAP session. if packet and packet["type"] == "event": @@ -576,6 +581,7 @@ def wait_for_stopped(self) -> Optional[List[Event]]: # If we exited, then we are done if stopped_event["event"] == "exited": break + # Otherwise we stopped and there might be one or more 'stopped' # events for each thread that stopped with a reason, so keep # checking for more 'stopped' events and return all of them diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 61226cceb6db0..52c8c6b9092b9 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -266,40 +266,49 @@ void DAP::SendJSON(const llvm::json::Value &json) { Send(message); } -void DAP::Send(const Message &message) { - if (const protocol::Event *event = std::get_if(&message)) { +Id DAP::Send(const Message &message) { + std::lock_guard guard(call_mutex); + Message msg = std::visit( + [this](auto &&msg) -> Message { + if (msg.seq == kCalculateSeq) + msg.seq = seq++; + return msg; + }, + Message(message)); + + if (const protocol::Event *event = std::get_if(&msg)) { if (llvm::Error err = transport.Send(*event)) DAP_LOG_ERROR(log, std::move(err), "({0}) sending event failed", m_client_name); - return; + return event->seq; } - if (const Request *req = std::get_if(&message)) { + if (const Request *req = std::get_if(&msg)) { if (llvm::Error err = transport.Send(*req)) DAP_LOG_ERROR(log, std::move(err), "({0}) sending request failed", m_client_name); - return; + return req->seq; } - if (const Response *resp = std::get_if(&message)) { + if (const Response *resp = std::get_if(&msg)) { // FIXME: After all the requests have migrated from LegacyRequestHandler > // RequestHandler<> this should be handled in RequestHandler<>::operator(). // If the debugger was interrupted, convert this response into a // 'cancelled' response because we might have a partial result. - llvm::Error err = - (debugger.InterruptRequested()) - ? transport.Send({/*request_seq=*/resp->request_seq, - /*command=*/resp->command, - /*success=*/false, - /*message=*/eResponseMessageCancelled, - /*body=*/std::nullopt}) - : transport.Send(*resp); - if (err) { + llvm::Error err = (debugger.InterruptRequested()) + ? transport.Send({ + /*request_seq=*/resp->request_seq, + /*command=*/resp->command, + /*success=*/false, + /*message=*/eResponseMessageCancelled, + /*body=*/std::nullopt, + /*seq=*/resp->seq, + }) + : transport.Send(*resp); + if (err) DAP_LOG_ERROR(log, std::move(err), "({0}) sending response failed", m_client_name); - return; - } - return; + return resp->seq; } llvm_unreachable("Unexpected message type"); diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index bf2c3f146a396..b4f111e4e720c 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -136,7 +136,7 @@ struct DAP final : public DAPTransport::MessageHandler { /// unless we send a "thread" event to indicate the thread exited. llvm::DenseSet thread_ids; - uint32_t reverse_request_seq = 0; + protocol::Id seq = 0; std::mutex call_mutex; llvm::SmallDenseMap> inflight_reverse_requests; @@ -220,8 +220,8 @@ struct DAP final : public DAPTransport::MessageHandler { /// Serialize the JSON value into a string and send the JSON packet to the /// "out" stream. void SendJSON(const llvm::json::Value &json); - /// Send the given message to the client - void Send(const protocol::Message &message); + /// Send the given message to the client. + protocol::Id Send(const protocol::Message &message); void SendOutput(OutputType o, const llvm::StringRef output); @@ -353,19 +353,13 @@ struct DAP final : public DAPTransport::MessageHandler { template void SendReverseRequest(llvm::StringRef command, llvm::json::Value arguments) { - int64_t id; - { - std::lock_guard locker(call_mutex); - id = ++reverse_request_seq; - inflight_reverse_requests[id] = std::make_unique(command, id); - } - - SendJSON(llvm::json::Object{ - {"type", "request"}, - {"seq", id}, - {"command", command}, - {"arguments", std::move(arguments)}, + protocol::Id id = Send(protocol::Request{ + command.str(), + std::move(arguments), }); + + std::lock_guard locker(call_mutex); + inflight_reverse_requests[id] = std::make_unique(command, id); } /// The set of capabilities supported by this adapter. diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp index de63c9d78efd1..d67437ad5b3ae 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp @@ -157,11 +157,12 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) { void BaseRequestHandler::Run(const Request &request) { // If this request was cancelled, send a cancelled response. if (dap.IsCancelled(request)) { - Response cancelled{/*request_seq=*/request.seq, - /*command=*/request.command, - /*success=*/false, - /*message=*/eResponseMessageCancelled, - /*body=*/std::nullopt}; + Response cancelled{ + /*request_seq=*/request.seq, + /*command=*/request.command, + /*success=*/false, + /*message=*/eResponseMessageCancelled, + }; dap.Send(cancelled); return; } diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 71e91f8f41a68..2780a5b7748e8 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -10,6 +10,7 @@ #include "DAP.h" #include "ExceptionBreakpoint.h" #include "LLDBUtils.h" +#include "Protocol/ProtocolBase.h" #include "ProtocolUtils.h" #include "lldb/API/SBAddress.h" #include "lldb/API/SBCompileUnit.h" @@ -284,7 +285,7 @@ void FillResponse(const llvm::json::Object &request, // Fill in all of the needed response fields to a "request" and set "success" // to true by default. response.try_emplace("type", "response"); - response.try_emplace("seq", (int64_t)0); + response.try_emplace("seq", protocol::kCalculateSeq); EmplaceSafeString(response, "command", GetString(request, "command").value_or("")); const uint64_t seq = GetInteger(request, "seq").value_or(0); @@ -417,7 +418,7 @@ llvm::json::Value CreateScope(const llvm::StringRef name, // } llvm::json::Object CreateEventObject(const llvm::StringRef event_name) { llvm::json::Object event; - event.try_emplace("seq", 0); + event.try_emplace("seq", protocol::kCalculateSeq); event.try_emplace("type", "event"); EmplaceSafeString(event, "event", event_name); return event; diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp index 9cd9028d879e9..72359214c8537 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp @@ -58,6 +58,8 @@ bool fromJSON(const json::Value &Params, MessageType &M, json::Path P) { } json::Value toJSON(const Request &R) { + assert(R.seq != kCalculateSeq && "invalid seq"); + json::Object Result{ {"type", "request"}, {"seq", R.seq}, @@ -103,8 +105,10 @@ bool operator==(const Request &a, const Request &b) { } json::Value toJSON(const Response &R) { + assert(R.seq != kCalculateSeq && "invalid seq"); + json::Object Result{{"type", "response"}, - {"seq", 0}, + {"seq", R.seq}, {"command", R.command}, {"request_seq", R.request_seq}, {"success", R.success}}; @@ -132,8 +136,9 @@ json::Value toJSON(const Response &R) { return std::move(Result); } -bool fromJSON(json::Value const &Params, - std::variant &M, json::Path P) { +static bool fromJSON(json::Value const &Params, + std::variant &M, + json::Path P) { auto rawMessage = Params.getAsString(); if (!rawMessage) { P.report("expected a string"); @@ -157,8 +162,7 @@ bool fromJSON(json::Value const &Params, Response &R, json::Path P) { return false; MessageType type; - int64_t seq; - if (!O.map("type", type) || !O.map("seq", seq) || + if (!O.map("type", type) || !O.map("seq", R.seq) || !O.map("command", R.command) || !O.map("request_seq", R.request_seq)) return false; @@ -168,12 +172,7 @@ bool fromJSON(json::Value const &Params, Response &R, json::Path P) { } if (R.command.empty()) { - P.field("command").report("expected to not be ''"); - return false; - } - - if (R.request_seq == 0) { - P.field("request_seq").report("expected to not be '0'"); + P.field("command").report("expected to not be empty"); return false; } @@ -217,9 +216,11 @@ bool fromJSON(json::Value const &Params, ErrorMessage &EM, json::Path P) { } json::Value toJSON(const Event &E) { + assert(E.seq != kCalculateSeq && "invalid seq"); + json::Object Result{ {"type", "event"}, - {"seq", 0}, + {"seq", E.seq}, {"event", E.event}, }; @@ -235,8 +236,7 @@ bool fromJSON(json::Value const &Params, Event &E, json::Path P) { return false; MessageType type; - int64_t seq; - if (!O.map("type", type) || !O.map("seq", seq) || !O.map("event", E.event)) + if (!O.map("type", type) || !O.map("seq", E.seq) || !O.map("event", E.event)) return false; if (type != eMessageTypeEvent) { @@ -244,13 +244,8 @@ bool fromJSON(json::Value const &Params, Event &E, json::Path P) { return false; } - if (seq != 0) { - P.field("seq").report("expected to be '0'"); - return false; - } - if (E.event.empty()) { - P.field("event").report("expected to not be ''"); + P.field("event").report("expected to not be empty"); return false; } diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolBase.h b/lldb/tools/lldb-dap/Protocol/ProtocolBase.h index 92e41b1dbf595..42c6c8890af24 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolBase.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolBase.h @@ -30,19 +30,15 @@ namespace lldb_dap::protocol { // MARK: Base Protocol +/// Message unique identifier type. using Id = int64_t; +/// A unique identifier that indicates the `seq` field should be calculated by +/// the current session. +static constexpr Id kCalculateSeq = INT64_MAX; + /// A client or debug adapter initiated request. struct Request { - /// Sequence number of the message (also known as message ID). The `seq` for - /// the first message sent by a client or debug adapter is 1, and for each - /// subsequent message is 1 greater than the previous message sent by that - /// actor. `seq` can be used to order requests, responses, and events, and to - /// associate requests with their corresponding responses. For protocol - /// messages of type `request` the sequence number can be used to cancel the - /// request. - Id seq; - /// The command to execute. std::string command; @@ -50,7 +46,16 @@ struct Request { /// /// Request handlers are expected to validate the arguments, which is handled /// by `RequestHandler`. - std::optional arguments; + std::optional arguments = std::nullopt; + + /// Sequence number of the message (also known as message ID). The `seq` for + /// the first message sent by a client or debug adapter is 1, and for each + /// subsequent message is 1 greater than the previous message sent by that + /// actor. `seq` can be used to order requests, responses, and events, and to + /// associate requests with their corresponding responses. For protocol + /// messages of type `request` the sequence number can be used to cancel the + /// request. + Id seq = kCalculateSeq; }; llvm::json::Value toJSON(const Request &); bool fromJSON(const llvm::json::Value &, Request &, llvm::json::Path); @@ -62,7 +67,16 @@ struct Event { std::string event; /// Event-specific information. - std::optional body; + std::optional body = std::nullopt; + + /// Sequence number of the message (also known as message ID). The `seq` for + /// the first message sent by a client or debug adapter is 1, and for each + /// subsequent message is 1 greater than the previous message sent by that + /// actor. `seq` can be used to order requests, responses, and events, and to + /// associate requests with their corresponding responses. For protocol + /// messages of type `request` the sequence number can be used to cancel the + /// request. + Id seq = kCalculateSeq; }; llvm::json::Value toJSON(const Event &); bool fromJSON(const llvm::json::Value &, Event &, llvm::json::Path); @@ -78,7 +92,7 @@ enum ResponseMessage : unsigned { /// Response for a request. struct Response { /// Sequence number of the corresponding request. - Id request_seq; + Id request_seq = 0; /// The command requested. std::string command; @@ -87,21 +101,31 @@ struct Response { /// attribute may contain the result of the request. If the value is false, /// the attribute `message` contains the error in short form and the `body` /// may contain additional information (see `ErrorMessage`). - bool success; + bool success = false; // FIXME: Migrate usage of fallback string to ErrorMessage /// Contains the raw error in short form if `success` is false. This raw error /// might be interpreted by the client and is not shown in the UI. Some /// predefined values exist. - std::optional> message; + std::optional> message = + std::nullopt; /// Contains request result if success is true and error details if success is /// false. /// /// Request handlers are expected to build an appropriate body, see /// `RequestHandler`. - std::optional body; + std::optional body = std::nullopt; + + /// Sequence number of the message (also known as message ID). The `seq` for + /// the first message sent by a client or debug adapter is 1, and for each + /// subsequent message is 1 greater than the previous message sent by that + /// actor. `seq` can be used to order requests, responses, and events, and to + /// associate requests with their corresponding responses. For protocol + /// messages of type `request` the sequence number can be used to cancel the + /// request. + Id seq = kCalculateSeq; }; bool fromJSON(const llvm::json::Value &, Response &, llvm::json::Path); llvm::json::Value toJSON(const Response &);