Add DumpBinaryEscaped method to JSONGenerator, avoid extra copy

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

Author: jasonmolenda

lldb/tools/debugserver/source/JSONGenerator.h

@@ -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;
   };
@@ -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;
@@ -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;
   };
@@ -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;
   };
@@ -184,6 +201,8 @@ class JSONGenerator {
         s << "false";
     }
 
+    void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
   protected:
     bool m_value;
   };
@@ -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:
@@ -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
@@ -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:
   };
 
@@ -304,6 +379,8 @@ class JSONGenerator {
 
     void Dump(std::ostream &s) const override;
 
+    void DumpBinaryEscaped(std::ostream &s) const override;
+
   private:
     void *m_object;
   };

lldb/tools/debugserver/source/RNBRemote.cpp

@@ -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.
@@ -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 << ';';
         }
@@ -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");
@@ -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");
       }
@@ -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");
       }