Add JSONGenerator::DumpBinaryEscaped method in debugserver, update most callers to use this, skip escaping step

[jasonmolenda]

Summary

Most packets which return JSON use JSONGenerator::Dump to print the string representation of the data, and then pass that string to another method to escape the #, $, }, and * characters which have special meaning in the gdb remote serial protocol. This generate-then-escape two step process involves duplicating the entire string, and for large data, can result in a lot of memory use.

This patch adds JSONGenerator::DumpBinaryEscaped methods for those cases where this is needed. The Dictionary, Array, and String methods involve duplicating the existing Dump methods, mostly to call the corresponding DumpBinaryEscaped methods on their children. The other JSON types do not need escaping, and call the regular Dump methods.

This is the follow on patch to https://reviews.llvm.org/D122848 , the two of them get us down from 3 string copies being held in heap, plus the JSON object tree, to the JSON object tree and one copy. For a large list of shared libraries, it can add up to quite a bit of memory use, and debugserver should try to be low impact on the system.

[JDevlieghere]

lldb/tools/debugserver/source/JSONGenerator.h

251–265

Can we avoid the quoted string and write directly to the stream?

[jasonmolenda]

Update patch to remove unnecessary temporary std::string when dumping a string into its JSON representation.

lldb/tools/debugserver/source/JSONGenerator.h

public:
     Generic *GetAsGeneric() {
       if (m_type == Type::eTypeGeneric)
         return (Generic *)this;
       return NULL;
     }
 
     virtual void Dump(std::ostream &s) const = 0;
 
+    virtual void DumpBinaryEscaped(std::ostream &s) const = 0;
+
   private:
     Type m_type;
   };
 
   class Array : public Object {
   public:
     Array() : Object(Type::eTypeArray) {}
 
     virtual ~Array() {}
 
     void AddItem(ObjectSP item) { m_items.push_back(item); }
 
     void Dump(std::ostream &s) const override {
       s << "[";
       const size_t arrsize = m_items.size();
       for (size_t i = 0; i < arrsize; ++i) {
         m_items[i]->Dump(s);
         if (i + 1 < arrsize)
           s << ",";
       }
       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;
   };
 
   class Integer : public Object {
   public:
     Integer(uint64_t value = 0) : Object(Type::eTypeInteger), m_value(value) {}
 
     virtual ~Integer() {}
 
     void SetValue(uint64_t value) { m_value = value; }
 
     void Dump(std::ostream &s) const override { s << m_value; }
 
+    void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
   protected:
     uint64_t m_value;
   };
 
   class Float : public Object {
   public:
     Float(double d = 0.0) : Object(Type::eTypeFloat), m_value(d) {}
 
     virtual ~Float() {}
 
     void SetValue(double value) { m_value = value; }
 
     void Dump(std::ostream &s) const override { s << m_value; }
 
+    void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
   protected:
     double m_value;
   };
 
   class Boolean : public Object {
   public:
     Boolean(bool b = false) : Object(Type::eTypeBoolean), m_value(b) {}
 
     virtual ~Boolean() {}
 
     void SetValue(bool value) { m_value = value; }
 
     void Dump(std::ostream &s) const override {
       if (m_value)
         s << "true";
       else
         s << "false";
     }
 
+    void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
   protected:
     bool m_value;
   };
 
   class String : public Object {
   public:
     String() : Object(Type::eTypeString), m_value() {}
 
     String(const std::string &s) : Object(Type::eTypeString), m_value(s) {}
 
     String(const std::string &&s) : Object(Type::eTypeString), m_value(s) {}
 
     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:
     std::string m_value;
   };
 
   class Dictionary : public Object {
   public:
     Dictionary() : Object(Type::eTypeDictionary), m_dict() {}
 
     virtual ~Dictionary() {}
 
     void AddItem(std::string key, ObjectSP value) {
       m_dict.push_back(Pair(key, value));
     }
 
     void AddIntegerItem(std::string key, uint64_t value) {
       AddItem(key, ObjectSP(new Integer(value)));
     }
 
     void AddFloatItem(std::string key, double value) {
       AddItem(key, ObjectSP(new Float(value)));
     }
 
     void AddStringItem(std::string key, std::string value) {
       AddItem(key, ObjectSP(new String(std::move(value))));
void Dump(std::ostream &s) const override {
           s << ",";
         }
         s << "\"" << iter->first.c_str() << "\":";
         iter->second->Dump(s);
       }
       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
     typedef std::pair<std::string, ObjectSP> Pair;
     typedef std::vector<Pair> collection;
     collection m_dict;
   };
 
   class Null : public Object {
   public:
     Null() : Object(Type::eTypeNull) {}
 
     virtual ~Null() {}
 
     bool IsValid() const override { return false; }
 
     void Dump(std::ostream &s) const override { s << "null"; }
 
+    void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
   protected:
   };
 
   class Generic : public Object {
   public:
     explicit Generic(void *object = nullptr)
         : Object(Type::eTypeGeneric), m_object(object) {}
 
     void SetValue(void *value) { m_object = value; }
 
     void *GetValue() const { return m_object; }
 
     bool IsValid() const override { return m_object != nullptr; }
 
     void Dump(std::ostream &s) const override;
 
+    void DumpBinaryEscaped(std::ostream &s) const override;
+
   private:
     void *m_object;
   };
 
 }; // class JSONGenerator
 
 #endif // LLDB_TOOLS_DEBUGSERVER_SOURCE_JSONGENERATOR_H

