Skip to content

[lldb][nfc] Remove unused parameters in ThreadPlanStepOut ctor #136161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lldb/include/lldb/API/SBThreadPlan.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ class LLDB_API SBThreadPlan {
lldb::addr_t range_size,
SBError &error);

// Note: first_insn is not used.
SBThreadPlan QueueThreadPlanForStepOut(uint32_t frame_idx_to_step_to,
bool first_insn = false);
SBThreadPlan QueueThreadPlanForStepOut(uint32_t frame_idx_to_step_to,
Expand Down
35 changes: 5 additions & 30 deletions lldb/include/lldb/Target/Thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -843,18 +843,6 @@ class Thread : public std::enable_shared_from_this<Thread>,
/// this one.
/// Otherwise this plan will go on the end of the plan stack.
///
/// \param[in] addr_context
/// When dealing with stepping through inlined functions the current PC is
/// not enough information to know
/// what "step" means. For instance a series of nested inline functions
/// might start at the same address.
// The \a addr_context provides the current symbol context the step
/// is supposed to be out of.
// FIXME: Currently unused.
///
/// \param[in] first_insn
/// \b true if this is the first instruction of a function.
///
/// \param[in] stop_other_threads
/// \b true if we will stop other threads while we single step this one.
///
Expand All @@ -876,9 +864,8 @@ class Thread : public std::enable_shared_from_this<Thread>,
/// A shared pointer to the newly queued thread plan, or nullptr if the
/// plan could not be queued.
virtual lldb::ThreadPlanSP QueueThreadPlanForStepOut(
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 abort_other_plans, bool stop_other_threads, Vote report_stop_vote,
Vote report_run_vote, uint32_t frame_idx, Status &status,
LazyBool step_out_avoids_code_without_debug_info = eLazyBoolCalculate);

/// Queue the plan used to step out of the function at the current PC of
Expand All @@ -892,18 +879,6 @@ class Thread : public std::enable_shared_from_this<Thread>,
/// this one.
/// Otherwise this plan will go on the end of the plan stack.
///
/// \param[in] addr_context
/// When dealing with stepping through inlined functions the current PC is
/// not enough information to know
/// what "step" means. For instance a series of nested inline functions
/// might start at the same address.
// The \a addr_context provides the current symbol context the step
/// is supposed to be out of.
// FIXME: Currently unused.
///
/// \param[in] first_insn
/// \b true if this is the first instruction of a function.
///
/// \param[in] stop_other_threads
/// \b true if we will stop other threads while we single step this one.
///
Expand Down Expand Up @@ -940,9 +915,9 @@ class Thread : public std::enable_shared_from_this<Thread>,
/// A shared pointer to the newly queued thread plan, or nullptr if the
/// plan could not be queued.
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);
bool abort_other_plans, bool stop_other_threads, Vote report_stop_vote,
Vote report_run_vote, uint32_t frame_idx, Status &status,
bool continue_to_next_branch = false);

/// 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.
Expand Down
12 changes: 7 additions & 5 deletions lldb/include/lldb/Target/ThreadPlanStepOut.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ 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,
ThreadPlanStepOut(Thread &thread, 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,
Expand Down Expand Up @@ -76,12 +75,15 @@ class ThreadPlanStepOut : public ThreadPlan, public ThreadPlanShouldStopHere {
StreamString m_constructor_errors;

friend lldb::ThreadPlanSP Thread::QueueThreadPlanForStepOut(
bool abort_other_plans, SymbolContext *addr_context, bool first_insn,
bool stop_others, Vote report_stop_vote, Vote report_run_vote,
uint32_t frame_idx, Status &status,
bool abort_other_plans, bool stop_others, Vote report_stop_vote,
Vote report_run_vote, uint32_t frame_idx, Status &status,
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.

Expand Down
8 changes: 4 additions & 4 deletions lldb/source/API/SBThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -659,8 +659,8 @@ void SBThread::StepOut(SBError &error) {
const LazyBool avoid_no_debug = eLazyBoolCalculate;
Status new_plan_status;
ThreadPlanSP new_plan_sp(thread->QueueThreadPlanForStepOut(
abort_other_plans, nullptr, false, stop_other_threads, eVoteYes,
eVoteNoOpinion, 0, new_plan_status, avoid_no_debug));
abort_other_plans, stop_other_threads, eVoteYes, eVoteNoOpinion, 0,
new_plan_status, avoid_no_debug));

if (new_plan_status.Success())
error = ResumeNewPlan(exe_ctx, new_plan_sp.get());
Expand Down Expand Up @@ -703,8 +703,8 @@ void SBThread::StepOutOfFrame(SBFrame &sb_frame, SBError &error) {

Status new_plan_status;
ThreadPlanSP new_plan_sp(thread->QueueThreadPlanForStepOut(
abort_other_plans, nullptr, false, stop_other_threads, eVoteYes,
eVoteNoOpinion, frame_sp->GetFrameIndex(), new_plan_status));
abort_other_plans, stop_other_threads, eVoteYes, eVoteNoOpinion,
frame_sp->GetFrameIndex(), new_plan_status));

if (new_plan_status.Success())
error = ResumeNewPlan(exe_ctx, new_plan_sp.get());
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/API/SBThreadPlan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,8 @@ SBThreadPlan::QueueThreadPlanForStepOut(uint32_t frame_idx_to_step_to,
Status plan_status;
SBThreadPlan plan =
SBThreadPlan(thread_plan_sp->GetThread().QueueThreadPlanForStepOut(
false, &sc, first_insn, false, eVoteYes, eVoteNoOpinion,
frame_idx_to_step_to, plan_status));
false, false, eVoteYes, eVoteNoOpinion, frame_idx_to_step_to,
plan_status));

