guiandrade updated this revision to Diff 208838.
guiandrade marked 3 inline comments as done.
guiandrade added a comment.

This rebases the change, rename *try_to_use_g_packet* to *use_g_packet*, 
defaults the setting to true, and modifies the condition of the 
read_all_registers_at_once bool in 
ThreadGDBRemote::CreateRegisterContextForFrame to guarantee the server supports 
'g' packet.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62931/new/

https://reviews.llvm.org/D62931

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===================================================================
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -128,6 +128,12 @@
   ASSERT_EQ(0,
             memcmp(buffer_sp->GetBytes(), one_register, sizeof one_register));
 
+  async_result = std::async(std::launch::async,
+                            [&] { return client.GetgPacketSupported(tid); });
+  Handle_QThreadSuffixSupported(server, true);
+  HandlePacket(server, "g;thread:0047;", all_registers_hex);
+  ASSERT_TRUE(async_result.get());
+
   read_result = std::async(std::launch::async,
                            [&] { return client.ReadAllRegisters(tid); });
   HandlePacket(server, "g;thread:0047;", all_registers_hex);
Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
@@ -25,7 +25,7 @@
 
 class ThreadGDBRemote : public Thread {
 public:
-  ThreadGDBRemote(Process &process, lldb::tid_t tid);
+  ThreadGDBRemote(Process &process, lldb::tid_t tid, bool use_g_packet);
 
   ~ThreadGDBRemote() override;
 
@@ -100,6 +100,7 @@
   uint64_t
       m_queue_serial_number; // Queue info from stop reply/stop info for thread
   lldb_private::LazyBool m_associated_with_libdispatch_queue;
+  bool m_use_g_packet;
 
   bool PrivateSetRegisterValue(uint32_t reg, llvm::ArrayRef<uint8_t> data);
 
Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
@@ -33,12 +33,14 @@
 
 // Thread Registers
 
-ThreadGDBRemote::ThreadGDBRemote(Process &process, lldb::tid_t tid)
+ThreadGDBRemote::ThreadGDBRemote(Process &process, lldb::tid_t tid,
+                                 bool use_g_packet)
     : Thread(process, tid), m_thread_name(), m_dispatch_queue_name(),
       m_thread_dispatch_qaddr(LLDB_INVALID_ADDRESS),
       m_dispatch_queue_t(LLDB_INVALID_ADDRESS), m_queue_kind(eQueueKindUnknown),
       m_queue_serial_number(LLDB_INVALID_QUEUE_ID),
-      m_associated_with_libdispatch_queue(eLazyBoolCalculate) {
+      m_associated_with_libdispatch_queue(eLazyBoolCalculate),
+      m_use_g_packet(use_g_packet) {
   Log *log(GetLogIfAnyCategoriesSet(GDBR_LOG_THREAD));
   LLDB_LOG(log, "this = {0}, pid = {1}, tid = {2}", this, process.GetID(),
            GetID());
@@ -303,10 +305,13 @@
     if (process_sp) {
       ProcessGDBRemote *gdb_process =
           static_cast<ProcessGDBRemote *>(process_sp.get());
-      // read_all_registers_at_once will be true if 'p' packet is not
+      // read_all_registers_at_once will be true if the server supports 'g'
+      // packet and either the flag m_use_g_packet is true or 'p' packet is not
       // supported.
       bool read_all_registers_at_once =
-          !gdb_process->GetGDBRemote().GetpPacketSupported(GetID());
+          gdb_process->GetGDBRemote().GetgPacketSupported(GetID()) &&
+          (m_use_g_packet ||
+           !gdb_process->GetGDBRemote().GetpPacketSupported(GetID()));
       reg_ctx_sp = std::make_shared<GDBRemoteRegisterContext>(
           *this, concrete_frame_idx, gdb_process->m_register_info,
           read_all_registers_at_once);
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -138,12 +138,20 @@
      nullptr,
      {},
      "If true, the libraries-svr4 feature will be used to get a hold of the "
-     "process's loaded modules."}};
+     "process's loaded modules."},
+    {"use-g-packet",
+     OptionValue::eTypeBoolean,
+     true,
+     1,
+     nullptr,
+     {},
+     "Specify if the server should use 'g' packets."}};
 
 enum {
   ePropertyPacketTimeout,
   ePropertyTargetDefinitionFile,
-  ePropertyUseSVR4
+  ePropertyUseSVR4,
+  ePropertyUseGPacket
 };
 
 class PluginProperties : public Properties {
@@ -180,6 +188,11 @@
     return m_collection_sp->GetPropertyAtIndexAsBoolean(
         nullptr, idx, g_properties[idx].default_uint_value != 0);
   }
