[Lldb-commits] [lldb] [lldb][nfc] Add customization flags for ThreadPlanStepOut (PR #135866)

Felipe de Azevedo Piovezan via lldb-commits lldb-commits at lists.llvm.org
Tue Apr 15 15:03:00 PDT 2025


https://github.com/felipepiovezan updated https://github.com/llvm/llvm-project/pull/135866

>From 3804a622a92ed541e4ee1a40298442e78b687507 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Tue, 15 Apr 2025 10:20:41 -0700
Subject: [PATCH] [lldb][nfc] Add customization flags for ThreadPlanStepOut

ThreadPlanStepOut always skips over Hidden/Artificial frames when
computing its destination frame, without providing any customization of
this behavior. This is problematic for some plans like StepThrough,
which may need to step out of a frame _without_ stepping out of a
Hidden/Artificial frame. Any first step towards fixing this requires the
ability to customize ThreadPlanStepOut, which is what this NFC patch
addresses.

Since the computation of which frames to skip is done by the
constructor, this patch adds a Flags parameter to
`ThreadPlanStepOut::ThreadPlanStepOut`.
---
 .../lldb/Target/ThreadPlanShouldStopHere.h    |  4 +-
 lldb/include/lldb/Target/ThreadPlanStepOut.h  | 16 ++++--
 lldb/source/Target/ThreadPlanStepOut.cpp      | 56 ++++++++++++-------
 3 files changed, 48 insertions(+), 28 deletions(-)

diff --git a/lldb/include/lldb/Target/ThreadPlanShouldStopHere.h b/lldb/include/lldb/Target/ThreadPlanShouldStopHere.h
index d0094c90b91a5..e712ee8d2d94f 100644
--- a/lldb/include/lldb/Target/ThreadPlanShouldStopHere.h
+++ b/lldb/include/lldb/Target/ThreadPlanShouldStopHere.h
@@ -60,7 +60,9 @@ class ThreadPlanShouldStopHere {
     eAvoidInlines = (1 << 0),
     eStepInAvoidNoDebug = (1 << 1),
     eStepOutAvoidNoDebug = (1 << 2),
-    eStepOutPastThunks = (1 << 3)
+    eStepOutPastThunks = (1 << 3),
+    eStepOutPastHiddenFunctions = (1 << 4),
+    eStepOutPastArtificialFunctions = (1 << 5),
   };
 
   // Constructors and Destructors
diff --git a/lldb/include/lldb/Target/ThreadPlanStepOut.h b/lldb/include/lldb/Target/ThreadPlanStepOut.h
index 013c675afc33d..030e532fe4d57 100644
--- a/lldb/include/lldb/Target/ThreadPlanStepOut.h
+++ b/lldb/include/lldb/Target/ThreadPlanStepOut.h
@@ -17,12 +17,12 @@ namespace lldb_private {
 
 class ThreadPlanStepOut : public ThreadPlan, public ThreadPlanShouldStopHere {
 public:
-  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);
+  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,
+      lldb_private::Flags flags = ThreadPlanStepOut::s_default_flag_values);
 
   ~ThreadPlanStepOut() override;
 
@@ -87,6 +87,10 @@ class ThreadPlanStepOut : public ThreadPlan, public ThreadPlanShouldStopHere {
 
   void CalculateReturnValue();
 
+  /// Computes the target frame this plan should step out to.
+  lldb::StackFrameSP ComputeTargetFrame(Thread &thread,
+                                        uint32_t start_frame_idx, Flags flags);
+
   ThreadPlanStepOut(const ThreadPlanStepOut &) = delete;
   const ThreadPlanStepOut &operator=(const ThreadPlanStepOut &) = delete;
 };
diff --git a/lldb/source/Target/ThreadPlanStepOut.cpp b/lldb/source/Target/ThreadPlanStepOut.cpp
index a05c46db6b8ca..340abd6731d0c 100644
--- a/lldb/source/Target/ThreadPlanStepOut.cpp
+++ b/lldb/source/Target/ThreadPlanStepOut.cpp
@@ -29,14 +29,44 @@
 using namespace lldb;
 using namespace lldb_private;
 
-uint32_t ThreadPlanStepOut::s_default_flag_values = 0;
+uint32_t ThreadPlanStepOut::s_default_flag_values =
+    eStepOutPastArtificialFunctions | eStepOutPastHiddenFunctions;
+
+StackFrameSP ThreadPlanStepOut::ComputeTargetFrame(Thread &thread,
+                                                   uint32_t start_frame_idx,
+                                                   Flags flags) {
+  uint32_t frame_idx = start_frame_idx + 1;
+  StackFrameSP return_frame_sp = thread.GetStackFrameAtIndex(frame_idx);
+  if (!return_frame_sp)
+    return nullptr;
+
+  // If asked to, step out past artificial/hidden frames.
+  while ((flags.Test(eStepOutPastArtificialFunctions) &&
+          return_frame_sp->IsArtificial()) ||
+         (flags.Test(eStepOutPastHiddenFunctions) &&
+          return_frame_sp->IsHidden())) {
+    m_stepped_past_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,
     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, bool gather_return_value)
+    bool continue_to_next_branch, bool gather_return_value, Flags flags)
     : ThreadPlan(ThreadPlan::eKindStepOut, "Step out", thread, report_stop_vote,
                  report_run_vote),
       ThreadPlanShouldStopHere(this), m_step_from_insn(LLDB_INVALID_ADDRESS),
@@ -44,34 +74,17 @@ ThreadPlanStepOut::ThreadPlanStepOut(
       m_return_addr(LLDB_INVALID_ADDRESS), m_stop_others(stop_others),
       m_immediate_step_from_function(nullptr),
       m_calculate_return_value(gather_return_value) {
-  Log *log = GetLog(LLDBLog::Step);
-  SetFlagsToDefault();
+  m_flags = flags;
   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_flags);
   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.
 
-  // 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);
-
-    ++return_frame_index;
-    return_frame_sp = thread.GetStackFrameAtIndex(return_frame_index);
-
-    // 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;
-    }
-  }
-
   m_step_out_to_id = return_frame_sp->GetStackID();
   m_immediate_step_from_id = immediate_return_from_sp->GetStackID();
 
@@ -125,6 +138,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),



More information about the lldb-commits mailing list