Skip to content

Commit fa0b6a6

Browse files
[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]: llvm#104523
1 parent 83658dd commit fa0b6a6

File tree

6 files changed

+20
-10
lines changed

6 files changed

+20
-10
lines changed

lldb/include/lldb/Target/Thread.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -942,7 +942,8 @@ class Thread : public std::enable_shared_from_this<Thread>,
942942
virtual lldb::ThreadPlanSP QueueThreadPlanForStepOutNoShouldStop(
943943
bool abort_other_plans, SymbolContext *addr_context, bool first_insn,
944944
bool stop_other_threads, Vote report_stop_vote, Vote report_run_vote,
945-
uint32_t frame_idx, Status &status, bool continue_to_next_branch = false);
945+
uint32_t frame_idx, Status &status, bool continue_to_next_branch = false,
946+
const Flags *flags = nullptr);
946947

947948
/// Gets the plan used to step through the code that steps from a function
948949
/// call site at the current PC into the actual function call.

lldb/include/lldb/Target/ThreadPlanShouldStopHere.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ class ThreadPlanShouldStopHere {
6060
eAvoidInlines = (1 << 0),
6161
eStepInAvoidNoDebug = (1 << 1),
6262
eStepOutAvoidNoDebug = (1 << 2),
63-
eStepOutPastThunks = (1 << 3)
63+
eStepOutPastThunks = (1 << 3),
64+
eStepOutPastHiddenFunctions = (1 << 4),
6465
};
6566

6667
// Constructors and Destructors

lldb/include/lldb/Target/ThreadPlanStepOut.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ class ThreadPlanStepOut : public ThreadPlan, public ThreadPlanShouldStopHere {
2222
Vote report_run_vote, uint32_t frame_idx,
2323
LazyBool step_out_avoids_code_without_debug_info,
2424
bool continue_to_next_branch = false,
25-
bool gather_return_value = true);
25+
bool gather_return_value = true,
26+
const Flags *flags = nullptr);
2627

2728
~ThreadPlanStepOut() override;
2829

lldb/source/Target/Thread.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1356,13 +1356,14 @@ ThreadPlanSP Thread::QueueThreadPlanForStepOut(
13561356
ThreadPlanSP Thread::QueueThreadPlanForStepOutNoShouldStop(
13571357
bool abort_other_plans, SymbolContext *addr_context, bool first_insn,
13581358
bool stop_other_threads, Vote report_stop_vote, Vote report_run_vote,
1359-
uint32_t frame_idx, Status &status, bool continue_to_next_branch) {
1359+
uint32_t frame_idx, Status &status, bool continue_to_next_branch,
1360+
const Flags *flags) {
13601361
const bool calculate_return_value =
13611362
false; // No need to calculate the return value here.
13621363
ThreadPlanSP thread_plan_sp(new ThreadPlanStepOut(
13631364
*this, addr_context, first_insn, stop_other_threads, report_stop_vote,
13641365
report_run_vote, frame_idx, eLazyBoolNo, continue_to_next_branch,
1365-
calculate_return_value));
1366+
calculate_return_value, flags));
13661367

13671368
ThreadPlanStepOut *new_plan =
13681369
static_cast<ThreadPlanStepOut *>(thread_plan_sp.get());

lldb/source/Target/ThreadPlanShouldStopHere.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ ThreadPlanSP ThreadPlanShouldStopHere::DefaultStepFromHereCallback(
177177
return_plan_sp =
178178
current_plan->GetThread().QueueThreadPlanForStepOutNoShouldStop(
179179
false, nullptr, true, stop_others, eVoteNo, eVoteNoOpinion,
180-
frame_index, status, true);
180+
frame_index, status, true, &flags);
181181
return return_plan_sp;
182182
}
183183

lldb/source/Target/ThreadPlanStepOut.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,15 @@
2929
using namespace lldb;
3030
using namespace lldb_private;
3131

32-
uint32_t ThreadPlanStepOut::s_default_flag_values = 0;
32+
uint32_t ThreadPlanStepOut::s_default_flag_values =
33+
ThreadPlanShouldStopHere::eStepOutPastHiddenFunctions;
3334

3435
// ThreadPlanStepOut: Step out of the current frame
3536
ThreadPlanStepOut::ThreadPlanStepOut(
3637
Thread &thread, SymbolContext *context, bool first_insn, bool stop_others,
3738
Vote report_stop_vote, Vote report_run_vote, uint32_t frame_idx,
3839
LazyBool step_out_avoids_code_without_debug_info,
39-
bool continue_to_next_branch, bool gather_return_value)
40+
bool continue_to_next_branch, bool gather_return_value, const Flags *flags)
4041
: ThreadPlan(ThreadPlan::eKindStepOut, "Step out", thread, report_stop_vote,
4142
report_run_vote),
4243
ThreadPlanShouldStopHere(this), m_step_from_insn(LLDB_INVALID_ADDRESS),
@@ -45,7 +46,10 @@ ThreadPlanStepOut::ThreadPlanStepOut(
4546
m_immediate_step_from_function(nullptr),
4647
m_calculate_return_value(gather_return_value) {
4748
Log *log = GetLog(LLDBLog::Step);
48-
SetFlagsToDefault();
49+
if (flags)
50+
m_flags = *flags;
51+
else
52+
SetFlagsToDefault();
4953
SetupAvoidNoDebug(step_out_avoids_code_without_debug_info);
5054

5155
m_step_from_insn = thread.GetRegisterContext()->GetPC(0);
@@ -58,7 +62,9 @@ ThreadPlanStepOut::ThreadPlanStepOut(
5862
return; // we can't do anything here. ValidatePlan() will return false.
5963

6064
// While stepping out, behave as-if artificial frames are not present.
61-
while (return_frame_sp->IsArtificial() || return_frame_sp->IsHidden()) {
65+
while (return_frame_sp->IsArtificial() ||
66+
(m_flags.Test(eStepOutPastHiddenFunctions) &&
67+
return_frame_sp->IsHidden())) {
6268
m_stepped_past_frames.push_back(return_frame_sp);
6369

6470
++return_frame_index;

0 commit comments

Comments
 (0)