omjavaid updated this revision to Diff 316354.
omjavaid added a comment.
Herald added a subscriber: pengfei.
This update adds a new test case that checks for register number mismatch
between lldb and gdb stub. LLDB client assigns register numbers to target xml
registers in increasing order starting with regnum = 0, while gdb-remote may
specify different regnum which is stored as eRegisterKindProcessPlugin. Remote
side will use its register number in expedited register list, value_regs and
invalidate_regnums.
For p/P packet LLDB was already using remote register numbers while this patch
fixes the behavior for all above mentioned cases.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77043/new/
https://reviews.llvm.org/D77043
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/test/API/functionalities/gdb_remote_client/TestRemoteRegNums.py
Index: lldb/test/API/functionalities/gdb_remote_client/TestRemoteRegNums.py
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestRemoteRegNums.py
@@ -0,0 +1,126 @@
+from __future__ import print_function
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+# This test case checks for register number mismatch between lldb and gdb stub.
+# LLDB client assigns register numbers to target xml registers in increasing
+# order starting with regnum = 0, while gdb-remote may specify different regnum
+# which is stored as eRegisterKindProcessPlugin. Remote side will use its
+# register number in expedited register list, value_regs and invalidate_regnums.
+#
+# This test creates a ficticious target xml with non-sequential regnums to test
+# that correct registers are accessed in all of above mentioned cases.
+
+class TestRemoteRegNums(GDBRemoteTestBase):
+
+ @skipIfXmlSupportMissing
+ def test(self):
+ class MyResponder(MockGDBServerResponder):
+ def haltReason(self):
+ return "T02thread:1ff0d;threads:1ff0d;thread-pcs:000000010001bc00;00:00bc010001000000;09:c04825ebfe7f0000;"
+
+ def threadStopInfo(self, threadnum):
+ return "T02thread:1ff0d;threads:1ff0d;thread-pcs:000000010001bc00;00:00bc010001000000;09:c04825ebfe7f0000;"
+
+ def writeRegisters(self):
+ return "E02"
+
+ def readRegisters(self):
+ return "E01"
+
+ rax_regnum2_val = "7882773ce0ffffff"
+ rbx_regnum4_val = "1122334455667788"
+
+ def readRegister(self, regnum):
+ # lldb will try sending "p0" to see if the p packet is supported,
+ # give a bogus value; in theory lldb could use this value in the
+ # register context and that would be valid behavior.
+
+ # notably, don't give values for registers 1 & 3 -- lldb should
+ # get those from the ? stop packet ("T11") and it is a pref regression
+ # if lldb is asking for these register values.
+ if regnum == 0:
+ return "5555555555555555"
+ if regnum == 2:
+ return self.rax_regnum2_val
+ if regnum == 4:
+ return self.rbx_regnum4_val
+
+ return "E03"
+
+ def writeRegister(self, regnum, value_hex):
+ if regnum == 2:
+ self.rax_regnum2_val = value_hex
+ if regnum == 4:
+ self.rbx_regnum4_val = value_hex
+
+ return "OK"
+
+ def qXferRead(self, obj, annex, offset, length):
+ if annex == "target.xml":
+ return """<?xml version="1.0"?>
+ <target version="1.0">
+ <architecture>i386:x86-64</architecture>
+ <feature name="org.gnu.gdb.i386.core">
+ <reg name="rip" bitsize="64" regnum="0" type="code_ptr" group="general" altname="pc" generic="pc"/>
+ <reg name="rax" bitsize="64" regnum="2" type="code_ptr" group="general"/>
+ <reg name="rbx" bitsize="64" regnum="4" type="code_ptr" group="general"/>
+ <reg name="eax" bitsize="32" regnum="5" value_regnums="2" invalidate_regnums="2" type="code_ptr" group="general"/>
+ <reg name="ebx" bitsize="32" regnum="7" value_regnums="4" invalidate_regnums="4" type="code_ptr" group="general"/>
+ <reg name="rsi" bitsize="64" regnum="9" type="code_ptr" group="general"/>
+ </feature>
+ </target>""", False
+ else:
+ return None, False
+
+ self.server.responder = MyResponder()
+ target = self.dbg.CreateTarget('')
+ if self.TraceOn():
+ self.runCmd("log enable gdb-remote packets")
+ self.addTearDownHook(
+ lambda: self.runCmd("log disable gdb-remote packets"))
+ process = self.connect(target)
+
+ thread = process.GetThreadAtIndex(0)
+ frame = thread.GetFrameAtIndex(0)
+ rax = frame.FindRegister("rax").GetValueAsUnsigned()
+ eax = frame.FindRegister("eax").GetValueAsUnsigned()
+ rbx = frame.FindRegister("rbx").GetValueAsUnsigned()
+ ebx = frame.FindRegister("ebx").GetValueAsUnsigned()
+ rsi = frame.FindRegister("rsi").GetValueAsUnsigned()
+ pc = frame.GetPC()
+ rip = frame.FindRegister("rip").GetValueAsUnsigned()
+
+ if self.TraceOn():
+ print("Register values: rax == 0x%x, rbx == 0x%x, rsi == 0x%x, pc == 0x%x, rip == 0x%x" % (
+ rax, rbx, rsi, pc, rip))
+
+ self.assertEqual(rax, 0xffffffe03c778278)
+ self.assertEqual(rbx, 0x8877665544332211)
+ self.assertEqual(eax, 0x3c778278)
+ self.assertEqual(ebx, 0x44332211)
+ self.assertEqual(rsi, 0x00007ffeeb2548c0)
+ self.assertEqual(pc, 0x10001bc00)
+ self.assertEqual(rip, 0x10001bc00)
+
+ frame.FindRegister("eax").SetValueFromCString("1")
+ frame.FindRegister("ebx").SetValueFromCString("0")
+ eax = frame.FindRegister("eax").GetValueAsUnsigned()
+ ebx = frame.FindRegister("ebx").GetValueAsUnsigned()
+ rax = frame.FindRegister("rax").GetValueAsUnsigned()
+ rbx = frame.FindRegister("rbx").GetValueAsUnsigned()
+
+ if self.TraceOn():
+ print("Register values: rax == 0x%x, rbx == 0x%x, rsi == 0x%x, pc == 0x%x, rip == 0x%x" % (
+ rax, rbx, rsi, pc, rip))
+
+ self.assertEqual(rax, 0xffffffe000000001)
+ self.assertEqual(rbx, 0x8877665500000000)
+ self.assertEqual(eax, 0x00000001)
+ self.assertEqual(ebx, 0x00000000)
+ self.assertEqual(rsi, 0x00007ffeeb2548c0)
+ self.assertEqual(pc, 0x10001bc00)
+ self.assertEqual(rip, 0x10001bc00)
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
@@ -1748,7 +1748,9 @@
if (thread_sp) {
ThreadGDBRemote *gdb_thread =
static_cast<ThreadGDBRemote *>(thread_sp.get());
- gdb_thread->GetRegisterContext()->InvalidateIfNeeded(true);
+ RegisterContextSP gdb_reg_ctx_sp(gdb_thread->GetRegisterContext());
+
+ gdb_reg_ctx_sp->InvalidateIfNeeded(true);
auto iter = std::find(m_thread_ids.begin(), m_thread_ids.end(), tid);
if (iter != m_thread_ids.end()) {
@@ -1760,7 +1762,10 @@
DataBufferSP buffer_sp(new DataBufferHeap(
reg_value_extractor.GetStringRef().size() / 2, 0));
reg_value_extractor.GetHexBytes(buffer_sp->GetData(), '\xcc');
- gdb_thread->PrivateSetRegisterValue(pair.first, buffer_sp->GetData());
+ uint32_t lldb_regnum =
+ gdb_reg_ctx_sp->ConvertRegisterKindToRegisterNumber(
+ eRegisterKindProcessPlugin, pair.first);
+ gdb_thread->PrivateSetRegisterValue(lldb_regnum, buffer_sp->GetData());
}
thread_sp->SetName(thread_name.empty() ? nullptr : thread_name.c_str());
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
@@ -242,11 +242,15 @@
// Index of the primordial register.
bool success = true;
for (uint32_t idx = 0; success; ++idx) {
- const uint32_t prim_reg = reg_info->value_regs[idx];
+ uint32_t prim_reg = reg_info->value_regs[idx];
if (prim_reg == LLDB_INVALID_REGNUM)
break;
// We have a valid primordial register as our constituent. Grab the
// corresponding register info.
+ uint32_t regnum = ConvertRegisterKindToRegisterNumber(
+ eRegisterKindProcessPlugin, prim_reg);
+ if (regnum != LLDB_INVALID_REGNUM)
+ prim_reg = regnum;
const RegisterInfo *prim_reg_info = GetRegisterInfoAtIndex(prim_reg);
if (prim_reg_info == nullptr)
success = false;
@@ -375,11 +379,15 @@
// Invalidate this composite register first.
for (uint32_t idx = 0; success; ++idx) {
- const uint32_t reg = reg_info->value_regs[idx];
+ uint32_t reg = reg_info->value_regs[idx];
if (reg == LLDB_INVALID_REGNUM)
break;
// We have a valid primordial register as our constituent. Grab the
// corresponding register info.
+ uint32_t lldb_regnum = ConvertRegisterKindToRegisterNumber(
+ eRegisterKindProcessPlugin, reg);
+ if (lldb_regnum != LLDB_INVALID_REGNUM)
+ reg = lldb_regnum;
const RegisterInfo *value_reg_info = GetRegisterInfoAtIndex(reg);
if (value_reg_info == nullptr)
success = false;
@@ -397,6 +405,10 @@
for (uint32_t idx = 0, reg = reg_info->invalidate_regs[0];
reg != LLDB_INVALID_REGNUM;
reg = reg_info->invalidate_regs[++idx]) {
+ uint32_t lldb_regnum = ConvertRegisterKindToRegisterNumber(
+ eRegisterKindProcessPlugin, reg);
+ if (lldb_regnum != LLDB_INVALID_REGNUM)
+ reg = lldb_regnum;
SetRegisterIsValid(reg, false);
}
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
