Merge pull request #10505 from felipepiovezan/felipe/fix_conformance_stepping

[cherry-pick][lldb] Fix stepping through conformance methods

Author: adrian-prantl

lldb/include/lldb/Target/ThreadPlanStepOut.h

@@ -17,13 +17,22 @@ namespace lldb_private {
 
 class ThreadPlanStepOut : public ThreadPlan, public ThreadPlanShouldStopHere {
 public:
+  /// Creates a thread plan to step out from frame_idx, skipping parent frames
+  /// if they are artificial or hidden frames. Also skips frames without debug
+  /// info based on step_out_avoids_code_without_debug_info.
   ThreadPlanStepOut(Thread &thread, SymbolContext *addr_context,
                     bool first_insn, bool stop_others, Vote report_stop_vote,
                     Vote report_run_vote, uint32_t frame_idx,
                     LazyBool step_out_avoids_code_without_debug_info,
                     bool continue_to_next_branch = false,
                     bool gather_return_value = true);
 
+  /// Creates a thread plan to step out from frame_idx to frame_idx + 1.
+  ThreadPlanStepOut(Thread &thread, bool stop_others, Vote report_stop_vote,
+                    Vote report_run_vote, uint32_t frame_idx,
+                    bool continue_to_next_branch = false,
+                    bool gather_return_value = true);
+
   ~ThreadPlanStepOut() override;
 
   void GetDescription(Stream *s, lldb::DescriptionLevel level) override;
@@ -88,6 +97,10 @@ class ThreadPlanStepOut : public ThreadPlan, public ThreadPlanShouldStopHere {
       LazyBool step_out_avoids_code_without_debug_info);
 
   void SetupAvoidNoDebug(LazyBool step_out_avoids_code_without_debug_info);
+
+  void SetupReturnAddress(lldb::StackFrameSP return_frame_sp,
+                          lldb::StackFrameSP immediate_return_from_sp,
+                          uint32_t frame_idx, bool continue_to_next_branch);
   // Need an appropriate marker for the current stack so we can tell step out
   // from step in.
 

lldb/source/Target/Thread.cpp

@@ -1392,9 +1392,8 @@ ThreadPlanSP Thread::QueueThreadPlanForStepOutNoShouldStop(
   const bool calculate_return_value =
       false; // No need to calculate the return value here.
   ThreadPlanSP thread_plan_sp(new ThreadPlanStepOut(
-      *this, addr_context, first_insn, stop_other_threads, report_stop_vote,
-      report_run_vote, frame_idx, eLazyBoolNo, continue_to_next_branch,
-      calculate_return_value));
+      *this, stop_other_threads, report_stop_vote, report_run_vote, frame_idx,
+      continue_to_next_branch, calculate_return_value));
 
   ThreadPlanStepOut *new_plan =
       static_cast<ThreadPlanStepOut *>(thread_plan_sp.get());

lldb/source/Target/ThreadPlanStepInRange.cpp

@@ -460,7 +460,7 @@ bool ThreadPlanStepInRange::DefaultShouldStopHereCallback(
   if (!should_stop_here)
     return false;
 
-  if (should_stop_here && current_plan->GetKind() == eKindStepInRange &&
+  if (current_plan->GetKind() == eKindStepInRange &&
       operation == eFrameCompareYounger) {
     ThreadPlanStepInRange *step_in_range_plan =
         static_cast<ThreadPlanStepInRange *>(current_plan);

lldb/source/Target/ThreadPlanStepOut.cpp

@@ -38,6 +38,32 @@ using namespace lldb_private;
 
 uint32_t ThreadPlanStepOut::s_default_flag_values = 0;
 
+/// Computes the target frame this plan should step out to.
+static StackFrameSP
+ComputeTargetFrame(Thread &thread, uint32_t start_frame_idx,
+                   std::vector<StackFrameSP> &skipped_frames) {
+  uint32_t frame_idx = start_frame_idx + 1;
+  StackFrameSP return_frame_sp = thread.GetStackFrameAtIndex(frame_idx);
+  if (!return_frame_sp)
+    return nullptr;
+
+  while (return_frame_sp->IsArtificial() || return_frame_sp->IsHidden()) {
+    skipped_frames.push_back(return_frame_sp);
+
+    frame_idx++;
+    return_frame_sp = thread.GetStackFrameAtIndex(frame_idx);
+
+    // We never expect to see an artificial frame without a regular ancestor.
+    // Defensively refuse to step out.
+    if (!return_frame_sp) {
+      LLDB_LOG(GetLog(LLDBLog::Step),
+               "Can't step out of frame with artificial ancestors");
+      return nullptr;
+    }
+  }
+  return return_frame_sp;
+}
+
 // ThreadPlanStepOut: Step out of the current frame
 ThreadPlanStepOut::ThreadPlanStepOut(
     Thread &thread, SymbolContext *context, bool first_insn, bool stop_others,
@@ -54,33 +80,46 @@ ThreadPlanStepOut::ThreadPlanStepOut(
       m_stop_others(stop_others), m_immediate_step_from_function(nullptr),
       m_is_swift_error_value(false),
       m_calculate_return_value(gather_return_value) {
-  Log *log = GetLog(LLDBLog::Step);
   SetFlagsToDefault();
   SetupAvoidNoDebug(step_out_avoids_code_without_debug_info);
 
   m_step_from_insn = thread.GetRegisterContext()->GetPC(0);
 
-  uint32_t return_frame_index = frame_idx + 1;
-  StackFrameSP return_frame_sp(thread.GetStackFrameAtIndex(return_frame_index));
+  StackFrameSP return_frame_sp =
+      ComputeTargetFrame(thread, frame_idx, m_stepped_past_frames);
   StackFrameSP immediate_return_from_sp(thread.GetStackFrameAtIndex(frame_idx));
 
-  if (!return_frame_sp || !immediate_return_from_sp)
-    return; // we can't do anything here.  ValidatePlan() will return false.
+  SetupReturnAddress(return_frame_sp, immediate_return_from_sp, frame_idx,
+                     continue_to_next_branch);
+}
 
-  // While stepping out, behave as-if artificial frames are not present.
-  while (return_frame_sp->IsArtificial() || return_frame_sp->IsHidden()) {
-    m_stepped_past_frames.push_back(return_frame_sp);
+ThreadPlanStepOut::ThreadPlanStepOut(Thread &thread, bool stop_others,
+                                     Vote report_stop_vote,
+                                     Vote report_run_vote, uint32_t frame_idx,
+                                     bool continue_to_next_branch,
+                                     bool gather_return_value)
+    : ThreadPlan(ThreadPlan::eKindStepOut, "Step out", thread, report_stop_vote,
+                 report_run_vote),
+      ThreadPlanShouldStopHere(this), m_return_bp_id(LLDB_INVALID_BREAK_ID),
+      m_return_addr(LLDB_INVALID_ADDRESS), m_stop_others(stop_others),
+      m_immediate_step_from_function(nullptr),
+      m_calculate_return_value(gather_return_value) {
+  SetFlagsToDefault();
+  m_step_from_insn = thread.GetRegisterContext()->GetPC(0);
 
-    ++return_frame_index;
-    return_frame_sp = thread.GetStackFrameAtIndex(return_frame_index);
+  StackFrameSP return_frame_sp = thread.GetStackFrameAtIndex(frame_idx + 1);
+  StackFrameSP immediate_return_from_sp =
+      thread.GetStackFrameAtIndex(frame_idx);
 
-    // We never expect to see an artificial frame without a regular ancestor.
-    // If this happens, log the issue and defensively refuse to step out.
-    if (!return_frame_sp) {
-      LLDB_LOG(log, "Can't step out of frame with artificial ancestors");
-      return;
-    }
-  }
+  SetupReturnAddress(return_frame_sp, immediate_return_from_sp, frame_idx,
+                     continue_to_next_branch);
+}
+
+void ThreadPlanStepOut::SetupReturnAddress(
+    StackFrameSP return_frame_sp, StackFrameSP immediate_return_from_sp,
+    uint32_t frame_idx, bool continue_to_next_branch) {
+  if (!return_frame_sp || !immediate_return_from_sp)
+    return; // we can't do anything here.  ValidatePlan() will return false.
 
   m_step_out_to_id = return_frame_sp->GetStackID();
   m_immediate_step_from_id = immediate_return_from_sp->GetStackID();
@@ -94,8 +133,8 @@ ThreadPlanStepOut::ThreadPlanStepOut(
       // First queue a plan that gets us to this inlined frame, and when we get
       // there we'll queue a second plan that walks us out of this frame.
       m_step_out_to_inline_plan_sp = std::make_shared<ThreadPlanStepOut>(
-          thread, nullptr, false, stop_others, eVoteNoOpinion, eVoteNoOpinion,
-          frame_idx - 1, eLazyBoolNo, continue_to_next_branch);
+          GetThread(), nullptr, false, m_stop_others, eVoteNoOpinion,
+          eVoteNoOpinion, frame_idx - 1, eLazyBoolNo, continue_to_next_branch);
       static_cast<ThreadPlanStepOut *>(m_step_out_to_inline_plan_sp.get())
           ->SetShouldStopHereCallbacks(nullptr, nullptr);
       m_step_out_to_inline_plan_sp->SetPrivate(true);
@@ -135,6 +174,7 @@ ThreadPlanStepOut::ThreadPlanStepOut(
 
     // Perform some additional validation on the return address.
     uint32_t permissions = 0;
+    Log *log = GetLog(LLDBLog::Step);
     if (!m_process.GetLoadAddressPermissions(m_return_addr, permissions)) {
       LLDB_LOGF(log, "ThreadPlanStepOut(%p): Return address (0x%" PRIx64
                 ") permissions not found.", static_cast<void *>(this),

lldb/test/API/lang/swift/protocols/stepping_through_witness/Makefile

@@ -0,0 +1,2 @@
+SWIFT_SOURCES := main.swift
+include Makefile.rules

lldb/test/API/lang/swift/protocols/stepping_through_witness/TestSwiftSteppingThroughWitness.py

@@ -0,0 +1,57 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+@skipIf(oslist=["windows", "linux"])
+class TestSwiftSteppingThroughWitness(TestBase):
+
+    def setUp(self):
+        TestBase.setUp(self)
+        self.runCmd(
+            "settings set target.process.thread.step-avoid-libraries libswift_Concurrency.dylib"
+        )
+
+    @swiftTest
+    def test_step_in_and_out(self):
+        """Test that stepping in and out of protocol methods work"""
+        self.build()
+        _, _, thread, _ = lldbutil.run_to_source_breakpoint(
+            self, "break here", lldb.SBFileSpec("main.swift")
+        )
+
+        thread.StepInto()
+        stop_reason = thread.GetStopReason()
+        self.assertStopReason(stop_reason, lldb.eStopReasonPlanComplete)
+
+        frame0 = thread.frames[0]
+        frame1 = thread.frames[1]
+        self.assertIn("SlowRandomNumberGenerator.random", frame0.GetFunctionName())
+        self.assertIn(
+            "protocol witness for a.RandomNumberGenerator.random",
+            frame1.GetFunctionName(),
+        )
+
+        thread.StepOut()
+        stop_reason = thread.GetStopReason()
+        self.assertStopReason(stop_reason, lldb.eStopReasonPlanComplete)
+        frame0 = thread.frames[0]
+        self.assertIn("doMath", frame0.GetFunctionName())
+
+    @swiftTest
+    def test_step_over(self):
+        """Test that stepping over protocol methods work"""
+        self.build()
+        _, _, thread, _ = lldbutil.run_to_source_breakpoint(
+            self, "break here", lldb.SBFileSpec("main.swift")
+        )
+
+        thread.StepOver()
+        stop_reason = thread.GetStopReason()
+        self.assertStopReason(stop_reason, lldb.eStopReasonPlanComplete)
+        frame0 = thread.frames[0]
+        self.assertIn("doMath", frame0.GetFunctionName())
+
+        line_entry = frame0.GetLineEntry()
+        self.assertEqual(14, line_entry.GetLine())

lldb/test/API/lang/swift/protocols/stepping_through_witness/main.swift

@@ -0,0 +1,18 @@
+protocol RandomNumberGenerator {
+    func random(in range: ClosedRange<Int>) async -> Int
+}
+
+class SlowRandomNumberGenerator: RandomNumberGenerator {
+    func random(in range: ClosedRange<Int>) async -> Int {
+        try? await Task.sleep(for: .milliseconds(500))
+        return Int.random(in: range)
+    }
+}
+
+func doMath<RNG: RandomNumberGenerator>(with rng: RNG) async {
+    let y = await rng.random(in: 101...200) // break here
+    print("Y is \(y)")
+}
+
+let rng = SlowRandomNumberGenerator()
+await doMath(with: rng)