+
+  bool GetUseGPacket() const {
+    const uint32_t idx = ePropertyUseGPacket;
+    return m_collection_sp->GetPropertyAtIndexAsBoolean(nullptr, idx, true);
+  }
 };
 
 typedef std::shared_ptr<PluginProperties> ProcessKDPPropertiesSP;
@@ -1732,7 +1745,8 @@
       ThreadSP thread_sp(
           old_thread_list_copy.RemoveThreadByProtocolID(tid, false));
       if (!thread_sp) {
-        thread_sp = std::make_shared<ThreadGDBRemote>(*this, tid);
+        thread_sp = std::make_shared<ThreadGDBRemote>(
+            *this, tid, GetGlobalPluginProperties()->GetUseGPacket());
         LLDB_LOGV(log, "Making new thread: {0} for thread ID: {1:x}.",
                   thread_sp.get(), thread_sp->GetID());
       } else {
@@ -1848,7 +1862,8 @@
 
       if (!thread_sp) {
         // Create the thread if we need to
-        thread_sp = std::make_shared<ThreadGDBRemote>(*this, tid);
+        thread_sp = std::make_shared<ThreadGDBRemote>(
+            *this, tid, GetGlobalPluginProperties()->GetUseGPacket());
         m_thread_list_real.AddThread(thread_sp);
       }
     }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -234,6 +234,8 @@
 
   bool GetpPacketSupported(lldb::tid_t tid);
 
+  bool GetgPacketSupported(lldb::tid_t tid);
+
   bool GetxPacketSupported();
 
   bool GetVAttachOrWaitSupported();
@@ -514,6 +516,7 @@
   LazyBool m_prepare_for_reg_writing_reply;
   LazyBool m_supports_p;
   LazyBool m_supports_x;
+  LazyBool m_supports_g;
   LazyBool m_avoid_g_packets;
   LazyBool m_supports_QSaveRegisterState;
   LazyBool m_supports_qXfer_auxv_read;
@@ -592,6 +595,8 @@
   Status GetQXferMemoryMapRegionInfo(lldb::addr_t addr,
                                      MemoryRegionInfo &region);
 
+  LazyBool GetThreadPacketSupported(lldb::tid_t tid, llvm::StringRef packetStr);
+
 private:
   DISALLOW_COPY_AND_ASSIGN(GDBRemoteCommunicationClient);
 };
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -72,7 +72,7 @@
       m_attach_or_wait_reply(eLazyBoolCalculate),
       m_prepare_for_reg_writing_reply(eLazyBoolCalculate),
       m_supports_p(eLazyBoolCalculate), m_supports_x(eLazyBoolCalculate),
-      m_avoid_g_packets(eLazyBoolCalculate),
+      m_supports_g(eLazyBoolCalculate), m_avoid_g_packets(eLazyBoolCalculate),
       m_supports_QSaveRegisterState(eLazyBoolCalculate),
       m_supports_qXfer_auxv_read(eLazyBoolCalculate),
       m_supports_qXfer_libraries_read(eLazyBoolCalculate),
@@ -546,21 +546,30 @@
 //
 // Takes a valid thread ID because p needs to apply to a thread.
 bool GDBRemoteCommunicationClient::GetpPacketSupported(lldb::tid_t tid) {
-  if (m_supports_p == eLazyBoolCalculate) {
-    m_supports_p = eLazyBoolNo;
-    StreamString payload;
-    payload.PutCString("p0");
-    StringExtractorGDBRemote response;
-    if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload),
-                                                   response, false) ==
-            PacketResult::Success &&
-        response.IsNormalResponse()) {
-      m_supports_p = eLazyBoolYes;
-    }
-  }
+  if (m_supports_p == eLazyBoolCalculate)
+    m_supports_p = GetThreadPacketSupported(tid, "p0");
   return m_supports_p;
 }
 
+bool GDBRemoteCommunicationClient::GetgPacketSupported(lldb::tid_t tid) {
+  if (m_supports_g == eLazyBoolCalculate)
+    m_supports_g = GetThreadPacketSupported(tid, "g");
+  return m_supports_g;
+}
+
+LazyBool GDBRemoteCommunicationClient::GetThreadPacketSupported(
+    lldb::tid_t tid, llvm::StringRef packetStr) {
+  StreamString payload;
+  payload.PutCString(packetStr);
+  StringExtractorGDBRemote response;
+  if (SendThreadSpecificPacketAndWaitForResponse(
+          tid, std::move(payload), response, false) == PacketResult::Success &&
+      response.IsNormalResponse()) {
+    return eLazyBoolYes;
+  }
+  return eLazyBoolNo;
+}
+
 StructuredData::ObjectSP GDBRemoteCommunicationClient::GetThreadsInfo() {
   // Get information on all threads at one using the "jThreadsInfo" packet
   StructuredData::ObjectSP object_sp;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to