[JSON] Use LLVM's library for encoding JSON in GDBRemoteCommunicationServerLLGS

[JDevlieghere]

Summary

This patch replaces the LLDB's JSON implementation with the one from
LLVM in GDBRemoteCommunicationServerLLGS.

[labath]

Cool. Thanks for doing this.

[rupprecht]

FYI, we've root caused some internal lldb asan failures to this patch. I don't have a repro yet (my lldb tests seem to be failing locally for unrelated reasons), but e.g. Register/x86-64-read.test is failing to read all the non-general purpose registers:

(lldb) register read --all
General Purpose Registers:
       rax = 0x00007fffffffd660
       ... etc. ...

Floating Point Registers:
42 registers were unavailable.

Advanced Vector Extensions:
16 registers were unavailable.

Memory Protection Extensions:
6 registers were unavailable.

(lldb) process continue
error: Failed to resume process: Resume timed out..

These tests are passing in all other modes, including other sanitizer modes

[labath]

FYI, we've root caused some internal lldb asan failures to this patch. I don't have a repro yet (my lldb tests seem to be failing locally for unrelated reasons)

This should be fixed with r373572.

BTW, are you sure those reasons were unrelated? If you have had nearly every test failing then it was very likely due to this. (The reason only a couple of tests failed for you in asan mode is because our CI does not run a large chunk of the lldb tests... :/ )

[rupprecht]

In D68299#1692503, @labath wrote:

FYI, we've root caused some internal lldb asan failures to this patch. I don't have a repro yet (my lldb tests seem to be failing locally for unrelated reasons)

This should be fixed with r373572.

Thanks!

BTW, are you sure those reasons were unrelated? If you have had nearly every test failing then it was very likely due to this. (The reason only a couple of tests failed for you in asan mode is because our CI does not run a large chunk of the lldb tests... :/ )

Yes, tests are still failing locally for me, e.g. Driver/TestConvenienceVariables.test in ninja check-lldb-driver fails:

Command Output (stderr):
--
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: No module named lldb.embedded_interpreter
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'run_one_line' is not defined
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'run_one_line' is not defined
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'run_one_line' is not defined
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'run_one_line' is not defined
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'run_one_line' is not defined
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name 'run_one_line' is not defined
error: python failed attempting to evaluate 'print(lldb.debugger)'

I think it's a python-related misconfiguration somewhere about not finding the lldb module, but I'm not sure when it started -- these tests used to pass, but I nuked my cmake tree and apparently didn't reconfigure it correctly.

[labath]

Yeah, that's definitely python-related, but it's hard to say how exactly without knowing more details. Also note that there have been some changes in how the python stuff is used/built in the last couple of weeks so it's possible this was not caused by a change on your side...

[labath]

TBE, this happens when lldb is not able to find the python package files (which should normally be in $BUILD/libXY/pythonA.B/site-packages).

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

 #include "lldb/Host/PosixApi.h"
 #include "lldb/Host/common/NativeProcessProtocol.h"
 #include "lldb/Host/common/NativeRegisterContext.h"
 #include "lldb/Host/common/NativeThreadProtocol.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/Endian.h"
-#include "lldb/Utility/JSON.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/UriParser.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/ScopedPrinter.h"
 
 #include "ProcessGDBRemote.h"
 #include "ProcessGDBRemoteLog.h"
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 using namespace lldb;
 using namespace lldb_private;
if (reg_value_p) {
     // Zero-out any unreadable values.
     if (reg_info.byte_size > 0) {
       std::basic_string<uint8_t> zeros(reg_info.byte_size, '\0');
       AppendHexValue(response, zeros.data(), zeros.size(), false);
     }
   }
 }
 
