Skip to content

Commit

Permalink
Add DumpBinaryEscaped method to JSONGenerator, avoid extra copy
Browse files Browse the repository at this point in the history 
All uses of JSONGenerator in debugserver would create a JSON text
dump of the object collection, then copy that string into a
binary-escaped string, then send it up to the lldb side or
make a compressed version and send that.

This adds a DumpBinaryEscaped method to JSONGenerator which
does the gdb remote serial protocol binary escaping directly,
and removes the need to pass over the string and have an
additional copy in memory.

Differential Revision: https://reviews.llvm.org/D122882
rdar://91117456
  • Loading branch information
@jasonmolenda
jasonmolenda committed Apr 4, 2022
1 parent 686406a commit 7ebcd88
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 17 deletions.
85 changes: 81 additions & 4 deletions lldb/tools/debugserver/source/JSONGenerator.h
View file
Expand Up @@ -113,6 +113,8 @@ class JSONGenerator {

virtual void Dump(std::ostream &s) const = 0;

virtual void DumpBinaryEscaped(std::ostream &s) const = 0;

private:
Type m_type;
};
Expand All @@ -136,6 +138,17 @@ class JSONGenerator {
s << "]";
}

void DumpBinaryEscaped(std::ostream &s) const override {
s << "[";
const size_t arrsize = m_items.size();
for (size_t i = 0; i < arrsize; ++i) {
m_items[i]->DumpBinaryEscaped(s);
if (i + 1 < arrsize)
s << ",";
}
s << "]";
}

protected:
typedef std::vector<ObjectSP> collection;
collection m_items;
Expand All @@ -151,6 +164,8 @@ class JSONGenerator {

void Dump(std::ostream &s) const override { s << m_value; }

void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }

protected:
uint64_t m_value;
};
Expand All @@ -165,6 +180,8 @@ class JSONGenerator {

void Dump(std::ostream &s) const override { s << m_value; }

void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }

protected:
double m_value;
};
Expand All @@ -184,6 +201,8 @@ class JSONGenerator {
s << "false";
}

void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }

protected:
bool m_value;
};
Expand All @@ -199,15 +218,33 @@ class JSONGenerator {
void SetValue(const std::string &string) { m_value = string; }

void Dump(std::ostream &s) const override {
std::string quoted;
s << '"';
const size_t strsize = m_value.size();
for (size_t i = 0; i < strsize; ++i) {
char ch = m_value[i];
if (ch == '"')
s << '\\';
s << ch;
}
s << '"';
}

void DumpBinaryEscaped(std::ostream &s) const override {
s << '"';
const size_t strsize = m_value.size();
for (size_t i = 0; i < strsize; ++i) {
char ch = m_value[i];
if (ch == '"')
quoted.push_back('\\');
quoted.push_back(ch);
s << '\\';
// gdb remote serial protocol binary escaping
if (ch == '#' || ch == '$' || ch == '}' || ch == '*') {
s << '}'; // 0x7d next character is escaped
s << static_cast<char>(ch ^ 0x20);
} else {
s << ch;
}
}
s << '"' << quoted.c_str() << '"';
s << '"';
}

protected:
Expand Down Expand Up @@ -269,7 +306,43 @@ class JSONGenerator {
s << "}";
}

void DumpBinaryEscaped(std::ostream &s) const override {
bool have_printed_one_elem = false;
s << "{";
for (collection::const_iterator iter = m_dict.begin();
iter != m_dict.end(); ++iter) {
if (!have_printed_one_elem) {
have_printed_one_elem = true;
} else {
s << ",";
}
s << "\"" << binary_encode_string(iter->first) << "\":";
iter->second->DumpBinaryEscaped(s);
}
// '}' must be escaped for the gdb remote serial
// protocol.
s << "}";
s << static_cast<char>('}' ^ 0x20);
}

protected:
std::string binary_encode_string(const std::string &s) const {
std::string output;
const size_t s_size = s.size();
const char *s_chars = s.c_str();

for (size_t i = 0; i < s_size; i++) {
unsigned char ch = *(s_chars + i);
if (ch == '#' || ch == '$' || ch == '}' || ch == '*') {
output.push_back('}'); // 0x7d
output.push_back(ch ^ 0x20);
} else {
output.push_back(ch);
}
}
return output;
}

// Keep the dictionary as a vector so the dictionary doesn't reorder itself
// when you dump it
// We aren't accessing keys by name, so this won't affect performance
Expand All @@ -288,6 +361,8 @@ class JSONGenerator {

void Dump(std::ostream &s) const override { s << "null"; }

void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }

protected:
};

Expand All @@ -304,6 +379,8 @@ class JSONGenerator {

void Dump(std::ostream &s) const override;

void DumpBinaryEscaped(std::ostream &s) const override;

private:
void *m_object;
};
Expand Down
23 changes: 10 additions & 13 deletions lldb/tools/debugserver/source/RNBRemote.cpp
View file
Expand Up @@ -582,9 +582,8 @@ RNBRemote::SendAsyncJSONPacket(const JSONGenerator::Dictionary &dictionary) {
// of these coming out at the wrong time (i.e. when the remote side
// is not waiting for a process control completion response).
stream << "JSON-async:";
dictionary.Dump(stream);
const std::string payload = binary_encode_string(stream.str());
return SendPacket(payload);
dictionary.DumpBinaryEscaped(stream);
return SendPacket(stream.str());
}

// Given a std::string packet contents to send, possibly encode/compress it.
Expand Down Expand Up @@ -2793,6 +2792,7 @@ rnb_err_t RNBRemote::SendStopReplyPacketForThread(nub_thread_t tid) {
ostrm << std::hex << "jstopinfo:";
std::ostringstream json_strm;
threads_info_sp->Dump(json_strm);
threads_info_sp->Clear();
append_hexified_string(ostrm, json_strm.str());
ostrm << ';';
}
Expand Down Expand Up @@ -5556,11 +5556,10 @@ rnb_err_t RNBRemote::HandlePacket_jThreadsInfo(const char *p) {

if (threads_info_sp) {
std::ostringstream strm;
threads_info_sp->Dump(strm);
threads_info_sp->DumpBinaryEscaped(strm);
threads_info_sp->Clear();
std::string binary_packet = binary_encode_string(strm.str());
if (!binary_packet.empty())
return SendPacket(binary_packet);
if (strm.str().size() > 0)
return SendPacket(strm.str());
}
}
return SendPacket("E85");
Expand Down Expand Up @@ -5881,11 +5880,10 @@ RNBRemote::HandlePacket_jGetLoadedDynamicLibrariesInfos(const char *p) {

if (json_sp.get()) {
std::ostringstream json_str;
json_sp->Dump(json_str);
json_sp->DumpBinaryEscaped(json_str);
json_sp->Clear();
if (json_str.str().size() > 0) {
std::string json_str_quoted = binary_encode_string(json_str.str());
return SendPacket(json_str_quoted);
return SendPacket(json_str.str());
} else {
SendPacket("E84");
}
Expand Down Expand Up @@ -5915,11 +5913,10 @@ rnb_err_t RNBRemote::HandlePacket_jGetSharedCacheInfo(const char *p) {

if (json_sp.get()) {
std::ostringstream json_str;
json_sp->Dump(json_str);
json_sp->DumpBinaryEscaped(json_str);
json_sp->Clear();
if (json_str.str().size() > 0) {
std::string json_str_quoted = binary_encode_string(json_str.str());
return SendPacket(json_str_quoted);
return SendPacket(json_str.str());
} else {
SendPacket("E86");
}
Expand Down

0 comments on commit 7ebcd88

Please sign in to comment.