omjavaid created this revision.
omjavaid added a reviewer: labath.
omjavaid requested review of this revision.
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.
https://reviews.llvm.org/D97498
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
lldb/test/API/functionalities/gdb_remote_client/TestPartialGPacket.py
Index: lldb/test/API/functionalities/gdb_remote_client/TestPartialGPacket.py
===================================================================
--- /dev/null
+++ 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)
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -118,6 +118,7 @@
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().
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -36,7 +36,7 @@
: 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::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 @@
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 @@
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 @@
" 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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