-static JSONObject::SP GetRegistersAsJSON(NativeThreadProtocol &thread) {
+static llvm::Expected<json::Object>
+GetRegistersAsJSON(NativeThreadProtocol &thread) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
 
   NativeRegisterContext& reg_ctx = thread.GetRegisterContext();
 
-  JSONObject::SP register_object_sp = std::make_shared<JSONObject>();
+  json::Object register_object;
 
 #ifdef LLDB_JTHREADSINFO_FULL_REGISTER_SET
   // Expedite all registers in the first register set (i.e. should be GPRs)
   // that are not contained in other registers.
   const RegisterSet *reg_set_p = reg_ctx_sp->GetRegisterSet(0);
   if (!reg_set_p)
-    return nullptr;
+    return llvm::make_error<llvm::StringError>("failed to get registers",
+                                               llvm::inconvertibleErrorCode());
   for (const uint32_t *reg_num_p = reg_set_p->registers;
        *reg_num_p != LLDB_INVALID_REGNUM; ++reg_num_p) {
     uint32_t reg_num = *reg_num_p;
 #else
   // Expedite only a couple of registers until we figure out why sending
   // registers is expensive.
   static const uint32_t k_expedited_registers[] = {
       LLDB_REGNUM_GENERIC_PC, LLDB_REGNUM_GENERIC_SP, LLDB_REGNUM_GENERIC_FP,
if (error.Fail()) {
                 reg_num, error.AsCString());
       continue;
     }
 
     StreamString stream;
     WriteRegisterValueInHexFixedWidth(stream, reg_ctx, *reg_info_p,
                                       &reg_value, lldb::eByteOrderBig);
 
-    register_object_sp->SetObject(
-        llvm::to_string(reg_num),
-        std::make_shared<JSONString>(stream.GetString()));
+    register_object.try_emplace(llvm::to_string(reg_num), stream.GetString());
   }
 
-  return register_object_sp;
+  return register_object;
 }
 
 static const char *GetStopReasonString(StopReason stop_reason) {
   switch (stop_reason) {
   case eStopReasonTrace:
     return "trace";
   case eStopReasonBreakpoint:
     return "breakpoint";
static const char *GetStopReasonString(StopReason stop_reason) {
   case eStopReasonPlanComplete:
   case eStopReasonThreadExiting:
   case eStopReasonNone:
     break; // ignored
   }
   return nullptr;
 }
 
-static JSONArray::SP GetJSONThreadsInfo(NativeProcessProtocol &process,
-                                        bool abridged) {
+static llvm::Expected<json::Value>
+GetJSONThreadsInfo(NativeProcessProtocol &process, bool abridged) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_THREAD));
 
-  JSONArray::SP threads_array_sp = std::make_shared<JSONArray>();
+  json::Array threads_array;
 
   // Ensure we can get info on the given thread.
   uint32_t thread_idx = 0;
   for (NativeThreadProtocol *thread;
        (thread = process.GetThreadAtIndex(thread_idx)) != nullptr;
        ++thread_idx) {
 
     lldb::tid_t tid = thread->GetID();
 
     // Grab the reason this thread stopped.
     struct ThreadStopInfo tid_stop_info;
     std::string description;
     if (!thread->GetStopReason(tid_stop_info, description))
-      return nullptr;
+      return llvm::make_error<llvm::StringError>(
+          "failed to get stop reason", llvm::inconvertibleErrorCode());
 
     const int signum = tid_stop_info.details.signal.signo;
     if (log) {
       LLDB_LOGF(log,
                 "GDBRemoteCommunicationServerLLGS::%s pid %" PRIu64
                 " tid %" PRIu64
                 " got signal signo = %d, reason = %d, exc_type = %" PRIu64,
                 __FUNCTION__, process.GetID(), tid, signum,
                 tid_stop_info.reason, tid_stop_info.details.exception.type);
     }
 
-    JSONObject::SP thread_obj_sp = std::make_shared<JSONObject>();
-    threads_array_sp->AppendObject(thread_obj_sp);
+    json::Object thread_obj;
 
     if (!abridged) {
-      if (JSONObject::SP registers_sp = GetRegistersAsJSON(*thread))
-        thread_obj_sp->SetObject("registers", registers_sp);
+      if (llvm::Expected<json::Object> registers =
+              GetRegistersAsJSON(*thread)) {
+        thread_obj.try_emplace("registers", std::move(*registers));
+      } else {
+        return registers.takeError();
       }
+    }
+
+    thread_obj.try_emplace("tid", static_cast<int64_t>(tid));
 
-    thread_obj_sp->SetObject("tid", std::make_shared<JSONNumber>(tid));
     if (signum != 0)
-      thread_obj_sp->SetObject("signal", std::make_shared<JSONNumber>(signum));
+      thread_obj.try_emplace("signal", signum);
 
     const std::string thread_name = thread->GetName();
     if (!thread_name.empty())
-      thread_obj_sp->SetObject("name",
-                               std::make_shared<JSONString>(thread_name));
+      thread_obj.try_emplace("name", thread_name);
 
-    if (const char *stop_reason_str = GetStopReasonString(tid_stop_info.reason))
-      thread_obj_sp->SetObject("reason",
-                               std::make_shared<JSONString>(stop_reason_str));
+    const char *stop_reason = GetStopReasonString(tid_stop_info.reason);
+    if (stop_reason)
+      thread_obj.try_emplace("reason", stop_reason);
 
     if (!description.empty())
-      thread_obj_sp->SetObject("description",
-                               std::make_shared<JSONString>(description));
+      thread_obj.try_emplace("description", description);
 
     if ((tid_stop_info.reason == eStopReasonException) &&
         tid_stop_info.details.exception.type) {
-      thread_obj_sp->SetObject(
-          "metype",
-          std::make_shared<JSONNumber>(tid_stop_info.details.exception.type));
+      thread_obj.try_emplace(
+          "metype", static_cast<int64_t>(tid_stop_info.details.exception.type));
 
-      JSONArray::SP medata_array_sp = std::make_shared<JSONArray>();
+      json::Array medata_array;
       for (uint32_t i = 0; i < tid_stop_info.details.exception.data_count;
            ++i) {
-        medata_array_sp->AppendObject(std::make_shared<JSONNumber>(
-            tid_stop_info.details.exception.data[i]));
+        medata_array.push_back(
+            static_cast<int64_t>(tid_stop_info.details.exception.data[i]));
       }
-      thread_obj_sp->SetObject("medata", medata_array_sp);
+      thread_obj.try_emplace("medata", std::move(medata_array));
     }
-
-    // TODO: Expedite interesting regions of inferior memory
+    threads_array.push_back(std::move(thread_obj));
   }
