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
This commit is contained in:
Todd Fiala 2016-09-10 00:06:29 +00:00
parent 99b555709e
commit fcdb1af655
5 changed files with 87 additions and 46 deletions

View File

@ -103,38 +103,9 @@ StateType GDBRemoteClientBase::SendContinuePacketAndWaitForResponse(
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.

View File

@ -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);

View File

@ -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 {

View File

@ -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.

View File

@ -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 GDBRemoteClientBase::ContinueDelegate {
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, SendContinueDelegateInterface) {
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;