From fa0b6a6086009926e04dbd77bc4a3a2af8440751 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Tue, 18 Mar 2025 07:13:39 -0300 Subject: [PATCH] [lldb] Consider "hidden" frames in ThreadPlanShouldStopHere Patch [1] introduced the notion of "hidden" frames, which are recognized by language plugins and can be hidden in backtraces. They also affect stepping out: when stepping out of a frame, if its parent frame is "hidden", then the debugger steps out to the parent's parent. However, this is problematic when stepping out is done as part of a larger plan. For example, when stepping through, the debugger is often in some frame A, then pushes some frame B, and decides to step out of B and back to A. If A is a hidden frame, LLDB currenly steps out past it, which is not what the step through plan intended. To address this issue, we create a new flag for the ShouldStopHere base class to allow for stepping past hidden frames. This flag is now the default for stepping out. Furthermore, `ThreadPlanShouldStopHere::DefaultStepFromHereCallback` now passes its own set of flags to the constructor of ThreadPlanStepOut. One potentially controversial choice: all StepOut plans created through `ThreadPlanShouldStopHere::DefaultStepFromHereCallback` will pass on their set of flags to ThreadPlanStepOut, which will disable the behavior of stepping past hidden frames. [1]: https://github.com/llvm/llvm-project/pull/104523 --- lldb/include/lldb/Target/Thread.h | 3 ++- .../include/lldb/Target/ThreadPlanShouldStopHere.h | 3 ++- lldb/include/lldb/Target/ThreadPlanStepOut.h | 3 ++- lldb/source/Target/Thread.cpp | 5 +++-- lldb/source/Target/ThreadPlanShouldStopHere.cpp | 2 +- lldb/source/Target/ThreadPlanStepOut.cpp | 14 ++++++++++---- 6 files changed, 20 insertions(+), 10 deletions(-) diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index 6ede7fa301a82..3c6b9791faeee 100644 --- a/lldb/include/lldb/Target/Thread.h +++ b/lldb/include/lldb/Target/Thread.h @@ -942,7 +942,8 @@ class Thread : public std::enable_shared_from_this, virtual lldb::ThreadPlanSP QueueThreadPlanForStepOutNoShouldStop( bool abort_other_plans, SymbolContext *addr_context, bool first_insn, bool stop_other_threads, Vote report_stop_vote, Vote report_run_vote, - uint32_t frame_idx, Status &status, bool continue_to_next_branch = false); + uint32_t frame_idx, Status &status, bool continue_to_next_branch = false, + const Flags *flags = nullptr); /// Gets the plan used to step through the code that steps from a function /// call site at the current PC into the actual function call. diff --git a/lldb/include/lldb/Target/ThreadPlanShouldStopHere.h b/lldb/include/lldb/Target/ThreadPlanShouldStopHere.h index d0094c90b91a5..8de53465010a9 100644 --- a/lldb/include/lldb/Target/ThreadPlanShouldStopHere.h +++ b/lldb/include/lldb/Target/ThreadPlanShouldStopHere.h @@ -60,7 +60,8 @@ class ThreadPlanShouldStopHere { eAvoidInlines = (1 << 0), eStepInAvoidNoDebug = (1 << 1), eStepOutAvoidNoDebug = (1 << 2), - eStepOutPastThunks = (1 << 3) + eStepOutPastThunks = (1 << 3), + eStepOutPastHiddenFunctions = (1 << 4), }; // Constructors and Destructors diff --git a/lldb/include/lldb/Target/ThreadPlanStepOut.h b/lldb/include/lldb/Target/ThreadPlanStepOut.h index 013c675afc33d..9f3138686a53e 100644 --- a/lldb/include/lldb/Target/ThreadPlanStepOut.h +++ b/lldb/include/lldb/Target/ThreadPlanStepOut.h @@ -22,7 +22,8 @@ class ThreadPlanStepOut : public ThreadPlan, public ThreadPlanShouldStopHere { 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); + bool gather_return_value = true, + const Flags *flags = nullptr); ~ThreadPlanStepOut() override; diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp index accc4708c24e1..191fc05fa7e5a 100644 --- a/lldb/source/Target/Thread.cpp +++ b/lldb/source/Target/Thread.cpp @@ -1356,13 +1356,14 @@ ThreadPlanSP Thread::QueueThreadPlanForStepOut( ThreadPlanSP Thread::QueueThreadPlanForStepOutNoShouldStop( bool abort_other_plans, SymbolContext *addr_context, bool first_insn, bool stop_other_threads, Vote report_stop_vote, Vote report_run_vote, - uint32_t frame_idx, Status &status, bool continue_to_next_branch) { + uint32_t frame_idx, Status &status, bool continue_to_next_branch, + const Flags *flags) { 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)); + calculate_return_value, flags)); ThreadPlanStepOut *new_plan = static_cast(thread_plan_sp.get()); diff --git a/lldb/source/Target/ThreadPlanShouldStopHere.cpp b/lldb/source/Target/ThreadPlanShouldStopHere.cpp index d2cca49987f0f..8c58bb92ac41b 100644 --- a/lldb/source/Target/ThreadPlanShouldStopHere.cpp +++ b/lldb/source/Target/ThreadPlanShouldStopHere.cpp @@ -177,7 +177,7 @@ ThreadPlanSP ThreadPlanShouldStopHere::DefaultStepFromHereCallback( return_plan_sp = current_plan->GetThread().QueueThreadPlanForStepOutNoShouldStop( false, nullptr, true, stop_others, eVoteNo, eVoteNoOpinion, - frame_index, status, true); + frame_index, status, true, &flags); return return_plan_sp; } diff --git a/lldb/source/Target/ThreadPlanStepOut.cpp b/lldb/source/Target/ThreadPlanStepOut.cpp index a05c46db6b8ca..738c019466acf 100644 --- a/lldb/source/Target/ThreadPlanStepOut.cpp +++ b/lldb/source/Target/ThreadPlanStepOut.cpp @@ -29,14 +29,15 @@ using namespace lldb; using namespace lldb_private; -uint32_t ThreadPlanStepOut::s_default_flag_values = 0; +uint32_t ThreadPlanStepOut::s_default_flag_values = + ThreadPlanShouldStopHere::eStepOutPastHiddenFunctions; // 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, const Flags *flags) : ThreadPlan(ThreadPlan::eKindStepOut, "Step out", thread, report_stop_vote, report_run_vote), ThreadPlanShouldStopHere(this), m_step_from_insn(LLDB_INVALID_ADDRESS), @@ -45,7 +46,10 @@ ThreadPlanStepOut::ThreadPlanStepOut( m_immediate_step_from_function(nullptr), m_calculate_return_value(gather_return_value) { Log *log = GetLog(LLDBLog::Step); - SetFlagsToDefault(); + if (flags) + m_flags = *flags; + else + SetFlagsToDefault(); SetupAvoidNoDebug(step_out_avoids_code_without_debug_info); m_step_from_insn = thread.GetRegisterContext()->GetPC(0); @@ -58,7 +62,9 @@ ThreadPlanStepOut::ThreadPlanStepOut( 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()) { + while (return_frame_sp->IsArtificial() || + (m_flags.Test(eStepOutPastHiddenFunctions) && + return_frame_sp->IsHidden())) { m_stepped_past_frames.push_back(return_frame_sp); ++return_frame_index;