Skip to content

Commit 936295b

Browse files
authored
Merge pull request #6703 from jimingham/cherry-pick-select-most-relevant
2 parents 80c77db + d0900fa commit 936295b

28 files changed

+146
-72
lines changed

lldb/include/lldb/Target/Process.h

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -835,6 +835,7 @@ class Process : public std::enable_shared_from_this<Process>,
835835
/// \see Thread:Suspend()
836836
Status Resume();
837837

838+
/// Resume a process, and wait for it to stop.
838839
Status ResumeSynchronous(Stream *stream);
839840

840841
/// Halts a running process.
@@ -2184,12 +2185,17 @@ class Process : public std::enable_shared_from_this<Process>,
21842185
// process is hijacked and use_run_lock is true (the default), then this
21852186
// function releases the run lock after the stop. Setting use_run_lock to
21862187
// false will avoid this behavior.
2188+
// If we are waiting to stop that will return control to the user,
2189+
// then we also want to run SelectMostRelevantFrame, which is controlled
2190+
// by "select_most_relevant".
21872191
lldb::StateType
21882192
WaitForProcessToStop(const Timeout<std::micro> &timeout,
21892193
lldb::EventSP *event_sp_ptr = nullptr,
21902194
bool wait_always = true,
21912195
lldb::ListenerSP hijack_listener = lldb::ListenerSP(),
2192-
Stream *stream = nullptr, bool use_run_lock = true);
2196+
Stream *stream = nullptr, bool use_run_lock = true,
2197+
SelectMostRelevant select_most_relevant =
2198+
DoNoSelectMostRelevantFrame);
21932199

21942200
uint32_t GetIOHandlerID() const { return m_iohandler_sync.GetValue(); }
21952201

@@ -2232,10 +2238,11 @@ class Process : public std::enable_shared_from_this<Process>,
22322238
/// \return
22332239
/// \b true if the event describes a process state changed event, \b false
22342240
/// otherwise.
2235-
static bool HandleProcessStateChangedEvent(const lldb::EventSP &event_sp,
2236-
Stream *stream,
2237-
bool &pop_process_io_handler,
2238-
bool &pop_command_interpreter);
2241+
static bool
2242+
HandleProcessStateChangedEvent(const lldb::EventSP &event_sp, Stream *stream,
2243+
SelectMostRelevant select_most_relevant,
2244+
bool &pop_process_io_handler,
2245+
bool &pop_command_interpreter);
22392246

22402247
Event *PeekAtStateChangedEvents();
22412248

lldb/include/lldb/Target/StackFrameList.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,15 @@ class StackFrameList {
4646
uint32_t SetSelectedFrame(lldb_private::StackFrame *frame);
4747

4848
/// Get the currently selected frame index.
49-
uint32_t GetSelectedFrameIndex();
49+
/// We should only call SelectMostRelevantFrame if (a) the user hasn't already
50+
/// selected a frame, and (b) if this really is a user facing
51+
/// "GetSelectedFrame". SMRF runs the frame recognizers which can do
52+
/// arbitrary work that ends up being dangerous to do internally. Also,
53+
/// for most internal uses we don't actually want the frame changed by the
54+
/// SMRF logic. So unless this is in a command or SB API, you should
55+
/// pass false here.
56+
uint32_t
57+
GetSelectedFrameIndex(SelectMostRelevant select_most_relevant_frame);
5058

5159
/// Mark a stack frame as the currently selected frame using the frame index
5260
/// \p idx. Like \ref GetFrameAtIndex, invisible frames cannot be selected.

lldb/include/lldb/Target/Thread.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -426,11 +426,16 @@ class Thread : public std::enable_shared_from_this<Thread>,
426426
return lldb::StackFrameSP();
427427
}
428428

429-
uint32_t GetSelectedFrameIndex() {
430-
return GetStackFrameList()->GetSelectedFrameIndex();
429+
// Only pass true to select_most_relevant if you are fulfilling an explicit
430+
// user request for GetSelectedFrameIndex. The most relevant frame is only
431+
// for showing to the user, and can do arbitrary work, so we don't want to
432+
// call it internally.
433+
uint32_t GetSelectedFrameIndex(SelectMostRelevant select_most_relevant) {
434+
return GetStackFrameList()->GetSelectedFrameIndex(select_most_relevant);
431435
}
432436

433-
lldb::StackFrameSP GetSelectedFrame();
437+
lldb::StackFrameSP
438+
GetSelectedFrame(SelectMostRelevant select_most_relevant);
434439

435440
uint32_t SetSelectedFrame(lldb_private::StackFrame *frame,
436441
bool broadcast = false);

lldb/include/lldb/lldb-private-enumerations.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,4 +288,9 @@ template <> struct format_provider<lldb_private::Vote> {
288288
};
289289
}
290290

291+
enum SelectMostRelevant : bool {
292+
SelectMostRelevantFrame = true,
293+
DoNoSelectMostRelevantFrame = false,
294+
};
295+
291296
#endif // LLDB_LLDB_PRIVATE_ENUMERATIONS_H

