[lldb] Add SBThread.selected_frame property #123981
Merged
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
|
@llvm/pr-subscribers-lldb Author: Dave Lee (kastiglione) ChangesFull diff: https://github.com/llvm/llvm-project/pull/123981.diff 5 Files Affected:
diff --git a/lldb/bindings/interface/SBThreadExtensions.i b/lldb/bindings/interface/SBThreadExtensions.i
index 860a2d765a6695..926e5b1623c407 100644
--- a/lldb/bindings/interface/SBThreadExtensions.i
+++ b/lldb/bindings/interface/SBThreadExtensions.i
@@ -51,6 +51,12 @@ STRING_EXTENSION_OUTSIDE(SBThread)
for idx in range(self.GetStopReasonDataCount())
]
+ def set_selected_frame(self, frame):
+ if isinstance(frame, SBFrame):
+ self.SetSelectedFrame(frame.idx)
+ else:
+ self.SetSelectedFrame(frame)
+
id = property(GetThreadID, None, doc='''A read only property that returns the thread ID as an integer.''')
idx = property(GetIndexID, None, doc='''A read only property that returns the thread index ID as an integer. Thread index ID values start at 1 and increment as threads come and go and can be used to uniquely identify threads.''')
return_value = property(GetStopReturnValue, None, doc='''A read only property that returns an lldb object that represents the return value from the last stop (lldb.SBValue) if we just stopped due to stepping out of a function.''')
@@ -65,6 +71,7 @@ STRING_EXTENSION_OUTSIDE(SBThread)
stop_reason_data = property(get_stop_reason_data, None, doc='''A read only property that returns the stop reason data as a list.''')
is_suspended = property(IsSuspended, None, doc='''A read only property that returns a boolean value that indicates if this thread is suspended.''')
is_stopped = property(IsStopped, None, doc='''A read only property that returns a boolean value that indicates if this thread is stopped but not exited.''')
+ selected_frame = property(GetSelectedFrame, set_selected_frame, doc='''A read/write property that gets and sets the selected frame of this SBThread.''')
%}
#endif
}
diff --git a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
index aa2a448087431e..3e9dbfe6d85552 100644
--- a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
+++ b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
@@ -20,7 +20,7 @@ def test_frame_recognizer_1(self):
target, process, thread, _ = lldbutil.run_to_name_breakpoint(
self, "foo", exe_name=exe
)
- frame = thread.GetSelectedFrame()
+ frame = thread.selected_frame
# Clear internal & plugins recognizers that get initialized at launch
self.runCmd("frame recognizer clear")
@@ -166,7 +166,7 @@ def test_frame_recognizer_hiding(self):
self.build()
target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "nested")
- frame = thread.GetSelectedFrame()
+ frame = thread.selected_frame
# Sanity check.
self.expect(
@@ -229,7 +229,6 @@ def test_frame_recognizer_multi_symbol(self):
target, process, thread, _ = lldbutil.run_to_name_breakpoint(
self, "foo", exe_name=exe
)
- frame = thread.GetSelectedFrame()
self.expect(
"frame recognizer info 0",
@@ -239,7 +238,6 @@ def test_frame_recognizer_multi_symbol(self):
target, process, thread, _ = lldbutil.run_to_name_breakpoint(
self, "bar", exe_name=exe
)
- frame = thread.GetSelectedFrame()
self.expect(
"frame recognizer info 0",
@@ -374,7 +372,7 @@ def test_frame_recognizer_not_only_first_instruction(self):
opts = lldb.SBVariablesOptions()
opts.SetIncludeRecognizedArguments(True)
- frame = thread.GetSelectedFrame()
+ frame = thread.selected_frame
variables = frame.GetVariables(opts)
self.assertEqual(variables.GetSize(), 2)
diff --git a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 84033daff77308..a97f4fc5e3d799 100644
--- a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++ b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -25,7 +25,7 @@ def check_local_vars(self, process: lldb.SBProcess, check_expr: bool):
# Find `bar` on the stack, then
# make sure we can read out the local
# variables (with both `frame var` and `expr`)
- for f in process.GetSelectedThread().frames:
+ for f in process.selected_thread.frames:
frame_name = f.GetDisplayFunctionName()
if frame_name is not None and frame_name.startswith("Foo::bar"):
argv = f.GetValueForVariablePath("argv").GetChildAtIndex(0)
@@ -34,7 +34,7 @@ def check_local_vars(self, process: lldb.SBProcess, check_expr: bool):
self.assertNotEqual(strm.GetData().find("a.out"), -1)
if check_expr:
- process.GetSelectedThread().SetSelectedFrame(f.idx)
+ process.selected_thread.selected_frame = f
self.expect_expr("this", result_type="Foo *")
@skipIf(oslist=["linux"], archs=["arm"])
diff --git a/lldb/test/API/lang/cpp/std-function-recognizer/TestStdFunctionRecognizer.py b/lldb/test/API/lang/cpp/std-function-recognizer/TestStdFunctionRecognizer.py
index 978bf2066e43b0..f5d0ea41e3114c 100644
--- a/lldb/test/API/lang/cpp/std-function-recognizer/TestStdFunctionRecognizer.py
+++ b/lldb/test/API/lang/cpp/std-function-recognizer/TestStdFunctionRecognizer.py
@@ -69,14 +69,14 @@ def test_up_down(self):
(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
self, "// break here", lldb.SBFileSpec("main.cpp")
)
- frame = thread.GetSelectedFrame()
+ frame = thread.selected_frame
# up
self.assertIn("foo", frame.GetFunctionName())
start_idx = frame.GetFrameID()
i = 0
while i < thread.GetNumFrames():
self.expect("up")
- frame = thread.GetSelectedFrame()
+ frame = thread.selected_frame
if frame.GetFunctionName() == "main":
break
end_idx = frame.GetFrameID()
@@ -86,7 +86,7 @@ def test_up_down(self):
start_idx = frame.GetFrameID()
for i in range(1, thread.GetNumFrames()):
self.expect("down")
- frame = thread.GetSelectedFrame()
+ frame = thread.selected_frame
if "foo" in frame.GetFunctionName():
break
end_idx = frame.GetFrameID()
@@ -99,11 +99,8 @@ def test_api(self):
(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
self, "// break here", lldb.SBFileSpec("main.cpp")
)
- frame = thread.GetSelectedFrame()
num_hidden = 0
- for i in range(1, thread.GetNumFrames()):
- thread.SetSelectedFrame(i)
- frame = thread.GetSelectedFrame()
+ for frame in thread.frames:
if frame.IsHidden():
num_hidden += 1
diff --git a/lldb/test/API/lang/objc/print-obj/TestPrintObj.py b/lldb/test/API/lang/objc/print-obj/TestPrintObj.py
index 60fc4fbc51cee1..125b78f0a30f46 100644
--- a/lldb/test/API/lang/objc/print-obj/TestPrintObj.py
+++ b/lldb/test/API/lang/objc/print-obj/TestPrintObj.py
@@ -69,12 +69,9 @@ def test_print_obj(self):
# We want to traverse the frame to the one corresponding to blocked.m to
# issue our 'po lock_me' command.
- depth = other_thread.GetNumFrames()
- for i in range(depth):
- frame = other_thread.GetFrameAtIndex(i)
- name = frame.GetFunctionName()
- if name == "main":
- other_thread.SetSelectedFrame(i)
+ for frame in other_thread.frames:
+ if frame.name == "main":
+ other_thread.selected_frame = frame
if self.TraceOn():
print("selected frame:" + lldbutil.get_description(frame))
break
|
kastiglione
commented
Jan 22, 2025
Comment on lines
-104
to
-106
| for i in range(1, thread.GetNumFrames()): | ||
| thread.SetSelectedFrame(i) | ||
| frame = thread.GetSelectedFrame() |
There was a problem hiding this comment.
this code didn't need to set the selected frame, it only needs to inspect whether a frame is hidden (which can be done without selecting it).
|
✅ With the latest revision this PR passed the Python code formatter. |
JDevlieghere
approved these changes
Jan 23, 2025
labath
reviewed
Jan 23, 2025
| @@ -51,6 +51,12 @@ STRING_EXTENSION_OUTSIDE(SBThread) | |||
| for idx in range(self.GetStopReasonDataCount()) | |||
| ] | |||
|
|
|||
| def set_selected_frame(self, frame): | |||
| if isinstance(frame, SBFrame): | |||
| self.SetSelectedFrame(frame.idx) | |||
There was a problem hiding this comment.
Maybe it should check that the frame really belongs to this thread before selecting a completely unrelated frame?
There was a problem hiding this comment.
Callers shouldn't have to do that; StackFrames know their Threads, so SetSelectedFrame should really reject this (though in that case it ought to also return an error.)
There was a problem hiding this comment.
SetSelectedFrame returns the index of the selected frame. It should check the thread and if it doesn't match, return LLDB_INVALID_FRAME_ID.
There was a problem hiding this comment.
good call, done.
There was a problem hiding this comment.
oh my reply was to @labath, before I saw @jimingham's comments.
There was a problem hiding this comment.
I've updated this helper method to check the SBFrame argument belongs to the target instance of SBThread. If it doesn't, a ValueError is raised. Does that work for you @jimingham?
|
I still think callers shouldn't have to do this, but to make the internal API good you'd really have to return a Status, so this is okay for now. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/3971 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/197/builds/1111 Here is the relevant piece of the build log for the reference |
kastiglione
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Apr 3, 2025
Adds a `selected_frame` property to `SBThread`. The setter accepts either a frame index (like `SetSelectedFrame`), or a frame object. Updates a few tests to make use of the new `selected_frame`. While doing so I noticed some of the usage could be cleaned up, so I did that too. (cherry picked from commit 7293455)
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.
Adds a
selected_frameproperty toSBThread. The setter accepts either a frame index (likeSetSelectedFrame), or a frame object.Updates a few tests to make use of the new
selected_frame. While doing so I noticed some of the usage could be cleaned up, so I did that too.