lldb/tools/debugserver/source/RNBRemote.cpp

 
 rnb_err_t
 RNBRemote::SendAsyncJSONPacket(const JSONGenerator::Dictionary &dictionary) {
   std::ostringstream stream;
   // We're choosing something that is easy to spot if we somehow get one
   // 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.
 // If compression is enabled, the returned std::string will be in one of two
 // forms:
 //
 //    N<original packet contents uncompressed>
 //    C<size of original decompressed packet>:<packet compressed with the
if (m_list_threads_in_stop_reply) {
       if (numthreads > 1) {
         const bool threads_with_valid_stop_info_only = true;
         JSONGenerator::ObjectSP threads_info_sp =
             GetJSONThreadsInfo(threads_with_valid_stop_info_only);
         if (threads_info_sp) {
           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 << ';';
         }
       }
     }
 
     if (g_num_reg_entries == 0)
       InitializeRegisters();
rnb_err_t RNBRemote::HandlePacket_jThreadsInfo(const char *p) {
   // If we haven't run the process yet, return an error.
   if (m_ctx.HasValidProcessID()) {
     const bool threads_with_valid_stop_info_only = false;
     JSONGenerator::ObjectSP threads_info_sp =
         GetJSONThreadsInfo(threads_with_valid_stop_info_only);
 
     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");
 }
 
 rnb_err_t RNBRemote::HandlePacket_jThreadExtendedInfo(const char *p) {
   nub_process_t pid;
   std::ostringstream json;
if (get_boolean_value_for_key_name_from_json("fetch_all_solibs", p,
           image_count != INVALID_NUB_ADDRESS) {
         json_sp = DNBGetLoadedDynamicLibrariesInfos(pid, image_list_address,
                                                     image_count);
       }
     }
 
     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");
       }
     }
   }
   return SendPacket("OK");
 }
 
rnb_err_t RNBRemote::HandlePacket_jGetSharedCacheInfo(const char *p) {
 
   const char get_shared_cache_info_str[] = {"jGetSharedCacheInfo:{"};
   if (strncmp(p, get_shared_cache_info_str,
               sizeof(get_shared_cache_info_str) - 1) == 0) {
     JSONGenerator::ObjectSP json_sp = DNBGetSharedCacheInfo(pid);
 
     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");
       }
     }
   }
   return SendPacket("OK");
 }