if (plan_status.Fail())
error.SetErrorString(plan_status.AsCString());
Expand Down
3 changes: 1 addition & 2 deletions lldb/source/Commands/CommandObjectThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -558,8 +558,7 @@ class CommandObjectThreadStepWithTypeAndScope : public CommandObjectParsed {
true, abort_other_plans, bool_stop_other_threads, new_plan_status);
} else if (m_step_type == eStepTypeOut) {
new_plan_sp = thread->QueueThreadPlanForStepOut(
abort_other_plans, nullptr, false, bool_stop_other_threads, eVoteYes,
eVoteNoOpinion,
abort_other_plans, bool_stop_other_threads, eVoteYes, eVoteNoOpinion,
thread->GetSelectedFrameIndex(DoNoSelectMostRelevantFrame),
new_plan_status, m_options.m_step_out_avoid_no_debug);
} else if (m_step_type == eStepTypeScripted) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,10 @@ bool AppleThreadPlanStepThroughObjCTrampoline::ShouldStop(Event *event_ptr) {
eSymbolContextEverything);
Status status;
const bool abort_other_plans = false;
const bool first_insn = true;
const uint32_t frame_idx = 0;
m_run_to_sp = GetThread().QueueThreadPlanForStepOutNoShouldStop(
abort_other_plans, &sc, first_insn, false, eVoteNoOpinion,
eVoteNoOpinion, frame_idx, status);
abort_other_plans, false, eVoteNoOpinion, eVoteNoOpinion, frame_idx,
status);
if (m_run_to_sp && status.Success())
m_run_to_sp->SetPrivate(true);
return false;
Expand Down Expand Up @@ -242,8 +241,7 @@ AppleThreadPlanStepThroughDirectDispatch ::
AppleThreadPlanStepThroughDirectDispatch(
Thread &thread, AppleObjCTrampolineHandler &handler,
llvm::StringRef dispatch_func_name)
: ThreadPlanStepOut(thread, nullptr, true /* first instruction */, false,
eVoteNoOpinion, eVoteNoOpinion,
: ThreadPlanStepOut(thread, false, eVoteNoOpinion, eVoteNoOpinion,
0 /* Step out of zeroth frame */,
eLazyBoolNo /* Our parent plan will decide this
when we are done */
Expand Down
27 changes: 12 additions & 15 deletions lldb/source/Target/Thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1341,28 +1341,26 @@ ThreadPlanSP Thread::QueueThreadPlanForStepInRange(
}

ThreadPlanSP Thread::QueueThreadPlanForStepOut(
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 abort_other_plans, bool stop_other_threads, Vote report_stop_vote,
Vote report_run_vote, uint32_t frame_idx, Status &status,
LazyBool step_out_avoids_code_without_debug_info) {
ThreadPlanSP thread_plan_sp(new ThreadPlanStepOut(
*this, addr_context, first_insn, stop_other_threads, report_stop_vote,
report_run_vote, frame_idx, step_out_avoids_code_without_debug_info));
*this, stop_other_threads, report_stop_vote, report_run_vote, frame_idx,
step_out_avoids_code_without_debug_info));

status = QueueThreadPlan(thread_plan_sp, abort_other_plans);
return thread_plan_sp;
}

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) {
bool abort_other_plans, bool stop_other_threads, Vote report_stop_vote,
Vote report_run_vote, uint32_t frame_idx, Status &status,
bool continue_to_next_branch) {
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,
eLazyBoolNo, continue_to_next_branch, calculate_return_value));

