[Lldb-commits] [lldb] 5367831 - Support GDB remote g packet partial read

Muhammad Omair Javaid via lldb-commits lldb-commits at lists.llvm.org
Tue Mar 2 04:24:34 PST 2021


Author: Muhammad Omair Javaid
Date: 2021-03-02T17:21:48+05:00
New Revision: 536783170f0843838b2d521f8a0331f174f60e3c

URL: https://github.com/llvm/llvm-project/commit/536783170f0843838b2d521f8a0331f174f60e3c
DIFF: https://github.com/llvm/llvm-project/commit/536783170f0843838b2d521f8a0331f174f60e3c.diff

LOG: Support GDB remote g packet partial read

GDB remote protocol does not specify length of g packet for register read. It depends on remote to include all or exclude certain registers from g packet. In case a register or set of registers is not included as part of g packet then we should fall back to p packet for reading all registers excluded from g packet by remote. This patch adds support for above feature and adds a test-case for the same.

Reviewed By: labath

Differential Revision: https://reviews.llvm.org/D97498

Added: 
    lldb/test/API/functionalities/gdb_remote_client/TestPartialGPacket.py

Modified: 
    lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
    lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index b607b156d7b4..1f50194e82f4 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -36,7 +36,7 @@ GDBRemoteRegisterContext::GDBRemoteRegisterContext(
     : RegisterContext(thread, concrete_frame_idx),
       m_reg_info_sp(std::move(reg_info_sp)), m_reg_valid(), m_reg_data(),
       m_read_all_at_once(read_all_at_once),
-      m_write_all_at_once(write_all_at_once) {
+      m_write_all_at_once(write_all_at_once), m_gpacket_cached(false) {
   // Resize our vector of bools to contain one bool for every register. We will
   // use these boolean values to know when a register value is valid in
   // m_reg_data.
@@ -57,6 +57,7 @@ void GDBRemoteRegisterContext::InvalidateAllRegisters() {
 }
 
 void GDBRemoteRegisterContext::SetAllRegisterValid(bool b) {
+  m_gpacket_cached = b;
   std::vector<bool>::iterator pos, end = m_reg_valid.end();
   for (pos = m_reg_valid.begin(); pos != end; ++pos)
     *pos = b;
@@ -200,7 +201,7 @@ bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info,
   const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
 
   if (!GetRegisterIsValid(reg)) {
-    if (m_read_all_at_once) {
+    if (m_read_all_at_once && !m_gpacket_cached) {
       if (DataBufferSP buffer_sp =
               gdb_comm.ReadAllRegisters(m_thread.GetProtocolID())) {
         memcpy(const_cast<uint8_t *>(m_reg_data.GetDataStart()),
@@ -221,7 +222,10 @@ bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info,
               m_reg_valid[i] = false;
             }
           }
-          return true;
+
+          m_gpacket_cached = true;
+          if (GetRegisterIsValid(reg))
+            return true;
         } else {
           Log *log(ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_THREAD |
                                                                 GDBR_LOG_PACKETS));
@@ -233,9 +237,9 @@ bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info,
               " bytes "
               "but only got %" PRId64 " bytes.",
               m_reg_data.GetByteSize(), buffer_sp->GetByteSize());
+          return false;
         }
       }
-      return false;
     }
     if (reg_info->value_regs) {
       // Process this composite register request by delegating to the

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
index 252d7b359ee8..18fcb73b9815 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -118,6 +118,7 @@ class GDBRemoteRegisterContext : public RegisterContext {
   DataExtractor m_reg_data;
   bool m_read_all_at_once;
   bool m_write_all_at_once;
+  bool m_gpacket_cached;
 
 private:
   // Helper function for ReadRegisterBytes().

diff  --git a/lldb/test/API/functionalities/gdb_remote_client/TestPartialGPacket.py b/lldb/test/API/functionalities/gdb_remote_client/TestPartialGPacket.py
new file mode 100644
index 000000000000..14614875811d
--- /dev/null
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestPartialGPacket.py
@@ -0,0 +1,106 @@
+from __future__ import print_function
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestPartialGPacket(GDBRemoteTestBase):
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test(self):
+        """
+        Test GDB remote fallback to 'p' packet when 'g' packet does not include all registers.
+        """
+        class MyResponder(MockGDBServerResponder):
+
+            def qXferRead(self, obj, annex, offset, length):
+                if annex == "target.xml":
+                    return """<?xml version="1.0"?>
+                        <!DOCTYPE feature SYSTEM "gdb-target.dtd">
+                        <target>
+                        <architecture>arm</architecture>
+                        <feature name="org.gnu.gdb.arm.m-profile">
+                        <reg name="r0" bitsize="32" type="uint32" group="general"/>
+                        <reg name="r1" bitsize="32" type="uint32" group="general"/>
+                        <reg name="r2" bitsize="32" type="uint32" group="general"/>
+                        <reg name="r3" bitsize="32" type="uint32" group="general"/>
+                        <reg name="r4" bitsize="32" type="uint32" group="general"/>
+                        <reg name="r5" bitsize="32" type="uint32" group="general"/>
+                        <reg name="r6" bitsize="32" type="uint32" group="general"/>
+                        <reg name="r7" bitsize="32" type="uint32" group="general"/>
+                        <reg name="r8" bitsize="32" type="uint32" group="general"/>
+                        <reg name="r9" bitsize="32" type="uint32" group="general"/>
+                        <reg name="r10" bitsize="32" type="uint32" group="general"/>
+                        <reg name="r11" bitsize="32" type="uint32" group="general"/>
+                        <reg name="r12" bitsize="32" type="uint32" group="general"/>
+                        <reg name="sp" bitsize="32" type="data_ptr" group="general"/>
+                        <reg name="lr" bitsize="32" type="uint32" group="general"/>
+                        <reg name="pc" bitsize="32" type="code_ptr" group="general"/>
+                        <reg name="xpsr" bitsize="32" regnum="25" type="uint32" group="general"/>
+                        <reg name="MSP" bitsize="32" regnum="26" type="uint32" group="general"/>
+                        <reg name="PSP" bitsize="32" regnum="27" type="uint32" group="general"/>
+                        <reg name="PRIMASK" bitsize="32" regnum="28" type="uint32" group="general"/>
+                        <reg name="BASEPRI" bitsize="32" regnum="29" type="uint32" group="general"/>
+                        <reg name="FAULTMASK" bitsize="32" regnum="30" type="uint32" group="general"/>
+                        <reg name="CONTROL" bitsize="32" regnum="31" type="uint32" group="general"/>
+                        </feature>
+                        </target>""", False
+                else:
+                    return None, False
+
+            def readRegister(self, regnum):
+                if regnum == 31:
+                    return "cdcc8c3f00000000"
+                return "E01"
+
+            def readRegisters(self):
+                return "20000000f8360020001000002fcb0008f8360020a0360020200c0020000000000000000000000000000000000000000000000000b87f0120b7d100082ed2000800000001"
+
+            def haltReason(self):
+                return "S05"
+
+            def qfThreadInfo(self):
+                return "mdead"
+
+            def qC(self):
+                return ""
+
+            def qSupported(self, client_supported):
+                return "PacketSize=4000;qXfer:memory-map:read-;QStartNoAckMode+;qXfer:threads:read+;hwbreak+;qXfer:features:read+"
+
+            def QThreadSuffixSupported(self):
+                return "OK"
+
+            def QListThreadsInStopReply(self):
+                return "OK"
+
+        self.server.responder = MyResponder()
+        if self.TraceOn():
+            self.runCmd("log enable gdb-remote packets")
+            self.addTearDownHook(
+                lambda: self.runCmd("log disable gdb-remote packets"))
+
+        self.dbg.SetDefaultArchitecture("armv7em")
+        target = self.dbg.CreateTargetWithFileAndArch(None, None)
+
+        process = self.connect(target)
+
+        if self.TraceOn():
+            interp = self.dbg.GetCommandInterpreter()
+            result = lldb.SBCommandReturnObject()
+            interp.HandleCommand("target list", result)
+            print(result.GetOutput())
+
+        r0_valobj = process.GetThreadAtIndex(
+            0).GetFrameAtIndex(0).FindRegister("r0")
+        self.assertEqual(r0_valobj.GetValueAsUnsigned(), 0x20)
+
+        pc_valobj = process.GetThreadAtIndex(
+            0).GetFrameAtIndex(0).FindRegister("pc")
+        self.assertEqual(pc_valobj.GetValueAsUnsigned(), 0x0800d22e)
+
+        pc_valobj = process.GetThreadAtIndex(
+            0).GetFrameAtIndex(0).FindRegister("CONTROL")
+        self.assertEqual(pc_valobj.GetValueAsUnsigned(), 0x3f8ccccd)


        


More information about the lldb-commits mailing list