Fix stepping away from the bottom most level of the inline call stack #114335
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
The test is currently passing everywhere but this 32-bit arm ubuntu bot. I don't have an easy way to debug this, so I'm skipping the test on that platform till we get a chance to figure this out.
Member
|
@llvm/pr-subscribers-lldb Author: None (jimingham) ChangesThe "Thread::IsVirtualStep" function was incorrect, it just checked whether there was an inlined call stack, but if you are at the bottom of the inlined call stack, then it is NOT a virtual step. Full diff: https://github.com/llvm/llvm-project/pull/114335.diff 3 Files Affected:
diff --git a/lldb/source/Target/ThreadPlanStepInRange.cpp b/lldb/source/Target/ThreadPlanStepInRange.cpp
index 325a70619908b6..224a17d896ccf0 100644
--- a/lldb/source/Target/ThreadPlanStepInRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepInRange.cpp
@@ -489,7 +489,8 @@ bool ThreadPlanStepInRange::DoWillResume(lldb::StateType resume_state,
bool ThreadPlanStepInRange::IsVirtualStep() {
if (m_virtual_step == eLazyBoolCalculate) {
Thread &thread = GetThread();
- if (thread.GetCurrentInlinedDepth() == UINT32_MAX)
+ uint32_t cur_inline_depth = thread.GetCurrentInlinedDepth();
+ if (cur_inline_depth == UINT32_MAX || cur_inline_depth == 0)
m_virtual_step = eLazyBoolNo;
else
m_virtual_step = eLazyBoolYes;
diff --git a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
index f52e0f0fd5bcfe..5997ffd38a2d0c 100644
--- a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
+++ b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
@@ -14,6 +14,8 @@ class TestInlineStepping(TestBase):
compiler="icc",
bugnumber="# Not really a bug. ICC combines two inlined functions.",
)
+
+ @skipIf(oslist=["ubuntu"], archs=["arm"]) # Fails for 32 bit arm
def test_with_python_api(self):
"""Test stepping over and into inlined functions."""
self.build()
@@ -364,7 +366,7 @@ def step_in_template(self):
step_sequence = [["// In max_value specialized", "into"]]
self.run_step_sequence(step_sequence)
- def run_to_call_site_and_step(self, source_regex, func_name, start_pos):
+ def run_to_call_site_and_step(self, source_regex, func_name, start_pos, one_more_step_loc = None):
main_spec = lldb.SBFileSpec("calling.cpp")
# Set the breakpoint by file and line, not sourced regex because
# we want to make sure we can set breakpoints on call sites:
@@ -408,6 +410,11 @@ def run_to_call_site_and_step(self, source_regex, func_name, start_pos):
# stepping for this function...
break
+ if one_more_step_loc:
+ thread.StepInto()
+ frame_0 = thread.frame[0]
+ self.assertEqual(frame_0.line_entry.line, line_number(self.main_source, one_more_step_loc),
+ "Was able to step one more time")
process.Kill()
target.Clear()
@@ -420,3 +427,7 @@ def virtual_inline_stepping(self):
self.run_to_call_site_and_step(
"In caller_trivial_inline_2", "caller_trivial_inline_2", 3
)
+ self.run_to_call_site_and_step(
+ "In caller_trivial_inline_3", "caller_trivial_inline_3", 4, "After caller_trivial_inline_3"
+ )
+
diff --git a/lldb/test/API/functionalities/inline-stepping/calling.cpp b/lldb/test/API/functionalities/inline-stepping/calling.cpp
index d7ee56b3c07909..7ed88a872c4eba 100644
--- a/lldb/test/API/functionalities/inline-stepping/calling.cpp
+++ b/lldb/test/API/functionalities/inline-stepping/calling.cpp
@@ -95,7 +95,7 @@ void caller_trivial_inline_1() {
void caller_trivial_inline_2() {
caller_trivial_inline_3(); // In caller_trivial_inline_2.
- inline_value += 1;
+ inline_value += 1; // After caller_trivial_inline_3
}
void caller_trivial_inline_3() {
|
You can test this locally with the following command:darker --check --diff -r 5545f76dc94e76ef6800823bdd1e107ad2264717...14c19bf9a7ad0b13a96be1ad45531d96fe6fae0d lldb/test/API/functionalities/inline-stepping/TestInlineStepping.pyView the diff from darker here.--- TestInlineStepping.py 2024-10-31 00:18:39.000000 +0000
+++ TestInlineStepping.py 2024-10-31 00:33:15.073065 +0000
@@ -12,12 +12,11 @@
@skipIf(oslist=["windows"], archs=["aarch64"]) # Flaky on buildbot
@expectedFailureAll(
compiler="icc",
bugnumber="# Not really a bug. ICC combines two inlined functions.",
)
-
- @skipIf(oslist=["ubuntu"], archs=["arm"]) # Fails for 32 bit arm
+ @skipIf(oslist=["ubuntu"], archs=["arm"]) # Fails for 32 bit arm
def test_with_python_api(self):
"""Test stepping over and into inlined functions."""
self.build()
self.inline_stepping()
@@ -364,11 +363,13 @@
self.thread = threads[0]
step_sequence = [["// In max_value specialized", "into"]]
self.run_step_sequence(step_sequence)
- def run_to_call_site_and_step(self, source_regex, func_name, start_pos, one_more_step_loc = None):
+ def run_to_call_site_and_step(
+ self, source_regex, func_name, start_pos, one_more_step_loc=None
+ ):
main_spec = lldb.SBFileSpec("calling.cpp")
# Set the breakpoint by file and line, not sourced regex because
# we want to make sure we can set breakpoints on call sites:
call_site_line_num = line_number(self.main_source, source_regex)
target, process, thread, bkpt = lldbutil.run_to_line_breakpoint(
@@ -411,12 +412,15 @@
break
if one_more_step_loc:
thread.StepInto()
frame_0 = thread.frame[0]
- self.assertEqual(frame_0.line_entry.line, line_number(self.main_source, one_more_step_loc),
- "Was able to step one more time")
+ self.assertEqual(
+ frame_0.line_entry.line,
+ line_number(self.main_source, one_more_step_loc),
+ "Was able to step one more time",
+ )
process.Kill()
target.Clear()
def virtual_inline_stepping(self):
"""Use the Python API's to step through a virtual inlined stack"""
@@ -426,8 +430,10 @@
)
self.run_to_call_site_and_step(
"In caller_trivial_inline_2", "caller_trivial_inline_2", 3
)
self.run_to_call_site_and_step(
- "In caller_trivial_inline_3", "caller_trivial_inline_3", 4, "After caller_trivial_inline_3"
- )
-
+ "In caller_trivial_inline_3",
+ "caller_trivial_inline_3",
+ 4,
+ "After caller_trivial_inline_3",
+ )
|
You can test this locally with the following command:git-clang-format --diff 5545f76dc94e76ef6800823bdd1e107ad2264717 14c19bf9a7ad0b13a96be1ad45531d96fe6fae0d --extensions cpp -- lldb/source/Target/ThreadPlanStepInRange.cpp lldb/test/API/functionalities/inline-stepping/calling.cppView the diff from clang-format here.diff --git a/lldb/test/API/functionalities/inline-stepping/calling.cpp b/lldb/test/API/functionalities/inline-stepping/calling.cpp
index 7ed88a872c..ba71c25a3c 100644
--- a/lldb/test/API/functionalities/inline-stepping/calling.cpp
+++ b/lldb/test/API/functionalities/inline-stepping/calling.cpp
@@ -95,7 +95,7 @@ void caller_trivial_inline_1() {
void caller_trivial_inline_2() {
caller_trivial_inline_3(); // In caller_trivial_inline_2.
- inline_value += 1; // After caller_trivial_inline_3
+ inline_value += 1; // After caller_trivial_inline_3
}
void caller_trivial_inline_3() {
|
Collaborator
Author
|
I have to redo this, the repo I started from had an older change as well. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The "Thread::IsVirtualStep" function was incorrect, it just checked whether there was an inlined call stack, but if you are at the bottom of the inlined call stack, then it is NOT a virtual step.
I fixed the algorithm, and added a test case for stepping away from the bottom-most level of the inlined call stack.