omjavaid updated this revision to Diff 305680.
omjavaid added a comment.
This update tries to fix concerns raised by removing AArch64 specific code and
makes offset assignment more generic.
We now have a new flag which is set to true if offset field was present and on
basis of that we assign offsets to Primary and Pseudo registers.
If offset is not specified for a primary register then its offset is equal to
last_register_offset + last_register_size. This is what LLDB was doing already
we just restrict this scheme to primary registers only.
If offset is not specified for a pseudo register then fall-back offset is same
as the offset of its first value_reg in value_regs list.
For assigning offsets to pseudo registers all primary registers should have
valid offsets. Primary registers are assigned offsets in ParseRegisters and
ProcessGDBRemote::BuildDynamicRegisterInfo while pseudo register offsets are
update once their value_regs are known in DynamicRegisterInfo::Finalize
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91241/new/
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
@@ -472,6 +472,7 @@
std::vector<uint32_t> value_regs;
std::vector<uint32_t> invalidate_regs;
std::vector<uint8_t> dwarf_opcode_bytes;
+ bool offset_field_set = false;
RegisterInfo reg_info = {
nullptr, // Name
nullptr, // Alt name
@@ -501,6 +502,7 @@
value.getAsInteger(0, reg_info.byte_size);
reg_info.byte_size /= CHAR_BIT;
} else if (name.equals("offset")) {
+ offset_field_set = true;
if (value.getAsInteger(0, reg_offset))
reg_offset = UINT32_MAX;
} else if (name.equals("encoding")) {
@@ -561,12 +563,17 @@
}
}
- 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();
+ if (offset_field_set)
+ reg_info.byte_offset = reg_offset;
+ else
+ reg_info.byte_offset = LLDB_INVALID_INDEX32;
+ } else {
+ reg_info.byte_offset = reg_offset;
+ reg_offset += reg_info.byte_size;
}
if (!invalidate_regs.empty()) {
invalidate_regs.push_back(LLDB_INVALID_REGNUM);
@@ -4293,6 +4300,7 @@
std::vector<uint8_t> dwarf_opcode_bytes;
bool encoding_set = false;
bool format_set = false;
+ bool offset_field_set = false;
RegisterInfo reg_info = {
nullptr, // Name
nullptr, // Alt name
@@ -4316,7 +4324,8 @@
reg_node.ForEachAttribute([&target_info, &gdb_group, &gdb_type,
®_name, &alt_name, &set_name, &value_regs,
&invalidate_regs, &encoding_set, &format_set,
- ®_info, ®_offset, &dwarf_opcode_bytes](
+ ®_info, ®_offset, &offset_field_set,
+ &dwarf_opcode_bytes](
const llvm::StringRef &name,
const llvm::StringRef &value) -> bool {
if (name == "name") {
@@ -4335,6 +4344,7 @@
reg_info.kinds[eRegisterKindProcessPlugin] = regnum;
}
} else if (name == "offset") {
+ offset_field_set = true;
reg_offset = StringConvert::ToUInt32(value.data(), UINT32_MAX, 0);
} else if (name == "altname") {
alt_name.SetString(value);
@@ -4430,12 +4440,17 @@
}
}
- 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();
+ if (offset_field_set)
+ reg_info.byte_offset = reg_offset;
+ else
+ reg_info.byte_offset = LLDB_INVALID_INDEX32;
+ } else {
+ reg_info.byte_offset = reg_offset;
+ reg_offset += reg_info.byte_size;
}
if (!invalidate_regs.empty()) {
invalidate_regs.push_back(LLDB_INVALID_REGNUM);
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
@@ -1825,8 +1825,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())
@@ -2887,10 +2889,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
@@ -466,9 +466,15 @@
// Now update all value_regs with each register info as needed
const size_t num_regs = m_regs.size();
for (size_t i = 0; i < num_regs; ++i) {
- if (m_value_regs_map.find(i) != m_value_regs_map.end())
+ if (m_value_regs_map.find(i) != m_value_regs_map.end()) {
m_regs[i].value_regs = m_value_regs_map[i].data();
- else
+ // Assign a valid offset to all pseudo registers if not assigned by stub.
+ // Pseudo registers with value_regs list populated will share same offset
+ // as that of their corresponding primary register in value_regs list.
+ if (m_regs[i].byte_offset == LLDB_INVALID_INDEX32)
+ m_regs[i].byte_offset =
+ GetRegisterInfoAtIndex(m_regs[i].value_regs[0])->byte_offset;
+ } else
m_regs[i].value_regs = nullptr;
}
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
