[lldb] Don't call AddRemoteRegisters if the target XML did not include any registers #96907
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e any registers Fixes llvm#92541 When e69a3d1 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.
|
@llvm/pr-subscribers-lldb Author: David Spickett (DavidSpickett) ChangesFixes #92541 When e69a3d1 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. Full diff: https://github.com/llvm/llvm-project/pull/96907.diff 3 Files Affected:
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
|
|
✅ With the latest revision this PR passed the Python code formatter. |
lravenclaw
pushed a commit
to lravenclaw/llvm-project
that referenced
this pull request
Jul 3, 2024
…e any registers (llvm#96907) Fixes llvm#92541 When e69a3d1 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.
AlexisPerry
pushed a commit
to llvm-project-tlp/llvm-project
that referenced
this pull request
Jul 9, 2024
…e any registers (llvm#96907) Fixes llvm#92541 When e69a3d1 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #92541
When e69a3d1 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.