-
-  return threads_array_sp;
+  return threads_array;
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::SendStopReplyPacketForThread(
     lldb::tid_t tid) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_THREAD));
 
   // Ensure we have a debugged process.
if (m_list_threads_in_stop_reply) {
     }
     response.PutChar(';');
 
     // Include JSON info that describes the stop reason for any threads that
     // actually have stop reasons. We use the new "jstopinfo" key whose values
     // is hex ascii JSON that contains the thread IDs thread stop info only for
     // threads that have stop reasons. Only send this if we have more than one
     // thread otherwise this packet has all the info it needs.
-    if (thread_index > 0) {
+    if (thread_index > 1) {
       const bool threads_with_valid_stop_info_only = true;
-      JSONArray::SP threads_info_sp = GetJSONThreadsInfo(
+      llvm::Expected<json::Value> threads_info = GetJSONThreadsInfo(
           *m_debugged_process_up, threads_with_valid_stop_info_only);
-      if (threads_info_sp) {
+      if (threads_info) {
         response.PutCString("jstopinfo:");
         StreamString unescaped_response;
-        threads_info_sp->Write(unescaped_response);
+        unescaped_response.AsRawOstream() << *threads_info;
         response.PutStringAsRawHex8(unescaped_response.GetData());
         response.PutChar(';');
-      } else
-        LLDB_LOG(log, "failed to prepare a jstopinfo field for pid {0}",
-                 m_debugged_process_up->GetID());
+      } else {
+        LLDB_LOG(log, "failed to prepare a jstopinfo field for pid {0}:",
+                 m_debugged_process_up->GetID(),
+                 llvm::toString(threads_info.takeError()));
+      }
     }
 
     uint32_t i = 0;
     response.PutCString("thread-pcs");
     char delimiter = ':';
     for (NativeThreadProtocol *thread;
          (thread = m_debugged_process_up->GetThreadAtIndex(i)) != nullptr;
          ++i) {
GDBRemoteCommunicationServerLLGS::Handle_jThreadsInfo(
   // Ensure we have a debugged process.
   if (!m_debugged_process_up ||
       (m_debugged_process_up->GetID() == LLDB_INVALID_PROCESS_ID))
     return SendErrorResponse(50);
   LLDB_LOG(log, "preparing packet for pid {0}", m_debugged_process_up->GetID());
 
   StreamString response;
   const bool threads_with_valid_stop_info_only = false;
-  JSONArray::SP threads_array_sp = GetJSONThreadsInfo(
+  llvm::Expected<json::Value> threads_info = GetJSONThreadsInfo(
       *m_debugged_process_up, threads_with_valid_stop_info_only);
-  if (!threads_array_sp) {
-    LLDB_LOG(log, "failed to prepare a packet for pid {0}",
-             m_debugged_process_up->GetID());
+  if (!threads_info) {
+    LLDB_LOG(log, "failed to prepare a packet for pid {0}: {1}",
+             m_debugged_process_up->GetID(),
+             llvm::toString(threads_info.takeError()));
     return SendErrorResponse(52);
   }
 
-  threads_array_sp->Write(response);
+  response.AsRawOstream() << *threads_info;
   StreamGDBRemote escaped_response;
   escaped_response.PutEscapedBytes(response.GetData(), response.GetSize());
   return SendPacketNoLock(escaped_response.GetString());
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::Handle_qWatchpointSupportInfo(
     StringExtractorGDBRemote &packet) {