[lldb] Expose QueueThreadPlanForStepSingleInstruction function to SBThreadPlan #137904

eronnen · 2025-04-30T00:01:48Z

Expose QueueThreadPlanForStepSingleInstruction function to SBThreadPlan

llvmbot · 2025-04-30T00:02:24Z

@llvm/pr-subscribers-lldb

Author: Ely Ronnen (eronnen)

Changes

Expose QueueThreadPlanForStepSingleInstruction function to SBThreadPlan

Full diff: https://github.com/llvm/llvm-project/pull/137904.diff

2 Files Affected:

(modified) lldb/include/lldb/API/SBThreadPlan.h (+4)
(modified) lldb/source/API/SBThreadPlan.cpp (+31)

diff --git a/lldb/include/lldb/API/SBThreadPlan.h b/lldb/include/lldb/API/SBThreadPlan.h
index d02ab80f76a76..ce1a1078d69b9 100644
--- a/lldb/include/lldb/API/SBThreadPlan.h
+++ b/lldb/include/lldb/API/SBThreadPlan.h
@@ -105,6 +105,10 @@ class LLDB_API SBThreadPlan {
   SBThreadPlan QueueThreadPlanForStepOut(uint32_t frame_idx_to_step_to,
                                          bool first_insn, SBError &error);
 
+  SBThreadPlan QueueThreadPlanForStepSingleInstruction(bool step_over);
+  SBThreadPlan QueueThreadPlanForStepSingleInstruction(bool step_over,
+                                                       SBError &error);
+
   SBThreadPlan QueueThreadPlanForRunToAddress(SBAddress address);
   SBThreadPlan QueueThreadPlanForRunToAddress(SBAddress address,
                                               SBError &error);
diff --git a/lldb/source/API/SBThreadPlan.cpp b/lldb/source/API/SBThreadPlan.cpp
index 083a050de8a38..a85afbb043875 100644
--- a/lldb/source/API/SBThreadPlan.cpp
+++ b/lldb/source/API/SBThreadPlan.cpp
@@ -325,6 +325,37 @@ SBThreadPlan::QueueThreadPlanForStepOut(uint32_t frame_idx_to_step_to,
   return SBThreadPlan();
 }
 
+SBThreadPlan
+SBThreadPlan::QueueThreadPlanForStepSingleInstruction(bool step_over) {
+  LLDB_INSTRUMENT_VA(this, step_over);
+
+  SBError error;
+  return QueueThreadPlanForStepSingleInstruction(step_over, error);
+}
+
+SBThreadPlan
+SBThreadPlan::QueueThreadPlanForStepSingleInstruction(bool step_over,
+                                                      SBError &error) {
+  LLDB_INSTRUMENT_VA(this, step_over, error);
+
+  ThreadPlanSP thread_plan_sp(GetSP());
+  if (thread_plan_sp) {
+    Status plan_status;
+    SBThreadPlan plan(
+        thread_plan_sp->GetThread().QueueThreadPlanForStepSingleInstruction(
+            step_over, false, false, plan_status));
+
+    if (plan_status.Fail())
+      error.SetErrorString(plan_status.AsCString());
+    else
+      plan.GetSP()->SetPrivate(true);
+
+    return plan;
+  }
+
+  return SBThreadPlan();
+}
+
 SBThreadPlan
 SBThreadPlan::QueueThreadPlanForRunToAddress(SBAddress sb_address) {
   LLDB_INSTRUMENT_VA(this, sb_address);

jimingham

You can compose the single step out of the range plans with the range chosen appropriately, but that seems annoying, and this is a pretty useful operation. So adding it is a good idea.

The code looks fine.

Needs a test, however.

eronnen · 2025-04-30T07:37:50Z

Added tests

github-actions · 2025-04-30T07:40:08Z

✅ With the latest revision this PR passed the Python code formatter.

jimingham · 2025-04-30T16:51:50Z

lldb/test/API/functionalities/step_scripted/TestStepScripted.py

+
+        # Verify that stepping over an instruction doesn't step into `foo`
+        frame = thread.GetFrameAtIndex(0)
+        self.assertEqual("main", frame.GetFunctionName())


If the thread plan failed to move at all, this assert would pass. Figuring out where you go with nexti over a function call is a little trickier than stepi, so it's probably okay here to just assert that you didn't step into foo but the PC did change.

You might also assert before the nexti that you really are at a branch (you can get the instruction list for the function, find the instruction you are stopped at and ask it IsBranch. It seems really unlikely that any compiler/architecture would would emit more than a single branch and link for a call to a void (*)(void), but it's better not to rely on that.

makes sense, I didn't have any other straightfoward idea for a 100% way to break on a branch instruction but adding a check is a good idea

jimingham · 2025-04-30T16:55:54Z

lldb/test/API/functionalities/step_scripted/TestStepScripted.py

+
+        # Verify that stepping a single instruction after "foo();" steps into `foo`
+        frame = thread.GetFrameAtIndex(0)
+        self.assertEqual("foo", frame.GetFunctionName())


In the case of step-i, we are really trying to assert that we step just one instruction. That's actually not all that hard to test exactly. Just add some simple arithmetic computation to the test, so it won't have branches, and then stop at the beginning of the computation, run your step-i plan, and assert that you did get to the NEXT instruction on the instruction list.

I think that testing a branch is more powerful though, because in a case of a non branch instruction both stepi and nexti would step into the same instruction

eronnen · 2025-04-30T20:37:05Z

Added branch instructions asserts to the tests

jimingham

LGTM

lldb/include/lldb/API/SBThreadPlan.h

jimingham

Sorry for more round trips, but can you remove the no-SBError overload. Adding that was really a poor choice on my part (which I seem to want to repeat for some reason...)

lldb/include/lldb/API/SBThreadPlan.h

eronnen · 2025-05-01T19:53:25Z

Removed the version without SBError, but I'm not sure what to do with it in the python test because this function is not called in the test body, so we can't assert it

eronnen · 2025-05-01T19:54:15Z

Also note that all the other QueueThreadPlanFor* functions still have a version without SBError :/

jimingham · 2025-05-01T21:50:45Z

Also note that all the other QueueThreadPlanFor* functions still have a version without SBError :/

Yes, we don't remove API's from the SB API set, which means we do get left with our mistakes.

jimingham

LEBTM

…truction

felipepiovezan · 2025-05-08T14:32:16Z

@eronnen this is breaking the incremental green dragon bots.
Could you revert while you look at this please?

https://green.lab.llvm.org//job/as-lldb-cmake/25425/changes#41321416815d74a4a7fd15c78fcfa5af457625bb

[2025-05-08T12:15:32.021Z] ======================================================================
[2025-05-08T12:15:32.021Z] FAIL: test_step_single_instruction (TestStepScripted.StepScriptedTestCase)
[2025-05-08T12:15:32.021Z] ----------------------------------------------------------------------
[2025-05-08T12:15:32.021Z] Traceback (most recent call last):
[2025-05-08T12:15:32.021Z]   File "/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/lldb/test/API/functionalities/step_scripted/TestStepScripted.py", line 63, in test_step_single_instruction
[2025-05-08T12:15:32.021Z]     (target, process, thread, bkpt) = self.run_until_branch_instruction()
[2025-05-08T12:15:32.021Z]   File "/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/lldb/test/API/functionalities/step_scripted/TestStepScripted.py", line 56, in run_until_branch_instruction
[2025-05-08T12:15:32.021Z]     self.assertEqual(
[2025-05-08T12:15:32.021Z] AssertionError: 2 != 0
[2025-05-08T12:15:32.021Z] Config=arm64-/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/lldb-build/bin/clang

felipepiovezan · 2025-05-08T14:51:39Z

lldb/test/API/functionalities/step_scripted/TestStepScripted.py

+        current_instruction = target.ReadInstructions(frame.GetPCAddress(), 1)[0]
+        self.assertEqual(
+            lldb.eInstructionControlFlowKindCall,
+            current_instruction.GetControlFlowKind(target),


FWIW I don't think is implemented for arm targets

This is only implemented for x86. Originally introduced in: #137904

felipepiovezan · 2025-05-08T15:12:36Z

To fix the bots, I've disabled your new test in all non x86 archs, feel free to update if you want to do something else

This is only implemented for x86. Originally introduced in: llvm/llvm-project#137904

slydiman · 2025-05-08T16:14:18Z

The following buildbots are broken by this patch too

https://lab.llvm.org/buildbot/#/builders/195/builds/8715

https://lab.llvm.org/buildbot/#/builders/197/builds/5054

Please fix ASAP.

eronnen · 2025-05-08T16:31:48Z

@felipepiovezan thanks for the temp fix, I'll try to understand the problem soon

eronnen · 2025-05-08T16:32:51Z

@slydiman Seems this buildbot is running aarch64 so this test should be disabled after @felipepiovezan 's fix.

felipepiovezan · 2025-05-08T20:09:41Z

@eronnen my fix does not seem to have worked

felipepiovezan · 2025-05-08T20:15:17Z

Ah there were two tests here. I've disabled the other one too.

felipepiovezan · 2025-05-08T20:15:41Z

2815653 20250508 fpiove.. [lldb] Disable test using GetControlFlowKind on arm (HEAD, llvm/main)

eronnen · 2025-05-08T20:26:48Z

@felipepiovezan Sorry for the trouble, do you know if there's a way for me to run the CI on arm/aarch64 before merging the PR next time?

DavidSpickett · 2025-05-09T09:57:53Z

At this time there is no pre-merge CI for Arm. If you feel like hacking something together, it is possible to use Github's Runners to test it on your own fork. I hope to make that route more user friendly at some point, and expand the automatic CI to Arm when we can.

Fix ups after landing are a fact of life for the time being :)

eronnen requested a review from JDevlieghere as a code owner April 30, 2025 00:01

llvmbot added the lldb label Apr 30, 2025

jimingham requested changes Apr 30, 2025

View reviewed changes

eronnen requested a review from jimingham April 30, 2025 07:37

eronnen force-pushed the lldb-expose-queue-thread-plan-for-single-instruction branch from d0f46d9 to e535984 Compare April 30, 2025 07:42

jimingham reviewed Apr 30, 2025

View reviewed changes

eronnen force-pushed the lldb-expose-queue-thread-plan-for-single-instruction branch from e535984 to b061968 Compare April 30, 2025 20:35

eronnen requested a review from jimingham April 30, 2025 20:37

eronnen force-pushed the lldb-expose-queue-thread-plan-for-single-instruction branch from b061968 to 7ab3ffd Compare April 30, 2025 21:10

jimingham approved these changes Apr 30, 2025

View reviewed changes

da-viper reviewed May 1, 2025

View reviewed changes

lldb/include/lldb/API/SBThreadPlan.h Outdated Show resolved Hide resolved

jimingham requested changes May 1, 2025

View reviewed changes

lldb/include/lldb/API/SBThreadPlan.h Outdated Show resolved Hide resolved

eronnen requested a review from jimingham May 1, 2025 20:09

jimingham approved these changes May 1, 2025

View reviewed changes

eronnen added 5 commits May 7, 2025 22:01

Expose QueueThreadPlanForStepSingleInstruction function to SBThreadPlan

998d59c

Add API tests for QueueThreadPlanForStepSingleInstruction

7cedaf5

improve QueueThreadPlanForStepSingleInstruction tests

b9658d5

force QueueThreadPlanForStepSingleInstruction to accept SBError

4d7a735

format

1faea15

eronnen force-pushed the lldb-expose-queue-thread-plan-for-single-instruction branch from e8aa0cf to 1faea15 Compare May 7, 2025 20:01

Merge branch 'main' into lldb-expose-queue-thread-plan-for-single-ins…

df629ba

…truction

eronnen merged commit 4132141 into llvm:main May 8, 2025
10 checks passed

felipepiovezan reviewed May 8, 2025

View reviewed changes

felipepiovezan added a commit that referenced this pull request May 8, 2025

[lldb] Disable test using GetControlFlowKind on arm

cb0b961

This is only implemented for x86. Originally introduced in: #137904

llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request May 8, 2025

Automerge: [lldb] Disable test using GetControlFlowKind on arm

506d7da

This is only implemented for x86. Originally introduced in: llvm/llvm-project#137904

[lldb] Expose QueueThreadPlanForStepSingleInstruction function to SBThreadPlan #137904

[lldb] Expose QueueThreadPlanForStepSingleInstruction function to SBThreadPlan #137904

Uh oh!

Conversation

eronnen commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Apr 30, 2025

Uh oh!

jimingham left a comment

Choose a reason for hiding this comment

Uh oh!

eronnen commented Apr 30, 2025

Uh oh!

github-actions bot commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jimingham Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jimingham Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eronnen Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

jimingham Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

eronnen Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

eronnen commented Apr 30, 2025

Uh oh!

jimingham left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jimingham left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eronnen commented May 1, 2025

Uh oh!

eronnen commented May 1, 2025

Uh oh!

jimingham commented May 1, 2025

Uh oh!

jimingham left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

felipepiovezan commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

felipepiovezan May 8, 2025

Choose a reason for hiding this comment

Uh oh!

felipepiovezan commented May 8, 2025

Uh oh!

slydiman commented May 8, 2025

Uh oh!

eronnen commented May 8, 2025

Uh oh!

eronnen commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

felipepiovezan commented May 8, 2025

Uh oh!

felipepiovezan commented May 8, 2025

Uh oh!

felipepiovezan commented May 8, 2025

Uh oh!

eronnen commented May 8, 2025

Uh oh!

DavidSpickett commented May 9, 2025

Uh oh!

Uh oh!

eronnen commented Apr 30, 2025 •

edited

Loading

github-actions bot commented Apr 30, 2025 •

edited

Loading

jimingham Apr 30, 2025 •

edited

Loading

jimingham Apr 30, 2025 •

edited

Loading

felipepiovezan commented May 8, 2025 •

edited

Loading

eronnen commented May 8, 2025 •

edited

Loading