tfiala created this revision.
tfiala added reviewers: clayborg, labath, jasonmolenda.
tfiala added a subscriber: lldb-commits.
This change does the following:
* Adds three new unit test entries to test handling of $JSON-asyc: packets.
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.
* Changes the StructuredData JSON parsing method to eliminate an unnecessary
string copy and instead take a const char*. Fixes up call sites.
* Xcode project: adds a needed TERM env var to the running portion of the gtest
debugging scheme.
https://reviews.llvm.org/D23884
Files:
include/lldb/Core/StructuredData.h
source/Core/StructuredData.cpp
source/Host/macosx/Host.mm
source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
Index: unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
===================================================================
--- unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
+++ unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
@@ -17,6 +17,7 @@
#include "GDBRemoteTestUtils.h"
#include "gtest/gtest.h"
+#include "lldb/Core/StreamGDBRemote.h"
#include "Plugins/Process/Utility/LinuxSignals.h"
#include "Plugins/Process/gdb-remote/GDBRemoteClientBase.h"
#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h"
@@ -30,12 +31,14 @@
namespace
{
-
+using StructuredDataVector = std::vector<StructuredData::ObjectSP>;
+
struct MockDelegate : public GDBRemoteClientBase::ContinueDelegate
{
std::string output;
std::string misc_data;
unsigned stop_reply_called = 0;
+ StructuredDataVector structured_data_entries;
void
HandleAsyncStdout(llvm::StringRef out)
@@ -57,8 +60,9 @@
HandleAsyncStructuredData(const StructuredData::ObjectSP
&object_sp)
{
- // TODO work in a test here after I fix the gtest breakage.
- return true;
+ if (object_sp)
+ structured_data_entries.push_back(object_sp);
+ return object_sp != nullptr;
}
};
@@ -339,6 +343,111 @@
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_data =
+ "{ \"type\": \"MyFeatureType\", "
+ " \"elements\": [ \"entry1\", \"entry2\" ] }";
+
+ // Escape it properly for transit.
+ StreamGDBRemote stream;
+ stream.PutCString("JSON-async:");
+ stream.PutEscapedBytes(json_data.c_str(), json_data.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(fix.delegate.structured_data_entries.size(), 1);
+
+ // Verify the structured data.
+ auto object_sp = fix.delegate.structured_data_entries[0];
+ ASSERT_NE(object_sp, nullptr);
+
+ auto dictionary = object_sp->GetAsDictionary();
+ ASSERT_NE(dictionary, nullptr);
+
+ std::string type_value;
+ dictionary->GetValueForKeyAsString("type", type_value);
+ ASSERT_EQ(type_value, "MyFeatureType");
+
+ StructuredData::Array *elements = nullptr;
+ dictionary->GetValueForKeyAsArray("elements", elements);
+ ASSERT_NE(elements, nullptr);
+ ASSERT_EQ(elements->GetSize(), 2);
+
+ std::string element_value;
+ elements->GetItemAtIndexAsString(0, element_value);
+ ASSERT_EQ(element_value, "entry1");
+
+ elements->GetItemAtIndexAsString(1, element_value);
+ ASSERT_EQ(element_value, "entry2");
+}
+
+TEST_F(GDBRemoteClientBaseTest, SendContinueDelegateJPacketStructuredDataBadPrefix)
+{
+ // Build the plain-text version of the JSON data we will have the
+ // server send.
+ const std::string json_data =
+ "{ \"type\": \"MyFeatureType\", "
+ " \"elements\": [ \"entry1\", \"entry2\" ] }";
+
+ // Escape it properly for transit.
+ StreamGDBRemote stream;
+ stream.PutCString("Jfoo:");
+ stream.PutEscapedBytes(json_data.c_str(), json_data.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(fix.delegate.structured_data_entries.size(), 0);
+}
+
+TEST_F(GDBRemoteClientBaseTest, SendContinueDelegateJPacketStructuredDataBadJSON)
+{
+ // Build the plain-text version of the JSON data we will have the
+ // server send.
+ const std::string json_data = "{ \"type\": \"MyFeatureType\", ";
+
+ // Escape it properly for transit.
+ StreamGDBRemote stream;
+ stream.PutCString("JSON-async:");
+ stream.PutEscapedBytes(json_data.c_str(), json_data.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(fix.delegate.structured_data_entries.size(), 0);
+}
+
TEST_F(GDBRemoteClientBaseTest, InterruptNoResponse)
{
StringExtractorGDBRemote continue_response, response;
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2523,7 +2523,7 @@
// This JSON contains thread IDs and thread stop info for all threads.
// It doesn't contain expedited registers, memory or queue info.
- m_jstopinfo_sp = StructuredData::ParseJSON (value);
+ m_jstopinfo_sp = StructuredData::ParseJSON (value.c_str());
}
else if (key.compare("hexname") == 0)
{
@@ -4229,7 +4229,7 @@
{
if (!response.Empty())
{
- object_sp = StructuredData::ParseJSON (response.GetStringRef());
+ object_sp = StructuredData::ParseJSON (response.GetStringRef().c_str());
}
}
}
@@ -4305,7 +4305,7 @@
{
if (!response.Empty())
{
- object_sp = StructuredData::ParseJSON (response.GetStringRef());
+ object_sp = StructuredData::ParseJSON (response.GetStringRef().c_str());
}
}
}
@@ -4344,7 +4344,7 @@
{
if (!response.Empty())
{
- object_sp = StructuredData::ParseJSON (response.GetStringRef());
+ object_sp = StructuredData::ParseJSON (response.GetStringRef().c_str());
}
}
}
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -662,7 +662,7 @@
}
else if (!response.Empty())
{
- object_sp = StructuredData::ParseJSON (response.GetStringRef());
+ object_sp = StructuredData::ParseJSON (response.GetStringRef().c_str());
}
}
}
@@ -2749,7 +2749,7 @@
if (SendPacketAndWaitForResponse("qQueryGDBServer", response, false) != PacketResult::Success)
return 0;
- StructuredData::ObjectSP data = StructuredData::ParseJSON(response.GetStringRef());
+ StructuredData::ObjectSP data = StructuredData::ParseJSON(response.GetStringRef().c_str());
if (!data)
return 0;
@@ -3978,7 +3978,7 @@
send_async) == PacketResult::Success)
{
m_supported_async_json_packets_sp = StructuredData::ParseJSON(
- response.GetStringRef());
+ response.GetStringRef().c_str());
if (m_supported_async_json_packets_sp &&
!m_supported_async_json_packets_sp->GetAsArray())
{
Index: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
@@ -23,6 +23,13 @@
static const std::chrono::seconds kInterruptTimeout(5);
+static const std::string &
+GetStructuredDataPacketPrefix()
+{
+ static const std::string prefix("JSON-async:");
+ return prefix;
+}
+
/////////////////////////
// GDBRemoteClientBase //
/////////////////////////
@@ -103,23 +110,44 @@
break;
case 'J':
- // Asynchronous JSON packet, destined for a
- // StructuredDataPlugin.
{
+ // Verify this J packet is a JSON-async: packet.
+ const std::string &expected_prefix =
+ GetStructuredDataPacketPrefix();
+ const std::string &packet = response.GetStringRef();
+ const std::string packet_prefix =
+ packet.substr(0, expected_prefix.length());
+ if (packet_prefix != expected_prefix)
+ {
+ if (log)
+ {
+ log->Printf("GDBRemoteCommmunicationClientBase::%s() "
+ "received $J packet but was not a "
+ "StructuredData packet: packet starts with "
+ "%s",
+ __FUNCTION__,
+ packet_prefix.c_str());
+ }
+ break;
+ }
+
+ // This is an asynchronous JSON packet, destined for a
+ // StructuredDataPlugin.
+
// Parse the content into a StructuredData instance.
- auto payload_index = strlen("JSON-async:");
+ const char *const encoded_json =
+ packet.c_str() + expected_prefix.length();
+
StructuredData::ObjectSP json_sp =
- StructuredData::ParseJSON(response.GetStringRef()
- .substr(payload_index));
+ StructuredData::ParseJSON(encoded_json);
if (log)
{
if (json_sp)
log->Printf(
"GDBRemoteCommmunicationClientBase::%s() "
"received Async StructuredData packet: %s",
__FUNCTION__,
- response.GetStringRef().
- substr(payload_index).c_str());
+ encoded_json);
else
log->Printf("GDBRemoteCommmunicationClientBase::%s"
"() received StructuredData packet:"
Index: source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
===================================================================
--- source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -891,7 +891,7 @@
response.GetResponseType() != response.eResponse)
return m_remote_signals_sp;
- auto object_sp = StructuredData::ParseJSON(response.GetStringRef());
+ auto object_sp = StructuredData::ParseJSON(response.GetStringRef().c_str());
if (!object_sp || !object_sp->IsValid())
return m_remote_signals_sp;
Index: source/Host/macosx/Host.mm
===================================================================
--- source/Host/macosx/Host.mm
+++ source/Host/macosx/Host.mm
@@ -1400,7 +1400,7 @@
return error;
}
- auto data_sp = StructuredData::ParseJSON(output);
+ auto data_sp = StructuredData::ParseJSON(output.c_str());
if (!data_sp)
{
error.SetErrorString("invalid JSON");
Index: source/Core/StructuredData.cpp
===================================================================
--- source/Core/StructuredData.cpp
+++ source/Core/StructuredData.cpp
@@ -152,10 +152,15 @@
}
StructuredData::ObjectSP
-StructuredData::ParseJSON (std::string json_text)
+StructuredData::ParseJSON (const char *json_text)
{
- JSONParser json_parser(json_text.c_str());
- StructuredData::ObjectSP object_sp = ParseJSONValue(json_parser);
+ StructuredData::ObjectSP object_sp;
+
+ if (json_text && json_text[0])
+ {
+ JSONParser json_parser(json_text);
+ object_sp = ParseJSONValue(json_parser);
+ }
return object_sp;
}
Index: include/lldb/Core/StructuredData.h
===================================================================
--- include/lldb/Core/StructuredData.h
+++ include/lldb/Core/StructuredData.h
@@ -752,7 +752,7 @@
};
static ObjectSP
- ParseJSON (std::string json_text);
+ ParseJSON (const char *json_text);
};
} // namespace lldb_private
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
