[Lldb-commits] [PATCH] D70574: [lldb-server] Verify 'g' is supported before relying on them

[Guilherme Andrade via Phabricator via lldb-commits]

guiandrade created this revision.
guiandrade added reviewers: jasonmolenda, labath.
guiandrade added a project: LLDB.
Herald added a subscriber: lldb-commits.

This ensures that 'g' packets are only used if the config
plugin.process.gdb-remote.use-g-packet-for-reading = true and
we get a normal packet in response to a 'g' packet.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70574

Files:
  lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
  lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoGPacketSupported.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp


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
@@ -304,7 +304,9 @@
       bool pSupported =
           gdb_process->GetGDBRemote().GetpPacketSupported(GetID());
       bool read_all_registers_at_once =
-          !pSupported || gdb_process->m_use_g_packet_for_reading;
+          !pSupported ||
+          (gdb_process->m_use_g_packet_for_reading &&
+           gdb_process->GetGDBRemote().GetgPacketSupported(GetID()));
       bool write_all_registers_at_once = !pSupported;
       reg_ctx_sp = std::make_shared<GDBRemoteRegisterContext>(
           *this, concrete_frame_idx, gdb_process->m_register_info,
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
@@ -235,6 +235,8 @@
 
   bool GetpPacketSupported(lldb::tid_t tid);
 
+  bool GetgPacketSupported(lldb::tid_t tid);
+
   bool GetxPacketSupported();
 
   bool GetVAttachOrWaitSupported();
@@ -516,6 +518,7 @@
   LazyBool m_attach_or_wait_reply;
   LazyBool m_prepare_for_reg_writing_reply;
   LazyBool m_supports_p;
+  LazyBool m_supports_g;
   LazyBool m_supports_x;
   LazyBool m_avoid_g_packets;
   LazyBool m_supports_QSaveRegisterState;
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
@@ -68,8 +68,8 @@
       m_watchpoints_trigger_after_instruction(eLazyBoolCalculate),
       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_p(eLazyBoolCalculate), m_supports_g(eLazyBoolCalculate),
+      m_supports_x(eLazyBoolCalculate), m_avoid_g_packets(eLazyBoolCalculate),
       m_supports_QSaveRegisterState(eLazyBoolCalculate),
       m_supports_qXfer_auxv_read(eLazyBoolCalculate),
       m_supports_qXfer_libraries_read(eLazyBoolCalculate),
@@ -548,6 +548,12 @@
   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;
Index: lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoGPacketSupported.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoGPacketSupported.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoGPacketSupported.py
@@ -70,9 +70,12 @@
                 else:
                     return None, False
 
+        self.dbg.HandleCommand(
+                "settings set plugin.process.gdb-remote.use-g-packet-for-reading true")
+        self.addTearDownHook(lambda:
+                self.runCmd("settings set plugin.process.gdb-remote.use-g-packet-for-reading false"))
         self.server.responder = MyResponder()
         target = self.dbg.CreateTarget('')
-        self.runCmd("log enable gdb-remote packets")
         if self.TraceOn():
           self.runCmd("log enable gdb-remote packets")
           self.addTearDownHook(
Index: lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -60,7 +60,8 @@
         target = self.createTarget("a.yaml")
         process = self.connect(target)
 
-        self.assertEquals(1, self.server.responder.packetLog.count("g"))
+        # 1 packet used to verify that 'g' is supported + 1 to fetch the registers.
+        self.assertEquals(2, self.server.responder.packetLog.count("g"))
         self.server.responder.packetLog = []
         self.read_registers(process)
         # Reading registers should not cause any 'p' packets to be exchanged.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70574.230554.patch
Type: text/x-patch
Size: 4739 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20191121/ed749521/attachment-0001.bin>