[Lldb-commits] [lldb] [lldb] Don't call AddRemoteRegisters if the target XML did not include any registers (PR #96907)
David Spickett via lldb-commits
lldb-commits at lists.llvm.org
Thu Jun 27 06:35:44 PDT 2024
https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/96907
Fixes #92541
When e69a3d18f48bc0d81b5dd12e735a2ec898ce64d added fallback register layouts, it assumed that the choices were target XML with registers, or no target XML at all.
In the linked issue, a user has a debug stub that does have target XML, but it's missing register information.
This caused us to finalize the register information using an empty set of registers got from target XML, then fail an assert when we attempted to add the fallback set. Since we think we've already completed the register information.
This change adds a check to prevent that first call and expands the existing tests to check each architecture without target XML and with target XML missing register information.
>From 25905a0f01c126dbebc854affd30d9ff838f259b Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Thu, 27 Jun 2024 10:55:28 +0000
Subject: [PATCH] [lldb] Don't call AddRemoteRegisters if the target XML did
not include any registers
Fixes #92541
When e69a3d18f48bc0d81b5dd12e735a2ec898ce64d added fallback register layouts,
it assumed that the choices were target XML with registers, or no target XML
at all.
In the linked issue, a user has a debug stub that does have target XML,
but it's missing register information.
This caused us to finalize the register information using an empty set
of registers got from target XML, then fail an assert when we attempted to add
the fallback set. Since we think we've already completed the register
information.
This change adds a check to prevent that first call and expands the
existing tests to check each archictecture without target XML and
with target XML missing register information.
---
.../Process/gdb-remote/ProcessGDBRemote.cpp | 4 +-
lldb/source/Target/DynamicRegisterInfo.cpp | 1 +
...y => TestGDBServerNoTargetXMLRegisters.py} | 78 +++++++++++++++----
3 files changed, 69 insertions(+), 14 deletions(-)
rename lldb/test/API/functionalities/gdb_remote_client/{TestGDBServerNoTargetXML.py => TestGDBServerNoTargetXMLRegisters.py} (82%)
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 3195587eb5676..7f1ce2303cde6 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4879,7 +4879,9 @@ bool ProcessGDBRemote::GetGDBServerRegisterInfo(ArchSpec &arch_to_use) {
m_registers_enum_types.clear();
std::vector<DynamicRegisterInfo::Register> registers;
if (GetGDBServerRegisterInfoXMLAndProcess(arch_to_use, "target.xml",
- registers))
+ registers) &&
+ // Target XML is not required to include register information.
+ (!registers.empty()))
AddRemoteRegisters(registers, arch_to_use);
return m_register_info_sp->GetNumRegisters() > 0;
diff --git a/lldb/source/Target/DynamicRegisterInfo.cpp b/lldb/source/Target/DynamicRegisterInfo.cpp
index 1a817449fa958..48f9a49f8d45a 100644
--- a/lldb/source/Target/DynamicRegisterInfo.cpp
+++ b/lldb/source/Target/DynamicRegisterInfo.cpp
@@ -437,6 +437,7 @@ size_t DynamicRegisterInfo::SetRegisterInfo(
}
void DynamicRegisterInfo::Finalize(const ArchSpec &arch) {
+ printf("DynamicRegisterInfo::Finalize\n");
if (m_finalized)
return;
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerNoTargetXML.py b/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerNoTargetXMLRegisters.py
similarity index 82%
rename from lldb/test/API/functionalities/gdb_remote_client/TestGDBServerNoTargetXML.py
rename to lldb/test/API/functionalities/gdb_remote_client/TestGDBServerNoTargetXMLRegisters.py
index 8c5bad007f569..7557c8108439f 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerNoTargetXML.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerNoTargetXMLRegisters.py
@@ -1,6 +1,6 @@
"""
Check that lldb falls back to default register layouts when the remote provides
-no target XML.
+no target XML or does not include registers in the target XML.
GPRS are passed to the responder to create register data to send back to lldb.
Registers in SUPPL are virtual registers based on those general ones. The tests
@@ -14,6 +14,7 @@
from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
import binascii
+from textwrap import dedent
class MyResponder(MockGDBServerResponder):
@staticmethod
@@ -22,8 +23,10 @@ def filecheck_to_blob(fc):
val = l.split("0x")[1]
yield binascii.b2a_hex(bytes(reversed(binascii.a2b_hex(val)))).decode()
- def __init__(self, reg_data, halt_reason):
+ def __init__(self, architecture, has_target_xml, reg_data, halt_reason):
super().__init__()
+ self.architecture = architecture
+ self.has_target_xml = has_target_xml
self.reg_data = "".join(self.filecheck_to_blob(reg_data))
self.halt_reason = halt_reason
@@ -36,13 +39,19 @@ def readRegisters(self):
def haltReason(self):
return self.halt_reason
+ def qXferRead(self, obj, annex, offset, length):
+ if self.has_target_xml and annex == "target.xml":
+ return dedent(f"""\
+ <?xml version="1.0"?>
+ <target version="1.0">
+ <architecture>{self.architecture}</architecture>
+ </target>"""), False
+
+ return None, False
-class TestGDBServerTargetXML(GDBRemoteTestBase):
- @skipIfRemote
- @skipIfLLVMTargetMissing("X86")
- def test_x86_64_regs(self):
- """Test grabbing various x86_64 registers from gdbserver."""
+class TestGDBServerTargetXML(GDBRemoteTestBase):
+ def check_x86_64_regs(self, has_target_xml):
GPRS = """
CHECK-AMD64-DAG: rax = 0x0807060504030201
CHECK-AMD64-DAG: rbx = 0x1817161514131211
@@ -125,6 +134,8 @@ def test_x86_64_regs(self):
"""
self.server.responder = MyResponder(
+ "x86-64",
+ has_target_xml,
GPRS,
"T02thread:1ff0d;threads:1ff0d;thread-pcs:000000010001bc00;07:0102030405060708;10:1112131415161718;",
)
@@ -155,10 +166,21 @@ def test_x86_64_regs(self):
self.match("register read flags", ["eflags = 0x1c1b1a19"])
@skipIfRemote
- @skipIfLLVMTargetMissing("AArch64")
- def test_aarch64_regs(self):
- """Test grabbing various aarch64 registers from gdbserver."""
+ @skipIfLLVMTargetMissing("X86")
+ def test_x86_64_regs_no_target_xml(self):
+ """Test grabbing various x86_64 registers from gdbserver when there
+ is no target XML."""
+ self.check_x86_64_regs(False)
+ @skipIfXmlSupportMissing
+ @skipIfRemote
+ @skipIfLLVMTargetMissing("X86")
+ def test_x86_64_regs_no_register_info(self):
+ """Test grabbing various x86_64 registers from gdbserver when there
+ is target XML but it does not include register info."""
+ self.check_x86_64_regs(True)
+
+ def check_aarch64_regs(self, has_target_xml):
GPRS = """
CHECK-AARCH64-DAG: x0 = 0x0001020304050607
CHECK-AARCH64-DAG: x1 = 0x0102030405060708
@@ -232,6 +254,8 @@ def test_aarch64_regs(self):
"""
self.server.responder = MyResponder(
+ "aarch64",
+ has_target_xml,
GPRS,
"T02thread:1ff0d;threads:1ff0d;thread-pcs:000000010001bc00;07:0102030405060708;10:1112131415161718;",
)
@@ -258,10 +282,21 @@ def test_aarch64_regs(self):
self.match("register read flags", ["cpsr = 0x21222324"])
@skipIfRemote
- @skipIfLLVMTargetMissing("X86")
- def test_i386_regs(self):
- """Test grabbing various i386 registers from gdbserver."""
+ @skipIfLLVMTargetMissing("AArch64")
+ def test_aarch64_regs_no_target_xml(self):
+ """Test grabbing various aarch64 registers from gdbserver when there
+ is no target XML."""
+ self.check_aarch64_regs(False)
+ @skipIfXmlSupportMissing
+ @skipIfRemote
+ @skipIfLLVMTargetMissing("AArch64")
+ def test_aarch64_regs_no_register_info(self):
+ """Test grabbing various aarch64 registers from gdbserver when there
+ is target XML but it does not include register info."""
+ self.check_aarch64_regs(True)
+
+ def check_i386_regs(self, has_target_xml):
GPRS = """
CHECK-I386-DAG: eax = 0x04030201
CHECK-I386-DAG: ecx = 0x14131211
@@ -307,6 +342,8 @@ def test_i386_regs(self):
"""
self.server.responder = MyResponder(
+ "i386",
+ has_target_xml,
GPRS,
"T02thread:1ff0d;threads:1ff0d;thread-pcs:000000010001bc00;07:0102030405060708;10:1112131415161718;",
)
@@ -329,3 +366,18 @@ def test_i386_regs(self):
self.match("register read sp", ["esp = 0x44434241"])
self.match("register read pc", ["eip = 0x84838281"])
self.match("register read flags", ["eflags = 0x94939291"])
+
+ @skipIfRemote
+ @skipIfLLVMTargetMissing("X86")
+ def test_i386_regs_no_target_xml(self):
+ """Test grabbing various i386 registers from gdbserver when there is
+ no target XML."""
+ self.check_i386_regs(False)
+
+ @skipIfXmlSupportMissing
+ @skipIfRemote
+ @skipIfLLVMTargetMissing("X86")
+ def test_i386_regs_no_register_info(self):
+ """Test grabbing various i386 registers from gdbserver when there is
+ target XML but it does not include register info."""
+ self.check_i386_regs(True)
\ No newline at end of file
More information about the lldb-commits
mailing list