ThreadPlanStepOut *new_plan =
static_cast<ThreadPlanStepOut *>(thread_plan_sp.get());
Expand Down Expand Up @@ -2035,13 +2033,12 @@ Status Thread::StepOut(uint32_t frame_idx) {
Status error;
Process *process = GetProcess().get();
if (StateIsStoppedState(process->GetState(), true)) {
const bool first_instruction = false;
const bool stop_other_threads = false;
const bool abort_other_plans = false;

ThreadPlanSP new_plan_sp(QueueThreadPlanForStepOut(
abort_other_plans, nullptr, first_instruction, stop_other_threads,
eVoteYes, eVoteNoOpinion, frame_idx, error));
ThreadPlanSP new_plan_sp =
QueueThreadPlanForStepOut(abort_other_plans, stop_other_threads,
eVoteYes, eVoteNoOpinion, frame_idx, error);

new_plan_sp->SetIsControllingPlan(true);
new_plan_sp->SetOkayToDiscard(false);
Expand Down
2 changes: 0 additions & 2 deletions lldb/source/Target/ThreadPlanCallOnFunctionExit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ void ThreadPlanCallOnFunctionExit::DidPush() {
Status status;
m_step_out_threadplan_sp = GetThread().QueueThreadPlanForStepOut(
false, // abort other plans
nullptr, // addr_context
true, // first instruction
true, // stop other threads
eVoteNo, // do not say "we're stopping"
eVoteNoOpinion, // don't care about run state broadcasting
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Target/ThreadPlanShouldStopHere.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,8 @@ ThreadPlanSP ThreadPlanShouldStopHere::DefaultStepFromHereCallback(
if (!return_plan_sp)
return_plan_sp =
current_plan->GetThread().QueueThreadPlanForStepOutNoShouldStop(
false, nullptr, true, stop_others, eVoteNo, eVoteNoOpinion,
frame_index, status, true);
false, stop_others, eVoteNo, eVoteNoOpinion, frame_index, status,
true);
return return_plan_sp;
}

Expand Down
3 changes: 1 addition & 2 deletions lldb/source/Target/ThreadPlanStepInstruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,7 @@ bool ThreadPlanStepInstruction::ShouldStop(Event *event_ptr) {
// for now it is safer to run others.
const bool stop_others = false;
thread.QueueThreadPlanForStepOutNoShouldStop(
false, nullptr, true, stop_others, eVoteNo, eVoteNoOpinion, 0,
m_status);
false, stop_others, eVoteNo, eVoteNoOpinion, 0, m_status);
return false;
} else {
if (log) {
Expand Down
61 changes: 39 additions & 22 deletions lldb/source/Target/ThreadPlanStepOut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,36 @@ 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,
Vote report_stop_vote, Vote report_run_vote, uint32_t frame_idx,
Thread &thread, 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)
: ThreadPlan(ThreadPlan::eKindStepOut, "Step out", thread, report_stop_vote,
Expand All @@ -44,34 +70,25 @@ 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();
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));

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.

// 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();

Expand All @@ -84,7 +101,7 @@ 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,
GetThread(), 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);
Expand All @@ -96,7 +113,6 @@ ThreadPlanStepOut::ThreadPlanStepOut(
}
} else {
// Find the return address and set a breakpoint there:
// FIXME - can we do this more securely if we know first_insn?

Address return_address(return_frame_sp->GetFrameCodeAddress());
if (continue_to_next_branch) {
Expand Down Expand Up @@ -125,6 +141,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),
Expand Down
3 changes: 1 addition & 2 deletions lldb/source/Target/ThreadPlanStepOverRange.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,7 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) {
if (m_next_branch_bp_sp)
return false;
new_plan_sp = thread.QueueThreadPlanForStepOutNoShouldStop(
false, nullptr, true, stop_others, eVoteNo, eVoteNoOpinion, 0,
m_status, true);
false, stop_others, eVoteNo, eVoteNoOpinion, 0, m_status, true);
break;
} else {
new_plan_sp = thread.QueueThreadPlanForStepThrough(
Expand Down
Loading