[Lldb-commits] [lldb] r281121 - async structured data packet handling improvements
Todd Fiala via lldb-commits
lldb-commits at lists.llvm.org
Fri Sep 9 17:06:30 PDT 2016
Author: tfiala
Date: Fri Sep 9 19:06:29 2016
New Revision: 281121
URL: http://llvm.org/viewvc/llvm-project?rev=281121&view=rev
Log:
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
Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp?rev=281121&r1=281120&r2=281121&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp Fri Sep 9 19:06:29 2016
@@ -103,38 +103,9 @@ StateType GDBRemoteClientBase::SendConti
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.
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h?rev=281121&r1=281120&r2=281121&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h Fri Sep 9 19:06:29 2016
@@ -25,14 +25,13 @@ public:
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);
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=281121&r1=281120&r2=281121&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Fri Sep 9 19:06:29 2016
@@ -4808,9 +4808,49 @@ void ProcessGDBRemote::HandleStopReply()
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 {
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h?rev=281121&r1=281120&r2=281121&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h Fri Sep 9 19:06:29 2016
@@ -415,8 +415,7 @@ private:
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.
Modified: lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp?rev=281121&r1=281120&r2=281121&view=diff
==============================================================================
--- lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp (original)
+++ lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp Fri Sep 9 19:06:29 2016
@@ -20,6 +20,7 @@
#include "Plugins/Process/Utility/LinuxSignals.h"
#include "Plugins/Process/gdb-remote/GDBRemoteClientBase.h"
#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h"
+#include "lldb/Core/StreamGDBRemote.h"
#include "llvm/ADT/STLExtras.h"
@@ -34,14 +35,14 @@ struct MockDelegate : public GDBRemoteCl
std::string output;
std::string misc_data;
unsigned stop_reply_called = 0;
+ std::vector<std::string> structured_data_packets;
void HandleAsyncStdout(llvm::StringRef out) { output += out; }
void HandleAsyncMisc(llvm::StringRef data) { misc_data += data; }
void HandleStopReply() { ++stop_reply_called; }
- bool HandleAsyncStructuredData(const StructuredData::ObjectSP &object_sp) {
- // TODO work in a test here after I fix the gtest breakage.
- return true;
+ void HandleAsyncStructuredDataPacket(llvm::StringRef data) {
+ structured_data_packets.push_back(data);
}
};
@@ -321,6 +322,37 @@ TEST_F(GDBRemoteClientBaseTest, SendCont
EXPECT_EQ(1u, fix.delegate.stop_reply_called);
}
+TEST_F(GDBRemoteClientBaseTest, SendContinueDelegateStructuredDataReceipt) {
+ // Build the plain-text version of the JSON data we will have the
+ // server send.
+ const std::string json_payload =
+ "{ \"type\": \"MyFeatureType\", "
+ " \"elements\": [ \"entry1\", \"entry2\" ] }";
+ const std::string json_packet = "JSON-async:" + json_payload;
+
+ // Escape it properly for transit.
+ StreamGDBRemote stream;
+ stream.PutEscapedBytes(json_packet.c_str(), json_packet.length());
+ stream.Flush();
+
+ // Set up the
+ StringExtractorGDBRemote response;
+ ContinueFixture fix;
+ if (HasFailure())
+ return;
+
+ // Send async structured data packet, then stop.
+ ASSERT_EQ(PacketResult::Success, fix.server.SendPacket(stream.GetData()));
+ ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T01"));
+ ASSERT_EQ(eStateStopped, fix.SendCPacket(response));
+ ASSERT_EQ("T01", response.GetStringRef());
+ ASSERT_EQ(1, fix.delegate.structured_data_packets.size());
+
+ // Verify the packet contents. It should have been unescaped upon packet
+ // reception.
+ ASSERT_EQ(json_packet, fix.delegate.structured_data_packets[0]);
+}
+
TEST_F(GDBRemoteClientBaseTest, InterruptNoResponse) {
StringExtractorGDBRemote continue_response, response;
ContinueFixture fix;
More information about the lldb-commits
mailing list