-
Notifications
You must be signed in to change notification settings - Fork 11.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix error in unrecognized register name handling for "SBFrame.register" #88047
Conversation
The code returned lldb.SBValue() which is not legal in the module. I changed this to just return SBValue(), but then this construct: (lldb) script >>> for reg_set in lldb.target.process.thread[0].frames[0].register ... print(reg) Runs forever printing "No Value". The __getitem__(key) gets called with an ever increasing by 1 series of integers. I don't know why Python decided the class we defined should have a generator that returns positive integers in order, but we can add a more useful one here by returning an iterator over the flattened list of registers. Note, the not very aptly named "SBFrame.registers" is an iterator over register sets, not registers, so the two are not redundant.
@llvm/pr-subscribers-lldb Author: None (jimingham) ChangesThe code returned lldb.SBValue() when you passed in an unrecognized variable name. But referring to "lldb" is apparently not legal within the module. I changed this to just return SBValue(), but then this construct: (lldb) script Runs forever printing "No Value". The getitem(key) gets called with a monotonically increasing by 1 series of integers. I don't know why Python decided the class we defined should have a generator that returns positive integers in order, but we can add a more useful one here by returning an iterator over the flattened list of registers. Note, the not very aptly named "SBFrame.registers" is an iterator over register sets, not registers, so the two are not redundant. Full diff: https://github.com/llvm/llvm-project/pull/88047.diff 2 Files Affected:
diff --git a/lldb/bindings/interface/SBFrameExtensions.i b/lldb/bindings/interface/SBFrameExtensions.i
index 43b22ed7a6b325..e0472280666ab9 100644
--- a/lldb/bindings/interface/SBFrameExtensions.i
+++ b/lldb/bindings/interface/SBFrameExtensions.i
@@ -44,6 +44,16 @@ STRING_EXTENSION_OUTSIDE(SBFrame)
def __init__(self, regs):
self.regs = regs
+ def __iter__(self):
+ return self.get_registers()
+
+ def get_registers(self):
+ for i in range(0,len(self.regs)):
+ rs = self.regs[i]
+ for j in range (0,rs.num_children):
+ reg = rs.GetChildAtIndex(j)
+ yield reg
+
def __getitem__(self, key):
if type(key) is str:
for i in range(0,len(self.regs)):
@@ -52,7 +62,7 @@ STRING_EXTENSION_OUTSIDE(SBFrame)
reg = rs.GetChildAtIndex(j)
if reg.name == key: return reg
else:
- return lldb.SBValue()
+ return SBValue()
return registers_access(self.registers)
diff --git a/lldb/test/API/python_api/frame/TestFrames.py b/lldb/test/API/python_api/frame/TestFrames.py
index a82b129bc8099d..dfa96d51830bae 100644
--- a/lldb/test/API/python_api/frame/TestFrames.py
+++ b/lldb/test/API/python_api/frame/TestFrames.py
@@ -73,7 +73,19 @@ def test_get_arg_vals_for_call_stack(self):
gpr_reg_set = lldbutil.get_GPRs(frame)
pc_value = gpr_reg_set.GetChildMemberWithName("pc")
self.assertTrue(pc_value, "We should have a valid PC.")
+ # Make sure we can also get this from the "register" property:
+ iterator_pc_value = 0
+ found_pc = False
+ for reg in frame.register:
+ if reg.name == "pc":
+ found_pc = True
+ iterator_pc_value = int(reg.GetValue(), 0)
+ break
+
pc_value_int = int(pc_value.GetValue(), 0)
+ self.assertTrue(found_pc, "Found the PC value in the register list")
+ self.assertEqual(iterator_pc_value, pc_value_int, "The methods of finding pc match")
+
# Make sure on arm targets we dont mismatch PC value on the basis of thumb bit.
# Frame PC will not have thumb bit set in case of a thumb
# instruction as PC.
|
You can test this locally with the following command:darker --check --diff -r 5b959310b0fae723bd119ed8815bf1cb1a8c67d4...d1336d9f8877c45dfd9a427eaa900f5208153de1 lldb/test/API/python_api/frame/TestFrames.py View the diff from darker here.--- TestFrames.py 2024-04-08 21:20:17.000000 +0000
+++ TestFrames.py 2024-04-08 21:34:19.896474 +0000
@@ -82,12 +82,14 @@
iterator_pc_value = int(reg.GetValue(), 0)
break
pc_value_int = int(pc_value.GetValue(), 0)
self.assertTrue(found_pc, "Found the PC value in the register list")
- self.assertEqual(iterator_pc_value, pc_value_int, "The methods of finding pc match")
-
+ self.assertEqual(
+ iterator_pc_value, pc_value_int, "The methods of finding pc match"
+ )
+
# Make sure on arm targets we dont mismatch PC value on the basis of thumb bit.
# Frame PC will not have thumb bit set in case of a thumb
# instruction as PC.
if self.getArchitecture() in ["arm", "armv7", "armv7k"]:
pc_value_int &= ~1
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
LGTM |
….register"" (#88468) Reverts #88047. TestFrames.py is failing on x86_64 GreenDragon: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/983/
The code returned lldb.SBValue() when you passed in an unrecognized register name. But referring to "lldb" is apparently not legal within the module.
I changed this to just return SBValue(), but then this construct:
(lldb) script
Runs forever printing "No Value". The getitem(key) gets called with a monotonically increasing by 1 series of integers. I don't know why Python decided the class we defined should have a generator that returns positive integers in order, but we can add a more useful one here by returning an iterator over the flattened list of registers.
Note, the not very aptly named "SBFrame.registers" is an iterator over register sets, not registers, so the two are not redundant.