lldb/source/API/SBThread.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -816,7 +816,11 @@ SBError SBThread::StepOverUntil(lldb::SBFrame &sb_frame,
816816
}
817817

818818
if (!frame_sp) {
819-
frame_sp = thread->GetSelectedFrame();
819+
// We don't want to run SelectMostRelevantFrame here, for instance if
820+
// you called a sequence of StepOverUntil's you wouldn't want the
821+
// frame changed out from under you because you stepped into a
822+
// recognized frame.
823+
frame_sp = thread->GetSelectedFrame(DoNoSelectMostRelevantFrame);
820824
if (!frame_sp)
821825
frame_sp = thread->GetStackFrameAtIndex(0);
822826
}
@@ -1160,7 +1164,8 @@ lldb::SBFrame SBThread::GetSelectedFrame() {
11601164
if (exe_ctx.HasThreadScope()) {
11611165
Process::StopLocker stop_locker;
11621166
if (stop_locker.TryLock(&exe_ctx.GetProcessPtr()->GetRunLock())) {
1163-
frame_sp = exe_ctx.GetThreadPtr()->GetSelectedFrame();
1167+
frame_sp =
1168+
exe_ctx.GetThreadPtr()->GetSelectedFrame(SelectMostRelevantFrame);
11641169
sb_frame.SetFrameSP(frame_sp);
11651170
}
11661171
}

lldb/source/Commands/CommandObjectFrame.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ class CommandObjectFrameDiagnose : public CommandObjectParsed {
134134
protected:
135135
bool DoExecute(Args &command, CommandReturnObject &result) override {
136136
Thread *thread = m_exe_ctx.GetThreadPtr();
137-
StackFrameSP frame_sp = thread->GetSelectedFrame();
137+
StackFrameSP frame_sp = thread->GetSelectedFrame(SelectMostRelevantFrame);
138138

139139
ValueObjectSP valobj_sp;
140140

@@ -307,7 +307,7 @@ class CommandObjectFrameSelect : public CommandObjectParsed {
307307
uint32_t frame_idx = UINT32_MAX;
308308
if (m_options.relative_frame_offset) {
309309
// The one and only argument is a signed relative frame index
310-
frame_idx = thread->GetSelectedFrameIndex();
310+
frame_idx = thread->GetSelectedFrameIndex(SelectMostRelevantFrame);
311311
if (frame_idx == UINT32_MAX)
312312
frame_idx = 0;
313313

@@ -361,7 +361,7 @@ class CommandObjectFrameSelect : public CommandObjectParsed {
361361
return false;
362362
}
363363
} else if (command.GetArgumentCount() == 0) {
364-
frame_idx = thread->GetSelectedFrameIndex();
364+
frame_idx = thread->GetSelectedFrameIndex(SelectMostRelevantFrame);
365365
if (frame_idx == UINT32_MAX) {
366366
frame_idx = 0;
367367
}
@@ -371,7 +371,7 @@ class CommandObjectFrameSelect : public CommandObjectParsed {
371371
bool success = thread->SetSelectedFrameByIndexNoisily(
372372
frame_idx, result.GetOutputStream());
373373
if (success) {
374-
m_exe_ctx.SetFrameSP(thread->GetSelectedFrame());
374+
m_exe_ctx.SetFrameSP(thread->GetSelectedFrame(SelectMostRelevantFrame));
375375
result.SetStatus(eReturnStatusSuccessFinishResult);
376376
} else {
377377
result.AppendErrorWithFormat("Frame index (%u) out of range.\n",

lldb/source/Commands/CommandObjectThread.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -555,8 +555,9 @@ class CommandObjectThreadStepWithTypeAndScope : public CommandObjectParsed {
555555
} else if (m_step_type == eStepTypeOut) {
556556
new_plan_sp = thread->QueueThreadPlanForStepOut(
557557
abort_other_plans, nullptr, false, bool_stop_other_threads, eVoteYes,
558-
eVoteNoOpinion, thread->GetSelectedFrameIndex(), new_plan_status,
559-
m_options.m_step_out_avoid_no_debug);
558+
eVoteNoOpinion,
559+
thread->GetSelectedFrameIndex(DoNoSelectMostRelevantFrame),
560+
new_plan_status, m_options.m_step_out_avoid_no_debug);
560561
} else if (m_step_type == eStepTypeScripted) {
561562
new_plan_sp = thread->QueueThreadPlanForStepScripted(
562563
abort_other_plans, m_class_options.GetName().c_str(),
@@ -1526,7 +1527,8 @@ class CommandObjectThreadReturn : public CommandObjectRaw {
15261527
bool success =
15271528
thread->SetSelectedFrameByIndexNoisily(0, result.GetOutputStream());
15281529
if (success) {
1529-
m_exe_ctx.SetFrameSP(thread->GetSelectedFrame());
1530+
m_exe_ctx.SetFrameSP(
1531+
thread->GetSelectedFrame(DoNoSelectMostRelevantFrame));
15301532
result.SetStatus(eReturnStatusSuccessFinishResult);
15311533
} else {
15321534
result.AppendErrorWithFormat(

lldb/source/Commands/CommandObjectType.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2804,7 +2804,8 @@ class CommandObjectFormatterInfo : public CommandObjectRaw {
28042804
return false;
28052805
}
28062806

2807-
StackFrameSP frame_sp = thread->GetSelectedFrame();
2807+
StackFrameSP frame_sp =
2808+
thread->GetSelectedFrame(DoNoSelectMostRelevantFrame);
28082809
ValueObjectSP result_valobj_sp;
28092810
EvaluateExpressionOptions options;
28102811
lldb::ExpressionResults expr_result = target_sp->EvaluateExpression(

lldb/source/Core/Debugger.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1665,6 +1665,7 @@ void Debugger::HandleProcessEvent(const EventSP &event_sp) {
16651665
// Display running state changes first before any STDIO
16661666
if (got_state_changed && !state_is_stopped) {
16671667
Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
1668+
DoNoSelectMostRelevantFrame,
16681669
pop_process_io_handler,
16691670
// BEGIN SWIFT
16701671
pop_command_interpreter
@@ -1708,6 +1709,7 @@ void Debugger::HandleProcessEvent(const EventSP &event_sp) {
17081709
// Now display any stopped state changes after any STDIO
17091710
if (got_state_changed && state_is_stopped) {
17101711
Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
1712+
DoNoSelectMostRelevantFrame,
17111713
pop_process_io_handler,
17121714
// BEGIN SWIFT
17131715
pop_command_interpreter

lldb/source/Core/IOHandlerCursesGUI.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5258,7 +5258,8 @@ class ThreadsTreeDelegate : public TreeDelegate {
52585258
for (size_t i = 0; i < num_threads; ++i) {
52595259
ThreadSP thread = threads.GetThreadAtIndex(i);
52605260
if (selected_thread->GetID() == thread->GetID()) {
5261-
selected_item = &root[i][thread->GetSelectedFrameIndex()];
5261+
selected_item =
5262+
&root[i][thread->GetSelectedFrameIndex(SelectMostRelevantFrame)];
52625263
selection_index = selected_item->GetRowIndex();
52635264
return;
52645265
}
@@ -6410,7 +6411,8 @@ class ApplicationDelegate : public WindowDelegate, public MenuDelegate {
64106411
if (process && process->IsAlive() &&
64116412
StateIsStoppedState(process->GetState(), true)) {
64126413
Thread *thread = exe_ctx.GetThreadPtr();
6413-
uint32_t frame_idx = thread->GetSelectedFrameIndex();
6414+
uint32_t frame_idx =
6415+
thread->GetSelectedFrameIndex(SelectMostRelevantFrame);
64146416
exe_ctx.GetThreadRef().StepOut(frame_idx);
64156417
}
64166418
}
@@ -6826,7 +6828,7 @@ class SourceFileWindowDelegate : public WindowDelegate {
68266828
if (process_alive) {
68276829
thread = exe_ctx.GetThreadPtr();
68286830
if (thread) {
6829-
frame_sp = thread->GetSelectedFrame();
6831+
frame_sp = thread->GetSelectedFrame(SelectMostRelevantFrame);
68306832
auto tid = thread->GetID();
68316833
thread_changed = tid != m_tid;
68326834
m_tid = tid;
@@ -7373,7 +7375,8 @@ class SourceFileWindowDelegate : public WindowDelegate {
73737375
if (exe_ctx.HasThreadScope() &&
73747376
StateIsStoppedState(exe_ctx.GetProcessRef().GetState(), true)) {
73757377
Thread *thread = exe_ctx.GetThreadPtr();
7376-
uint32_t frame_idx = thread->GetSelectedFrameIndex();
7378+
uint32_t frame_idx =
7379+
thread->GetSelectedFrameIndex(SelectMostRelevantFrame);
73777380
exe_ctx.GetThreadRef().StepOut(frame_idx);
73787381
}
73797382
}
@@ -7412,15 +7415,16 @@ class SourceFileWindowDelegate : public WindowDelegate {
74127415
m_debugger.GetCommandInterpreter().GetExecutionContext();
74137416
if (exe_ctx.HasThreadScope()) {
74147417
Thread *thread = exe_ctx.GetThreadPtr();
7415-
uint32_t frame_idx = thread->GetSelectedFrameIndex();
7418+
uint32_t frame_idx =
7419+
thread->GetSelectedFrameIndex(SelectMostRelevantFrame);
74167420
if (frame_idx == UINT32_MAX)
74177421
frame_idx = 0;
74187422
if (c == 'u' && frame_idx + 1 < thread->GetStackFrameCount())
74197423
++frame_idx;
74207424
else if (c == 'd' && frame_idx > 0)
74217425
--frame_idx;
74227426
if (thread->SetSelectedFrameByIndex(frame_idx, true))
7423-
exe_ctx.SetFrameSP(thread->GetSelectedFrame());
7427+
exe_ctx.SetFrameSP(thread->GetSelectedFrame(SelectMostRelevantFrame));
74247428
}
74257429
}
74267430
return eKeyHandled;

0 commit comments

Comments
 (0)