[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:00:30 PDT 2025
https://github.com/felipepiovezan created https://github.com/llvm/llvm-project/pull/135866
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`.
>From 166d0321ea59ac9797a9e97f21d42d1c91f145b4 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 | 15 +++--
lldb/source/Target/ThreadPlanStepOut.cpp | 57 ++++++++++++-------
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..c8eb334e4311c 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,9 @@ class ThreadPlanStepOut : public ThreadPlan, public ThreadPlanShouldStopHere {
void CalculateReturnValue();
+ lldb::StackFrameSP
+ ComputeReturnToFrame(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..ef060b376407d 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::ComputeReturnToFrame(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,18 @@ 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 =
+ ComputeReturnToFrame(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 +139,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