https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/212727
>From 248efcbe7a2e64f1f1801011c922513256075b7d Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Tue, 28 Jul 2026 14:41:12 +0000 Subject: [PATCH 1/5] [lldb] Fix GetIndexOfChildWithName on register sets And GetChildMemberWithName which had the same issue. Fixes #211787. Both of these methods were doing a lookup on the register info array overall, rather than the subset of indexes into that array. That subset is the "register set". This lead to problems like this where index and name getters disagreed: >>> lldb.frame.GetRegisters()[1].GetChildAtIndex(0) (unsigned char __attribute__((ext_vector_type(16)))) v0 = (0x2f, 0x2f, 0x2f, 0x2f, 0x2f, 0x2f, 0x2f, 0x2f, 0x2f, 0x2f, 0x2f, 0x2f, 0x2f, 0x2f, 0x2f, 0x2f) >>> lldb.frame.GetRegisters()[1].GetIndexOfChildWithName("v0") 63 And it meant you could get a register using ChildMemberWithName on a register set that did not contain the register. To fix this I've changed both to look through only the register infos of the registers within the set. This exposed some problems in existing tests, which I've fixed and I've added a full test that checks both methods agree. That test is not checking the literal values (the integer values) between registers because: * Overlapping register names is very unlikely. * Some registers have different types (e.g. floating point) and I didn't want to complicate the test with N different comparison functions. Keeping the test high level means it can run anywhere. Aside from the alias check, which has to be target specific. --- .../lldb/ValueObject/ValueObjectRegister.h | 3 + .../ValueObject/ValueObjectRegister.cpp | 41 +++++++---- .../minidump-new/TestMiniDumpNew.py | 8 +-- .../TestAArch64LinuxAArch32Compat.py | 19 +---- .../test/API/python_api/value/TestValueAPI.py | 69 +++++++++++++++++++ llvm/docs/ReleaseNotes.md | 11 +++ 6 files changed, 117 insertions(+), 34 deletions(-) diff --git a/lldb/include/lldb/ValueObject/ValueObjectRegister.h b/lldb/include/lldb/ValueObject/ValueObjectRegister.h index 3db4b00bd1b15..54ead9f385b2b 100644 --- a/lldb/include/lldb/ValueObject/ValueObjectRegister.h +++ b/lldb/include/lldb/ValueObject/ValueObjectRegister.h @@ -75,6 +75,9 @@ class ValueObjectRegisterSet : public ValueObject { return nullptr; } + std::optional<std::pair<size_t, const RegisterInfo *>> + GetRegisterInfoForChildWithName(llvm::StringRef name); + // For ValueObject only ValueObjectRegisterSet(const ValueObjectRegisterSet &) = delete; const ValueObjectRegisterSet & diff --git a/lldb/source/ValueObject/ValueObjectRegister.cpp b/lldb/source/ValueObject/ValueObjectRegister.cpp index 0d6e54b39ac1d..8cb8e8e65b11a 100644 --- a/lldb/source/ValueObject/ValueObjectRegister.cpp +++ b/lldb/source/ValueObject/ValueObjectRegister.cpp @@ -125,28 +125,41 @@ ValueObject *ValueObjectRegisterSet::CreateChildAtIndex(size_t idx) { return nullptr; } +std::optional<std::pair<size_t, const RegisterInfo *>> +ValueObjectRegisterSet::GetRegisterInfoForChildWithName(llvm::StringRef name) { + if (!m_reg_ctx_sp || !m_reg_set) + return {}; + + for (size_t i = 0; i < m_reg_set->num_registers; ++i) { + if (const RegisterInfo *reg_info = + m_reg_ctx_sp->GetRegisterInfoAtIndex(m_reg_set->registers[i])) { + if (name.equals_insensitive(reg_info->name) || + name.equals_insensitive(reg_info->alt_name)) { + return std::make_pair(i, reg_info); + } + } + } + + return {}; +} + lldb::ValueObjectSP ValueObjectRegisterSet::GetChildMemberWithName(llvm::StringRef name, bool can_create) { - ValueObject *valobj = nullptr; - if (m_reg_ctx_sp && m_reg_set) { - const RegisterInfo *reg_info = m_reg_ctx_sp->GetRegisterInfoByName(name); - if (reg_info != nullptr) - valobj = new ValueObjectRegister(*this, m_reg_ctx_sp, reg_info); - } - if (valobj) + if (auto maybe_info = GetRegisterInfoForChildWithName(name)) { + ValueObject *valobj = + new ValueObjectRegister(*this, m_reg_ctx_sp, maybe_info->second); return valobj->GetSP(); - else - return ValueObjectSP(); + } + + return {}; } llvm::Expected<size_t> ValueObjectRegisterSet::GetIndexOfChildWithName(llvm::StringRef name) { - if (m_reg_ctx_sp && m_reg_set) { - const RegisterInfo *reg_info = m_reg_ctx_sp->GetRegisterInfoByName(name); - if (reg_info != nullptr) - return reg_info->kinds[eRegisterKindLLDB]; - } + if (auto maybe_info = GetRegisterInfoForChildWithName(name)) + return maybe_info->first; + return llvm::createStringErrorV("type has no child named '{0}'", name); } diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py index 4b7d24ef58e7e..36ca8c3d5754b 100644 --- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py +++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py @@ -245,8 +245,8 @@ def test_arm64_registers(self): self.check_register_string_value(fpr, "d%i" % (i), d, lldb.eFormatHex) self.check_register_string_value(fpr, "s%i" % (i), s, lldb.eFormatHex) self.check_register_string_value(fpr, "h%i" % (i), h, lldb.eFormatHex) - self.check_register_unsigned(gpr, "fpsr", 0x55667788) - self.check_register_unsigned(gpr, "fpcr", 0x99AABBCC) + self.check_register_unsigned(fpr, "fpsr", 0x55667788) + self.check_register_unsigned(fpr, "fpcr", 0x99AABBCC) def verify_arm_registers(self, apple=False): """ @@ -265,7 +265,7 @@ def verify_arm_registers(self, apple=False): self.assertEqual(stop_description, "") registers = thread.GetFrameAtIndex(0).GetRegisters() # Verify the GPR registers are all correct - # Verify x0 - x31 register values + # Verify r0 - r15 register values gpr = registers.GetValueAtIndex(0) for i in range(1, 16): self.check_register_unsigned(gpr, "r%i" % (i), i + 1) @@ -284,7 +284,7 @@ def verify_arm_registers(self, apple=False): # Verify the FPR registers are all correct fpr = registers.GetValueAtIndex(1) # Check d0 - d31 - self.check_register_unsigned(gpr, "fpscr", 0x55667788AABBCCDD) + self.check_register_unsigned(fpr, "fpscr", 0x55667788AABBCCDD) for i in range(32): value = (i + 1) | (i + 1) << 8 | (i + 1) << 32 | (i + 1) << 48 self.check_register_unsigned(fpr, "d%i" % (i), value) diff --git a/lldb/test/API/linux/aarch64/aarch32_compat/TestAArch64LinuxAArch32Compat.py b/lldb/test/API/linux/aarch64/aarch32_compat/TestAArch64LinuxAArch32Compat.py index b9a83b06cb59d..d783f5eb60802 100644 --- a/lldb/test/API/linux/aarch64/aarch32_compat/TestAArch64LinuxAArch32Compat.py +++ b/lldb/test/API/linux/aarch64/aarch32_compat/TestAArch64LinuxAArch32Compat.py @@ -92,23 +92,10 @@ def test_aarch32_compat(self): fpr = registers[1] - # FIXME: there is a bug with fpr register indexes where it seems to be - # counting the GPRs as part of itself: - # (Pdb) fpr.GetChildAtIndex(0) - # (float) s0 = 1.40129846E-45 - # (Pdb) fpr.GetIndexOfChildWithName("s0") - # 17 - # (Pdb) fpr.GetChildAtIndex(17) - # (float) s17 = 2.52233724E-44 - # - # See https://github.com/llvm/llvm-project/issues/211787. - # - # So we will assume that index 0 is s0 and not go via name lookup for - # fpr. - - expected_fpr = {} + # Check s0-s31. for n in range(32): - reg = fpr.GetChildAtIndex(n) + reg = fpr.GetChildMemberWithName(f"s{n}") + self.assertTrue(reg.IsValid()) # We cannot call GetValueAsUnsigned on the value directly, as these # are floating point registers. error = lldb.SBError() diff --git a/lldb/test/API/python_api/value/TestValueAPI.py b/lldb/test/API/python_api/value/TestValueAPI.py index dba5f959ba60d..f0acc2beac122 100644 --- a/lldb/test/API/python_api/value/TestValueAPI.py +++ b/lldb/test/API/python_api/value/TestValueAPI.py @@ -282,3 +282,72 @@ def test(self): self.assertEqual( a_null_int_ptr.Dereference().GetLoadAddress(), lldb.LLDB_INVALID_ADDRESS ) + + @no_debug_info_test + def test_register(self): + """ + Test SBValue APIs when the values are backed by registers. + """ + d = {"EXE": self.exe_name} + self.build(dictionary=d) + self.setTearDownCleanup(dictionary=d) + exe = self.getBuildArtifact(self.exe_name) + + target = self.dbg.CreateTarget(exe) + self.assertTrue(target, VALID_TARGET) + + breakpoint = target.BreakpointCreateByLocation("main.c", self.line) + self.assertTrue(breakpoint, VALID_BREAKPOINT) + + process = target.LaunchSimple(None, None, self.get_process_working_directory()) + self.assertTrue(process, PROCESS_IS_VALID) + + self.assertState(process.GetState(), lldb.eStateStopped) + thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint) + self.assertTrue( + thread.IsValid(), + "There should be a thread stopped due to breakpoint condition", + ) + frame = thread.GetFrameAtIndex(0) + + register_sets = frame.GetRegisters() + for set_idx in range(register_sets.GetSize()): + reg_set = register_sets.GetValueAtIndex(set_idx) + num_registers = reg_set.GetNumChildren() + + for child_idx in range(num_registers): + reg_value = reg_set.GetChildAtIndex(child_idx) + self.assertTrue(reg_value.IsValid()) + reg_name = reg_value.GetName() + + # GetIndexOfChildWithName should return the same index. + child_with_name_idex = reg_set.GetIndexOfChildWithName(reg_name) + self.assertTrue(child_with_name_idex < num_registers) + self.assertEqual(child_idx, child_with_name_idex) + + # GetChildMemberWithName should return a value with a matching name. + child_member = reg_set.GetChildMemberWithName(reg_name) + self.assertTrue(child_member.IsValid()) + self.assertEqual(reg_name, child_member.GetName()) + + # That lookup should be case insensitive. + child_member = reg_set.GetChildMemberWithName(reg_name.swapcase()) + self.assertTrue(child_member.IsValid()) + # Note that the value's name is the one lldb uses, not the + # differently cased one used to get it. + self.assertEqual(reg_name, child_member.GetName()) + + if self.isAArch64(): + # Name lookup also checks register aliases and is case insensitive. + gpr = register_sets.GetValueAtIndex(0) + + # x30 is the link register. LLDB has lr as the primary name and x30 + # as the alias. + lr = gpr.GetChildMemberWithName("lR") + self.assertTrue(lr.IsValid()) + self.assertEqual("lr", lr.GetName()) + + x30 = gpr.GetChildMemberWithName("X30") + self.assertTrue(x30.IsValid()) + # Note that the SBValue's name is primary name not the alias. + self.assertEqual("lr", x30.GetName()) diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md index 3a88da50ff8b3..e99f093aa26e7 100644 --- a/llvm/docs/ReleaseNotes.md +++ b/llvm/docs/ReleaseNotes.md @@ -113,6 +113,17 @@ Makes programs 10x faster by doing Special New Thing. ### Changes to LLDB +#### SBAPI + +* A [bug](https://github.com/llvm/llvm-project/issues/211787) involving SBValues + representing a register set was fixed. The methods `GetIndexOfChildWithName` + and `GetChildMemberWithName` were incorrectly looking up values in all + register sets. This meant that `GetIndexOfChildWithName` could return an index + greater than the size of the set, and that `GetChildMemberWithName` could + return values that were actually in a different set. Both methods are now fixed + so that they are limited to the registers within the register set. Scripts + using these methods may have to be updated as a result. + #### Windows * Python 3.11 or later is now required for building LLDB 24 on Windows. >From 42b0e98c8cefdb9f143d36c77ac319d705bb79a6 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Wed, 29 Jul 2026 13:23:37 +0000 Subject: [PATCH 2/5] use GetRegisterInfoByName so we preserve behaviour --- .../lldb/ValueObject/ValueObjectRegister.h | 2 +- .../ValueObject/ValueObjectRegister.cpp | 24 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lldb/include/lldb/ValueObject/ValueObjectRegister.h b/lldb/include/lldb/ValueObject/ValueObjectRegister.h index 54ead9f385b2b..57ee2eadce4fb 100644 --- a/lldb/include/lldb/ValueObject/ValueObjectRegister.h +++ b/lldb/include/lldb/ValueObject/ValueObjectRegister.h @@ -76,7 +76,7 @@ class ValueObjectRegisterSet : public ValueObject { } std::optional<std::pair<size_t, const RegisterInfo *>> - GetRegisterInfoForChildWithName(llvm::StringRef name); + LookupChildWithName(llvm::StringRef name); // For ValueObject only ValueObjectRegisterSet(const ValueObjectRegisterSet &) = delete; diff --git a/lldb/source/ValueObject/ValueObjectRegister.cpp b/lldb/source/ValueObject/ValueObjectRegister.cpp index 8cb8e8e65b11a..1168215336bc4 100644 --- a/lldb/source/ValueObject/ValueObjectRegister.cpp +++ b/lldb/source/ValueObject/ValueObjectRegister.cpp @@ -126,19 +126,19 @@ ValueObject *ValueObjectRegisterSet::CreateChildAtIndex(size_t idx) { } std::optional<std::pair<size_t, const RegisterInfo *>> -ValueObjectRegisterSet::GetRegisterInfoForChildWithName(llvm::StringRef name) { +ValueObjectRegisterSet::LookupChildWithName(llvm::StringRef name) { if (!m_reg_ctx_sp || !m_reg_set) return {}; - for (size_t i = 0; i < m_reg_set->num_registers; ++i) { - if (const RegisterInfo *reg_info = - m_reg_ctx_sp->GetRegisterInfoAtIndex(m_reg_set->registers[i])) { - if (name.equals_insensitive(reg_info->name) || - name.equals_insensitive(reg_info->alt_name)) { - return std::make_pair(i, reg_info); - } - } - } + // See if the register exists at all in any set. + const RegisterInfo *reg_info = m_reg_ctx_sp->GetRegisterInfoByName(name); + if (!reg_info) + return {}; + + // See if this register is in this register set. + for (size_t i = 0; i < m_reg_set->num_registers; ++i) + if (reg_info->kinds[eRegisterKindLLDB] == m_reg_set->registers[i]) + return std::make_pair(i, reg_info); return {}; } @@ -146,7 +146,7 @@ ValueObjectRegisterSet::GetRegisterInfoForChildWithName(llvm::StringRef name) { lldb::ValueObjectSP ValueObjectRegisterSet::GetChildMemberWithName(llvm::StringRef name, bool can_create) { - if (auto maybe_info = GetRegisterInfoForChildWithName(name)) { + if (auto maybe_info = LookupChildWithName(name)) { ValueObject *valobj = new ValueObjectRegister(*this, m_reg_ctx_sp, maybe_info->second); return valobj->GetSP(); @@ -157,7 +157,7 @@ ValueObjectRegisterSet::GetChildMemberWithName(llvm::StringRef name, llvm::Expected<size_t> ValueObjectRegisterSet::GetIndexOfChildWithName(llvm::StringRef name) { - if (auto maybe_info = GetRegisterInfoForChildWithName(name)) + if (auto maybe_info = LookupChildWithName(name)) return maybe_info->first; return llvm::createStringErrorV("type has no child named '{0}'", name); >From e745784fe7c16964d00796a75bd9e7ff08549819 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Wed, 29 Jul 2026 13:36:04 +0000 Subject: [PATCH 3/5] spelling --- lldb/test/API/python_api/value/TestValueAPI.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/python_api/value/TestValueAPI.py b/lldb/test/API/python_api/value/TestValueAPI.py index f0acc2beac122..5dc25aad1db66 100644 --- a/lldb/test/API/python_api/value/TestValueAPI.py +++ b/lldb/test/API/python_api/value/TestValueAPI.py @@ -321,9 +321,9 @@ def test_register(self): reg_name = reg_value.GetName() # GetIndexOfChildWithName should return the same index. - child_with_name_idex = reg_set.GetIndexOfChildWithName(reg_name) - self.assertTrue(child_with_name_idex < num_registers) - self.assertEqual(child_idx, child_with_name_idex) + child_with_name_index = reg_set.GetIndexOfChildWithName(reg_name) + self.assertTrue(child_with_name_index < num_registers) + self.assertEqual(child_idx, child_with_name_index) # GetChildMemberWithName should return a value with a matching name. child_member = reg_set.GetChildMemberWithName(reg_name) >From 1bc5b4ec0d9eeed232848bc748609be7b432aca6 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Wed, 29 Jul 2026 14:10:24 +0000 Subject: [PATCH 4/5] x86 rsp->sp --- lldb/test/API/python_api/value/TestValueAPI.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lldb/test/API/python_api/value/TestValueAPI.py b/lldb/test/API/python_api/value/TestValueAPI.py index 5dc25aad1db66..69d70ab420c60 100644 --- a/lldb/test/API/python_api/value/TestValueAPI.py +++ b/lldb/test/API/python_api/value/TestValueAPI.py @@ -320,6 +320,21 @@ def test_register(self): self.assertTrue(reg_value.IsValid()) reg_name = reg_value.GetName() + if ( + self.getArchitecture() in ["amd64", "i386", "x86_64"] + and reg_name == "sp" + ): + # x86 has "rsp", and "sp" which is a subset of "rsp". Then there is + # the ABI name "sp", which LLDB resolves to "rsp", not to the + # architectural register "sp". + # See https://github.com/llvm/llvm-project/issues/212778. + sp_with_name_index = reg_set.GetIndexOfChildWithName(reg_name) + self.assertTrue(sp_with_name_index < num_registers) + rsp_with_name_index = reg_set.GetIndexOfChildWithName("rsp") + self.assertTrue(rsp_with_name_index < num_registers) + self.assertEqual(sp_with_name_index, rsp_with_name_index) + continue + # GetIndexOfChildWithName should return the same index. child_with_name_index = reg_set.GetIndexOfChildWithName(reg_name) self.assertTrue(child_with_name_index < num_registers) >From 93e0b5a1b4e1e6d8a2bea40963068a51c82e6964 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Wed, 29 Jul 2026 14:14:47 +0000 Subject: [PATCH 5/5] another check --- lldb/test/API/python_api/value/TestValueAPI.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lldb/test/API/python_api/value/TestValueAPI.py b/lldb/test/API/python_api/value/TestValueAPI.py index 69d70ab420c60..53fcccbe93146 100644 --- a/lldb/test/API/python_api/value/TestValueAPI.py +++ b/lldb/test/API/python_api/value/TestValueAPI.py @@ -333,6 +333,13 @@ def test_register(self): rsp_with_name_index = reg_set.GetIndexOfChildWithName("rsp") self.assertTrue(rsp_with_name_index < num_registers) self.assertEqual(sp_with_name_index, rsp_with_name_index) + + # FIXME: There is another bug that the conversion only looks for lower + # case names. By using mixed case, you can reach the real "sp. + sp_with_name_index_mixed = reg_set.GetIndexOfChildWithName("sP") + self.assertTrue(sp_with_name_index_mixed < num_registers) + self.assertEqual(sp_with_name_index_mixed, child_idx) + continue # GetIndexOfChildWithName should return the same index. _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
