async structured data packet handling improvements
This change does the following:
* Changes the signature for the continuation delegate method that handles
async structured data from accepting an already-parsed structured data
element to taking just the packet contents.
* Moves the conversion of the JSON-async: packet contents from
GDBRemoteClientBase to the continuation delegate method.
* Adds a new unit test for verifying that the $JSON-asyc: packets get
decoded and that the decoded packets get forwarded on to the delegate
for further processing. Thanks to Pavel for making that whole section of
code easily unit testable!
* Tightens up the packet verification on reception of a $JSON-async:
packet contents. The code prior to this change is susceptible to a
segfault if a packet is carefully crafted that starts with $J but
has a total length shorter than the length of "$JSON-async:".
Reviewers: labath, clayborg, zturner
Differential Revision: https://reviews.llvm.org/D23884
llvm-svn: 281121
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
index 3b27c10..853e201 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
@@ -103,38 +103,9 @@
delegate.HandleAsyncMisc(
llvm::StringRef(response.GetStringRef()).substr(1));
break;
-
case 'J':
- // Asynchronous JSON packet, destined for a
- // StructuredDataPlugin.
- {
- // Parse the content into a StructuredData instance.
- auto payload_index = strlen("JSON-async:");
- StructuredData::ObjectSP json_sp = StructuredData::ParseJSON(
- response.GetStringRef().substr(payload_index));
- if (log) {
- if (json_sp)
- log->Printf("GDBRemoteCommmunicationClientBase::%s() "
- "received Async StructuredData packet: %s",
- __FUNCTION__,
- response.GetStringRef().substr(payload_index).c_str());
- else
- log->Printf("GDBRemoteCommmunicationClientBase::%s"
- "() received StructuredData packet:"
- " parse failure",
- __FUNCTION__);
- }
-
- // Pass the data to the process to route to the
- // appropriate plugin. The plugin controls what happens
- // to it from there.
- bool routed = delegate.HandleAsyncStructuredData(json_sp);
- if (log)
- log->Printf("GDBRemoteCommmunicationClientBase::%s()"
- " packet %s",
- __FUNCTION__, routed ? "handled" : "not handled");
- break;
- }
+ delegate.HandleAsyncStructuredDataPacket(response.GetStringRef());
+ break;
case 'T':
case 'S':
// Do this with the continue lock held.
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
index f392895..15d7ec2 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
@@ -25,14 +25,13 @@
virtual void HandleAsyncMisc(llvm::StringRef data) = 0;
virtual void HandleStopReply() = 0;
- //
- /// Processes async structured data.
+ // =========================================================================
+ /// Process asynchronously-received structured data.
///
- /// @return
- /// true if the data was handled; otherwise, false.
- //
- virtual bool
- HandleAsyncStructuredData(const StructuredData::ObjectSP &object_sp) = 0;
+ /// @param[in] data
+ /// The complete data packet, expected to start with JSON-async.
+ // =========================================================================
+ virtual void HandleAsyncStructuredDataPacket(llvm::StringRef data) = 0;
};
GDBRemoteClientBase(const char *comm_name, const char *listener_name);
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index e64ce50..504aa4f 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4808,9 +4808,49 @@
BuildDynamicRegisterInfo(true);
}
-bool ProcessGDBRemote::HandleAsyncStructuredData(
- const StructuredData::ObjectSP &object_sp) {
- return RouteAsyncStructuredData(object_sp);
+static const char *const s_async_json_packet_prefix = "JSON-async:";
+
+static StructuredData::ObjectSP
+ParseStructuredDataPacket(llvm::StringRef packet) {
+ Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
+
+ if (!packet.consume_front(s_async_json_packet_prefix)) {
+ if (log) {
+ log->Printf(
+ "GDBRemoteCommmunicationClientBase::%s() received $J packet "
+ "but was not a StructuredData packet: packet starts with "
+ "%s",
+ __FUNCTION__,
+ packet.slice(0, strlen(s_async_json_packet_prefix)).str().c_str());
+ }
+ return StructuredData::ObjectSP();
+ }
+
+ // This is an asynchronous JSON packet, destined for a
+ // StructuredDataPlugin.
+ StructuredData::ObjectSP json_sp = StructuredData::ParseJSON(packet);
+ if (log) {
+ if (json_sp) {
+ StreamString json_str;
+ json_sp->Dump(json_str);
+ json_str.Flush();
+ log->Printf("ProcessGDBRemote::%s() "
+ "received Async StructuredData packet: %s",
+ __FUNCTION__, json_str.GetString().c_str());
+ } else {
+ log->Printf("ProcessGDBRemote::%s"
+ "() received StructuredData packet:"
+ " parse failure",
+ __FUNCTION__);
+ }
+ }
+ return json_sp;
+}
+
+void ProcessGDBRemote::HandleAsyncStructuredDataPacket(llvm::StringRef data) {
+ auto structured_data_sp = ParseStructuredDataPacket(data);
+ if (structured_data_sp)
+ RouteAsyncStructuredData(structured_data_sp);
}
class CommandObjectProcessGDBRemoteSpeedTest : public CommandObjectParsed {
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
index 493176d..d607d35 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -415,8 +415,7 @@
void HandleAsyncStdout(llvm::StringRef out) override;
void HandleAsyncMisc(llvm::StringRef data) override;
void HandleStopReply() override;
- bool
- HandleAsyncStructuredData(const StructuredData::ObjectSP &object_sp) override;
+ void HandleAsyncStructuredDataPacket(llvm::StringRef data) override;
using ModuleCacheKey = std::pair<std::string, std::string>;
// KeyInfo for the cached module spec DenseMap.