omjavaid created this revision.
omjavaid added a reviewer: labath.
Herald added a subscriber: kristof.beyls.
omjavaid requested review of this revision.
This patch carries forward our aim to remove offset field from qRegisterInfo
packets and XML register description. I have created a new function which
returns if offset fields are dynamic meaning client can calculate offset on its
own based on register number sequence and register size. For now this function
only returns true for NativeRegisterContextLinux_arm64 but we can test this for
other architectures and make it standard later.
As a consequence we do not send offset field from lldb-server (arm64 for now)
while other stubs dont have an offset field so it wont effect them for now. On
the client side we already have a mechanism to calculate the offset but a minor
adjustment has been made to make offset increment only for primary registers.
Also some tests have been adjusted for this change.
https://reviews.llvm.org/D91241
Files:
lldb/include/lldb/Host/common/NativeRegisterContext.h
lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
lldb/unittests/tools/lldb-server/tests/TestClient.cpp
Index: lldb/unittests/tools/lldb-server/tests/TestClient.cpp
===================================================================
--- lldb/unittests/tools/lldb-server/tests/TestClient.cpp
+++ lldb/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -219,6 +219,7 @@
}
Error TestClient::qRegisterInfos() {
+ uint32_t reg_offset = 0;
for (unsigned int Reg = 0;; ++Reg) {
std::string Message = formatv("qRegisterInfo{0:x-}", Reg).str();
Expected<RegisterInfo> InfoOr = SendMessage<RegisterInfoParser>(Message);
@@ -227,6 +228,12 @@
break;
}
m_register_infos.emplace_back(std::move(*InfoOr));
+
+ if (m_register_infos[Reg].byte_offset == LLDB_INVALID_INDEX32)
+ m_register_infos[Reg].byte_offset = reg_offset;
+
+ reg_offset =
+ m_register_infos[Reg].byte_offset + m_register_infos[Reg].byte_size;
if (m_register_infos[Reg].kinds[eRegisterKindGeneric] ==
LLDB_REGNUM_GENERIC_PC)
m_pc_register = Reg;
Index: lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
===================================================================
--- lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
+++ lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
@@ -171,7 +171,7 @@
Info.byte_size /= CHAR_BIT;
if (!to_integer(Elements["offset"], Info.byte_offset, 10))
- return make_parsing_error("qRegisterInfo: offset");
+ Info.byte_offset = LLDB_INVALID_INDEX32;
Info.encoding = Args::StringToEncoding(Elements["encoding"]);
if (Info.encoding == eEncodingInvalid)
Index: lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
===================================================================
--- lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
+++ lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
@@ -65,5 +65,8 @@
self.assertEqual(q_info_reg["set"], xml_info_reg.get("group"))
self.assertEqual(q_info_reg["format"], xml_info_reg.get("format"))
self.assertEqual(q_info_reg["bitsize"], xml_info_reg.get("bitsize"))
- self.assertEqual(q_info_reg["offset"], xml_info_reg.get("offset"))
+
+ if not self.getArchitecture() == 'aarch64':
+ self.assertEqual(q_info_reg["offset"], xml_info_reg.get("offset"))
+
self.assertEqual(q_info_reg["encoding"], xml_info_reg.get("encoding"))
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
@@ -563,11 +563,13 @@
reg_info.byte_offset = reg_offset;
assert(reg_info.byte_size != 0);
- reg_offset += reg_info.byte_size;
+
if (!value_regs.empty()) {
value_regs.push_back(LLDB_INVALID_REGNUM);
reg_info.value_regs = value_regs.data();
- }
+ } else
+ reg_offset += reg_info.byte_size;
+
if (!invalidate_regs.empty()) {
invalidate_regs.push_back(LLDB_INVALID_REGNUM);
reg_info.invalidate_regs = invalidate_regs.data();
@@ -4428,11 +4430,13 @@
reg_info.byte_offset = reg_offset;
assert(reg_info.byte_size != 0);
- reg_offset += reg_info.byte_size;
+
if (!value_regs.empty()) {
value_regs.push_back(LLDB_INVALID_REGNUM);
reg_info.value_regs = value_regs.data();
- }
+ } else
+ reg_offset += reg_info.byte_size;
+
if (!invalidate_regs.empty()) {
invalidate_regs.push_back(LLDB_INVALID_REGNUM);
reg_info.invalidate_regs = invalidate_regs.data();
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1795,8 +1795,10 @@
response.PutChar(';');
}
- response.Printf("bitsize:%" PRIu32 ";offset:%" PRIu32 ";",
- reg_info->byte_size * 8, reg_info->byte_offset);
+ response.Printf("bitsize:%" PRIu32 ";", reg_info->byte_size * 8);
+
+ if (!reg_context.RegisterOffsetIsDynamic())
+ response.Printf("offset:%" PRIu32 ";", reg_info->byte_offset);
llvm::StringRef encoding = GetEncodingNameOrEmpty(*reg_info);
if (!encoding.empty())
@@ -2857,10 +2859,11 @@
continue;
}
- response.Printf("<reg name=\"%s\" bitsize=\"%" PRIu32 "\" offset=\"%" PRIu32
- "\" regnum=\"%d\" ",
- reg_info->name, reg_info->byte_size * 8,
- reg_info->byte_offset, reg_index);
+ response.Printf("<reg name=\"%s\" bitsize=\"%" PRIu32 "\" regnum=\"%d\" ",
+ reg_info->name, reg_info->byte_size * 8, reg_index);
+
+ if (!reg_context.RegisterOffsetIsDynamic())
+ response.Printf("offset=\"%" PRIu32 "\" ", reg_info->byte_offset);
if (reg_info->alt_name && reg_info->alt_name[0])
response.Printf("altname=\"%s\" ", reg_info->alt_name);
Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
===================================================================
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -531,6 +531,18 @@
}
}
+ // On AArch64 architecture we set offsets on client side in accordance with
+ // GDB g/G packet standard in sequence of increasing register numbers.
+ // All primary registers (i.e., reg.value_regs == nullptr) will have unique
+ // offset and registers with value_regs list populated will share same offset
+ // as that of the primary register.
+ if (arch.GetTriple().isAArch64()) {
+ for (auto ® : m_regs)
+ if (reg.value_regs != nullptr)
+ reg.byte_offset =
+ GetRegisterInfoAtIndex(reg.value_regs[0])->byte_offset;
+ }
+
if (!generic_regs_specified) {
switch (arch.GetMachine()) {
case llvm::Triple::aarch64:
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -44,6 +44,8 @@
void InvalidateAllRegisters() override;
+ bool RegisterOffsetIsDynamic() const override { return true; }
+
// Hardware breakpoints/watchpoint management functions
uint32_t NumSupportedHardwareBreakpoints() override;
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -664,7 +664,10 @@
# Check the bare-minimum expected set of register info keys.
self.assertTrue("name" in reg_info)
self.assertTrue("bitsize" in reg_info)
- self.assertTrue("offset" in reg_info)
+
+ if not self.getArchitecture() == 'aarch64':
+ self.assertTrue("offset" in reg_info)
+
self.assertTrue("encoding" in reg_info)
self.assertTrue("format" in reg_info)
Index: lldb/include/lldb/Host/common/NativeRegisterContext.h
===================================================================
--- lldb/include/lldb/Host/common/NativeRegisterContext.h
+++ lldb/include/lldb/Host/common/NativeRegisterContext.h
@@ -116,6 +116,8 @@
virtual NativeThreadProtocol &GetThread() { return m_thread; }
+ virtual bool RegisterOffsetIsDynamic() const { return false; }
+
const RegisterInfo *GetRegisterInfoByName(llvm::StringRef reg_name,
uint32_t start_idx = 0);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
