[Lldb-commits] [lldb] 8561ad9 - Use remote regnums in expedited list, value regs and invalidate regs
Muhammad Omair Javaid via lldb-commits
lldb-commits at lists.llvm.org
Mon Feb 8 01:12:25 PST 2021
Author: Muhammad Omair Javaid
Date: 2021-02-08T14:09:15+05:00
New Revision: 8561ad9296b70b5a2af1574a1576090520d62a7c
URL: https://github.com/llvm/llvm-project/commit/8561ad9296b70b5a2af1574a1576090520d62a7c
DIFF: https://github.com/llvm/llvm-project/commit/8561ad9296b70b5a2af1574a1576090520d62a7c.diff
LOG: Use remote regnums in expedited list, value regs and invalidate regs
Native register descriptions in LLDB specify lldb register numbers in
value_regs and invalidate_regs lists. These register numbers may not
match with Process gdb-remote register numbers which are generated by
native process after counting all registers in its register sets.
It was coincidentally not causing any problems as we never came across
a native target with dynamically changing register sets and register
numbers generated by counter matched with LLDB native register numbers.
This came up while testing target AArch64 SVE which can choose register
sets based on underlying hardware.
This patch fixes this behavior and always tries to use remote register
numbers while reading/writing registers over gdb-remote protocol.
Reviewed By: labath
Differential Revision: https://reviews.llvm.org/D77043
Added:
lldb/test/API/functionalities/gdb_remote_client/TestRemoteRegNums.py
Modified:
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Removed:
################################################################################
diff --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
index 5463a071503c..a85d7bd6f525 100644
--- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -697,6 +697,14 @@ RegisterInfo *DynamicRegisterInfo::GetRegisterInfoAtIndex(uint32_t i) {
return nullptr;
}
+const RegisterInfo *DynamicRegisterInfo::GetRegisterInfo(uint32_t kind,
+ uint32_t num) const {
+ uint32_t reg_index = ConvertRegisterKindToRegisterNumber(kind, num);
+ if (reg_index != LLDB_INVALID_REGNUM)
+ return &m_regs[reg_index];
+ return nullptr;
+}
+
const RegisterSet *DynamicRegisterInfo::GetRegisterSet(uint32_t i) const {
if (i < m_sets.size())
return &m_sets[i];
diff --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
index fbf9db685b71..7e90454c6d9d 100644
--- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
+++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
@@ -60,6 +60,9 @@ class DynamicRegisterInfo {
uint32_t ConvertRegisterKindToRegisterNumber(uint32_t kind,
uint32_t num) const;
+ const lldb_private::RegisterInfo *GetRegisterInfo(uint32_t kind,
+ uint32_t num) const;
+
void Dump() const;
void Clear();
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index 10006616b0c6..b607b156d7b4 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -249,7 +249,8 @@ bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info,
break;
// We have a valid primordial register as our constituent. Grab the
// corresponding register info.
- const RegisterInfo *prim_reg_info = GetRegisterInfoAtIndex(prim_reg);
+ const RegisterInfo *prim_reg_info =
+ GetRegisterInfo(eRegisterKindProcessPlugin, prim_reg);
if (prim_reg_info == nullptr)
success = false;
else {
@@ -395,7 +396,8 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const RegisterInfo *reg_info,
break;
// We have a valid primordial register as our constituent. Grab the
// corresponding register info.
- const RegisterInfo *value_reg_info = GetRegisterInfoAtIndex(reg);
+ const RegisterInfo *value_reg_info =
+ GetRegisterInfo(eRegisterKindProcessPlugin, reg);
if (value_reg_info == nullptr)
success = false;
else
@@ -414,9 +416,10 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const RegisterInfo *reg_info,
if (reg_info->invalidate_regs) {
for (uint32_t idx = 0, reg = reg_info->invalidate_regs[0];
reg != LLDB_INVALID_REGNUM;
- reg = reg_info->invalidate_regs[++idx]) {
- SetRegisterIsValid(reg, false);
- }
+ reg = reg_info->invalidate_regs[++idx])
+ SetRegisterIsValid(ConvertRegisterKindToRegisterNumber(
+ eRegisterKindProcessPlugin, reg),
+ false);
}
return success;
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index aba870c42e55..a2e41e738dbd 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1748,7 +1748,9 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
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 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
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());
}
// AArch64 SVE specific code below calls AArch64SVEReconfigure to update
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestRemoteRegNums.py b/lldb/test/API/functionalities/gdb_remote_client/TestRemoteRegNums.py
new file mode 100644
index 000000000000..c3feb2aa9bda
--- /dev/null
+++ b/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
diff erent 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)
More information about the lldb-commits
mailing list